Skip to content

fix: make the worktree conflict guard fail closed; unpin the installer tests - #226

Merged
Jammy2211 merged 1 commit into
mainfrom
feature/dev-workflow-helpers-laptop-paths
Aug 10, 2026
Merged

fix: make the worktree conflict guard fail closed; unpin the installer tests#226
Jammy2211 merged 1 commit into
mainfrom
feature/dev-workflow-helpers-laptop-paths

Conversation

@Jammy2211

@Jammy2211 Jammy2211 commented Aug 10, 2026

Copy link
Copy Markdown
Contributor

Summary

Two of the three defects in #225. The third (PyAutoMind prompt_sync_push) ships in a separate PyAutoMind PR — see below.

Branched from main and touching files disjoint from #224, so the two can land in either order.

1. worktree_check_conflict failed open — the important one

worktree_list_claimed returned 0 with empty output when active.md could not be resolved under $PYAUTO_MAIN:

if [[ ! -f "$active" ]]; then
  return 0
fi

So the guard could not distinguish "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 $HOME/Code/PyAutoLabs got 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_MAIN set 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_path so the listing can signal failure in its exit code, and the guard reports CANNOT VERIFY with the paths it tried and returns 3:

worktree_check_conflict: CANNOT VERIFY — no active.md found.
  tried: /nonexistent/PyAutoMind/active.md
         /nonexistent/PyAutoPrompt/active.md
  PYAUTO_MAIN=/nonexistent
  Set PYAUTO_MAIN to the directory holding your PyAutoMind checkout,
  or pass --allow-missing-registry to proceed without the guard.

--allow-missing-registry proceeds unguarded and says so; it is never the default. Exit 1 still means a real conflict, 0 still means genuinely clear.

Note for review: test_missing_active_md_yields_no_claims asserted 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_stale in the same file is documented "fail-open by design". It resolves active.md itself and never calls worktree_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.py failed locally, passed in CI

Two tests depended on the ambient checkout being named PyAutoBrain and sitting one level under PYAUTO_ROOT — which defaults to bin/../.., this repo's grandparent. True on a laptop; false for a clone at any other path. A clone at pyautobrain never resolves PyAutoBrain/skills, so intake is never installed and the asserted SKIP intake (Codex skill line never prints.

They now pin PYAUTO_ROOT at a fixture root whose PyAutoBrain/ symlinks to this checkout — the same pattern the already-passing test_invalid_codex_name_does_not_suppress_claude_surfaces used. 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_library fallback 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 time
  • Guard, all five paths exercised: missing registry → 3; --allow-missing-registry0 (loud); repo claimed by another task → 1; repo free → 0; same task resuming → 0
  • The two previously-failing installer tests now pass — tests/test_skill_install.py 7 passed
  • bin/check_skill_line_counts.sh — within budget

Not in this PR

The third defect, PyAutoMind scripts/prompt_sync.sh hardcoding git 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 main pushes the local main ref, never the commit just made on a branch. So when local main matches the remote (the normal case) the push is a no-op that exits 0 and the work never leaves the machine; only when local main is ahead does it publish unreviewed commits. Both halves are the same root cause and both are fixed by pushing HEAD. See the correction comment on #225.

#225 tracks all three and stays open until both land.

API Changes

  • worktree_check_conflict gains a leading optional --allow-missing-registry flag and a new exit code 3 (registry unresolvable). Exits 0 and 1 are unchanged.
  • worktree_list_claimed now returns non-zero when the registry cannot be resolved; it previously returned 0. Only worktree_check_conflict consumes it programmatically; other callers invoke it for display and now get a non-zero code alongside the empty output rather than silence.
  • New helper worktree_registry_path.

Generated by the PyAutoLabs agent workflow.

…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
@Jammy2211
Jammy2211 merged commit 9a43f00 into main Aug 10, 2026
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants