[brettheap]

Lane: openreposhape-2 (openRepoShape-2)
Claim: #285 (comment)

Merged main after #269 landed

#269 landed on main as the squash c1b6732 (its final head was 8a460e8, which had merged main c7c02fa), and GitHub retargeted this PR to main. gh pr update-branch refused with conflicts, because the squash and this branch's copy of #269's original commits touch the same lines. Two merges, nothing else:

• 25e38af merges update-shape.py: a path the root holds a symlink or a Windows junction at, dangling or live, or under one, is present and apply --add refuses it as update-add-symlink before it writes anything, and the ledger creates an addition exclusively, so a dangling link no longer has the standard's file written through it outside the root, and a red validator no longer deletes the operator's link (#239) #269's 8a460e8 into 189c30e. No conflict.
• 818db36 merges origin/main a1b3a4b into 25e38af. One conflict, in update-shape.py, resolved as this branch's own version of the whole file (git diff 8a460e8 origin/main -- update-shape.py tests/test_update_shape_add_symlink.py is empty: c1b6732 is the only main commit touching either file since c7c02fa, and its content is 8a460e8's). tests/test_update_shape_add_symlink.py merged with no conflict.

At 818db36: git diff HEAD^1 HEAD -- update-shape.py tests/test_update_shape_add_symlink.py is empty; git diff --stat origin/main HEAD lists this PR's two files only (update-shape.py, tests/test_update_shape_add_not_a_directory.py); git merge-tree --write-tree origin/main HEAD is rc 0. Targeted counts at 818db36 are in Tests, below.

The numbers in the sections below were measured at the heads they name (ca68458, 3b9ed21, 961c68e, 189c30e) unless a line says 818db36 or a1b3a4b.

What

update-shape.py, the two changes #285 asks for, built on #269's link walk and its exclusive write_added (main holds both since c1b6732):

1. A path under something that is not a directory is refused by name before anything is written.
   • not_a_directory_on_the_way(root, rel) walks the components above the path with lstat, as update-shape.py: a path the root holds a symlink or a Windows junction at, dangling or live, or under one, is present and apply --add refuses it as update-add-symlink before it writes anything, and the ledger creates an addition exclusively, so a dangling link no longer has the standard's file written through it outside the root, and a red validator no longer deletes the operator's link (#239) #269's symlink_on_the_way does. It returns the first one the root holds as anything but a directory (a plain file, a FIFO). It stops with no answer at a link (symlink_on_the_way's, refused first) or at a component it cannot lstat.
   • find_additions keeps that answer on the Addition as blocked.
   • resolve_additions refuses it as update-add-not-a-directory, after the link check and before the merge check. The refusal names what is in the way, says to move it and run the same --add again, and exits 2.
   • check still reports the path as upstream-added (the root holds nothing at it), with a detail saying what is in the way. It no longer lists that path in ADDED UPSTREAM: ... --add <path>. apply's note for an addition it was not asked to take likewise stops saying pass --add <path> for such a path, and says what is in the way instead.
2. The write phase rolls back on OSError too.
   • Rollback.writing(step) is a context manager. It turns an OSError out of a phase that writes through the ledger into update-write-failed, naming the step, the path (relative to the root) and the error. This is the way _rewrite_readme already handles the README.
   • cmd_apply wraps _write_copies ("writing the copies and the --add files") and _repin ("re-pinning contracts/shape-pin.yaml and project.yaml") in it.
   • The refusal arm that already exists then puts back every copy, addition and directory written before the failure, and lists them. write_added records a path only once it exists (update-shape.py: a path the root holds a symlink or a Windows junction at, dangling or live, or under one, is present and apply --add refuses it as update-add-symlink before it writes anything, and the ledger creates an addition exclusively, so a dangling link no longer has the standard's file written through it outside the root, and a red validator no longer deletes the operator's link (#239) #269), so the undo never removes something the run did not make.
   • The module docstring gains one bullet saying so. No --help text changes (scripts/render-cli-reference.py --check: ok).

Why

find_additions asked only whether something was AT the path, and a path under a plain file is not there: lexists says no, because the kernel answers ENOTDIR. So check offered the path with the --add to paste, and apply --add took it. _write_copies rewrote every upstream-changed copy, then Rollback.write_added died making the directory under the file (NotADirectoryError), or creating the file in a directory it cannot write (PermissionError). cmd_apply rolls back on Refusal and CommandFailed only, so the run ended in a traceback with the copies rewritten and the pin unmoved, and the next check read those copies as already-at-target.

Reproduced first on ca68458 (#269's head when this branch was cut; main's update-shape.py is now #269's final 8a460e8 version, byte for byte) with the issue's own steps: every repository local, gh a PATH shim that exits 97 and was never called, and .github/workflows/validate.yml's row and file dropped in one commit:

.github a plain file          check: ADDED UPSTREAM ... --add .github/workflows/validate.yml
                              apply: exit 1  NotADirectoryError: [Errno 20] Not a directory: '<C>/.github/workflows'
                                     git status:  M scripts/validate-manifest.py   pin: unmoved
.github/workflows/ mode 0555  apply: exit 1  PermissionError: [Errno 13] Permission denied: '<C>/.github/workflows/validate.yml'
                                     git status:  M scripts/validate-manifest.py   pin: unmoved
nothing at .github (control)  apply: exit 0, added

and with the fix, at 3b9ed21 (the fix's first commit, before main was merged in):

.github a plain file          check: upstream-added .github/workflows/validate.yml
                                       (the standard added this file after this pin, but .github in this root is not a directory, so --add refuses it until .github is moved)
                                     no ADDED UPSTREAM line (it was the only addition)
                              apply: exit 2  REFUSED update-add-not-a-directory: --add .github/workflows/validate.yml names a file the upstream's lists add,
                                             but .github in this root is not a directory: ...
                                     git status: clean   pin: unmoved
.github/workflows/ mode 0555  apply: exit 2  REFUSED update-write-failed: writing the copies and the --add files failed at .github/workflows/validate.yml: Permission denied
                                             ROLLED BACK: every byte this command wrote has been put back ...  restored  scripts/validate-manifest.py
                                     git status: clean   pin: unmoved
nothing at .github (control)  apply: exit 0, added

Decisions for the reviewer.

• A second walk beside symlink_on_the_way, not a second answer from it. Its (name, target) return is a contract with _taken, the junction tests and update-shape.py apply rewrites a pinned copy, contracts/shape-pin.yaml or the manifest THROUGH a symlink the root keeps at that path (Rollback.write opens it r+b), so the standard's bytes land at the link's end, outside the root if that is where it points, with exit 0, and --branch reports a commit holding a file that commit does not contain #286, which uses it for the rewrite paths. The new walk is the same lstat loop over the components above the path.
• apply's untaken-addition note is changed too, not only check. It printed the same pass --add <path> that apply then refuses. One conditional, in _note_untaken_additions.
• The OSError arm wraps the two write phases the issue names and nothing else. An OSError before the first write, from the validators, or from a push after the commit is not turned into this refusal. _rewrite_readme keeps its own update-readme-unwritable.
• The tests are in a new file beside tests/test_update_shape_add_symlink.py, importing its fixtures (standard, root, update_shape; pytest registers imported fixtures) and its snapshot/assert_untouched. This keeps one scaffold definition, and it keeps update-shape.py apply rewrites a pinned copy, contracts/shape-pin.yaml or the manifest THROUGH a symlink the root keeps at that path (Rollback.write opens it r+b), so the standard's bytes land at the link's end, outside the root if that is where it points, with exit 0, and --branch reports a commit holding a file that commit does not contain #286, which also extends that file, from colliding with this PR at its end.

Tests

New tests/test_update_shape_add_not_a_directory.py, 13 cases:

• the walk:
  • names the first non-directory above the path, and nothing for a directory, a missing component, the path itself or a top-level path (1);
  • leaves a symlink above the path to symlink_on_the_way (1, @WINDOWS_SKIP).
• (1):
  • check reports the path with why and no --add for it, while the other addition keeps its --add (1);
  • (a) apply --add docs/shape-guide.md with docs a committed file, an untracked file or a FIFO: exit 2 update-add-not-a-directory, no traceback, no ROLLED BACK, and the whole tree untouched (snapshot/assert_untouched), the pinned copy B fixed included (3; the FIFO case skips where there is no mkfifo);
  • with one blocked addition among several, all are refused (1);
  • apply's note names what is in the way, not pass --add (1).
• (2):
  • (b) docs/ an empty mode-0555 directory, and contracts/shape-pin.yaml mode 0444: exit 2 update-write-failed naming the step, the path and Permission denied, ROLLED BACK: every byte, restored scripts/validate-manifest.py, and for the pin case the --add file and the docs/ it made removed. The tree is as it was (2; skipped on Windows and as root, where the mode bits stop nothing).
  • A file arriving at docs while an interactive apply waits in confirm (in-process) is refused as update-write-failed and rolled back (1).
• (c) the controls: nothing at docs, and docs/ a directory holding the project's own file, both add, with the row appended and the validators green (2).

Against origin/main's code (a1b3a4b, which holds #269 as c1b6732), 11 of the 13 fail for the issue's reason, and the 2 controls pass. Measured at the merge: a scratch export of origin/main plus this file gave 11 failed, 2 passed. The same 11 and 2 were first measured against ca68458 (#269's head when this branch was cut). The reasons, in both runs:

• changed ['scripts/validate-manifest.py']: the copy is rewritten and left (the plain-file, FIFO, several-additions and read-only-directory cases);
• written ['docs', 'docs/shape-guide.md'], changed ['scripts/validate-manifest.py'] (the read-only pin);
• NotADirectoryError out of cmd_apply (the arrives-while-waiting case);
• pass --add docs/shape-guide.md printed (check, and the note);
• no not_a_directory_on_the_way (the walk).

Mutants at 961c68e, each killed:

mutant	tests that fail
the update-add-not-a-directory refusal removed	4, by the refusal id. The tree is still rolled back, by (2).
writing() catches nothing	3: both read-only cases and the arrives-while-waiting case
_repin unwrapped	1: the read-only pin
the walk blocks regular files only	1: the FIFO

Targeted runs at 3b9ed21, with git-filter-repo 2.47.0 on PATH:

file	result
tests/test_update_shape_add_not_a_directory.py	13 passed
tests/test_update_shape_add_symlink.py	38 passed, 3 skipped (NTFS junctions)
tests/test_update_shape_add.py	17 passed
tests/test_update_shape_add_ignored.py	26 passed
tests/test_update_shape.py	77 passed
tests/test_shape_doctor.py	181 passed
tests/test_cli_reference.py	13 passed
tests/test_repo_hygiene.py	50 passed

scripts/render-cli-reference.py --check: ok.

At 189c30e, after two docstring-only commits: tests/test_repo_hygiene.py, tests/test_cli_reference.py and the new file, 76 passed.

Targeted runs at the merge, 818db36, with git-filter-repo 2.47.0 on PATH:

file	result
tests/test_update_shape_add_not_a_directory.py	13 passed
tests/test_update_shape_add_symlink.py	38 passed, 3 skipped (NTFS junctions)
tests/test_update_shape_add.py	17 passed
tests/test_update_shape_add_ignored.py	26 passed
tests/test_update_shape.py	77 passed
tests/test_update_shape_landing.py	33 passed
tests/test_update_shape_pr_base.py	27 passed
tests/test_shape_doctor.py	254 passed, 3 skipped (PowerShell, not on this host)
tests/test_cli_reference.py	13 passed
tests/test_repo_hygiene.py	50 passed

tests/test_update_shape*.py: 231 passed, 3 skipped. The other three files: 317 passed, 3 skipped. scripts/render-cli-reference.py --check: ok.

Composes with

git merge-tree --write-tree 818db36 <other>, as this lane's own review of 818db36 measured it, with origin/main at ab623dd. #270 has landed as f795cf7, which ab623dd holds, so its row is gone. The earlier table's #286 row (2c9b516) and #267 row (9f64bd0) are replaced by the heads below. Its #264, #266, #268 and #336 rows are dropped, not re-measured.

other	rc	notes
origin/main ab623dd (was a1b3a4b; holds #269 as c1b6732 and #270 as f795cf7)	0
#347 e965ab0 (fix/update-shape-pinned-copy-symlink, the branch the earlier table filed under #286, then at 2c9b516)	0	The same file as this PR.
#343 039530b	0
#267 8badae0 (was 9f64bd0)	0
lane 3's f1dcd2a (fix/update-shape-pr-rerun-pin-source)	0
#358 ebffb08	1	shape-doctor.py only. That is #358's own conflict with main: ab623dd against ebffb08 is also rc 1.
lane 3's dd14195 (fix/update-shape-next-quoting, no PR yet)	1	One hunk, in cmd_check. The resolution is in the first bullet below.

This lane's own review of 818db36: MERGEABLE WITH NITS (#346 (comment)); its other findings are follow-ups.

• dd14195's one hunk. There is no sequencing constraint. Whichever of this PR and dd14195 lands second resolves the cmd_check hunk the same way: keep this PR's offered in the ADDED UPSTREAM join and take dd14195's apply_command NEXT line. With that resolution this branch merged with dd14195 gave 199 passed, 3 skipped (the new file, next_quoting, update_shape, add and add_symlink). The resolved diff is handoffs/xFactory/attachments/lane-openRepoShape-2/review346-818db36/tools/m4-dd14195-resolution.diff in opensoft/brett-wip.
• Merge-tree, earlier heads. At 3b9ed21 this branch conflicted with main, update-shape.py: a path the root holds a symlink or a Windows junction at, dangling or live, or under one, is present and apply --add refuses it as update-add-symlink before it writes anything, and the ledger creates an addition exclusively, so a dangling link no longer has the standard's file written through it outside the root, and a red validator no longer deletes the operator's link (#239) #269's 8a460e8, scaffold-project.py: a leg path the project's own YAML reader would read back as another type (null, ~, true, False, 2026, 1.5) is refused before anything is created, under --dry-run alike, through the one definition adopt-project.py asks, so --spec-path 2026 no longer creates three repositories whose first bootstrap fails manifest-leg-path, and adopt-project.py execute no longer says adoption verified before that bootstrap fails (#225) #266, adopt-project.py: a value execute writes into a file is asked whether a file can hold it, in plan, in check and in execute before any leg exists, so a byte that is not UTF-8 in --elected-by, --reference or a local --source directory no longer pushes both legs before write_lf raises (#255) #270 and tests/test_update_shape_landing.py: the gh its landing tests drive update-shape.py with is a Python stub the update's child starts through a sitecustomize hook rather than a #!/bin/sh file on PATH, so the eight tests that carried WINDOWS_SKIP run on Windows too and the --pr landing and re-run paths (#204) are run by all three CI jobs, not two (#334) #336 on one line: the module docstring's EXIT CODES line, which main's update-shape.py apply --branch --push --pr: when gh pr create fails after the commit and the push succeeded, it exits 2 with "REFUSED committing the re-synced shape" and no NEXT line, and a re-run on that branch says "nothing to do" and exits 0, so the tool never opens the pull request #204 also rewrote. 961c68e left that line as the base has it. At 961c68e the only conflict was with update-shape.py apply rewrites a pinned copy, contracts/shape-pin.yaml or the manifest THROUGH a symlink the root keeps at that path (Rollback.write opens it r+b), so the standard's bytes land at the link's end, outside the root if that is where it points, with exit 0, and --branch reports a commit holding a file that commit does not contain #286, on the update-shape.py apply --add stages the new file with a plain git add, so a path the operator's excludes ignore fails after the copies, the re-pin and the branch are written, nothing is rolled back, and check then exits 0 over a re-pin nobody committed #202 paragraph's last line. 189c30e moved this branch's sentence into a bullet of its own above that paragraph.
• Proved by running, not only by merge-tree.
  • This branch at 961c68e (189c30e differs only by a docstring bullet) merged with update-shape.py: a path the root holds a symlink or a Windows junction at, dangling or live, or under one, is present and apply --add refuses it as update-add-symlink before it writes anything, and the ledger creates an addition exclusively, so a dangling link no longer has the standard's file written through it outside the root, and a red validator no longer deletes the operator's link (#239) #269's 8a460e8, which includes main c7c02fa (scratch clone, tree 6e90d96): the new file, test_update_shape_add_symlink.py, test_update_shape_landing.py and test_update_shape_pr_base.py, 111 passed and 3 skipped. That merge is now this branch's own history (25e38af); the runs at 818db36, above, cover the same four files (13 + 38 + 33 + 27 = 111 passed, and the 3 skipped).
  • This branch merged with update-shape.py apply rewrites a pinned copy, contracts/shape-pin.yaml or the manifest THROUGH a symlink the root keeps at that path (Rollback.write opens it r+b), so the standard's bytes land at the link's end, outside the root if that is where it points, with exit 0, and --branch reports a commit holding a file that commit does not contain #286's ba373db (tree b8740ac): the new file (13), update-shape.py apply rewrites a pinned copy, contracts/shape-pin.yaml or the manifest THROUGH a symlink the root keeps at that path (Rollback.write opens it r+b), so the standard's bytes land at the link's end, outside the root if that is where it points, with exit 0, and --branch reports a commit holding a file that commit does not contain #286's test_update_shape_rewrite_symlink.py (27), test_update_shape_add_symlink.py (41) and test_update_shape_add.py (17), 95 passed and 3 skipped (NTFS junctions).
  • update-shape.py apply rewrites a pinned copy, contracts/shape-pin.yaml or the manifest THROUGH a symlink the root keeps at that path (Rollback.write opens it r+b), so the standard's bytes land at the link's end, outside the root if that is where it points, with exit 0, and --branch reports a commit holding a file that commit does not contain #286 raises its link refusals from Rollback.write and before the writes as Refusal, which writing() passes through untouched.

Left out

• The late case is not named as a blocked path. A file that arrives at docs after prepare looked is reported as update-write-failed and rolled back, not as update-add-not-a-directory. Teaching write_added to name it would need a second check in write_added, which update-shape.py apply --add <dir>/<file> where the root holds a plain FILE at <dir> (or a directory it cannot write) dies in a NotADirectoryError (or PermissionError) traceback from Rollback.write_added after the pinned copies were written, and nothing puts them back, because cmd_apply rolls back only on Refusal and CommandFailed #285 does not ask for.
• An OSError out of the rollback itself (undo writing back a file that has since become unwritable) is unchanged; 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 is that question in bump-leg.py.
• Non-OSError failures in the write phase (UnicodeError, an interrupt) are unchanged.
• Windows: the read-only cases are skipped there, since Windows ignores the mode bits on a directory. The rest of the file runs on all three OSes, except the FIFO case and the link-walk case.
• update-shape.py apply rewrites a pinned copy, contracts/shape-pin.yaml or the manifest THROUGH a symlink the root keeps at that path (Rollback.write opens it r+b), so the standard's bytes land at the link's end, outside the root if that is where it points, with exit 0, and --branch reports a commit holding a file that commit does not contain #286's concerns (pinned copies, the pin or the manifest kept as symlinks) are not touched here.
• No README or AGENTS.md change. The README is capped and docs/handbook.html pins its blob, as update-shape.py: a path the root holds a symlink or a Windows junction at, dangling or live, or under one, is present and apply --add refuses it as update-add-symlink before it writes anything, and the ledger creates an addition exclusively, so a dangling link no longer has the standard's file written through it outside the root, and a red validator no longer deletes the operator's link (#239) #269 found. The refusal ids live in update-shape.py and its tests, as update-shape.py: a path the root holds a symlink or a Windows junction at, dangling or live, or under one, is present and apply --add refuses it as update-add-symlink before it writes anything, and the ledger creates an addition exclusively, so a dangling link no longer has the standard's file written through it outside the root, and a red validator no longer deletes the operator's link (#239) #269's did.

Closes #285

🤖 Generated with Claude Code

Summary by Sourcery

Make update-shape.py reject blocked additions safely and roll back writes that fail during apply.

New Features:

• Detect additions whose paths are blocked by non-directory entries and refuse them with a dedicated diagnostic before writing.
• Report write-phase operating-system errors as handled refusals and restore all changes made before the failure.

Bug Fixes:

• Prevent apply --add from leaving rewritten copies behind or terminating with a traceback when an addition cannot be created or a later write fails.
• Stop check and apply from suggesting --add for paths known to be blocked by a file or other non-directory entry.

Documentation:

• Document the new write-failure rollback behavior in the update tool module documentation.

Tests:

• Add coverage for blocked additions, rollback after directory and pin write failures, concurrent path changes, and valid control cases.

[brettheap]

@codex review

[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 11 hours and 23 minutes by commenting @sourcery-ai review. Upgrade to get a review now.

[sourcery-ai]

Reviewer's Guide

The PR hardens update-shape.py additions by rejecting paths blocked by non-directories before any write and by wrapping copy/addition and repin phases so OSError failures become descriptive refusals that roll back all writes, with extensive regression and compatibility coverage.

Sequence diagram for safe update-shape apply with additions

sequenceDiagram
    participant User
    participant Apply as cmd_apply
    participant Resolve as resolve_additions
    participant Ledger as Rollback
    participant Writes as _write_copies
    participant Repin as _repin

    User->>Apply: apply --add path
    Apply->>Resolve: resolve_additions(rows, additions)
    alt blocked by non-directory
        Resolve-->>Apply: Refusal update-add-not-a-directory
        Apply-->>User: Exit 2, no writes
    else addition is writable
        Apply->>Ledger: writing(writing the copies and the --add files)
        Ledger->>Writes: _write_copies(...)
        alt copy or addition raises OSError
            Writes-->>Ledger: OSError
            Ledger-->>Apply: Refusal update-write-failed
            Apply->>Ledger: undo()
            Ledger-->>User: Exit 2, restore every write
        else copies succeed
            Apply->>Ledger: writing(re-pinning contracts/shape-pin.yaml and project.yaml)
            Ledger->>Repin: _repin(...)
            alt repin raises OSError
                Repin-->>Ledger: OSError
                Ledger-->>Apply: Refusal update-write-failed
                Apply->>Ledger: undo()
                Ledger-->>User: Exit 2, restore copies, additions, and directories
            else repin succeeds
                Apply-->>User: Complete update
            end
        end
    end

Loading

Flow diagram for blocked additions and write rollback

flowchart TD
    A[find_additions] --> B[symlink_on_the_way]
    B --> C[not_a_directory_on_the_way]
    C --> D[Addition.blocked]
    D --> E{resolve_additions}
    E -->|blocked| F[Refusal: update-add-not-a-directory]
    E -->|accepted| G[Rollback.writing]
    G --> H[_write_copies]
    H --> I{OSError?}
    I -->|yes| J[Refusal: update-write-failed]
    J --> K[undo written copies, additions, and directories]
    I -->|no| L[_repin]
    L --> M{OSError?}
    M -->|yes| J
    M -->|no| N[rewrite README and validate]

Loading

File-Level Changes

Change	Details	Files
Detect blocked addition paths before writes and expose actionable refusals and diagnostics.	Added an lstat component walk that identifies the first non-directory above an addition while deferring symlink handling to the existing link walk. Stored the blocking component on additions and reject --add with update-add-not-a-directory before merge or write processing. Updated check and untaken-addition notes to omit misleading --add suggestions and explain the blocking path. Added coverage for regular files, FIFOs, symlinks, multiple additions, and valid directory controls.	update-shape.py tests/test_update_shape_add_not_a_directory.py
Convert write-phase operating-system errors into transactional refusals with rollback.	Added Rollback.writing to translate OSError from copy/addition and repin phases into update-write-failed messages containing the phase, path, and error. Wrapped copy/addition writes and repinning in the new context manager so the existing refusal path restores all ledger-tracked changes. Preserved existing handling boundaries for validation, README rewriting, post-commit operations, and non-OSError failures. Tested failures caused by unwritable directories, unwritable pins, and a directory being replaced during confirmation, including rollback verification.	update-shape.py tests/test_update_shape_add_not_a_directory.py
Document the new failure behavior and validate compatibility with existing update-shape behavior.	Added a module-level documentation bullet describing write failures as rollback refusals rather than tracebacks. Added regression and control tests covering successful additions, pin preservation, untouched trees, diagnostics, and CLI/reference hygiene.	update-shape.py tests/test_update_shape_add_not_a_directory.py

Possibly linked issues

• update-shape.py apply --add <dir>/<file> where the root holds a plain FILE at <dir> (or a directory it cannot write) dies in a NotADirectoryError (or PermissionError) traceback from Rollback.write_added after the pinned copies were written, and nothing puts them back, because cmd_apply rolls back only on Refusal and CommandFailed #285: The PR exactly addresses issue update-shape.py apply --add <dir>/<file> where the root holds a plain FILE at <dir> (or a directory it cannot write) dies in a NotADirectoryError (or PermissionError) traceback from Rollback.write_added after the pinned copies were written, and nothing puts them back, because cmd_apply rolls back only on Refusal and CommandFailed #285 by adding non-directory validation and transactional rollback for failed writes.

---

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.

[chatgpt-codex-connector]

Codex Review Summary

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

Review	Status	Commit	Review trigger
📝 Code Review	✅ Completed 2026-10-12T12:12:05.174204Z	189c30e	Manual request

ℹ️ 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: Didn't find any major issues. What shall we delve into next?

Reviewed commit: 189c30e449

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

[brettheap]

(lane openRepoShape-2, 2026-10-12T15:00Z)

This lane reviewed its own PR #346 at 818db36, read-only in its own detached worktrees, with nothing authored.

MERGEABLE WITH NITS. There is no sequencing constraint. Whichever of this PR and lane 3's fix/update-shape-next-quoting (dd14195) lands second has one cmd_check hunk to resolve.

Findings, at 818db36:

1. P3, follow-up: update-shape.py:2117-2128. A file that arrives at docs while apply waits in confirm is refused as update-write-failed and rolled back. The detail names the path and the OS error, not docs. On Windows that error is No such file or directory. The up-front refusal names the cause on every OS.
2. P3, follow-up: update-shape.py:2196-2211. An undo whose own restore raises still ends in a traceback. This is older than the PR, and the PR's "Left out" names it.
3. nit, docs follow-up: README.md:1081-1084 and docs/handbook.html:1388 predate update-add-symlink and update-add-not-a-directory.
4. nit: the PR body's compose table names superseded heads.

Probes: we ran 12 cases on the real scaffold against main and head, and 7 in-process. On head, a plain file, a nested file, a FIFO or a socket in the way is refused before any write, and the tree is identical. A read-only directory, copy, pin or manifest gives update-write-failed and is fully rolled back. On main, a read-only project.yaml left the pin moved and the manifest not. The doctor and check agree. The new tests fail on main (11 failed, 2 passed), and four new mutants were each killed.

Tests:

• head: 231 passed, 3 skipped.
• head merged with ab623dd: 548 passed, 6 skipped (update-shape, doctor, cli_reference, hygiene).
• with 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 e965ab0: 513 passed, 6 skipped.
• with dd14195, resolved: 199 passed, 3 skipped.
• with f1dcd2a: 89 passed, 3 skipped.
• CI is green on all three OSes.

Merges (merge-tree): rc 0 with ab623dd, #347, #343, #267 and f1dcd2a. #358 conflicts only in shape-doctor.py, which is its own conflict with main. dd14195 has one hunk.

The full report is handoffs/xFactory/attachments/lane-openRepoShape-2/review346-818db36/final-report.md in opensoft/brett-wip.

[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