diff --git a/skills/commission/bin/claude-team b/skills/commission/bin/claude-team index dd7a0825b..53daae7e4 100755 --- a/skills/commission/bin/claude-team +++ b/skills/commission/bin/claude-team @@ -9,6 +9,7 @@ import importlib.util import json import os import re +import subprocess import sys from pathlib import Path @@ -154,6 +155,125 @@ def _build_error(msg, code=1): return code +def _list_worktrees(git_root): + """Return [(worktree_path, branch_or_None), ...] for the repo at ``git_root``. + + Parses ``git worktree list --porcelain``. The porcelain format emits one + record per worktree as a sequence of ``key value`` lines (or bare keys for + flags), separated by blank lines. We care about the ``worktree`` and + ``branch`` lines; ``HEAD`` and ``bare``/``detached`` are ignored. + + Returns an empty list on any subprocess failure (git missing, not a repo, + permission denied, etc.). The caller treats an empty list as "no fallback + targets to try" and surfaces a clear error message — silent fallback into + a different code path would be worse than a loud miss. + """ + try: + result = subprocess.run( + ['git', '-C', git_root, 'worktree', 'list', '--porcelain'], + capture_output=True, + text=True, + check=True, + ) + except (FileNotFoundError, subprocess.CalledProcessError): + return [] + + worktrees = [] + current_path = None + current_branch = None + for line in result.stdout.splitlines(): + if not line.strip(): + if current_path is not None: + worktrees.append((current_path, current_branch)) + current_path = None + current_branch = None + continue + if line.startswith('worktree '): + current_path = line[len('worktree '):] + elif line.startswith('branch '): + # Format: "branch refs/heads/" + ref = line[len('branch '):] + if ref.startswith('refs/heads/'): + current_branch = ref[len('refs/heads/'):] + else: + current_branch = ref + if current_path is not None: + worktrees.append((current_path, current_branch)) + return worktrees + + +def _resolve_entity_read_path(entity_path, workflow_dir): + """Resolve ``entity_path`` to a readable filesystem path. + + Two-step resolution: + + 1. If the project-root path itself is a file, return it (flat layout or + worktree-mirrored folder layout where the entity has been merged to + main — the original happy path, zero behavior change). + + 2. Otherwise, enumerate the repo's worktrees via + ``git worktree list --porcelain`` and probe each for a copy of the + file at the same repo-relative location. This handles folder-mode + entities (``-/index.md``) that are git-tracked on a + worktree branch but not yet merged into main — a legitimate dispatch + target that the original ``os.path.isfile(entity_path)`` gate + rejected outright. + + Returns ``(resolved_path, tried_paths)``. ``resolved_path`` is the path + the caller should read; ``None`` means no readable copy exists in any + worktree. ``tried_paths`` is a list of every candidate filesystem path + we probed (including ``entity_path`` itself), surfaced in error + messages so the captain can see exactly where the helper looked. + """ + tried = [entity_path] + if os.path.isfile(entity_path): + return entity_path, tried + + git_root = find_git_root(workflow_dir) + if git_root is None: + return None, tried + + try: + entity_rel = os.path.relpath(entity_path, git_root) + except ValueError: + # Different drives on Windows — relpath is undefined. Fallback can't help. + return None, tried + # If entity_path is OUTSIDE git_root (relpath starts with ..), the worktree + # mirror logic doesn't apply — the path simply doesn't belong to this repo. + if entity_rel.startswith('..'): + return None, tried + + for worktree_path, _branch in _list_worktrees(git_root): + # Skip the main worktree (git_root) — we already probed entity_path above. + if os.path.realpath(worktree_path) == os.path.realpath(git_root): + continue + candidate = os.path.join(worktree_path, entity_rel) + tried.append(candidate) + if os.path.isfile(candidate): + return candidate, tried + + return None, tried + + +def _derive_entity_slug(entity_path): + """Derive the entity's slug from its path. + + Folder-mode entities (``-/index.md``) use the parent directory + name as the slug, not the literal string ``index``. Without this fix, + the derived agent name (``{worker_key}-index-{stage}``) collides across + every folder-mode entity, breaking team-mode dispatch. + + Flat-layout entities (``.md``) keep the original behavior: the + filename stem. + """ + basename = os.path.basename(entity_path) + if basename == 'index.md': + parent = os.path.basename(os.path.dirname(entity_path)) + if parent: + return parent + return os.path.splitext(basename)[0] + + def cmd_build(args): """Assemble a structured dispatch JSON from entity, workflow, and FO-supplied fragments.""" workflow_dir = args.workflow_dir @@ -217,9 +337,22 @@ def cmd_build(args): if not isinstance(checklist, list) or len(checklist) == 0: return _build_error('checklist must not be empty') - # Rule 10: Entity file readable - if not os.path.isfile(entity_path): - return _build_error(f"entity file not readable at '{entity_path}'") + # Rule 10: Entity file readable. + # + # Folder-mode entities (``-/index.md``) live ONLY on a worktree + # branch until merge — they are git-tracked there but absent from the main + # filesystem. Probing only ``entity_path`` would force every folder-mode + # dispatch onto the captain's break-glass path. Resolve via worktree + # fallback when the project-root path is missing. + entity_read_path, tried_paths = _resolve_entity_read_path(entity_path, workflow_dir) + if entity_read_path is None: + attempted = ', '.join(repr(p) for p in tried_paths) + return _build_error( + f"entity file not readable at '{entity_path}'. " + f"Folder-mode worktree fallback also failed: no checked-out worktree " + f"contains a copy at the same repo-relative location. " + f"Tried: [{attempted}]." + ) # Rule 11: Workflow README readable readme_path = os.path.join(workflow_dir, 'README.md') @@ -281,7 +414,10 @@ def cmd_build(args): # the stage's declared `worktree: true|false`. The stage `worktree:` field # gates worktree *creation* (FO-side, before the build call), not per-stage # routing. See first-officer-shared-core.md Reuse conditions #3. - entity_fields = parse_frontmatter(entity_path) + # + # Read frontmatter from the resolved path (worktree-side for folder-mode + # entities not yet on main; project-root path otherwise). + entity_fields = parse_frontmatter(entity_read_path) entity_title = entity_fields.get('title', '') entity_worktree = entity_fields.get('worktree', '').strip() @@ -306,9 +442,12 @@ def cmd_build(args): # Rule 6: Derive subagent_type from stage agent field subagent_type = stage_meta.get('agent', 'spacedock:ensign') - # Derive worker_key and name + # Derive worker_key and name. + # Slug derivation is layout-aware: folder-mode (``/index.md``) + # uses the parent directory name; flat-layout uses the filename stem. + # Without this, every folder-mode entity collides on the literal "index". worker_key = subagent_type.replace(':', '-') - slug = os.path.splitext(os.path.basename(entity_path))[0] + slug = _derive_entity_slug(entity_path) derived_name = f'{worker_key}-{slug}-{stage}' # Rule 7: Name length and safety diff --git a/tests/test_claude_team.py b/tests/test_claude_team.py index e33db9e4c..05adb04a8 100644 --- a/tests/test_claude_team.py +++ b/tests/test_claude_team.py @@ -2156,5 +2156,231 @@ def test_build_surfaces_unparseable_heading_diagnostic(self, tmp_path): assert "first content token" in result.stderr +def _init_git_repo(repo_root: Path) -> None: + """Initialise a real git repo at ``repo_root`` so ``git worktree`` works. + + Tests for the folder-mode fallback need actual ``git worktree add`` calls + because the helper invokes ``git worktree list --porcelain``. A bare + ``.git`` marker directory is not enough. + """ + subprocess.run( + ["git", "init", "-q", "-b", "main", str(repo_root)], + check=True, capture_output=True, + ) + subprocess.run( + ["git", "-C", str(repo_root), "config", "user.email", "test@example.com"], + check=True, capture_output=True, + ) + subprocess.run( + ["git", "-C", str(repo_root), "config", "user.name", "Test"], + check=True, capture_output=True, + ) + subprocess.run( + ["git", "-C", str(repo_root), "config", "commit.gpgsign", "false"], + check=True, capture_output=True, + ) + + +class TestBuildFolderModeWorktreeOnly: + """#NEW: folder-mode entities that live ONLY on a worktree branch dispatch. + + Failure mode (pre-fix): ``cmd_build`` rejected entity paths whose + filesystem copy did not exist on main, even when a checked-out + worktree branch had the file at the same repo-relative location. + Folder-mode entities (``-/index.md``) are git-tracked on a + worktree branch until merge, so this gate forced break-glass dispatch. + """ + + def _make_workflow_readme(self, wf_dir: Path) -> None: + wf_dir.mkdir(parents=True, exist_ok=True) + (wf_dir / "README.md").write_text( + "---\n" + "commissioned-by: spacedock@test\n" + "entity-label: feature\n" + "stages:\n" + " defaults:\n" + " worktree: true\n" + " states:\n" + " - name: ship\n" + " initial: true\n" + " - name: done\n" + " terminal: true\n" + "---\n" + "\n" + "## Stages\n\n" + "### `ship`\n\nShip the deliverable.\n\n" + "### `done`\n\nTerminal.\n" + ) + + def _make_folder_entity( + self, wf_dir: Path, slug: str, worktree_rel: str + ) -> Path: + """Create a folder-mode entity inside ``wf_dir`` returning the index.md path.""" + entity_dir = wf_dir / slug + entity_dir.mkdir() + entity_md = entity_dir / "index.md" + entity_md.write_text( + "---\n" + "id: 057\n" + f"title: Folder mode entity for {slug}\n" + "status: ship\n" + f"worktree: {worktree_rel}\n" + "---\n" + "\n" + "Folder-mode body.\n" + ) + return entity_md + + def test_folder_mode_worktree_only_entity_dispatches(self, tmp_path): + """Folder entity that exists ONLY on a worktree branch resolves via fallback. + + Setup: + - git repo at tmp_path with workflow at docs/ship-flow/ + - workflow README committed to main; folder entity NOT on main + - separate worktree branch contains the folder entity + Expected: build returns exit 0, prompt references the worktree-side + index.md path, derived agent name uses the parent-dir slug (not "index"). + """ + _init_git_repo(tmp_path) + wf_dir = tmp_path / "docs" / "ship-flow" + self._make_workflow_readme(wf_dir) + # Commit README to main so worktree add can branch off cleanly + subprocess.run( + ["git", "-C", str(tmp_path), "add", "docs/ship-flow/README.md"], + check=True, capture_output=True, + ) + subprocess.run( + ["git", "-C", str(tmp_path), "commit", "-q", "-m", "init workflow"], + check=True, capture_output=True, + ) + + slug = "057-sc-844-tagpicker" + wt_rel = ".worktrees/ship-flow-sc-844" + wt_abs = tmp_path / wt_rel + # Create worktree on a new branch + subprocess.run( + [ + "git", "-C", str(tmp_path), + "worktree", "add", "-b", "ship-flow/sc-844", str(wt_abs), + ], + check=True, capture_output=True, + ) + # Add the folder entity ONLY in the worktree (not on main) + wt_wf_dir = wt_abs / "docs" / "ship-flow" + wt_entity = self._make_folder_entity(wt_wf_dir, slug, wt_rel) + subprocess.run( + ["git", "-C", str(wt_abs), "add", "."], + check=True, capture_output=True, + ) + subprocess.run( + ["git", "-C", str(wt_abs), "commit", "-q", "-m", "add folder entity"], + check=True, capture_output=True, + ) + + # Sanity: project-root copy must NOT exist (this is the failing scenario) + project_root_entity = wf_dir / slug / "index.md" + assert not project_root_entity.exists() + # Worktree copy DOES exist + assert wt_entity.exists() + + inp = { + "schema_version": 1, + "entity_path": str(project_root_entity), + "workflow_dir": str(wf_dir), + "stage": "ship", + "checklist": ["1. Verify folder-mode dispatch"], + "team_name": "test-team", + "feedback_context": None, + "scope_notes": None, + "bare_mode": False, + "is_feedback_reflow": False, + } + result = run_build(wf_dir, inp, cwd=tmp_path) + assert result.returncode == 0, ( + f"build failed unexpectedly. stderr={result.stderr}" + ) + out = json.loads(result.stdout) + # Worker name uses the FOLDER name (slug), not the literal "index" + assert out["name"] == f"spacedock-ensign-{slug}-ship", ( + f"slug derivation regressed: derived name={out['name']!r}" + ) + # Read instruction points at the worktree-side path + expected_read = str(wt_abs / "docs" / "ship-flow" / slug / "index.md") + assert f"Read the entity file at {expected_read}" in out["prompt"] + # Working-directory instruction points at the worktree + assert f"Your working directory is {wt_abs}" in out["prompt"] + + def test_folder_mode_fallback_failure_lists_tried_paths(self, tmp_path): + """When no worktree contains the entity, error lists every probed path.""" + _init_git_repo(tmp_path) + wf_dir = tmp_path / "docs" / "ship-flow" + self._make_workflow_readme(wf_dir) + subprocess.run( + ["git", "-C", str(tmp_path), "add", "docs/ship-flow/README.md"], + check=True, capture_output=True, + ) + subprocess.run( + ["git", "-C", str(tmp_path), "commit", "-q", "-m", "init"], + check=True, capture_output=True, + ) + # Create a worktree but do NOT put the entity in it + wt_abs = tmp_path / ".worktrees" / "unrelated" + subprocess.run( + [ + "git", "-C", str(tmp_path), + "worktree", "add", "-b", "unrelated", str(wt_abs), + ], + check=True, capture_output=True, + ) + + missing_entity = wf_dir / "057-missing" / "index.md" + inp = { + "schema_version": 1, + "entity_path": str(missing_entity), + "workflow_dir": str(wf_dir), + "stage": "ship", + "checklist": ["1. Should fail"], + "team_name": "t", + "feedback_context": None, + "scope_notes": None, + "bare_mode": False, + "is_feedback_reflow": False, + } + result = run_build(wf_dir, inp, cwd=tmp_path) + assert result.returncode != 0 + assert "Folder-mode worktree fallback also failed" in result.stderr + # Both the project-root path AND the unrelated worktree's candidate path + # should appear in the "Tried" list so the captain can debug. + assert str(missing_entity) in result.stderr + assert str(wt_abs / "docs" / "ship-flow" / "057-missing" / "index.md") in result.stderr + + def test_flat_layout_regression_unchanged_when_file_present(self, tmp_path): + """Flat-layout entity with main-side file resolves WITHOUT touching git. + + This guards against accidental subprocess invocation overhead and + confirms the original happy path is byte-equivalent. + """ + wf_dir, entity = _make_workflow_fixture(tmp_path) + # Note: tmp_path is NOT a git repo here. If the helper fell through + # to _list_worktrees, the subprocess would still succeed (returning + # empty), but the assertion below confirms entity_path was returned + # via the fast `os.path.isfile()` short-circuit. + inp = { + "schema_version": 1, + "entity_path": str(entity), + "workflow_dir": str(wf_dir), + "stage": "ideation", + "checklist": ["1. Flat-layout regression"], + "team_name": "regression-team", + "bare_mode": False, + } + result = run_build(wf_dir, inp) + assert result.returncode == 0, f"stderr: {result.stderr}" + out = json.loads(result.stdout) + # Slug derived from stem (NOT folder name): file is "my-task.md" + assert out["name"] == "spacedock-ensign-my-task-ideation" + assert f"Read the entity file at {entity}" in out["prompt"] + + if __name__ == "__main__": raise SystemExit(pytest.main([__file__]))