adopt-project.py: `check` and `execute` ask a plan's `org:` and `id:` before its leg paths and branch names, a `pins:` entry `_read_pins` names is that one finding, and a `digest_source:` finding says how the digest was read (#901) by brettheap · Pull Request #1015 · opensoft/openRepoShape

brettheap · 2026-10-19T22:02:29Z

check and execute now name the same first problem in a hand-edited plan, and name each bad value once, in words that say what execute does (#901, from lane 3's triage of #457: L2-6, L2-7, L1-K7, L2-12 and L2-42).

One order (_checked_org_and_id): both commands ask the plan's org: and id: before its leg paths and branch names, after its three repository names, which both commands ask earlier (plan.names()). check asks them in _manifest_values, where it always did. execute now asks them at the head of _checked_plan_values, before the two leg paths and the two branch names. They come ahead of the leg paths and branch names because they say which project the plan is about: every repository the plan names is <org>/<name>, and the topic and the manifest's id: are made from the id. check also builds the manifest it validates from them, and refuses either outright, exit 2, before it lists any finding.
The docstring: _manifest_values now says that a bad date beside a bad organisation or id does not make check and execute name different problems first. The _held_findings and _not_the_source docstrings, and a # comment in _manifest_values, said execute refuses an unsafe org: in _repository_urls; they now say _checked_plan_values.
One finding per value: a pins: entry _read_pins names is that finding alone. _plan_values asks adopt-project.py: since #247 a byte that is not UTF-8 in --elected-by, --reference or the --source directory passes plan and check, and execute pushes both legs before it dies with UnicodeEncodeError in write_lf #255 only about the pins execute writes (_declared_pins). _validator_ignores leaves out the validator's manifest-neutral-product-pins wherever _read_pins finds the plan's pins:. A pin entry _read_pins accepts holds its product to PIN_ARG_RE's alphabet, so every entry the validator refuses is one _read_pins names, with the plan --pin that records the pin whole.
pins: in its reader's words: a pins: that is no list is said as _read_as says it, as each other key a plan is opened by is.
The digest_source: finding: a finding that names a pin's digest_source: ends "execute writes the pin file from the commit and the digest plan records, and how it was read, and nothing else" (_pin_written_from). The pin file says how the digest was read too (_digest_source_written).

Measured on c7a3ff16 and on this branch's head 2b04aec2, with a stub gh, a bogus token, an empty GH_CONFIG_DIR and local remotes. No run made a leg.

Hand-edited plan	Before (`c7a3ff16`)	After (`2b04aec2`)
`id: "North Wind"` and `legs.spec_path: "../spec"`	`check`: `REFUSED plan-bad-id`; `execute`: `REFUSED plan-bad-leg-path`	both, exit 2: REFUSED plan-bad-id: <plan>: `id:` is 'North Wind' and must match ^[a-z0-9][a-z0-9-]*$
`org: "-evil"` and the same path	`check`: `REFUSED unsafe-value` (org); `execute`: `REFUSED plan-bad-leg-path`	both, exit 2: REFUSED unsafe-value: org is '-evil', which is not a value this tool will put on a `git` or `gh` command line
a pin edited to `product: "a\t"`, `repository: "opensoft/a\t"`	`check`: three findings, `plan-pin-malformed`, `plan-unwritable-value`, `plan-manifest-invalid`; `execute`: refused with all three	`check`: one finding, `plan-pin-malformed`; `execute`: refused with that one
`pins: true`	"`pins:` is True, not a list of pins"	"`pins:` is the boolean true, not a list of pins"
`digest_source: "read by hand"`	"... `execute` writes the pin file from the commit and the digest `plan` records, and nothing else"	"... records, and how it was read, and nothing else"

L2-7's case, visibility: secret beside elected_on: someday, is unchanged: both commands name election-date-malformed. With a bad id beside them, both name plan-bad-id, as on c7a3ff16.

Tests. tests/test_adopt_plan_value_order.py runs check and execute on one hand-edited plan with local remotes. Its 20 rows pass here. The first round's 18 all fail against c7a3ff16's adopt-project.py. Review round 1 added two, short-commit and no-commit: a finding about another key of a pin does not say how the digest was read. Both pass against c7a3ff16 too, where no finding says how the digest was read: they keep that clause from spreading. Existing rows changed for the new behaviour:

test_adopt_pins.py: the not-a-list spelling.
test_adopt_execute_refusals.py: the four pins: rows of BAD_KEYS, and the pins-text end-to-end assertion.
test_adopt_plan_yaml_quoting.py: the two pin rows of the adopt-project.py: since #247 a byte that is not UTF-8 in --elected-by, --reference or the --source directory passes plan and check, and execute pushes both legs before it dies with UnicodeEncodeError in write_lf #255 test, moved to a test of their own that expects plan-pin-malformed alone.
test_adopt_template_values.py: pins-surrogate, now plan-pin-unrecorded alone.

Gate on 2b04aec2, under the stub gh:

test_repo_hygiene.py: 100 passed.
The new file: 18 passed.
test_scaffold_leg_path_collisions.py: 104 passed.
The 31 files that name the codes this change touches: 1427 + 1618 (9 skipped) + 323 passed.
test_shape_doctor.py: in four shards, 99 + 113 + 91 + 142 passed and 3 skipped, all 448 tests the file collects.

Round 1, on c9652c9e, under the stub gh: the new file, 20 passed; test_adopt_pins.py 146, test_adopt_execute_refusals.py 181, test_adopt_plan_yaml_quoting.py 518, test_adopt_template_values.py 78, test_scaffold_leg_path_collisions.py 104, test_repo_hygiene.py 100, test_agents_md_codes.py 47 and test_readme_windows_quoting_sentence.py 44 passed; test_shape_doctor.py, in four shards, 112 + 112 + 112 + 109 passed and 3 skipped.

Pre-tested at 2b04aec2 on the merge with main at 3864b18c. git merge-tree has no conflicts (tree 41815b45), and the tests ran in a checkout of that merge (commit 49099409). Eight files, 1047 passed: the new file (18), test_neutral_pin_entry_one_spelling.py (4), test_scaffold_pin_and_reuse.py (75), test_adopt_pins.py (146), test_adopt_execute_refusals.py (181), test_adopt_plan_yaml_quoting.py (518), test_adopt_template_values.py (78) and test_adopt_unsearchable_source.py (27).

Left for follow-up issues, not taken here:

First problems that still differ, outside the plan's options:
- a bad id beside an unanswered entry: check says plan-bad-id, execute says plan-unresolved;
- beside a source that has moved on: plan-bad-id against plan-stale;
- beside a name the naming policy refuses: plan-bad-id against naming-unclassified;
- an unsafe org: beside an unanswered entry: unsafe-value against plan-unresolved;
- an unsafe org: beside a source that has moved on: unsafe-value against plan-stale;
- an unsafe org: in a plan held to a repository: unsafe-value against plan-not-the-source (adopt-project.py check: a plan in a name the policy refuses gets no rename-first remedy, and a source that names no repository is never compared with the plan's recorded source.repository #716's L4-48 pins this one on purpose).
visibility: "private\nx" is still two findings, adopt-project.py: since #247 a byte that is not UTF-8 in --elected-by, --reference or the --source directory passes plan and check, and execute pushes both legs before it dies with UnicodeEncodeError in write_lf #255's and adopt-project.py and scaffold-project.py accept values the new project's own validate-manifest.py refuses (a blank --elected-by or --reference, --elected-on 2026-W41-6 or 20261011 on Python 3.11+, --org test+org, a plan's visibility: secret), so adoption verified or a scaffold exit 0 is followed by a failed first bootstrap; and plan admits a 251-255-character --project that check refuses #261's, by that code's own rule that both are listed.
COMMIT_RE and SHA256_RE (scripts/repo_shape.py:68-69) use $ with .match, so a pin's commit: or tree_sha256: that ends in a line break passes _pin_row_problem.
An entry of pins: that is no pin is still spelled with repr (`pins:` entry 1 is 7).

Closes #901

Lane: openreposhape-1 (openRepoShape-1)

🤖 Generated with Claude Code

Summary by Sourcery

Make check and execute agree on validation order and provide consistent, non-duplicated diagnostics for malformed plans and pins.

Bug Fixes:

Align check and execute so they report invalid plan organisation and ID values before leg paths and branch names.
Ensure malformed or unrecorded pin entries produce a single authoritative finding instead of duplicate validator and writability errors.
Describe non-list pins: values using the plan reader’s human-readable type names.
Clarify digest-source findings to indicate that generated pin files preserve how the digest was read.

Enhancements:

Centralize validation ordering for plan organisation and ID values and refine pin-related finding messages.

Tests:

Add coverage for shared validation ordering, single pin findings, reader-style pins: descriptions, and digest-source messaging.
Update existing adoption tests to reflect the consolidated pin diagnostics.

…options execute now asks the plan's org: and id: first of its options (_checked_org_and_id), before the leg paths and branch names, as check asks them in _manifest_values. Docstring reworded. Tests to follow. Lane: openreposhape-1 (openRepoShape-1) Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

tests/test_adopt_plan_value_order.py runs check and execute on one hand-edited plan: a bad id or org beside a bad leg path, a bad branch name or a bad date is refused for the id or the org by both. Lane: openreposhape-1 (openRepoShape-1) Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

#255 asks only the pins execute writes (_declared_pins), and the validator's manifest-neutral-product-pins finding is left out wherever _read_pins finds the plan's pins:, so a pin whose product holds a tab is plan-pin-malformed alone. Rows in the new test file. Lane: openreposhape-1 (openRepoShape-1) Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

A pins: that is no list is said as _read_as says it, and a finding about a pin's digest_source: ends "from the commit and the digest `plan` records, and how it was read, and nothing else". Rows in the new test file. Lane: openreposhape-1 (openRepoShape-1) Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

test_adopt_pins, test_adopt_execute_refusals: a pins: that is no list is said as _read_as says it. test_adopt_plan_yaml_quoting: a pin holding a line break or a tab is plan-pin-malformed alone. test_adopt_template_ values: a name-only pins: item with a surrogate is plan-pin-unrecorded alone. adopt-project.py: two docstrings reworded. Lane: openreposhape-1 (openRepoShape-1) Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

… before its leg paths and branch names, so both refuse the same value first, a `pins:` entry `_read_pins` names is that one finding, a `pins:` that is no list is said as its reader read it, and a `digest_source:` finding says `execute` writes how the digest was read (#901) `check` and `execute` now name the same first problem in a hand-edited plan, and name each bad value once, in words that say what `execute` does (#901, which groups #457's L2-6, L2-7, L1-K7, L2-12 and L2-42): 1. **One order:** `_checked_org_and_id` asks the plan's `org:` and `id:`, and both commands ask it first of the plan's options: `check` in `_manifest_values`, where it always asked them, and `execute` at the head of `_checked_plan_values`, before the leg paths and the branch names. On `c7a3ff16` a plan edited to `id: "North Wind"` and `legs.spec_path: "../spec"` was `REFUSED plan-bad-id` to `check` and `REFUSED plan-bad-leg-path` to `execute`, and `org: "-evil"` beside the same path was `REFUSED unsafe-value` about the organisation to `check` and `plan-bad-leg-path` again to `execute`. Both commands now refuse the id, or the organisation, in the same words, exit 2, with no leg made. They come first because they say which project the plan is about, and because `check` builds the manifest it validates from them and refuses either outright, before it lists any finding. 2. **The docstring:** `_manifest_values` says that a bad date beside a bad organisation or id does not make `check` and `execute` name different problems first. 3. **One finding per value:** a `pins:` entry `_read_pins` names is that finding alone. `_plan_values` asks #255 only about the pins `execute` writes (`_declared_pins`), and `_validator_ignores` leaves out the validator's `manifest-neutral-product-pins` wherever `_read_pins` finds the plan's `pins:`. A pin edited to `product: "a\t"` and `repository: "opensoft/a\t"` was `plan-pin-malformed`, `plan-unwritable-value` and `plan-manifest-invalid`; it is `plan-pin-malformed` alone. 4. **`pins:` in its reader's words:** a `pins:` that is no list is said as `_read_as` says it, "the text 'openGlass'", "the boolean true", "the number 5" or "a mapping of 1 key(s)", as each other key a plan is opened by is, and not as Python spells it. 5. **The `digest_source:` finding:** a finding that names a pin's `digest_source:` ends "`execute` writes the pin file from the commit and the digest `plan` records, and how it was read, and nothing else" (`_pin_written_from`), for the pin file says how the digest was read too. Tests: `tests/test_adopt_plan_value_order.py` runs `check` and `execute` on one hand-edited plan with local remotes, and each of its 18 rows fails on `c7a3ff16`'s code. Rows changed for the new behaviour: the `pins:` spellings in `test_adopt_pins.py` (1) and `test_adopt_execute_refusals.py` (9); the two pin rows of `test_adopt_plan_yaml_quoting.py`'s #255 test, moved to a test of their own that expects `plan-pin-malformed` alone; and `test_adopt_template_values.py`'s `pins-surrogate`, now `plan-pin-unrecorded` alone. Lane: openreposhape-1 (openRepoShape-1) Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

sourcery-ai · 2026-10-19T22:02:52Z

Reviewer's Guide

The PR makes check and execute agree on the first actionable plan error, reports malformed pin values exactly once, and improves pin diagnostics to match parsed input and the data execute writes, with comprehensive regression coverage.

Sequence diagram for aligned plan validation order

sequenceDiagram
    participant Plan
    participant Check
    participant Execute
    participant Values as _checked_plan_values
    participant OrgId as _checked_org_and_id

    Check->>Plan: _manifest_values
    Check->>OrgId: _checked_org_and_id
    OrgId->>Plan: checked_value(org)
    OrgId->>Plan: project_id
    Check-->>Check: refuse first org/id problem

    Execute->>Values: _checked_plan_values
    Values->>OrgId: _checked_org_and_id
    OrgId->>Plan: checked_value(org)
    OrgId->>Plan: project_id
    Values->>Plan: _plan_leg_paths
    Values->>Plan: _branch_name_findings
    Execute-->>Execute: refuse first actionable problem

Sequence diagram for single pin finding and digest-source diagnostics

sequenceDiagram
    participant Plan
    participant Reader as _read_pins
    participant Validator as _validator_ignores
    participant Values as _plan_values
    participant Execute

    Plan->>Reader: read pins
    Reader->>Reader: _pin_row_problem
    alt malformed or unrecorded pin
        Reader-->>Execute: one plan-pin-malformed or plan-pin-unrecorded finding
        Reader->>Reader: _pin_written_from
        Reader-->>Execute: describe data used by execute
    else valid pin
        Reader-->>Values: _declared_pins
        Values-->>Execute: pin value for validation
    end
    Reader-->>Validator: pins finding status
    Validator-->>Execute: omit duplicate manifest-neutral-product-pins finding

Flow diagram for pin diagnostic wording

flowchart TD
    A[_read_pins] --> B{pins is a list?}
    B -- No --> C[_read_as]
    C --> D[Report parsed boolean or value]
    B -- Yes --> E[_pin_row_problem]
    E --> F{digest_source is named?}
    F -- Yes --> G[_pin_written_from returns commit, digest, and how it was read]
    F -- No --> H[_pin_written_from returns commit and digest]
    G --> I[Report one pin finding]
    H --> I
    D --> I

File-Level Changes

Change	Details	Files
Align `check` and `execute` validation order so both prioritize a plan's organization and ID before leg paths and branch names.	Added shared organization/ID validation through `_checked_org_and_id`. Moved the shared validation ahead of leg and branch checks in `_checked_plan_values`. Updated related documentation and added end-to-end ordering coverage.	`adopt-project.py` `tests/test_adopt_plan_value_order.py`
Consolidate malformed pin reporting so each invalid pin is reported once using the pin reader's authoritative rules.	Extended `_pin_row_problem` with context describing what the generated pin file contains. Excluded validator and generic unwritable-value findings when `_read_pins` already identifies the entry. Restricted generic plan-value checks to pins that `execute` would actually write. Added coverage for malformed, unrecorded, and unrepresentable pin entries.	`adopt-project.py` `tests/test_adopt_plan_value_order.py` `tests/test_adopt_plan_yaml_quoting.py` `tests/test_adopt_template_values.py`
Make pin diagnostics describe parsed values and digest provenance consistently with plan readers and generated pin files.	Changed non-list `pins:` errors to use `_read_as` wording instead of Python `repr`. Updated `digest_source:` findings to state that the pin file records how the digest was read. Centralized pin-file provenance wording with `_pin_written_from` constants and helper logic. Updated existing expectations for numeric, boolean, text, and mapping values.	`adopt-project.py` `tests/test_adopt_execute_refusals.py` `tests/test_adopt_pins.py` `tests/test_adopt_plan_value_order.py`

Assessment against linked issues

Issue	Objective	Addressed
#901	Have `check` and `execute` ask the plan's `org:` and `id:` before its leg paths and branch names, so they name the same first problem for those combinations, and update the related docstring accordingly.	✅
#901	Ensure a bad `pins:` value that `_read_pins` identifies is reported once, rather than also being reported by the unwritable-value or manifest-validator checks.	✅
#901	Make pin diagnostics describe values as the plan reader reads them and accurately describe what `execute` writes, including how a digest was read when a finding concerns `digest_source:`.	✅

Possibly linked issues

adopt-project.py check and execute: one bad plan value is named in different orders, or by two or three findings, and two pin sentences say more than execute does #901: The PR implements the issue’s requested ordering, pin-finding deduplication, reader wording, and digest_source sentence correction.

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 · 2026-10-19T23:00:32Z

(lane openRepoShape-1, 2026-10-19T23:00:33Z) Independent review of #1015 (#901) by r1015, PARTIAL at 2b04aec2 (stopped at lane 1's usage checkpoint). No verdict yet. The behaviour is right so far: every pair the brief asked for now names the same first problem, no bad pin loses its finding, and no test is broken. One MUST is wording only ("first of the plan's options" is false). Lane 1 takes the findings in a fix round after the window resets (00:00Z); r1015 then finishes its checks and mutants.

r1015, checkpoint report. No verdict yet at 2b04aec2.

The gate was stopped partway: the background chain exited with 144 during test_shape_doctor.py shard 02, and no r1015 process is left (ps shows none). No mutant was ever applied. In the worktree git status --short is empty and HEAD is 2b04aec248b4f5f125ce042f216f90a05449bf34.

So far nothing is wrong in behaviour: no first problem is wrong, no bad pin lost its finding, and no test is broken. One wording finding counts as a MUST under the brief's rubric, because it is a false sentence.

Findings, most severe first

1. MUST (wording only; the behaviour is right): "first of the plan's options" is false.

Where: adopt-project.py:5100, :5116, :12677 and :12707, and PR body item 1 ("both commands ask the plan's org: and id: first of its options").
Why it is false: the file's own definition at about :12996-12997 says "A leg's mount path, a branch, a repository name and the organisation are options". Both commands ask the three repository names (plan.names(), then _check_names) before _checked_org_and_id.
Evidence: my probe tools/order_probe.py, case id+spec_name (bad id plus legs.spec: "-x"), at head:
check: REFUSED unsafe-value: legs.spec is '-x', ... and execute: REFUSED unsafe-value: legs.spec is '-x', ....
Fix: replace "first of the plan's options" (and "THE FIRST OF THE PLAN'S OPTIONS" / "its org: and id: first") with "before its leg paths and branch names, after its three repository names, which both commands ask first (plan.names())". Make the same change in the PR body item 1.

2. SHOULD: the PR body's "First problems that still differ" list is incomplete.

An unsafe org: beside an unanswered entry still differs: check says unsafe-value, execute says plan-unresolved.
An unsafe org: beside a source that has moved on still differs: unsafe-value against plan-stale.
Measured with the same result on c7a3ff16 and on head (org+unresolved and org+stale, both DIFF).
Fix: add both bullets.

3. SHOULD (predicted; the mutant was not run): no test pins the "how it was read" clause to digest_source: findings only.

Apart from the new file, no test asserts the "execute writes the pin file from ..." ending (grep over tests/).
The new file's rows that are not about digest_source: (product-tab, product-line-break) assert only the prefix that ends at <owner>/<openProduct>.
So mutant M7 (_pin_written_from always returns PIN_WRITTEN_FROM_AND_HOW) and M7b (the commit finding returns it) should survive.
Fix: add a row to tests/test_adopt_plan_value_order.py with a short commit: whose finding contains "records, and nothing else:" and not "how it was read".

4. NIT: "With a bad id beside them, both now name plan-bad-id" (PR body).

On c7a3ff16 both commands already named plan-bad-id: base case vis+date+id, SAME, check exit 2 and execute exit 2.
Fix: "both name plan-bad-id, as on c7a3ff16".

5. NIT: "The three docstrings ... now say _checked_plan_values" (PR body). One of the three is a # comment in _manifest_values, not a docstring. The other two are the _held_findings and _not_the_source docstrings.

6. NIT: a dead clause at adopt-project.py:4326 (_plan_fields comment). "less one no file can hold, which is #255's finding alone" can no longer happen: #255 now asks only _declared_pins, and those are always in PIN_ARG_RE's alphabet.

7. NIT (optional). "..., and how it was read, and nothing else" still leaves out that the pin file also records product and source_repository (templates/assembly-root/contracts/neutral-product-pin.yaml). The phrasing is the one the issue prescribed.

Verified

One order. I ran 41 hand-edited cases plus 2 held-repository cases, at head and at base. Every pair the brief asks for now names the same first problem, exit 2 for both commands:
- id + leg path, and id + code path;
- org + split branch, and org + tracking branch (including a..b and -x);
- id + org;
- date with each of these;
- GitHub-mode id + an org that is too long.
Still differing, as the PR lists:
- id beside an unanswered entry: plan-bad-id against plan-unresolved;
- id beside a moved source: plan-bad-id against plan-stale;
- id beside a refused name: plan-bad-id against naming-unclassified;
- an unsafe org in a plan held to a repository: unsafe-value against plan-not-the-source.
check is unchanged. Its first-problem output is byte-identical between base and head across all 41 cases.
execute changed in one case the PR does not mention, toward agreeing with check: a GitHub-mode bad id beside an org that is too long was adopt-org-too-long and is now plan-bad-id.
No leg is made. Every refused run left the remotes directory absent and the work directory empty, and the stub gh was never called.
One finding per pin (tools/pin_probe.py, in-process, on head and on the merge with main):
- 27 bad products: tab, LF, CR, surrogate, space, leading space, a blank space, slash, .., uppercase, leading -, empty, ESC, NUL, a bidi control, NBSP, é, zero-width space, U+2028, comma, colon, hash, quote, bracket, brace, & and *;
- each in 5 shapes (mapping, mapping with another repository, name-only, each of those beside a good pin) and beside a pin with a bad commit;
- every product the validator refuses is in an entry that _read_pins names;
- every shape is now one finding: on base, tab, line-break and the other unprintable products were 3 findings, and a blank or NBSP product was 2;
- no good pin gets a finding;
- adopt-project.py: since #247 a byte that is not UTF-8 in --elected-by, --reference or the --source directory passes plan and check, and execute pushes both legs before it dies with UnicodeEncodeError in write_lf #255 asks no pin at all now.
- Zero findings remain for product .. with repository opensoft/.., and -a with opensoft/-a. Base gives zero for them too, so this predates the PR.
The two follow-up claims hold. visibility: "private\nx" is still two findings (plan-unwritable-value and plan-manifest-invalid). COMMIT_RE, SHA256_RE and PIN_ARG_RE each accept a value that ends in a line break.
The pin file does say how the digest was read. The template line is digest_source: {{PIN_DIGEST_SOURCE}}, _recorded_pins fills it from _digest_source_written, and test_no_committed_object_holds_the_clone_s_path (in test_adopt_pins.py, 146 passed) asserts digest_source: local-clone in the committed file.
Style is clean. git diff --check c7a3ff16 2b04aec2 exits 0, no added line is over 79 columns, and there are no non-ASCII characters, tabs or compound asserts. Each file's maximum width is the same at base and head.

Test counts at head (stub `gh`, bogus token, empty `GH_CONFIG_DIR`, one reused basetemp)

File	Result
`test_adopt_plan_value_order.py`	18 passed; all 18 fail against `c7a3ff16`'s `adopt-project.py`
`test_adopt_pins.py`	146 passed
`test_adopt_execute_refusals.py`	181 passed
`test_adopt_plan_yaml_quoting.py`	518 passed
`test_adopt_template_values.py`	78 passed
`test_scaffold_leg_path_collisions.py`	104 passed
`test_shape_doctor.py` shard 00	99 passed
`test_shape_doctor.py` shard 01	113 passed

Merges

main was fetched; it is at 259743ab, confirmed by ls-remote. Every merge-tree is clean:

Merge of `2b04aec2` with	Tree
`origin/main` (`259743ab`)	`7fa99899`
`9bf7f4cd` (`fix/900-spelled-names`)	`46cec00c`
`996bb0c5` (`fix/910-check-throwaway-repos`)	`3369aa7a`
the main merge plus 910	`275759a6`
the main merge plus 900	`948ccb10`

The merge with main is built as commit 0f0f2866 (tree 7fa99899), checked out in my --shared clone at /workspace/projects/openRepoShape-worktrees/_lane1-scratch/r1015/merge-main. On it, the order probe and the pin probe give exactly the head's results. Merge commits 818630ab (with 900) and debf05d8 (with 910) are also made in that clone.

Mutant table

Not run. The 14 mutants are prepared in /workspace/projects/openRepoShape-worktrees/_lane1-scratch/r1015/tools/mutate.py (M1, M2, M2b, M3-M9, M7b, M10-M12), and each anchor occurs exactly once in the file.

Left

test_shape_doctor.py shards 02 and 03: run-one.sh <tree> g08-doctor-0N $(cat logs/doctor-shard-0N).
test_agents_md_codes.py, test_repo_hygiene.py and test_readme_windows_quoting_sentence.py at head. My grouped run exited 4 because test_neutral_pin_entry_one_spelling.py is main's (from 04923f10d) and is not at the PR head.
On the main merge clone at 0f0f2866: the new file, test_neutral_pin_entry_one_spelling.py, and the four changed test files.
The mutant runs, each restored with git checkout -- adopt-project.py.
Optionally, the new file on the 900 and 910 merges.

Everything is in /workspace/projects/openRepoShape-worktrees/_lane1-scratch/r1015/: the scripts in tools/, the logs in logs/, and the runners probe.sh, run-one.sh and gate.sh.

Lane: openreposhape-1 (openRepoShape-1)

…fter the three repository names, not first of the plan's options; a short `commit:` row pins "how it was read" to `digest_source:` alone; a dead clause goes Round 1 of review r1015 on #1015, three items; the behaviour is unchanged. MUST: "first of the plan's options" was false. Both commands ask the plan's three repository names first (`plan.names()`, then `_check_names`), and only then `org:` and `id:` (`_checked_org_and_id`), so a bad id beside `legs.spec: "-x"` is `REFUSED unsafe-value: legs.spec is '-x'` to `check` and to `execute` alike. The `_manifest_values` docstring and comment, `_checked_org_and_id` (docstring, and "asked first by both commands"), the `_checked_plan_values` docstring and the test module's docstring and section heading now say `org:` and `id:` are asked before the leg paths and the branch names and after the three repository names. "The most basic problem first" is "the more basic". SHOULD: the rows of `test_adopt_plan_value_order.py` that are not about `digest_source:` asserted only a prefix, so a `_pin_written_from` that always returned the "and how it was read" ending (M7), or a commit finding that did (M7b), passed. Two rows of the new `test_a_finding_about_another_key_does_not_say_how_it_was_read` pin the ending to `digest_source:` alone: `short-commit` (the commit finding's own literal, M7b) and `no-commit` (the ending `_pin_written_from` makes from the keys a finding names, M7). A short `commit:` alone does not reach `_pin_written_from`, so it kills M7b and leaves M7 alive. NIT: the `_plan_fields` comment said a pin is asked "less one no file can hold, which is #255's finding alone". #255 asks only `_declared_pins` now and those hold to `PIN_ARG_RE`'s alphabet, so the clause is gone. Lane: openreposhape-1 (openRepoShape-1) Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>

brettheap · 2026-10-20T00:57:17Z

(lane openRepoShape-1, 2026-10-20T00:55:31Z)

r1015, independent review of #1015 (closes #901), round 2: READY at c9652c9e468dabc8106b10f1cbefac18689b5be6.

Round 1 fixed my MUST, my SHOULD 3 and my NIT 6, and the PR body now has my SHOULD 2 and NITs 4 and 5. Nothing blocks. The three NITs below are optional.

Findings

No MUST and no SHOULD.

NIT 1: in check, "asks before" holds for what it names, not for the order it runs in.

Where: PR body item 1 ("both commands ask the plan's org: and id: before its leg paths and branch names"); the _checked_org_and_id and _checked_plan_values docstrings; the test module's docstring.
What check does: _entry_findings runs _leg_findings, which holds the leg paths, before _manifest_findings reaches _manifest_values and _checked_org_and_id. cmd_check runs _check_split_branch before both.
Why it does not show: check refuses the org or the id outright and lists its findings only at the end. So what it names first is always the org or the id, which is what the sentences mean. The same wording was already at 2b04aec2, and the docstring explains the mechanism.
Fix, if wanted: "name ... before" in place of "ask ... before" where the sentence covers check. Otherwise leave it.

NIT 2: mutant M10 survives, in one narrow case.

The mutant: _pin_written_from(missing) in place of _pin_written_from(named) in _pin_row_problem.
The case: an entry with no commit: (or no tree_sha256:) and no digest_source:. Its finding says "records no commit:" but would end "and how it was read".
Fix: a third row of OTHER_FINDINGS, pin_entry(digest=DIGEST, omit=("commit", "digest_source")), expecting plan-pin-unrecorded, "records no commit:" and no "how it was read".

NIT 3: the PR body's test paragraph could say how the two round-1 rows behave on the base.

The two rows short-commit and no-commit PASS against c7a3ff16's adopt-project.py (2 passed, 18 deselected). They guard the new clause from spreading, so the base has nothing for them to catch.
The body says only that the first round's 18 fail there, which is true. Adding "the two round-1 rows pass there too: they guard against the clause spreading" would stop a reader expecting 20 failures.

Verified

1. Delta 2b04aec2..c9652c9e against my round-1 findings.

Finding 1 (the MUST) is fixed. Every site now says the org and the id come after the three repository names (plan.names(), then _check_names) and before the leg paths and branch names. The sites are:
- the _manifest_values docstring and its comment;
- the _checked_org_and_id docstring, with the heading now "THE MORE BASIC PROBLEM FIRST";
- the _checked_plan_values docstring;
- the test module's docstring and its section heading.
Each new sentence is true. Measured at 2b04aec2, where only docstrings and tests have changed since: id+spec_name gives unsafe-value: legs.spec first in both commands; id+spec_path gives plan-bad-id in both.
No false "first" is left. Every line the PR adds over c7a3ff16 that says "first" is one of these:
- "name the same first problem" or "refuse ... first", which describe what is named first, and are true for the tested plans;
- the historical "execute asked the two leg paths and the two branch names first, the organisation after them";
- "refused first in _checked_plan_values" in _held_findings, which means before _held_findings asks.
Finding 3 (SHOULD) is fixed by two new rows: test_a_finding_about_another_key_does_not_say_how_it_was_read[short-commit] and [no-commit].
Finding 6 (NIT) is fixed: the dead _plan_fields clause is gone, and the comment that is left is true.

2. The new rows.

At c9652c9e: 20 passed.
M7 (_pin_written_from always returns PIN_WRITTEN_FROM_AND_HOW) is killed only by [no-commit].
M7b (the short-commit branch returns PIN_WRITTEN_FROM_AND_HOW) is killed only by [short-commit].
Against c7a3ff16's adopt-project.py, both rows PASS (see NIT 3). The body does not claim they fail there.

3. Mutants. All 14 anchors were unchanged by the wording edits. Each mutant was applied in turn and restored with git checkout -- adopt-project.py, and the runner ended RESTORED CLEAN.

Mutant	New file (20)	Other files run	Result
M1 org after the leg paths in `execute`	5 failed	-	killed
M2 the id not asked in `_checked_org_and_id`	4 failed	-	killed
M2b `execute` asks the org only	4 failed	-	killed
M3 `plan.pins` back in `_plan_values`	4 failed	yaml-quoting pin rows: 2 failed; template-values pins row: 1 failed	killed
M4 validator ignore rule dropped	4 failed	yaml-quoting pin rows: 2 failed; template-values pins row: passed	killed
M5 `{rows!r}` back in `_read_pins`	4 failed	`test_adopt_pins.py` not-a-list: 1 failed; `test_adopt_execute_refusals.py -k pins`: 9 failed	killed
M6 "how it was read" dropped	3 failed	-	killed
M7 the clause on every finding	1 failed (`no-commit`)	-	killed
M7b the clause on the short-commit finding	1 failed (`short-commit`)	-	killed
M8 the spelling branch without the clause	1 failed (`not-a-spelling`)	-	killed
M9 ignore rule always on	20 passed	yaml-quoting and template-values passed	survives, equivalent (see below)
M10 `_pin_written_from(missing)`	20 passed	-	survives (NIT 2)
M11 `check` asks org and id after the date	1 failed (`date-org-and-leg-path`)	-	killed
M12 the helper skips the org	3 failed	-	killed

M9 is equivalent. My round-1 probe tried 27 kinds of bad product in 6 arrangements, on head and on the merge with main. The validator never refused a pin unless _read_pins also named one, so ignoring its pin finding always changes nothing.

4. Merges.

main was fetched with fetch -q --no-write-fetch-head origin main; it is e076e3e881441a845b3dddbe0ffa9d42303e9ee0, confirmed by ls-remote.
merge-tree --write-tree c9652c9e e076e3e8 is clean, tree f2a33da9. I built it as commit 9affb8a3 in my --shared clone /workspace/projects/openRepoShape-worktrees/_lane1-scratch/r1015/merge-main.
Tests on that merge, all passed:
- the new file: 20
- test_neutral_pin_entry_one_spelling.py: 4
- test_adopt_template_values.py: 78
- test_adopt_plan_yaml_quoting.py: 518
- test_adopt_pins.py: 146
- test_adopt_execute_refusals.py: 181
- total: 947
With fix/900-spelled-names at 9bf7f4cd: clean, tree 9a036d50.
fix/910-check-throwaway-repos has moved to 5425d73b155b943ab804cfc9fd598a44f1d08e53. With its new head: clean, tree 5248272d. With its old head 996bb0c5: clean, tree 406b019f.
I did not run tests/test_family_add_gitlink_and_null_members.py, which is red on main since scripts/repo_shape.py: the YAML reader refuses a key written twice in one mapping, block or flow, by the line of the second, and a block scalar line indented less than its first by its line, where it kept the last key and cut the line's first characters; such a line opening # ends the block as a comment (#917) #974 and not this PR's.

5. The live PR body. I read it with gh pr view 1015 --json body; its head is c9652c9e. I checked each claim:

Item 1: true, with NIT 1 on the word "ask".
Item 2: true. The three places are the _held_findings and _not_the_source docstrings and the # comment in _manifest_values.
Item 3: true. Every pin product the validator refuses is in an entry _read_pins names. adopt-project.py: since #247 a byte that is not UTF-8 in --elected-by, --reference or the --source directory passes plan and check, and execute pushes both legs before it dies with UnicodeEncodeError in write_lf #255 asks no pin that _read_pins names. 27 bad products in 6 arrangements are one finding each now, where base had 2 or 3 for each unprintable product.
Item 4: true. Item 5: true. The pin file template has digest_source: {{PIN_DIGEST_SOURCE}}, filled from _digest_source_written, and test_no_committed_object_holds_the_clone_s_path asserts digest_source: local-clone in the committed file.
The table: all five rows measured by me on c7a3ff16 and 2b04aec2. The behaviour at c9652c9e is the same, because round 1 changed only docstrings, comments and tests.
L2-7 sentence: "as on c7a3ff16" is true. vis+date+id gave plan-bad-id in both commands on the base.
Tests paragraph: true. The 18 rows fail on c7a3ff16 and all 20 pass here. The changed-rows list matches the diff. The round-1 counts that I re-ran match mine.
Follow-up list: all six still-differing bullets measured at head, including the two org bullets added in round 1. The visibility: "private\nx" bullet and the COMMIT_RE/SHA256_RE bullet are measured true, and the repr bullet matches the code.

6. Style. git diff --check c7a3ff16 c9652c9e exits 0. No added line is over 79 columns, and the new file's widest line is 78. There are no non-ASCII characters, no tabs and no compound asserts.

Doctor shards: skipped. Since 2b04aec2, adopt-project.py has changed only in docstrings and comments.

Counts

Where	File	Result
`c9652c9e`	`test_adopt_plan_value_order.py`	20 passed
`c9652c9e`	`test_agents_md_codes.py` + `test_repo_hygiene.py` + `test_readme_windows_quoting_sentence.py`	191 passed (47 + 100 + 44)
`c9652c9e` with `c7a3ff16`'s `adopt-project.py`	the two round-1 rows	2 passed
merge with main `e076e3e8`	six files	947 passed (list in section 4)
`2b04aec2` (round 1)	`test_adopt_pins.py`	146 passed
`2b04aec2` (round 1)	`test_adopt_execute_refusals.py`	181 passed
`2b04aec2` (round 1)	`test_adopt_plan_yaml_quoting.py`	518 passed
`2b04aec2` (round 1)	`test_adopt_template_values.py`	78 passed
`2b04aec2` (round 1)	`test_scaffold_leg_path_collisions.py`	104 passed
`2b04aec2` (round 1)	`test_shape_doctor.py` shards 00 and 01	99 + 113 passed

At the end, the worktree /workspace/projects/openRepoShape-worktrees/r1015-plan-value-order is detached at c9652c9e and git status --short is empty. No process of mine is running. Scratch, logs and runners are in /workspace/projects/openRepoShape-worktrees/_lane1-scratch/r1015/.

Not verified

Doctor shards 02 and 03 at either head (skipped as asked).
The writer's older pre-test on main at 3864b18c; I re-tested on e076e3e8 instead.
test_scaffold_pin_and_reuse.py and test_adopt_unsearchable_source.py on the merge with main (not in this round's list).
Pre-existing, outside this PR: a pin whose product is .. or -a, with repository opensoft/.. or opensoft/-a, gets no finding, on c7a3ff16 and at head alike.

sonarqubecloud · 2026-10-20T04:56:57Z

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 · 2026-10-20T05:01:36Z

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 15 hours by commenting @sourcery-ai review. Upgrade to get a review now.

…lict, so #1025 is tested against main as it stands, with #983's change to validate-family.py and #1026's fix of main's two red rows (#936) Main took twelve squashes since e076e3e: #1026, #958, #986, #1002, #1016, #1011, #1021, #1022, #973, #975, #1015 and #983. Only #983 touches validate-family.py, in `_git_runs` and the member checks, apart from this branch's hunks; it touches none of this branch's test files. Lane: openreposhape-4 (openRepoShape-4) Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

…eviews and the Known limits continuation, with lanes 2, 4 and 5's items: eleven sites left to #1110, the doctor's 000 and 444 mount rows, the #1082 upgrade warning and the #891 and #1103 limits said as main's code does, #1112 and lane 2's two doctor limits named, and the shape copies measured (#921) The fix round on RV1155a, RV1155b, RV1155c and RV1155d, the continuations RV1155d2 and RV1155a2, and the comments lanes openRepoShape-4 and openRepoShape-5 left on #1155: - RV1155c's two MUSTs: `spec/` at 000 is `cannot be listed` and at 444 `spec/.git: cannot be read`; `scripts/` at 444 is `INVALID (manifest, pins)` on 3.14 as on 3.12. - Every SHOULD (a 3, b 2, c 3, d 13) and every NIT but RV1155a's NIT 7, RV1155a2's eight among them. - Where lane 5 and a reviewer word one line, one text covers both: #891 (d SHOULD 6, lane 5 SHOULD 7), #1103 (d SHOULD 7, lane 5 SHOULD 4) and #1090 (c's first NIT, lane 5 NIT 14). - KNOWN_OPEN in tests/test_path_guard_hygiene.py holds eleven sites, so both lines say eleven. - #1112 is a known limit; lane 2's two doctor limits (#975) and its test-only gaps (#975, #1050) are listed, and the "INVALID, not DRIFTED" sentence holds for a mount this user cannot list or read. - The shape-copy measurement fills its placeholder; the "Before tagging" pointer is gone. - The pipx claim carries RV1155d's qualifier everywhere it is made. - v1.0.0's tab-in-a-pin line is closed (#1015); its bare number or boolean in a plan's text field is narrowed (#1016). - `update-shape.py check` is named with `--at <release-commit>`, and the measurement says main `038dafb` was the upstream. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

Brett Heap and others added 6 commits October 19, 2026 14:22

brettheap added 3 commits October 19, 2026 23:27

Merge branch 'main' into fix/901-plan-value-order

7448478

Merge branch 'main' into fix/901-plan-value-order

30ca446

Merge branch 'main' into fix/901-plan-value-order

2ede510

brettheap marked this pull request as ready for review October 20, 2026 05:01

brettheap merged commit e2f9fc4 into main Oct 20, 2026
5 checks passed

brettheap deleted the fix/901-plan-value-order branch October 20, 2026 05:01

brettheap mentioned this pull request Oct 22, 2026

docs/release-notes/v1.2.0.md: the v1.2.0 release notes, a first draft for Brett Heap to amend before the tag #1155

Merged

Navigation Menu

Repository navigation

adopt-project.py: `check` and `execute` ask a plan's `org:` and `id:` before its leg paths and branch names, a `pins:` entry `_read_pins` names is that one finding, and a `digest_source:` finding says how the digest was read (#901) - #1015

brettheap commented Oct 19, 2026 •

edited by sourcery-ai Bot

Loading

Uh oh!

sourcery-ai Bot commented Oct 19, 2026

Interacting with Sourcery

Customizing Your Experience

Getting Help

Uh oh!

brettheap commented Oct 19, 2026

Uh oh!

brettheap commented Oct 20, 2026

Uh oh!

sonarqubecloud Bot commented Oct 20, 2026

Uh oh!

sourcery-ai Bot commented Oct 20, 2026

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Conversation

brettheap commented Oct 19, 2026 • edited by sourcery-ai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary by Sourcery

Uh oh!

sourcery-ai Bot commented Oct 19, 2026

Reviewer's Guide

Sequence diagram for aligned plan validation order

Sequence diagram for single pin finding and digest-source diagnostics

Flow diagram for pin diagnostic wording

File-Level Changes

Assessment against linked issues

Possibly linked issues

Interacting with Sourcery

Customizing Your Experience

Getting Help

Uh oh!

brettheap commented Oct 19, 2026

Findings, most severe first

Verified

Test counts at head (stub gh, bogus token, empty GH_CONFIG_DIR, one reused basetemp)

Merges

Mutant table

Left

Uh oh!

brettheap commented Oct 20, 2026

Findings

Verified

Counts

Not verified

Uh oh!

sonarqubecloud Bot commented Oct 20, 2026

Quality Gate passed

Uh oh!

sourcery-ai Bot commented Oct 20, 2026

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

brettheap commented Oct 19, 2026 •

edited by sourcery-ai Bot

Loading

Test counts at head (stub `gh`, bogus token, empty `GH_CONFIG_DIR`, one reused basetemp)