Rectify output replacement recovery and header evidence - #320
Conversation
…d swap Two P1 review findings on the merged #305 (4a2d5f1), both in write_outputs's atomic-replace path for an existing destination. write_outputs() staged a symlinked destination by renaming a temp file onto the *link's own name*: `os.replace(temporary, path)` where `path` was the symlink. `os.replace` does not follow a symlink at the destination -- it replaces the link itself, the same as `unlink` would -- so writing through a symlinked --out or --manifest silently turned a live link into a plain file, leaving whatever else reads through that link looking at stale content forever. It now resolves through the link first: the sibling temp file is staged next to, and the final rename targets, `os.path.realpath(path)`, so the swap lands on the link's target and the link itself is untouched. Windows behaviour is unchanged -- an existing destination is still refused there rather than staged, symlink or not. Second half: the final swap-in loop (`for temporary, path in staged: os.replace(temporary, path)`) ran *after* the try/except that covers claiming and writing, so a second target's swap failing left an already- succeeded first target's swap in place with no rollback -- a mid-sequence failure undid only the destinations it had not yet reached. Each existing destination is now backed up to a sibling name (an atomic rename, so it is never briefly missing) immediately before its swap, and the swap itself moved inside the same try; on any failure every backup made so far is restored, not only the one in progress, and a reserved-but-unused backup name is discarded rather than left behind. New assertions that would fail if either fix were reverted: * `test_write_outputs_writes_through_a_symlinked_destination` creates a real symlink, writes through it, and asserts the link itself survives (`link.is_symlink()`) and its target received the payload. * `test_a_failed_swap_rolls_back_every_staged_replacement` stages two existing destinations, lets the first's swap land, fails the second's, and asserts the first destination is restored to its pre-run content with no stray `.part`/`.bak` file left behind. Ran `python3 scripts/bank_statement_import.test.py`: 49 tests passed on origin/master before this change; 51 after, the two new ones included. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
P1 review finding on the merged #312 (80360d3): test_account_binding was supposed to prove the account binding rejects any *other* header field's number, but only ever exercised one field (the customer id, "4230") -- so the test would still pass against a version that read the whole header block, as long as it happened to land on a field the test never tried. Verified against the real capture first, to be precise about what it can and cannot prove: in scripts/fixtures/hdfc-bbox-capture.xml, every header field except phone/MICR sanitises to an unbroken run of the same digit as the real account number, so its own tail is a *suffix* of the correct answer and cannot be used as a wrong value there without also matching it; phone and MICR themselves sanitise to the identical tail ("1112"). So the real capture can supply at most one genuinely distinct wrong header value, and test_parse_real_hdfc_capture's comment claiming five ("phone, customer id, IFSC digits, MICR, postcode") was wrong -- three of its five loop values (1113-1115) are real, but from the transaction table, not the header. Fixed the comment to say so; the assertions themselves were not weakened, all five values are still real and are still refused. The discriminating test the real capture cannot provide belongs on the constructed-page fixture instead (this file's own stated tier for exactly this case). HDFC_PAGE now carries four more header lines -- phone, IFSC, MICR, postcode -- each with its own number, and test_account_binding checks each one's tail is refused independently rather than trying the customer id five times. New assertion that would fail if this were reverted: mutating `HDFC.account_anchors` to match the phone line instead of "Account No" (simulating a version that reads the wrong header line) still passed the old test -- confirmed by running it against that mutation before writing this fix. The new phone/IFSC/MICR/postcode cases in test_account_binding catch it: `require_account_match` incorrectly succeeds against the mutated anchor and `refuses()` raises `AssertionError`. Ran `python3 scripts/bank_statement_import.test.py`: 51 tests passed, same count as after the previous commit -- this changes what two existing tests assert, not how many tests exist. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5199f7c30b
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
Three findings Codex raised against this code — but posted on #319, because that PR had accidentally swept your uncommitted working-tree changes into its diff. #319 is now rebuilt with only its two-line redaction, so these have nowhere to live but here. They have not been assessed against this PR’s own description yet. P1 — Bind swaps to the path that passed collision validation (
This one deserves a careful read: it is a TOCTOU between your P2 — Keep the destination present while creating its backup (~1575)
The guarantee is stated in the function’s own docstring, so either the code or the docstring has to move. P2 — Surface failures to remove sensitive backups (~1600)
A stale copy of a client’s bank data under a name nobody knows, with a clean exit code. That is the worst half of it — not the leftover file, the successful report. Two notes from my side, both mine to own rather than yours:
Please treat the P1 as the priority; the other two are real but bounded. |
Three sessions hit the same defect independently in one day: a pinned file's bytes moved and the compatibility surface was not resealed. Once by editing, once by running `cargo fmt` *after* resealing, and once by rebasing — where the rebase takes the base's manifest and the author touches nothing. That spread of causes is the point. The rule people had written down was "the reseal is the last step before `git add`", and each of us broke it while believing we were following it, because each filed it under the *situation* we had just been in rather than under the actual invariant: **any operation that can change the bytes of a pinned file — an edit, a formatter, a merge, a rebase — invalidates the seal, and the reseal runs after the last of them.** Nothing in the local loop re-reads pins before a commit, so CI's gate is the only thing that notices, and every instance therefore reaches a reviewer instead of its author. That makes it a class, not a set of mistakes, and a class is worth closing here rather than writing down again. The check needs no checkout: read the 211 pinned paths from the manifest at the PR head, intersect with the PR's changed files, and require the manifest to have moved if any of them did. It is deliberately weaker than CI's gate and says so: it proves the reseal was *performed*, not that the hashes are *right*. Only the real gate proves that. But every instance observed was a reseal that never ran at all, so this catches the whole observed failure while costing one API call. Verified in three directions rather than two: a pinned file changed without the manifest blocks; the same change with the manifest passes; a PR touching the manifest alone has nothing to reseal and passes. Against live PRs, #314 reports one pinned file with the manifest moved alongside it, and #320 reports nothing pinned. Credit where due — this was suggested by the lane on #288, which had just been bitten by the `cargo fmt` variant, on the grounds that closing the class beats closing the instances. It was right. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Three sessions hit the same defect independently in one day: a pinned file's bytes moved and the compatibility surface was not resealed. Once by editing, once by running `cargo fmt` *after* resealing, and once by rebasing — where the rebase takes the base's manifest and the author touches nothing. That spread of causes is the point. The rule people had written down was "the reseal is the last step before `git add`", and each of us broke it while believing we were following it, because each filed it under the *situation* we had just been in rather than under the actual invariant: **any operation that can change the bytes of a pinned file — an edit, a formatter, a merge, a rebase — invalidates the seal, and the reseal runs after the last of them.** Nothing in the local loop re-reads pins before a commit, so CI's gate is the only thing that notices, and every instance therefore reaches a reviewer instead of its author. That makes it a class, not a set of mistakes, and a class is worth closing here rather than writing down again. The check needs no checkout: read the 211 pinned paths from the manifest at the PR head, intersect with the PR's changed files, and require the manifest to have moved if any of them did. It is deliberately weaker than CI's gate and says so: it proves the reseal was *performed*, not that the hashes are *right*. Only the real gate proves that. But every instance observed was a reseal that never ran at all, so this catches the whole observed failure while costing one API call. Verified in three directions rather than two: a pinned file changed without the manifest blocks; the same change with the manifest passes; a PR touching the manifest alone has nothing to reseal and passes. Against live PRs, #314 reports one pinned file with the manifest moved alongside it, and #320 reports nothing pinned. Credit where due — this was suggested by the lane on #288, which had just been bitten by the `cargo fmt` variant, on the grounds that closing the class beats closing the instances. It was right. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3c13be14c7
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
lamemustafa
changed the title
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1bec9e6a99
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Three sessions hit the same defect independently in one day: a pinned file's bytes moved and the compatibility surface was not resealed. Once by editing, once by running `cargo fmt` *after* resealing, and once by rebasing — where the rebase takes the base's manifest and the author touches nothing. That spread of causes is the point. The rule people had written down was "the reseal is the last step before `git add`", and each of us broke it while believing we were following it, because each filed it under the *situation* we had just been in rather than under the actual invariant: **any operation that can change the bytes of a pinned file — an edit, a formatter, a merge, a rebase — invalidates the seal, and the reseal runs after the last of them.** Nothing in the local loop re-reads pins before a commit, so CI's gate is the only thing that notices, and every instance therefore reaches a reviewer instead of its author. That makes it a class, not a set of mistakes, and a class is worth closing here rather than writing down again. The check needs no checkout: read the 211 pinned paths from the manifest at the PR head, intersect with the PR's changed files, and require the manifest to have moved if any of them did. It is deliberately weaker than CI's gate and says so: it proves the reseal was *performed*, not that the hashes are *right*. Only the real gate proves that. But every instance observed was a reseal that never ran at all, so this catches the whole observed failure while costing one API call. Verified in three directions rather than two: a pinned file changed without the manifest blocks; the same change with the manifest passes; a PR touching the manifest alone has nothing to reseal and passes. Against live PRs, #314 reports one pinned file with the manifest moved alongside it, and #320 reports nothing pinned. Credit where due — this was suggested by the lane on #288, which had just been bitten by the `cargo fmt` variant, on the grounds that closing the class beats closing the instances. It was right. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0ebb774286
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
https://github.com/lamemustafa/bridge/blob/06953660dc7745cd939f7404fa6b924e7b17750c/scripts/b_import.py.py#L1962-L1963
The recorded macOS validation is explicitly at 0ebb774, but the committed-cleanup and interrupt-reconciliation path here was added afterward in 5e05407 and then included in this reviewed squash. Fresh evidence therefore shows that the final platform-sensitive filesystem behavior was not what the cited macOS run exercised; rerun the contracts at this revision or explicitly disclose the remaining macOS evidence gap.
AGENTS.md reference: AGENTS.md:L195-L196
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
💡 Codex Reviewbridge/scripts/bank_statement_import.py Lines 1862 to 1863 in 24d9aa3  Report failed Windows alias inspection
On Windows, if AGENTS.md reference: AGENTS.md:L162-L167 bridge/scripts/bank_statement_import.py Line 2573 in 24d9aa3 Translate a vanished pending backup into a refusal
If the current AGENTS.md reference: AGENTS.md:L110-L118 bridge/scripts/bank_statement_import.py Lines 2152 to 2154 in 24d9aa3 Reconcile moved backups when preserving foreign destinations
When an already-swapped destination is replaced by a foreign inode and its private backup is also renamed before a later failure, this branch records only the now-stale AGENTS.md reference: AGENTS.md:L162-L167 ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
|
Addressed the three findings in pushed head
The three focused regressions and the complete importer suite pass: 158 offline controls, exit 0. The retained receipt is |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: eaccd861d0
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 075b0b8a6f
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6094269b58
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6f1e53fa06
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: fcf9d780db
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3e20092b71
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 969c6e273f
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5d899304e2
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3759f245c3
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
master moved when #320 landed, which stales the pinned digests on this branch. The only conflicts were the two compatibility manifests; the pin sets were identical on both sides (212 entries, none added or removed), so master's sealed manifest was taken and the digests recomputed from the merged tree rather than hand-edited. Reseal ran in the documented order: rehash-surface reported 4 changed entries, matching exactly the four pinned files this branch modifies (TALLY_PROTOCOL_REFERENCE.md, master_binding.rs, agent_import.rs, source_draft/catalog.rs). A confirming rehash after seal-surface and repoint-matrix reports 0 changed entries. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
1 participant
Addresses #317 review debt in the offline statement writer.
Functional summary
The writer opens and registers each existing destination before creating any staged sibling, derives its identity from that retained descriptor, and verifies that the pathname still names it before the claim gains overwrite authority. It does this before payload generation, keeps the destination present until atomic replacement, and retains an exclusive owner-only rollback copy. Open descriptors pin original, staged and backup ownership through recovery. Before replacement it rechecks supplied path resolution, original identity and link count, staged identity and backup ownership. Multiple hard links are refused to preserve topology.
Fresh outputs are created through the captured canonical path. Every supplied spelling, including an existing destination reached through a symlink, must resolve to the owned inode at the final transaction boundary. All fresh and replaced outputs and every rollback copy are revalidated after the last swap. Earlier outputs cannot disappear or become foreign while a later output is being prepared without a refusal. If a fresh path disappears, is replaced or has its symlinked parent retargeted, the writer refuses and rolls back prior swaps. Cleanup removes only the recorded owned canonical entry and preserves foreign paths. If an actual parent directory moves, a still-linked pinned fresh output that cannot be found is reported explicitly; an already unlinked inode does not produce a phantom retained-output warning. Staged outputs, fresh outputs and rollback copies are rechecked for hard-link aliases before commit, with alias uncertainty distinguished from a deleted pin.
One handler separates caught-error rollback from committed cleanup. Before commit it restores only still-owned destinations with a still-single-linked backup. If the backup gained an unknown hard-link alias during a later failed swap, it retains the new destination and recoverable old backup and reports the alias. Missing or changed backup pins are diagnosed separately from aliases. Foreign replacements are preserved and recoverable backups reported. After commit, cleanup failures retain new output and report old copies or descriptor-close failures separately. Registration failures close owned descriptors and either clean proven created paths or report uncertain retained paths. POSIX rollback restores captured bytes, mode, owner/group, timestamps and readable extended attributes, and reports unverified ACL/BSD-flag scope. Attributes and timestamps are applied before the final restrictive mode. Windows close-before-unlink cleanup reconciles a close error after taking effect against successful removal, avoiding a stale retained-path claim.
Header controls now use unchanged captured HDFC Account No and neighboring selectors. The capture has no postcode label and does not prove distinct raw values after sanitization; broader template coverage is not claimed.
Interrupted committed cleanup preserves the original escaping exception through backup-inspection failures and closes later pins, including an already-closed missing backup. Rollback uses directory-entry identity and preserves a foreign symlink even when it points to the displaced staged inode. A negative control reproduces the former symlink-following overwrite. The original-pin registration control explicitly exercises
created=False, preserves the same error object and verifies descriptor closure.Cleanup uses non-following inspection that distinguishes missing entries from inspection errors. Fresh/staged cleanup, no-swap rollback backups and fresh-output registration recovery reconcile the pinned inode after removing the owned name, including an unlink that raises after taking effect. A retained hard-link alias is disclosed before closing the pin; an inode with no links produces no phantom warning. An actual parent-directory rename reports the precise unlocated rollback-copy pathname without inventing a new authorized path. A shared path resolver translates Python 3.10–3.12 symlink-loop
RuntimeErrorintooutput_path_changed, including after outputs have already been claimed.Fresh, staged and backup creation now share one descriptor-registration helper. Its exception boundary owns the handle through identity registration, POSIX permission restoration and insertion into the cleanup collection. A failure preserves foreign pathname replacements and appends any retained-copy diagnostic to the original exception. Existing original pins use the same handoff with close-only authority. Created POSIX outputs and backups are explicitly set to mode 0600 through their descriptor, including under an umask that removes owner bits; Windows retains its existing ACL-based creation behavior.
A removed existing leaf under its original parent produces the typed
output_path_changedrefusal. Parent-rename recovery retains its uncertainty diagnostic. Both normal and interrupted committed cleanup use the canonical output path for staged-pin close diagnostics, without a separate post-commit record rewrite.Descriptor-close failures are now reconciled against the retained descriptor before being classified. Only an EBADF result confirms an after-effect close; a still-open descriptor, another inspection error or descriptor reuse is reported without a blind close retry. Original pins remain visible in descriptor diagnostics after replacement. If a rollback copy disappears, shared recovery classifies every still-committed destination, including failures during preparation of a later output and loss of multiple backups. The original exception is preserved, actual partial results are disclosed, and vanished private backup or staging names are not presented as retained files.
The verified backup digest is now retained and checked through its owned descriptor before both final commit acceptance and rollback. A same-inode content mutation or unreadable backup never grants restore authority. Integrity checks preserve the descriptor offset and introduce no extra descriptor. Unavailable-backup handling inspects the pinned inode before closing it: zero links produce no phantom path, a moved single-link copy is reported as unlocated, and unknown aliases remain explicit. The original exception and each actual partial destination are preserved.
Created files cannot escape ownership recovery between factory completion and registration: POSIX defers SIGINT across that handoff. Existing-path revalidation now fails with the typed path-change refusal. A restore which fails before replacing staged bytes reports both its retained backup and the actual partial destination. Interrupted committed cleanup inspects a live rollback pin for aliases before release; an already-closed pin retains its known path without an invalid descriptor operation.
The no-pthread-signal-mask fallback now retains Ctrl-C until created-file ownership has been registered, then restores and delivers the caller handler. Windows close-before-unlink cleanup inspects a live pin for hard-link aliases before close. A post-copy disappearance or uninspectable original path becomes the typed
output_path_changedrefusal. An alias on a rollback backup marks the new destination as a partial commit, and a post-effect unlink which finds the old spelling reclaimed preserves that state so pinned-inode reconciliation discloses the moved owned bytes without touching foreign content.An existing destination that vanishes before its initial pinned open now takes the same typed claim-time refusal. A rollback backup-pin inspection error marks the retained new destination as an explicit partial commit. Every fresh and staged payload is now hashed and re-read through its owned descriptor at the final authority boundary, so same-inode in-place mutation before replacement or commit refuses and cleans up only the proven owned path. Fresh private descriptors are O_RDWR solely to support this pinned read-back; no pathname is reopened for verification.
If the existing destination disappears after identity capture but before the rollback source can be opened, recovery reconciles the retained original pin before a typed
output_path_changedrefusal. Pre-replacement inspection failures for the pinned backup or original are likewise typed before any replacement is attempted, preserving the old destination and clean staging state.Any non-missing I/O failure while opening the rollback source now follows the same typed boundary and original-pin reconciliation, without masking later backup read or write failures. Final rollback backup-pin inspection failures remain typed while preserving the triggering swap failure and conservative partial-result diagnostics. A failed restore re-inspects the present destination before describing a partial output, so a foreign replacement is never named as this run's data; a zero-link staged pin is fully removed rather than reported at a reclaimed foreign pathname.
The current owner-authorized stopping rule defers four further P2 recovery hardenings to #352, #353, #354, and #355. This PR does not claim those behaviors as fixed.
After a replacement has begun, an uninspectable destination is treated as an unrollbackable partial result rather than as proof of a foreign writer. A vanished or uninspectable staged
.partentry is likewise translated intooutput_path_changed, retaining the original destination and the conservative recovery path rather than exposing a raw filesystem exception. A rejected regular-output pin now preserves its typed refusal when close also fails, and separately records the close diagnostic. A failed restore whose destination cannot be inspected discloses that partial state and retains the named private backup. A transient final backup inspection error likewise retains the named backup without retrying or replaying recovery. Immediately before each restore or existing-output commit replacement, the named backup or staged entry is revalidated against its pinned identity. A renamed and recreated pathname therefore either produces retained partial recovery or a typedoutput_path_changedrefusal; it never installs foreign bytes. Reclaimed backup names are removed from owned-path diagnostics, known backups remain named after an uninspectable post-swap destination, moved committed output pins are reconciled before close, zero-link originals produceoutput_path_changed, and restored-output handle closes use descriptor-aware recovery. The Windows-modeled reclamation path preserves a foreign replacement while reporting only the owned inode's unlocated state. Pre-commit unrollbackable output pins reconcile before close, descriptor ownership remains recorded across interruptible close handoff, vanished parents produce typed path-change refusal, and restored destinations are identity-checked after metadata handling before any success claim.Test or reproduction command
Candidate
3759f245c3969f2508e8a30426189e7e64d709e3normally integrates masterba1742cf0ec8f17c3f10d537a5bfa845739b946f:python3 scripts/bank_statement_import.test.py: all 188 offline controls passed on the final candidate. The retained run covers descriptor-close, all-swaps partial recovery, backup integrity, moved-copy disclosure, POSIX and no-pthread factory-return ownership handoff, typed initial and post-copy path revalidation, failed restore partial results, interrupted cleanup aliases, Windows-modeled alias discovery, reclaimed-path reconciliation, rollback-pin and destination-inspection partial disclosure, vanished staged and pending-backup entries, moved backup reconciliation, immediate pre-replacement identity checks, staged/fresh in-place byte mutation, preservation of typed rejection through close failure, uninspectable failed-restore disclosure, named-backup retention after transient final inspection failure, rename/recreate races immediately before backup restore and staged replacement, reclaimed backup diagnostic suppression, moved committed-output reconciliation, zero-link classification, descriptor-aware restored-output close handling, pre-commit staged-pin reconciliation, interrupt-safe close ownership, parent disappearance typing, restored-output identity revalidation, restored-open backup reconciliation, metadata-time alias disclosure, deferred failed-close classification, typed final ownership inspection failure, rollback-source disappearance and non-missing open-error reconciliation, typed pre-replacement and final rollback backup/original-pin inspection failure, post-restore foreign replacement recognition, and zero-link staged-pin removal. Independent before/after probes reproduce altered-byte restoration and silent moved-copy retention; both pass on this final candidate. Root and independent review accepted the immutable correction. Both existing-destination controls are explicitly POSIX-only because Windows refuses this operation.python3 -m py_compile scripts/bank_statement_import.py scripts/bank_statement_import.test.pypassed.python3 scripts/sanitise-bbox-capture.test.py: unchanged sanitizer contracts passed earlier in this change; this final batch changes only the writer and its tests.git diff --checkpassed. Normal master integration retained the bank implementation and test bytes before this batch. Compatibility rehash reported zero changes; gate, exact membership and actual-byte verification passed for all 212 pins. The final batch changes only the two unpinned bank script files; their publication did not require another seal.Migration compatibility
No schema or output-format change. Existing guarded Windows behavior remains; Windows filesystem execution and extended ACL/BSD-flag preservation have not been established. Local tests ran on macOS. The Windows close-before-unlink control is modeled; it is not Windows filesystem qualification. The POSIX ordering control verifies xattrs precede fchmod. The previously published head passed the Linux workflow, but the readonly-xattr test can return if setup is unsupported; that status alone does not prove actual Linux enforcement. Current-head Linux CI and provider review remain required before merge.
Rollback notes
Reverting restores the old recovery defects; retain equivalent controls in any replacement. This is caught-exception rollback, not a multi-file crash transaction or a filesystem lock. Process/host crashes can leave private backups and mixed versions. Hostile filesystem changes after identity checks remain outside the CLI's guarantee.
No live Tally call, customer fixture or financial mutation occurred.
Security impact
Prior statement bytes use exclusive mode-0600 POSIX backups. Failure diagnostics identify recovery locations without statement contents. Windows permission acceptance remains explicit; credential and DSC paths are unchanged.