Skip to content

An adopted assembly records nothing about which leg a human chose for a source submodule, so shape-doctor.py cannot verify a kept gitlink's placement and can only note it; adopt should write the plan's submodule answers into the assembly #189

Closed

Description

@brettheap

(lane openRepoShape-2, 2026-10-09T08:16:29Z) Raised by Codex's review of PR #186 (the #176 fix) and accepted by lane openRepoShape-2 as a real gap; filed so it is tracked. Not claimed.

What is wrong. Under #166 (fix/adopt-split-submodule-plan), every plan entry that covers a gitlink is answered by a human, and a source submodule may legitimately land in a leg whose rule names the OTHER leg or the root (.cursor in code; docs/theme in code). Nothing in the resulting assembly records that answer: the adoption plan is not committed, project.yaml carries no per-path placement, and the leg's own .gitmodules only registers the submodule. So shape-doctor.py (after #186) can report such a gitlink only as a note, in two flavours: kept because the root keeps no source submodule, or unverifiable because the rule says the other leg. It can never say COMPLIANT or MISPLACED with evidence, and a submodule added by hand after adoption looks identical to one a human placed through plan.

What would fix it. execute writes the plan's gitlink answers into the assembly, e.g. a submodules: block in project.yaml (path, leg, the resolution: line, the plan's commit) or a small contracts/submodule-placement.yaml, committed with the split; shape-doctor.py reads it and turns the unverifiable note into COMPLIANT when the leg matches the recorded answer, MISPLACED when it does not, and keeps the note only for a gitlink no record covers. #166's ask_about_submodules already has every fact at plan time; the writer side is a few lines in _commit_the_split or materialize_assembly_root, plus the doctor's read. Sequencing: after #166 lands, since the question and the answers it records are #166's.

What must not break. Assemblies adopted before the record exists keep the note behaviour; the doctor's output on a project with no submodules is unchanged.

Context: PR #186 thread #186 (comment); this lane's #166 reviews under opensoft/brett-wip handoffs/xFactory/attachments/lane-openRepoShape-2/review166-794828c/r4-report.md (the shape-doctor follow-up section).

Activity

  1. brettheap commented on Oct 9, 2026

    @brettheap
    ContributorAuthor

    CLAIMED — lane openRepoShape-1, session f711ec94-a922-4e5f-8d35-efcfe983caa7@Eagle, 2026-10-09T14:02:15Z, for #189

    Logged in lanes/log/openRepoShape-1.md at f57ec0f9807e4321093988e65594860ad31dfa48.

    The three reads (lane-collision-protocol Rule 1):

    1. existing `CLAIMED —` comments on opensoft/openRepoShape#189:
       none
    2. `gh pr list --repo opensoft/openRepoShape --state all --search "#189"`, kept where the row or its body names `#189`:
       none
    3. `git ls-remote --heads git@github.com:opensoft/openRepoShape` branches naming `189` as a whole token:
       none
    
  2. brettheap commented on Oct 11, 2026

    @brettheap
    ContributorAuthor

    (lane openRepoShape-2, 2026-10-11T19:04:19Z)

    This lane reviewed fix/adopt-record-submodule-placement at e167d8b (no PR yet). The review was read-only, in this lane's own detached worktrees; it authored nothing and holds no claim. This was the fourth run of the review; the first three stopped on account swaps or usage limits.

    Verdict: CHANGES NEEDED. One test needs one argument. With that change the branch is MERGEABLE WITH NITS (0 P1, 1 P2, 3 P3). Sequencing with #188 and #199 is below.

    #189's ask is delivered. execute writes a submodules: record into project.yaml, with path, leg, resolution and source_commit for each submodule. The doctor then reports:

    • docs/theme in code as recorded. On main it is unverifiable.
    • A submodule moved to the other leg as MISPLACED, exit 1. On main that is COMPLIANT.

    P2-1. The branch merges cleanly with main a5b09c3, but one of its own tests then fails: tests/test_shape_doctor.py:3570 dies with TypeError: _leg_findings() missing 1 required positional argument: 'tree'.

    P3-1. Names holding NBSP, U+3000, ZWSP or similar characters never match their record row. adopt-project.py:2543-2548 writes them as \x.. or \u...., but scripts/repo_shape.py:248 does not read those escapes back. Moving such a submodule to the other leg stays COMPLIANT.

    P3-2. Nothing keeps the record current after adoption.

    • Stale rows are silent.
    • A deliberate move stays MISPLACED until someone hand-edits the manifest, which AGENTS.md:211-212 forbids.
    • The validator accepts ./x, x/, a//b and a\b, which the doctor never matches.

    The spelling half of #201's rule (_spelling_reason, scripts/shape_materialize.py:1287) refuses exactly those spellings. It has to move into scripts/repo_shape.py, because a project carries that file but not shape_materialize.py. Do not reuse leg_path_problem: it would refuse recorded names like café.

    P3-3. The placement row is never ok on a real adoption. shape-doctor.py:2213 returns note for the leg's .gitmodules before the record is consulted (:2229, :2136). The verdicts themselves are right.

    Nits:

    1. AGENTS.md is not updated for the record.
    2. _verify does not cross-check the rows against the legs.
    3. test_shape_doctor.py:3714 does not assert the row's status.
    4. Mutation MU12 (the last duplicate row wins) survives. When two rows name one path, the first row decides (shape-doctor.py:1851), and no test covers that. On head, rows code, spec give INVALID with placement ok, and rows spec, code give MISPLACED. Both exit 1, and the manifest row names the duplicate.

    Mutations: MU1 to MU11 are killed by the branch's own tests; MU12 survives.

    Tests:

    • e167d8b: 994 passed, 1 skipped.
    • +a5b09c3: 403 passed, 1 failed (P2-1). Main's new adoption tests: 470 passed. render-cli-reference --check: ok.
    • +a5b09c3 with the fix: 404 passed.
    • +b0c959e: 359 passed.
    • +0448944: 160 passed.
    • CI has not run.

    Merges (merge-tree e167d8b):

    Sequencing:

    Report and drivers: opensoft/brett-wip handoffs/xFactory/attachments/lane-openRepoShape-2/review189-e167d8b/ (final-report.md, tools/relaunch4/).

  3. 5 remaining items

  4. added a commit that references this issue on Oct 12, 2026
  5. added 7 commits that reference this issue on Oct 12, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions