[brettheap]

What

#1065 made plan, check and execute name a --work-dir that is, or
runs through, a symlink round a loop. r1065, the review of #1065, found
three things it got wrong and a set of rows no test held; #1074 lists them
as N1 to N7, S1 and S2. This puts the three right and holds the rest.

Outside v1.2.0 (Brett Heap, 2026-10-21T10:32:04Z): no milestone, a draft.

The code stays adopt-work-dir-unwritable, exit 2, nothing is made, and the
way out is the one #1065 gave. Three changes in adopt-project.py, in
_unmade_work_dir and _the_loop_in:

• N1. _the_loop_in stopped at the first symlink left in the path: when
  os.stat of that link succeeded it answered "no loop" without reading
  the links below it. It now asks each link left in the path, from / down,
  and takes the first that loops. A link that fails otherwise, a dangling
  one, still ends the walk, as it did.
• N6. A new constant, CANNOT_RESOLVE_WINERROR = 1921, Windows' "cannot
  resolve". os.stat of a link round a loop raises it there, with an errno
  that is not ELOOP, as validate-pins.py's _what_stands reads it (not
  measured on Windows: below). _the_loop_in counts it as a loop as it
  counts ELOOP. Any other failure, an EINVAL alone or winerror 123, is no
  loop. N6 is live only for a --work-dir that os.path.realpath reads as
  the link itself (self, or a for into -> a): there os.mkdir
  of a name that exists gives EEXIST on Windows too (winerror 183), which
  passes the errno gate in _unmade_work_dir, so _the_loop_in is asked. A
  loop ABOVE the work dir on Windows (loop\sub) still gets the system's
  words: CreateDirectoryW there most likely fails with winerror 1921 and
  so errno EINVAL, and the gate keeps _the_loop_in from being asked. That
  chain is reasoned, not measured.
• N5. Where the system finds no link at the path as typed but
  os.path.realpath reads it as the loop's own link (looped == root), the
  reason says so: "read as <A>/self, it is a symbolic link that leads
  round a loop", not "the way to it leads round a loop, at the symbolic
  link <A>/self", where the path read IS that link and not the way to it.
  The way out is unchanged: it names that link.

Measured with plan of a local source, <A> the directory that holds the
links, before on main at 038dafba and after on the merged head 06cc799c
(Python 3.12.3, exit 2 in every case). The first two rows measured earlier
on 6da757cb and 6d85d59b read the same. The NIT round after r1143
(b5b0e6ca, 7af5fabc) changes a docstring and tests only; the first and
third rows were read again on its head 7af5fabc, the third with plan,
check and execute, which agree, with 038dafba's adopt-project.py in a
scratch copy of this tree for the before.

--work-dir	Before	After
w, with w -> loop/../d/self2, loop -> loop, d -> realdir and realdir/self2 -> self2	--work-dir `<A>/w` cannot be made: it is a symbolic link that leads to `<A>/d/self2`, which is not a directory	--work-dir `<A>/w` cannot be made: it is a symbolic link that leads round a loop, at the symbolic link `<A>/d/self2`
<A>/nothere/../self, with self -> self and no nothere	--work-dir `<A>/nothere/../self` cannot be made: the way to it leads round a loop, at the symbolic link `<A>/self`	--work-dir `<A>/nothere/../self` cannot be made: read as `<A>/self`, it is a symbolic link that leads round a loop
w, with w -> loop/../d/self2/sub, loop -> loop, d -> realdir and realdir/self2 -> self2	--work-dir `<A>/d/self2/sub` cannot be made: making it a directory failed (Too many levels of symbolic links)	--work-dir `<A>/w` cannot be made: it is a symbolic link that leads round a loop, at the symbolic link `<A>/d/self2`

The way out is the same before and after in the first two rows:
"Remediation: Remove the symbolic link <A>/w, or make it lead to a
directory, or pass another --work-dir, or none: without one, each run makes
a new one." in the first row, and the same with <A>/self in the second.
The first row said only that <A>/d/self2 is not a directory, and not that
it is a link round a loop; the new words say so. The third row is a change
the review r1143 found that the table lacked (its NIT-3), and an
improvement: the first row's folded path with one name more, where mkdir
of <A>/d/self2/sub fails with ELOOP and not EEXIST. Before, the reason was
the system's, of a path the person never typed, with #524's way out
("Remediation: Pass a --work-dir this user can write in, or none: without
one, each run makes a new one."); after, it names w and the link in the
loop, with the way out for w, the first row's.

Python. The first row's old words need Python 3.12, the one measured,
and I believe earlier ones. There os.path.realpath stops at loop,
leaves the rest of the path unread and folds its .., which leaves
<A>/d/self2, a link that does not loop (d) before one that does. I
believe 3.13 and later read on past a loop and give <A>/realdir/self2,
which holds no other link, so that the old words need 3.12. That is from
reading 3.13's realpath, not from a run: no 3.13 interpreter was on this
machine. If CI's 3.x job is on 3.13 or later, the command-level N1 rows
pass on main there as well, and only the direct test of _the_loop_in
catches the old code.

The docstrings of _unmade_work_dir and _the_loop_in say the same.

Tests

tests/test_adopt_work_dir_refusal.py: 29 new tests, 53 to 82, and the N2
row added inside the existing test_only_a_symlink_that_loops_is_called_a_loop.

• Rows that pin a change (9). N1 w -> loop/../d/self2 through the
  three commands (3), and _the_loop_in asked directly of
  <A>/to-dir/self2 (1), which holds the walk on every Python; N5
  nothere/../self through the three commands (3); N6 winerror 1921, asked
  of _the_loop_in (1) and, r1143's NIT-4, of _unmade_work_dir with a
  FileExistsError for a --work-dir that is a link to itself and os.stat
  patched to raise an OSError with winerror 1921 and errno EINVAL (1),
  the one Windows path that is live.
• Controls (20), named as such in their docstrings. They pass on main
  and hold what r1065's surviving mutants break:
  • S1, w -> a-file/sub and w -> locked/x with locked at mode 555:
    the system's words and adopt-project.py check: refuse a kept file at the shape's pin path, and an unwritable --work-dir, before any leg exists #524's way out (6);
  • S2, viadir/self with viadir -> realdir: a loop spelled through a
    linked folder is named once (3);
  • N3, a relative self and <A>/x/../self: named as given, made
    absolute, not resolved (6);
  • N4, tofile/sub with tofile -> a-file: named as realpath reads it
    (3);
  • N6, an EINVAL alone and winerror 123: no loop (2).
• N2, a dangling link, is added to the direct test: no loop.
• Helpers: run_in and refused_for_a_link take cwd=, and
  refused_for_a_link takes named=; cannot_make asks mkdir of the
  real path; a new locked_folder fixture (it skips where chmod cannot
  keep this user out, root or Windows) and refused_in_the_systems_words;
  stat_fails_at, the one os.stat patch, which the two N6 rows share.
  check is no longer imported.
• Windows (r1143's NIT-5). The three N5 rows take a new POSIX_ONLY
  skip, a file-local skipif on os.name == "nt" with its reason: Win32
  folds <A>\nothere\..\self to <A>\self before the file system is
  asked, so there the path IS the link and the reason is not "read as". The
  file had no Windows skip of its own, and conftest.WINDOWS_SKIP's reason
  is about setup.sh, so it has its own, as test_adopt_entry_paths.py
  does. Windows CI runs none of this file.

Fail on base. With main's adopt-project.py in this head's tree, the
two files tests/test_adopt_work_dir_refusal.py and
tests/test_adopt_clone_claimed.py: 9 failed, 115 passed, on 038dafba
(with the NIT round's tests). Before NIT-4's row it was 8 failed, 115
passed, on 038dafba and on 6da757cb. The 9 are exactly the rows that pin
a change, each failing on main's words: the three N1 rows through plan,
check and execute, the three N5 rows, the direct N1 test, the N6
winerror 1921 row, and the N6 row of _unmade_work_dir, which fails with
"it is a symbolic link that leads to <A>/self, which is not a directory".
All 20 controls and the N2 row pass on both. On this head 7af5fabc: 124
passed (82 and 42), no skips.

Mutants. Each applied alone to a scratch copy of adopt-project.py at
6d85d59b (sha256 ae1fc9f1e3c2d143a7a0b945f122b8f10521cb2e92e569d67ae015f867f817d0),
then the refusal file and
test_a_loop_or_a_file_above_the_work_dir_keeps_its_650_refusal (87 tests)
run, then the copy restored and its sha256 read back equal. R3, R5, R9, R11,
R19 and R20 are r1065's surviving mutants: 6 of 6 killed. W1 to W3 each
undo one of this change's three edits: 3 of 3 killed. The merge of 038dafba
changes no line of the functions they mutate: main's last hunk in
adopt-project.py starts at line 11191, and _work_root,
_unmade_work_dir, _the_loop_in and _same_entry are at lines 17031,
17088, 17159 and 17190 on the merged head.

Mutant	Failed	Killed by
R3 any failing stat taken for a loop, no ELOOP test	3	the dangling link in test_only_a_symlink_that_loops_is_called_a_loop; the two N6 controls (EINVAL, winerror 123)
R5 _same_entry compares the spellings, not lstat identity	6	the S2 rows (3); the N3 dot-dot rows (3)
R9 the link-to-a-non-directory branch taken for any errno, not EEXIST	6	the S1 rows, both forms (6)
R11 the fallback names the path as typed, not as realpath reads it	9	the S1 rows (6); the N4 rows (3)
R19 the --work-dir not made absolute	3	the N3 relative rows
R20 the --work-dir normalised with os.path.abspath, which folds ..	6	the N5 rows (3); the N3 dot-dot rows (3)
W1 N1 undone: the walk ends at a link whose stat succeeds	4	the N1 rows (3); the direct N1 test
W2 N6 undone: winerror 1921 not taken for a loop	2	the N6 winerror 1921 row; the N6 row of _unmade_work_dir
W3 N5 undone: "the way to it" for a path read as the loop's link	3	the N5 rows
R4 _the_loop_in walks from the leaf up, not from / down	0	survives: 87 passed

W2's 2 is from a re-run on the NIT round's head 7af5fabc, with the two
files whole (124 tests): 2 failed, 122 passed. On the same selection r1143's
X9, the constant set to 1920 (ERROR_CANT_ACCESS_FILE), is killed by the
same two rows: 2 failed, 122 passed. The other rows of this table were
measured on 6d85d59b with the 87 tests above.

The gate, lane 1's, on 6d85d59b (this branch merged with main at
6da757cb, before the merge below), one basetemp, EMAIL exported,
LANES_LANE unset, a stub gh first on PATH, GH_TOKEN=bogus-w1074, an
empty GH_CONFIG_DIR, GIT_CONFIG_GLOBAL=/dev/null and
GIT_SSH_COMMAND=false, in 63 chunks, each one pytest call under 9
minutes: 9157 passed, 34 skipped, 0 failed. It is 122 test files, the
ones that name work-dir or work_dir as grep reads them and
test_agents_md_codes.py: 121 of them in 59 chunks of up to 150 tests by
node id (8,743 tests), and test_shape_doctor.py, all 448 tests, in 4
shards. It is not the whole suite. The 34 skips are the tests' own, none a
row of this change: template rows in test_family_init_values.py (19) and
test_leg_path_yaml_scalars.py (8), the PowerShell line in
test_shape_doctor.py (3), a GIT_CONFIG_GLOBAL set at the session's start
in test_no_auto_maintenance.py (2), and two in
test_adopt_eol_block_through_links.py, each on its own reason.

On the merged head 06cc799c, the same environment, one call at a time,
one basetemp, each under 9 minutes (four runs):

• tests/test_path_guard_hygiene.py, which tests/test_path_guard_hygiene.py and shape-doctor.py: a bare is_file/is_dir/exists guard in a shipped tool fails by file and line unless its _is_a or a reasoned allowlist covers it, and a project.yaml linked into an unsearchable folder is yaml-unreadable (#430) #1111 added: 49 passed;
• tests/test_adopt_work_dir_refusal.py and
  tests/test_adopt_clone_claimed.py: 123 passed;
• tests/test_shape_doctor.py, in 4 shards by node id: 445 passed, 3
  skipped (112, 112, 112 and 109 passed; the 3 are the PowerShell line);
• tests/test_scaffold_leg_path_collisions.py: 104 passed.

On the NIT round's head 7af5fabc (main still 038dafba, which the head
holds; no merge), the same environment, at most three calls at once, one
basetemp each, each under 9 minutes, test_shape_doctor.py in 4 shards by
node id (112 each, lists checked non-empty):

• tests/test_adopt_work_dir_refusal.py and
  tests/test_adopt_clone_claimed.py: 124 passed;
• tests/test_path_guard_hygiene.py: 49 passed;
• tests/test_scaffold_leg_path_collisions.py: 104 passed;
• tests/test_shape_doctor.py: 445 passed, 3 skipped (111 + 1, 112, 111 + 1
  and 111 + 1; the 3 are the PowerShell line);
• tests/test_adopt_plan_flag_refusals_first.py,
  tests/test_adopt_root_symlinks.py,
  tests/test_adopt_shape_pin_and_work_dir.py and
  tests/test_adopt_check_throwaway_repos.py, which with the two above name
  adopt-work-dir-unwritable: 343 passed, 3 skipped (the reftable backend
  needs git 2.45 or later; the gate's skips are as listed above).

Not the full suite, and not the 63-chunk gate again: the NIT round changes a
docstring, one test file and this body, and CI runs the full suite.

r1143's NITs

r1143, the independent review of this PR on 06cc799c (#1143
issuecomment-6327473568), found it ready on condition (tests on that head)
and no MUST or SHOULD. That condition passed. Its six NITs are answered, in
b5b0e6ca and 7af5fabc, with no change of behaviour:

• NIT-1. _the_loop_in's docstring says ELOOP also means more than 40
  links on the way (Linux's limit), so the walk can name a link that does
  not itself loop where over 40 links are left; the probe
  contrived-toolong is in the commit message. Measured again with plan:
  w -> loop/../l0/l1/.../l45/x, with loop -> loop, l0 -> r0 and each
  r(i-1)/li -> ../ri: main says "making it a directory failed (Too many
  levels of symbolic links)" of <A>/l0/.../l45/x; this head names <A>/w,
  "at the symbolic link <A>/l0/.../l40", a link that does not loop.
• NIT-2. A bullet in "Not in this PR": the dangling link.
• NIT-3. The third row of the table above, measured on main and on this
  head.
• NIT-4. The row for _unmade_work_dir in the Tests section: it fails
  on main, and under W2 and X9; 124 passed here.
• NIT-5. POSIX_ONLY on the three N5 rows, file-local because the file
  had no Windows skip of its own.
• NIT-6. The N6 bullet under "What" and the Windows bullet under "Not in
  this PR" say N6 is live only for a --work-dir os.path.realpath reads as
  the link, and that a loop above the work dir on Windows keeps the system's
  words.

Merge with main

Two plain merges, no rebase, amend, squash or force.

• 58ba91b6: origin/main at 6da757cb (shape_materialize.py, adopt-project.py and bump-leg.py: a tag a clone holds without its commit is one it cannot answer for, check says a commit it cannot ask was not asked, and a pinned product YAML 1.1 or 1.2 reads as no text is quoted (#912) #1090), clean. It came before
  the tests were written.
• 06cc799c (head): origin/main at 038dafba, which holds adopt-project.py: check names what the clone's .git holds as git keeps refs, and asks a pin's file through the links the root keeps, so a link into .git or onto a pin's file is refused before any leg or adopted with the copy beside it (#904) #1091, scaffold-project.py: the --reuse-empty-repo help, its docs and each remedy that sends a person into that re-run say ssh, a credential helper or askpass can still make it wait, and how to keep it from waiting (#915) #1101,
  templates/assembly-root/project.yaml and templates/family-root/family.yaml: the comment above visibility: says it is the visibility the run asked for, and that a repository the run reuses keeps its own, where it said "this is all three repositories' visibility" (#913) #1102 and tests/test_path_guard_hygiene.py and shape-doctor.py: a bare is_file/is_dir/exists guard in a shipped tool fails by file and line unless its _is_a or a reasoned allowlist covers it, and a project.yaml linked into an unsearchable folder is yaml-unreadable (#430) #1111. Clean. Of this branch's two files only adopt-project.py
  moved on main, in _kept_pin_findings, _leg_findings, _PlannedRoot and
  _SymlinkProbe among others, none of them _work_root, _unmade_work_dir
  or _the_loop_in. tests/test_adopt_work_dir_refusal.py and
  tests/test_adopt_clone_claimed.py did not move on main. tests/test_path_guard_hygiene.py and shape-doctor.py: a bare is_file/is_dir/exists guard in a shipped tool fails by file and line unless its _is_a or a reasoned allowlist covers it, and a project.yaml linked into an unsearchable folder is yaml-unreadable (#430) #1111's
  tests/test_path_guard_hygiene.py passes with this branch's
  adopt-project.py. The gate above ran on 6d85d59b, before this merge;
  on the merged head only the four runs listed above were made, and CI's
  run is the full one.

Not in this PR

• N7 is moot: nothing to change.
• --work-dir <A>/tofile/sub, with tofile -> a-file, still names the path
  os.path.realpath reads, <A>/a-file/sub, in the system's words, and
  names no link, as adopt-project.py: a --work-dir that is, or runs through, a symlink round a loop is refused in words that name the link and the loop, not "(File exists)", and a symlink to a file is named as one (#978) #1065 left it; w -> a-file/sub reads the same way.
  The new N4 and S1 rows hold both as controls.
• The same class as N1 is still missed where the folded path meets a
  DANGLING link (r1143's NIT-2): w -> loop/../dang/self2, with loop -> loop and dang -> missing. Following w the system meets <A>/loop
  first, but os.path.realpath (3.12) folds the path to <A>/dang/self2,
  and the walk ends at dang, a link that dangles, as it must. Main at
  038dafba and this head say alike, with plan: "--work-dir <A>/w
  cannot be made: it is a symbolic link that leads to <A>/dang/self2,
  which is not a directory". In N1's row <A>/d/self2 is named only because
  the folded reading reaches a second loop; here there is none.
• Windows is not measured. N6 is held by an OSError with winerror set,
  built by hand with os.stat patched; no Windows host ran it. N6 is live
  only for a --work-dir that os.path.realpath reads as the link itself
  (mkdir gives EEXIST, which passes the errno gate), and a loop ABOVE the
  work dir on Windows (loop\sub) still gets the system's words, because
  the gate keeps _the_loop_in from being asked. The errno gate in
  _unmade_work_dir, exc.errno in (errno.EEXIST, errno.ELOOP), was not
  widened to winerror 1921, so a mkdir that fails there with another
  errno still never asks _the_loop_in. Widening it is a follow-up if
  wanted: r1143's R14, the loop question asked for every errno, changes
  nothing on Linux (123 of 123 pass).
• R4 survives: no row holds the order of _the_loop_in's walk. r1065's R14
  and R23 were not run by this branch's writers; r1143 ran both, and both
  survive, equivalent on a real file system.

Closes #1074

Lane: openreposhape-1 (openRepoShape-1)

🤖 Generated with Claude Code

Summary by Sourcery

Fix work-directory symlink-loop detection and diagnostics while adding regression coverage for platform-specific and path-resolution edge cases.

Bug Fixes:

• Correct symlink-loop detection for work directories by continuing past non-looping symlinks and recognizing Windows error 1921 as a loop.
• Improve refusal messages so paths interpreted as the loop link identify that interpretation accurately.

Enhancements:

• Expand work-directory refusal coverage across plan, check, and execute, including path normalization, dangling links, inaccessible locations, and symlinked files or directories.

Documentation:

• Update work-directory handling documentation and test guidance for symlink-loop behavior across Python and Windows environments.

Tests:

• Add comprehensive regression and control tests for symlink-loop detection, Windows error handling, path reporting, and preservation of existing refusal behavior.

[sourcery-ai]

Reviewer's Guide

Implementation

Update work-directory refusal handling to identify loops beyond non-looping symlinks, recognize Windows error 1921, and describe a path read as the loop link accurately.

• adopt-project.py L17948-17955, L17990-17997, L18002-18021 (+2 small edits)

Tests

Add command-level and direct regression tests for loop detection and Windows error handling, with controls for existing refusal behavior and helper adjustments to support the new cases.

• tests/test_adopt_work_dir_refusal.py L332-443, L460-531, L553-624 (+9 small edits)

---

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.

[brettheap]

(lane openRepoShape-1, 2026-10-21T21:58:55Z) r1143, lane 1's independent review of #1143 at 06cc799c: READY on condition, with no MUST or SHOULD and six NITs. It is not a writer of this PR (w1074 and w1074b).

• The condition: CI's tests must pass on 06cc799c. It was still pending at 21:58Z; SonarCloud, package and tests-git-bash pass.
• N6 is reachable on Windows for a --work-dir that realpath reads as the link itself: mkdir gives EEXIST, which passes the errno gate, and then stat gives winerror 1921. This is reasoned, not run on Windows. A loop above the work dir stays with the system's words, as the body says.
• Every measured body claim holds, and base fails 8 rows as stated.
• Mutants: 10 of 15 of the reviewer's own are killed, and the body's nine are re-killed. The five survivors (R4, R14, R23, X1 and X11) are equivalent on a real Linux file system.
• Next: lane 1 takes the six NITs in a small round before this lands, after the v1.2.0 tag, on a later word: NIT-1 a docstring line, NIT-2, NIT-3 and NIT-6 body lines, NIT-4 an optional direct row, and NIT-5 a Windows note or skip.

The full review

r1143: independent review of PR #1143 (Closes #1074), head 06cc799c

Verdict: READY on condition. The condition is that CI's tests job passes on 06cc799c. At 21:57:28Z it was still pending (it started at 21:23:03Z). SonarCloud, package and tests-git-bash had passed. I found no MUST and no SHOULD, only six NITs.

Nothing I did changed the worktree. It is still at 06cc799c, and git status --short --ignored prints 0 lines. Its adopt-project.py has sha256 cde2424c… and blob 9837c221, the same as HEAD:adopt-project.py. All four mutant clones read back cde2424c… after every mutant. I made no write to GitHub and no commit.

Is N6 reachable?

Yes, for a --work-dir that realpath reads as the link itself. That covers self, a, into (read as a) and through. The PR's N6 row tests a function the real code calls there, so this is not a MUST. The chain is reasoned, not measured, because no Windows host was available:

1. os.mkdir of a name that already exists fails on Windows with ERROR_ALREADY_EXISTS (183). CPython's winerror_to_errno maps that to EEXIST, the familiar [WinError 183] FileExistsError.
2. _made_0755 re-raises it. Path.is_dir() answers False to winerror 1921: on-disk /usr/lib/python3.12/pathlib.py:41-53 lists _WINERROR_CANT_RESOLVE_FILENAME = 1921 # broken symlink pointing to itself among the ignored errors.
3. EEXIST passes the gate at adopt-project.py:17124-17125, so _the_loop_in is asked.
4. Its os.stat of the link then raises winerror 1921. 1921 is not in CPython's errmap table, so its errno is the default EINVAL. CPython's own pathlib.py:1238 treats the pair the same way: if e.errno == ELOOP or winerror == _WINERROR_CANT_RESOLVE_FILENAME: raise RuntimeError("Symlink loop …"). templates/assembly-root/scripts/validate-pins.py:496-497 reads it the same way.
5. So before this PR, Windows would have said "leads to <A>\self, which is not a directory"; with it, the loop is named.

What N6 does not reach is a loop above the work dir (loop\sub). CreateDirectoryW there most likely fails with 1921, so EINVAL, and the gate keeps _the_loop_in from being asked. The body states this limit, and states it truly.

Findings

MUST: none.

SHOULD: none.

NIT

NIT-1. The walk can name a link that does not loop, when more than 40 links are left in the path.

• Where: adopt-project.py:17178-17186. It needs Python 3.12 or earlier, a loop in the link's target that realpath folds out, and more than 40 links after it.
• Probe contrived-toolong:
  • set-up: w -> loop/../l0/l1/…/l45/x, with loop -> loop, l0 -> r0 and each r(i-1)/li -> ../ri;
  • base: --work-dir `<A>/l0/…/l45/x` cannot be made: making it a directory failed (Too many levels of symbolic links);
  • head: --work-dir `<A>/w` cannot be made: it is a symbolic link that leads round a loop, at the symbolic link `<A>/l0/…/l40`. check and execute say the same.
• l40 is no loop:
  • readlink r39/l40 -> ../r40, and r40 is a directory;
  • stat l0/…/l39 (40 links) is ok;
  • stat l0/…/l40 (41 links on the way) gives ELOOP, which is Linux's 40-link limit.
• The way out still names <A>/w, which is right.
• Fix: none needed. At most, say in _the_loop_in's docstring that ELOOP also means "more than 40 links on the way".

NIT-2. The same class as N1 is still missed where the folded path meets a dangling link.

• Probe contrived-dangling: w -> loop/../dang/self2, with dang -> missing.
• Base and head both say "leads to <A>/dang/self2, which is not a directory".
• In the PR's own N1 row, following w the system meets <A>/loop first (stat w gives ELOOP). <A>/d/self2 is named only because realpath's folded reading reaches a second loop. The docstring says so honestly.
• Fix: one bullet in "Not in this PR".

NIT-3. A behaviour change that is not in the before/after table.

• w -> loop/../d/self2/sub:
  • base: `<A>/d/self2/sub` … (Too many levels of symbolic links), with adopt-project.py check: refuse a kept file at the shape's pin path, and an unwritable --work-dir, before any leg exists #524's way out;
  • head: `<A>/w` … leads round a loop, at the symbolic link `<A>/d/self2`, with the link's way out.
• It is an improvement. Fix: add a row to the table, or a line.

NIT-4. The N6 test holds _the_loop_in only.

• Where: tests/test_adopt_work_dir_refusal.py, test_windows_cannot_resolve_is_a_loop_too.
• No row drives _unmade_work_dir with an EEXIST error plus a winerror-1921 stat, which is the only Windows path that is live.
• Fix: optionally, one direct row calling _unmade_work_dir(work, root, FileExistsError(...)) with os.stat patched.

NIT-5. The N5 rows would fail on a Windows host that can make symlinks (reasoned).

• Win32 folds .. before the path reaches the file system, so <A>\nothere\..\self is the link there. islink would be True, and the reason would be "it is a symbolic link that leads round a loop", not "read as …".
• Windows CI (tests-git-bash) runs only three other files (tests.yml:263-279), so CI is not affected.
• Fix: a skip on Windows, or a docstring note that this behaviour is POSIX-only.

NIT-6. Body wording about N6.

• Say that N6 is live only for a --work-dir that realpath reads as the link (EEXIST), and that a loop above the work dir on Windows still gets the system's words.
• Optional follow-up: widen the gate. My R14 shows that asking for every errno changes nothing on Linux (123 of 123 pass).

The rest of the walk review

• Bounded: one lstat per path part and at most one stat per link. A 1,510-part path takes 200.8 ms on head and 207.9 ms on base; the whole plan takes about 1.1 s either way. There is no new quadratic cost.
• A link that fails otherwise:
  • EACCES gives "(Permission denied)" with adopt-project.py check: refuse a kept file at the shape's pin path, and an unwritable --work-dir, before any leg exists #524's way out, on base and head alike (unsearchable, unsearchable-via-link, contrived-eacces);
  • mkdir errnos: ro/self gives EEXIST, locked0/self gives EACCES, ro/self/sub gives ELOOP.
• Relative links (sub/up -> ../a), .. after a link (vd/../self), viadir/self, x/../self, self, into, through, loop/sub, a 45-link chain into a loop, tofile, tofile/sub, w -> a-file/sub, w -> locked/x and a dangling link (exit 0) all read the same on base and head.
• plan, check and execute agree in every probe. execute prefixes "no leg repository was created and nothing was pushed."
• The link is named once, and the way out matches it.
• <A>/nothere/../into now reads "read as <A>/a, it is a symbolic link that leads round a loop"; base said "the way to it … at <A>/a". The way out is <A>/a on both. This is consistent with the N5 docstring.

Body claims

Claim	Measured
N1 row Before (plan, main 038dafba, Python 3.12.3)	true, word for word
N1 row After (head)	true, and check/execute match
N5 row Before / After	true / true
Same way out before and after, exit 2 in every case	true
A dangling link still ends the walk	true (direct row; X1 is equivalent)
N6: winerror 1921 is a loop; EINVAL alone and winerror 123 are not	true by direct test (X2, X3, X9, X12 killed); on Windows unverified, reasoned reachable for EEXIST
3.12 realpath(w) is <A>/d/self2	true (3.12.3)
3.13+ gives <A>/realdir/self2; there the N1 command rows pass on main too	unverified. My reading of 3.13's algorithm agrees. CI is CPython 3.14.8, per main's last green tests run 39655123686.
28 new tests, 53 to 81; head 123 = 81 + 42, no skips	true (collect-only: 53 and 123; head run: 123 passed)
Base 8 failed, 115 passed on 038dafba, exactly the 8 rows that pin a change	true
The same on 6da757cb	unverified (not re-run)
All 20 controls and the N2 row pass on both	true
The mutant table: R3 3, R5 6, R9 6, R11 9, R19 3, R20 6, W1 4, W2 1, W3 3, with their "killed by" columns	true, re-measured on the same 87-test selection
R4 survives	true; it is equivalent on a real file system
sha256 ae1fc9f1… at 6d85d59b	true
Functions at 17031 / 17088 / 17159 / 17190; main's last hunk at 11191	true
The merges do not touch those functions or the two test files	true
Gate: 122 files, 9157 passed, 34 skipped on 6d85d59b	not re-run. The writer's logs add up: 8,743 + 448 node ids; skips 2 + 8 + 16 + 3 + 2 + 3. The file list matches git grep plus test_agents_md_codes.py.
Merged-head runs: 49; 123; 445 passed + 3 skipped; 104	true (my reruns)
Both merges are clean	true (merge-tree tree equals commit tree: fe49b7ed, 08134580)
Main at 038dafba holds #1091, #1101, #1102 and #1111	true
Of the PR's two files only adopt-project.py moved on main	true
"Not in this PR": tofile/sub wording	true (`<A>/a-file/sub` … (Not a directory))
"Not in this PR": the errno gate was not widened	true (:17124-17125)
R14 and R23 were not run	true of the writers; I ran both, and both survive

Hygiene

• Closes #1074 is the only closing keyword in the body, the title or the four commit messages. closingIssuesReferences is [1074].
• All four commits are authored by service@opensoft.dev, and neither PR commit names the operator's address.
• The docs need no change: docs/cli.md:612/668/728 stay true, and README, AGENTS.md and templates/ do not mention the refusal's wording.

Base vs head

Tree	The two PR test files
Base (038dafba's adopt-project.py in a scratch copy of head)	8 failed, 115 passed. The 8: N1 through plan, check and execute; N5 through the same three; test_a_link_that_does_not_loop_does_not_end_the_walk; winerror-1921.
Head	123 passed

My mutants: 10 of 15 killed

I also re-ran the body's nine mutants (R3, R5, R9, R11, R19, R20, W1-W3); all nine are killed, as noted in the table above. Killed:

Mutant	Change	Failed
X2	every _ABSENT_WINERRORS winerror taken for a loop	1
X3	a missing winerror defaults to 1921	2
X4	ELOOP and 1921 needed	36
X5	root itself never asked	27
X6	the N5 branch also taken where the --work-dir is itself a link	21
X7	"read as" names the typed path	3
X8	the N5 way out names the typed path	3
X9	the constant is 1920	1
X10	the walk returns root, not the link	11
X12	EINVAL also taken for a loop	2

The five survivors are all equivalent on a real Linux file system:

• R4, the walk from the leaf up. For a deeper link to be lstat-able, every link above it must resolve to a directory, so only the deepest link can fail stat.
• R14, the loop question asked for every errno. Any loop in the path makes mkdir fail with EEXIST or ELOOP; even in a folder at mode 555 it gives EEXIST. This is the change that would matter on Windows.
• R23, _same_entry answering True on an OSError. Both paths are known to be lstat-able by then.
• X1, a failing stat that does not end the walk. Nothing below that link can be lstat-ed.
• X11, every part asked, link or not. A link above any failing part is reached first.

Only a mocked stat could kill these, and only R14 matters, on Windows.

Main now

• fetch origin main at 21:56:32Z: still 038dafba. The head already holds it, so there was no merge-tree to build, and I ran on head.
• Results:
  • test_path_guard_hygiene.py: 49 passed;
  • test_shape_doctor.py in 4 shards: 111 + 112 + 111 + 111 = 445 passed, 3 skipped, all three the PowerShell line;
  • test_scaffold_leg_path_collisions.py: 104 passed;
  • the two PR files: 123 passed;
  • the other files that name adopt-work-dir-unwritable: test_adopt_plan_flag_refusals_first.py 89, test_adopt_root_symlinks.py 205 passed and 3 skipped (reasons not captured), test_adopt_shape_pin_and_work_dir.py 20, test_adopt_check_throwaway_repos.py 29.
• No failures.

Everything is under /workspace/projects/openRepoShape-worktrees/_lane1-scratch/r1143/:

• scripts: probe.sh, mutants.py, walktime.py, run2.sh, runids.sh, env.sh;
• outputs: probes/, mutants-m*.out, mutants-body-m*.out, head-2files.out, base-2files.out, now-*.out.

Lane: openreposhape-1 (openRepoShape-1)

[brettheap]

(lane openRepoShape-1, 2026-10-22T02:45:54Z) r1143b, lane 1's independent re-check of the NIT round at 7af5fabc: READY on condition, with no MUST or SHOULD. It is not the writer f1143.

• The condition: CI's tests must pass on 7af5fabc. It was pending at 02:44Z; SonarCloud, package and tests-git-bash pass.
• The round changes no code. adopt-project.py differs from 06cc799c only in a docstring (the ASTs with docstrings blanked are equal). The new direct row fails on base and under W2 and X9. POSIX_ONLY skips exactly the three N5 rows, and only on Windows.
• Every count in the body re-measures.
• NIT-2 (the body listed through among the paths where N6 is live; it is a loop above the work dir) is fixed in the body by lane 1.
• NIT-1 (the docstring's "the way out still names the --work-dir" holds only where the --work-dir is itself a link) is optional and left for now.

The full re-check

Verdict: READY on condition. The condition is that CI's tests job passes on 7af5fabc. At 02:44:22Z it is still pending; it started at 02:26:22Z. The tests job on 06cc799c passed. I found no MUST and no SHOULD, and two NITs.

Checks

1. The diff: confirmed.

• git diff 06cc799c 7af5fabc --name-status lists exactly M adopt-project.py and M tests/test_adopt_work_dir_refusal.py.
• In adopt-project.py the only change is seven lines in _the_loop_in's docstring (b5b0e6ca).
• I parsed adopt-project.py at both commits with ast.parse. With docstrings blanked the two trees are equal (True); with docstrings kept they differ (False). So there is no code change.
• git diff --check is clean, there is no non-ASCII in the added lines, and no added line is over 79 columns.
• Both new commits are authored by Brett Heap <service@opensoft.dev> with the Lane: and Co-Authored-By trailers.
• The operator's address appears in none of the commits, the diff or the body.

2. NIT-1: true. I re-ran contrived-toolong with plan, check and execute on base 038dafba and on head (Python 3.12.3).

• Base: --work-dir `<A>/l0/…/l45/x` cannot be made: making it a directory failed (Too many levels of symbolic links).
• Head: --work-dir `<A>/w` cannot be made: it is a symbolic link that leads round a loop, at the symbolic link `<A>/l0/…/l40`. The way out is "Remove the symbolic link <A>/w…". check and execute agree.
• l40 does not loop. readlink r39/l40 is ../r40, and r40 is a directory. stat of l0/…/l39 is ok. stat of l0/…/l40 and of l0/…/l41 gives errno 40, "Too many levels of symbolic links".

3. NIT-4: confirmed.

• On head the new row test_a_link_windows_cannot_resolve_is_named_as_a_loop_by_the_refusal passes. The whole two-file run is 124 passed (82 + 42).
• On base (a scratch copy of head's tree with 038dafba's adopt-project.py, sha256 0c1c1f3d…) the two files give 9 failed and 115 passed.
  • The 9 are the 3 N1 rows, the 3 N5 rows, the direct N1 test, winerror-1921 and the new row.
  • The new row fails with "it is a symbolic link that leads to …/self, which is not a directory".
• W2 and X9 each give 2 failed and 122 passed. The two failures are test_windows_cannot_resolve_is_a_loop_too[winerror-1921] and the new row. The mutant file read back at sha256 2c5e4f31…, equal to head, after every run.
• Extra mutant G1, which drops EEXIST from the errno gate: 25 failed, and the new row is one of them.
• The refactor lost nothing.
  • stat_fails_at is the old inline stat patch moved out unchanged.
  • The old row still asserts adopter._the_loop_in(link) == (link if loops else None).
  • With -v, [winerror-1921], [control-EINVAL] and [control-winerror-123] all PASSED.
  • The controls still kill their mutants: X2 fails 1 (control-winerror-123), X3 fails 2, X12 fails 2. These match r1143's earlier counts.

4. NIT-5: confirmed.

• POSIX_ONLY = pytest.mark.skipif(os.name == "nt", …) is at line 94 and is applied once, at line 363, to test_a_work_dir_read_as_the_link_in_the_loop_is_named_as_read. nothere appears in no other test.
• On Linux, head has 0 skips: -rfs over the two files lists none, and the three N5 rows PASSED.
• To see the scope, I changed the condition to os.name == "posix" in a scratch copy. The refusal file then gave 79 passed, 3 skipped, with SKIPPED [3] tests/test_adopt_work_dir_refusal.py:363. Exactly the three N5 rows skip, and 79 + 3 = 82.
• The style matches test_adopt_entry_paths.py:51, which has its own file-local POSIX_ONLY.
• conftest.WINDOWS_SKIP's reason is about setup.sh. No workflow job runs this test file; the tests-git-bash job runs three other files.

5. The body: true, except one list item (NIT-6 below).

• Table, measured with plan on base and head.
  • Row 1: base says "leads to <A>/d/self2, which is not a directory"; head says "leads round a loop, at the symbolic link <A>/d/self2".
  • Row 2: base says "the way to it leads round a loop, at the symbolic link <A>/self"; head says "read as <A>/self, it is a symbolic link that leads round a loop".
  • Row 3 (w -> loop/../d/self2/sub), for plan, check and execute:
    • Base: `<A>/d/self2/sub` … (Too many levels of symbolic links) with adopt-project.py check: refuse a kept file at the shape's pin path, and an unwritable --work-dir, before any leg exists #524's way out ("Pass a --work-dir this user can write in, or none…").
    • Head: `<A>/w` … leads round a loop, at the symbolic link `<A>/d/self2` with the way out for <A>/w.
    • execute prefixes "no leg repository was created and nothing was pushed."
    • All word for word as the body has them.
• NIT-2 bullet. Base and head both say `<A>/w` … leads to `<A>/dang/self2`, which is not a directory, as the bullet says.
• Counts.
  • The refusal file collects 53 tests at 038dafba and 82 at head, so 29 are new. test_adopt_clone_claimed.py is untouched at 42.
  • Base gives 9 failed and 115 passed; head gives 124 passed.
  • The 9 rows that pin a change are 3 + 1 + 3 + 2. The 20 controls are 6 + 3 + 6 + 3 + 2. The two add up to 29.
• W2 and X9 rows. The body says each gives 2 failed and 122 passed. I measured exactly that.
• Closing keywords.
  • closingIssuesReferences is [1074].
  • The body's only keyword hits are Closes #1074, a quoted "cannot resolve" and "not resolved (6)". Neither of those is followed by an issue number.
  • The commit messages contain only the fix/1074-… merge-message branch name.
• Draft and milestone. The PR is a draft, with no milestone and no body or title keyword beyond that.
• Comment, main and head. r1143's comment 6327473568 exists. Main is still 038dafba and the branch is still 7af5fabc.

6. Runs on head.

• Two PR files: 124 passed.
• tests/test_path_guard_hygiene.py: 49 passed.
• The doctor shards were not run, as you said.

7. CI for 7af5fabc, as it stands at 02:44:22Z.

Check	State
SonarCloud Code Analysis	pass
tests-git-bash	pass
package	pass
tests	pending (run 39711368187)

I did not wait for it.

Findings

MUST: none.

SHOULD: none.

NIT-1 (docstring over-generalises). adopt-project.py:17177-17182, the new sentence, says "the way out still names the --work-dir". That holds when the --work-dir is itself a link, as in contrived-toolong and the commit message. It does not hold when the --work-dir is typed through the fold.

• Probe toolong-typed: --work-dir <A>/loop/../l0/…/l45/x.
• Head says "the way to it leads round a loop, at the symbolic link <A>/l0/…/l40", and the way out says "Remove the symbolic link <A>/l0/…/l40".
• Base said "making it a directory failed (Too many levels of symbolic links)".
• Fix, optional: "the way out names the --work-dir where that is a link, else the link the walk names". Nothing is needed in code.

NIT-2 (body, a wrong item in the N6 list).

• The N6 bullet lists through among the paths os.path.realpath reads as the link itself (self, a for into -> a, through).
• Measured on head: realpath(self) is <A>/self (a link, mkdir gives EEXIST). realpath(into) is <A>/a (a link, EEXIST). realpath(through) is <A>/loop/sub, which is not a link and where mkdir gives ELOOP (errno 40).
• So through is a loop above the work dir, the case the body's own Windows bullet says N6 does not reach.
• The error came from my first review (it listed through among the reachable cases) and f1143 copied it.
• Fix: drop through from the parenthetical. For example, self, a for into -> a, or <A>/d/self2 for N1's w.

Nothing else is wrong. The probe dirs are under /workspace/projects/openRepoShape-worktrees/_lane1-scratch/r1143/r2/ (probes/, mutants-*.out, head-2files.out, base-2files.out, sim-posixonly.out, pr-body.md, probe.sh, mutate.py).

Hygiene.

• My worktree is untouched. It is at 7af5fabc3aaccb4243116cc9cbec868b7ea6779c, git status --short --ignored prints 0 lines, and git hash-object reads a1a7bfc9… and c0300f91…, equal to HEAD:'s blobs.
• No commit, no push, no GitHub write, and I ran no rm or rmdir.
• One slip: my first probe call sourced env.sh after setting S, so it wrote six contrived-toolong-{base,head}-{plan,check,execute} dirs into _lane1-scratch/w1074/probes/. I moved them with mv into _lane1-scratch/r1143/r2/stray-from-w1074-probes/ and fixed the script. Nothing else in w1074/ was touched.

Lane: openreposhape-1 (openRepoShape-1)

[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 6 days and 19 hours by commenting @sourcery-ai review. Upgrade to get a review now.