fix: make the worktree conflict guard fail closed; unpin the installer tests - #226
Merged
Merged
Conversation
…r tests Two of the three defects in #225 (the PyAutoMind prompt_sync half ships separately). worktree_check_conflict FAILED OPEN. worktree_list_claimed returned 0 with empty output when active.md could not be resolved under $PYAUTO_MAIN, so the guard could not tell "nothing claims this repo" from "I could not read the registry" — and answered the former. start_dev step 6 and start_library step 1 both document this call and act on its answer, so any session whose roots are not at the default path got a green light it never earned, and two sessions could each be told the same repo was free. Reproduced during the #224 session: a conflict check was recorded as clean having read nothing. Now split into worktree_registry_path, so the listing can signal the failure in its exit code, and the guard reports CANNOT VERIFY with the paths it tried and returns 3. --allow-missing-registry proceeds unguarded and says so; it is never the default. Exit 1 still means a real conflict, 0 still means clear. test_missing_active_md_yields_no_claims previously asserted exactly the fail-open behaviour, so it is rewritten to the corrected contract with the reason recorded — it was pinning the defect. test_skill_install.py: two tests depended on the ambient checkout being NAMED PyAutoBrain and sitting one level under PYAUTO_ROOT, which defaults to bin/../.. — true on a laptop, false for a clone at any other path. A clone at `pyautobrain` never finds PyAutoBrain/skills, so intake is never installed and the asserted SKIP line never prints. They now pin PYAUTO_ROOT at a fixture root, the same pattern the passing sibling test already used, so they test the installer rather than the checkout layout. This is what made the ship_library fallback gate spuriously RED on #224 and cost a human acknowledgement to override. `pytest tests/` is now green in a cloud session: 331 passed, nothing ignored. Closes #225 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PY39MRi1oPbxhhr6tDToQU
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Two of the three defects in #225. The third (PyAutoMind
prompt_sync_push) ships in a separate PyAutoMind PR — see below.Branched from
mainand touching files disjoint from #224, so the two can land in either order.1.
worktree_check_conflictfailed open — the important oneworktree_list_claimedreturned0with empty output whenactive.mdcould not be resolved under$PYAUTO_MAIN:So the guard could not distinguish "nothing claims this repo" from "I could not read the registry" — and answered the former.
start_devstep 6 andstart_librarystep 1 both document this call and act on its answer, so any session whose roots are not at the default$HOME/Code/PyAutoLabsgot a green light it never earned, and two sessions could each be told the same repo was free. This is the guard that serialises parallel agents.Reproduced during the #224 session: a conflict check was recorded as clean having read nothing. Re-run with
PYAUTO_MAINset it worked and returned the same verdict — but the first answer was worth nothing, and had a conflict existed the session would have started conflicting work.Now split into
worktree_registry_pathso the listing can signal failure in its exit code, and the guard reportsCANNOT VERIFYwith the paths it tried and returns3:--allow-missing-registryproceeds unguarded and says so; it is never the default. Exit1still means a real conflict,0still means genuinely clear.Note for review:
test_missing_active_md_yields_no_claimsasserted exactly the fail-open behaviour. It is rewritten to the corrected contract with the reason recorded in its docstring — it was pinning the defect, so overriding it is the point rather than a casualty.Not a contradiction of the sibling policy:
worktree_claim_is_stalein the same file is documented "fail-open by design". It resolvesactive.mditself and never callsworktree_list_claimed, so it is untouched. Both functions fail toward safety — for staleness the safe answer is "don't remove", for conflicts it is "don't start".2.
test_skill_install.pyfailed locally, passed in CITwo tests depended on the ambient checkout being named
PyAutoBrainand sitting one level underPYAUTO_ROOT— which defaults tobin/../.., this repo's grandparent. True on a laptop; false for a clone at any other path. A clone atpyautobrainnever resolvesPyAutoBrain/skills, sointakeis never installed and the assertedSKIP intake (Codex skillline never prints.They now pin
PYAUTO_ROOTat a fixture root whosePyAutoBrain/symlinks to this checkout — the same pattern the already-passingtest_invalid_codex_name_does_not_suppress_claude_surfacesused. They test the installer's behaviour instead of the checkout layout. This does not narrow coverage: PyAutoBrain CI only ever had PyAutoBrain's own skills on disk, so the fixture matches what CI already scanned, minus the accidental dependency on directory naming.This is what made the
ship_libraryfallback gate spuriously RED on #224 and cost a human acknowledgement to override.Test Plan
pytest tests/— 331 passed, nothing ignored, green in a cloud session for the first time3;--allow-missing-registry→0(loud); repo claimed by another task →1; repo free →0; same task resuming →0tests/test_skill_install.py7 passedbin/check_skill_line_counts.sh— within budgetNot in this PR
The third defect, PyAutoMind
scripts/prompt_sync.shhardcodinggit push origin main, is a PyAutoMind change and ships as PyAutoMind#178 with its own tests — 7 new, driving the real script against throwaway repos with real bare remotes, of which 5 fail against the pre-fix script.A later audit corrected that defect's description:
git push origin mainpushes the localmainref, never the commit just made on a branch. So when localmainmatches the remote (the normal case) the push is a no-op that exits 0 and the work never leaves the machine; only when localmainis ahead does it publish unreviewed commits. Both halves are the same root cause and both are fixed by pushingHEAD. See the correction comment on #225.#225 tracks all three and stays open until both land.
API Changes
worktree_check_conflictgains a leading optional--allow-missing-registryflag and a new exit code3(registry unresolvable). Exits0and1are unchanged.worktree_list_claimednow returns non-zero when the registry cannot be resolved; it previously returned0. Onlyworktree_check_conflictconsumes it programmatically; other callers invoke it for display and now get a non-zero code alongside the empty output rather than silence.worktree_registry_path.Generated by the PyAutoLabs agent workflow.