Skip to content

update-shape.py: update-accept-local-missing-name asks a copy an earlier apply kept, read unchanged, as a module a written copy imports from and as an importer of a written one, so a plain apply that stops either on its import is refused (#1082) - #1133

Draft
brettheap wants to merge 4 commits into
mainfrom
fix/1082-guard-earlier-kept
Draft

update-shape.py: update-accept-local-missing-name asks a copy an earlier apply kept, read unchanged, as a module a written copy imports from and as an importer of a written one, so a plain apply that stops either on its import is refused (#1082)#1133
brettheap wants to merge 4 commits into
mainfrom
fix/1082-guard-earlier-kept

Conversation

@brettheap

@brettheap brettheap commented Oct 21, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Goal

#1082: update-shape.py apply refuses, before any write, every run that would leave a copy importing a name from a copy that does not define it, wherever the kept copy came from: this run's --accept-local, or an earlier run's.

Fix round 1 (53791e45, on a merge of main 038dafba) answers rv1133's review: its SHOULD 1 (the known limit was inside this Goal) is fixed here rather than filed, and its SHOULD 2 and NIT are taken.

What was measured on main, before the change

All probes are offline. gh is a stub that exits 97, GIT_SSH_COMMAND=false, GIT_CONFIG_GLOBAL=/dev/null, and git is 2.43.0. The tool is main's update-shape.py (sha256 ca47f90e, blob a7ae7505, the same at 2a119f28, 6da757cb, 4ed7c7bf and 038dafba). Scripts and logs are in lane 5's scratch v118/w1082/ (tools/probe1023.sh, tools/probe1075.sh, probe-*) and, for fix round 1, v120/f1133/ (tools/knownlimit.sh, tools/probe1075.sh, limit-*, probe-*1075).

#1075's untraced, on main's own history. #1075 has now landed, so main itself holds the case.

#1023's member_make_environment. A holder is cut at f1946d04, the parent of #1023's 9e154d87. Its bootstrap.py has no member_make_environment, and its siblings.py does not name it. Its own edit to bootstrap.py is committed.

The reverse direction: a copy an earlier run kept, as the importer (rv1133's SHOULD 1). rv1133 measured it with main's tool and with aa7ddf59's; fix round 1 measured it again with main's (limit-m/).

  • A family holder is cut at 4ed7c7bf. Its own edit to siblings.py adds from bootstrap import told_past, which the upstream's siblings.py does not import, and is committed. siblings.py --help still exits 0.
  • apply --at U1 --accept-local scripts/siblings.py --branch shape/first exits 0. U1 is main plus an unrelated change to validate-family.py.
  • U2 then renames told_past in bootstrap.py alone. check --at U2 reads upstream-changed scripts/bootstrap.py and unchanged scripts/siblings.py.
  • apply --at U2 --branch shape/second, with no --accept-local, exits 0, prints copied scripts/bootstrap.py, and commits. siblings.py --help then exits 1 with ImportError: cannot import name 'told_past' from 'bootstrap'.
  • This run's own write breaks a copy that worked before it, so it is the Goal's case. The one-step run, --accept-local scripts/siblings.py, was already refused by update-shape.py apply --accept-local: a kept .py copy that imports a name the run's new repo_shape.py dropped passes, and make bootstrap then dies on an ImportError #604 with scripts/bootstrap.py (written) no longer defines told_past, which scripts/siblings.py (kept) imports from it.

What changed

update-shape.py, _refuse_kept_copies_missing_names (#536, #604):

  • Each written .py copy is asked against its module as this run leaves it, whoever kept it. The reading is _copies_as_left, _written_copies_lacking and _names_as_left. The module may be:
    • kept by this run's --accept-local: as before, the line says (kept);
    • an unchanged row whose pin records the project's own bytes, which is a copy an earlier run kept that the upstream has not changed since. The line says (kept by an earlier run);
    • the upstream's own bytes at the target: written by this run (an upstream-changed row or an --add file), already-at-target, or unchanged with the target's bytes. The line says (the upstream's). No shipped copy meets this today: the reader rows of tests/test_update_shape_kept_helper.py find every name each shipped importer takes defined beside it. So it refuses only a target whose own copies disagree.
  • A copy an earlier run kept is asked as an importer too (fix round 1). update-shape.py apply --accept-local: a kept .py copy that imports a name the run's new repo_shape.py dropped passes, and make bootstrap then dies on an ImportError #604 asked only a copy this run's --accept-local keeps against a module this run writes (_kept_copies_importing_dropped). The rows _copies_as_left reads as kept by an earlier run are now passed beside the kept ones, under a new side, WRITTEN_UNDER_EARLIER.
    • Its line says who kept the importer: scripts/bootstrap.py (written) no longer defines told_past, which scripts/siblings.py (kept by an earlier run) imports from it. KEPT_IMPORTERS maps each (written) side to its importer's word, so a (kept) importer's line is unchanged.
    • _names_dropped still limits the refusal to names this run takes away: a name the module's bytes on disk define and its written bytes do not. A copy an earlier run kept whose names all stay, or that takes a name the module lacked before too, still applies.
    • An unchanged copy holding the upstream's own bytes is still not asked as an importer: that pair is two of the target's own copies.
  • The early if not kept: return is gone. A run with no --accept-local is asked too. The guard still stands after update-conflict, update-local-drift and update-rewrite-symlink, and before confirm and any write.
  • The refusal's code is kept, and so are its words where every line is one this run's --accept-local makes.
  • The remediation gives each kind its exit.
    • For a copy an earlier run kept, it says why check reads it unchanged, then gives two exits. As a module that lacks a name: merge the upstream's version by hand so it defines every name its line names. As an importer of a name the written module drops (fix round 1): change it so it no longer takes any name its (written) no longer defines line names. Then commit, and run again with --accept-local on each copy an earlier run kept that you changed, whose row then reads locally-modified. Or write the upstream's bytes over it (the remedy's own git show line names that copy), commit, and run again. All four exits are tested end to end, and each checks the state check then reads: locally-modified after the first, already-at-target after the second.
    • For two of the upstream's own copies, it says run this again with an --at whose copies agree.
  • The known limit is gone. The docstring paragraph that named it, and the row that pinned it, are replaced by the rows below that hold the refusal. KEPT_LEAD, the code and every (kept) line are as main has them.

tests/test_update_shape_kept_helper.py:

Fix round 1, after rv1133's review

  • Main merged. git merge origin/main (038dafba) was clean: merge commit 7aee5efa, tree 27a2785e, the same as git merge-tree --write-tree aa7ddf59 038dafba. The fix is one new commit on it, 53791e45. Nothing was amended, rebased or forced.
  • SHOULD 1 (the known limit). Fixed as above, rather than filed. New rows:
    • test_an_earlier_kept_copy_is_asked_when_the_next_apply_drops_its_name: end to end, check reads the kept bootstrap.py unchanged and repo_shape.py upstream-changed; the plain apply to dropped is refused with nothing written, one detail line, the general opening, and that copy's exits, and none of the other kinds' exits. Its premise is measured: the kept bootstrap.py beside dropped's repo_shape.py stops on the import.
    • test_an_earlier_kept_copy_importing_what_this_run_keeps_applies (2 controls): a name repo_shape.py lacked before too, and one the written repo_shape.py still defines. Both apply with no --accept-local, and the kept copy starts after the run exactly where it started before.
    • test_either_exit_for_an_earlier_kept_importer_applies (2): each exit the remedy gives that copy, with the state check reads after it; the apply then writes repo_shape.py, and bootstrap.py starts.
    • test_an_importer_an_earlier_run_kept_is_asked_against_a_written_module (2, in process): the line, the opening, the exits, and the again wording alone and beside a copy this run keeps.
    • test_an_importer_holding_the_targets_bytes_is_not_asked replaces test_an_importer_this_run_leaves_alone_is_not_asked, whose second param pinned the limit.
  • The scenario itself, with the head's tool (limit-h/): the second apply exits 2 with scripts/bootstrap.py (written) no longer defines told_past, which scripts/siblings.py (kept by an earlier run) imports from it, the holder is unchanged, and siblings.py --help exits 0. The one-step, first-apply and check logs match main's tool once paths and shas are masked. the family root's siblings.py: with a token in use, the clone and the fetch that carry it run in bootstrap.py's untraced() environment, so make siblings and family.py siblings write the token to no trace the caller asks git for (#1055) #1075's probe (probe-h1075/) is refused at its second apply too, with (kept by an earlier run) lacks untraced.
  • SHOULD 2. test_keeping_a_copy_that_imports_a_name_the_run_drops_is_refused, a update-shape.py apply --accept-local: a kept .py copy that imports a name the run's new repo_shape.py dropped passes, and make bootstrap then dies on an ImportError #604 end-to-end (written) row, now asserts startswith(KEPT_LEAD). rv1133's R2 is killed (below).
  • NIT. The exits rows (test_either_exit_the_remedy_names_applies and the new one) check that check reads locally-modified after the change exit and already-at-target after the drop exit. The first exit's id now says the edit was changed by hand, as the remedy's changed does.

Rows that fail on main

The new test file (185 rows) was run against main's update-shape.py (038dafba, sha256 ca47f90e, in a scratch clone): 17 failed, 168 passed (base-main.log). Against aa7ddf59's: 13 failed, 172 passed (base-aa7.log). With the change: 185 passed.

row on main on aa7ddf59
test_an_earlier_kept_copy_is_asked_when_the_next_apply_writes_its_importer assert 0 == 2: the second apply copies siblings.py fails: the old opening
test_with_branch_an_earlier_kept_copy_is_refused_before_any_branch assert 0 == 2 passes
test_either_exit_the_remedy_names_applies (2 params) assert 0 == 2: the apply the remedy follows is not refused passes
test_a_written_copy_is_asked_against_its_module_as_this_run_leaves_it (4 params) no refusal fails: the old opening
test_each_kind_of_module_is_given_its_own_exit (2 params) no refusal 1 fails: its remedy says merged, not changed
test_every_kind_of_line_in_one_run_is_one_refusal (2 params) only the (kept) line fails: the old opening
new: test_an_earlier_kept_copy_is_asked_when_the_next_apply_drops_its_name assert 0 == 2: the plain apply copies repo_shape.py the same
new: test_either_exit_for_an_earlier_kept_importer_applies (2 params) assert 0 == 2: the apply the remedy follows is not refused the same
new: test_an_importer_an_earlier_run_kept_is_asked_against_a_written_module (2 params) no refusal; beside a kept copy, only its (kept) line the same

These pass on main and after: the one-step row (test_the_one_step_run_is_refused_as_it_was, which pins the old opening), the #604 row that now pins KEPT_LEAD, the controls (test_an_earlier_kept_copy_that_defines_every_name_applies, the two new earlier-kept importer controls, the four module-defines-every-name controls), and test_an_importer_holding_the_targets_bytes_is_not_asked.

Mutants

Each was applied alone in a scratch clone, run on the test file, and restored by checkout with its sha256 checked.

Fix round 1, at 53791e45 (185 rows):

mutant result
N1, the earlier-kept rows not passed to _kept_copies_importing_dropped ({}), as aa7ddf59 had it killed, 5 failed: the 5 new rows that fail on main
N2, an earlier-kept importer called (kept) in its line killed, 3 failed: the end-to-end refusal row and both in-process rows
N3, an earlier-kept importer given the (written)-under-(kept) side killed, 3 failed: the same three
N4, the KEPT_LEAD opening kept for a (written) line under a copy an earlier run kept killed, 3 failed: the same three
N5, the change exit for an earlier-kept importer dropped killed, 3 failed: the same three
N6, the unchanged why and the --accept-local on each copy an earlier run kept exit given to an earlier-kept module only killed, 3 failed: the same three
N7, the module, not the earlier-kept importer, named for git show killed, 3 failed: the same three
N8, the "what the upstream's own copy of that file imports instead" clause for a (written)-under-(kept) line only killed, 1 failed: the end-to-end refusal row
N9, every unchanged row asked as an importer, the target's own bytes too killed, 1 failed: test_an_importer_holding_the_targets_bytes_is_not_asked
N10, the old general opening (a copy this run writes or keeps imports ...) killed, 10 failed
rv1133's R2, a (written)-only refusal given the general opening (sides <= {KEPT_MODULE}) killed, 1 failed: test_keeping_a_copy_that_imports_a_name_the_run_drops_is_refused (SHOULD 2); it survived at aa7ddf59
the first round's M1, the old kept set restored killed, 17 failed: the 17 rows that fail on main
the first round's M3, the guard called after _write_copies killed, 15 failed: every end-to-end refusal row, the 3 new ones among them
the first round's M7, earlier-kept and upstream's own swapped killed, 12 failed

No survivor. Every mutant was restored by checkout with its sha256 checked (5b386f8b), and each clone's status was clean at the end (mutants1.out, mutants2.out, mut-*.log).

The first round, at 7b447602 (179 rows):

mutant result
M1, the old kept set restored (left = dict.fromkeys(kept, KEPT_MODULE)) killed, 12 failed: the 12 rows that fail on main
M2, unchanged rows asked as importers (against the modules this run writes) but the written importers asked against the old kept set only killed, 14 failed
M3, the guard called after _write_copies (refused after a write) killed, 12 failed: every end-to-end refusal row sees copied
M4, a copy an earlier run kept called (kept) killed, 5 failed
M5, the upstream's own modules not asked killed, 5 failed
M6, the new opening always used killed, 1 failed: the one-step row's old opening
M7, earlier-kept and upstream's own swapped killed, 6 failed
M8, the --accept-local on each copy an earlier run kept exit dropped killed, 3 failed
M9, a written module read from its bytes on disk killed, 4 failed
M10, the unchanged sentence dropped killed, 2 failed

Runs

All runs went through lane 5's run9.sh, with the stub gh, GIT_SSH_COMMAND=false, GIT_CONFIG_GLOBAL=/dev/null and EMAIL exported. No more than two ran at once. The full suite was not run.

Fix round 1, at 53791e45 (each START line records the head and dirty=0; logs in v120/f1133/):

run git result log
the new file, tests/test_update_shape_kept_helper.py 2.43.0 185 passed in 100.27s own243.log
the same 2.47.1 185 passed in 95.67s own247.log
tests/test_update_shape.py, tests/test_update_shape_messages.py, tests/test_update_shape_paste_lines.py 2.43.0 144 passed in 181.71s runC.log
tests/test_update_shape_apply_lines_and_headers.py, tests/test_update_shape_merged_away.py, tests/test_update_shape_rewrite_symlink.py, tests/test_update_shape_landing.py 2.43.0 163 passed in 223.09s runD.log
tests/test_update_shape_add_not_a_directory.py 2.43.0 13 passed in 33.11s runF.log
tests/test_path_guard_hygiene.py and the trio (tests/test_repo_hygiene.py, tests/test_agents_md_codes.py, tests/test_cli_reference.py) 2.43.0 219 passed in 54.58s runE.log

No failure and no skip in any run at this head. The two runs of the new file went through run9.sh on 2026-10-22 at 13:31Z, the next three on 2026-10-22 from 15:16Z to 15:21Z, and runF.log at 15:23Z; each START line reads head=53791e45... and dirty=0. Of the 19 tests/test_update_shape*.py files, these 9 ran at this head. The other 10 did not run at this head: all 10 ran at the first round's head 7b447602 in shard A or B (below), over the first round's update-shape.py, which this round's commit changed. The full suite was not run.

The first round, at 7b447602 (tree e0e2d19e, the same tree as aa7ddf59): the new file 179 passed on git 2.43.0 and on git 2.47.1; shard A (29 files) 2031 passed, 5 skipped; shard B (29 files) 1814 passed, 5 skipped; the trio 170 passed. No failure in any run.

Merges

Read at 53791e45, after a re-fetch of main (origin/main is still 038dafba, the second parent of the branch's merge commit 7aee5efa):

  • git merge-tree --write-tree origin/main HEAD exits 0 and writes tree bfa8c1d4, which is HEAD's own tree: the branch holds 038dafba as an ancestor, so the merge adds nothing.
  • The merge commit 7aee5efa (parents aa7ddf59 and 038dafba) has tree 27a2785e, the same tree as git merge-tree --write-tree aa7ddf59 038dafba, which exits 0. It was clean.
  • Open pull requests: 37 were listed with gh pr list --state open --limit 100 --json number,headRefName,headRefOid,files. None but this one touches update-shape.py or tests/test_update_shape_kept_helper.py, so there is no other head to merge against. The list is open-prs-d1.json in v120/f1133/.
  • git diff --name-only origin/main HEAD names those two files and no other.

NIT round, after rc1133's re-check

rc1133 re-checked 53791e45 (#1133 issuecomment-6347273216): READY, with three NITs. This round takes all three, in one new commit, a7bfee1b, on 53791e45. It was pushed as a fast-forward of the branch; nothing was amended, rebased or forced. The commit changes tests/test_update_shape_kept_helper.py alone (+7, -4). update-shape.py is 53791e45's, sha256 5b386f8b, so the fix and every measurement in the sections above are unchanged.

  • NIT 1 (taken): a refusal in both directions at once now has its opening pinned. _missing_name_refusal opens with KEPT_LEAD where every line is one this run's --accept-local makes. A run with a (kept) lacks line and a (written) no longer defines line is such a run, but test_both_directions_in_one_run_are_one_refusal pinned its lines, its clause and its remedy, not its opening. rc1133's mutant O1 (lead = (KEPT_LEAD if len(sides) == 1 and sides <= {KEPT_MODULE, WRITTEN_MODULE} else LEFT_LEAD)) therefore survived the 185 rows. The row now asserts result.stderr.startswith(KEPT_LEAD), result.stderr, right after assert_refused_writing_nothing, and its docstring says why the opening is that one.
  • NIT 2 (taken): the Merges section lacked validate-manifest.py, validate-family.py, family.py and update-shape.py: an id:, a date or a repository: ending in a line feed is refused by name, and a pin's source_repository: that is no path or owner/repo name is named as the pin's (#1139) #1166. Its row is in "Merges at a7bfee1b" below, and it replaces the Merges section's "Open pull requests" line.
  • NIT 3 (taken): the title named only the first round's direction, a written copy asked against its module. It now names the copy an earlier apply kept in both roles: as the module a written copy imports from, and as an importer of a written module. It is 249 characters and ASCII. rc1133's 242-character rewrite was close, but its "a plain apply that would break it" is true only where the kept copy is the importer: where it is the module, the apply breaks the written importer. The new title says "either".

Mutant O1, before and after

Applied alone in this worktree and restored by checkout, sha256 5b386f8b checked, status clean afterwards. The mutant is rc1133's (mutate_rc.py, copied unchanged).

test file run result
53791e45's, without the new assert the row alone, with O1 applied survives: 1 passed, 184 deselected (o1-before-row.log). rc1133 measured the whole file: 185 passed
a7bfee1b's, with the new assert the whole file, with O1 applied killed: 1 failed, 184 passed (o1-after-file.log). The failure is test_both_directions_in_one_run_are_one_refusal at the new assert: the opening read a copy imports a name from a copy beside it whose bytes, as this run leaves them, do not define it, ...

No other mutant was needed: the round adds no behaviour and no new row, and the earlier rounds' 14 + 10 mutants stand at an unchanged update-shape.py.

Rows that fail on main

None, and that is measured, not assumed. The new assert pins an opening the head keeps exactly as main prints it, so it cannot fail on main. rc1133 had that only by reading, so it is measured here: the row, new assert included, run with main's update-shape.py (origin/main 961640c6, sha256 ca47f90e) swapped into the worktree, gives 1 passed (main-row.log); the file was restored to 5b386f8b afterwards. The assert guards that opening against a later regression like O1, as SHOULD 2's assert guards the one-step opening against R2.

Runs at a7bfee1b

All through lane 5's run9.sh, each START line at head=a7bfee1b... and dirty=0, with the stub gh and EMAIL exported; two ran at once, and the full suite was not run. No failure and no skip. Logs are in lane 5's scratch v124/n1133/.

run git result log
tests/test_update_shape_kept_helper.py and tests/test_path_guard_hygiene.py 2.43.0 234 passed (185 + 49) in 61.57s own243.log
tests/test_update_shape_kept_helper.py 2.47.1 (first on PATH, printed in the log) 185 passed in 48.73s own247.log

The commit changes no function, no path guard and no refusal text, so no other test file names anything it changed. The test file still has no assert that joins conditions with and or or (206 asserts, checked on the syntax tree), no non-ASCII byte, and its widest line is 79 characters.

Merges at a7bfee1b

Read after a re-fetch of main: origin/main is 961640c6 (#1155, which adds docs/release-notes/v1.2.0.md and nothing else). It touches neither of this PR's files.

Closes #1082

Claim: #1082 (comment)
Lane: openreposhape-5 (openRepoShape-5)

🤖 Generated with Claude Code

Summary by Sourcery

Extend apply-time import validation to detect and explain missing-name disagreements involving copies retained by earlier runs, preventing broken updates before any changes are written.

Bug Fixes:

  • Prevent update-shape.py apply from writing changes that would leave copies kept by earlier applies with missing imported names.
  • Validate each written copy against the module bytes as the run leaves them, including copies retained from earlier runs and the upstream's own copies.

Enhancements:

  • Provide source-specific refusal messages and remediation guidance for copies kept by the current run, earlier runs, or the upstream.
  • Run missing-name validation before branch creation or any file write, including applies without --accept-local.

Tests:

  • Add comprehensive end-to-end and in-process coverage for earlier-kept copies, reverse import relationships, remediation paths, target-byte controls, and combined refusals.

Brett Heap @claude
update-shape.py: update-accept-local-missing-name asks each copy an…
aa7ddf5 
… apply writes against its module as the run leaves it, so a copy an earlier apply kept, read `unchanged`, is asked too, and no `siblings.py` stopping on its import is copied (#1082)

#536 built the guard's set of modules from the rows `--accept-local`
names in this run, so a copy an earlier apply kept, which reads
`unchanged` once the upstream stops changing it, was never asked. The
next apply wrote an importer beside it unasked. Measured offline: a
holder cut at 6476c17 that applied 2a119f2 keeping its own
bootstrap.py was then written main's siblings.py, with its `untraced`
import (#1075), by `apply --at 6da757c`. That apply exited 0 and
committed, and `siblings.py --help` stopped on an ImportError. The same
held for #1023's `member_make_environment` with #1023 landed in two
halves.

Each written .py copy is now asked against its module as the run leaves
it: a copy this run keeps (`(kept)`), a copy an earlier run kept, whose
pin records the project's own bytes (`(kept by an earlier run)`), or the
upstream's own bytes at the target (`(the upstream's)`). The code stays
`update-accept-local-missing-name`, and a refusal whose every line is
one this run's --accept-local makes prints what it printed. The guard
still runs before `confirm` and before any write. An importer this run
does not write is still not asked, but for a kept one against a module
this run writes (#604). So a copy an earlier run kept that imports a
name the written module drops is a known limit, named in the docstring.

Closes #1082

Lane: openreposhape-5 (openRepoShape-5)

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@sourcery-ai

sourcery-ai Bot commented Oct 21, 2026

Copy link
Copy Markdown

Reviewer's Guide

The PR closes a cross-run safety gap in update-shape.py apply by validating written importers against every module’s bytes as the run will leave them—not just copies accepted in the current run—and adds ownership-aware refusals, remediation guidance, and comprehensive regression coverage.

Sequence diagram for apply missing-name refusal

sequenceDiagram
    participant Apply as update-shape.py apply
    participant Rows as Copy rows
    participant Validator as Missing-name validator
    participant Files as Copies as left

    Apply->>Rows: _written_copies
    Apply->>Files: _copies_as_left
    Files-->>Validator: Module ownership and bytes
    Apply->>Validator: _written_copies_lacking
    Validator->>Files: _names_as_left
    Files-->>Validator: Names defined by module
    alt Missing imported name
        Validator-->>Apply: _missing_name_refusal
        Apply-->>Apply: Nothing written
    else Names agree
        Validator-->>Apply: Continue to confirm and write
    end
Loading

Flow diagram for cross-run missing-name validation

flowchart TD
    A["apply starts"] --> B["Build copies as left"]
    B --> C["Classify module bytes"]
    C --> C1["Current --accept-local"]
    C --> C2["Earlier-run kept"]
    C --> C3["Upstream target"]
    C1 --> D["Check written importers"]
    C2 --> D
    C3 --> D
    D --> E{"Imported names all defined?"}
    E -->|Yes| F["Continue to confirm and write"]
    E -->|No| G["Refuse before any write"]
    G --> H["Explain module ownership and remediation"]
Loading

File-Level Changes

Change Details Files
Expand missing-import validation to evaluate every Python copy written by the run against the module bytes that will exist after the run.
  • Classify modules as kept by this run, kept by an earlier apply, or the upstream’s bytes based on row state, pins, and target data.
  • Remove the early return that skipped validation when no current --accept-local copies were present.
  • Read module definitions from pending write bytes or on-disk bytes via the new as-left helpers, while retaining the existing reverse check for kept importers and dropped names.
 update-shape.py
Make refusal diagnostics and remediation accurately describe the ownership and resolution path for each conflicting copy.
  • Add distinct (kept by an earlier run), (the upstream's), and existing (kept)/(written) line classifications.
  • Preserve the legacy refusal opening when all conflicts involve this run’s --accept-local, otherwise use a generalized opening that covers all as-left copies.
  • Provide tailored remediation for earlier-run-kept copies, including merge-and-reaccept or overwrite-and-rerun options, and require an agreeing --at target for upstream-owned conflicts.
  • Continue refusing before branch creation, confirmation, or any write.
 update-shape.py
Add end-to-end and unit coverage for cross-run kept copies, module ownership, diagnostics, remedies, controls, and known limits.
  • Create an earlier_standard fixture modeling a dependency introduced in a later split change and an earlier apply that preserves stale project bytes.
  • Verify the next apply refuses before writing or creating a branch, reports the earlier-kept dependency, and that both documented remedies produce a working importer.
  • Cover unchanged, already-at-target, written, and earlier-kept module states, including mixed refusal lines and the legacy one-step message.
  • Retain controls proving compatible copies apply, untouched importers are not newly checked, and the documented earlier-kept-importer limitation remains.
 tests/test_update_shape_kept_helper.py

Assessment against linked issues

Issue Objective Addressed Explanation
#1082 Before writing files or creating a branch, refuse an apply when a copy it writes would import a name missing from the module copy as the run leaves it—including modules kept by this run, modules an earlier apply kept and now reads as unchanged, and upstream copies already at or written to the target. ✅
#1082 Preserve the existing one-step --accept-local refusal while reporting the source of the module bytes and providing remediation appropriate to earlier-kept copies and upstream copies. ✅
#1082 Keep the guard scoped to importers written by this run, while retaining the existing check for kept importers against modules this run writes. ✅
Tips and commands

Interacting with Sourcery

  • Trigger a new review: Comment @sourcery-ai review on the pull request.
  • Continue discussions: Reply directly to Sourcery's review comments.
  • Generate a GitHub issue from a review comment: Ask Sourcery to create an
    issue from a review comment by replying to it. You can also reply to a
    review comment with @sourcery-ai issue to create an issue from it.
  • Generate a pull request title: Write @sourcery-ai anywhere in the pull
    request title to generate a title at any time. You can also comment
    @sourcery-ai title on the pull request to (re-)generate the title at any time.
  • Generate a pull request summary: Write @sourcery-ai summary anywhere in
    the pull request body to generate a PR summary at any time exactly where you
    want it. You can also comment @sourcery-ai summary on the pull request to
    (re-)generate the summary at any time.
  • Generate reviewer's guide: Comment @sourcery-ai guide on the pull
    request to (re-)generate the reviewer's guide at any time.
  • Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
    pull request to resolve all Sourcery comments. Useful if you've already
    addressed all the comments and don't want to see them anymore.
  • Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
    request to dismiss all existing Sourcery reviews. Especially useful if you
    want to start fresh with a new review - don't forget to comment
    @sourcery-ai review to trigger a new review!

Customizing Your Experience

Access your dashboard to:

  • Enable or disable review features such as the Sourcery-generated pull request
    summary, the reviewer's guide, and others.
  • Change the review language.
  • Add, remove or edit custom review instructions.
  • Adjust other review settings.

Getting Help

@brettheap brettheap mentioned this pull request Oct 21, 2026
Draft
@brettheap

brettheap commented Oct 21, 2026

Copy link
Copy Markdown
Contributor Author

Independent review of #1133 (#1082) at aa7ddf59, for lane openRepoShape-5: READY

The change does what #1082's Verify section asks. Both two-step cases are now refused before anything is written, the one-step refusal prints exactly what main prints, and no valid run I tried is refused. There are two SHOULDs: a real gap the PR documents but that is still inside the issue's Goal, and one claim of unchanged words that no row pins. Neither blocks a draft.

Findings

SHOULD 1: the known limit is a real gap inside #1082's Goal, so it needs a follow-up issue before this lands.

  • Where: the docstring paragraph at update-shape.py:4261, and the call at update-shape.py:4315, which passes kept alone to _kept_copies_importing_dropped. The row that pins the limit is tests/test_update_shape_kept_helper.py:2109.
  • Measured scenario ($R/tools/knownlimit.sh, where $R is /tmp/claude-1000/-workspace-projects-openRepoShape/50cede29-ab82-4ca3-9f67-d98e1a58c8d2/scratchpad/v118/rv1133; logs in $R/limit-h/ and $R/limit-m/):
    1. A family holder is cut at 4ed7c7bf. Its own edit to siblings.py adds from bootstrap import told_past, which the upstream's siblings.py does not import. --help still exits 0.
    2. apply --at U1 --accept-local scripts/siblings.py exits 0. U1 is main plus an unrelated change.
    3. U2 then renames told_past in bootstrap.py only. check reads upstream-changed scripts/bootstrap.py and unchanged scripts/siblings.py.
    4. A plain apply --at U2 exits 0, copies bootstrap.py and commits. siblings.py --help then exits 1 with ImportError: cannot import name 'told_past'. Main's tool and the head's behave the same.
    5. The one-step run, --accept-local scripts/siblings.py, is refused by update-shape.py apply --accept-local: a kept .py copy that imports a name the run's new repo_shape.py dropped passes, and make bootstrap then dies on an ImportError #604 with (written) no longer defines told_past.
  • Why it is the Goal's case: this run's own write breaks a copy an earlier run kept. The docstring's reason ("an edit that already broke an untouched importer") does not cover it: the importer worked before the run.
  • Cheap to fix: I applied the sketch below in a scratch tree: 
    earlier = {row.path: row for row in rows
           if left.get(row.path) == EARLIER_MODULE}
... + _kept_copies_importing_dropped(root, written, {**kept, **earlier})
    • The new file then gave 1 failed and 178 passed. The one failure is the known-limit row itself.
    • The scenario above was then refused before any write, and the holder was unchanged.
    • _names_dropped already limits the refusal to names this run takes away, so this refuses no valid run.
    • The words still need work: the sketch prints (kept), and the opening for a copy this run's --accept-local keeps.
  • Ask: file that scenario as a follow-up issue, or widen the guard here.

SHOULD 2: nothing pins the opening of a (written)-only refusal.

NIT: the exits row does not check the row state the remedy promises.

  • Where: tests/test_update_shape_kept_helper.py:1955.
  • The remedy says the merged copy's row "then reads locally-modified". The row never checks that state, nor the drop exit's already-at-target.
  • By hand both are true. The merge exit gives locally-modified scripts/bootstrap.py. The drop exit gives already-at-target scripts/bootstrap.py, with kept ... (already the target's bytes; the row was stale).

NIT, the coordinator's #1111 check: there are none.

Checked and true

Could not verify

  • Shards A (2031 passed, 5 skipped) and B (1814 passed, 5 skipped). I only read the writer's logs; runs that size are not for a subagent.
  • The writer's M2, M4-M6 and M8-M10.
  • The writer's T1 shas, which depend on when they were made.
  • CI's git 2.55 and a Windows host.
  • S3776 counted by hand only; I sent no code to Sonar.

Checklist candidates

  • When a refusal gains a second opening, pin the old opening for every old kind of line, not just one.
  • When a guard is widened from "what this run's flag names" to "what earlier runs left", ask both directions, or file the direction left out at PR time.

Scratch left

  • Notes, probes and logs: /tmp/claude-1000/-workspace-projects-openRepoShape/50cede29-ab82-4ca3-9f67-d98e1a58c8d2/scratchpad/v118/rv1133/.
    • The local upstream clone is up0/.
    • Probe trees are probe-*, chain-* and limit-*.
    • Scripts are tools/probe1075.sh, tools/probe1023.sh, tools/chain.sh, tools/exits.sh, tools/knownlimit.sh, tools/scan_pairs.py, tools/mutate_r.py, tools/mutate_w.py and tools/mutants.sh.
  • Basetemps: /tmp/claude-1000/-workspace-projects-openRepoShape/50cede29-ab82-4ca3-9f67-d98e1a58c8d2/scratchpad/v110/bt/rv1133-*. The runs meant to fail (base243, mut, sketch, guard1111) kept their trees.
  • Removed: the worktrees rv1133-head, rv1133-merge and rv1133-mut, and the refs refs/l5rv/rv1133-pr, rv1133-merge and rv1133-pr1111. The predicted-merge commit e30a4324 is left as an unreferenced object.
  • Nothing of mine is running. I pushed, posted and filed nothing.

Lane: openreposhape-5 (openRepoShape-5)

@brettheap brettheap mentioned this pull request Oct 22, 2026
Merged
Brett Heap @claude
update-shape.py: update-accept-local-missing-name asks a copy an ea…
53791e4 
…rlier apply kept, read `unchanged`, as an importer too, so a plain apply that drops a name it imports is refused before any write, naming who kept it and giving it its exits (#1082)

Fix round 1 of #1133, after rv1133's review. #604 asked only a copy
this run's --accept-local keeps against a module this run writes, so
the head left a known limit inside #1082's Goal. Measured by the
reviewer offline: a holder whose `apply --accept-local
scripts/siblings.py` kept its own siblings.py, edited to take
`told_past` from bootstrap.py, read `unchanged` against a target whose
bootstrap.py renamed it. A plain `apply` exited 0, copied bootstrap.py,
committed, and `siblings.py --help` stopped on an ImportError. Main's
tool and the head's behaved the same.

The rows read `kept by an earlier run` as a module are now asked as
importers too, beside the kept ones. `_names_dropped` still limits the
refusal to names this run takes away, so a copy an earlier run kept
whose names all stay still applies. Its line reads `(written) no longer
defines X, which Y (kept by an earlier run) imports from it`; the
general opening now fits every kind of line ("a copy imports a name
from a copy beside it whose bytes, as this run leaves them, do not
define it, and this run writes one of the two"); and the remedy gives
that copy its two exits, `--accept-local` on it once changed, or the
upstream's bytes by `git show`. A refusal whose every line is one this
run's --accept-local makes still prints what main prints, and a #604
row now pins its opening. The code stays
`update-accept-local-missing-name`. The known-limit paragraph and the
row that pinned the limit are replaced by rows that hold the refusal,
its controls and its exits; the exits rows also check the state `check`
reads after each exit.

Lane: openreposhape-5 (openRepoShape-5)

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This was referenced Oct 22, 2026
Merged
Draft
@brettheap

brettheap commented Oct 22, 2026

Copy link
Copy Markdown
Contributor Author

Re-check of #1133 (#1082) for lane openRepoShape-5

Verdict: READY at 53791e45239d55d696d8f37291196d61c5f809b3. I re-measured fix round 1 against rv1133's review (#1133 issuecomment-6321333729): SHOULD 1, SHOULD 2 and the NIT are all met. The round broke nothing I could find. There are three NITs, and none blocks the draft.

Findings

NIT 1: nothing pins how a refusal in both directions at once opens.

  • Where: update-shape.py:4561 sets the opening: lead = KEPT_LEAD if sides <= {KEPT_MODULE, WRITTEN_MODULE} else LEFT_LEAD. The row test_both_directions_in_one_run_are_one_refusal (tests/test_update_shape_kept_helper.py:1432) checks the lines, the clause and the remedy, but not the opening.
  • Mutant O1: lead = (KEPT_LEAD if len(sides) == 1 and sides <= {KEPT_MODULE, WRITTEN_MODULE} else LEFT_LEAD). Result: 185 passed, it survives (mut-O1-both-directions-lead-general.log).
  • Under O1, a --accept-local run whose lines are (kept) lacks plus (written) no longer defines would open with the general words. Every line in that run is one this run's --accept-local makes. The body and the commit message say such a refusal "still prints what main prints".
  • By reading only, not measured: the head gives that case main's KEPT_LEAD. I did not run a byte comparison with main for the both-directions log, because the LAST STEP came first.
  • Ask: add assert result.stderr.startswith(KEPT_LEAD), result.stderr to that row.
  • Why NIT: this is the gap class of rv1133's SHOULD 2 and of its checklist candidate. But SHOULD 2's own ask, one update-shape.py apply --accept-local: a kept .py copy that imports a name the run's new repo_shape.py dropped passes, and make bootstrap then dies on an ImportError #604 row, is met, and rv1133's R2 is killed.

NIT 2: the Merges section predates #1166.

NIT 3: the title names only the first round's direction.

Checked and true

Each finding is met. I ran the reviewer's own scripts, copied into my folder, with the head's tool and with main's.

Rows fail where the body says. I ran the head's test file with the other tools in scratch clones:

  • With main's update-shape.py (ca47f90e): 17 failed, 168 passed (base-main.log).
  • With aa7ddf59's (6c8467ee): 13 failed, 172 passed (base-aa7.log).
  • The 5 new rows fail on both: ..._drops_its_name, either_exit_for_an_earlier_kept_importer (2) and ..._asked_against_a_written_module (2). On main they fail as assert 0 == 2 or None is not None. The failing rows match the body's table.

Mutants. Each was applied alone in a scratch clone, then restored by checkout with its sha256 checked (5b386f8b). The clone's status was clean at the end (mutants.out).

mutant result
N3 (body) killed, 3 failed
N8 (body) killed, 1 failed
N9 (body) killed, 1 failed
R2 (rv1133's) killed, 1 failed
O3, mine: the mixed again wording for (kept) modules only killed, 1 failed
O1, mine survived: NIT 1

N3, N8, N9 and R2 fail the same counts the body gives.

What the round could have broken. I found nothing.

  • Fresh roots: family and project roots cut at 4ed7c7bf and at v1.0.0 hold no row the head reads as kept by an earlier run. I wrapped the head's _copies_as_left and stopped before any write; every .py row reads the upstream's (fresh-*/left.log). So the new importer pass asks nothing of a root no run has kept a copy in.
  • Real history, with a copy kept by --accept-local and then plain applies:
    • Family: siblings.py kept, cut 9e154d87, then 00f6d7f7, c13c3985, ff0c72eb.
    • Project: bootstrap.py kept, cut af0e60f2, then d3e1e490.
    • Every apply exits 0, and every apply log is identical to main's tool once masked (chain-[hm]-*).
  • A copy an earlier run kept whose import has its own except ImportError fallback: the plain apply that drops the name still applies, and the copy starts (fallback-h2.out).
  • After the conflict refusals, every row left is unchanged, upstream-changed, already-at-target, merged-away, or an accepted both or locally-modified. No --accept-local row can also be an earlier-kept one, and every use of a side constant handles WRITTEN_UNDER_EARLIER.

Gates and runs, all through run9.sh at 53791e45 with dirty=0:

git files result log
2.43.0 the test file and test_path_guard_hygiene.py 234 passed (185 + 49) own243.log
2.47.1 (first on PATH, printed in the log) the test file 185 passed own247.log
2.43.0 _add, _add_ignored, _add_symlink, _branch_any_directory, _hardlink 136 passed, 3 skipped rest1.log
2.43.0 _header_comments, _next_quoting, _partial_rollback, _pr_base, _unreadable 202 passed rest2.log
  • The last two rows are the 10 test_update_shape*.py files the body says did not run at this head.
  • CI at this head (run 39789285738): tests, package, tests-git-bash and SonarCloud all passed. The quality gate passed with 0 new issues.

Merges and body claims.

  • merge-tree origin/main HEAD gives bfa8c1d4, which is HEAD's own tree. origin/main is still 038dafba.
  • The merge commit 7aee5efa has tree 27a2785e, the same as merge-tree aa7ddf59 038dafba. It leaves both PR files as aa7ddf59 had them.
  • The writer's logs say what the body says: own243/own247 185 each, runC 144, runD 163, runE 219, runF 13, each START line at head=53791e45 dirty=0.
  • The test file has 205 asserts, and none joins conditions with and or or (checked on the syntax tree).
  • The fix round adds no path guard and no non-ASCII byte, and its widest added line is 79 characters.
  • closingIssuesReferences is [1082].
  • The commit subject is 250 characters, and the commit carries the Lane and Co-Authored-By lines.
  • The shipped template comments that name the code (siblings.py:216-234, bootstrap.py:2290-2299) are still true.

Could not verify

  • The byte comparison with main for the both-directions refusal (NIT 1). The LAST STEP came first.
  • CI's Windows behaviour; S3776 complexity was not re-counted.

Scratch left

  • Notes, logs and probes: /tmp/claude-1000/-workspace-projects-openRepoShape/50cede29-ab82-4ca3-9f67-d98e1a58c8d2/scratchpad/v123/rc1133/ (STATE.md, tools/).
  • Clones there:
    • base-main (keeps main's update-shape.py on purpose)
    • base-aa7
    • mut (restored)
    • merge1166 (predicted merge c5a98365, made in that clone only)
  • Worktree: /workspace/projects/openRepoShape-worktrees/rc1133-recheck, detached at 53791e45, clean.
  • Nothing of mine is running. I pushed, posted and filed nothing.

Lane: openreposhape-5 (openRepoShape-5)

Brett Heap @claude
tests/test_update_shape_kept_helper.py: the both-directions row asser…
a7bfee1 
…ts the refusal opens with `KEPT_LEAD`, as a refusal of this run's own `--accept-local` always has, so an opening that falls back to the general words there fails it (#1082)

rc1133's NIT 1 on #1133: update-shape.py:4561 gives a refusal whose lines are
all ones this run's `--accept-local` makes the opening `KEPT_LEAD`, and
`test_both_directions_in_one_run_are_one_refusal` (a `(kept) lacks` line plus
a `(written) no longer defines` line) pinned its lines, its clause and its
remedy but not its opening. rc1133's mutant O1, `lead = (KEPT_LEAD if
len(sides) == 1 and sides <= {KEPT_MODULE, WRITTEN_MODULE} else LEFT_LEAD)`,
survived the 185 rows. The row now asserts `startswith(KEPT_LEAD)`, and its
docstring says why the opening is that one. Tests only; update-shape.py is
untouched.

Lane: openreposhape-5 (openRepoShape-5)
Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
@sonarqubecloud

sonarqubecloud Bot commented Oct 22, 2026

Copy link
Copy Markdown

Quality Gate Passed 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

@brettheap brettheap changed the title update-shape.py: update-accept-local-missing-name asks each copy an apply writes against its module as the run leaves it, so a copy an earlier apply kept, read unchanged, is asked too, and no siblings.py stopping on its import is copied (#1082) update-shape.py: update-accept-local-missing-name asks a copy an earlier apply kept, read unchanged, as a module a written copy imports from and as an importer of a written one, so a plain apply that stops either on its import is refused (#1082) Oct 22, 2026
@brettheap brettheap mentioned this pull request Oct 22, 2026
Merged
@brettheap

brettheap commented Oct 22, 2026

Copy link
Copy Markdown
Contributor Author

Re-check of #1133 (#1082) NIT round for lane openRepoShape-5

READY at a7bfee1bd (parent 53791e45). The commit changes tests/test_update_shape_kept_helper.py alone (+7, -4), so update-shape.py is 53791e45's (sha256 5b386f8b). All three of rc1133's NITs (issuecomment-6347273216) are met. I found nothing new.

Checked and true

NIT 1: the startswith(KEPT_LEAD) assert

  • rc1133's O1 is lead = (KEPT_LEAD if len(sides) == 1 and sides <= {KEPT_MODULE, WRITTEN_MODULE} else LEFT_LEAD). I applied it alone to the head's update-shape.py. The file was restored and sha-checked each time (5b386f8b9b8bfe13, status 0 lines).
Run, git 2.43.0 Result
O1 and the head's test file (whole file) killed: 1 failed, 184 passed. The failure is assert result.stderr.startswith(KEPT_LEAD) at line 1452. The opening read a copy imports a name from a copy beside it whose bytes, as this run leaves them, do not define it, ..., which is the body's quote.
O1 and 53791e45's test file (whole file) survives: 185 passed. So the new assert is what kills O1.
The head's row alone, with main's update-shape.py (961640c68, sha256 ca47f90e) swapped in 1 passed, as the body says. The assert pins an opening the head keeps as main prints it.
  • The head's own runs pass:
    • git 2.43.0: 234 passed (185 plus the 49 rows of tests/test_path_guard_hygiene.py);
    • git 2.47.1 (printed in the log): 185 passed.
  • Each START line reads head=a7bfee1b dirty=0.

NIT 3: the title is true

  • It is 249 characters and ASCII. The code update-accept-local-missing-name is real (update-shape.py:4197).
  • A copy an earlier apply kept, read unchanged, is asked in both roles:
    • as the module a written copy imports from (_written_copies_lacking, EARLIER_MODULE);
    • as an importer of a written module (_kept_copies_importing_dropped, WRITTEN_UNDER_EARLIER).
  • The rows test_an_earlier_kept_copy_is_asked_when_the_next_apply_writes_its_importer and ..._when_the_next_apply_drops_its_name hold both directions and pass on the head.
  • The body is right about rc1133's 242-character rewrite (which I measured at 242 characters). Its "break it" is true only where the kept copy is the importer. Where it is the module, the apply breaks the written importer.

NIT 2: the Merges row for #1166

Body and commit

  • The live body equals the writer's body-post.md.
  • body-guard exits 0 through the Lane line.
  • closingIssuesReferences is [1082].
  • The commit subject is 241 characters and ASCII. The commit carries the Lane and Co-Authored-By lines.
  • The SonarQube API for PR 1133 shows 0 open issues and the quality gate OK. It lists the PR under the older title, so that analysis may predate the NIT commit.

Not measured

Lane: openreposhape-5 (openRepoShape-5)

@brettheap brettheap mentioned this pull request Oct 23, 2026
Merged
@brettheap brettheap added this to the v1.2.1 milestone Oct 23, 2026
This was referenced Oct 23, 2026
Draft
Draft

This branch has not been deployed

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

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

v1.2.1

1 participant

@brettheap