[brettheap]

Lane: openreposhape-4 (openRepoShape-4)
Claim: #422 (comment)
Claim: #428 (comment)

What changed and why

The update-shape rollback pair, one pull request by the 2026-10-14 triage. Three commits: #422, #428, then the fold of R560's review (below).

#422: a file with a second, hard-linked name. Rollback.write opens every file apply rewrites r+b and overwrites its bytes in place. So when a pinned copy, contracts/shape-pin.yaml, the manifest or (since #511) the root's own .gitattributes was kept as a hard link, its other name was rewritten too: a file outside the root, or a cp -al snapshot of the whole root, that no check showed, no pin covers and git does not record. Only the README's reader asked (#152).

• One question, hard_links: os.lstat(path).st_nlink > 1 for a regular file. A symlink is still update-shape.py: a pinned copy, contracts/shape-pin.yaml or the manifest kept as a symlink, or under one, is refused as update-rewrite-symlink before anything is written, and check calls such a copy copy-is-link, so apply no longer writes the standard's bytes at the link's end and reports a commit that does not hold them (#286) #347's, asked first.
• apply asks it before confirm and before any write, of every file it will rewrite: each upstream-changed copy, the pin, the manifest, and .gitattributes when --add would extend its tests-windows never executes an adoption: git-filter-repo is not installed there, so the eight adopt suites skip and v1.0.0's "adopt works on Windows" is unverified #471 block (asked of extend_eol_block without writing). It refuses them together as update-rewrite-hardlink, exit 2, before update-conflict and update-local-drift.
• The remediation prints, per file, a POSIX-quoted cp -p -- <f> <f>.update-shape-copy && mv -f -- <f>.update-shape-copy <f>: a copy of the file put in its place, so the other name keeps its own bytes. git sees no change, so there is nothing to commit before the same command is run again.
• Rollback.write asks the same question after its symlink walk, for a name that arrives while confirm waits; the refusal arm puts back what was already written.
• Not by replace: os.replace would leave the other name holding the old bytes and the link broken, which no rollback puts back. That is the README guard's own reason.
• check says so in one REFUSAL AHEAD line after the conflicts one. No new row state: a hard-linked copy holds the right bytes, so it is not drift.
• README.md is not asked: its reader still keeps a hard-linked README (kept README.md: another name points at these same bytes, untouched).

#428: a rollback that cannot put a byte back. undo restored with a bare write_bytes and removed what --add made with a bare unlink and rmdir. One OSError escaped _rolled_back and main as a traceback, exit 1, with the refusal lost and the paths after it never tried.

• Every restore and every removal is attempted, each under its own except OSError. The refusal that started the rollback is still the one reported, exit 2, with ROLLED BACK IN PART under its detail; a CommandFailed start prints with its step, as before.

• A file is written back only where it no longer holds its old bytes, as bump-leg.py's _restore_file does (bump-leg.py _apply_rewrite rolls back only on Refusal and CommandFailed, so a PermissionError on a workflow file it rewrites, or the UnicodeEncodeError that #230's errors="replace" moves the decode crash to, leaves the pin rewritten and the leg on the new commit, with a traceback and no rollback #253).

• Nothing is written or removed at or through a symlink, or written through a second name, that reached a path after this run wrote it (the review fold). Such a path is listed, so the block never says "exactly as it was" while one is left.

• A restore that fails is listed under NOT put back with a line:

  • git -C <root> --literal-pathspecs checkout HEAD -- <path>, where that writes exactly the old bytes and changes nothing else: HEAD commits a blob there, the index holds the same blob, cat-file --filters of it equals the old bytes, and the directory takes a new file (a checkout unlinks and creates).
  • Otherwise the old bytes go into the object store with git hash-object -w --stdin, and the line is git -C <root> cat-file blob <id> > <path>, which writes in place.
  • Where git keeps nothing either, the line says the bytes are nowhere this run can point at.
  • For a path a link reached: a symlink is left to the operator, with the line to run once it is not a link; a second name is broken from this side first, with update-rewrite-hardlink's cp -p / mv -f line.

• The HEAD claim, measured on main 5ed3e5d before building on it (in-process, red validators, ledger read at undo):

  • a --branch run over a clean tree held HEAD's bytes at every path it wrote (copy, pin, manifest);
  • an apply without --branch over an earlier in-place apply held HEAD's bytes at none of the three;
  • a --branch run accepts a CRLF working-tree manifest (hash-object normalises it to HEAD's blob) whose checkout would be LF.

  So the question is asked per path, by bytes, as a checkout would write them.

• A removal that fails is rm -- <path> or rmdir -- <path>. A directory still holding an --add file that would not go is listed after it, not kept for "something this run did not write".

• _said prints nothing under Put back: when nothing was.

Review folded (R560, APPROVE WITH NITS at 5d542c1)

R560's findings, as the coordinator relayed them, are folded in the third commit:

• SHOULD-1: the index condition of the checkout line had no test (its mutant survived 34/34). A test now stages another blob at the path: the line keeps the bytes in the object store, never the checkout, and the index is left as it was.
• SHOULD-2: the undo wrote through a symlink a validator made mid-run and called the tree "exactly as it was" (main does the same). _restore now asks symlink_on_the_way and hard_links first, as the forward write does, and lists such a path with a true line. _remove never unlinks or removes through a link it did not make (update-shape.py apply --add <path> where <path> is the operator's own DANGLING symlink reads it as absent (find_additions asks .exists()) and writes THROUGH it, so when the project's validators go red the rollback unlinks the operator's symlink, leaves the file it wrote at the link's target outside the root, and prints that the tree is exactly as it was #239's hazard). Tests cover a symlink, a second name and a link at an --add path.
• NIT-1: with contracts/ at 0555, the checkout line failed however writable the pin was. It is offered only where the directory takes a new file; elsewhere the in-place cat-file line. Tested POSIX-only.
• NIT-2: two behaviours are pinned: a file already holding its old bytes is not written again, and a linked .gitattributes whose block already names the --add file is not refused.
• NIT-3: two comments this PR had made false are corrected: ReadmeShapeLine._classify's, and tests/test_update_shape_add_symlink.py:387-388.
• NIT-4 (docs), NIT-5 and NIT-6 are left, as the coordinator ruled; README.md, AGENTS.md and the handbook are untouched.

Verification folded (round 2, at 7b68d80 and e0c7185)

The verification of the third commit (R560v at 6d43f6e, APPROVE WITH NITS) asked for five items. They are the fourth and fifth commits:

• SHOULD-3: a test where contracts/ is moved out of the root and linked back (WINDOWS_SKIP). The undo never writes through it; it lists the path with a line to run "once contracts is this root's own directory again". The test pastes that line after moving the directory back.
• SHOULD-4: the same for a directory linked in above an --add path. Nothing is removed under it.
• NIT-7: a validator that deletes contracts/ gets the checkout line again. The write question is asked of the nearest directory that exists, since git makes the rest. "Make it writable" is said only of a file that is there. The object-store line for such a path is prefixed mkdir -p <dir> && (fifth commit).
• NIT-8: that "own directory again" wording, asserted in SHOULD-3's test.
• Sonar S9073: nine composite asserts are split, each half with its own message.

R560v verified e0c7185: APPROVE WITH NITS. The two NITs it left are optional, and neither is worth a CI cycle on its own:

• NIT-A: nothing pins the lexists choice behind "make it writable", though R560v showed by a real run that the alternative prints a line that fails.
• NIT-B: a residual corner that is not a regression.

CI at e0c7185 was green on all three OSes, and SonarCloud showed 0 open issues.

Tests

New: tests/test_update_shape_hardlink.py (#422) and tests/test_update_shape_partial_rollback.py (#428). The #428 tests run in process and patch Path.write_bytes, unlink or rmdir for one path, only while undo runs, or have the stubbed validators do what a validator could (put a file back, move it out and link it, give it a second name); nothing is patched at the write. One POSIX-only test chmods the pin and contracts/, where the permission is the point. os.link needs no privilege on NTFS, so the hard-link tests run on all three platforms; the sh -c pastes and the symlink cases are WINDOWS_SKIP.

The brief's targeted set at the final head (the two new files, test_update_shape.py, _rewrite_symlink, _add_ignored, _add_symlink, _add, _landing, _merged_away, test_shape_doctor.py, test_repo_hygiene.py, test_cli_reference.py, test_agents_md_codes.py): 820 passed, 6 skipped, 0 failed in 9:48 on Linux (Python 3.12.3, pytest 9.1.1, git 2.43.0, git-filter-repo on PATH). The six skips are Windows-only: three NTFS junction tests and three PowerShell-line tests. The new files hold 42 tests (24 + 18). At the first head the same set was 812 passed, 6 skipped. CI's three OSes are the gate.

Mutants, each run over the two new files at the final head (42 tests):

• W1, drop the hard-link check in apply: 9 fail. The four late-arrival tests still pass: Rollback.write refuses there, which is their job.
• W2, restore the bare write_bytes in undo: 14 fail, on the PermissionError traceback or the write through a link.
• M1, drop the index condition of the checkout line: 1 fails (the staged-blob test).
• M2, write a file that already holds its old bytes: 1 fails.
• M3, refuse a linked .gitattributes whose block --add leaves as it is: 1 fails.
• No symlink check in _restore: 1 fails. No second-name check in _restore: 2 fail. No link guard in _remove: 1 fails. No directory condition on the checkout line: 1 fails.
• At the first head, also: stopping the restore loop after the first failure failed 4; taking not passed, the check line removed, the removal handler dropped, and cat-file blob for --filters each failed at least 1.

Not covered end to end: .gitattributes with --add on a real adopted root. The test stands in with a scaffolded root whose .gitattributes carries the #471 block and whose pin row was moved to it, because an adoption costs a filter-repo run.

Shared files

update-shape.py is shared with lane 4's P8 (#431/#437/#535) and P4 (#536), on other lines. This PR touches:

• the module docstring;
• link_refusal's docstring and one comment in ReadmeShapeLine._classify (sentences this PR made false);
• one line in cmd_check, after the conflicts REFUSAL AHEAD;
• Rollback.write, undo, _said and new helpers in the class;
• a block of new functions after _refuse_linked_copies;
• one line after _refuse_linked_copies(rows) in cmd_apply.

One comment changes in tests/test_update_shape_add_symlink.py. No import changed. git merge-tree --write-tree of this head is clean against main 5ed3e5d and against main b8b7062 (#518, which landed during the review round and changes nothing this PR uses in shape_materialize.py), against lane 3's #554 head 4619757, and against #558 (fix/update-shape-small-fixes) at fabdd7e. Every helper the new tests import from test_update_shape.py, test_update_shape_add.py and conftest.py is still defined on #554's head.

The doctor is unchanged, and a hard-linked copy is not drift.

Since then, #562 (P4) and #558 (P8) have landed. This branch took two clean merges of main for ruling 27, with no hand resolution: bbcfd34 at f10824b and 64fc9a8 at a541b04. Each merge commit's tree equals git merge-tree --write-tree's. At a541b04, every tests/test_update_shape*.py plus test_update_add_codes_documented.py, test_repo_hygiene.py, test_agents_md_codes.py and test_cli_reference.py gave 734 passed and 3 skipped, all Windows-only (NTFS junctions).

v1.1.0.

Closes #422
Closes #428

🤖 Generated with Claude Code

[sourcery-ai]

Reviewer's Guide

The PR protects apply from corrupting external or snapshot files through hard links and hardens rollback so individual restore/removal failures no longer mask the original refusal or prevent subsequent cleanup, with comprehensive in-process and cross-platform coverage.

Sequence diagram for hard-link protection during apply

sequenceDiagram
    participant User
    participant Apply as apply
    participant Check as hard_linked_rewrites
    participant FS as Filesystem
    participant Rollback as Rollback.write

    User->>Apply: cmd_apply()
    Apply->>Check: hard_linked_rewrites(root, rows, kind, taking)
    Check->>FS: hard_links(path)
    FS-->>Check: link count
    alt hard-linked rewrite found
        Check-->>Apply: hard_link_refusal()
        Apply-->>User: update-rewrite-hardlink, exit 2
    else no hard-linked rewrite
        Apply->>Rollback: write(path, data)
        Rollback->>FS: hard_links(path)
        alt link arrives while confirm waits
            Rollback-->>Apply: hard_link_refusal(arrived=True)
        else safe to write
            Rollback->>FS: overwrite bytes in place
        end
    end

Loading

Sequence diagram for partial rollback after restore failures

sequenceDiagram
    participant Apply
    participant Rollback as Rollback.undo
    participant Restore as _restore
    participant Remove as _remove
    participant FS as Filesystem

    Apply->>Rollback: undo()
    loop each written path
        Rollback->>Restore: _restore(path, original, done, left)
        Restore->>FS: read current bytes
        alt bytes differ
            Restore->>FS: write_bytes(original)
            alt restore fails
                Restore->>Rollback: append NOT put back command
            else restore succeeds
                Restore->>Rollback: append restored status
            end
        else original bytes already held
            Restore->>Rollback: append restored status
        end
    end
    loop each created path in reverse
        Rollback->>Remove: _remove(path, stuck, done, left)
        Remove->>FS: unlink() or rmdir()
        alt removal fails
            Remove->>Rollback: append NOT put back command
        else removal succeeds
            Remove->>Rollback: append removed status
        end
    end
    Rollback-->>Apply: _said(done, left)
    Apply-->>Apply: retain original refusal and exit 2

Loading

File-Level Changes

Change	Details	Files
Prevent in-place rewrites from mutating files through hard links.	Detect regular files with multiple directory entries using lstat and st_nlink. Refuse all files that apply would rewrite before confirmation or writes, including pinned copies, pin, manifest, and applicable .gitattributes. Add a late-arrival guard in Rollback.write and preserve the existing README hard-link behavior. Print POSIX-quoted copy-and-replace remediation commands while leaving row verdicts unchanged. Advertise hard-link refusals during check without treating them as drift.	update-shape.py tests/test_update_shape_hardlink.py
Make rollback resilient to restore and removal failures while preserving actionable recovery instructions.	Attempt every file restoration and created-path removal independently instead of aborting on the first OSError. Avoid rewriting files that already contain their original bytes. Report failed restores with either a filtered HEAD checkout command, an object-store cat-file command, or an explicit unavailable-bytes message. Report failed --add file and directory removals with rm/rmdir commands, including dependent directories after failed files. Retain the initiating refusal and exit status, distinguish partial rollback, and print nothing when no rollback action succeeded.	update-shape.py tests/test_update_shape_partial_rollback.py

Assessment against linked issues

Issue	Objective	Addressed	Explanation
#422	Prevent apply from rewriting any file it will modify in place when that file has multiple hard links, refusing before confirmation or any write and identifying the affected paths with a remedy.	✅
#422	Have check warn when apply would refuse because a file to be rewritten has another hard-linked name, without changing the row verdicts.	✅
#422	Recheck for hard links at the write itself so a link created after the initial check cannot be written through.	✅
#428	Make byte rollback resilient to individual restore and --add removal failures: catch OSError per path, continue attempting all remaining paths, and report every item that could not be restored or removed with a practical manual recovery command where possible.	✅
#428	Preserve and report the failure that initiated rollback, including its refusal details and exit status 2, while attaching a “ROLLED BACK IN PART” report that distinguishes completed rollback actions from paths left for manual recovery.	✅

Possibly linked issues

• update-shape.py Rollback.undo restores each rewritten file with a bare write_bytes and removes what --add made with rmdir and unlink, none under a handler, so an apply that fails after its first write and then cannot put one file back (a copy that became unwritable, a full disk, a lock) ends in an OSError traceback, exit 1, with the refusal that started the rollback lost and the paths after it never attempted, leaving a re-pinned manifest over a restored pin #428: PR update-shape.py Rollback.undo restores each rewritten file with a bare write_bytes and removes what --add made with rmdir and unlink, none under a handler, so an apply that fails after its first write and then cannot put one file back (a copy that became unwritable, a full disk, a lock) ends in an OSError traceback, exit 1, with the refusal that started the rollback lost and the paths after it never attempted, leaving a re-pinned manifest over a restored pin #428 catches rollback OSErrors per path, continues remaining work, names failed paths with recovery commands, and preserves the initiating refusal.

---

Tips and commands

Interacting with Sourcery

• Trigger a new review: Comment @sourcery-ai review on the pull request.
• Continue discussions: Reply directly to Sourcery's review comments.
• Generate a GitHub issue from a review comment: Ask Sourcery to create an
  issue from a review comment by replying to it. You can also reply to a
  review comment with @sourcery-ai issue to create an issue from it.
• Generate a pull request title: Write @sourcery-ai anywhere in the pull
  request title to generate a title at any time. You can also comment
  @sourcery-ai title on the pull request to (re-)generate the title at any time.
• Generate a pull request summary: Write @sourcery-ai summary anywhere in
  the pull request body to generate a PR summary at any time exactly where you
  want it. You can also comment @sourcery-ai summary on the pull request to
  (re-)generate the summary at any time.
• Generate reviewer's guide: Comment @sourcery-ai guide on the pull
  request to (re-)generate the reviewer's guide at any time.
• Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
  pull request to resolve all Sourcery comments. Useful if you've already
  addressed all the comments and don't want to see them anymore.
• Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
  request to dismiss all existing Sourcery reviews. Especially useful if you
  want to start fresh with a new review - don't forget to comment
  @sourcery-ai review to trigger a new review!

Customizing Your Experience

Access your dashboard to:

• Enable or disable review features such as the Sourcery-generated pull request
  summary, the reviewer's guide, and others.
• Change the review language.
• Add, remove or edit custom review instructions.
• Adjust other review settings.

Getting Help

• Contact our support team for questions or feedback.
• Visit our documentation for detailed guides and information.
• Keep in touch with the Sourcery team by following us on X/Twitter, LinkedIn or GitHub.

[sonarqubecloud]

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

[sourcery-ai]

Sorry @brettheap, you've used your own review budget of 250,000 diff characters for the last 7 days.

You can request another review in 2 days and 12 hours by commenting @sourcery-ai review. Upgrade to get a review now.