[brettheap]

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

Slice B of #430's plan (lane 1, issuecomment-6251069236): update-shape.py.
A v1.2.0 DRAFT. It lands only on a v1.2.0 merge word.

Decisions

The plan's rule, as it stands:

• Stat the path. ENOENT, ENOTDIR, EBADF and ELOOP (and Windows 21,
  123 and 1921) mean absent.
• Any other OSError is refused by name, as
  <path relative to the root>: cannot be read (<strerror>), exit 2.
• The advice is validate-pins.py's _unreadable_file: chmod u+r /
  chmod u+rx, or its OTHER_REASON for any cause other than permission.
• A local _is_a, with validate-pins.py:253's errno set.
• scripts/repo_shape.py is untouched.

One new code, update-unreadable, made in one place: main. read_pin
and prepare RAISE the refused read rather than refuse it. The doctor tells
the two apart:

• it names a raised OSError of a file in the root (refused_in_root, shape-doctor.py: for a pin or a contracts/ this user cannot read, the legs, manifest kinds and shape currency rows disagree with validate-pins.py's named refusal, and manifest kinds names a git checkout HEAD -- that discards work #847);
• it reads a Refusal out of prepare as CANNOT ANSWER, with a git fetch
  NEXT.

Per site (lines at 50178305):

Site	Decision	Why
:599 read_pin	raises	absent stays update-pin-missing; the doctor already catches an OSError from read_pin. A pin at mode 000, whose stat answers and whose read is refused, raises too (_read_yaml), where it was yaml-unreadable naming the absolute path with no remedy
:778 _classify_row	raises	absent, or not a regular file, stays copy-missing. A copy at mode 000, whose read already raised on every Python (a traceback), is named by main too
:2310 run_validator	refuses (a Refusal)	it runs after the writes, and cmd_apply rolls back on a Refusal only; absent stays is absent; SKIPPED
:291 root_kind	raises	only a root this user cannot search refuses either stat, and prepare asks that first
:2114 prepare	raises	the root's own stat (a folder above it this user cannot search); then NEW, os.stat(<root>/.), so a root it cannot search is named whole, not as a family.yaml a project has not got
:496/:498 open_upstream	raises	on 3.14, a path under a folder this user cannot search was "neither a path ... nor an owner/repo name" (and a relative a/b would have been cloned from github.com), and a checkout it cannot search was update-upstream-not-a-repo
:719/:721 merged_into	raises	os.path.lexists answers False to a refused lstat on EVERY Python, so a shape/ this user cannot search made the row merged-away, and apply would drop its pin row
:1893 ReadmeShapeLine._classify (and :1885)	a reading, never a raise	that class promises not to raise: a refused stat or lstat is unreadable, never absent. Prose: no exit code moves
:1147/:1174 symlink_on_the_way, not_a_directory_on_the_way	kept: "no link" (BENIGN)	every caller asks the path itself next, which now raises or refuses by name; and raising there would end the doctor's linked_record_row, which runs before read_pin and catches no OSError, in "could not be run" for a contracts/ at 000 that #847 names today

Two more in the same file, of the same class:

• :1230 find_additions raises. os.path.lexists read an
  upstream-added path under a folder this user cannot search as "nothing
  there", so check offered --add over a file the root may hold.
• :3212/:3227 Rollback._remove lists a refused stat as not removed.
  On 3.14 a refused stat answered False to is_dir() and exists(), so an
  --add file was neither removed nor listed. The rollback then said "the
  tree is exactly as it was". An --add file that something else took away
  before the rollback (a validator, the operator) is still not listed, as on
  50178305: there is nothing left to remove.

apply mid-run: never a half-written root.

• Before the first write: prepare makes every read the run judges by:
  the pin, the manifest's link, every pinned copy and every addition.
• After it:
  • run_validator refuses an unreadable validator, a Refusal;
  • cmd_apply gains an OSError arm. It turns any refused read with a
    file name into update-unreadable and hands it to _rolled_back, which
    puts back every byte and lists what it could not;
  • any other OSError is raised as it was.

A refused read closes the upstream, as a Refusal does (review of this
PR). prepare closed what open_upstream had opened on a Refusal alone.
A refused read out of classify, find_additions or merged_into went past
upstream.close() on its way to main. Without --upstream, every
update-unreadable from there left the bare clone of the pin's owner/repo
in the temporary directory, for check and apply. The handler is now
except (Refusal, OSError):.

No other except Refusal: holds an upstream or a temporary without a
finally:

• Upstream.resolve and _read_yaml hold nothing;
• main only prints the refusal;
• cmd_check and cmd_apply close their upstream in a finally;
• open_upstream removes its own temporary when the clone fails.

What changed

Measured by probes in a scaffolded root (local remotes, a stub gh), on
Python 3.12.3 and with a 3.14-like sitecustomize (Path.is_file,
is_dir, exists and is_symlink answer False to any OSError). Every
"after" is the same on both.

`scripts/` 444 (listable, not searchable): check, and apply --yes
  before, 3.12   Traceback ... in _classify_row ... PermissionError: [Errno 13]
                 Permission denied: '<root>/scripts/validate-pins.py'   (exit 1)
  before, 3.14   check: copy-missing scripts/validate-pins.py, and four more (exit 1)
                 apply: REFUSED update-conflict: ... are pinned and gone from the root (exit 2)
  after          REFUSED update-unreadable: scripts/validate-pins.py: cannot be read (Permission denied)
                 Remediation: this command reads the shape pin, the manifest, every copy the
                 shape pin lists and the --upstream checkout, and one it cannot read is a
                 question it cannot answer, never an absence. Make what this user may not open
                 readable by this user: `chmod u+r` on a file, `chmod u+rx` on a directory (and
                 on every directory above it inside the root), or fix who owns it. Then run the
                 same command again.                                     (exit 2)

`contracts/` 000: check
  before, 3.12   Traceback ... in read_pin ... PermissionError ... contracts/shape-pin.yaml (exit 1)
  before, 3.14   REFUSED update-pin-missing: <root>/contracts/shape-pin.yaml does not exist, ...
                 A project without a shape pin is adopted with `adopt-project.py`, not updated.
  after          REFUSED update-unreadable: contracts/shape-pin.yaml: cannot be read (Permission denied)

`.github/workflows/` 444: check
  before, 3.12   Traceback ... in _classify_row ... .github/workflows/validate.yml (exit 1)
  before, 3.14   copy-missing .github/workflows/validate.yml               (exit 1)
  after          REFUSED update-unreadable: .github/workflows/validate.yml: cannot be read (Permission denied)

the pin at 000, in a readable contracts/: check
  before, both   REFUSED yaml-unreadable: <root>/contracts/shape-pin.yaml: cannot be read (Permission denied)
                 (no remedy)
  after          REFUSED update-unreadable: contracts/shape-pin.yaml: cannot be read (Permission denied)

a copy at 000, in a readable scripts/: check
  before, both   Traceback ... in _classify_row ... read_bytes ... PermissionError (exit 1)
  after          REFUSED update-unreadable: scripts/bootstrap.py: cannot be read (Permission denied)

the root at 600: check
  before, 3.12   Traceback ... in root_kind ... '<root>/family.yaml'            (exit 1)
  before, 3.14   REFUSED update-pin-missing: <root>/contracts/shape-pin.yaml does not exist, ...
  after          REFUSED update-unreadable: <root>: cannot be read (Permission denied)
                 ... `chmod u+rx` on a directory (and on every directory above it), ...

--upstream under a folder at 000 / --upstream at 000
  before, 3.12   Traceback ... in open_upstream ... exists ...                  (exit 1)
  before, 3.14   REFUSED update-upstream-unresolvable: --upstream '<path>' is neither a path
                   on this machine nor an `owner/repo` name
                 REFUSED update-upstream-not-a-repo: --upstream <path> exists but is not a git
                   repository
  after          REFUSED update-unreadable: <path>: cannot be read (Permission denied)
                 REFUSED update-unreadable: <path>/.git: cannot be read (Permission denied)

in process (stand-ins), before -> after, on both:
  merged_into(root, "shape/Makefile"), shape/ at 000   'Makefile' (merged-away) -> PermissionError
  find_additions, docs/ at 000 holding docs/new.md    present False -> PermissionError
  README.md whose stat is refused                     3.12 raised / 3.14 `absent` ->
                                                      readme-shape-line: unreadable (README.md is not readable — Permission denied)
  Rollback.undo, an --add file whose stat is refused  3.14: "ROLLED BACK: ... exactly as it was" ->
                                                      "NOT put back ... docs/new.md, written by --add, was not removed (Permission denied): rm -- <path>"
  `scripts/` refused, the upstream a temporary clone   3.14: a false copy-missing, clone removed;
                                                      round 0 of this PR: update-unreadable, clone LEFT ->
                                                      update-unreadable, clone removed (check and apply)
  a read refused with EIO or ENAMETOOLONG             validate-pins.py's OTHER_REASON, never `chmod`

the doctor, under the 3.14-like shim (shape-doctor.py not edited)
  `scripts/` 444            DRIFTED (5 copy-missing) -> INVALID (pins)
  `.github/workflows/` 444  DRIFTED (1 copy-missing) -> INVALID (pins)

controls, unchanged on both: a root with nothing changed `nothing to do` (exit 0); a pinned
copy that is gone `copy-missing` and `apply`'s `update-conflict ... pinned and gone from the
root`; a pin that is gone `update-pin-missing`.

Tests

New file tests/test_update_shape_unreadable.py, 48 tests. On 50178305, 39 fail and 9 pass; the 9 are the controls, which pass on both. After the change, all 48 pass.

• The MODE cases use a real chmod, restored in a finally (mode_of). They skip on Windows and as root (needs_a_mode).
• The STAND-IN cases run in process with refused_stat, or a refused os.lstat, which answer as 3.14 does on any Python, root included.

Test	Kind	What it pins
test_check_names_what_it_cannot_read [scripts-444, contracts-000, workflows-444]	mode	#430's three measured check cells refuse update-unreadable by name, exit 2, with no copy-missing, does not exist or traceback
test_apply_names_what_it_cannot_read_and_writes_nothing	mode	scripts/ 444 under apply --yes: refused before the first write, pin untouched
test_a_refused_stat_is_named_on_every_python [3 cells x check, apply]	stand-in	the same six as 3.14 answers them
test_a_read_refused_in_prepare_closes_the_upstream [check, apply]	stand-in	scripts/ refused, with the upstream handed a temporary as the owner/repo clone is: update-unreadable, and the temporary removed (review of this PR)
test_a_copy_that_is_gone_is_still_copy_missing [this-python, as-3.14]	control	a copy that is NOT there is copy-missing, and apply update-conflict
test_a_pin_that_is_gone_is_still_missing	control	update-pin-missing, "does not exist"
test_a_readable_root_has_nothing_to_do	control	nothing to do, exit 0
test_a_pin_this_user_cannot_open_is_named_from_the_root	mode	a pin at 000: update-unreadable, never yaml-unreadable with an absolute path
test_a_copy_this_user_cannot_open_is_named	mode	a copy at 000: named, never a traceback
test_a_read_refused_is_named_on_every_python [the-pin, a-copy]	stand-in	the two above, with the read refused
test_a_read_refused_for_another_reason_gets_the_gates_other_advice [EIO, ENAMETOOLONG]	unit	a cause other than permission gets validate-pins.py's OTHER_REASON and RUN_AGAIN, read from the gate itself, and no chmod (review of this PR)
test_a_root_this_user_cannot_search_is_named_in_full	mode	a root at 600 is named whole, never as family.yaml; no "inside the root"
test_a_root_under_a_folder_this_user_cannot_search_is_not_missing	mode	never update-root-missing
test_a_root_whose_stat_is_refused_is_not_missing	stand-in	the same
test_root_kind_raises_a_refused_stat [2], test_root_kind_still_reads_what_is_there	stand-in, control	each manifest's refused stat is raised; the two kinds still read
test_an_upstream_this_user_cannot_read_is_named [2]	mode	--upstream under a folder at 000, or at 000 itself, is named in full
test_open_upstream_raises_a_refused_stat [3]	stand-in	each of open_upstream's three questions
test_a_shape_folder_this_user_cannot_search_is_no_merge	mode	merged_into raises, never reads the row as merged-away
test_merged_into_raises_a_refused_read [2], test_merged_into_still_reads_what_is_there	stand-in, control	both of its guards; a real merge still reads
test_an_addition_under_a_folder_this_user_cannot_search_is_named	mode	find_additions raises, never says "not present"
test_an_addition_whose_lstat_is_refused_is_raised, test_an_addition_still_reads_what_is_there	stand-in, control	the same
test_a_readme_this_user_cannot_stat_is_unreadable_not_absent [stat, lstat]	stand-in	the README is unreadable, never absent and never a raise
test_a_validator_whose_stat_is_refused_is_not_skipped, test_a_validator_that_is_not_there_is_still_skipped	stand-in, control	update-unreadable, never SKIPPED; an absent validator is still SKIPPED
test_a_validator_unreadable_after_the_writes_rolls_them_back	stand-in	mid-run: the refusal, and every byte put back
test_any_read_refused_after_the_writes_rolls_them_back	stand-in	cmd_apply's OSError arm rolls back
test_a_rollback_that_cannot_stat_what_it_made_says_so	stand-in	the rollback lists the --add file it could not remove
test_an_add_file_gone_before_the_rollback_is_not_listed	control	an --add file already gone is not listed as left, and its folder is removed (review of this PR)

Mutants: 24 one-line mutants, run on the final file, each killed:

• M1 and M2 put back read_pin's and _classify_row's is_file();
• M3 run_validator's;
• M4 and M5 root_kind's two;
• M6 prepare's is_dir();
• M7 drops _search;
• M8 to M10 put back open_upstream's three;
• M11 and M12 merged_into's lexists and is_file();
• M13 find_additions' lexists;
• M14 and M15 the README's is_file() and is_symlink();
• M16 _remove's exists();
• M18 drops cmd_apply's OSError arm;
• M19 drops main's;
• M20 keeps yaml-unreadable;
• M21 drops ENOENT from the absent set;
• M22 always says "inside the root";
• X16o puts back prepare's except Refusal:, and fails both upstream cases on the temporary left behind;
• X8 gives every cause the chmod advice, and fails both advice cases;
• X15 makes _remove's elif mode is not None: an else:, and fails the rollback case, which then says "ROLLED BACK IN PART ... docs/new.md, written by --add, was not removed (No such file or directory)".

(M17, _remove's is_dir(), was retired when _remove came to ask one stat: of the two guards it had, either alone masked a mutant of the other.)

Targeted runs, one pytest at a time, /usr/bin/python3 3.12.3:

Set	On 50178305	Round 0 (7086e8ca)	This head (59b12500d)
tests/test_update_shape_unreadable.py	39 failed, 9 passed	43 passed (of 43)	48 passed
tests/test_update_shape*.py, the first 9 files	335 passed, 3 skipped	335 passed, 3 skipped	335 passed, 3 skipped
tests/test_update_shape*.py, the other 9	418 passed	418 passed	434 passed (main's merge added tests)
test_shape_doctor.py and six test_doctor_* files that run update-shape	238 run, none failed (stopped, host load 300)	831 passed, 9 skipped	836 passed, 9 skipped
the trio (test_repo_hygiene.py, test_cli_reference.py, test_agents_md_codes.py)	not run	148 passed	170 passed
the trio and the other files that name what changed (14 files)	not run	1142 passed, 3 skipped	not run again
test_family.py -k update_shape	not run	4 passed, 108 deselected	not run again
the tests that scan the shipped tools' source (7 files)	not run	460 passed	not run again

The reviewer's runs (rv976), on the merge of 59b12500d with main 3864b18cc:

• the doctor batch, 6 files: 650 passed, 3 skipped;
• the hygiene and source-scanning files: 528 passed, 1 skipped;
• test_family.py -k update_shape: 4 passed.

Nothing is red after the change, so no test needed a base run to tell whether it was red before. scripts/render-cli-reference.py --check: ok.

Not changed

• shape-doctor.py is slice C's, so it is not edited. These are left for it (lines at 50178305):
  • refusals_ahead (:3227, :3236) catches only a Refusal from read_pin and prepare. A refused read now raises there on 3.14, as it already did on 3.12. It is reached only from with_merged_away_rows, which needs the pins gate at exit 1 with only merged-away findings, and the root's own validate-pins.py refuses an unreadable path at exit 2 first. The review of this PR found it cannot be reached today.
  • merged_away_findings (:3118) calls merged_into, and linked_record_row (:6231) calls root_kind, with no OSError handler. Reaching either needs the same gate exit, or a root that cannot be searched, which is the root check slice C adds.
  • Under the shim the doctor's manifest row for scripts/ 444 runs the TEMPLATE validator (validator_for, :2000); on 3.12 the pins and manifest rows say could not be run: [Errno 13].
• Not changed in update-shape.py, as BENIGN or not reachable (lines at 50178305):
  • :2840 write_added's p.exists(): it is update-write-failed on both Pythons, and rolled back.
  • :3035, :3132 and :3148: the rollback's deliberate os.path calls. They choose a by-hand line's words, never a write. One wording gap: a folder this user cannot SEARCH is told "make writable", where its cure is chmod u+x.
  • :3549 hard_links: the write re-asks with open.
  • :4595 _extend_the_eol_block's is_file() on .gitattributes: a root-level file, refused only for a root prepare has refused. Slice Z's hygiene test may list it.
  • :5258 _differs_from_head.
  • _imports_on_disk (:4071) and _names_defined (:4398): "what it cannot tell, it does not refuse".
  • Upstream.close (:369): its own temporary directory.
• Other limits:
  • Only a REFUSED read after the first write is turned into a rolled-back refusal. Any other OSError there (a file gone between the stat and the read, a full disk on a read) still escapes apply as it did, with nothing put back: cmd_apply's new arm raises it again (:5166-5167 at this head). The review of this PR named it, and it goes to a follow-up.
  • A manifest at mode 000 is not read by check. apply refuses it later as yaml-unreadable, with the absolute path.
  • update-unreadable names the FIRST path it cannot read, where validate-pins.py's workflow-unreadable lists every one. For a folder, its cure covers them all.
• scripts/repo_shape.py is untouched. No --help text changed, and no document lists update-shape.py's codes apart from the update-add-* sentence.

Part of #430

🤖 Generated with Claude Code

Summary by Sourcery

Make update-shape.py treat unreadable paths as explicit refusals rather than absences, while preserving rollback safety and cleanup.

New Features:

• Report unreadable paths consistently as update-unreadable with path-specific remediation instead of treating them as missing or emitting tracebacks.
• Rollback changes made by apply when a required read becomes unreadable after writes begin.

Bug Fixes:

• Distinguish unreadable files, directories, roots, upstreams, additions, merged paths, and validators from genuinely absent paths across supported Python versions.
• Ensure unreadable upstreams and temporary clones are closed and cleaned up on failure.
• Preserve readable README classification and distinguish absent validators from validators that cannot be read.

Enhancements:

• Centralize filesystem checks so only absence-related errors are treated as missing while other operating-system errors propagate for user-facing handling.
• Improve rollback reporting when files cannot be inspected or have already disappeared.

Documentation:

• Document unreadable-path behavior and its exit-code semantics in update-shape.py help and documentation text.

Tests:

• Add comprehensive coverage for unreadable roots, pins, copies, upstreams, additions, validators, README files, and rollback behavior across Python filesystem semantics.

[sourcery-ai]

Reviewer's Guide

Updates update-shape.py to consistently distinguish unreadable paths from missing paths across Python versions, report them as named update-unreadable refusals with actionable remediation, and preserve transactional apply behavior through rollback; adds 43 regression tests covering filesystem permissions, refused stat/lstat operations, controls, and mid-run failures.

Sequence diagram for preflight unreadable-path refusal

sequenceDiagram
    participant User
    participant Main as main
    participant Prepare as prepare
    participant PathChecks as _is_a/_mode_of
    participant Refusal as unreadable_refusal

    User->>Main: check or apply
    Main->>Prepare: prepare(args)
    Prepare->>PathChecks: stat root and required paths
    PathChecks-->>Prepare: OSError for refused read
    Prepare-->>Main: raised OSError
    Main->>Refusal: unreadable_refusal(root, exc)
    Refusal-->>Main: update-unreadable Refusal
    Main-->>User: named path, remediation, exit 2

Loading

Sequence diagram for transactional apply rollback on unreadable reads

sequenceDiagram
    participant User
    participant Apply as cmd_apply
    participant Prepare as prepare
    participant Ledger
    participant Validator as run_validator
    participant Rollback as _rolled_back

    User->>Apply: apply --yes
    Apply->>Prepare: prepare(args)
    Prepare-->>Apply: validated plan
    Apply->>Ledger: perform writes
    Apply->>Validator: run_validator(root, script)
    Validator-->>Apply: Refusal(update-unreadable)
    Apply->>Rollback: _rolled_back(refusal, ledger)
    Rollback-->>Apply: restore written bytes and report failures
    Apply-->>User: update-unreadable, rolled back

Loading

Flow diagram for unreadable-path handling in update-shape.py

flowchart TD
    A["update-shape.py inspects a path"] --> B{_mode_of: stat or lstat}
    B -->|ENOENT, ENOTDIR, EBADF, ELOOP, or Windows absent error| C["Treat as absent"]
    B -->|Other OSError| D["Raise refused read"]
    C --> E["Existing missing-path behavior"]
    D --> F["main calls unreadable_refusal"]
    F --> G["REFUSED update-unreadable with named path and remediation"]

Loading

File-Level Changes

Change	Details	Files
Introduce cross-version detection that distinguishes unreadable paths from genuinely absent paths.	Add errno-aware stat and lstat helpers that preserve refused OSErrors while treating only the established absence errors as missing. Replace path predicates across root, pin, copy, upstream, merge, addition, validator, and rollback handling. Ensure unreadable paths are named relative to the root, or fully for the root and upstream paths, with permission-specific or fallback remediation.	update-shape.py
Route unreadable-read failures through consistent refusal and rollback behavior.	Add the update-unreadable refusal code and centralize formatting of path, cause, and remediation. Keep read_pin and prepare exceptions raisable for doctor integration while converting top-level failures into named refusals. Convert validator failures and post-write OSErrors into rollback-triggering refusals; report rollback stat failures instead of silently claiming success.	update-shape.py
Preserve non-raising README classification while correcting unreadable-state reporting.	Classify refused README stat or lstat operations as unreadable rather than absent. Leave intentional benign path-walk behavior unchanged where callers subsequently inspect the target path.	update-shape.py
Add comprehensive regression coverage for permission and refused-stat behavior.	Test real permission modes and Python-3.14-like stat/lstat shims across check and apply. Verify absent controls remain copy-missing, pin-missing, or validator SKIPPED. Verify pre-write refusal prevents writes, post-write refusal restores all bytes, and rollback reports files it cannot inspect or remove.	tests/test_update_shape_unreadable.py

Possibly linked issues

• Every Path.is_file() / is_dir() / exists() guard in the shipped tools answers False to any OSError on Python 3.14 (3.12 and 3.13 raise PermissionError), so a directory this user cannot search reads as ABSENT: validate-pins.py skips its workflow @<sha> check, update-shape.py says copy-missing and update-pin-missing ("adopt the project") for files that are there, the doctor says DRIFTED (5 copy-missing), and bump-leg.py's remaining guards, and every other tool's, skip or misreport instead of refusing by name #430: Link: this is explicitly slice B of issue Every Path.is_file() / is_dir() / exists() guard in the shipped tools answers False to any OSError on Python 3.14 (3.12 and 3.13 raise PermissionError), so a directory this user cannot search reads as ABSENT: validate-pins.py skips its workflow @<sha> check, update-shape.py says copy-missing and update-pin-missing ("adopt the project") for files that are there, the doctor says DRIFTED (5 copy-missing), and bump-leg.py's remaining guards, and every other tool's, skip or misreport instead of refusing by name #430 and fixes its update-shape.py cases, including missing copies, pins, and skipped validators.

---

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-2, 2026-10-19T21:51:00Z) Independent review of #976 (#430 slice B) at 59b12500d, for lane openRepoShape-2: READY

Fixed since 7086e8ca (all three re-checked at 59b12500d, on the merge with main 04923f10d):

• Was a MUST: update-shape.py:2373 (prepare). prepare closed the upstream only on except Refusal:. A refused read raised out of classify, find_additions or merged_into therefore skipped upstream.close(). With no --upstream, the default clones the pin's owner/repo into a temporary folder, so every update-unreadable from that block left that clone behind.
  • It is now except (Refusal, OSError):.
  • test_a_read_refused_in_prepare_closes_the_upstream[check, apply] covers it. With the fix reverted, both tests fail.
  • My own probe passes all 7 cases.
• Was a SHOULD: update-shape.py:431 (unreadable_refusal). No test held which advice a refusal gets, the chmod advice or OTHER_REASON.
  • test_a_read_refused_for_another_reason_gets_the_gates_other_advice[EIO, ENAMETOOLONG] now checks it against validate-pins.py's own OTHER_REASON, word for word.
  • The mutant that gives every cause the chmod advice now fails 2 tests.
• Was a NIT: update-shape.py:3504 (Rollback._remove). Changing elif mode is not None: to else: survived every test.
  • test_an_add_file_gone_before_the_rollback_is_not_listed now catches it (1 failed, 69 passed).

Findings

1. NIT (a follow-up, not this PR): update-shape.py:5160 (cmd_apply's new OSError arm).
   • Only a read the system refused after the first write is rolled back. Any other OSError there still escapes with the tree half-written, for example a file gone between the stat and the read.
   • Ruling: accept it as the known limit the body names. Before this PR, every OSError after the first write escaped, so the PR only narrows the gap.
   • The fix is to roll back on any OSError and then re-raise. That changes how the tool answers errors other than Every Path.is_file() / is_dir() / exists() guard in the shipped tools answers False to any OSError on Python 3.14 (3.12 and 3.13 raise PermissionError), so a directory this user cannot search reads as ABSENT: validate-pins.py skips its workflow @<sha> check, update-shape.py says copy-missing and update-pin-missing ("adopt the project") for files that are there, the doctor says DRIFTED (5 copy-missing), and bump-leg.py's remaining guards, and every other tool's, skip or misreport instead of refusing by name #430's, so it belongs in its own issue.
2. NIT (for slice C or slice Z): shape-doctor.py:3228 and :3237 at main (refusals_ahead).
   • They still catch only a Refusal, and so does shape-doctor.py and shape_advisory.py: a path in the root this user cannot reach is named on every Python, never read as absent or stood in for by the template, a root it cannot search is refused first, and a row that could not ask is never DRIFTED #975 at 3fd80cbe (:3278, :3287).
   • A pin at mode 000 in a readable contracts/ now raises out of read_pin there on every Python, where before it was a Refusal.
   • Ruling: no change in update-shape.py: a path in the root or the upstream this user cannot read is update-unreadable on every Python, so a scripts/ it can list but not search is no longer five copy-missing and a validator it cannot stat is never SKIPPED #976. refusals_ahead is reached only after the root's own pins gate exits 1 with nothing but merged-away findings. A current validate-pins.py refuses an unreadable pin or copy with exit 2 first. Otherwise the doctor's run_checks catches an OSError, so the raise is contained.
   • If you want, add a except (Refusal, OSError) there in slice C or Z.

Checked and true

• The rule at every changed guard.
  • _is_a, _lexists and _is_link treat ENOENT, ENOTDIR, EBADF, ELOOP and Windows 21, 123 and 1921 as absent, and raise any other OSError.
  • scripts/repo_shape.py is untouched; the diff has two files.
  • The sites it covers: read_pin, _classify_row, root_kind, prepare and its new _search, open_upstream's three questions, merged_into, find_additions, the README reader, run_validator and Rollback._remove.
• The sites the writer called benign. I agree with each. Line numbers are at 7086e8ca:
  • write_added's parent exists() (:3103) runs inside ledger.writing, so a failure is update-write-failed and rolled back;
  • the rollback's os.path calls (:3298, :3395, :3411) only choose the words of a by-hand line;
  • hard_links;
  • the .gitattributes is_file();
  • _differs_from_head, _imports_on_disk and _names_defined, whose paths prepare has already read;
  • Upstream.close.
• The link walk keeps "no link" on purpose, and a test holds it. Making symlink_on_the_way raise fails 2 tests.
• The tests fail on the old code. The new file at 7086e8ca, run against main's update-shape.py, gave 35 failed and 8 passed, and the 8 are the controls.
• Raising instead of refusing is right for the doctor.
  • check_shape_currency names an OSError from read_pin (pin_refusal_row) and from prepare (refused_in_root), and reads a Refusal from prepare as CANNOT ANSWER.
  • The doctor is the only caller of read_pin or prepare outside update-shape.py.
• apply is never left half-written in the cases I tried:
  • a read refused after the first write is rolled back;
  • a validator refused partway through is rolled back;
  • an --add path under a folder this user cannot search is refused in prepare, before any write;
  • an --add file whose stat is refused at rollback time is listed as not removed;
  • a root at mode 600 is named in full.
• The admitted limit that only the first unreadable path is named: I accept it. The chmod u+rx cure for a folder covers every file under it.
• The docs.
  • No document lists update-shape.py's refusal codes apart from the update-add-* sentence.
  • tests/test_agents_md_codes.py checks only from the document to the tools.
  • tests/test_cli_reference.py passes.
• Sonar, by my own estimate.
  • No changed function goes over 15 for cognitive complexity. The highest are ReadmeShapeLine._classify at 14, Rollback._remove at 14, and cmd_apply and main at 12.
  • The test file has no composite asserts.
  • The only 4-line repeat shared with another test file is an import block.
• Merges. 59b12500d merges clean with main 3864b18cc, and with shape-doctor.py and shape_advisory.py: a path in the root this user cannot reach is named on every Python, never read as absent or stood in for by the template, a root it cannot search is refused first, and a row that could not ask is never DRIFTED #975 at both 76ebca2e and 3fd80cbe.
• Tests on the merge with 3864b18cc:
  • tests/test_shape_doctor.py and five tests/test_doctor_*.py files: 650 passed, 3 skipped;
  • 11 hygiene and source-scanning files: 528 passed, 1 skipped;
  • tests/test_family.py -k update_shape: 4 passed.
• Tests on the merge with 04923f10d: the new file, 48 passed.
• Tests on the merge with c7a3ff16 (the 18 tests/test_update_shape*.py files have not changed since): the first 9 gave 335 passed and 3 skipped, the other 9 gave 434 passed.

Could not verify

• A real Python 3.14. This host has 3.12.3; I used the in-process stand-ins and real chmod modes, as uid 1000.
• Windows, and a run as root.
• Sonar's own numbers: the SonarQube connection failed, so the complexity figures are my estimate. The coordinator reports SonarCloud passes at 59b12500d.
• A real network clone for --upstream owner/repo. I tested it with an in-process stand-in.
• The live PR body's wording on :5160. I never run the real gh, and the writer's attachments are dated 2026-10-18.
• The doctor tests on a three-way merge with shape-doctor.py and shape_advisory.py: a path in the root this user cannot reach is named on every Python, never read as absent or stood in for by the template, a root it cannot search is refused first, and a row that could not ask is never DRIFTED #975.

Lane 2's next step: #976 is green at 59b12500d (tests and SonarCloud), so lane 2 gates it on the predicted main and asks for the turn on #473. The two NITs go to follow-ups.

Lane: openreposhape-2 (openRepoShape-2)

[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 1 day and 21 hours by commenting @sourcery-ai review. Upgrade to get a review now.