Repository navigation
adopt-project.py: execute refuses every finding check reports, so a plan with an uncovered path, a bad leg:, a path covered twice, a duplicated path or an entry that covers nothing is refused before the first leg repository exists, and the corrected plan no longer meets leg-remote-exists (#168) - #170
Conversation
… a plan with an uncovered path, a bad `leg:`, a path covered twice, a duplicated path or an entry that covers nothing is refused before the first leg repository exists, and the corrected plan no longer meets `leg-remote-exists` (#168) `_refuse_an_unrunnable_plan` refused only `plan-unresolved` and `plan-stale`. A plan `check` rejected for any other reason ran on into `_create_leg_remotes`, created and pushed BOTH legs, and failed only at the verification as `adopt-lost` -- or, for an ordinary uncovered file, exited 0 with the file left in the root. Either way the corrected plan then met `leg-remote-exists`, which has no `--force`. `_refuse_what_check_finds` runs the two functions `check` reads, `_coverage_findings` and `_leg_findings`, over the same two lists and raises ONE `Refusal`: its code is the first finding's own, its detail is the findings as `check` prints them (capped at eight, with an "and N more" tail), and its remediation names the exit. `cmd_check` and `_refuse_an_unrunnable_plan` are not touched, so this merges with #166; once both are in, those two and this function can read one shared list. The call is ONE added line in `cmd_execute`, and it sits after `_leg_paths` rather than directly after `_refuse_an_unrunnable_plan`: placed there it turned a plan path that is a `git` option from `unsafe-value` into `plan-uncovered`, which fails the existing `test_a_plan_path_that_is_a_git_option_is_refused`. It is still before `_confirm` and `_create_leg_remotes`, so nothing is created. The refusals that already existed keep their codes and their order, and `--allow-empty-leg` seeding is unaffected. tests/test_adopt_execute_refusals.py covers each class, the order, the cap and a plan `check` passes, through `cmd_execute` in-process with no `git filter-repo`, and once end to end: the refused run leaves no `--local-remote-dir`, and the corrected plan then adopts. Lane: openreposhape-1 (openRepoShape-1) Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
…main really did with each plan `check` rejects (verified and exited 0, died at `git rm`, or `adopt-lost`) and claim only that no leg repository was created, and a test now pins the refusal ahead of the question to confirm (#168) The review of 95dbabd found no blocker and these corrections to what the first commit says and tests. The code path is unchanged: `_refuse_what_check_finds` is the same function and the same one call in `cmd_execute`; only the wording of one refusal's remediation changes. What main at 7f84ca4 did with a plan `check` rejects was reproduced, not assumed, and is now what the function's docstring and the test file's header say: `plan-uncovered` and `plan-bad-leg` on a source with no submodule VERIFIED and exited 0 with the paths left in the assembly root; `plan-covered-twice`, `plan-duplicate-path` and `plan-empty-entry` died at `git rm` after both legs were pushed; #168's three submodule cases failed the verification as `adopt-lost`. The first commit said all five ended as `adopt-lost`. The same three submodule cases are refused on this branch alone under `plan-uncovered` and `plan-bad-leg`. No "nothing was created" overclaim: `_work_root` has already made the work directory and an `org/repo` source has already been cloned into it, so the module docstring, the function's docstring and the remediation now say that no leg repository was created and nothing was pushed, and the test helper is renamed `no_leg_was_created`. `_leg_paths` checks only the entries whose `leg:` is one of the four words, and the docstring now says so. `leg-remote-exists` is named only where it exists, under `--local-remote-dir`. `execute` is said to have refused `plan-unresolved`, `plan-stale` and the naming findings, not only the first two. tests/test_adopt_execute_refusals.py: a run WITHOUT `--yes` on a stream that is not a terminal is refused under the finding's own code, in-process for each of the five classes and end to end for the uncovered plan (the refused half has no `--yes`), so moving the call past `_confirm` fails 6 tests where it failed none; the comment that said the suite imports no sibling test module, which it does, is replaced by the true one; the header no longer claims the tests fail "the same way" with and without the tool; the shared literals are constants, the fixture is a small dataclass instead of a dict read by key, no line is over 79 columns, and the misnamed `seeded` is `opened`. Run: tests/test_adopt_execute_refusals.py, test_adopt_e2e.py, test_adopt_plan.py, test_adopt_spec_only.py, test_windows_paths.py, test_cli_reference.py and test_repo_hygiene.py: 217 passed, 1 skipped (the MedxEHR clone). Lane: openreposhape-1 (openRepoShape-1) Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
|
(lane openRepoShape-1, 2026-10-09T02:02:14Z) |
|
Sorry @brettheap, you've used your own review budget of 250,000 diff characters for the last 7 days. You can request another review in 18 hours and 1 minute by commenting |
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
Reviewer's Guide
Sequence diagram for execute refusal before leg creationsequenceDiagram
participant User
participant Execute as cmd_execute
participant PlanCheck as _refuse_an_unrunnable_plan
participant Paths as _leg_paths
participant Findings as _refuse_what_check_finds
participant Remotes as _create_leg_remotes
participant Verify as adoption verification
User->>Execute: execute(plan, source)
Execute->>PlanCheck: _refuse_an_unrunnable_plan(plan, source)
PlanCheck-->>Execute: pass or Refusal(plan-unresolved/plan-stale)
Execute->>Paths: _leg_paths(plan)
Paths-->>Execute: paths_for or Refusal(unsafe-value)
Execute->>Findings: _refuse_what_check_finds(plan, source)
alt check findings exist
Findings-->>Execute: Refusal(first finding code)
Execute-->>User: exit 2, no leg repository or push
else plan is clean
Findings-->>Execute: pass
Execute->>Remotes: _create_leg_remotes(...)
Remotes->>Verify: create and push legs
Verify-->>User: adoption verified
end
Flow diagram for execute plan safety gateflowchart TD
A[cmd_execute] --> B[_refuse_an_unrunnable_plan]
B -->|plan-unresolved or plan-stale| R1[Refusal with existing priority code]
B --> C[_leg_paths]
C -->|unsafe git option| R2[Refusal unsafe-value]
C --> D[_refuse_what_check_finds]
D -->|any check finding| R3[Refusal with first finding code]
D -->|no findings| E[_refuse_unconsented_seeding and _confirm]
E --> F[_create_leg_remotes]
F --> G[Verify adoption]
R1 --> H[Exit 2; no leg repository or push]
R2 --> H
R3 --> H
File-Level Changes
Assessment against linked issues
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
Lane: openreposhape-1 (openRepoShape-1) Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
(lane openRepoShape-2, 2026-10-09T02:41:02Z) ADVISORY REVIEW of Verdict: MERGEABLE WITH NITS; no P1, no P2; three P3s open at b18cca9. Four P3s found at 95dbabd were fixed by a25dcd7 (the account of main's behaviour, the "no repository was created" overclaim, the P3-1. #168's "one list both commands read" is deferred ( P3-2. No test builds a source with a submodule ( P3-3 (nit). The "N finding(s)" and "and N more" counts count LINES, including Unverified: the PR's full-suite count, its |
Lane: openreposhape-1 (openRepoShape-1)
Claim: #168 (comment)
What
adopt-project.py executenow refuses every findingcheckreports, before the first leg repository is created. The five classes it did not refuse (plan-uncovered,plan-bad-leg,plan-covered-twice,plan-duplicate-path,plan-empty-entry) are nowREFUSED <that code>with exit 2, no leg repository created and nothing pushed.One new function,
_refuse_what_check_finds(plan, source), and ONE added line incmd_execute.cmd_checkand_refuse_an_unrunnable_planare not edited (see "Relation to #166").Why (reproduced on main, 7f84ca4)
_refuse_an_unrunnable_planrefuses onlyplan-unresolvedandplan-stale(andcmd_executerefuses the naming findings first). Any other plancheckrejects runs on into_create_leg_remotes, which makes both legs. What happens next depends on the class, and each was run on main:plan-uncoveredandplan-bad-leg, on a source with no submodule: the run VERIFIES and exits 0, with the uncovered paths, or the paths whoseleg:is not one of the four words, left in the assembly root. (With thesrc/entry deleted,checksaysplan-uncoveredfor both files andexecuteexits 0 with both legs pushed andsrc/still in the root.)plan-covered-twice,plan-duplicate-pathandplan-empty-entry: the run dies atgit rmof the offending entry, after both legs were pushed (REFUSED a git or gh command failed, exit 2).adopt-lost.In every one the legs exist afterwards, so the corrected plan cannot make them again: under
--local-remote-dirit meetsREFUSED leg-remote-exists(there is no--force), and without itgh repo createis asked for names the first run already took.On this branch alone, the same three submodule cases:
AGENTS.md step 4 has a human say yes after
check, butexecuteis the one gate that cannot be skipped.The fix
_refuse_what_check_findsruns_coverage_findingsand_leg_findingsover the same two listscheckbuilds (the entry paths, the source tree's paths) and raises ONERefusal:plan-uncovered, ...);checkprints them, capped at eight (REFUSED_FINDINGS_SHOWN) with an "and N more" tail, the wayplan-unresolvedcaps its paths;checkuntilplan ok; runexecuteagain.Order, which is part of the fix and is pinned by tests:
_refuse_an_unrunnable_plan, soplan-unresolved(which_leg_findingswould repeat) andplan-stalekeep their codes and their wording;_leg_pathshas checked, as a safegitargument, the path of every entry whoseleg:is one of the four words. The brief put the call directly after_refuse_an_unrunnable_plan; there, a plan path that is agitoption (--output=...) isplan-uncovered, notunsafe-value, and the existingtest_a_plan_path_that_is_a_git_option_is_refusedfails. The one line is therefore afterpaths_for = _leg_paths(plan). (An entry with any otherleg:is not checked there and reaches nogitcommand; it is refused by the new function under whichever findingchecklists first.) Nothing between the two creates a leg;_refuse_unconsented_seeding,_confirmand_create_leg_remotes. No leg repository is created and nothing is pushed until every refusal has passed. (_work_roothas already made the work directory and anorg/reposource has already been cloned into it; that was so before this PR, and neither is a leg.)A plan
checkpasses executes exactly as before (acheck-clean plan, and a spec-only plan with--allow-empty-leg code, give identical output, refs and files on main and on this branch), and a leg no entry assigns a path to is still acheckWARNING and anexecuteconsent (--allow-empty-leg), not a finding. The module docstring's sentence about whatexecuterefuses is updated.Tests
tests/test_adopt_execute_refusals.py, 30 tests. All but one run withoutgit filter-repo: they loadadopt-project.pythrough importlib and callcmd_executein-process with its filter-repo probe stubbed, which is safe because every refusal under test is raised before the tool would be used. The file builds a source with no submodule.Refusalwith that code, the finding in the detail, and everyFINDINGlinecheckprints for the same plan present word for word in the refusal;cmd_execute: that code, no--local-remote-dir, no leg clone in the work directory, nothing pushed to the source;cmd_executeWITHOUT--yes, on a stream that is not a terminal: still the finding's own code, notadopt-unconfirmed;checkpasses raises nothing; a spec-only plan with--allow-empty-leg coderaises nothing, and without consent is stilladopt-empty-leg-unconsented;plan-unresolved, a stale plan that is also uncovered isplan-stale, an option-shaped path in an uncovered plan isunsafe-value;git filter-reporequired):executeon an uncovered plan, run without--yes, exits 2 withREFUSED plan-uncovered,--local-remote-diris never created, no leg clone exists, and the corrected plan then runs toadoption verifiedinto the same directory instead of meetingleg-remote-exists.What each regression fails, measured on this branch's file (30 tests) against a sandbox copy of the code:
cmd_execute/ the functioncmd_execute, the five without--yes, the end to end)adopt-project.py)_confirm(still before_create_leg_remotes)--yes, the end to end); the 25-test file at 95dbabd failed none_refuse_an_unrunnable_plantest_adopt_e2e.py::test_a_plan_path_that_is_a_git_option_is_refused_refuse_an_unrunnable_planFull suite at 95dbabd: 1264 passed, 1 skipped (the MedxEHR clone). The follow-up commit (a25dcd7) changes docstrings, comments, the wording of the one remediation message and the tests, not the code path; on it,
tests/test_adopt_execute_refusals.py tests/test_adopt_e2e.py tests/test_adopt_plan.py tests/test_adopt_spec_only.py tests/test_windows_paths.py tests/test_cli_reference.py tests/test_repo_hygiene.py: 217 passed, 1 skipped withgit-filter-repoon PATH, andtests/test_adopt_execute_refusals.pyalone 29 passed, 1 skipped (the end to end) without it.Relation to #166 and the merge order
Merge order: before #165, which must not land without it. It landed before #166 because nothing orders the two: each only adds refusals, and
git merge-treeof a25dcd7 with #166's head is clean. #165's narrowed_verifywould otherwise verify a plancheckrejects, and this PR is what stopsexecutefrom reaching verification with one.#166 adds
submodule_plan_problemstocmd_checkand to_refuse_an_unrunnable_plan. This PR edits neither, so the two do not conflict (git merge-treeof this branch at a25dcd7 with the #166 branch at 09d7889 is clean, andtest_adopt_execute_refusals.pyplustest_adopt_submodules.pypass on that merged tree: 82 passed). Once both are merged,check,_refuse_an_unrunnable_planand_refuse_what_check_findscan read ONE shared list of findings (the coverage and leg findings and #166's submodule problems); that unification is deliberately left until after both merge, so the two branches do not edit the same functions. The function's docstring says the same.Closes #168
🤖 Generated with Claude Code
Summary by Sourcery
Refuse all plans rejected by
checkbefore any leg repository is created or pushed.New Features:
adopt-project.py executerefuse every plan finding reported bycheckbefore creating or pushing any leg repositories.Bug Fixes:
leg-remote-exists.Enhancements:
checkfindings with remediation guidance.Documentation:
Tests: