Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
48 changes: 43 additions & 5 deletions bin/worktree.sh
Original file line number Diff line number Diff line change
Expand Up @@ -306,13 +306,23 @@ worktree_remove() {
# Used by conflict-guard steps. A task is "claimed" if its entry has a
# `worktree:` field; legacy non-worktree tasks are emitted with worktree-path
# set to the literal string "-".
worktree_list_claimed() {
# worktree_registry_path
# Echoes the resolved PyAutoMind/active.md, or returns 3 (printing nothing) when
# it cannot be found. Split out so a caller can tell "nothing is claimed" apart
# from "I could not read the registry" — see worktree_check_conflict.
worktree_registry_path() {
local active="$PYAUTO_MAIN/PyAutoMind/active.md"
# Back-compat: fall back to the pre-rename PyAutoPrompt/ path if present.
[[ -f "$active" ]] || active="$PYAUTO_MAIN/PyAutoPrompt/active.md"
if [[ ! -f "$active" ]]; then
return 0
fi
[[ -f "$active" ]] || return 3
printf '%s\n' "$active"
}

worktree_list_claimed() {
local active
# Returns 3, NOT 0, when the registry is unreadable — empty output must never
# be mistaken for "nothing is claimed".
active="$(worktree_registry_path)" || return 3
awk '
# Claim rows are buffered and flushed at the end of each task entry:
# `worktree:` may appear either side of the `repos:` block, so it is not
Expand Down Expand Up @@ -363,13 +373,41 @@ worktree_list_claimed() {
' "$active"
}

# worktree_check_conflict <task-name> <repo1> [repo2 ...]
# worktree_check_conflict [--allow-missing-registry] <task-name> <repo1> [repo2 ...]
# Exits 0 if none of the requested repos are claimed by a different task.
# Exits 1 and prints the conflicts to stderr otherwise.
# Exits 3 when the registry cannot be resolved — see below.
#
# THIS GUARD FAILS CLOSED. It used to return 0 when `active.md` could not be
# found, which meant a cloud/web/CI session (where the roots are not under the
# default $HOME/Code/PyAutoLabs) got "no conflict" from a guard that had read
# nothing. That is worse than no guard, because the workflow documents this
# call and the skills act on its answer: two sessions could each be told the
# same repo was free. It now reports the failure and returns non-zero, so the
# caller stops instead of proceeding on a green light it never earned.
worktree_check_conflict() {
local allow_missing=0
if [[ "${1:-}" == "--allow-missing-registry" ]]; then
allow_missing=1
shift
fi
local task="$1"
shift
local want repo existing_task existing_repo existing_branch existing_wt rc=0

if ! worktree_registry_path >/dev/null; then
if (( allow_missing )); then
echo "worktree_check_conflict: no registry under \$PYAUTO_MAIN — proceeding UNGUARDED (--allow-missing-registry)" >&2
return 0
fi
echo "worktree_check_conflict: CANNOT VERIFY — no active.md found." >&2
echo " tried: $PYAUTO_MAIN/PyAutoMind/active.md" >&2
echo " $PYAUTO_MAIN/PyAutoPrompt/active.md" >&2
echo " PYAUTO_MAIN=${PYAUTO_MAIN:-<unset>}" >&2
echo " Set PYAUTO_MAIN to the directory holding your PyAutoMind checkout," >&2
echo " or pass --allow-missing-registry to proceed without the guard." >&2
return 3
fi
for want in "$@"; do
while IFS=$'\t' read -r existing_task existing_repo existing_branch existing_wt; do
if [[ "$existing_repo" == "$want" && "$existing_task" != "$task" ]]; then
Expand Down
7 changes: 7 additions & 0 deletions skills/WORKFLOW.md
Original file line number Diff line number Diff line change
Expand Up @@ -223,6 +223,13 @@ Task worktrees keep parallel work isolated (`PyAutoBrain/bin/worktree.sh`):
**dev workflow's own git mechanics** — feature-development work, **not** Build.
Build is reached only for the release/packaging step (PyPI/tags/notebooks).

`worktree_check_conflict` **fails closed**: it reads `active.md` under
`$PYAUTO_MAIN` (default `$HOME/Code/PyAutoLabs`) and exits `3` with
`CANNOT VERIFY` when that registry cannot be resolved, instead of reporting
"no conflict" from a read that never happened (#225). In `web-github` / `ci-only`
environments set `PYAUTO_MAIN` to the directory holding the PyAutoMind checkout,
or pass `--allow-missing-registry` to proceed knowingly unguarded.

## Repo → GitHub owner mapping

<!-- repos_sync:begin -->
Expand Down
2 changes: 2 additions & 0 deletions skills/start_dev/reference.md
Original file line number Diff line number Diff line change
Expand Up @@ -105,6 +105,8 @@ PyAutoMind. Shared organ boundary and the execution-environment model are in

This prints one line per `(task, repo, branch, worktree_path)` quadruple currently registered in `active.md`. For each affected repo the new plan wants to touch, check whether a different task already claims it via a `worktree:` field. If so, flag it as a **hard conflict** — the new task cannot start until the other one ships.

**The guard fails closed (#225).** `worktree_check_conflict` resolves `active.md` under `$PYAUTO_MAIN` (default `$HOME/Code/PyAutoLabs`). When it cannot find the registry it exits **3** with `CANNOT VERIFY`, rather than reporting "no conflict" from a read that never happened — outside local-dev, set `PYAUTO_MAIN` to the directory holding your PyAutoMind checkout. `--allow-missing-registry` proceeds **unguarded** and says so; use it only when you have confirmed by other means that nothing else claims the repos. Exit `1` is a real conflict; `0` is genuinely clear.

Then, for each affected repo, also run:
```bash
git -C <repo_path> branch --sort=-committerdate | head -5
Expand Down
23 changes: 23 additions & 0 deletions tests/test_skill_install.py
Original file line number Diff line number Diff line change
Expand Up @@ -45,11 +45,33 @@ def test_local_skill_links_resolve():
assert broken == []


def _pyauto_root(tmp_path):
"""A fixture PYAUTO_ROOT whose `PyAutoBrain/` is this checkout.

Without this the installer falls back to `DEFAULT_PYAUTO_ROOT`
(`bin/../..`, i.e. this repo's grandparent) and looks for
`<grandparent>/PyAutoBrain/skills`. That resolves only when the checkout
happens to be *named* `PyAutoBrain` and sits one level under the root —
true on a laptop, false for a clone at any other path (a cloud session
cloning to `pyautobrain` finds nothing, so the Brain skills are never
scanned and the assertions below have nothing to assert on).

Pinning the root makes these tests depend on the installer's behaviour
rather than on where the repo happens to be checked out. Same pattern as
`test_invalid_codex_name_does_not_suppress_claude_surfaces` below.
"""
root = tmp_path / "PyAutoLabs"
root.mkdir(parents=True, exist_ok=True)
(root / "PyAutoBrain").symlink_to(BRAIN_HOME, target_is_directory=True)
return root


def test_installer_keeps_commands_and_installs_both_skill_homes(tmp_path):
claude_home = tmp_path / "claude"
codex_home = tmp_path / "codex"
env = os.environ | {
"HOME": str(tmp_path / "home"),
"PYAUTO_ROOT": str(_pyauto_root(tmp_path)),
"CLAUDE_HOME": str(claude_home),
"CODEX_HOME": str(codex_home),
}
Expand Down Expand Up @@ -85,6 +107,7 @@ def test_installer_preserves_non_symlink_destinations(tmp_path):
marker.write_text("keep\n")
env = os.environ | {
"HOME": str(tmp_path / "home"),
"PYAUTO_ROOT": str(_pyauto_root(tmp_path)),
"CLAUDE_HOME": str(claude_home),
"CODEX_HOME": str(codex_home),
}
Expand Down
34 changes: 32 additions & 2 deletions tests/test_worktree_conflict_guard.py
Original file line number Diff line number Diff line change
Expand Up @@ -200,7 +200,37 @@ def test_entry_claiming_no_repos_yields_no_claims(tmp_path):
assert _claims(tmp_path, NO_CLAIMS) == []


def test_missing_active_md_yields_no_claims(tmp_path):
def test_missing_active_md_reports_failure_rather_than_no_claims(tmp_path):
"""A missing registry is NOT the same as "nothing is claimed".

This test previously asserted the opposite — `returncode == 0` with empty
output — and that pinned behaviour turned out to BE the defect (#225).
`worktree_check_conflict` consumes this function, so returning 0 meant a
session whose roots are not under `$PYAUTO_MAIN` got "no conflict" from a
guard that had read nothing. Empty output must never be mistaken for a
clean registry, so the listing now signals the failure in its exit code.
"""
proc = _run(tmp_path, None, "worktree_list_claimed")
assert proc.returncode != 0
assert proc.stdout == "" # still emits no bogus rows


def test_conflict_guard_fails_closed_when_the_registry_is_missing(tmp_path):
"""The defect, at the level the skills actually call.

Documented in `start_dev` step 6 and `start_library` step 1, and its answer
decides whether a task may start — so "I could not check" has to stop the
caller, not wave it through.
"""
proc = _run(tmp_path, None, "worktree_check_conflict some-task PyAutoFit")
assert proc.returncode != 0
assert "CANNOT VERIFY" in proc.stderr
assert "PYAUTO_MAIN" in proc.stderr # names what to set


def test_conflict_guard_can_be_forced_past_a_missing_registry(tmp_path):
"""The escape hatch is explicit and loud — never the default."""
proc = _run(tmp_path, None,
"worktree_check_conflict --allow-missing-registry t PyAutoFit")
assert proc.returncode == 0
assert proc.stdout == ""
assert "UNGUARDED" in proc.stderr
Loading