Skip to content

update-shape.py: prepare asks the shape pin's source_repository: once, with --upstream passed too, and refuses a path or a value that is no owner/repo in the pin's name; shape-doctor.py answers as it did (#1203) - #1269

Draft
brettheap wants to merge 4 commits into
mainfrom
fix/1203-pin-source-asked-once
Draft

update-shape.py: prepare asks the shape pin's source_repository: once, with --upstream passed too, and refuses a path or a value that is no owner/repo in the pin's name; shape-doctor.py answers as it did (#1203)#1269
brettheap wants to merge 4 commits into
mainfrom
fix/1203-pin-source-asked-once

Conversation

@brettheap

@brettheap brettheap commented Nov 8, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Goal

update-shape.py asks the shape pin's source_repository: once, whichever way a run reaches the upstream, and refuses a value that is no owner/repo in the pin's name, never in the name of a flag nobody passed (#1203, from lane 5's review of #1166, findings 3 and 9).

What held on main

Measured on main be21751c3 (Python 3.12.3, git 2.43.0, a stub gh first on PATH, a bogus token, no network). This branch's rows ran against main's update-shape.py and shape-doctor.py in a scratch worktree, and two probes ran main's tool directly.

What changed

  • update-shape.py: a new pin_source(pin, pin_path). prepare asks it once, after reading the pin and before opening the upstream, with or without --upstream. It asks three questions, under the codes these values already had:
    • Something is at the value where the command runs, and it holds no git repository: update-upstream-not-a-repo. The value is looked up the way open_upstream looks up --upstream: expanduser, relative to the working directory.
    • REPOSITORY_RE does not take the whole value (fullmatch): update-upstream-unresolvable.
    • checked_value refuses it: unsafe-value.
  • What each refusal says. Each names the pin and its field, <root>/contracts/shape-pin.yaml: `source_repository:` .
  • open_upstream(spec) asks only about --upstream again. validate-manifest.py, validate-family.py, family.py and update-shape.py: an id:, a date or a repository: ending in a line feed is refused by name, and a pin's source_repository: that is no path or owner/repo name is named as the pin's (#1139) #1166's pin_path and its two pin-named branches moved into pin_source. Both functions use _holds_a_repository as their one repository question.
  • prepare(args, *, ask_source=True). shape-doctor.py's check_shape_currency call passes ask_source=False. Its refusals_ahead call asks the field (S2 of the review round, below).
    • Why: the doctor reads any Refusal out of prepare as its own checkout being unable to answer. update-shape.py's comment above _ABSENT_ERRNOS documents this.
    • Measured with the field asked there: a hand-edited source_repository: turned the shape currency row from exit 1 (behind) into CANNOT ANSWER, exit 3. Its NEXT was git -C <standard> fetch --all, and its why was "this checkout of the standard does not carry the commit the pin names".
    • With the opt-out at check_shape_currency the doctor answers as it does on main. Where the root is behind, the doctor's NEXT is update-shape.py check, which refuses the field in the pin's name.
  • Codes: none added, renamed or removed.
  • Tests:

Review round (rv1269: two SHOULDs)

rv1269 (opus) found this PR NOT READY at 86c1e6ea2 on two SHOULDs and six NITs (#1269 issuecomment-6725508050). It confirmed the fix, its rows, mutants, gates and merge-trees, and the writer's three choices. This round fixes the two SHOULDs and N1 to N5, keeps N6 as it was, and changes nothing else. Commits: 175d9b718 (the code and its rows) and c792615f4 (one more row).

  • S1. A pin value ~<unknown user>/... was a Python traceback, exit 1.
    • Path(declared).expanduser() raises RuntimeError: Could not determine home directory. for a user the machine does not have (measured on 3.12; rv1269 found 3.13 and 3.14 raise the same). Exit 1 is also check's "behind" code. With --upstream passed this was new (main exited 1 with a report); with none, main crashed too.
    • Fix: a new _named_here(declared) looks the value up as open_upstream does and, where expanduser raises, keeps the bare path. Nothing is there, so the name question refuses it update-upstream-unresolvable in the pin's name, in both routes. The scaffolded value is unchanged.
    • Rows (4, each failing at 86c1e6ea2): test_a_tilde_user_who_is_not_there_is_refused_in_the_pins_name, and test_a_tilde_user_who_is_not_there_is_looked_up_as_the_bare_path (a folder of that very name where the command runs, holding no repository, is update-upstream-not-a-repo), each in both routes.
  • N1. A first part over 255 bytes was update-unreadable ... (File name too long), with a permissions remedy, naming neither the pin nor the field.
    • Fix: _named_here takes ENAMETOOLONG, and only that errno, as nothing there. Every other refused stat is raised as it was.
    • Rows (5): "r" * 300 + "/has space" is update-upstream-unresolvable and "r" * 300 + "/x" is unsafe-value (checked_value refuses over 255 characters), each in the pin's name and in both routes (4 rows, each failing at 86c1e6ea2); and test_any_other_refused_stat_of_the_pins_value_is_raised_as_it_was, a control that passes at 86c1e6ea2 too: a PermissionError out of the stat is raised out of pin_source.
  • S2. The doctor's refusals_ahead opt-out made a pins row sentence false.
    • Over two collision copies merged away plus source_repository: "has space/x", the pins row said "update-shape.py check reports them merged-away, and the apply it prints drops their pin rows", and its NEXT was that check, which exits 2 with update-upstream-unresolvable and reports nothing.
    • Fix: ask_source=False is dropped at refusals_ahead only. prepare refuses, so refusals_ahead returns None, as its docstring says it does where prepare refuses, and the pins row keeps the gate's finding. check_shape_currency keeps the opt-out. The scaffolded case, with the same two copies, is unchanged: test_a_merged_away_copys_row_goes_and_the_project_is_green_again still passes and still asserts merged_away and the check NEXT. rv1269's mutant R1 (refusals_ahead asks) is now the code.
    • Row: tests/test_update_shape_merged_away.py::test_the_pins_row_keeps_the_gate_where_check_refuses_the_pins_source, failing at 86c1e6ea2. It also holds that check exits 2 update-upstream-unresolvable, and that the shape currency row still counts the two rows merged-away, with no CANNOT ANSWER.
  • N2. The Limits bullet on a non-repository opensoft/openRepoShape folder is now measured (Limits, below). The remedy of update-upstream-not-a-repo for a pin's value ends with "The value is also looked up as a path from the directory this runs in: if it is already right, run this from a directory where nothing is at that path." The other pinned words did not change: WRITE_THE_PIN stays what validate-manifest.py, validate-family.py, family.py and update-shape.py: an id:, a date or a repository: ending in a line feed is refused by name, and a pin's source_repository: that is no path or owner/repo name is named as the pin's (#1139) #1166's rows and the name refusals say.
    • Rows (3): the scaffolded pin run from a folder holding a non-repository opensoft/openRepoShape, in both routes (failing at 86c1e6ea2: no such sentence); and the same pin with --upstream run from a folder where nothing is at that path, exit 1 as the scaffolded pin gives (a control).
  • N3. The comment above REPOSITORY_RE no longer says it is the pin's value "where no --upstream is passed".
  • N4. test_the_doctor_does_not_call_a_hand_edited_source_the_standards also asserts the shape currency row's status is ok, main's answer (measured on main's code in the scratch worktree, ok). Mutant D1 below is what only that assert kills.
  • N5. The Merges section below is refreshed: main and the PRs were read fresh.
  • N6. The Sourcery summary below the footer is kept exactly as it was: no closing keyword; body-guard exits 0.

Rows failing at 86c1e6ea2's update-shape.py and shape-doctor.py (this round's tests, the scratch worktree detached at c792615f4): 19 failed, 34 passed of 53 tests. The 19: 8 rows whose expected remedy gained N2's sentence (the issue's three path values in both routes, and ~/notes in both), the 4 S1 rows, the 4 N1 rows, the 2 N2 rows, and the S2 row. The controls that pass at 86c1e6ea2 are the N1 PermissionError row, the N2 run-elsewhere row, and the N4 assert.

The decision: a pin that names a path, with --upstream passed

Refused.

  • Why. The value is the name prepare gives the upstream in both routes. The README Shape: line is compared with it, and apply writes it into its commit subject, its pull request's title and the lines it prints.
  • Cost. No tool writes a path there: both templates hold {{SHAPE_REPOSITORY}}. So refusing one costs no root that a tool made.
  • One answer for both routes. The same question is asked whether or not --upstream is passed:
    • A value that names something here holding no repository (contracts, scripts/repo_shape.py, ~/notes) is update-upstream-not-a-repo, with or without --upstream.
    • A path not spelled owner/repo (~/shape, /srv/shape) is update-upstream-unresolvable, even where it holds a clone. This is the one answer that changes for a run with no --upstream: main opened such a clone in place.
    • An owner/repo spelling that is a repository where the command runs is taken, as on main. An example is a checkout laid out by owner, run from the folder above opensoft/openRepoShape. With no --upstream it is opened in place.

Rows failing on main

Command: this branch's tests/test_update_shape_pin_source.py and tests/test_value_pattern_line_break.py, and the S2 row of tests/test_update_shape_merged_away.py, run against main's (be21751c3) update-shape.py and shape-doctor.py, in the scratch worktree detached at c792615f4.

Result: 33 failed, 33 passed in 47.41s (the first round's two files gave 21 failed, 32 passed).

The 33 are the first round's 21 and the review round's 12 (S1: 4, N1: 4, N2: 2, S2: 1, and the PermissionError row, which fails on main because there is no pin_source). The first round's 21:

Four rows pass on main by design, because they hold answers this change keeps:

  • the scaffolded pin with no --upstream reaches open_upstream with opensoft/openRepoShape (in-process, with a stand-in that opens the local upstream);
  • the owner/repo clone where the command runs is opened in place;
  • the doctor's shape currency row is no environment row and the doctor does not exit 3, and its status is ok;
  • the scaffolded pin with --upstream passed, run from a folder where nothing is at opensoft/openRepoShape, is exit 1 over the upstream one commit on.

Mutants

Setup:

The review round's mutants:

Mutant Result Killed by
S1 the RuntimeError guard removed 4 failed the S1 rows, both routes
S1 the bare path not kept (return None) 2 failed the bare-path row, both routes
N1 the ENAMETOOLONG test removed 4 failed the four long-name rows
N1 every OSError taken as nothing there 1 failed the PermissionError row
S2 the opt-out put back at refusals_ahead (the inverse of rv1269's R1, which is the code now) 1 failed the S2 row
N2 the sentence removed from the not-a-repo remedy 12 failed the three path values and ~/notes (both routes each), the two N2 rows, the bare-path rows
N2 the sentence added to the name refusals too 15 failed the name refusals: the issue's three name values and ~/shape, the tilde and long-name rows, named-first, #1166's two
D1 the doctor's shape currency row asks the field and calls a refusal a plain FINDING (exit 1, no environment) 1 failed the guard row's new status assert
D1 with N4's assert undone survives (1 passed) nothing: that assert alone kills D1
N4's assert undone, production as committed 53 passed a control: the assert holds main's answer

The first round's mutants, run again:

Mutant Result Killed by
M1 the question removed from the --upstream route 17 failed every upstream-a-clone row, checked_value, named-first, apply, the S1, N1, N2 and S2 rows
M2 the question removed from the no---upstream route 18 failed every no-upstream row, #1166's three pin rows, the doctor row, the S1, N1, N2 and S2 rows
M3 fullmatch put back to match survives: equivalent (53 passed) REPOSITORY_RE ends in \Z since #1166, so match and fullmatch agree on every string (rv1269 checked 66,430)
M3b match and REPOSITORY_RE's \Z put back to $ 7 failed the line-feed rows (both routes), named-first, apply, #1166's line-feed rows, the pattern's own row
M4 the path question removed 12 failed the three path values and ~/notes, both routes; the bare-path and N2 rows
M5 any path refused, a repository or not 3 failed ~/shape both routes, the owner/repo clone where it runs
M6 checked_value dropped 4 failed the checked_value rows and the long-name unsafe-value rows
M7 no expanduser 2 failed ~/notes both routes
M8 asked after the upstream is opened 18 failed every no-upstream row but ~/shape, named-first, the doctor row, #1166's three, the S1, N1, N2 and S2 rows
M9 the doctor's shape currency call asks the field 2 failed the doctor row and the S2 row
M10 ask_source defaults to False 17 failed as M1

Runs

Gates ran on the commit c792615f4 (dirty=0), with a stub gh first on PATH, GH_TOKEN bogus, an empty GH_CONFIG_DIR, a blank git identity, GIT_SSH_COMMAND=false, GIT_CONFIG_GLOBAL=/dev/null, Python 3.12.3 and git-filter-repo 2.47.0.

  • git 2.43.0 (/usr/bin/git):
    • Gate A: the 20 tests/test_update_shape*.py files (this PR's new one among them), tests/test_value_pattern_line_break.py, the hygiene five (test_path_guard_hygiene, test_repo_hygiene, test_agents_md_codes, test_cli_reference, test_pinned_copy_text) and test_no_auto_maintenance. 1352 passed, 5 skipped in 699s (the first round: 1339; this round adds 12 rows to the new file and 1 to test_update_shape_merged_away.py).
    • Gate B, run because shape-doctor.py changed: test_shape_doctor, test_doctor_unreadable_pin, test_doctor_next_is_a_command, test_doctor_partial_clone, test_doctor_pin_restore_merges, test_shape_pin_files_not_a_list, test_pin_source_env_collisions and test_bom_reads. 824 passed, 3 skipped in 1045s.
  • git 2.47.1 (built from kernel.org's tarball, first on PATH):
    • Gate A, in 17 runs of fewer than 100 tests each, because a gate on 2.43.0 held the one slot for a run over 100: 65 + 880 + 407 = 1352 passed, 5 skipped, the same count as on 2.43.0.
    • Gate B, by node id in ten runs of at most 90: 824 passed, 3 skipped, the same count as on 2.43.0.
  • The skips are the same on both gits: 3 test_update_shape_add_symlink.py:832 (junctions are NTFS's), 2 test_no_auto_maintenance.py:222 (the session began with GIT_CONFIG_GLOBAL set) and 3 test_shape_doctor.py:10516 (the PowerShell line, Windows only).
  • The first round's runs, on e1692c147 and 86c1e6ea2: gate A 1339 passed, 5 skipped and gate B 824 passed, 3 skipped, on both gits. A first doctor run on 2.47.1, at e1692c147, had failed three rows of test_doctor_unreadable_pin.py: their stand-in for prepare took one argument and the doctor now passes ask_source. 86c1e6ea2 widens the stand-in. rv1269 re-ran both gates at 86c1e6ea2 with the same counts.
  • The rows on main, the rows at 86c1e6ea2 and the mutants above ran on git 2.43.0.

Merges

Run with git merge-tree --write-tree <other> c792615f4 (git 2.43.0) at 2026-11-08T19:34Z, at heads read fresh just before the push. All are clean (exit 0, no conflict line):

Main, #1248 and #1172 touch shape-doctor.py. None of their added or removed lines in update-shape.py, shape-doctor.py or the tests this PR touches mentions open_upstream, pin_path, prepare(, source_repository, WRITE_THE_PIN, REPOSITORY_RE, refusals_ahead, ask_source, pin_source, expanduser or ENAMETOOLONG (read from git diff against each merge base). The rows on the merged tree with current main are left to the landing gate. Lane 3 says the later lander takes the refresh.

Limits

  • The doctor does not name a hand-edited source_repository: itself, as on main: where the pin names the standard's commit, its row is ok. Naming it on the pin's side would mean teaching pin_restore, whose history walk reads pins with check_pin, the same question, and the path half of that question reads the working directory. That is a follow-up candidate, not this PR.
  • The path question reads the working directory, as open_upstream does for --upstream. So scripts/repo_shape.py is refused when run from the root, and taken as an owner/repo name when run from elsewhere. For any one directory, both routes give the same answer.
  • A folder holding no repository at opensoft/openRepoShape where the command runs (an unpacked archive, say) refuses the scaffolded pin with --upstream passed. On main only the run with no --upstream refused it. Measured by rv1269 and by this round's rows: the refusal is update-upstream-not-a-repo in the pin's name, in both routes, and its remedy now ends by saying to run it from a directory where nothing is at that path. That run answers as the scaffolded pin does (--upstream route: exit 1).
  • The doctor's shape currency row, over a hand-edited source and merged-away copies, still names update-shape.py check as its NEXT, and that check refuses in the pin's name (exit 2). The row answers as on main by design (the first bullet). The pins row, which states what check reports, asks the field since the review round.
  • --upstream ~<unknown user>/x on the command line is still a RuntimeError traceback out of open_upstream (measured in process), as on main, and an argv value whose first part is over 255 bytes is update-unreadable naming that path. Both name the value a person passed, not the pin. A follow-up candidate, not this PR.
  • REPOSITORY_RE takes . and .. as an owner, so ../shape is owner/repo-shaped. Unchanged.

Closes #1203

Lane: openreposhape-5 (openRepoShape-5)

🤖 Generated with Claude Code

Summary by Sourcery

Validate the pin’s source repository consistently and keep shape-doctor reporting compatible with the new refusal behavior.

New Features:

  • Validate the shape pin’s source_repository: consistently before opening the upstream, regardless of whether --upstream is provided.
  • Refuse path-based or invalid repository values in the pin and prevent apply from writing changes for them.

Bug Fixes:

  • Ensure invalid pin sources are reported against the pin field rather than being misreported as --upstream errors.
  • Preserve shape-doctor’s existing currency behavior while allowing its pins gate to surface update-shape refusals.

Enhancements:

  • Centralize upstream repository detection and pin-source validation, including safe handling of missing users and overlong paths.

Tests:

  • Add comprehensive coverage for pin-source validation across upstream invocation modes, path handling, refusal ordering, apply safety, and doctor behavior.
  • Update existing tests for the revised refusal messages and upstream handling.

Brett Heap and others added 2 commits November 8, 2026 16:36
Brett Heap @claude
update-shape.py: prepare asks the shape pin's source_repository: …
e1692c1 
…once, before the upstream opens and with `--upstream` passed too, refusing a path or a value that is no `owner/repo` in the pin's name; shape-doctor.py answers as it did (#1203)

`prepare` read the pin's `source_repository:` and used it two ways. With
no `--upstream` it was the upstream `open_upstream` opened, and a value
naming something in the working directory that is no repository
(`contracts`, `scripts/repo_shape.py`) was `update-upstream-not-a-repo:
--upstream contracts exists but is not a git repository`, a flag nobody
passed. With `--upstream` passed it was never asked: a value ending in a
line feed went into `apply`'s commit subject and the `gh pr create` line
it prints, both broken by it, exit 0 (#1166's review, findings 3 and 9).

`pin_source` now asks it once, in `prepare`, before the upstream is
opened, whichever way the run reaches the upstream, under the codes these
values had: something there that holds no repository is
`update-upstream-not-a-repo`, a value `REPOSITORY_RE` does not take whole
is `update-upstream-unresolvable`, and `checked_value`'s is
`unsafe-value`, each naming the pin and its field. A path is refused with
`--upstream` passed too, and a clone kept at a path that is not spelled
`owner/repo` is refused with no `--upstream` (main opened it in place):
the pin records the name `apply` commits and opens its pull request with.
The remedy no longer offers `--upstream` a path, which asks the field too.
An `owner/repo` spelling that is a repository where the command runs is
taken, as it was. `open_upstream` asks `--upstream` alone again.

`shape-doctor.py` passes `prepare(..., ask_source=False)` at its two
calls: it reads a refusal out of `prepare` as its own checkout unable to
answer, so asking there turned a hand-edited field into CANNOT ANSWER,
exit 3, with a `git fetch --all` NEXT (measured). Its rows are main's.

Closes #1203

Lane: openreposhape-5 (openRepoShape-5)

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Brett Heap @claude
tests/test_doctor_unreadable_pin.py: the stand-in for prepare takes…
86c1e6e 
… the `ask_source` keyword the doctor now passes, so the rows that raise an `OSError` out of the comparison raise it again (#1203)

`shape-doctor.py` calls `prepare(args, ask_source=False)` since the
commit before this one, and the stand-in took one argument, so its three
rows ended in a `TypeError` instead of the error they raise.

Lane: openreposhape-5 (openRepoShape-5)

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@brettheap brettheap added this to the v1.2.1 milestone Nov 8, 2026
@sourcery-ai

sourcery-ai Bot commented Nov 8, 2026 •
edited
Loading

Copy link
Copy Markdown

Reviewer's Guide

API

Update prepare to validate the pin’s source before opening the upstream in either route, and keep open_upstream focused on the explicit upstream argument.

  • update-shape.py L646-659, L2502-2514, L2528-2534 (+6 small edits)

Implementation

Add the shared repository check and pin-source validation, including safe handling of unresolved home directories and overlong paths, with pin-specific refusals and remedies.

  • update-shape.py L727-820

Have the doctor’s currency check skip pin-source validation while its ahead-refusal check asks it, so each row retains the intended answer and gate.

  • shape-doctor.py L3344-3346, L3357-3363, L6323-6330

Tests

Add coverage for pin-source validation across both upstream routes, including malformed values, paths, refusal ordering, doctor behavior, and protection against writes.

  • tests/test_update_shape_pin_source.py L1-425

Cover the doctor’s pins-row behavior when its named check refuses the pin source.

  • tests/test_update_shape_merged_away.py L525-568

Update test doubles to accept the doctor’s new ask_source keyword and preserve the unreadable-error assertions.

  • tests/test_doctor_unreadable_pin.py L504

Update the upstream-opening test wrapper for the simplified open_upstream(spec) interface.

  • tests/test_update_shape_unreadable.py L228-229

Align value-pattern tests and their remediation expectations with pin validation in both routes and the revised refusal wording.

  • tests/test_value_pattern_line_break.py L390-398, L436-442 (+3 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

@brettheap brettheap mentioned this pull request Nov 8, 2026
Merged
@brettheap

brettheap commented Nov 8, 2026

Copy link
Copy Markdown
Contributor Author

(lane openRepoShape-5, 2026-11-08T18:42Z) Independent review of #1269 (#1203) by rv1269, lane 5's opus reviewer, which wrote none of it: NOT READY at 86c1e6ea, on two SHOULDs with measured fixes. A fix round follows. The report follows as the reviewer wrote it.

Independent review of #1269 (#1203) for lane openRepoShape-5, by rv1269 (opus), at 86c1e6e: NOT READY

The fix itself holds: the rows, mutants, gates, merge-trees and the writer's three choices all check out. Two SHOULD findings block it. Each is a small defect the PR introduces, with a fix of one to three lines that I measured. There are no MUST findings.

The three choices

  1. ask_source=False in the doctor's check_shape_currency: right, and the narrowest fix.
    • With the field asked there (mutant M9, my probe), every hand-edited value turned the row into CANNOT ANSWER, exit 3. Its NEXT was git -C <standard> fetch --all and its reason "this checkout of the standard does not carry the commit the pin names". That held for a current root too, so the body's claim is confirmed.
    • With the opt-out, the doctor gives main's answer in all 6 probe cases (3 values x a current or a behind root): same exit, verdict, row status and reason.
    • Where the root is behind, the row's NEXT (update-shape.py check) now refuses in the pin's name. On main it exited 1.
    • The doctor loads update-shape.py from its own directory, so the new keyword cannot meet an older copy.
    • The doctor's second call is a different matter: see S2.
  2. A path in the pin with no --upstream is now refused: within update-shape.py: a shape pin's source_repository: that names a path is refused as --upstream, and with --upstream passed one that ends in a line feed is never asked and reaches the commit subject and the pull request title (#1166 follow-up) #1203.
    • The Suggested fix's first bullet asks REPOSITORY_RE.fullmatch in both routes, which refuses ~/shape and /srv/shape.
    • Nothing documents a path in that field:
      • README and the handbook say --upstream "defaults to the repository the pin names".
      • docs/cli.md says "default: the pin's source_repository".
      • AGENTS-shape.md and bootstrap's notice document update-shape.py check --root .; README gives --upstream <path> as the offline route.
      • Both templates write {{SHAPE_REPOSITORY}}, which is the constant opensoft/openRepoShape.
      • No test writes a path there.
    • The body discloses the change.
  3. validate-manifest.py, validate-family.py, family.py and update-shape.py: an id:, a date or a repository: ending in a line feed is refused by name, and a pin's source_repository: that is no path or owner/repo name is named as the pin's (#1139) #1166's words: no stale copy anywhere.
    • I grepped tests/, README, docs/, the handbook and AGENTS.md for "or pass --upstream a path to a clone of openRepoShape" and for the pin-named "neither a path on this machine".
    • The remaining "neither a path on this machine" strings belong to the argv --upstream refusal, where a path is still valid.

SHOULD

S1. A pin value ~<unknown user>/... is a Python traceback; with --upstream passed this is new.

  • pin_source calls Path(declared).expanduser(), which raises RuntimeError: Could not determine home directory. (Python 3.12; 3.13 and 3.14 raise the same).
  • Measured with source_repository: "~rv1269nosuchuser/shape" and check --upstream <clone>:
    • at the head: traceback, exit 1;
    • on main: exit 1 with a report.
  • With no --upstream both versions crash; that was already true on main.
  • The PR's Goal is "refuses a value that is no owner/repo in the pin's name". Exit 1 is also check's "behind" code, so a script can misread the crash.
  • Realistic case: a pin hand-edited to ~alice/... and committed crashes on every machine without a user alice.
  • Fix, shown in-process on a copy in work/rv1269/fixcheck/ (no tree touched): wrap the expanduser in try/except RuntimeError and keep the bare path. The value is then update-upstream-unresolvable in the pin's name, and the scaffolded value is unchanged. Add a row in each route.

S2. The doctor's second opt-out, in refusals_ahead, makes a pins row sentence false (the #676 class).

  • Setup (my probe 2): two collision copies merged away plus source_repository: "has space/x".
  • At the head:
    • the pins row says "update-shape.py check reports them merged-away, and the apply it prints drops their pin rows";
    • that check, run as the row's NEXT, exits 2 with update-upstream-unresolvable.
  • On main the same sentence is true: check exits 1 and lists both copies as merged-away.
  • Under R1 (refusals_ahead calls prepare(args) and asks the field):
    • prepare refuses, so refusals_ahead returns None, as its docstring says it should when prepare refuses;
    • the pins row keeps the validator's plain finding and says nothing false about check;
    • the scaffolded case is identical at the head and under R1.
  • R1 passes the 40 rows, plus test_update_shape_merged_away, test_update_shape_hardlink, test_update_shape_rewrite_symlink and test_doctor_partial_clone: 141 passed.
  • Fix: drop ask_source=False at shape-doctor.py:3358 only, and add a row for this setup.

NIT

Measured

  • Rows (Python 3.12.3, git 2.43.0):

  • Mutants: each alone on the writer's 40-test set, each restored with a sha256 check (17 of 17 restored=yes).

    • The writer's M1-M10 and M3b match the body's table in count and killing rows: 11, 12, survives, 7, 8, 3, 2, 2, 12, 1, 11.

    • M3 is equivalent, by construction and checked: match and fullmatch of REPOSITORY_RE disagree on 0 of 66,430 strings (every string up to length 5 over a, /, LF, CR, space, -, ., ~).

    • My own:

      Mutant Result
      R1 refusals_ahead asks survives (see S2)
      R2 or not args.upstream dropped survives; unreachable, since no caller passes ask_source=False without upstream=
      R3 name question before path question killed, 4
      R4 bare HEAD question dropped killed, 1 (test_open_upstream_raises_a_refused_stat[a-bare-repositorys-head])
      R5 checked_value first killed, 10
      R6 pin named relative killed, 22

  • Probes: real check and apply --yes runs, gh stub first on PATH, an unreachable proxy.

    • The issue's 6 values in both routes at the head: exit 2 in the pin's name. apply writes nothing: no branch, HEAD unchanged, the pin's commit: unchanged, clean tree.
    • On main with --upstream: check exit 1, and apply exit 0 committing "Re-sync the shape copies to contracts @ ..." and the like. The line-feed value broke the subject, and the gh pr create line split inside its --title quotes.
    • The scaffolded pin with --upstream: the same answer at the head and on main.

  • Gates at the head (Python 3.12.3, git-filter-repo 2.47.0, gh stub, bogus token, GIT_CONFIG_GLOBAL=/dev/null):

    Gate git 2.43.0 (/usr/bin/git) git 2.47.1 (L/tools/git247/bin/git)
    A (27 files) 1339 passed, 5 skipped, one run 1339 passed, 5 skipped, 15 parts of at most 90
    B (8 doctor files) 824 passed, 3 skipped, one run 824 passed, 3 skipped, 10 parts
    • Gate A's skips: 3 test_update_shape_add_symlink.py:832 "junctions are NTFS's", and 2 test_no_auto_maintenance.py:222 "the session began with GIT_CONFIG_GLOBAL set".
    • Gate B's skips: 3 test_shape_doctor.py:10516, the PowerShell line (Windows only).
    • These match the body's counts.

  • CI at 18:41Z: SonarCloud, package and tests-git-bash pass; tests is still pending.

Merge-trees

git merge-tree --write-tree <other> 86c1e6ea2, git 2.43.0, heads read fresh. All rc=0 with 0 conflicts.

Other Head Tree
main (18:40Z) 51a9c5f10 be211791
main (17:57Z) 9729315f1 d88ff40d
#1231 aca00dfd 7abe1a3b
#1241 ab137eb5 9c02c2ba
#1248 2dd144f5 b1655305
#1172 (18:41Z) ba81fd7d2 6c0616c5
#1172 (earlier) 2eabec8e d0401e70
#1159 (merged) 9e4aab8c 7d77b481
#1147 (merged since) 4d299dd3 3d1efda2
  • The six PR trees at the heads the body names equal the body's trees.
  • No added or removed line in main's diff since be21751c3 (including +270/-20 in shape-doctor.py) or in the open PRs' diffs mentions prepare(, open_upstream, pin_path, source_repository, WRITE_THE_PIN or REPOSITORY_RE.
  • Not done: the rows on the merged tree with current main. That is left to the landing gate.

Files

Everything is in /workspace/projects/.lane-worktrees/openRepoShape-5/work/rv1269/:

  • STATE.md (with the final CHECKPOINT)
  • logs/, mutants.summary, mutate.py, merge-trees.log
  • probe-*.jsonl and probe/ (the two probe files)
  • chunks-gate*-2471.summary
  • fixcheck/ (the S1 fix check)

Both of my worktrees are clean and left in place (no rm):

  • /workspace/projects/openRepoShape-worktrees/rv1269-review
  • /workspace/projects/openRepoShape-worktrees/rv1269-mut

I made no commits, pushes or GitHub writes, and touched neither of the writer's trees.

Lane: openreposhape-5 (openRepoShape-5)

Brett Heap and others added 2 commits November 8, 2026 18:50
Brett Heap @claude
update-shape.py and shape-doctor.py: a pin's source_repository: of …
175d9b7 
…`~<unknown user>/...` or over 255 bytes is nothing there and is refused in the pin's name, the doctor's `pins` row asks the field, a folder-named value is told where to run (#1203)

Review of PR #1269 (rv1269), two SHOULDs and the NITs it asked for:

- S1: `Path(declared).expanduser()` raised `RuntimeError` for `~<user>/...`
  with no such user, a traceback with exit 1, the code `check` uses for
  "behind". `_named_here` keeps the bare path, so the name question
  refuses it `update-upstream-unresolvable` in the pin's name, with or
  without `--upstream`.
- N1: a first part over 255 bytes made `stat` raise `ENAMETOOLONG`, which
  was `update-unreadable` naming neither the pin nor the field. That errno
  is nothing there; every other refused `stat` is raised as it was.
- S2: `refusals_ahead` no longer passes `ask_source=False`. Where `check`
  refuses the pin's field, the `pins` row said `check` reports the copies
  `merged-away` and the `apply` it prints drops their rows; it exits 2 and
  does neither. `check_shape_currency` keeps the opt-out.
- N2: the `update-upstream-not-a-repo` remedy of a pin's value says to run
  from a directory where nothing is at that path.
- N3: `REPOSITORY_RE`'s comment no longer says it is for the pin "where no
  `--upstream` is passed".
- N4: the doctor guard row holds the `shape currency` row to main's `ok`.

Lane: openreposhape-5 (openRepoShape-5)

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Brett Heap @claude
tests/test_update_shape_pin_source.py: a pin's ~<user> that `expand…
c792615 
…user` cannot expand is looked up as the bare path, so a folder of that name where the command runs is `update-upstream-not-a-repo` in both routes (#1203)

S1 of the review of PR #1269 asks the guard to "keep the bare path". The
row holds that half: a guard that returned "nothing there" instead gave
the same answer for every value but this one.

Lane: openreposhape-5 (openRepoShape-5)

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
@brettheap brettheap mentioned this pull request Nov 8, 2026
Draft
@sonarqubecloud

sonarqubecloud Bot commented Nov 8, 2026

Copy link
Copy Markdown

Quality Gate Passed Quality Gate passed

Issues
1 New issue
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

@brettheap brettheap mentioned this pull request Nov 8, 2026
Draft
@brettheap

brettheap commented Nov 8, 2026

Copy link
Copy Markdown
Contributor Author

(lane openRepoShape-5, 2026-11-08T20:40Z) Delta check of #1269's fix round, 86c1e6ea..c792615f, by rv1269b, lane 5's sonnet reviewer, which wrote none of it: READY, with three NITs. A short round for NITs A and B (two one-line fixes, each with a row) and C follows. The report follows as the reviewer wrote it.

Delta check of #1269 (#1203) for lane openRepoShape-5, by rv1269b (sonnet), 86c1e6e..c792615: READY

All findings are NITs. S1, S2 and N1 to N4 hold, with a measured one-line fix for each of the two new NITs. No MUST or SHOULD. I made no commits, pushes or GitHub writes, and no subagents.

Findings

1. The delta is what the body says. git diff 86c1e6ea2 c792615f4 is four files:

  • update-shape.py and shape-doctor.py;
  • tests/test_update_shape_pin_source.py (+171) and tests/test_update_shape_merged_away.py (+44).

The two stand-in files (test_update_shape_unreadable.py, test_doctor_unreadable_pin.py) and test_value_pattern_line_break.py are byte-identical between 86c1e6e and the head. The test file has 34 rows: 22 from the first round and 12 new, which is S1 4, N1 5, N2 3. Both commits carry the Lane and Co-Authored-By lines, and their subjects are 249 and 222 characters.

2. S1 holds. I ran real check and apply --yes with a stub gh and an unreachable proxy.

  • source_repository: "~rv1269bnosuchuser/shape" gives REFUSED update-upstream-unresolvable, exit 2, in the pin's name, in both routes, with no traceback.
  • apply writes nothing: HEAD unmoved, no branch, the pin's commit: unchanged, clean tree.
  • At 86c1e6e both routes are tracebacks, exit 1. On main, no --upstream is a traceback; with --upstream, check exits 1 and apply exits 0 and commits.
  • The scaffolded value with --upstream is identical at the head, at 86c1e6e and on main: check exit 1, apply commits.

3. N1 holds.

  • "r"*300 + "/x" is unsafe-value in both routes at the head (the writer's finding is right). "r"*300 + "/has space" and a 5000-character path are update-upstream-unresolvable.
  • The control is a real run with a mode-000 folder locked and the value locked/x. It gives REFUSED update-unreadable: locked/x: cannot be read (Permission denied), exit 2, at the head and at 86c1e6e. Only ENAMETOOLONG is taken as nothing there.
  • Mutants Y1 (wrong errno), Y2 (EACCES also nothing) and N1-any-oserror are each killed.

4. S2 holds. I ran rv1269's probe-2 setup: two collision copies merged away, plus a hand-edited value.

  • For has space/x, a line feed, ~nosuchuser/shape and "r"*300+"/x", the doctor's pins row falls back to the gate's finding and no longer claims check reports merged-away. check exits 2 in the pin's name.
  • At 86c1e6e the same rows still carried the false sentence.
  • The scaffolded case with the two copies is unchanged on all three versions: the pins row names merged-away and check exits 1.
  • The shape currency row is identical to main for all six values, current and behind, with no traceback: same exit, verdict and status, and no environment key.

5. N2 holds.

  • The added sentence ends "run this from a directory where nothing is at that path." It appears only in the update-upstream-not-a-repo remedy.
  • The scaffolded pin run from a folder holding a non-repository opensoft/openRepoShape shows it at the head in both routes. The line-feed and contracts refusals read the name remedy.
  • No README, docs/cli.md, handbook or AGENTS.md text names these codes or this remedy. The old remedy words live only in update-shape.py, test_update_shape_pin_source.py and test_value_pattern_line_break.py, and test_value_pattern_line_break.py is unchanged.
  • The Limits claim that --upstream ~<unknown user>/x is still a RuntimeError traceback, and that a long argv first part is still update-unreadable, both measured true.

6. N3 and N4. I checked N3 by reading. N4's assert is what kills the doctor-asks mutant D1 (below). On main, shape currency is ok for a current root, so the asserted status is main's.

7. Rows and mutants (Python 3.12.3, git 2.43.0). Every mutant ran alone on the writer's 53-test selection, restored with a sha256 check of the four files (restored=yes for all 31).

  • At 86c1e6e's production: 19 failed, 34 passed. The 19 are the S2 row, 4 S1, 4 N1, 2 N2, and 8 remedy-sentence rows, which is the body's list.

  • On main's (be21751c3) production: 33 failed, 33 passed. This is the body's 21 plus 12 (S1 4, N1 4, N2 2, S2 1, the PermissionError row).

  • Writer's undos, all killed:

    Undo Killing rows
    S1 guard removed 4
    S1 bare path not kept 2
    N1 errno removed 4
    N1 any OSError 1
    S2 opt-out back 1
    N2 sentence removed 12
    N2 sentence everywhere 15
    D1, doctor asks and calls a refusal a plain finding 1

  • D1 with N4's assert undone survives, so that assert alone kills D1. N4's assert undone, with the production code as committed, passes (53), which makes it a control.

  • First-round M1 to M10 give 17, 18, survives, 7, 12, 3, 4, 2, 18, 2, 17 killing rows. These are M1, M2, M3 (equivalent), M3b, M4, M5, M6, M7, M8, M9, M10, in order. M3 (match for fullmatch) is equivalent: rv1269 checked 66,430 strings.

  • My own mutants, with their results:

    Mutant Result
    Y1 wrong errno killed, 4
    Y2 EACCES also nothing killed, 1
    Y4 remedy words changed killed, 12
    Y7 nothing-there kept as a path killed, 23
    Y3 except (RuntimeError, ValueError) survives (see NIT A)
    Y5 or not args.upstream dropped survives; unreachable, since the only ask_source=False caller passes upstream=; this is rv1269's R2
    Y6 .resolve() dropped survives; equivalent

  • rv1269's R1 is the code now, and its inverse S2-optout-back is killed.

NIT A. A raw NUL byte after ~ in the pin value is still a traceback, exit 1.

  • Setup: source_repository: ~a<NUL>b/x, plain or quoted, in either route. Path.expanduser() raises ValueError: embedded null byte, which S1's except RuntimeError does not catch.
  • On main with --upstream, check exited 1 with a report, so this is new in that route; with no --upstream main crashed too.
  • It is contrived, since the file would be binary.
  • Fix: except (RuntimeError, ValueError). Mutant Y3 turns all four tracebacks into update-upstream-unresolvable, exit 2. No row covers it, which is why Y3 survives.

NIT B. The doctor's pins row can read "could not be run" in one narrow case.

  • Setup: merged-away copies, a pin value like locked/x, and an unsearchable locked in the doctor's cwd.
  • S2's new asking in refusals_ahead lets the OSError from pin_source's stat escape, because only Refusal is caught. The row reads could not be run: [Errno 13] Permission denied: 'locked/x' instead of the gate's finding.
  • On main and at 86c1e6e it said the merged-away sentence, and check, the row's NEXT, now exits 2 update-unreadable.
  • Verdict, exit and the shape currency row are unchanged.
  • Fix: except (Refusal, OSError): return None. Mutant Z1 restores the gate finding.

NIT C. The Merges section is stale again. The body names main 9252ef208; main is now 77c56dde6. #1172 moved to bbe5b414e, and the trees below are re-read fresh. That is for the landing refresh.

Not changed: the live body is the writer's text plus Sourcery's regenerated summary after the footer. Body-guard exits 0 and closingIssuesReferences is #1203 only.

Gates

Head c792615f4, dirty=0, Python 3.12.3, git-filter-repo 2.47.0 (L/tools/venv247), stub gh, bogus token, GIT_CONFIG_GLOBAL=/dev/null. I used n1203's lists (27 files for A, 8 for B). One run of more than 100 tests at a time.

Gate git 2.43.0 (/usr/bin/git) git 2.47.1 (L/tools/git247/bin/git)
A 1352 passed, 5 skipped, one run, 799.7s 1352 passed, 5 skipped, 16 parts of at most 90 ids
B 824 passed, 3 skipped, one run, 1254.5s 824 passed, 3 skipped, 10 parts of at most 90 ids

Skips, the same on both gits and the same as the body:

  • A: 3 at test_update_shape_add_symlink.py:832 ("junctions are NTFS's") and 2 at test_no_auto_maintenance.py:222 (the session began with GIT_CONFIG_GLOBAL set).
  • B: 3 at test_shape_doctor.py:10516 (the PowerShell line, Windows only).

The live PR's tests check was still IN_PROGRESS at the last read. tests-git-bash, package and SonarCloud passed.

Merge-trees

git merge-tree --write-tree <other> c792615f4, git 2.43.0, heads read fresh at 20:39Z. All exit 0, no conflict lines.

Other Head Tree
main 77c56dde6 b4f709c6
#1231 aca00dfd9 487cf667
#1241 ab137eb5f 1fbe57e7
#1248 1b747f7ab 36531a9b
#1172 bbe5b414e 435cfd98

Disclosures

  • One bash line of mine ended in a fallback || rm -f /dev/null. It ran once and was denied (Permission denied); nothing was removed.
  • I once ran my probe script with mutant Y3 in the review tree by mistake. It restored production to the head's bytes, which I checked by sha256 and git status.
  • At the end both worktrees were clean at c792615f4, with the four files at their commit sha256.

Files

Everything is in /workspace/projects/.lane-worktrees/openRepoShape-5/work/rv1269b/:

  • STATE.md
  • mutants.summary, mutants-rf.summary, mutate2.py, mut2.sh
  • probe-*.jsonl and probe/
  • gates.summary, gateA-2471.summary, gateB-2471.summary, logs/
  • merge-trees.log, merge-trees-final.log

Worktrees /workspace/projects/openRepoShape-worktrees/rv1269-review and rv1269-mut are clean at c792615f4.

Lane: openreposhape-5 (openRepoShape-5)

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

v1.2.1

1 participant

@brettheap