[brettheap]

#901 made check and execute name the same first problem among a plan's options. Outside them, the two still named different problems first (#1020, found by lane 1's writer on #901).

1. One rule, the refusal before what check lists. check refuses a bad org: or id: outright, exit 2, and lists none of its findings. execute refuses at the first of them, but it asked the two only after it had refused four things that check lists as findings: a plan for another repository (plan-not-the-source), a name the naming policy refuses (naming-unclassified), an unanswered entry (plan-unresolved) and a source that has moved on (plan-stale). And check asked the two only where the policy took the plan's names (_manifest_values), so an unsafe org: beside a refused name was a finding about the name, or about another repository. Both commands now ask the two first of the plan's values, right after the three repository names (plan.names(), then _checked_org_and_id): execute in cmd_execute, ahead of _refuse_a_plan_for_another_repository, and check at its head, after the triad advisory. Review round 1, below, moves both to right after the plan's keys, before _work_root. _checked_plan_values no longer asks them, since cmd_execute has.
2. 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, decided. An org: that checked_value refuses, held to a repository, was unsafe-value to check and plan-not-the-source to execute. Such an org: names no repository, on GitHub or in the manifest, so it is not compared with the source's. It is unsafe-value in both commands now, held to a repository or not. Once it is mended, a plan for another repository is still plan-not-the-source in both. This reverses three tests in tests/test_adopt_check_plan_name.py, which pinned the old answers on purpose:
   • test_an_unsafe_org_is_unsafe_in_check_and_compared_by_execute_if_held expected plan-not-the-source from execute in its six held rows. As test_an_unsafe_org_is_unsafe_in_check_and_in_execute_held_or_not, all twelve expect unsafe-value from both, in the same words.
   • test_check_compares_an_unsafe_org_where_no_manifest_is_built expected check to compare org: acme labs beside the refused name bad_name: plan-not-the-source and naming-unclassified, exit 1. As test_an_unsafe_org_is_asked_before_the_plans_refused_names, it expects unsafe-value, exit 2, from check and from execute.
   • test_check_asks_an_unsafe_org_first_where_a_kept_submodule_url_is_relative expected plan-not-the-source from execute. It now expects unsafe-value, in check's words.
3. A pins: entry that is no pin was spelled with repr (`pins:` entry 1 is 7, True, None). 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 made pins: itself read as _read_as reads it, and an entry reads the same way now: "the number 7", "the boolean true", "null", "a list of 2 item(s)".

Docstrings and comments now say this: _checked_org_and_id, _checked_plan_values, _manifest_values, _held_findings, _not_the_source, _refuse_a_plan_for_another_repository, _read_pins and the #499 block comment. No finding is added or removed: only which problem is named first changes, and the spelling of an entry.

Measured on main at 6da757cb and on this branch, with a stub gh, a bogus token, an empty GH_CONFIG_DIR, GIT_SSH_COMMAND=false and --local-remote-dir. No run made a leg.

The plan has	check on main	execute on main	both, here
a bad id beside an unanswered entry	plan-bad-id	plan-unresolved	plan-bad-id
a bad id beside a source that has moved on	plan-bad-id	plan-stale	plan-bad-id
a bad id beside a name the policy refuses	plan-bad-id	naming-unclassified	plan-bad-id
a bad id beside another organisation, held to a repository	plan-bad-id	plan-not-the-source	plan-bad-id
no id:, and legs.assembly: north_wind (a derived id the policy refuses too)	plan-bad-id	naming-unclassified	plan-bad-id
an unsafe org: held to a repository (L4-48)	unsafe-value	plan-not-the-source	unsafe-value
an unsafe org: beside an unanswered entry	unsafe-value	plan-unresolved	unsafe-value
an unsafe org: beside a source that has moved on	unsafe-value	plan-stale	unsafe-value
an unsafe org: beside a refused name	finding naming-unclassified, exit 1	naming-unclassified	unsafe-value
the same, held to a repository	finding plan-not-the-source, exit 1	plan-not-the-source	unsafe-value

Each "here" row is exit 2 from both, word for word. The last two rows agreed on main. They change because check now asks the two where the policy refuses the names, as execute now does: without that, moving execute alone would have made them disagree. Plans with only one of these problems (an unanswered entry, a moved source, a refused name, another repository) are named as on main by both.

Tests.

• tests/test_adopt_plan_value_order.py gains 16 rows:
  • the 10 above, through both commands, comparing the first line;
  • four pins: entries that are no pin: number, boolean, null and list;
  • two guard rows, id and org: a leg name checked_value refuses (legs.spec: "-spec") beside a bad id: or org: is that name's unsafe-value in both, so the three names are still asked first.
• The 14 non-guard rows all fail against 6da757cb's adopt-project.py and pass here. The two guard rows pass on both, by design.
• Existing rows changed for the new behaviour:
  • test_adopt_check_plan_name.py: the three tests above, and its docstring's L4-48 line.
  • test_adopt_pins.py: a-number.
  • test_adopt_execute_refusals.py: the three entry rows of PIN_ENTRIES.

Mutants, each applied to adopt-project.py, run against the 16 new rows and the L4-48 tests (30 tests), and restored with its sha256 checked (9178964d, the same after each one):

Mutant	Killed by
M1: cmd_execute does not ask _checked_org_and_id early	18: the 10 new rows, the 6 held L4-48 rows, and the two other reversed tests
M2: cmd_execute asks it after _refuse_a_plan_for_another_repository	11: the 3 new held rows, the 6 held L4-48 rows, and the two other reversed tests
M3: cmd_check asks plan.names() but not _checked_org_and_id at its head	3: org-and-refused-name, org-held-and-refused-name, test_an_unsafe_org_is_asked_before_the_plans_refused_names
M4: cmd_check asks _checked_org_and_id but not plan.names() at its head	2: both guard rows
M5: a pins: entry that is no pin spelled with repr again	4: the four new entry rows

Gate, on this branch's tree 66845bb3, under a stub gh, GH_TOKEN=bogus, an empty GH_CONFIG_DIR, GIT_CONFIG_GLOBAL an empty file, GIT_CONFIG_NOSYSTEM=1, GIT_ALLOW_PROTOCOL and GIT_SSH_COMMAND unset, and EMAIL set. 125 files in six shards. The branch is based on main at 6da757cb. main has since moved to 823624d9 (#1101, #1102), and its diff against 6da757cb is empty in the five files this branch changes.

• every tests/test_adopt_*.py (68 files, 34 in each of s1 and s2):
  • s1: 2013 passed, 2 skipped (local run on this head).
  • s2: 2560 passed, 1 skipped (local run on this head).
• the 55 other files that name ADOPT, adopt-project.py or adopter, among them test_repo_hygiene.py and test_agents_md_codes.py (28 files in s3, 27 in s4):
  • s3: 1992 passed, 8 skipped (local run on this head).
  • s4: 1860 passed, 1 skipped (local run on this head).
• tests/test_shape_doctor.py, s5: 445 passed, 3 skipped (local run on this head).
• tests/test_scaffold_leg_path_collisions.py, s6: 104 passed (local run on this head).

Merge-trees, git merge-tree --write-tree, run on 2026-10-21 against main at 823624d9 and against each open PR's head that touches adopt-project.py. Every one exits 0 and is clean:

• main at 823624d9: tree 728fb9e4. At review round 1's head 9549006f, against main at 038dafba (it took adopt-project.py: check names what the clone's .git holds as git keeps refs, and asks a pin's file through the links the root keeps, so a link into .git or onto a pin's file is refused before any leg or adopted with the copy beside it (#904) #1091): clean, tree 2cf183ed.
• README.md, adopt-project.py and the handbook: a kept directory entry makefile/ is tested, README says make at the root reads a kept GNUmakefile or makefile instead, and a docstring says the shape's Makefile alone holds CONTRACTS_DIR (#1061) #1123 (5c3908f8), adopt-project.py: an org/repo --source too long for the disk is refused as a path is, naming no GitHub repository and offering no url, so --source <300 a's>/Thing is no longer offered https://github.com/<300 a's>/Thing, which can never clone (#1003) #1122 (e75587e7), adopt-project.py: execute's NEXT prints python3 scripts/bootstrap.py where GNU make at the root reads a kept GNUmakefile or makefile, and its NOTE names that file and what it takes for make bootstrap to run there (#1047) #1121 (c04bb710), adopt-project.py: plan's source line names the source's branch as _shown names a path, so a bidi control, a line boundary or a byte that is not UTF-8 in it is spelled and no longer reaches the terminal raw (#1035) #1120 (c55e55e3), adopt-project.py: check asks the split's own clone of the source in the work directory, so a clone this git will not make is adopt-git-unasked before any leg, and _resolved and _answers make their repositories as check's other throwaways do (#1083) #1119 (bdcf9952), adopt-project.py, shape_materialize.py and tests: a named bad link left out of the copy and pin findings takes no other path's finding with it, and the docstrings and the materializer's refusal name the #979 exception and what helps it (#1089) #1118 (c60066cb), adopt-project.py: under a one-byte locale such as Latin-1, check reads a plan written in UTF-8 whose source is there, asking the plan's UTF-8 bytes where the spelling as written names nothing, rather than refusing plan-source-gone (#1078) #1106 (504a1dd6), shape_materialize.py and the pinned AGENTS-shape.md: where the root keeps a GNUmakefile or a makefile, which GNU make reads first, the Makefile collision follow-up no longer says the shape's make targets arrive with the merge, and the pinned file says make reads the first of the three and the block reaches it only once that file holds it (#1048) #1105 (5a4de7f7) and adopt-project.py: check names what the clone's .git holds as git keeps refs, and asks a pin's file through the links the root keeps, so a link into .git or onto a pin's file is refused before any leg or adopted with the copy beside it (#904) #1091 (160de118).

The merged trees were not run through the tests: the merges are clean as text only.

Review round 1 (RV1125's review of 2a53a8e6: one SHOULD and two NITs), head 9549006f, one commit on top of 2a53a8e6:

• SHOULD-1. A bad id: or org: beside a --work-dir this user cannot write in was plan-bad-id (or unsafe-value) to check and adopt-work-dir-unwritable to execute: check lists the directory as a finding (adopt-project.py check: refuse a kept file at the shape's pin path, and an unwritable --work-dir, before any leg exists #524), and execute refuses it before it opens the source, ahead of the two asks. Both commands now ask plan.names() and _checked_org_and_id(plan) right after the mistyped-plan question, before _work_root, as RV1125 sketched. They read only the plan. The comments in cmd_execute, cmd_check, _checked_org_and_id and _checked_plan_values say so, and the "A REFUSAL OF check'S IS ASKED BEFORE WHAT IT LISTS" paragraph names the work directory among what is asked after. A --work-dir that is a file or cannot be made agrees as well: both ask _work_root after the two now.
• Two side effects, said here. (1) check printed the Triad advisory, on stdout, once it had opened the source and before it refused a bad org: or id:; the refusal now comes before the source is opened, so it prints nothing on stdout. (2) execute asks the two before _require_filter_repo(), so a bad id: is the refusal even where git filter-repo is missing; check asks no such question.
• Two more reorders, said here (RV1125b's NIT-B). Besides the --work-dir that is a file, a bad id: or org: is now asked, in check and in execute, before _work_root refuses a --work-dir every user can write in, without the sticky bit (adopt-work-dir-shared), and before a source that has moved on is refused (plan-source-gone). On main at 6da757cb both commands said adopt-work-dir-shared or plan-source-gone of such a plan; here both say plan-bad-id or unsafe-value, so only which refusal comes first changes: the two commands agreed before and agree now. The same plan with no bad id: or org: is refused as before, in both. Measured at 6da757cb and at 9549006f, for a bad id: and for a bad org: beside each. No row pins either yet: see tests/test_adopt_plan_value_order.py: follow-up of PR #1125 (#1020) from its review: four mutants of the head ask (L1, L2, L4, L11) that none of its 85 rows kills, so a bad id: or org: beside a --work-dir that is a file or a shared one, or beside a gone source, is pinned through neither check nor execute #1179.
• Rows in tests/test_adopt_plan_value_order.py: both_on takes a --work-dir for both commands; test_the_org_or_the_id_is_named_before_an_unwritable_work_dir has a row for a bad id: and one for a bad org:, each beside a --work-dir that is chmod 500, asserting exit 2 from both, the same first problem, and no adopt-work-dir-unwritable. It skips where chmod cannot keep this user from writing (root, Windows), by test_adopt_root_symlinks.unwritable_work.
• NIT-1. The docstring of test_check_asks_an_unsafe_org_first_where_a_kept_submodule_url_is_relative says it is a regression row: since adopt-project.py: after #901, check and execute still name different first problems for a bad id beside an unanswered entry, a stale source or a refused name, and a pins: entry that is no pin is spelled with repr #1020 check asks org: at its head, so the relative url decides nothing.
• NIT-2. check prints its findings only at its end, so no finding tells a late ask from one at its head. Its stdout does: the Triad advisory comes after _work_root and the source are opened. test_check_asks_the_org_and_the_id_before_it_says_anything_else (a row for a bad id: and one for a bad org:) asserts that check prints nothing on stdout and refuses first on stderr, and its control, the unedited plan, says the advisory first.
• Rows against 2a53a8e6's adopt-project.py: the 4 new rows fail (the work-dir rows: execute says adopt-work-dir-unwritable where check says the id or the org; the NIT-2 rows: stdout holds the advisory), and the 2 guard rows pass.
• Mutants of this round, each applied to the new adopt-project.py, run against test_adopt_plan_value_order.py and test_adopt_check_plan_name.py (85 tests), restored with sha256 checked (412edbda, the same after each one):

Mutant	Killed by
N1: check asks the two after the advisory (this PR's place before the round)	2: both NIT-2 rows
N2: check asks them only before _print_what_will_happen (RV1125's K3)	2: both NIT-2 rows
N3: check asks them only before _not_the_source (K9)	2: both NIT-2 rows
N4: execute asks them after the work-dir refusal, before the source is opened	2: both work-dir rows
N5: execute asks them where it did before the round, after _require_filter_repo()	2: both work-dir rows

• Gate on a scratch merge commit 8bd1e05f of main at 038dafba and 9549006f (tree 2cf183ed, clean, not pushed), in the landing gate's environment, six shards: the PR's four test files, 416 passed; test_adopt_work_dir_refusal.py and test_adopt_shape_pin_and_work_dir.py, 73 passed; test_cli_reference.py, test_agents_md_codes.py, test_repo_hygiene.py, test_path_guard_hygiene.py and test_scaffold_leg_path_collisions.py, 323 passed; test_shape_doctor.py in three shards of 150, 150 and 148 ids (448 in all): 150 passed, 150 passed, 145 passed and 3 skipped. 0 failures. test_path_guard_hygiene.py passes on the merge: its three guards from adopt-project.py: check names what the clone's .git holds as git keeps refs, and asks a pin's file through the links the root keeps, so a link into .git or onto a pin's file is refused before any leg or adopted with the copy beside it (#904) #1091 are on main now, and the test is green there.
• Not done: RV1125's list of other open PRs was not re-run through the merge; a PR whose tests assume check printed the advisory before a refusal would fail, and none was found in this repository's tests.

Left for follow-up issues, not taken here (see #1113 and #1114):

• adopt-project.py: check and execute still name different first problems for four pairs with no bad id: stale and unanswered, another repository and unanswered, another repository and stale, a refused name and unanswered #1113: check and execute still name different first problems for four pairs that have no bad id or org::

  • a source that has moved on beside an unanswered entry;
  • a plan for another repository beside an unanswered entry;
  • a plan for another repository beside a moved source;
  • a refused name beside an unanswered entry.

  Each is check's finding order against execute's refusal order, and is the same on main and here.

• adopt-project.py: _pin_row_problem spells a pin field that is not text with Python's repr (commit: 7, True), where _read_as gives the number 7 and the boolean true #1114: _pin_row_problem spells a pin field that is not text with Python's repr (`commit: 7`, True).

Lane 4 lands nothing outside v1.2.0 until after the v1.2.0 tag, so this stays a draft until then.

Closes #1020

🤖 Generated with Claude Code
Lane: openreposhape-4 (openRepoShape-4)

[sourcery-ai]

Reviewer's Guide

The PR makes check and execute validate plan repository names, org:, and id: in the same precedence order and with the same refusal behavior, while formatting malformed pins: entries through _read_as instead of Python repr; extensive regression tests cover ordering, unsafe organizations, and diagnostic wording.

Sequence diagram for aligned plan validation in check and execute

sequenceDiagram
    participant User
    participant Command
    participant Plan
    participant Validation

    User->>Command: invoke check or execute
    Command->>Plan: names()
    Command->>Validation: _checked_org_and_id(plan)
    alt org or id refused
        Validation-->>Command: Refusal: unsafe-value or plan-bad-id
        Command-->>User: exit 2
    else values accepted
        alt check
            Command->>Validation: list findings
            Validation-->>Command: findings or success
        else execute
            Command->>Validation: _refuse_a_plan_for_another_repository()
            Command->>Validation: _check_names()
            Validation-->>Command: refusal or execution
        end
        Command-->>User: result
    end

Loading

Flow diagram for unified plan value precedence

flowchart TD
    A[Plan] --> B["plan.names()"]
    B --> C[_checked_org_and_id]
    C -->|refused| D[unsafe-value or plan-bad-id]
    C -->|accepted| E{Command}
    E -->|check| F[List findings]
    E -->|execute| G[_refuse_a_plan_for_another_repository]
    G --> H[_check_names]
    H --> I[Continue execution]

Loading

Flow diagram for human-readable malformed pin diagnostics

flowchart TD
    A[pins entry] --> B{Entry is a mapping?}
    B -->|yes| C[Process as pin]
    B -->|no| D[_read_as entry]
    D --> E[Report plan-pin-malformed with readable value]

Loading

File-Level Changes

Change	Details	Files
Unified validation order for plan organization and ID values across check and execute.	Ask the three repository names first, then validate org: and id: before repository matching, naming-policy checks, unresolved entries, stale sources, and other findings. Move execute validation ahead of repository refusal and remove the duplicate validation from _checked_plan_values. Add head-of-command validation in check, causing either command to refuse invalid values with matching code, wording, and exit status. Preserve the guard ordering so repository names still precede organization and ID validation. Update unsafe-organization behavior so invalid values are never compared as repositories and consistently produce unsafe-value. Revise related tests and add matrix coverage for competing org:/id: problems and refusal ordering.	adopt-project.py tests/test_adopt_check_plan_name.py tests/test_adopt_plan_value_order.py
Changed malformed pins: entry diagnostics to use the reader-facing value formatter.	Replace Python repr output with _read_as formatting for scalar, null, and list entries that are not pins. Update documentation and existing expectations for number, boolean, null, and list values. Add cross-command tests verifying matching diagnostic wording.	adopt-project.py tests/test_adopt_execute_refusals.py tests/test_adopt_pins.py tests/test_adopt_plan_value_order.py
Updated implementation documentation and regression coverage to describe the new refusal semantics.	Refresh docstrings and comments covering manifest extraction, held findings, repository comparisons, and refusal ordering. Add 16 focused value-order and pin-format cases, including guard cases and competing-problem scenarios. Update L4-48 expectations so unsafe organizations consistently win over repository comparison.	adopt-project.py tests/test_adopt_check_plan_name.py tests/test_adopt_plan_value_order.py tests/test_adopt_execute_refusals.py tests/test_adopt_pins.py

Assessment against linked issues

Issue	Objective	Addressed	Explanation
#1020	Make check and execute name the same first problem when a bad org: or id: appears alongside an unanswered entry, stale source, refused name, or another repository, while deciding the held unsafe-org: case.	✅
#1020	Spell a pins: entry that is not a pin using the same reader-oriented wording as _read_as, rather than Python repr.	✅

---

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]

RV1125: independent review of #1125 at 2a53a8e6: NOT READY

One SHOULD: a bad id: or org: beside an unwritable --work-dir is still two different first problems, and the new comments say it cannot be. Everything else holds. The 10 rows agree, L4-48 is decided the right way, _read_pins reads as _read_as, and the merge with main 4ed7c7bf is clean and green.

What I measured

All of it was on a scratch merge commit of main 4ed7c7bf and the head 2a53a8e6: commit e394d426, tree 6bb897ee, adopt-project.py sha256 3da1ea61. I compared it with main itself. Both commands ran on each hand-edited plan, with the same --local-remote-dir. The probe environment:

• a stub gh first on PATH, GH_TOKEN=bogus and an empty GH_CONFIG_DIR;
• GIT_SSH_COMMAND=false and an https_proxy that goes nowhere;
• GIT_CONFIG_GLOBAL an empty file and GIT_CONFIG_NOSYSTEM=1.

No run made a leg. The table gives the code each command names first, with check's exit code. On the merge, every bad-id row is exit 2 from both commands, with stderr identical byte for byte.

The plan has	main: check / execute	merge: both	Issue
a bad id beside an unanswered entry	plan-bad-id / plan-unresolved	plan-bad-id	#1020
a bad id beside a source that moved on	plan-bad-id / plan-stale	plan-bad-id	#1020
a bad id beside a refused name	plan-bad-id / naming-unclassified	plan-bad-id	#1020
a bad id beside another org, held	plan-bad-id / plan-not-the-source	plan-bad-id	#1020
no id:, legs.assembly: north_wind	plan-bad-id / naming-unclassified	plan-bad-id	#1020
org: "-evil", held (L4-48)	unsafe-value / plan-not-the-source	unsafe-value	#1020
org: "-evil" beside an unanswered entry	unsafe-value / plan-unresolved	unsafe-value	#1020
org: "-evil" beside a moved source	unsafe-value / plan-stale	unsafe-value	#1020
org: "-evil" beside a refused name	finding naming-unclassified (1) / same	unsafe-value	#1020
the same, held	finding plan-not-the-source (1) / same	unsafe-value	#1020
id: 007 beside an unanswered entry	plan-bad-id / plan-unresolved	plan-bad-id	#1020
org: [a, b], held	unsafe-value / plan-not-the-source	unsafe-value	#1020
a bad id beside a bad date and an unanswered entry	plan-bad-id / plan-unresolved	plan-bad-id	#1020
a bad id, another org (held), moved, unanswered	plan-bad-id / plan-not-the-source	plan-bad-id	#1020
a bad id, another org, held only by source: repository:	plan-bad-id / plan-not-the-source	plan-bad-id	#1020
org: "-evil", held only by source: repository:	unsafe-value / plan-not-the-source	unsafe-value	#1020
a bad id beside pins: [7], follow_ups: 7, a taken split branch, or legs.spec: "-spec"	agree on main	agree (unchanged)	-
a bad id beside paths: 7	finding plan-bad-paths (1) / refused plan-bad-paths	unchanged, agree	-
a bad id: beside an unwritable --work-dir	plan-bad-id / adopt-work-dir-unwritable	still differ	SHOULD-1
org: "-evil" beside an unwritable --work-dir	unsafe-value / adopt-work-dir-unwritable	still differ	SHOULD-1
a moved source and an unanswered entry	finding plan-stale / plan-unresolved	unchanged, still differ	#1113
another repository and an unanswered entry	finding plan-unresolved / plan-not-the-source	unchanged, still differ	#1113
another repository and a moved source	finding plan-stale / plan-not-the-source	unchanged, still differ	#1113
a refused name and an unanswered entry	finding plan-unresolved / naming-unclassified	unchanged, still differ	#1113

The single-problem plans name the same thing on the merge as on main:

• an unanswered entry;
• a moved source;
• a refused name;
• another repository;
• an unheld org: "-evil".

In each, stderr is identical on main and on the merge, apart from the commit hashes of the moved source. So are #1113's four pairs. The PR changes nothing it does not claim to change.

Findings

SHOULD-1: a bad id: or org: beside an unwritable --work-dir still gives two first problems, and the new comments say otherwise.

• Measured on main and on the merge alike: check --work-dir <a chmod 500 dir> gives REFUSED plan-bad-id (or unsafe-value), exit 2, and says nothing of the directory. execute with the same --work-dir gives REFUSED adopt-work-dir-unwritable, exit 2.

• Why: check lists the directory as a finding (_unwritable_work_dir, adopt-project.py check: refuse a kept file at the shape's pin path, and an unwritable --work-dir, before any leg exists #524), and execute refuses it before it opens the source, ahead of plan.names() and _checked_org_and_id.

• The claims it breaks:

  • the new comment in cmd_execute ("before anything check lists as a finding is refused");
  • _checked_org_and_id's new paragraph "A REFUSAL OF check'S IS ASKED BEFORE WHAT IT LISTS".

• Not a regression, but it is a bad-id case, adopt-project.py: after #901, check and execute still name different first problems for a bad id beside an unanswered entry, a stale source or a refused name, and a pins: entry that is no pin is spelled with repr #1020's class. adopt-project.py: check and execute still name different first problems for four pairs with no bad id: stale and unanswered, another repository and unanswered, another repository and stale, a refused name and unanswered #1113 lists it only as "suspected but not measured", under a title that says "no bad id". The PR body's follow-up list does not name it.

• Fix (preferred): ask the two in both commands right after the mistyped-plan question, before _work_root. They read only the plan:

  • in cmd_check, move plan.names() and _checked_org_and_id(plan) to before work_root = _work_root(args);
  • in cmd_execute, move them to right after _refuse_a_mistyped_plan(plan).

  I applied that sketch to the merge's adopt-project.py, ran the checks below, and restored the file (sha256 checked):

  • both work-dir rows agree, as plan-bad-id and as unsafe-value;
  • every row above that agreed still agrees, and adopt-project.py: check and execute still name different first problems for four pairs with no bad id: stale and unanswered, another repository and unanswered, another repository and stale, a refused name and unanswered #1113's four are unchanged;
  • 187 passed: my probe, test_adopt_plan_value_order.py, test_adopt_check_plan_name.py, test_adopt_work_dir_refusal.py and test_adopt_shape_pin_and_work_dir.py.

  Add the two rows to BESIDE, passing the same --work-dir to both commands.

• Fix (minimum), if this is left to adopt-project.py: check and execute still name different first problems for four pairs with no bad id: stale and unanswered, another repository and unanswered, another repository and stale, a refused name and unanswered #1113:

  • Narrow both comments to "first of the plan's values", and name the exception: an unwritable --work-dir is refused by execute before the source is opened (adopt-project.py: check and execute still name different first problems for four pairs with no bad id: stale and unanswered, another repository and unanswered, another repository and stale, a refused name and unanswered #1113).
  • List it under "Left for follow-up" in the body.
  • Record the measurement on adopt-project.py: check and execute still name different first problems for four pairs with no bad id: stale and unanswered, another repository and unanswered, another repository and stale, a refused name and unanswered #1113, saying that it does involve a bad id.

NIT-1: test_check_asks_an_unsafe_org_first_where_a_kept_submodule_url_is_relative's docstring still opens on RV825's premise. The premise was that _submodule_findings asks org: earlier than _manifest_values. After this PR, check asks it at its head, before either, so the relative url no longer decides anything, and the test now matches the one above it. Say that it is kept as a regression row: since #1020 check asks org: at its head, whatever the submodule urls.

NIT-2: no test pins check's head position against a later one.

• The survivors: mutants K3 and K9, which ask _checked_org_and_id late in check (before _print_what_will_happen, or before _not_the_source), pass every row.
• Why: check prints its findings only at the end, so any refusal before that point wins. The position shows only against a refusal raised between the head and the late spot. _check_split_branch and _entry_findings raise none directly.
• Not a defect; the head placement is the right hygiene. Note it in the docstring, or add a row if such a refusal exists.

L4-48: decided right (no finding)

unsafe-value is the right first problem. Reversing the three rows is justified.

• 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 rider says such an org: "is refused as unsafe-value by check, as on main, and is never compared as a repository". adopt-project.py: check prints the rename-first remedy as execute does, and a plan's recorded source: repository: holds its names where the live source names none, so check and execute both refuse plan-not-the-source (#716) #825 then documented the split, held or not, without deciding it. adopt-project.py: after #901, check and execute still name different first problems for a bad id beside an unanswered entry, a stale source or a refused name, and a pins: entry that is no pin is spelled with repr #1020 asked to decide it, and this PR decides it in the rider's own words.
• What the comparison could say: an org: that checked_value refuses names no <org>/<repo>. Comparing it can only say "not the source", which hides the defect. Held to no repository, it was already unsafe-value in both commands, so "held or not" now has one answer.
• Safety is unchanged. _checked_org_and_id reads only the plan, so execute still asks no naming policy and no remote before plan-not-the-source (adopt-project.py plan never compares --project (or --org) with the repository it adopts in place, so a source named billing-service is refused with a remediation that steers to --project BillingService, which plans and checks plan ok against acme/billing-service, and execute then acts on acme/BillingService: a raw git ls-remote failure when no such repository exists, and, when a different repository of that name does, both legs, the split branch, the pull request and the topic on that repository #499). Once the org: is mended, a plan for another repository is still plan-not-the-source in both. My org: acme rows agree.
• AGENTS.md spells neither unsafe-value nor plan-not-the-source (grep), so it pins neither answer, and test_agents_md_codes.py passes on the merge.
• The reversed rows are stronger than the ones they replace. The held rows now assert that execute's code: detail is check's line, byte for byte. The old rows asserted only the code.

_read_pins: right (no finding)

An entry that is neither a name nor a mapping is now _read_as(row): "the number 7", "the boolean true", "null", "a list of 2 item(s)". That is how pins: itself is said (_plan_key_findings), and how the items of follow_ups: and allow_empty_legs: are said. No doc quotes the old repr text; the only hits outside adopt-project.py are the changed tests. A name entry keeps repr, which is right: it is text, quoted as the other names are. _pin_row_problem's fields are #1114's.

Merge-trees

• Main 4ed7c7bf: git merge-tree --write-tree exits 0, tree 6bb897ee. Its diff from main is exactly the PR's five files, and its changed lines equal the PR's diff from 6da757cb.

• adopt-project.py: check names what the clone's .git holds as git keeps refs, and asks a pin's file through the links the root keeps, so a link into .git or onto a pin's file is refused before any leg or adopted with the copy beside it (#904) #1091: its changes to check sit in _entry_findings (the _SymlinkProbe moved before _leg_findings), after the new head ask, so the order holds on the merge. The probes above and the rows below show it.

• The merge against each open PR that touches adopt-project.py: clean (exit 0) with README.md, adopt-project.py and the handbook: a kept directory entry makefile/ is tested, README says make at the root reads a kept GNUmakefile or makefile instead, and a docstring says the shape's Makefile alone holds CONTRACTS_DIR (#1061) #1123 5c3908f8, adopt-project.py: an org/repo --source too long for the disk is refused as a path is, naming no GitHub repository and offering no url, so --source <300 a's>/Thing is no longer offered https://github.com/<300 a's>/Thing, which can never clone (#1003) #1122 c01c4d73, adopt-project.py: execute's NEXT prints python3 scripts/bootstrap.py where GNU make at the root reads a kept GNUmakefile or makefile, and its NOTE names that file and what it takes for make bootstrap to run there (#1047) #1121 1c517375, adopt-project.py: plan's source line names the source's branch as _shown names a path, so a bidi control, a line boundary or a byte that is not UTF-8 in it is spelled and no longer reaches the terminal raw (#1035) #1120 c55e55e3, adopt-project.py: check asks the split's own clone of the source in the work directory, so a clone this git will not make is adopt-git-unasked before any leg, and _resolved and _answers make their repositories as check's other throwaways do (#1083) #1119 9b998936, adopt-project.py: under a one-byte locale such as Latin-1, check reads a plan written in UTF-8 whose source is there, asking the plan's UTF-8 bytes where the spelling as written names nothing, rather than refusing plan-source-gone (#1078) #1106 504a1dd6 and shape_materialize.py and the pinned AGENTS-shape.md: where the root keeps a GNUmakefile or a makefile, which GNU make reads first, the Makefile collision follow-up no longer says the shape's make targets arrive with the merge, and the pinned file says make reads the first of the three and the block reaches it only once that file holds it (#1048) #1105 5a4de7f7.

• adopt-project.py, shape_materialize.py and tests: a named bad link left out of the copy and pin findings takes no other path's finding with it, and the docstrings and the materializer's refusal name the #979 exception and what helps it (#1089) #1118 c60066cb conflicts in tests/test_adopt_root_shape_paths.py. It conflicts with main alone in the same file, and merges clean with this head, so the conflict is adopt-project.py, shape_materialize.py and tests: a named bad link left out of the copy and pin findings takes no other path's finding with it, and the docstrings and the materializer's refusal name the #979 exception and what helps it (#1089) #1118's with adopt-project.py: check names what the clone's .git holds as git keeps refs, and asks a pin's file through the links the root keeps, so a link into .git or onto a pin's file is refused before any leg or adopted with the copy beside it (#904) #1091, not this PR's.

• tests/test_path_guard_hygiene.py and shape-doctor.py: a bare is_file/is_dir/exists guard in a shipped tool fails by file and line unless its _is_a or a reasoned allowlist covers it, and a project.yaml linked into an unsearchable folder is yaml-unreadable (#430) #1111 (b4484647, tests/test_path_guard_hygiene.py): this PR adds no is_file, is_dir, exists, is_symlink or os.path guard (grep of the added lines: none). On a scratch merge of main + adopt-project.py: check and execute ask a plan's org: and id: right after its three repository names, so a bad one beside an unanswered entry, a moved source, a refused name or another repository is that one refusal in both, and a pins: entry that is no pin is said as _read_as says it (#1020) #1125 + tests/test_path_guard_hygiene.py and shape-doctor.py: a bare is_file/is_dir/exists guard in a shipped tool fails by file and line unless its _is_a or a reasoned allowlist covers it, and a project.yaml linked into an unsearchable folder is yaml-unreadable (#430) #1111, the test fails (1 failed, 48 passed) on three guards. All three are on main + tests/test_path_guard_hygiene.py and shape-doctor.py: a bare is_file/is_dir/exists guard in a shipped tool fails by file and line unless its _is_a or a reasoned allowlist covers it, and a project.yaml linked into an unsearchable folder is yaml-unreadable (#430) #1111 without this PR as well, and all come from adopt-project.py: check names what the clone's .git holds as git keeps refs, and asks a pin's file through the links the root keeps, so a link into .git or onto a pin's file is refused before any leg or adopted with the copy beside it (#904) #1091:

  • _PlannedRoot._into_git;
  • _PlannedRoot._no_directory;
  • _clone_ref_store.

  So the failure is tests/test_path_guard_hygiene.py and shape-doctor.py: a bare is_file/is_dir/exists guard in a shipped tool fails by file and line unless its _is_a or a reasoned allowlist covers it, and a project.yaml linked into an unsearchable folder is yaml-unreadable (#430) #1111's to resolve with adopt-project.py: check names what the clone's .git holds as git keeps refs, and asks a pin's file through the links the root keeps, so a link into .git or onto a pin's file is refused before any leg or adopted with the copy beside it (#904) #1091, not adopt-project.py: check and execute ask a plan's org: and id: right after its three repository names, so a bad one beside an unanswered entry, a moved source, a refused name or another repository is that one refusal in both, and a pins: entry that is no pin is said as _read_as says it (#1020) #1125's.

Tests on the scratch merge e394d426

All runs used the landing gate's environment: a stub gh, GH_TOKEN=bogus, an empty GH_CONFIG_DIR, GIT_CONFIG_GLOBAL an empty file, GIT_CONFIG_NOSYSTEM=1, GIT_ALLOW_PROTOCOL and GIT_SSH_COMMAND unset, EMAIL set, and -p no:cacheprovider -o tmp_path_retention_policy=failed.

• The PR's four files (test_adopt_plan_value_order.py, test_adopt_check_plan_name.py, test_adopt_execute_refusals.py, test_adopt_pins.py): 412 passed.

• test_cli_reference.py, test_agents_md_codes.py, test_repo_hygiene.py and test_scaffold_leg_path_collisions.py: 274 passed, exit 0.

• test_shape_doctor.py, in three shards of 154, 153 and 141: 154 passed; 153 passed; 138 passed and 3 skipped (PowerShell, Windows only). In all, 445 passed and 3 skipped, exit 0 in each shard.

• The PR's new rows and its 8 reversed L4-48 rows, against main 4ed7c7bf's adopt-project.py:

  • all 14 new non-guard rows fail;
  • all 8 reversed rows fail;
  • the 2 guards and the 6 held-to-none rows pass.

  That is the body's claim, against today's main.

Mutants

Each mutant was applied to the merge's adopt-project.py and run against 47 tests:

• the 16 new rows;
• the 14 L4-48 tests;
• 17 probe rows of mine that assert the two commands name the same code.

The file was restored after each one, sha256 3da1ea61...b255 each time.

Mutant	Result
K1: execute asks org/id after _check_names	killed, 24 (14 PR tests)
K2: execute asks org/id after _refuse_an_unrunnable_plan	killed, 34 (18 PR tests)
K3: check asks org/id only before _print_what_will_happen	survived, equivalent on stderr (NIT-2)
K4: check asks org/id before plan.names()	killed, 2 (the guards)
K5: execute asks org/id before plan.names()	killed, 2 (the guards)
K6: _checked_org_and_id asks the id before the org	survived: #901's order, shared by both, so it cannot split them; outside this PR
K7: an entry that is no pin spelled with str()	killed, 4
K8: execute asks org/id after plan-not-the-source, before _check_names	killed, 18 (11 PR tests)
K9: check asks org/id only before _not_the_source	survived, equivalent on stderr (NIT-2)

Lane: openreposhape-4 (openRepoShape-4)

[sonarqubecloud]

Quality Gate passed

Issues
2 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

[brettheap]

RV1125b: independent review of #1125 at 9549006f: READY

No MUST and no SHOULD. RV1125's SHOULD-1 is fixed, NIT-1 and NIT-2 are done, and no plan that agreed at 2a53a8e6 disagrees now. One NIT below: the head position is pinned against the advisory and the unwritable directory, but not against _work_root's own refusals or a gone source.

What I measured

Scratch merge 68f8fa79 of origin/main 038dafba and 9549006f (tree 2cf183ed, adopt-project.py sha256 333e5b89). git merge-tree exits 0; the tree differs from main only in the PR's five files, and its changed lines equal the PR's diff from 6da757cb. For the round's own change I also merged main with 2a53a8e6 (3606b4b7).

A probe of 56 hand-edited plans (RV1125's 33 and 23 more) ran both commands on main, on main + 2a53a8e6 and on the merge: stub gh, GH_TOKEN=bogus, empty GH_CONFIG_DIR, GIT_SSH_COMMAND=false, a dead proxy, empty global git config, --local-remote-dir.

The plan has	main and 2a53a8e6: check / execute	merge: both
a bad id: beside an unwritable --work-dir (SHOULD-1)	plan-bad-id / adopt-work-dir-unwritable	plan-bad-id
a bad org: beside an unwritable --work-dir (SHOULD-1)	unsafe-value / adopt-work-dir-unwritable	unsafe-value
a derived bad id, or a bad id beside a moved source and an unanswered entry, beside an unwritable --work-dir	plan-bad-id / adopt-work-dir-unwritable	plan-bad-id
legs.spec: "-spec" beside an unwritable --work-dir	unsafe-value / adopt-work-dir-unwritable	unsafe-value
a bad id:, org: or leg name with git filter-repo off PATH	value / filter-repo-missing	the value's code
a bad id: or org: beside a --work-dir that is a file, or one every user can write in	adopt-work-dir-unwritable or adopt-work-dir-shared, both	the value's code
a bad id: or org: beside a gone source (source.local_path moved; with and without a recorded repository to clone again)	plan-source-gone, both	the value's code

• On the merge every one of the 40 plans with a bad id:, org: or leg name beside another problem is exit 2 from both commands with the same first problem, and stderr is byte-identical between them in 38. The two others are paths: 7: a finding against a refusal, the same code, as on main.
• No plan that agreed at 2a53a8e6 disagrees now. The rows that changed answer changed in both commands together.
• The 15 plans with no bad value (adopt-project.py: check and execute still name different first problems for four pairs with no bad id: stale and unanswered, another repository and unanswered, another repository and stale, a refused name and unanswered #1113's four pairs, four single problems, seven work-dir, source and filter-repo controls) print the same stderr and stdout on main and on the merge, commit hashes masked.

The two side effects, and one more

• (1) is true and acceptable: for every bad-value plan check's stdout is empty on the merge, where it held the Triad advisory. An unheld org: "-evil" differs from main only there.
• (2) is true and acceptable: with git filter-repo missing, execute said filter-repo-missing where check named the bad value; both name the value now.
• Not named in the body, and acceptable: a bad value is now asked before _work_root refuses a shared --work-dir (adopt-work-dir-shared) and before a gone source is refused (plan-source-gone), in both commands. Both agreed before and agree now, and a refused run now makes no work directory and clones nothing again. The body names only the --work-dir that is a file.

Findings

NIT-A: no row pins the head ask against _work_root's own refusals or a gone source. The PR's 85 tests in the two order files pass under L1, L4 and L11 (and L2), which put the asks of one command after _work_root (or after the source is opened). Each makes the two commands disagree on a bad id: or org: beside a --work-dir that is a file or a shared one (and, for L2, beside a gone source): my probe kills them. Fix: add --work-dir-is-a-file and gone-source rows to test_the_org_or_the_id_is_named_before_an_unwritable_work_dir, through both commands. A follow-up is fine.

NIT-B: the body could name the shared --work-dir and the gone source beside "a --work-dir that is a file".

Mutants

Each applied to the merge's adopt-project.py in its own worktree, run against test_adopt_plan_value_order.py and test_adopt_check_plan_name.py (85 tests) and against my probe's 40 bad-value plans asserting that both commands name the same code; restored after each, sha256 333e5b89 every time.

Mutant	PR tests (85)	probe (40)
L1: check asks after _work_root	survive	killed, 5
L2: check asks after the source is opened	survive	killed, 9
L3: check asks after the advisory (the round's old place)	killed, 2 (NIT-2 rows)	killed, 9
K3: check asks org/id only before _print_what_will_happen	killed, 2 (NIT-2 rows)	killed, 8
K9: check asks org/id only before _not_the_source	killed, 2 (NIT-2 rows)	killed, 8
L4: execute asks after _work_root	survive	killed, 5
L5: execute asks after the unwritable refusal (N4)	killed, 2 (work-dir rows)	killed, 11
L6: execute asks after the source is opened	killed, 2 (work-dir rows)	killed, 14
L7: execute asks after _require_filter_repo (N5)	killed, 2 (work-dir rows)	killed, 17
L8: both ask after _work_root	survive	survive: both name the work dir, so they still agree
L9: check asks org/id before the three names	killed, 2 (guards)	killed, 1
L10: execute asks org/id before the three names	killed, 2 (guards)	killed, 1
L11: check names at the head, org/id after _work_root	survive	killed, 4
L12: execute org/id at the head, names after _require_filter_repo	killed, 2 (guards)	killed, 4
L13: check org/id at the head, names after _work_root	killed, 2 (guards)	killed, 2

RV1125's K3 and K9 die now, as W1020b said.

#1111's rule

test_path_guard_hygiene.py passes on the merge (below). The PR adds no is_file, is_dir, exists, is_symlink or os.path guard (grep of the added lines: 0).

Gate on the merge 68f8fa79

Landing gate's environment: stub gh, GH_TOKEN=bogus, empty GH_CONFIG_DIR, GIT_CONFIG_GLOBAL an empty file, GIT_CONFIG_NOSYSTEM=1, GIT_ALLOW_PROTOCOL and GIT_SSH_COMMAND unset, EMAIL set, -p no:cacheprovider -o tmp_path_retention_policy=failed.

• The PR's four files: test_adopt_plan_value_order.py and test_adopt_check_plan_name.py, 85 passed; test_adopt_execute_refusals.py and test_adopt_pins.py, 331 passed (416 in all).
• test_adopt_work_dir_refusal.py and test_adopt_shape_pin_and_work_dir.py: 73 passed.
• test_cli_reference.py, test_agents_md_codes.py and test_repo_hygiene.py: 170 passed.
• test_path_guard_hygiene.py and test_scaffold_leg_path_collisions.py: 153 passed.
• test_shape_doctor.py in 8 shards of 56 ids (bash mapfile lists, each non-empty; together all 448 collected ids once): 56; 56; 55 + 1 skipped; 55 + 1 skipped; 55 + 1 skipped; 56; 56; 56. In all 445 passed, 3 skipped, exit 0 in each.

Merge-trees

Main + this PR against each of the 36 other open PRs: 35 clean (exit 0). #1117 conflicts in docs/handbook.html, as it does with main alone, and is clean against this head. For the 12 that touch adopt-project.py (#1105, #1106, #1118 to #1123, #1141, #1143, #1153, #1154), cmd_check and cmd_execute are unchanged on the merged tree from their head down to NamingPolicy.load. No open PR's tests assume check prints the advisory before a refusal (grep of their added test lines). #1119 and #1140 edit tests/test_adopt_root_symlinks.py but leave the unwritable_work fixture this PR imports as it is.

Lane: openreposhape-4 (openRepoShape-4)