Rectify output replacement recovery and header evidence by lamemustafa · Pull Request #320 · lamemustafa/bridge

lamemustafa · 2026-09-12T07:18:22Z

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 RuntimeError into output_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_changed refusal. 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_changed refusal. 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_changed refusal. 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 .part entry is likewise translated into output_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 typed output_path_changed refusal; 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 produce output_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 3759f245c3969f2508e8a30426189e7e64d709e3 normally integrates master ba1742cf0ec8f17c3f10d537a5bfa845739b946f:

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.py passed.
Other retained controls cover interrupted swaps/restores, ownership registration, descriptor failures, staged/fresh/backup hard-link aliases, earlier outputs changing during later swaps, actual parent renames, post-claim symlink-loop refusal, metadata restoration and retained recovery diagnostics.
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 --check passed. 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.
Root reviewed the final cleanup call graph and retained tests; independent review covers the final original-pin, rollback-alias, cleanup and metadata-order corrections. Actual SIGINT and platform-scope controls remain covered. Hosted current-head checks must complete before merge.

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.

Existing component reused: the existing staged writer and ownership cleanup helpers. No new dependency.
What is deleted: hard-link backups and path-following rollback authority are replaced by private copies and pinned directory-entry checks.
What breaks if this is not built: recovery can overwrite another writer's output or leave undisclosed retained copies.
Net LOC change: +6323 (6359 added, 36 deleted).

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.

Errors are actionable without exposing sensitive values: recovery paths are visible without logging statement contents.

…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>

chatgpt-codex-connector · 2026-09-12T07:18:29Z

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review	Status	Commit	Review trigger
📝 Code Review	✅ Completed 2026-09-13T16:46:48.712563Z	`3759f24`	New commits

ℹ️ 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" or "@codex security review".

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

💡 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".

lamemustafa · 2026-09-12T07:32:26Z

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 (bank_statement_import.py ~1545)

When an existing output is a symlink that can be retargeted during the run, this caches its target before main invokes the after_claim _check_paths(args) callback. That callback re-resolves the symlink rather than validating real_path, so changing the link from a benign target to the input PDF or mapping before this line and back before the callback passes both checks; the later swap then overwrites the cached input path despite the collision guard.

This one deserves a careful read: it is a TOCTOU between your realpath cache and the collision check, and the consequence is overwriting the input statement. Worth deciding whether a retargetable symlink should simply be rejected.

P2 — Keep the destination present while creating its backup (~1575)

this rename removes real_path until the following os.replace completes. The two renames are not atomic as a pair, so a concurrent reader can observe ENOENT, contrary to the function’s stated atomic-replacement guarantee.

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)

when the output swaps succeed but deleting a backup fails … this suppresses the error and reports a successful run while leaving the previous bank-statement output under an unreported random .bak name.

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:

The shared working tree at /Users/tapishkhandelwal/Desktop/dev/bridge was being rebased under you mid-task — that was me landing feat(tally): qualify Payment, Receipt and Contra on the import writer #289 and restacking docs(tally): four findings left open when #306 was merged too early #314/Rectify ambiguous mask preservation and recovery guidance #316, and it is why your changes were loose in that tree. You handled it correctly by moving to an isolated worktree; I should have said what I was doing first.
I then staged those two files by name while your work was in them and committed without running git diff --cached. That is how your 123 insertions ended up in a PR described as prose-only. Nothing was lost — it is all here in Rectify output replacement recovery and header evidence #320.

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>

…y-pr320-20260912

chatgpt-codex-connector

💡 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".

chatgpt-codex-connector

💡 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>

…y-pr320-20260912

chatgpt-codex-connector

💡 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".

chatgpt-codex-connector

💡 Codex Review

https://github.com/lamemustafa/bridge/blob/06953660dc7745cd939f7404fa6b924e7b17750c/scripts/b_import.py.py#L1962-L1963
Validate the final cleanup changes on macOS

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".

chatgpt-codex-connector · 2026-09-13T09:18:51Z

💡 Codex Review

bridge/scripts/bank_statement_import.py

Lines 1862 to 1863 in 24d9aa3

    
           except OSError: 
        
               return

Report failed Windows alias inspection

On Windows, if fstat fails while cleanup owns a file that has a hard-link alias, this branch silently treats the pre-close alias check as successful. Cleanup then closes the only pin and may successfully unlink the known name, leaving the alias with statement bytes while the command reports no retained-output uncertainty. Fresh evidence beyond the resolved Windows-alias thread is this new swallowed inspection-error path; record it as an uninspectable retained-output failure before releasing the descriptor.

AGENTS.md reference: AGENTS.md:L162-L167

bridge/scripts/bank_statement_import.py

Line 2573 in 24d9aa3

if _entry_identity(pending_swap["backup"]["path"]) != \

Translate a vanished pending backup into a refusal

If the current .bak is removed after _copy_private_backup returns but before this validation, _entry_identity raises FileNotFoundError directly, so recovery preserves the original destination but the CLI emits a raw traceback instead of the stable output_path_changed refusal. Fresh evidence beyond the staged-entry fix is that this separate pending-backup check remains outside exception translation; handle missing or uninspectable backup entries with the same typed refusal.

AGENTS.md reference: AGENTS.md:L110-L118

bridge/scripts/bank_statement_import.py

Lines 2152 to 2154 in 24d9aa3

    
           if current_identity != staged_identity: 
        
               failures.append(backup) 
        
               return

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 .bak spelling and returns, after which the backup pin is closed. The moved file still contains the prior statement, but the operator receives no unlocated-copy diagnostic. Fresh evidence beyond the resolved unavailable-backup path is that this foreign-destination branch returns before _mark_rollback_unavailable; reconcile the pinned backup while preserving the foreign destination.

AGENTS.md reference: AGENTS.md:L162-L167

ℹ️ 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 · 2026-09-13T09:26:29Z

Addressed the three findings in pushed head eaccd861d036f0acb41091207e52c5807b82defb.

A failed Windows pre-close alias inspection is now retained-output uncertainty before the descriptor is closed.
A missing or uninspectable pending .bak now returns the typed output_path_changed refusal.
A private backup moved while a foreign destination is preserved is reconciled from its pin and reported as an unlocated retained copy; the foreign destination remains untouched.

The three focused regressions and the complete importer suite pass: 158 offline controls, exit 0. The retained receipt is capture-2ax1e2sy (SHA-256 70c8fe2eeabfc09fed80b8d04a52fcd01032f4ff7ae20d2c7c136be084c08e9d). Windows filesystem behavior remains modeled offline and is not claimed as physical-host validation.

chatgpt-codex-connector

💡 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".

chatgpt-codex-connector

💡 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".

chatgpt-codex-connector

💡 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".

chatgpt-codex-connector

💡 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".

chatgpt-codex-connector

💡 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".

chatgpt-codex-connector

💡 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".

chatgpt-codex-connector

💡 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".

chatgpt-codex-connector

💡 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".

chatgpt-codex-connector

💡 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>

t and others added 2 commits September 12, 2026 12:46

chatgpt-codex-connector Bot reviewed Sep 12, 2026

View reviewed changes

t added 5 commits September 12, 2026 16:18

Merge master into rectify PR 320

0472052

fix(scripts): preserve output replacement recovery

e25d008

fix(scripts): revalidate output at swap boundary

37005ad

test(scripts): pin captured account header geometry

e0ca932

Merge remote-tracking branch 'origin/master' into tapish-codex/rectif…

3c13be1

…y-pr320-20260912

chatgpt-codex-connector Bot reviewed Sep 12, 2026

View reviewed changes

Comment thread scripts/bank_statement_import.py Outdated

Comment thread scripts/bank_statement_import.py Outdated

Comment thread scripts/bank_statement_import.py Outdated

Comment thread scripts/bank_statement_import.py Outdated

Comment thread scripts/bank_statement_import.py Outdated

t added 2 commits September 12, 2026 17:19

fix(import): retain owned backups on rollback conflicts

6cee197

fix(import): reconcile post-effect backup restores

1bec9e6

lamemustafa changed the title ~~fix(scripts): symlink-safe atomic replace, full rollback, distinct header-field tails~~ Rectify output replacement recovery and header evidence Sep 12, 2026

chatgpt-codex-connector Bot reviewed Sep 12, 2026

View reviewed changes

Comment thread scripts/bank_statement_import.py Outdated

Comment thread scripts/bank_statement_import.py Outdated

Comment thread scripts/bank_statement_import.py

Comment thread scripts/bank_statement_import.test.py

t added 5 commits September 12, 2026 17:39

fix(import): preserve rollback file identity

cdd9034

fix(import): pin rollback ownership records

b5dd06c

fix(import): retain claimed output ownership

f85bb02

Merge remote-tracking branch 'origin/master' into tapish-codex/rectif…

c642f98

…y-pr320-20260912

fix(import): disclose rollback metadata scope

0ebb774

chatgpt-codex-connector Bot reviewed Sep 12, 2026

View reviewed changes

Comment thread scripts/bank_statement_import.py Outdated

Comment thread scripts/bank_statement_import.py

Comment thread scripts/bank_statement_import.py Outdated

t added 2 commits September 12, 2026 19:22

fix(import): reconcile interrupted output cleanup

5e05407

merge: integrate master into import cleanup

0695366

chatgpt-codex-connector Bot reviewed Sep 12, 2026

View reviewed changes

Comment thread scripts/bank_statement_import.py

Comment thread scripts/bank_statement_import.py Outdated

Comment thread scripts/bank_statement_import.py Outdated

Comment thread scripts/bank_statement_import.py Outdated

t added 3 commits September 12, 2026 19:53

fix importer ownership registration cleanup

150cac9

preserve existing output on pin failure

009bd29

Integrate verified WebKit test baseline

9823824

Preserve bank output recovery uncertainty

eaccd86

chatgpt-codex-connector Bot reviewed Sep 13, 2026

View reviewed changes

Comment thread scripts/bank_statement_import.py

Comment thread scripts/bank_statement_import.py

Comment thread scripts/bank_statement_import.py Outdated

Revalidate bank output replacement boundaries

075b0b8

chatgpt-codex-connector Bot reviewed Sep 13, 2026

View reviewed changes

Comment thread scripts/bank_statement_import.py

Comment thread scripts/bank_statement_import.py Outdated

Comment thread scripts/bank_statement_import.py Outdated

Retain bank output recovery diagnostics

6094269

chatgpt-codex-connector Bot reviewed Sep 13, 2026

View reviewed changes

Comment thread scripts/bank_statement_import.py

Comment thread scripts/bank_statement_import.py

Revalidate bank replacement sources

6f1e53f

chatgpt-codex-connector Bot reviewed Sep 13, 2026

View reviewed changes

Comment thread scripts/bank_statement_import.py Outdated

Comment thread scripts/bank_statement_import.py Outdated

Comment thread scripts/bank_statement_import.py

Comment thread scripts/bank_statement_import.py

Comment thread scripts/bank_statement_import.py Outdated

t added 2 commits September 13, 2026 16:56

Rectify bank output recovery ownership

c7b9d2e

Cover Windows reclaimed backup cleanup

fcf9d78

chatgpt-codex-connector Bot reviewed Sep 13, 2026

View reviewed changes

Comment thread scripts/bank_statement_import.py

Comment thread scripts/bank_statement_import.py Outdated

Comment thread scripts/bank_statement_import.py Outdated

Comment thread scripts/bank_statement_import.py Outdated

t added 3 commits September 13, 2026 18:03

Harden bank recovery pin handoffs

8a96fe7

Update rollback recovery observation count

508fcdc

Update rollback inspection control count

3e20092

chatgpt-codex-connector Bot reviewed Sep 13, 2026

View reviewed changes

Comment thread scripts/bank_statement_import.py Outdated

Comment thread scripts/bank_statement_import.py

Comment thread scripts/bank_statement_import.py

Comment thread scripts/bank_statement_import.py Outdated

Comment thread scripts/bank_statement_import.py

Harden restored output recovery authority

969c6e2

chatgpt-codex-connector Bot reviewed Sep 13, 2026

View reviewed changes

Comment thread scripts/bank_statement_import.py Outdated

Comment thread scripts/bank_statement_import.py Outdated

Type pre-replacement ownership failures

5d89930

chatgpt-codex-connector Bot reviewed Sep 13, 2026

View reviewed changes

Comment thread scripts/bank_statement_import.py

Comment thread scripts/bank_statement_import.py

Comment thread scripts/bank_statement_import.py Outdated

Comment thread scripts/bank_statement_import.py Outdated

fix(bank): preserve rollback recovery evidence

3759f24

chatgpt-codex-connector Bot reviewed Sep 13, 2026

View reviewed changes

Comment thread scripts/bank_statement_import.py

Comment thread scripts/bank_statement_import.py

Comment thread scripts/bank_statement_import.py

Comment thread scripts/bank_statement_import.py

This was referenced Sep 13, 2026

Rectify staged output pin inspection typing #352

Open

Rectify final rollback-pin inspection typing #353

Open

Rectify interrupted duplicated descriptor handoff #354

Open

Rectify reclaimed fresh-path cleanup disclosure #355

Open

lamemustafa merged commit 8b17f95 into master Sep 15, 2026
11 checks passed

lamemustafa deleted the fix/atomic-replace-symlink-and-header-tails branch September 15, 2026 08:37

Conversation

lamemustafa commented Sep 12, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Functional summary

Test or reproduction command

Migration compatibility

Rollback notes

Security impact

Uh oh!

chatgpt-codex-connector Bot commented Sep 12, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codex Review Summary

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

lamemustafa commented Sep 12, 2026

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

Uh oh!

Uh oh!

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

chatgpt-codex-connector Bot commented Sep 13, 2026

💡 Codex Review

Uh oh!

lamemustafa commented Sep 13, 2026

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

Uh oh!

Uh oh!

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

Uh oh!

Uh oh!

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

lamemustafa commented Sep 12, 2026 •

edited

Loading

chatgpt-codex-connector Bot commented Sep 12, 2026 •

edited

Loading