diff --git a/bin/worktree.sh b/bin/worktree.sh index 2a3b3e4..04a442c 100644 --- a/bin/worktree.sh +++ b/bin/worktree.sh @@ -314,7 +314,17 @@ worktree_list_claimed() { return 0 fi 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 + # known when a repo bullet is read. + function flush( i) { + for (i = 1; i <= n; i++) { + printf "%s\t%s\t%s\t%s\n", task, repos[i], branches[i], wt + } + n = 0 + } /^## / { + flush() task = $2 wt = "-" next @@ -324,13 +334,32 @@ worktree_list_claimed() { next } /^ - [A-Za-z]/ { - # Matches lines like " - PyAutoFit: feature/foo" - gsub(/^ - /, "") - split($0, parts, ": ") - repo = parts[1] - branch = parts[2] - printf "%s\t%s\t%s\t%s\n", task, repo, branch, wt + # Two schemas in the wild, both accepted: + # " - autolens_workspace (feature/foo)" what every skill writes + # " - PyAutoFit: feature/foo" the documented legacy form + # The branch is informational (it appears only in the conflict message + # and repo_cleanup CLAIMED pairs); the bare repo name is what the guard + # compares, so it must survive either shape. + line = $0 + sub(/^ - /, "", line) + branch = "" + if (match(line, /: /)) { + repo = substr(line, 1, RSTART - 1) + branch = substr(line, RSTART + 2) + } else if (match(line, / *\(/)) { + repo = substr(line, 1, RSTART - 1) + branch = substr(line, RSTART) + sub(/^ *\(/, "", branch) + sub(/\)[^)]*$/, "", branch) + } else { + repo = line + } + sub(/[ \t]+$/, "", repo) + n++ + repos[n] = repo + branches[n] = branch } + END { flush() } ' "$active" } diff --git a/skills/start_library/start_library.md b/skills/start_library/start_library.md index 56daa7b..a29ec2b 100644 --- a/skills/start_library/start_library.md +++ b/skills/start_library/start_library.md @@ -84,8 +84,11 @@ Update the task entry to record the worktree path and claimed repos: - PyAutoArray: feature/ ``` -The `worktree:` field is what `worktree_check_conflict` reads to detect -collisions from other sessions. Then: +The ` - ` bullets under `repos:` are the claim — they are what +`worktree_check_conflict` compares to detect collisions from other sessions +(`worktree:` is reported alongside, but claims nothing on its own). Both +` - : ` and ` - ()` parse; the branch is +informational and may be omitted. Then: ```bash source PyAutoMind/scripts/prompt_sync.sh diff --git a/skills/start_workspace/reference.md b/skills/start_workspace/reference.md index 00c1a34..bde79c0 100644 --- a/skills/start_workspace/reference.md +++ b/skills/start_workspace/reference.md @@ -76,6 +76,10 @@ the workspace repos, set `status: workspace-dev`, add `library-pr:`: - autofit_workspace: feature/ ``` +The ` - ` bullets are the claim `worktree_check_conflict` reads. Both +` - : ` and ` - ()` parse; the branch is +informational and may be omitted. + Standalone mode — same shape without `library-pr:` and with only workspace repos. Push with `prompt_sync_push "prompt: register workspace repos in active.md"`. diff --git a/tests/test_worktree_conflict_guard.py b/tests/test_worktree_conflict_guard.py new file mode 100644 index 0000000..2fb4587 --- /dev/null +++ b/tests/test_worktree_conflict_guard.py @@ -0,0 +1,206 @@ +"""tests/test_worktree_conflict_guard.py — the active.md claim parser. + +`worktree_check_conflict` is the start_dev step-6 guard that decides whether a +new task registers in `active.md` (can start) or `planned.md` (blocked). It +reads `worktree_list_claimed`, whose awk was written for the schema the +start_library/start_workspace references still document: + + - repos: + - PyAutoFit: feature/foo + +Every writer drifted to the paren form instead: + + - repos: + - autolens_workspace (feature/foo) + +which the `": "` split swallowed whole, so `repo` never equalled the requested +repo name and the guard exited 0 for every task. The parser now accepts both +forms; these tests pin that, plus the field-order independence of `worktree:`. + +Drives the real bash functions against a temp PYAUTO_MAIN fixture, on the +idiom of test_worktree_claim_guard.py. +""" + +from __future__ import annotations + +import subprocess +from pathlib import Path + +WORKTREE_SH = Path(__file__).resolve().parents[1] / "bin" / "worktree.sh" + +PAREN = """# Active Tasks + +## paren-task +- issue: http://x +- worktree: ~/wt/paren-task +- repos: + - autolens_workspace (feature/paren-task) +""" + +COLON = """# Active Tasks + +## colon-task +- issue: http://y +- worktree: ~/wt/colon-task +- repos: + - PyAutoFit: feature/colon-task +""" + +# `repos:` before `worktree:` — both orderings occur in the real ledger. +REPOS_FIRST = """# Active Tasks + +## rbw-task +- repos: + - PyAutoArray (feature/rbw-task) +- worktree: ~/wt/rbw-task +""" + +# Two tasks claiming the same repo: the reproducer from the bug prompt. +TWO_CLAIMS = """# Active Tasks + +## first-task +- worktree: ~/wt/first-task +- repos: + - autolens_workspace (feature/first-task) + +## second-task +- worktree: ~/wt/second-task +- repos: + - autolens_workspace (feature/second-task) +""" + +# Both shapes carry trailing notes in the real ledger, and some claims name no +# branch at all. Sampled from active.md history: 258 claim lines, 162 colon-form, +# 139 paren-form, plus bare names like ` - PyAutoReduce`. +ANNOTATED = """# Active Tasks + +## annotated-task +- worktree: ~/wt/annotated-task +- repos: + - HowToFit: feature/howto-smoke (base 65e8fbd == origin/main) + - PyAutoReduce +""" + +# A task that claims nothing — the `repos-none-claimed:` shape live entries use. +NO_CLAIMS = """# Active Tasks + +## release-drive +- issue: (no issue) +- repos-none-claimed: this entry claims NO repos — deliberately on one line. +""" + + +def _run(tmp_path: Path, active_body: str | None, snippet: str): + """Source worktree.sh against a fixture PYAUTO_MAIN and run `snippet`.""" + main = tmp_path / "main" + if active_body is None: + main.mkdir() + else: + (main / "PyAutoMind").mkdir(parents=True) + (main / "PyAutoMind" / "active.md").write_text(active_body) + return subprocess.run( + ["bash", "-c", f'source "{WORKTREE_SH}"; {snippet}'], + env={ + "PATH": "/usr/bin:/bin", + "HOME": str(tmp_path), + "PYAUTO_MAIN": str(main), + "PYAUTO_WT_ROOT": str(tmp_path / "wt"), + }, + capture_output=True, + text=True, + ) + + +def _claims(tmp_path: Path, active_body: str) -> list[list[str]]: + proc = _run(tmp_path, active_body, "worktree_list_claimed") + assert proc.returncode == 0, proc.stderr + return [line.split("\t") for line in proc.stdout.splitlines() if line] + + +# --- the regression: the paren form is what every writer emits ---------------- + +def test_paren_form_claim_conflicts(tmp_path): + proc = _run(tmp_path, PAREN, "worktree_check_conflict new-task autolens_workspace") + assert proc.returncode == 1, "paren-form claim did not register as a conflict" + assert "paren-task" in proc.stderr + + +def test_paren_form_repo_is_bare(tmp_path): + task, repo, branch, wt = _claims(tmp_path, PAREN)[0] + assert repo == "autolens_workspace" + assert branch == "feature/paren-task" + assert wt == "~/wt/paren-task" + + +# --- back-compat: the documented colon form must keep working ----------------- + +def test_colon_form_claim_conflicts(tmp_path): + proc = _run(tmp_path, COLON, "worktree_check_conflict new-task PyAutoFit") + assert proc.returncode == 1 + assert "colon-task" in proc.stderr + + +def test_colon_form_repo_is_bare(tmp_path): + task, repo, branch, wt = _claims(tmp_path, COLON)[0] + assert repo == "PyAutoFit" + assert branch == "feature/colon-task" + + +# --- field-order independence ------------------------------------------------- + +def test_worktree_captured_when_repos_precede_it(tmp_path): + # The awk set `wt` on sight, so a `repos:` block above `worktree:` emitted + # "-" and repo_cleanup lost the worktree path for that claim. + task, repo, branch, wt = _claims(tmp_path, REPOS_FIRST)[0] + assert repo == "PyAutoArray" + assert wt == "~/wt/rbw-task", "worktree: must be captured regardless of field order" + + +def test_worktree_does_not_leak_between_tasks(tmp_path): + body = REPOS_FIRST + "\n## later-task\n- repos:\n - PyAutoLens (feature/later)\n" + rows = {r[0]: r for r in _claims(tmp_path, body)} + assert rows["later-task"][3] == "-", "a task with no worktree: must not inherit one" + + +# --- the guard's own contract ------------------------------------------------- + +def test_task_does_not_conflict_with_itself(tmp_path): + proc = _run(tmp_path, PAREN, "worktree_check_conflict paren-task autolens_workspace") + assert proc.returncode == 0, proc.stderr + + +def test_unclaimed_repo_does_not_conflict(tmp_path): + proc = _run(tmp_path, PAREN, "worktree_check_conflict new-task PyAutoGalaxy") + assert proc.returncode == 0, proc.stderr + + +def test_conflict_names_every_claiming_task(tmp_path): + proc = _run(tmp_path, TWO_CLAIMS, "worktree_check_conflict new-task autolens_workspace") + assert proc.returncode == 1 + assert "first-task" in proc.stderr + assert "second-task" in proc.stderr + + +def test_trailing_note_does_not_corrupt_repo(tmp_path): + # ` - HowToFit: feature/howto-smoke (base 65e8fbd == origin/main)` — the + # repo name is what the guard compares, so the note must not reach it. + rows = {r[1]: r for r in _claims(tmp_path, ANNOTATED)} + assert set(rows) == {"HowToFit", "PyAutoReduce"} + assert rows["HowToFit"][2].startswith("feature/howto-smoke") + + +def test_claim_without_a_branch_still_claims(tmp_path): + # ` - PyAutoReduce` with no branch is a real shape; it must still conflict. + proc = _run(tmp_path, ANNOTATED, "worktree_check_conflict new-task PyAutoReduce") + assert proc.returncode == 1 + assert "annotated-task" in proc.stderr + + +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): + proc = _run(tmp_path, None, "worktree_list_claimed") + assert proc.returncode == 0 + assert proc.stdout == ""