From 5cac27b50f890af52d7c2636348f952a6a33c7e2 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 16 Jun 2026 18:50:14 +0800 Subject: [PATCH 01/13] =?UTF-8?q?feat(engine):=20reachability.py=20?= =?UTF-8?q?=E2=80=94=20orphan-module=20detection=20(new-modules=20+=20impo?= =?UTF-8?q?rter=20sweep=20+=20verdict)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-Authored-By: Claude Opus 4.8 --- prd_taskmaster/reachability.py | 466 ++++++++++++++++++++++++++++++++ tests/core/test_reachability.py | 441 ++++++++++++++++++++++++++++++ 2 files changed, 907 insertions(+) create mode 100644 prd_taskmaster/reachability.py create mode 100644 tests/core/test_reachability.py diff --git a/prd_taskmaster/reachability.py b/prd_taskmaster/reachability.py new file mode 100644 index 0000000..8aa291d --- /dev/null +++ b/prd_taskmaster/reachability.py @@ -0,0 +1,466 @@ +"""Reachability gate — detect orphan modules (code with passing tests but imported by nothing). + +This module is READ-ONLY at runtime: it uses git and grep (via subprocess) to inspect +the repository; it never writes files. + +Design notes +------------ +- A module is WIRED if at least one file outside of its own co-located test imports it. +- A module is ORPHAN if no such importer exists AND no exempt scheme is declared. +- A module is EXEMPT if ``reachable_via`` starts with a known scheme prefix + (cli:|route:|tool:|hook:|plugin:|dynamic:). In v1 we accept the declared scheme on + trust; scheme-registration verification (e.g. checking that the entry-point actually + exists) is a TODO. +- Errors in git/grep are logged to stderr and treated conservatively: an empty new-module + set is considered WIRED/EXEMPT (not ORPHAN), because we cannot confirm the module is + new. A grep error for a specific module is also treated as conservative WIRED so we + never silently block a legitimate module. + +Conservative rule: *fail toward "needs review"* only applies when we have a module we +could not sweep. If new_modules_for_task returns empty (git error), sweep_task returns +WIRED because there is nothing to flag. If find_importers raises (grep error), we return +WIRED (we couldn't confirm orphan status — but we log the anomaly). +""" + +from __future__ import annotations + +import logging +import re +import subprocess +import sys +from datetime import datetime, timezone +from pathlib import Path +from typing import Any + +logger = logging.getLogger(__name__) + +# ─── Exempt scheme prefixes ─────────────────────────────────────────────────── + +_EXEMPT_SCHEMES = ("cli:", "route:", "tool:", "hook:", "plugin:", "dynamic:") + +# ─── Exclusion patterns ─────────────────────────────────────────────────────── + +# Files excluded from "new module" lists even if git says they're added. +_EXCLUDE_NAMES = { + "__init__.py", + "__main__.py", + "conftest.py", + "setup.py", +} + +_EXCLUDE_NAME_RE = re.compile( + r"(^|/)(" + r"test_[^/]+\.(py|ts|js|go)" # test_*.py / test_*.ts … + r"|[^/]+_test\.(py|ts|js|go)" # *_test.py … + r"|[^/]+\.test\.(py|ts|js|go)" # *.test.py / *.test.ts … + r"|index\.[^/]+" # index.* barrels + r"|[^/]+migration[^/]*\.(py|ts|js|go)" # migration files + r")$" +) + +# Directories whose children are always excluded. +_EXCLUDE_DIRS_RE = re.compile(r"(^|/)(__tests__|tests)/") + +# Language source extensions. +_LANG_EXTS: dict[str, set[str]] = { + "py": {".py"}, + "ts": {".ts", ".tsx"}, + "js": {".js", ".jsx", ".mjs", ".cjs"}, + "go": {".go"}, +} + + +# ─── Public API ─────────────────────────────────────────────────────────────── + +def language_for_repo(repo_root: Path) -> str: + """Detect the primary language of a repository from marker files. + + Priority order: + 1. pyproject.toml or setup.py -> 'py' + 2. package.json + tsconfig.json -> 'ts' + 3. package.json alone -> 'js' + 4. go.mod -> 'go' + 5. none of the above -> 'unknown' + """ + root = Path(repo_root) + if (root / "pyproject.toml").exists() or (root / "setup.py").exists(): + return "py" + has_pkg = (root / "package.json").exists() + if has_pkg and (root / "tsconfig.json").exists(): + return "ts" + if has_pkg: + return "js" + if (root / "go.mod").exists(): + return "go" + return "unknown" + + +def new_modules_for_task( + repo_root: Path, + start_commit: str, + head: str = "HEAD", +) -> list[Path]: + """Return repo-relative Paths of source files added between *start_commit* and *head*. + + Files are filtered to: + - Only source extensions for the repo language (as detected by language_for_repo). + - Exclude tests, __init__.py, __main__.py, index.* barrels, conftest.py, + setup.py, and migration files (see module-level patterns). + + If the git diff command fails, logs a warning and returns [] (conservative: caller + sees no new modules and will not flag ORPHAN on missing data). + """ + repo_root = Path(repo_root) + lang = language_for_repo(repo_root) + exts = _LANG_EXTS.get(lang, set()) + + result = subprocess.run( + ["git", "-C", str(repo_root), "diff", + f"{start_commit}..{head}", + "--diff-filter=A", "--name-only"], + capture_output=True, + text=True, + ) + if result.returncode != 0: + logger.warning( + "new_modules_for_task: git diff failed (rc=%d): %s", + result.returncode, + result.stderr.strip(), + ) + return [] + + modules: list[Path] = [] + for line in result.stdout.splitlines(): + path = line.strip() + if not path: + continue + p = Path(path) + # Must be a recognised source extension for this language. + if p.suffix not in exts: + continue + # Excluded by filename. + if p.name in _EXCLUDE_NAMES: + continue + # Excluded by path pattern (tests/, __tests__/, test_*.py, etc.). + if _EXCLUDE_NAME_RE.search(path) or _EXCLUDE_DIRS_RE.search(path): + continue + modules.append(p) + + return modules + + +def find_importers(repo_root: Path, module: Path, lang: str) -> list[Path]: + """Return repo-relative Paths of files that import *module*, excluding the module itself + and its own co-located test. + + Python (lang='py'): + Given foo/bar.py the pkg dotted path is foo.bar and the bare name is bar. + Grep for any of: + import foo.bar + from foo.bar import + from foo import bar + import bar (word boundary) + across *.py files. + + TypeScript/JavaScript (lang='ts'|'js'): + Grep for ``from '...bar'`` / ``require('...bar')`` referencing the module's + basename (without extension). + + Go (lang='go'): + Grep for the module's directory path (package) in import blocks. + + If grep fails (e.g. no files found), logs and returns [] (conservative: treat as WIRED + — we cannot confirm orphan status). + + Returns repo-relative Paths. + """ + repo_root = Path(repo_root) + module = Path(module) + module_str = module.as_posix() + + # Build the set of paths to exclude (the module itself + its co-located test). + exclude_paths: set[str] = {module_str} + stem = module.stem + parent = module.parent + for candidate in [ + parent / f"test_{stem}{module.suffix}", + parent / f"{stem}_test{module.suffix}", + parent / f"{stem}.test{module.suffix}", + ]: + exclude_paths.add(candidate.as_posix()) + # Also check tests/ sibling directory. + exclude_paths.add((parent.parent / "tests" / f"test_{stem}{module.suffix}").as_posix()) + exclude_paths.add( + (parent.parent / "tests" / f"test_{stem}{module.suffix}").as_posix() + ) + + if lang == "py": + patterns = _py_import_patterns(module) + raw = _grep_patterns(repo_root, patterns, "*.py") + elif lang in ("ts", "js"): + patterns = _ts_import_patterns(module) + globs = ["*.ts", "*.tsx", "*.js", "*.jsx", "*.mjs"] + raw = [] + for g in globs: + raw.extend(_grep_patterns(repo_root, patterns, g)) + elif lang == "go": + patterns = _go_import_patterns(repo_root, module) + raw = _grep_patterns(repo_root, patterns, "*.go") + else: + return [] + + importers: list[Path] = [] + seen: set[str] = set() + for p in raw: + rel = p.as_posix() + if rel in seen or rel in exclude_paths: + continue + seen.add(rel) + importers.append(p) + + return importers + + +def reachability_verdict( + repo_root: Path, + module: Path, + lang: str, + reachable_via: str | None = None, + tier: str = "domain-model", +) -> dict: + """Compute the reachability verdict for a single *module*. + + Steps: + 1. EXEMPT if *reachable_via* starts with a known scheme prefix (cli:|route:|tool:|hook:|plugin:|dynamic:). + (v1: scheme accepted on trust; verification is a TODO.) + 2. Call find_importers. WIRED if any found, else ORPHAN. + + Returns a dict:: + + { + "verdict": "WIRED" | "ORPHAN" | "EXEMPT", + "module": str, + "importers": [str, ...], + "reachable_via": str | None, + "exempt_reason": str | None, # scheme name if EXEMPT + "lang": str, + } + """ + module = Path(module) + module_str = module.as_posix() + + # Step 1: scheme exemption. + if reachable_via: + for scheme in _EXEMPT_SCHEMES: + if reachable_via.startswith(scheme): + return { + "verdict": "EXEMPT", + "module": module_str, + "importers": [], + "reachable_via": reachable_via, + "exempt_reason": scheme.rstrip(":"), + "lang": lang, + } + + # Step 2: importer sweep. + try: + importers = find_importers(repo_root, module, lang) + except Exception as exc: + logger.warning( + "reachability_verdict: find_importers raised for %s: %s — treating as WIRED (conservative)", + module_str, + exc, + ) + importers = [] + # Conservative: we cannot confirm orphan → WIRED. + return { + "verdict": "WIRED", + "module": module_str, + "importers": [], + "reachable_via": reachable_via, + "exempt_reason": None, + "lang": lang, + } + + verdict = "WIRED" if importers else "ORPHAN" + return { + "verdict": verdict, + "module": module_str, + "importers": [p.as_posix() for p in importers], + "reachable_via": reachable_via, + "exempt_reason": None, + "lang": lang, + } + + +def sweep_task( + repo_root: Path, + task: dict, + start_commit: str, +) -> dict: + """Run the reachability sweep for a task. + + Used by execute-task and ship-check Gate 6. + + Logic: + - tier = task.phaseConfig.tier or task.tier or "domain-model". + - If tier in {spike, domain-model}: EXEMPT (not swept; reachability not required at this phase). + - If task.entrypoint is truthy: EXEMPT (entrypoints are by definition reachable via the outside world). + - Else (tier in {wired, live}): sweep new modules and compute per-module verdicts. + Task verdict = ORPHAN if ANY module is ORPHAN; else WIRED (EXEMPT modules don't make the task orphan). + + Returns:: + + { + "verdict": "WIRED" | "ORPHAN" | "EXEMPT", + "tier": str, + "modules": [], + "reason": str | None, # only for EXEMPT + "checked_at": str (ISO-8601), + "start_commit": str, + } + """ + repo_root = Path(repo_root) + tier = ( + (task.get("phaseConfig") or {}).get("tier") + or task.get("tier") + or "domain-model" + ) + checked_at = datetime.now(timezone.utc).isoformat() + + # Tier-exempt (spike / domain-model): do not sweep. + if tier in ("spike", "domain-model"): + return { + "verdict": "EXEMPT", + "reason": "tier-exempt", + "tier": tier, + "modules": [], + "checked_at": checked_at, + "start_commit": start_commit, + } + + # Entrypoint tasks are by definition wired to an external caller. + if task.get("entrypoint"): + return { + "verdict": "EXEMPT", + "reason": "entrypoint", + "tier": tier, + "modules": [], + "checked_at": checked_at, + "start_commit": start_commit, + } + + # Sweep (tier in {wired, live}). + lang = language_for_repo(repo_root) + try: + new_modules = new_modules_for_task(repo_root, start_commit) + except Exception as exc: + logger.warning("sweep_task: new_modules_for_task raised: %s — returning WIRED (conservative)", exc) + return { + "verdict": "WIRED", + "tier": tier, + "modules": [], + "checked_at": checked_at, + "start_commit": start_commit, + } + + reachable_via = task.get("reachableVia") + module_verdicts: list[dict] = [] + + for mod in new_modules: + v = reachability_verdict( + repo_root=repo_root, + module=mod, + lang=lang, + reachable_via=reachable_via, + tier=tier, + ) + module_verdicts.append(v) + + # Task is ORPHAN if ANY module is ORPHAN (EXEMPT modules don't contribute to orphan status). + task_verdict = "WIRED" + for v in module_verdicts: + if v["verdict"] == "ORPHAN": + task_verdict = "ORPHAN" + break + + return { + "verdict": task_verdict, + "tier": tier, + "modules": module_verdicts, + "checked_at": checked_at, + "start_commit": start_commit, + } + + +# ─── Internal helpers ───────────────────────────────────────────────────────── + +def _py_import_patterns(module: Path) -> list[str]: + """Build grep -E patterns that match Python import statements for *module*.""" + # Convert path to dotted module name: foo/bar.py -> foo.bar + parts = list(module.with_suffix("").parts) + dotted = ".".join(parts) # e.g. "pkg.foo" + name = module.stem # e.g. "foo" + parent_pkg = ".".join(parts[:-1]) # e.g. "pkg" (may be empty for top-level) + + patterns = [ + rf"import {re.escape(dotted)}(\s|$|;|,)", + rf"from {re.escape(dotted)} import", + ] + if parent_pkg: + patterns.append(rf"from {re.escape(parent_pkg)} import {re.escape(name)}(\s|$|;|,)") + # Bare "import name" — less precise but catches flat imports. + patterns.append(rf"import {re.escape(name)}(\s|$|;|,)") + return patterns + + +def _ts_import_patterns(module: Path) -> list[str]: + """Build grep -E patterns that match TS/JS import/require for *module*.""" + stem = module.stem + # Match: from '...stem' | from "...stem" | require('...stem') | require("...stem") + return [ + rf"""from ['"][^'"]*{re.escape(stem)}['"]""", + rf"""require\(['"][^'"]*{re.escape(stem)}['"]\)""", + ] + + +def _go_import_patterns(repo_root: Path, module: Path) -> list[str]: + """Build grep patterns for Go package imports.""" + # Use the directory containing the module file as the package path fragment. + pkg_dir = module.parent.as_posix() # e.g. "pkg/foo" + return [rf'"{re.escape(pkg_dir)}"'] + + +def _grep_patterns(repo_root: Path, patterns: list[str], glob: str) -> list[Path]: + """Run grep -rEl for each pattern across files matching *glob* under *repo_root*. + + Returns a deduplicated list of repo-relative Paths of matching files. + On error, logs a warning and returns [] (conservative: caller decides what to do). + """ + found: set[str] = set() + for pattern in patterns: + result = subprocess.run( + ["grep", "-rEl", "--include", glob, pattern, "."], + cwd=str(repo_root), + capture_output=True, + text=True, + ) + if result.returncode not in (0, 1): + # rc=1 means "no match" — that is expected. Any other rc is an error. + logger.warning( + "_grep_patterns: grep returned rc=%d for pattern %r: %s", + result.returncode, + pattern, + result.stderr.strip(), + ) + # Conservative: do NOT abort; keep going with other patterns. + continue + for line in result.stdout.splitlines(): + line = line.strip() + if line: + # Strip leading "./" from grep output. + if line.startswith("./"): + line = line[2:] + found.add(line) + + return [Path(p) for p in sorted(found)] diff --git a/tests/core/test_reachability.py b/tests/core/test_reachability.py new file mode 100644 index 0000000..2334cad --- /dev/null +++ b/tests/core/test_reachability.py @@ -0,0 +1,441 @@ +"""Tests for prd_taskmaster.reachability — orphan-module detection. + +Each test that exercises WIRED vs ORPHAN builds a genuine tiny git repo with +real commits and real files so that the grep logic is executed against actual +source content. No mocking of the load-bearing grep/git calls. +""" + +from __future__ import annotations + +import subprocess +from pathlib import Path + +import pytest + +from prd_taskmaster.reachability import ( + find_importers, + language_for_repo, + new_modules_for_task, + reachability_verdict, + sweep_task, +) + + +# ─── Git repo fixture helper ────────────────────────────────────────────────── + +def _git(repo: Path, *args: str) -> str: + """Run a git command in *repo* and return stdout (stripped).""" + result = subprocess.run( + ["git", "-C", str(repo), *args], + capture_output=True, + text=True, + check=True, + ) + return result.stdout.strip() + + +def _init_repo(tmp_path: Path) -> Path: + """Create a bare-minimum git repo in *tmp_path*.""" + repo = tmp_path / "repo" + repo.mkdir() + _git(repo, "init") + _git(repo, "config", "user.email", "test@example.com") + _git(repo, "config", "user.name", "Test") + return repo + + +def _commit_all(repo: Path, message: str) -> str: + """Stage everything under *repo* and commit. Returns the commit SHA.""" + _git(repo, "add", "-A") + _git(repo, "commit", "-m", message) + return _git(repo, "rev-parse", "HEAD") + + +def _make_py_repo(tmp_path: Path) -> tuple[Path, str, str]: + """Build a two-commit Python repo. + + Commit 1 (start): pyproject.toml + pkg/__init__.py + Commit 2 (head): pkg/foo.py + tests/test_foo.py (the new module + its test) + + Returns (repo, start_sha, head_sha). + """ + repo = _init_repo(tmp_path) + (repo / "pyproject.toml").write_text("[project]\nname = 'mypkg'\n") + pkg = repo / "pkg" + pkg.mkdir() + (pkg / "__init__.py").write_text("") + tests_dir = repo / "tests" + tests_dir.mkdir() + (tests_dir / "__init__.py").write_text("") + start = _commit_all(repo, "initial") + + # Add the new module and its test. + (pkg / "foo.py").write_text("def hello():\n return 'hello'\n") + (tests_dir / "test_foo.py").write_text( + "from pkg.foo import hello\ndef test_hello():\n assert hello() == 'hello'\n" + ) + head = _commit_all(repo, "add foo") + return repo, start, head + + +# ─── 1. language_for_repo ───────────────────────────────────────────────────── + +class TestLanguageForRepo: + def test_pyproject_toml_detected_as_py(self, tmp_path): + repo = tmp_path / "proj" + repo.mkdir() + (repo / "pyproject.toml").write_text("[project]\nname='x'\n") + assert language_for_repo(repo) == "py" + + def test_setup_py_detected_as_py(self, tmp_path): + repo = tmp_path / "proj" + repo.mkdir() + (repo / "setup.py").write_text("from setuptools import setup; setup()\n") + assert language_for_repo(repo) == "py" + + def test_package_json_plus_tsconfig_detected_as_ts(self, tmp_path): + repo = tmp_path / "proj" + repo.mkdir() + (repo / "package.json").write_text("{}") + (repo / "tsconfig.json").write_text("{}") + assert language_for_repo(repo) == "ts" + + def test_package_json_alone_detected_as_js(self, tmp_path): + repo = tmp_path / "proj" + repo.mkdir() + (repo / "package.json").write_text("{}") + assert language_for_repo(repo) == "js" + + def test_go_mod_detected_as_go(self, tmp_path): + repo = tmp_path / "proj" + repo.mkdir() + (repo / "go.mod").write_text("module example.com/m\ngo 1.21\n") + assert language_for_repo(repo) == "go" + + def test_bare_directory_is_unknown(self, tmp_path): + repo = tmp_path / "proj" + repo.mkdir() + assert language_for_repo(repo) == "unknown" + + def test_pyproject_takes_priority_over_package_json(self, tmp_path): + """If both pyproject.toml and package.json exist, Python wins.""" + repo = tmp_path / "proj" + repo.mkdir() + (repo / "pyproject.toml").write_text("[project]\n") + (repo / "package.json").write_text("{}") + assert language_for_repo(repo) == "py" + + +# ─── 2. new_modules_for_task ────────────────────────────────────────────────── + +class TestNewModulesForTask: + def test_returns_source_module_and_excludes_test(self, tmp_path): + repo, start, head = _make_py_repo(tmp_path) + modules = new_modules_for_task(repo, start, head) + names = [m.name for m in modules] + assert "foo.py" in names, "source module must be included" + assert "test_foo.py" not in names, "test file must be excluded" + + def test_excludes_init_py(self, tmp_path): + """__init__.py added in a commit should not appear as a 'new module'.""" + repo = _init_repo(tmp_path) + (repo / "pyproject.toml").write_text("[project]\n") + start = _commit_all(repo, "init") + pkg = repo / "mypkg" + pkg.mkdir() + (pkg / "__init__.py").write_text("") + (pkg / "core.py").write_text("x = 1\n") + head = _commit_all(repo, "add pkg") + modules = new_modules_for_task(repo, start, head) + names = [m.name for m in modules] + assert "__init__.py" not in names + assert "core.py" in names + + def test_excludes_main_py(self, tmp_path): + repo = _init_repo(tmp_path) + (repo / "pyproject.toml").write_text("[project]\n") + start = _commit_all(repo, "init") + (repo / "__main__.py").write_text("import sys\n") + (repo / "util.py").write_text("def f(): pass\n") + head = _commit_all(repo, "add files") + modules = new_modules_for_task(repo, start, head) + names = [m.name for m in modules] + assert "__main__.py" not in names + assert "util.py" in names + + def test_empty_range_returns_empty_list(self, tmp_path): + repo, start, head = _make_py_repo(tmp_path) + # Same commit as start and head — no changes. + modules = new_modules_for_task(repo, head, head) + assert modules == [] + + def test_excludes_files_under_tests_dir(self, tmp_path): + repo = _init_repo(tmp_path) + (repo / "pyproject.toml").write_text("[project]\n") + start = _commit_all(repo, "init") + tests = repo / "tests" + tests.mkdir() + (tests / "helper.py").write_text("# helper\n") + (repo / "real_module.py").write_text("x = 1\n") + head = _commit_all(repo, "add") + modules = new_modules_for_task(repo, start, head) + paths = [m.as_posix() for m in modules] + assert not any("tests/" in p for p in paths), "tests/ subtree must be excluded" + assert "real_module.py" in paths + + +# ─── 3. WIRED: importer exists ──────────────────────────────────────────────── + +class TestReachabilityWired: + """pkg/foo.py is imported by pkg/app.py — must be WIRED.""" + + def test_wired_when_importer_exists(self, tmp_path): + repo = _init_repo(tmp_path) + (repo / "pyproject.toml").write_text("[project]\n") + pkg = repo / "pkg" + pkg.mkdir() + (pkg / "__init__.py").write_text("") + start = _commit_all(repo, "init") + + # New module. + (pkg / "foo.py").write_text("def hello():\n return 'hello'\n") + # An importer — genuinely imports the new module. + (pkg / "app.py").write_text("from pkg.foo import hello\n\nprint(hello())\n") + # Co-located test (must NOT make it wired on its own). + tests = repo / "tests" + tests.mkdir() + (tests / "test_foo.py").write_text("from pkg.foo import hello\n") + head = _commit_all(repo, "add foo + app") + + verdict = reachability_verdict(repo, Path("pkg/foo.py"), "py") + assert verdict["verdict"] == "WIRED" + assert any("app.py" in imp for imp in verdict["importers"]) + + def test_wired_importer_paths_are_repo_relative(self, tmp_path): + repo = _init_repo(tmp_path) + (repo / "pyproject.toml").write_text("[project]\n") + pkg = repo / "pkg" + pkg.mkdir() + (pkg / "__init__.py").write_text("") + (pkg / "bar.py").write_text("VALUE = 42\n") + (pkg / "main.py").write_text("from pkg.bar import VALUE\n") + _commit_all(repo, "add") + + verdict = reachability_verdict(repo, Path("pkg/bar.py"), "py") + assert verdict["verdict"] == "WIRED" + # Paths should be relative (no leading slash or absolute prefix). + for imp in verdict["importers"]: + assert not imp.startswith("/"), f"importer path should be relative, got: {imp}" + + +# ─── 4. ORPHAN: no importers ────────────────────────────────────────────────── + +class TestReachabilityOrphan: + """pkg/foo.py exists with only a test importing it — must be ORPHAN.""" + + def test_orphan_when_only_colocated_test_imports(self, tmp_path): + repo, start, head = _make_py_repo(tmp_path) + # tests/test_foo.py imports pkg.foo, but it's excluded from importers. + # No other file imports pkg.foo. + verdict = reachability_verdict(repo, Path("pkg/foo.py"), "py") + assert verdict["verdict"] == "ORPHAN" + assert verdict["importers"] == [] + + def test_orphan_verdict_fields(self, tmp_path): + repo, start, head = _make_py_repo(tmp_path) + verdict = reachability_verdict(repo, Path("pkg/foo.py"), "py") + assert verdict["module"] == "pkg/foo.py" + assert verdict["lang"] == "py" + assert verdict["reachable_via"] is None + assert verdict["exempt_reason"] is None + + def test_orphan_vs_wired_is_not_tautological(self, tmp_path): + """Confirm the grep genuinely distinguishes ORPHAN from WIRED by adding/removing an importer.""" + repo = _init_repo(tmp_path) + (repo / "pyproject.toml").write_text("[project]\n") + pkg = repo / "pkg" + pkg.mkdir() + (pkg / "__init__.py").write_text("") + (pkg / "util.py").write_text("def compute(): return 99\n") + start = _commit_all(repo, "init") + + # --- ORPHAN state: only test imports util --- + tests = repo / "tests" + tests.mkdir() + (tests / "test_util.py").write_text("from pkg.util import compute\n") + _commit_all(repo, "add test only") + v_orphan = reachability_verdict(repo, Path("pkg/util.py"), "py") + assert v_orphan["verdict"] == "ORPHAN", "Must be ORPHAN when only test imports" + + # --- WIRED state: add a real importer --- + (pkg / "service.py").write_text("from pkg.util import compute\nresult = compute()\n") + _commit_all(repo, "add service importer") + v_wired = reachability_verdict(repo, Path("pkg/util.py"), "py") + assert v_wired["verdict"] == "WIRED", "Must be WIRED once a real importer exists" + + +# ─── 5. EXEMPT (scheme) ─────────────────────────────────────────────────────── + +class TestReachabilityExempt: + def test_cli_scheme_exempts_orphan(self, tmp_path): + repo, start, head = _make_py_repo(tmp_path) + # pkg/foo.py has no real importer, but declared as cli-reachable. + verdict = reachability_verdict( + repo, Path("pkg/foo.py"), "py", reachable_via="cli:foo" + ) + assert verdict["verdict"] == "EXEMPT" + assert verdict["exempt_reason"] == "cli" + + def test_route_scheme_exempts(self, tmp_path): + repo, start, head = _make_py_repo(tmp_path) + verdict = reachability_verdict( + repo, Path("pkg/foo.py"), "py", reachable_via="route:/api/v1/foo" + ) + assert verdict["verdict"] == "EXEMPT" + assert verdict["exempt_reason"] == "route" + + def test_tool_scheme_exempts(self, tmp_path): + repo, start, head = _make_py_repo(tmp_path) + verdict = reachability_verdict( + repo, Path("pkg/foo.py"), "py", reachable_via="tool:my_tool" + ) + assert verdict["verdict"] == "EXEMPT" + assert verdict["exempt_reason"] == "tool" + + def test_hook_scheme_exempts(self, tmp_path): + repo, start, head = _make_py_repo(tmp_path) + verdict = reachability_verdict( + repo, Path("pkg/foo.py"), "py", reachable_via="hook:post_save" + ) + assert verdict["verdict"] == "EXEMPT" + assert verdict["exempt_reason"] == "hook" + + def test_plugin_scheme_exempts(self, tmp_path): + repo, start, head = _make_py_repo(tmp_path) + verdict = reachability_verdict( + repo, Path("pkg/foo.py"), "py", reachable_via="plugin:my_plugin" + ) + assert verdict["verdict"] == "EXEMPT" + assert verdict["exempt_reason"] == "plugin" + + def test_dynamic_scheme_exempts(self, tmp_path): + repo, start, head = _make_py_repo(tmp_path) + verdict = reachability_verdict( + repo, Path("pkg/foo.py"), "py", reachable_via="dynamic:importlib" + ) + assert verdict["verdict"] == "EXEMPT" + assert verdict["exempt_reason"] == "dynamic" + + def test_no_scheme_does_not_exempt(self, tmp_path): + """A free-form reachable_via string without a known scheme is NOT exempt.""" + repo, start, head = _make_py_repo(tmp_path) + verdict = reachability_verdict( + repo, Path("pkg/foo.py"), "py", reachable_via="just a comment" + ) + # Should still be ORPHAN (no real importers) — no scheme match. + assert verdict["verdict"] == "ORPHAN" + assert verdict["exempt_reason"] is None + + +# ─── 6. sweep_task ──────────────────────────────────────────────────────────── + +class TestSweepTask: + def test_domain_model_tier_is_tier_exempt(self, tmp_path): + repo, start, head = _make_py_repo(tmp_path) + task = {"tier": "domain-model"} + result = sweep_task(repo, task, start) + assert result["verdict"] == "EXEMPT" + assert result["reason"] == "tier-exempt" + assert result["modules"] == [] + + def test_spike_tier_is_tier_exempt(self, tmp_path): + repo, start, head = _make_py_repo(tmp_path) + task = {"tier": "spike"} + result = sweep_task(repo, task, start) + assert result["verdict"] == "EXEMPT" + assert result["reason"] == "tier-exempt" + + def test_phase_config_tier_takes_priority(self, tmp_path): + repo, start, head = _make_py_repo(tmp_path) + task = {"tier": "wired", "phaseConfig": {"tier": "domain-model"}} + result = sweep_task(repo, task, start) + assert result["verdict"] == "EXEMPT" + assert result["reason"] == "tier-exempt" + + def test_entrypoint_task_is_exempt(self, tmp_path): + repo, start, head = _make_py_repo(tmp_path) + task = {"tier": "wired", "entrypoint": True} + result = sweep_task(repo, task, start) + assert result["verdict"] == "EXEMPT" + assert result["reason"] == "entrypoint" + + def test_wired_tier_with_orphan_module_returns_orphan(self, tmp_path): + """A 'wired' tier task whose new module has no importer must be ORPHAN.""" + repo, start, head = _make_py_repo(tmp_path) + # pkg/foo.py was added in head but has no importer (only tests/test_foo.py). + task = {"tier": "wired"} + result = sweep_task(repo, task, start) + assert result["verdict"] == "ORPHAN" + # There should be exactly one module entry. + assert len(result["modules"]) == 1 + assert result["modules"][0]["verdict"] == "ORPHAN" + + def test_live_tier_with_orphan_module_returns_orphan(self, tmp_path): + repo, start, head = _make_py_repo(tmp_path) + task = {"tier": "live"} + result = sweep_task(repo, task, start) + assert result["verdict"] == "ORPHAN" + + def test_wired_tier_with_wired_module_returns_wired(self, tmp_path): + """A 'wired' tier task where the new module is imported by an EXISTING file is WIRED. + + The importer (app.py) must exist in the start commit so it is not listed as + a 'new module' — only feature.py is new. This ensures the importer is a + genuine pre-existing wire, not itself an orphan added in the same sweep window. + """ + repo = _init_repo(tmp_path) + (repo / "pyproject.toml").write_text("[project]\n") + pkg = repo / "pkg" + pkg.mkdir() + (pkg / "__init__.py").write_text("") + # app.py exists BEFORE the task window — it's the importer, not a new module. + (pkg / "app.py").write_text("# placeholder — will be updated to import feature\n") + start = _commit_all(repo, "init") + + # Add the new module. Update app.py to import it (but app.py is not new — it + # was already committed, so it won't appear in new_modules_for_task). + (pkg / "feature.py").write_text("def run(): pass\n") + (pkg / "app.py").write_text("from pkg.feature import run\nrun()\n") + head = _commit_all(repo, "add feature + wire app") + + task = {"tier": "wired"} + result = sweep_task(repo, task, start) + assert result["verdict"] == "WIRED" + + def test_sweep_result_has_required_fields(self, tmp_path): + repo, start, head = _make_py_repo(tmp_path) + task = {"tier": "wired"} + result = sweep_task(repo, task, start) + assert "verdict" in result + assert "tier" in result + assert "modules" in result + assert "checked_at" in result + assert "start_commit" in result + assert result["start_commit"] == start + + def test_default_tier_is_domain_model_exempt(self, tmp_path): + """A task with no tier field defaults to domain-model (EXEMPT).""" + repo, start, head = _make_py_repo(tmp_path) + task = {} # no tier key + result = sweep_task(repo, task, start) + assert result["verdict"] == "EXEMPT" + assert result["reason"] == "tier-exempt" + + def test_reachable_via_scheme_exempts_module_in_wired_tier(self, tmp_path): + """A wired-tier task with an orphan module declared as cli: is task-WIRED (module EXEMPT).""" + repo, start, head = _make_py_repo(tmp_path) + task = {"tier": "wired", "reachableVia": "cli:foo"} + result = sweep_task(repo, task, start) + # Module verdict is EXEMPT (cli scheme), so task verdict is WIRED (no ORPHAN modules). + assert result["verdict"] == "WIRED" + assert result["modules"][0]["verdict"] == "EXEMPT" From 8f6ea8fe2bf97e644d6418728c87f39316a43923 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 16 Jun 2026 19:06:46 +0800 Subject: [PATCH 02/13] fix(engine): reachability fail-closed on git/grep error; exclude nested tests; anchor TS import match --- prd_taskmaster/reachability.py | 212 +++++++++++++++++++++-------- tests/core/test_reachability.py | 233 ++++++++++++++++++++++++++++++++ 2 files changed, 386 insertions(+), 59 deletions(-) diff --git a/prd_taskmaster/reachability.py b/prd_taskmaster/reachability.py index 8aa291d..eac9fec 100644 --- a/prd_taskmaster/reachability.py +++ b/prd_taskmaster/reachability.py @@ -3,23 +3,41 @@ This module is READ-ONLY at runtime: it uses git and grep (via subprocess) to inspect the repository; it never writes files. +Verdict contract +---------------- +- WIRED : at least one non-test file outside the module's own co-located test imports it. + This is a PASS verdict. +- EXEMPT : ``reachable_via`` starts with a known scheme prefix + (cli:|route:|tool:|hook:|plugin:|dynamic:). In v1 we accept the declared scheme + on trust; scheme-registration verification is a TODO. + This is a PASS verdict. +- ORPHAN : no non-test importer found AND no exempt scheme declared. + This is a BLOCKING verdict. +- ERROR : git or grep encountered a real failure (exit code != 0 for git; + exit code >= 2 for grep). The sweep cannot be trusted. + This is a BLOCKING verdict. + +CRITICAL: ERROR must NEVER be mis-reported as WIRED or EXEMPT. +Only WIRED and EXEMPT are passing verdicts. ORPHAN and ERROR both block. + Design notes ------------ -- A module is WIRED if at least one file outside of its own co-located test imports it. -- A module is ORPHAN if no such importer exists AND no exempt scheme is declared. -- A module is EXEMPT if ``reachable_via`` starts with a known scheme prefix - (cli:|route:|tool:|hook:|plugin:|dynamic:). In v1 we accept the declared scheme on - trust; scheme-registration verification (e.g. checking that the entry-point actually - exists) is a TODO. -- Errors in git/grep are logged to stderr and treated conservatively: an empty new-module - set is considered WIRED/EXEMPT (not ORPHAN), because we cannot confirm the module is - new. A grep error for a specific module is also treated as conservative WIRED so we - never silently block a legitimate module. - -Conservative rule: *fail toward "needs review"* only applies when we have a module we -could not sweep. If new_modules_for_task returns empty (git error), sweep_task returns -WIRED because there is nothing to flag. If find_importers raises (grep error), we return -WIRED (we couldn't confirm orphan status — but we log the anomaly). +- new_modules_for_task raises ReachabilityError on git failure so callers can never + silently swallow a broken-environment result and pass an orphan sweep. +- find_importers / _grep_patterns raise ReachabilityError on grep exit code >= 2 + (a real error, not "no matches"). grep exit code 1 ("no matches") is NORMAL and + causes ORPHAN, not an error. +- reachability_verdict and sweep_task catch ReachabilityError and return + {"verdict": "ERROR", ...} so the gate is blocked. +- Test files are excluded from importers whether co-located, under tests/, tests/core/, + or any __tests__/ subtree. + +Known v1 limitations (not fixed here; deferred to future iterations) +----------------------------------------------------------------------- +- Python commented-out imports (# import bar) can produce a false-WIRED via grep. + A future AST-based scan would eliminate this. +- Migration-filename exclusion in new_modules_for_task covers filename patterns but not + content (e.g. a migration that also defines a domain model). Low-risk in practice. """ from __future__ import annotations @@ -34,6 +52,16 @@ logger = logging.getLogger(__name__) +# ─── Sentinel exception ─────────────────────────────────────────────────────── + +class ReachabilityError(Exception): + """Raised when a git or grep subprocess fails with a real error. + + Distinct from 'no matches' (grep rc=1), which is a normal, expected result + that produces an ORPHAN verdict rather than an error. + """ + + # ─── Exempt scheme prefixes ─────────────────────────────────────────────────── _EXEMPT_SCHEMES = ("cli:", "route:", "tool:", "hook:", "plugin:", "dynamic:") @@ -107,8 +135,12 @@ def new_modules_for_task( - Exclude tests, __init__.py, __main__.py, index.* barrels, conftest.py, setup.py, and migration files (see module-level patterns). - If the git diff command fails, logs a warning and returns [] (conservative: caller - sees no new modules and will not flag ORPHAN on missing data). + Raises ReachabilityError if git exits with a non-zero return code (real git failure). + An empty diff (git succeeds but no files were added) returns [] normally — this is + NOT an error. + + Contract: callers must handle ReachabilityError to avoid silently passing a sweep + on a broken git environment. """ repo_root = Path(repo_root) lang = language_for_repo(repo_root) @@ -122,12 +154,12 @@ def new_modules_for_task( text=True, ) if result.returncode != 0: - logger.warning( - "new_modules_for_task: git diff failed (rc=%d): %s", - result.returncode, - result.stderr.strip(), + msg = ( + f"new_modules_for_task: git diff failed (rc={result.returncode}): " + f"{result.stderr.strip()}" ) - return [] + logger.warning(msg) + raise ReachabilityError(msg) modules: list[Path] = [] for line in result.stdout.splitlines(): @@ -151,7 +183,8 @@ def new_modules_for_task( def find_importers(repo_root: Path, module: Path, lang: str) -> list[Path]: """Return repo-relative Paths of files that import *module*, excluding the module itself - and its own co-located test. + and all test files (co-located, under tests/, tests/core/, __tests__/, or any + path matching the test-file patterns). Python (lang='py'): Given foo/bar.py the pkg dotted path is foo.bar and the bare name is bar. @@ -163,14 +196,17 @@ def find_importers(repo_root: Path, module: Path, lang: str) -> list[Path]: across *.py files. TypeScript/JavaScript (lang='ts'|'js'): - Grep for ``from '...bar'`` / ``require('...bar')`` referencing the module's - basename (without extension). + Grep for ``from '...{stem}'`` / ``require('...{stem}')`` referencing the module's + basename (without extension), anchored at a path-component boundary (/, ./, or start + of path segment) so that stem='bar' does NOT match './foobar'. Go (lang='go'): Grep for the module's directory path (package) in import blocks. - If grep fails (e.g. no files found), logs and returns [] (conservative: treat as WIRED - — we cannot confirm orphan status). + Error policy (fail CLOSED): + - grep exit code 1 = "no matches" → NORMAL, returns [] → caller produces ORPHAN. + - grep exit code >= 2 = real error → RAISES ReachabilityError. + Test-file importers are excluded from the result. Returns repo-relative Paths. """ @@ -190,9 +226,6 @@ def find_importers(repo_root: Path, module: Path, lang: str) -> list[Path]: exclude_paths.add(candidate.as_posix()) # Also check tests/ sibling directory. exclude_paths.add((parent.parent / "tests" / f"test_{stem}{module.suffix}").as_posix()) - exclude_paths.add( - (parent.parent / "tests" / f"test_{stem}{module.suffix}").as_posix() - ) if lang == "py": patterns = _py_import_patterns(module) @@ -215,6 +248,10 @@ def find_importers(repo_root: Path, module: Path, lang: str) -> list[Path]: rel = p.as_posix() if rel in seen or rel in exclude_paths: continue + # Exclude ALL test files — co-located, nested under tests/, __tests__/, or any + # path matching the test-file or test-directory patterns. + if _EXCLUDE_NAME_RE.search(rel) or _EXCLUDE_DIRS_RE.search(rel): + continue seen.add(rel) importers.append(p) @@ -234,16 +271,24 @@ def reachability_verdict( 1. EXEMPT if *reachable_via* starts with a known scheme prefix (cli:|route:|tool:|hook:|plugin:|dynamic:). (v1: scheme accepted on trust; verification is a TODO.) 2. Call find_importers. WIRED if any found, else ORPHAN. + On ReachabilityError (real grep/subprocess failure): return ERROR verdict. + + Verdict contract (fail CLOSED): + WIRED → PASS (imported by at least one non-test file) + EXEMPT → PASS (declared reachable via scheme) + ORPHAN → BLOCKING (no non-test importer found) + ERROR → BLOCKING (subprocess failure; sweep result is untrustworthy) Returns a dict:: { - "verdict": "WIRED" | "ORPHAN" | "EXEMPT", + "verdict": "WIRED" | "ORPHAN" | "EXEMPT" | "ERROR", "module": str, "importers": [str, ...], "reachable_via": str | None, "exempt_reason": str | None, # scheme name if EXEMPT "lang": str, + "error": str | None, # only present for ERROR verdict } """ module = Path(module) @@ -265,21 +310,20 @@ def reachability_verdict( # Step 2: importer sweep. try: importers = find_importers(repo_root, module, lang) - except Exception as exc: + except ReachabilityError as exc: logger.warning( - "reachability_verdict: find_importers raised for %s: %s — treating as WIRED (conservative)", + "reachability_verdict: find_importers raised ReachabilityError for %s: %s — returning ERROR", module_str, exc, ) - importers = [] - # Conservative: we cannot confirm orphan → WIRED. return { - "verdict": "WIRED", + "verdict": "ERROR", "module": module_str, "importers": [], "reachable_via": reachable_via, "exempt_reason": None, "lang": lang, + "error": str(exc), } verdict = "WIRED" if importers else "ORPHAN" @@ -307,17 +351,26 @@ def sweep_task( - If tier in {spike, domain-model}: EXEMPT (not swept; reachability not required at this phase). - If task.entrypoint is truthy: EXEMPT (entrypoints are by definition reachable via the outside world). - Else (tier in {wired, live}): sweep new modules and compute per-module verdicts. - Task verdict = ORPHAN if ANY module is ORPHAN; else WIRED (EXEMPT modules don't make the task orphan). + Task verdict = ORPHAN if ANY module is ORPHAN. + Task verdict = ERROR if ANY module is ERROR (or if new_modules_for_task raises). + Else WIRED (EXEMPT modules don't make the task orphan). + + Verdict contract (fail CLOSED): + WIRED → PASS + EXEMPT → PASS + ORPHAN → BLOCKING + ERROR → BLOCKING (never silently pass on a broken environment) Returns:: { - "verdict": "WIRED" | "ORPHAN" | "EXEMPT", + "verdict": "WIRED" | "ORPHAN" | "EXEMPT" | "ERROR", "tier": str, "modules": [], "reason": str | None, # only for EXEMPT "checked_at": str (ISO-8601), "start_commit": str, + "error": str | None, # only for ERROR } """ repo_root = Path(repo_root) @@ -354,14 +407,15 @@ def sweep_task( lang = language_for_repo(repo_root) try: new_modules = new_modules_for_task(repo_root, start_commit) - except Exception as exc: - logger.warning("sweep_task: new_modules_for_task raised: %s — returning WIRED (conservative)", exc) + except ReachabilityError as exc: + logger.warning("sweep_task: new_modules_for_task raised ReachabilityError: %s — returning ERROR", exc) return { - "verdict": "WIRED", + "verdict": "ERROR", "tier": tier, "modules": [], "checked_at": checked_at, "start_commit": start_commit, + "error": str(exc), } reachable_via = task.get("reachableVia") @@ -377,20 +431,29 @@ def sweep_task( ) module_verdicts.append(v) - # Task is ORPHAN if ANY module is ORPHAN (EXEMPT modules don't contribute to orphan status). + # Task is ORPHAN if ANY module is ORPHAN; ERROR if ANY module is ERROR. + # Only WIRED if no module is ORPHAN or ERROR (EXEMPT modules don't contribute). task_verdict = "WIRED" + task_error: str | None = None for v in module_verdicts: if v["verdict"] == "ORPHAN": task_verdict = "ORPHAN" break + if v["verdict"] == "ERROR": + task_verdict = "ERROR" + task_error = v.get("error") + break - return { + result: dict = { "verdict": task_verdict, "tier": tier, "modules": module_verdicts, "checked_at": checked_at, "start_commit": start_commit, } + if task_error is not None: + result["error"] = task_error + return result # ─── Internal helpers ───────────────────────────────────────────────────────── @@ -415,12 +478,30 @@ def _py_import_patterns(module: Path) -> list[str]: def _ts_import_patterns(module: Path) -> list[str]: - """Build grep -E patterns that match TS/JS import/require for *module*.""" - stem = module.stem - # Match: from '...stem' | from "...stem" | require('...stem') | require("...stem") + """Build grep -E patterns that match TS/JS import/require for *module*. + + The stem is anchored at a path-component boundary so that stem='bar' does NOT + match './foobar'. The pattern requires that the character immediately before + the stem (within the quoted path) is a '/' — ensuring 'bar' is the last path + segment, not a suffix of a longer name. + + Examples (stem='bar'): + from './bar' ✓ '/' immediately before stem + from '../pkg/bar' ✓ '/' immediately before stem + from './foobar' ✗ no '/' immediately before 'bar' (preceded by 'o') + require('./bar') ✓ + require('./foobar') ✗ + + Correctness note: [^'"]*/{stem}['"] applied to './foobar' — the only '/' in + the quoted string is at position 1 (between '.' and 'f'), and '/foobar' does + not contain '/bar' as an anchored suffix starting after a '/'. grep backtracks + through all positions and finds no valid split, so './foobar' correctly does + NOT match. + """ + stem = re.escape(module.stem) return [ - rf"""from ['"][^'"]*{re.escape(stem)}['"]""", - rf"""require\(['"][^'"]*{re.escape(stem)}['"]\)""", + rf"""from ['"][^'"]*/{stem}['"]""", + rf"""require\(['"][^'"]*/{stem}['"]\)""", ] @@ -435,26 +516,39 @@ def _grep_patterns(repo_root: Path, patterns: list[str], glob: str) -> list[Path """Run grep -rEl for each pattern across files matching *glob* under *repo_root*. Returns a deduplicated list of repo-relative Paths of matching files. - On error, logs a warning and returns [] (conservative: caller decides what to do). + + Error policy (fail CLOSED): + - grep exit code 0 = matches found → normal. + - grep exit code 1 = no matches → normal, returns [] for this pattern. + - grep exit code >= 2 = real error (e.g. invalid regex, I/O error) → + RAISES ReachabilityError. The caller must propagate this upward so that + the sweep returns ERROR rather than a false WIRED/ORPHAN. """ found: set[str] = set() for pattern in patterns: result = subprocess.run( - ["grep", "-rEl", "--include", glob, pattern, "."], + ["grep", "-rEl", "--include", glob, + "--exclude-dir=tests", "--exclude-dir=__tests__", + pattern, "."], cwd=str(repo_root), capture_output=True, text=True, ) - if result.returncode not in (0, 1): - # rc=1 means "no match" — that is expected. Any other rc is an error. - logger.warning( - "_grep_patterns: grep returned rc=%d for pattern %r: %s", - result.returncode, - pattern, - result.stderr.strip(), - ) - # Conservative: do NOT abort; keep going with other patterns. + if result.returncode == 0: + # Matches found. + pass + elif result.returncode == 1: + # No matches — completely normal, not an error. continue + else: + # rc >= 2: real grep error. + msg = ( + f"_grep_patterns: grep returned rc={result.returncode} " + f"for pattern {pattern!r}: {result.stderr.strip()}" + ) + logger.warning(msg) + raise ReachabilityError(msg) + for line in result.stdout.splitlines(): line = line.strip() if line: diff --git a/tests/core/test_reachability.py b/tests/core/test_reachability.py index 2334cad..686407b 100644 --- a/tests/core/test_reachability.py +++ b/tests/core/test_reachability.py @@ -13,6 +13,7 @@ import pytest from prd_taskmaster.reachability import ( + ReachabilityError, find_importers, language_for_repo, new_modules_for_task, @@ -439,3 +440,235 @@ def test_reachable_via_scheme_exempts_module_in_wired_tier(self, tmp_path): # Module verdict is EXEMPT (cli scheme), so task verdict is WIRED (no ORPHAN modules). assert result["verdict"] == "WIRED" assert result["modules"][0]["verdict"] == "EXEMPT" + + +# ─── 7. Regression: fail-closed on git/grep errors ──────────────────────────── + +class TestFailClosed: + """These tests MUST fail against pre-fix code (which returned WIRED on errors). + + After the fix: + - new_modules_for_task raises ReachabilityError on git failure + - sweep_task returns ERROR (not WIRED) on git failure + - _grep_patterns raises ReachabilityError on grep rc >= 2 + - grep rc=1 ("no matches") is NOT an error: produces ORPHAN, no exception + """ + + def test_git_error_raises_reachability_error(self, tmp_path): + """new_modules_for_task raises ReachabilityError when git fails. + + Using a non-git directory ensures git diff exits non-zero. + PRE-FIX BEHAVIOR: returned [] silently → caller saw WIRED (false pass). + POST-FIX BEHAVIOR: raises ReachabilityError. + """ + non_git_dir = tmp_path / "not_a_repo" + non_git_dir.mkdir() + # Also need a pyproject.toml so language detection doesn't bail early. + (non_git_dir / "pyproject.toml").write_text("[project]\n") + + with pytest.raises(ReachabilityError): + new_modules_for_task(non_git_dir, "abc1234", "HEAD") + + def test_sweep_task_git_error_returns_error_verdict(self, tmp_path): + """sweep_task returns ERROR verdict (not WIRED) when git fails. + + PRE-FIX BEHAVIOR: new_modules_for_task returned [] → no modules swept → + task_verdict defaulted to WIRED → silent false pass. + POST-FIX BEHAVIOR: ReachabilityError propagates → verdict = ERROR. + """ + non_git_dir = tmp_path / "not_a_repo" + non_git_dir.mkdir() + (non_git_dir / "pyproject.toml").write_text("[project]\n") + + task = {"tier": "wired"} + result = sweep_task(non_git_dir, task, "abc1234") + assert result["verdict"] == "ERROR", ( + f"Expected ERROR on git failure, got {result['verdict']!r}. " + "Pre-fix code would return WIRED (false pass)." + ) + assert "error" in result + + def test_sweep_task_bogus_commit_returns_error_verdict(self, tmp_path): + """sweep_task with a real git repo but a bogus commit SHA returns ERROR. + + PRE-FIX BEHAVIOR: swallowed git error → WIRED. + POST-FIX BEHAVIOR: ERROR verdict. + """ + repo = _init_repo(tmp_path) + (repo / "pyproject.toml").write_text("[project]\n") + _commit_all(repo, "init") + + task = {"tier": "wired"} + result = sweep_task(repo, task, "deadbeef00000000000000000000000000000000") + assert result["verdict"] == "ERROR", ( + f"Expected ERROR on bogus commit, got {result['verdict']!r}." + ) + + def test_grep_no_match_is_not_an_error_returns_orphan(self, tmp_path): + """A module with zero importers → ORPHAN verdict, no exception raised. + + grep exits with rc=1 ("no matches") — this is NORMAL and must NOT raise + ReachabilityError. The verdict is ORPHAN (blocking) but NOT ERROR. + + PRE-FIX BEHAVIOR: same (grep no-match was already handled) — this test + confirms the fix didn't accidentally break the no-match path. + """ + repo = _init_repo(tmp_path) + (repo / "pyproject.toml").write_text("[project]\n") + pkg = repo / "pkg" + pkg.mkdir() + (pkg / "__init__.py").write_text("") + (pkg / "isolated.py").write_text("# nothing imports this\n") + _commit_all(repo, "init") + + # Should not raise — grep rc=1 is not an error. + verdict = reachability_verdict(repo, Path("pkg/isolated.py"), "py") + assert verdict["verdict"] == "ORPHAN", ( + f"Expected ORPHAN for module with no importers, got {verdict['verdict']!r}." + ) + assert verdict["importers"] == [] + + +# ─── 8. Regression: nested test importer → ORPHAN ──────────────────────────── + +class TestNestedTestImporterIsOrphan: + """A module imported ONLY by a deeply-nested test must be ORPHAN. + + PRE-FIX BEHAVIOR: tests/core/test_foo.py was NOT in the exclude set + (only tests/test_foo.py was excluded) → returned as importer → WIRED (false pass). + POST-FIX BEHAVIOR: any file under tests/ or __tests__/ is excluded → ORPHAN. + """ + + def test_module_imported_only_by_nested_test_is_orphan(self, tmp_path): + repo = _init_repo(tmp_path) + (repo / "pyproject.toml").write_text("[project]\n") + pkg = repo / "pkg" + pkg.mkdir() + (pkg / "__init__.py").write_text("") + (pkg / "util.py").write_text("def compute(): return 42\n") + + # Nested test under tests/core/ — only importer. + tests_core = repo / "tests" / "core" + tests_core.mkdir(parents=True) + (tests_core / "__init__.py").write_text("") + (tests_core / "test_util.py").write_text( + "from pkg.util import compute\n" + "def test_compute():\n assert compute() == 42\n" + ) + _commit_all(repo, "add module + nested test") + + verdict = reachability_verdict(repo, Path("pkg/util.py"), "py") + assert verdict["verdict"] == "ORPHAN", ( + f"Expected ORPHAN — module imported only by tests/core/test_util.py, " + f"got {verdict['verdict']!r} with importers {verdict['importers']!r}. " + "Pre-fix code would return WIRED." + ) + assert verdict["importers"] == [] + + def test_module_imported_by_nested_test_and_real_code_is_wired(self, tmp_path): + """When both a nested test AND real production code import the module → WIRED.""" + repo = _init_repo(tmp_path) + (repo / "pyproject.toml").write_text("[project]\n") + pkg = repo / "pkg" + pkg.mkdir() + (pkg / "__init__.py").write_text("") + (pkg / "util.py").write_text("def compute(): return 42\n") + (pkg / "service.py").write_text("from pkg.util import compute\n") + + tests_core = repo / "tests" / "core" + tests_core.mkdir(parents=True) + (tests_core / "test_util.py").write_text("from pkg.util import compute\n") + _commit_all(repo, "add all") + + verdict = reachability_verdict(repo, Path("pkg/util.py"), "py") + assert verdict["verdict"] == "WIRED" + assert any("service.py" in imp for imp in verdict["importers"]) + # The nested test must NOT appear in importers. + assert not any("test_util" in imp for imp in verdict["importers"]) + + +# ─── 9. Regression: TS/JS substring → ORPHAN ───────────────────────────────── + +class TestTsSubstringIsOrphan: + """import x from './foobar' must NOT count as importing module 'bar'. + + PRE-FIX BEHAVIOR: _ts_import_patterns used `[^'"]*{stem}` without a path + boundary anchor → './foobar' matched stem 'bar' as a suffix → WIRED (false pass). + POST-FIX BEHAVIOR: pattern requires '/' immediately before stem → ORPHAN. + """ + + def _make_ts_repo(self, tmp_path: Path) -> tuple[Path, str]: + """Create a minimal TS git repo with src/bar.ts and a single TS file that + imports './foobar' (NOT './bar'). Returns (repo, head_sha).""" + repo = _init_repo(tmp_path) + (repo / "package.json").write_text('{"name": "test"}') + (repo / "tsconfig.json").write_text("{}") + src = repo / "src" + src.mkdir() + # The module under test. + (src / "bar.ts").write_text("export const bar = 42;\n") + # A file that imports './foobar' — should NOT count as importing 'bar'. + (src / "consumer.ts").write_text("import { foobar } from './foobar';\n") + # A file that correctly imports './bar'. + (src / "real_consumer.ts").write_text("import { bar } from './bar';\n") + head = _commit_all(repo, "add ts files") + return repo, head + + def test_foobar_import_does_not_wire_bar_module(self, tmp_path): + """src/consumer.ts does `import from './foobar'` — must NOT make src/bar.ts WIRED.""" + repo = _init_repo(tmp_path) + (repo / "package.json").write_text('{"name": "test"}') + (repo / "tsconfig.json").write_text("{}") + src = repo / "src" + src.mkdir() + (src / "bar.ts").write_text("export const bar = 42;\n") + # Only importer uses './foobar' — NOT './bar' + (src / "consumer.ts").write_text("import { something } from './foobar';\n") + _commit_all(repo, "add ts files") + + importers = find_importers(repo, Path("src/bar.ts"), "ts") + importer_paths = [p.as_posix() for p in importers] + assert "src/consumer.ts" not in importer_paths, ( + "src/consumer.ts imports './foobar', NOT './bar' — must not appear as importer. " + "Pre-fix code would include it (false WIRED)." + ) + + def test_real_bar_import_wires_bar_module(self, tmp_path): + """src/real_consumer.ts does `import from './bar'` — MUST make src/bar.ts WIRED.""" + repo = _init_repo(tmp_path) + (repo / "package.json").write_text('{"name": "test"}') + (repo / "tsconfig.json").write_text("{}") + src = repo / "src" + src.mkdir() + (src / "bar.ts").write_text("export const bar = 42;\n") + # A correct importer of './bar' + (src / "real_consumer.ts").write_text("import { bar } from '../src/bar';\n") + _commit_all(repo, "add ts files") + + importers = find_importers(repo, Path("src/bar.ts"), "ts") + importer_paths = [p.as_posix() for p in importers] + assert "src/real_consumer.ts" in importer_paths, ( + "src/real_consumer.ts imports '../src/bar' — must appear as importer." + ) + + def test_ts_substring_full_sweep_orphan(self, tmp_path): + """End-to-end: src/bar.ts imported only via './foobar' reference → ORPHAN. + + PRE-FIX: reachability_verdict would return WIRED (false pass). + POST-FIX: ORPHAN (correct blocking verdict). + """ + repo = _init_repo(tmp_path) + (repo / "package.json").write_text('{"name": "test"}') + (repo / "tsconfig.json").write_text("{}") + src = repo / "src" + src.mkdir() + (src / "bar.ts").write_text("export const bar = 42;\n") + (src / "consumer.ts").write_text("import { something } from './foobar';\n") + _commit_all(repo, "add ts files") + + verdict = reachability_verdict(repo, Path("src/bar.ts"), "ts") + assert verdict["verdict"] == "ORPHAN", ( + f"Expected ORPHAN — only importer uses './foobar' (not './bar'), " + f"got {verdict['verdict']!r} with importers {verdict['importers']!r}. " + "Pre-fix code would return WIRED." + ) From a7a0d6233a148cfa9673ecca1a60b5644d745801 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 16 Jun 2026 19:21:00 +0800 Subject: [PATCH 03/13] =?UTF-8?q?feat(engine):=20tier=20(spike|domain-mode?= =?UTF-8?q?l|wired|live)=20=E2=80=94=20deterministic=20default=20in=20enri?= =?UTF-8?q?ch=20+=20schema=20hint?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adds phaseConfig.tier to every enriched task, derived from the same keyword signals that drive complexity classification. Priority order: live (user-test / user validation checkpoint) > spike (research/investigate/evaluate stems) > wired (integrate/auth/api/deploy/database/cli/mcp/…) > domain-model (safe default). Key design decisions: - _classify_tier() is a standalone helper next to _classify_task(), testable in isolation. It uses two new prefix-stem regexes (_TIER_RESEARCH_RE, _WIRED_KEYWORDS) that drop the trailing \b so "investigate", "evaluate", "integrate" etc. are caught — existing exact-word regexes are not modified. - run_enrich_tasks() is idempotent in two ways: (a) a task with no phaseConfig gets a full phaseConfig including tier; (b) a task with phaseConfig but no tier (pre-feature enrichment) gets tier backfilled; (c) a pre-existing non-empty tier is preserved (honours LLM or manually-set values). - Default is domain-model — existing graphs never suddenly require reachability. - TASKS_SCHEMA_HINT in backend.py gains the tier field and a one-line rule. - 33 new tests in tests/core/test_tier.py; full suite 422 passed, 3 skipped. Co-Authored-By: Claude Opus 4.8 --- prd_taskmaster/backend.py | 6 +- prd_taskmaster/tasks.py | 71 +++++++++- tests/core/test_tier.py | 286 ++++++++++++++++++++++++++++++++++++++ 3 files changed, 361 insertions(+), 2 deletions(-) create mode 100644 tests/core/test_tier.py diff --git a/prd_taskmaster/backend.py b/prd_taskmaster/backend.py index 2d113a9..f2e8601 100644 --- a/prd_taskmaster/backend.py +++ b/prd_taskmaster/backend.py @@ -42,6 +42,7 @@ def rate(self, tag=None, research=True) -> dict: ... "status": "pending", "dependencies": [], "priority": "high", + "tier": "domain-model", "subtasks": [ { "id": 1, @@ -66,7 +67,10 @@ def rate(self, tag=None, research=True) -> dict: ... task must include id, title, description, details, testStrategy, status, dependencies, priority, and at least 2 subtasks; dependencies must reference existing task or sibling subtask IDs; use only priority high, medium, or low; -do not include placeholders, generic tasks, or empty testStrategy fields.""" +do not include placeholders, generic tasks, or empty testStrategy fields; +tier ∈ {spike|domain-model|wired|live}: the altitude of the claim — spike=research, +domain-model=pure logic, wired=integration, live=user-visible; wired/live require +reachability evidence (the deterministic enrich step will set this if omitted).""" PARALLEL_RESULT_SCHEMA_HINT = """{ diff --git a/prd_taskmaster/tasks.py b/prd_taskmaster/tasks.py index a8466c2..fdb6cf3 100644 --- a/prd_taskmaster/tasks.py +++ b/prd_taskmaster/tasks.py @@ -121,6 +121,27 @@ def cmd_backup_prd(args: argparse.Namespace) -> None: re.IGNORECASE, ) +# Tier-specific regexes — stem-matching (prefix \b, no trailing \b) so that +# "integrate", "investigate", "evaluate" etc. are caught by their root stems. +# These are intentionally separate from _RESEARCH_KEYWORDS / _COMPLEX_KEYWORDS +# (which use \b…\b exact-word matching and must not be changed). +# +# _TIER_RESEARCH_RE: overlaps with _RESEARCH_KEYWORDS but uses prefix \b so +# "investigate", "evaluate", "analyze" are reliably caught. +_TIER_RESEARCH_RE = re.compile( + r'\b(research|investigat|analyz|explor|evaluat|assess|discover|' + r'benchmark|audit|review|spike|poc|proof.of.concept)', + re.IGNORECASE, +) + +# _WIRED_KEYWORDS: integration / API / CLI signals that require reachability. +# Prefix \b with no trailing \b so "integrate", "authenticate", etc. match. +_WIRED_KEYWORDS = re.compile( + r'\b(integrat|auth|webhook|deploy|pipeline|api|endpoint|route|' + r'wire|connect|cli|mcp|database|migration)', + re.IGNORECASE, +) + # Lifecycle phase assignments by complexity _LIFECYCLE_MAP = { "SIMPLE": ["implementation", "testing"], @@ -131,6 +152,46 @@ def cmd_backup_prd(args: argparse.Namespace) -> None: } +def _classify_tier(task: dict) -> str: + """Deterministically assign an altitude tier from keyword signals. + + Mapping (in priority order): + VALIDATION / "user-test" / "user validation checkpoint" → live + RESEARCH keywords → spike + WIRED keywords (integration signals) → wired + else → domain-model (safe default) + + The default (domain-model) means existing task graphs that have no + integration/research/validation signal will NOT require reachability + evidence when the reachability gate is enforced later. + """ + title = task.get("title", "") or task.get("name", "") or "" + description = task.get("description", "") or "" + details = task.get("details", "") or "" + # Strip RESEARCH NOTES appended by the parallel enrichment pass — those + # notes should not influence tier any more than they influence complexity. + classification_details = re.split( + r'\n\s*RESEARCH NOTES\s*\(parallel pass\):', + details, + maxsplit=1, + flags=re.IGNORECASE, + )[0] + + combined = f"{title} {description} {classification_details}".lower() + + # Priority 1: user-visible / validation work → live + if "user-test" in combined or "user validation checkpoint" in title.lower(): + return "live" + # Priority 2: research/spike work → spike + if _TIER_RESEARCH_RE.search(combined): + return "spike" + # Priority 3: integration/wiring signals → wired + if _WIRED_KEYWORDS.search(combined): + return "wired" + # Default: pure logic / domain work → domain-model + return "domain-model" + + def _classify_task(task: dict) -> dict: """Derive phaseConfig for a single task dict.""" title = task.get("title", "") or task.get("name", "") @@ -168,6 +229,7 @@ def _classify_task(task: dict) -> dict: return { "complexity": complexity, + "tier": _classify_tier(task), "requiresCDD": requires_cdd, "requiresResearch": requires_research, "lifecycle": _LIFECYCLE_MAP[complexity], @@ -258,8 +320,15 @@ def run_enrich_tasks(input_path: str | None) -> dict: if not isinstance(task, dict): continue - # Skip if already enriched (idempotent) if "phaseConfig" in task: + # Idempotent back-fill: if an older enrich run wrote phaseConfig but + # did not include tier (pre-tier-feature), add it now rather than + # skipping the whole task. An explicit non-empty tier already present + # is left untouched (honours LLM-set or manually-set values). + pc = task["phaseConfig"] + if not pc.get("tier"): + pc["tier"] = _classify_tier(task) + enriched_count += 1 continue phase_config = _classify_task(task) diff --git a/tests/core/test_tier.py b/tests/core/test_tier.py new file mode 100644 index 0000000..486ceca --- /dev/null +++ b/tests/core/test_tier.py @@ -0,0 +1,286 @@ +"""Tests for deterministic tier classification (altitude field). + +Tier ∈ {spike, domain-model, wired, live}. +- spike = research / investigation +- domain-model = pure logic (safe default; no reachability required) +- wired = integration / API / CLI work (reachability required later) +- live = user-visible / validation (reachability required later) +""" + +import json + +import pytest + +from prd_taskmaster.tasks import _classify_tier, run_enrich_tasks +from prd_taskmaster.backend import TASKS_SCHEMA_HINT + + +# ─── _classify_tier unit tests ──────────────────────────────────────────────── + + +class TestClassifyTier: + """Unit tests for the _classify_tier helper.""" + + # spike ─────────────────────────────────────────────────────────────────── + + def test_research_keyword_in_title_gives_spike(self): + task = {"title": "Research the best auth framework", "description": ""} + assert _classify_tier(task) == "spike" + + def test_investigate_keyword_gives_spike(self): + task = {"title": "Investigate memory leak in parser", "description": ""} + assert _classify_tier(task) == "spike" + + def test_spike_keyword_gives_spike(self): + task = {"title": "Spike: evaluate caching strategies", "description": ""} + assert _classify_tier(task) == "spike" + + def test_evaluate_keyword_in_description_gives_spike(self): + task = {"title": "Auth options", "description": "Evaluate OAuth vs SAML trade-offs"} + assert _classify_tier(task) == "spike" + + def test_poc_keyword_gives_spike(self): + task = {"title": "PoC for streaming responses", "description": ""} + assert _classify_tier(task) == "spike" + + # live ──────────────────────────────────────────────────────────────────── + + def test_user_test_in_combined_gives_live(self): + task = {"title": "End-to-end user-test pass", "description": ""} + assert _classify_tier(task) == "live" + + def test_user_validation_checkpoint_in_title_gives_live(self): + task = {"title": "User Validation Checkpoint: checkout flow", "description": ""} + assert _classify_tier(task) == "live" + + def test_user_test_in_description_gives_live(self): + task = {"title": "QA sign-off", "description": "Perform full user-test of the onboarding flow"} + assert _classify_tier(task) == "live" + + # live takes priority over research keywords + def test_live_beats_research_when_both_present(self): + task = { + "title": "User Validation Checkpoint", + "description": "Evaluate and review the user flow", + } + assert _classify_tier(task) == "live" + + # wired ─────────────────────────────────────────────────────────────────── + + def test_wire_keyword_gives_wired(self): + task = {"title": "Wire authentication into the billing module", "description": ""} + assert _classify_tier(task) == "wired" + + def test_api_endpoint_gives_wired(self): + task = {"title": "Add /api/v1/users endpoint", "description": ""} + assert _classify_tier(task) == "wired" + + def test_connect_to_database_gives_wired(self): + task = {"title": "Connect the service to the database", "description": ""} + assert _classify_tier(task) == "wired" + + def test_integration_keyword_gives_wired(self): + task = {"title": "Integrate Stripe payment gateway", "description": ""} + assert _classify_tier(task) == "wired" + + def test_webhook_gives_wired(self): + task = {"title": "Handle incoming webhook from GitHub", "description": ""} + assert _classify_tier(task) == "wired" + + def test_deploy_gives_wired(self): + task = {"title": "Deploy service to production", "description": ""} + assert _classify_tier(task) == "wired" + + def test_mcp_keyword_gives_wired(self): + task = {"title": "Register new MCP tool in the router", "description": ""} + assert _classify_tier(task) == "wired" + + def test_cli_keyword_gives_wired(self): + task = {"title": "Add CLI subcommand for enrich-tasks", "description": ""} + assert _classify_tier(task) == "wired" + + def test_migration_keyword_gives_wired(self): + task = {"title": "Write migration for tasks schema", "description": ""} + assert _classify_tier(task) == "wired" + + def test_route_keyword_gives_wired(self): + task = {"title": "Add route for user profile page", "description": ""} + assert _classify_tier(task) == "wired" + + def test_pipeline_keyword_gives_wired(self): + task = {"title": "Build CI pipeline for staging", "description": ""} + assert _classify_tier(task) == "wired" + + # domain-model ──────────────────────────────────────────────────────────── + + def test_plain_logic_task_gives_domain_model(self): + task = {"title": "Implement the scoring function", "description": "Pure Python calculation."} + assert _classify_tier(task) == "domain-model" + + def test_empty_task_gives_domain_model(self): + task = {} + assert _classify_tier(task) == "domain-model" + + def test_generic_task_gives_domain_model(self): + task = {"title": "Add unit tests for the parser", "description": ""} + assert _classify_tier(task) == "domain-model" + + def test_name_field_used_as_fallback_for_title(self): + task = {"name": "Implement token counter", "description": ""} + assert _classify_tier(task) == "domain-model" + + # wired beats domain-model when integration keyword appears in details + def test_integration_signal_in_details_gives_wired(self): + task = { + "title": "Implement task storage", + "description": "Store tasks.", + "details": "Must connect to the database via SQLAlchemy.", + } + assert _classify_tier(task) == "wired" + + # research notes appended by parallel pass should NOT influence tier + def test_research_notes_section_ignored_for_tier(self): + task = { + "title": "Implement scoring function", + "description": "Pure calculation.", + "details": ( + "Standard implementation.\n" + "RESEARCH NOTES (parallel pass):\n" + "Integrate with the API endpoint via database migration." + ), + } + # The wired keywords appear only in the stripped RESEARCH NOTES section + assert _classify_tier(task) == "domain-model" + + +# ─── run_enrich_tasks integration tests ────────────────────────────────────── + + +class TestEnrichTasksTier: + """Integration tests for tier injection via run_enrich_tasks.""" + + def _make_tasks_file(self, tmp_path, tasks: list) -> str: + p = tmp_path / "tasks.json" + p.write_text(json.dumps({"tasks": tasks})) + return str(p) + + def _read_tasks(self, path: str) -> list: + return json.loads(open(path).read())["tasks"] + + def test_enrich_sets_tier_for_every_task(self, tmp_path): + tasks = [ + {"id": 1, "title": "Research auth options", "description": "", "subtasks": []}, + {"id": 2, "title": "Implement scoring function", "description": "", "subtasks": []}, + {"id": 3, "title": "Wire the API endpoint", "description": "", "subtasks": []}, + ] + path = self._make_tasks_file(tmp_path, tasks) + result = run_enrich_tasks(path) + assert result["ok"] is True + + written = self._read_tasks(path) + for task in written: + assert "phaseConfig" in task + assert "tier" in task["phaseConfig"], f"missing tier on task {task.get('id')}" + + def test_enrich_tier_matches_classifier_per_task(self, tmp_path): + tasks = [ + {"id": 1, "title": "Research auth options", "description": "", "subtasks": []}, + {"id": 2, "title": "Implement scoring function", "description": "", "subtasks": []}, + {"id": 3, "title": "Wire the API endpoint", "description": "", "subtasks": []}, + {"id": 4, "title": "User Validation Checkpoint", "description": "", "subtasks": []}, + ] + path = self._make_tasks_file(tmp_path, tasks) + run_enrich_tasks(path) + written = self._read_tasks(path) + + by_id = {t["id"]: t for t in written} + assert by_id[1]["phaseConfig"]["tier"] == "spike" + assert by_id[2]["phaseConfig"]["tier"] == "domain-model" + assert by_id[3]["phaseConfig"]["tier"] == "wired" + assert by_id[4]["phaseConfig"]["tier"] == "live" + + def test_enrich_is_idempotent_same_tiers_on_second_run(self, tmp_path): + tasks = [ + {"id": 1, "title": "Research auth options", "description": "", "subtasks": []}, + {"id": 2, "title": "Wire the API endpoint", "description": "", "subtasks": []}, + ] + path = self._make_tasks_file(tmp_path, tasks) + run_enrich_tasks(path) + tiers_after_first = { + t["id"]: t["phaseConfig"]["tier"] + for t in self._read_tasks(path) + } + + run_enrich_tasks(path) + tiers_after_second = { + t["id"]: t["phaseConfig"]["tier"] + for t in self._read_tasks(path) + } + + assert tiers_after_first == tiers_after_second + + def test_plain_task_defaults_to_domain_model(self, tmp_path): + tasks = [{"id": 1, "title": "Implement the parser", "description": "", "subtasks": []}] + path = self._make_tasks_file(tmp_path, tasks) + run_enrich_tasks(path) + written = self._read_tasks(path) + assert written[0]["phaseConfig"]["tier"] == "domain-model" + + def test_pre_existing_phaseconfig_without_tier_gets_backfilled(self, tmp_path): + """Tasks enriched before the tier feature get tier added on re-run.""" + tasks = [ + { + "id": 1, + "title": "Wire auth to billing", + "description": "", + "subtasks": [], + "phaseConfig": { + "complexity": "COMPLEX", + "requiresCDD": True, + "requiresResearch": True, + "lifecycle": ["planning", "implementation", "testing", "review"], + "cddCardId": None, + "acceptanceCriteria": [], + # no "tier" key — simulates pre-feature enrichment + }, + } + ] + path = self._make_tasks_file(tmp_path, tasks) + result = run_enrich_tasks(path) + assert result["ok"] is True + written = self._read_tasks(path) + assert written[0]["phaseConfig"]["tier"] == "wired" + + def test_pre_existing_explicit_tier_not_overwritten(self, tmp_path): + """An already-present non-empty tier is preserved (honours LLM or manual value).""" + tasks = [ + { + "id": 1, + "title": "Wire auth to billing", + "description": "", + "subtasks": [], + "phaseConfig": { + "complexity": "COMPLEX", + "tier": "live", # explicitly set (e.g. by LLM) + "requiresCDD": True, + "requiresResearch": True, + "lifecycle": [], + "cddCardId": None, + "acceptanceCriteria": [], + }, + } + ] + path = self._make_tasks_file(tmp_path, tasks) + run_enrich_tasks(path) + written = self._read_tasks(path) + # "live" is kept even though the keywords would normally give "wired" + assert written[0]["phaseConfig"]["tier"] == "live" + + +# ─── Schema hint test ───────────────────────────────────────────────────────── + + +def test_tasks_schema_hint_contains_tier(): + assert '"tier"' in TASKS_SCHEMA_HINT or "'tier'" in TASKS_SCHEMA_HINT + # Also confirm the four valid values are documented + assert "domain-model" in TASKS_SCHEMA_HINT From 6f4dda0ffeb1bbe6e5569adc44269e6553ddb9cc Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 16 Jun 2026 19:33:38 +0800 Subject: [PATCH 04/13] fix(engine): tighten tier keyword anchoring (cli/auth/wire collisions; drop review/assess from spike) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - cli: \bcli(?!ent|mat|nic) — excludes client/climate/clinic/clinical; CLI, cli-based, cli entrypoint still fire - auth: \bauthenticat|\bauthoriz|\bauth\b — excludes author/authority/authentic; auth token, authenticate still fire - wire: \bwire(?!frame|less) — excludes wireframe/wireless; wiring/wire X into Y still fire - api: anchored to \bapi\b (word-boundary both sides) — confirmed no collision with rapid/therapist - _TIER_RESEARCH_RE: drop review and assess (high-frequency innocent words); keep audit, spike, investigate, evaluate, etc. - 12 negative regression tests added to tests/core/test_tier.py (each fails against pre-fix code) - All 45 tier tests green; full suite 434 passed 3 skipped Co-Authored-By: Claude Opus 4.8 --- prd_taskmaster/tasks.py | 17 ++++++++--- tests/core/test_tier.py | 67 +++++++++++++++++++++++++++++++++++++++++ 2 files changed, 79 insertions(+), 5 deletions(-) diff --git a/prd_taskmaster/tasks.py b/prd_taskmaster/tasks.py index fdb6cf3..3f8e39f 100644 --- a/prd_taskmaster/tasks.py +++ b/prd_taskmaster/tasks.py @@ -129,16 +129,23 @@ def cmd_backup_prd(args: argparse.Namespace) -> None: # _TIER_RESEARCH_RE: overlaps with _RESEARCH_KEYWORDS but uses prefix \b so # "investigate", "evaluate", "analyze" are reliably caught. _TIER_RESEARCH_RE = re.compile( - r'\b(research|investigat|analyz|explor|evaluat|assess|discover|' - r'benchmark|audit|review|spike|poc|proof.of.concept)', + r'\b(research|investigat|analyz|explor|evaluat|discover|' + r'benchmark|audit|spike|poc|proof.of.concept)', re.IGNORECASE, ) # _WIRED_KEYWORDS: integration / API / CLI signals that require reachability. -# Prefix \b with no trailing \b so "integrate", "authenticate", etc. match. +# Anchored carefully to avoid false-positives: +# cli — \bcli(?!ent|mat|nic) excludes client/climate/clinic/clinical +# auth — authenticat|authoriz|\bauth\b excludes author/authority/authentic +# wire — wire(?!frame|less) excludes wireframe/wireless +# Other stems (integrat, webhook, deploy, pipeline, api, endpoint, route, +# connect, mcp, database, migration) retain prefix \b; no trailing collisions +# identified for these. _WIRED_KEYWORDS = re.compile( - r'\b(integrat|auth|webhook|deploy|pipeline|api|endpoint|route|' - r'wire|connect|cli|mcp|database|migration)', + r'(\bintegrat|\bauthenticat|\bauthoriz|\bauth\b|\bwebhook|\bdeploy|' + r'\bpipeline|\bapi\b|\bendpoint|\broute|\bwire(?!frame|less)|\bconnect|' + r'\bcli(?!ent|mat|nic)|\bmcp\b|\bdatabase|\bmigration)', re.IGNORECASE, ) diff --git a/tests/core/test_tier.py b/tests/core/test_tier.py index 486ceca..21050de 100644 --- a/tests/core/test_tier.py +++ b/tests/core/test_tier.py @@ -129,6 +129,73 @@ def test_name_field_used_as_fallback_for_title(self): task = {"name": "Implement token counter", "description": ""} assert _classify_tier(task) == "domain-model" + # ── Negative regression tests: keyword over-matching ───────────────────── + # Each of these MUST classify as domain-model, not wired or spike. + # They fail against the pre-fix regexes (cli/auth/wire stem collisions, + # review/assess in _TIER_RESEARCH_RE). + + def test_client_side_does_not_give_wired(self): + """'client' must not match the 'cli' wired stem.""" + task = {"title": "Build a client-side validation rules module", "description": ""} + assert _classify_tier(task) == "domain-model" + + def test_client_validation_does_not_give_wired(self): + """'client' (standalone) must not match the 'cli' wired stem.""" + task = {"title": "Client validation module", "description": ""} + assert _classify_tier(task) == "domain-model" + + def test_wireframe_does_not_give_wired(self): + """'wireframe' must not match the 'wire' wired stem.""" + task = {"title": "Create wireframes for the dashboard", "description": ""} + assert _classify_tier(task) == "domain-model" + + def test_author_does_not_give_wired(self): + """'author' must not match the 'auth' wired stem.""" + task = {"title": "Author bio component", "description": ""} + assert _classify_tier(task) == "domain-model" + + def test_authority_does_not_give_wired(self): + """'authority' must not match the 'auth' wired stem.""" + task = {"title": "Model the authority hierarchy", "description": ""} + assert _classify_tier(task) == "domain-model" + + def test_code_review_does_not_give_spike(self): + """'review' in a task title must not trigger the spike tier.""" + task = {"title": "Code review the scoring PR", "description": ""} + assert _classify_tier(task) != "spike" + + def test_assess_alone_does_not_give_spike(self): + """'assess' alone must not trigger the spike tier (dropped from _TIER_RESEARCH_RE).""" + task = {"title": "Assess the risk level of the change", "description": ""} + assert _classify_tier(task) != "spike" + + # Confirm genuine wired / spike cases still classify correctly (no over-correction) + + def test_wire_scorer_into_cli_still_gives_wired(self): + """'Wire … CLI' — both 'wire' and 'cli' must still match as wired.""" + task = {"title": "Wire the scorer into the CLI", "description": ""} + assert _classify_tier(task) == "wired" + + def test_cli_entrypoint_still_gives_wired(self): + """'cli' as a bare token (not followed by ent/mat/nic) must still match.""" + task = {"title": "Set up the cli entrypoint", "description": ""} + assert _classify_tier(task) == "wired" + + def test_authenticate_via_oauth_still_gives_wired(self): + """'authenticate' must still match as a wired integration signal.""" + task = {"title": "Authenticate users via OAuth", "description": ""} + assert _classify_tier(task) == "wired" + + def test_auth_token_still_gives_wired(self): + """'\bauth\b' (bare word) must still fire as wired.""" + task = {"title": "Rotate the auth token on each request", "description": ""} + assert _classify_tier(task) == "wired" + + def test_investigate_still_gives_spike(self): + """'investigate' must remain a spike trigger despite 'review'/'assess' removal.""" + task = {"title": "Investigate caching options for the feed", "description": ""} + assert _classify_tier(task) == "spike" + # wired beats domain-model when integration keyword appears in details def test_integration_signal_in_details_gives_wired(self): task = { From bd7efce5f872a62ea8791fa9157576b3dd9f5b5b Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 16 Jun 2026 19:42:37 +0800 Subject: [PATCH 05/13] =?UTF-8?q?feat(engine):=20ship-check=20Gate=206=20?= =?UTF-8?q?=E2=80=94=20block=20done=20wired/live=20tasks=20whose=20CDD=20r?= =?UTF-8?q?eachability=20verdict=20is=20ORPHAN/ERROR/missing?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adds gate_reachability() to both skel/ship-check.py (stdlib-only, binding) and prd_taskmaster/shipcheck.py (display heuristic), wired into run_all_gates after Gate 5 (oracle). Reads the 'reachability' block from each task's CDD card; fail-closed on missing card, missing block, ORPHAN, ERROR, or unknown verdict for wired/live-tier done tasks. domain-model/spike/untiered and non-done tasks skipped. 31 new tests in tests/core/test_ship_check_reachability.py; full suite 465 passed. Co-Authored-By: Claude Opus 4.8 --- prd_taskmaster/shipcheck.py | 99 +++++ skel/ship-check.py | 100 +++++ tests/core/test_ship_check_reachability.py | 449 +++++++++++++++++++++ 3 files changed, 648 insertions(+) create mode 100644 tests/core/test_ship_check_reachability.py diff --git a/prd_taskmaster/shipcheck.py b/prd_taskmaster/shipcheck.py index 3c9791e..821f142 100644 --- a/prd_taskmaster/shipcheck.py +++ b/prd_taskmaster/shipcheck.py @@ -135,6 +135,102 @@ def gate_plan(repo_root: Path) -> Tuple[bool, List[str]]: return False, ["no plan file at .taskmaster/docs/plan.md or docs/superpowers/plans/*.md"] +def _card_path_for(cdd_dir: Path, tid) -> "Path | None": + """Return the Path to the CDD card for task , or None if absent. + + Mirrors the _has_card_for existence check but returns the actual path so + callers can read the card contents. Prefers the direct task-.json; + falls back to the first combined card whose hyphen-separated id-list + contains . + """ + tid_str = str(tid) + direct = cdd_dir / f"task-{tid_str}.json" + if direct.exists(): + return direct + for card in cdd_dir.glob("task-*.json"): + stem = card.stem + if not stem.startswith("task-"): + continue + ids = stem[len("task-"):].split("-") + if tid_str in ids: + return card + return None + + +def gate_reachability(repo_root: Path, tasks: list) -> Tuple[bool, List[str]]: + """Gate 6 — block done wired/live tasks whose recorded reachability verdict + is ORPHAN, ERROR, or absent. + + Reads the per-task verdict from the CDD card's 'reachability' block (written + by execute-task step RA6). Does NOT re-execute the reachability sweep. + FAIL-CLOSED on missing/unknown verdicts for wired/live done tasks. + + Tiers that require a reachability check: 'wired', 'live'. + All other tiers (spike, domain-model, unset) are skipped. + Non-done tasks are also skipped. + """ + failures: List[str] = [] + cdd_dir = repo_root / ".atlas-ai" / "cdd" + _REQUIRED_TIERS = {"wired", "live"} + _PASS_VERDICTS = {"WIRED", "EXEMPT"} + _FAIL_VERDICTS = {"ORPHAN", "ERROR"} + + for t in tasks: + if t.get("status") != "done": + continue + tier = ( + t.get("phaseConfig", {}).get("tier") + or t.get("tier") + or "domain-model" + ) + if tier not in _REQUIRED_TIERS: + continue + + tid = t.get("id") + card_path = _card_path_for(cdd_dir, tid) + if card_path is None: + failures.append( + f"task {tid}: tier={tier} requires reachability but no CDD card found" + ) + continue + + try: + card = json.loads(card_path.read_text()) + except (OSError, json.JSONDecodeError) as exc: + failures.append( + f"task {tid}: tier={tier} — cannot read CDD card ({exc})" + ) + continue + + reach = card.get("reachability") + if not reach: + failures.append( + f"task {tid}: tier={tier} requires a reachability check, but its CDD card has" + f" no 'reachability' block — run the execute-task reachability sweep, then wire" + f" it or re-status deferred/scaffold" + ) + continue + + verdict = reach.get("verdict") if isinstance(reach, dict) else None + if verdict in _PASS_VERDICTS: + continue + elif verdict in _FAIL_VERDICTS: + modules = reach.get("modules", []) if isinstance(reach, dict) else [] + mod_str = f" ({', '.join(modules)})" if modules else "" + failures.append( + f"task {tid}: reachability {verdict}{mod_str} — wire the module(s)" + f" into the running system or re-status deferred/scaffold" + ) + else: + # Unknown / garbage verdict — fail-closed. + failures.append( + f"task {tid}: tier={tier} — unknown reachability verdict {verdict!r};" + f" expected WIRED, EXEMPT, ORPHAN, or ERROR" + ) + + return len(failures) == 0, failures + + def gate_exit_codes(atlas: Path) -> Tuple[bool, List[str]]: failures: List[str] = [] evidence_dir = atlas / "evidence" @@ -174,6 +270,9 @@ def run_all_gates(repo_root: Path) -> Tuple[bool, List[str]]: _, f3 = gate_cdd(atlas, tasks) failures.extend(f3) + _, f6 = gate_reachability(repo_root, tasks) + failures.extend(f6) + _, f4 = gate_plan(repo_root) failures.extend(f4) diff --git a/skel/ship-check.py b/skel/ship-check.py index e5103bf..f7ed5b7 100755 --- a/skel/ship-check.py +++ b/skel/ship-check.py @@ -172,6 +172,103 @@ def _head_commit(repo_root: Path) -> str: return sha or "UNKNOWN" +def _card_path_for(cdd_dir: Path, tid) -> "Path | None": + """Return the Path to the CDD card for task , or None if absent. + + Mirrors the _has_card_for existence check but returns the actual path so + callers can read the card contents. Prefers the direct task-.json; + falls back to the first combined card whose hyphen-separated id-list + contains . + """ + tid_str = str(tid) + direct = cdd_dir / f"task-{tid_str}.json" + if direct.exists(): + return direct + for card in cdd_dir.glob("task-*.json"): + stem = card.stem + if not stem.startswith("task-"): + continue + ids = stem[len("task-"):].split("-") + if tid_str in ids: + return card + return None + + +def gate_reachability(repo_root: Path, tasks: list) -> Tuple[bool, List[str]]: + """Gate 6 — block done wired/live tasks whose recorded reachability verdict + is ORPHAN, ERROR, or absent. + + Reads the per-task verdict from the CDD card's 'reachability' block (written + by execute-task step RA6). Does NOT re-execute the reachability sweep — the + standalone skel must stay stdlib-only. FAIL-CLOSED on missing/unknown verdicts + for wired/live done tasks. + + Tiers that require a reachability check: 'wired', 'live'. + All other tiers (spike, domain-model, unset) are skipped. + Non-done tasks are also skipped. + """ + failures: List[str] = [] + cdd_dir = repo_root / ".atlas-ai" / "cdd" + _REQUIRED_TIERS = {"wired", "live"} + _PASS_VERDICTS = {"WIRED", "EXEMPT"} + _FAIL_VERDICTS = {"ORPHAN", "ERROR"} + + for t in tasks: + if t.get("status") != "done": + continue + tier = ( + t.get("phaseConfig", {}).get("tier") + or t.get("tier") + or "domain-model" + ) + if tier not in _REQUIRED_TIERS: + continue + + tid = t.get("id") + card_path = _card_path_for(cdd_dir, tid) + if card_path is None: + failures.append( + f"task {tid}: tier={tier} requires reachability but no CDD card found" + ) + continue + + try: + card = json.loads(card_path.read_text()) + except (OSError, json.JSONDecodeError) as exc: + failures.append( + f"task {tid}: tier={tier} — cannot read CDD card ({exc})" + ) + continue + + reach = card.get("reachability") + if not reach: + failures.append( + f"task {tid}: tier={tier} requires a reachability check, but its CDD card has" + f" no 'reachability' block — run the execute-task reachability sweep, then wire" + f" it or re-status deferred/scaffold" + ) + continue + + verdict = reach.get("verdict") if isinstance(reach, dict) else None + if verdict in _PASS_VERDICTS: + continue + elif verdict in _FAIL_VERDICTS: + modules = reach.get("modules", []) if isinstance(reach, dict) else [] + mod_str = f" ({', '.join(modules)})" if modules else "" + failures.append( + f"task {tid}: reachability {verdict}{mod_str} — wire the module(s)" + f" into the running system or re-status deferred/scaffold" + ) + else: + # Unknown / garbage verdict — fail-closed. + failures.append( + f"task {tid}: tier={tier} — unknown reachability verdict {verdict!r};" + f" expected WIRED, EXEMPT, ORPHAN, or ERROR" + ) + + return len(failures) == 0, failures + + def gate_oracle(repo_root: Path, tasks: list, head_commit: str) -> Tuple[bool, List[str]]: """Re-grade every DONE task via the atlas oracle CLI. FAIL-CLOSED. @@ -256,6 +353,9 @@ def run_all_gates(repo_root: Path) -> Tuple[bool, List[str]]: _, f5 = gate_oracle(repo_root, tasks, head) failures.extend(f5) + _, f6 = gate_reachability(repo_root, tasks) + failures.extend(f6) + _, f4 = gate_plan(repo_root) failures.extend(f4) diff --git a/tests/core/test_ship_check_reachability.py b/tests/core/test_ship_check_reachability.py new file mode 100644 index 0000000..727a39c --- /dev/null +++ b/tests/core/test_ship_check_reachability.py @@ -0,0 +1,449 @@ +"""Gate 6 — reachability contract for skel/ship-check.py. + +Gate 6 reads the 'reachability' block recorded in each task's CDD card +(written by execute-task RA6). It does NOT re-execute the sweep; the +standalone skel stays stdlib-only. + +Contract: + * done + tier in {wired, live} + no CDD card → FAIL-CLOSED block + * done + tier in {wired, live} + card has no reach key → FAIL-CLOSED block + * done + tier in {wired, live} + verdict ORPHAN → block + * done + tier in {wired, live} + verdict ERROR → block + * done + tier in {wired, live} + verdict WIRED/EXEMPT → pass + * done + tier domain-model (any reachability or none) → skipped (pass) + * status != done (wired tier, no reachability) → skipped (pass) + +The oracle gate (Gate 5) must also be satisfied for run_all_gates to pass in +the subprocess tests; we wire ATLAS_ORACLE_CMD to a fake PASS oracle. +""" +from __future__ import annotations + +import importlib.util +import json +import os +import stat +import subprocess +import sys +import types +from pathlib import Path + +import pytest + +REPO_ROOT = Path(__file__).resolve().parents[2] +SHIP = REPO_ROOT / "skel" / "ship-check.py" + + +# ─── importlib loader (mirrors test_ship_check_oracle.py) ──────────────────── + + +def _load(): + spec = importlib.util.spec_from_file_location("ship_check_mod", SHIP) + mod = importlib.util.module_from_spec(spec) + prev = sys.dont_write_bytecode + sys.dont_write_bytecode = True + try: + spec.loader.exec_module(mod) + finally: + sys.dont_write_bytecode = prev + return mod + + +# ─── fake subprocess.run factory (reuses oracle-test pattern) ──────────────── + + +def _fake_run_oracle_pass(cmd, *args, **kwargs): + """Stub subprocess.run: rev-parse → deadbeef; oracle grade → PASS.""" + joined = " ".join(str(c) for c in cmd) if isinstance(cmd, (list, tuple)) else str(cmd) + if "rev-parse" in joined: + return types.SimpleNamespace(stdout="deadbeef\n", stderr="", returncode=0) + if "oracle" in joined and "grade" in joined: + return types.SimpleNamespace(stdout='{"verdict":"PASS"}', stderr="", returncode=0) + raise AssertionError(f"unexpected subprocess command: {joined}") + + +# ─── project scaffolding helpers ───────────────────────────────────────────── + + +def _setup_base(tmp_path: Path, tasks: list) -> Path: + """Write Gates 1-4 green with the given task list. + + Each entry in ``tasks`` is a dict with at least 'id' and 'status'. + CDD cards are created for every task whose status == 'done', with a + grading block so Gate 5 (oracle) can run. No reachability block is added + by default — individual tests add it as needed. + """ + atlas = tmp_path / ".atlas-ai" + (atlas / "state").mkdir(parents=True) + (atlas / "state" / "pipeline.json").write_text( + json.dumps({"current_phase": "EXECUTE"}) + ) + tm = tmp_path / ".taskmaster" / "tasks" + tm.mkdir(parents=True) + (tm / "tasks.json").write_text(json.dumps({"master": {"tasks": tasks}})) + docs = tmp_path / ".taskmaster" / "docs" + docs.mkdir(parents=True) + (docs / "plan.md").write_text("# Plan\n") + cdd = atlas / "cdd" + cdd.mkdir(parents=True) + for t in tasks: + if t.get("status") == "done": + card: dict = {"id": t["id"], "grading": {"command": ["sh", "grade.sh"]}} + (cdd / f"task-{t['id']}.json").write_text(json.dumps(card)) + return atlas + + +def _add_reachability(atlas: Path, tid, verdict: str, modules: list | None = None) -> None: + """Patch an existing CDD card to include a reachability block.""" + card_path = atlas / "cdd" / f"task-{tid}.json" + card = json.loads(card_path.read_text()) + reach: dict = {"verdict": verdict} + if modules is not None: + reach["modules"] = modules + card["reachability"] = reach + card_path.write_text(json.dumps(card)) + + +# ─── fake oracle shell script for subprocess tests ─────────────────────────── + + +def _fake_oracle_cmd(tmp_path: Path) -> str: + """Return ATLAS_ORACLE_CMD pointing at a tiny shell script that emits PASS.""" + script = tmp_path / "fake_atlas.sh" + script.write_text( + "#!/bin/sh\n" + 'for a in "$@"; do\n' + ' if [ "$a" = "grade" ]; then\n' + ' printf \'{"verdict":"PASS"}\'\n' + " exit 0\n" + " fi\n" + "done\n" + "exit 0\n" + ) + script.chmod(script.stat().st_mode | stat.S_IXUSR | stat.S_IXGRP | stat.S_IXOTH) + return f"sh {script}" + + +def _run_subprocess(tmp_path: Path, *extra: str) -> subprocess.CompletedProcess: + env = dict(os.environ) + env["ATLAS_ORACLE_CMD"] = _fake_oracle_cmd(tmp_path) + return subprocess.run( + ["python3", str(SHIP), *extra], + cwd=str(tmp_path), + capture_output=True, + text=True, + check=False, + env=env, + ) + + +# ═══════════════════════════════════════════════════════════════════════════════ +# Unit tests for gate_reachability directly +# ═══════════════════════════════════════════════════════════════════════════════ + + +class TestGateReachabilityUnit: + """Direct unit tests — bypass run_all_gates to isolate Gate 6.""" + + def test_wired_task_with_wired_verdict_passes(self, tmp_path): + tasks = [{"id": 1, "status": "done", "tier": "wired"}] + atlas = _setup_base(tmp_path, tasks) + _add_reachability(atlas, 1, "WIRED") + mod = _load() + ok, failures = mod.gate_reachability(tmp_path, tasks) + assert ok is True, failures + assert failures == [] + + def test_wired_task_with_exempt_verdict_passes(self, tmp_path): + tasks = [{"id": 2, "status": "done", "tier": "wired"}] + atlas = _setup_base(tmp_path, tasks) + _add_reachability(atlas, 2, "EXEMPT") + mod = _load() + ok, failures = mod.gate_reachability(tmp_path, tasks) + assert ok is True, failures + + def test_wired_task_with_orphan_verdict_blocks(self, tmp_path): + tasks = [{"id": 3, "status": "done", "tier": "wired"}] + atlas = _setup_base(tmp_path, tasks) + _add_reachability(atlas, 3, "ORPHAN", modules=["myapp.core"]) + mod = _load() + ok, failures = mod.gate_reachability(tmp_path, tasks) + assert ok is False + assert any("3" in f and "ORPHAN" in f for f in failures), failures + + def test_wired_task_with_error_verdict_blocks(self, tmp_path): + tasks = [{"id": 4, "status": "done", "tier": "wired"}] + atlas = _setup_base(tmp_path, tasks) + _add_reachability(atlas, 4, "ERROR") + mod = _load() + ok, failures = mod.gate_reachability(tmp_path, tasks) + assert ok is False + assert any("4" in f and "ERROR" in f for f in failures), failures + + def test_wired_task_missing_reachability_key_blocks(self, tmp_path): + """No 'reachability' key in the CDD card → fail-closed.""" + tasks = [{"id": 5, "status": "done", "tier": "wired"}] + _setup_base(tmp_path, tasks) # card has grading but no reachability + mod = _load() + ok, failures = mod.gate_reachability(tmp_path, tasks) + assert ok is False + assert any("5" in f and "reachability" in f for f in failures), failures + + def test_wired_task_no_cdd_card_blocks(self, tmp_path): + """No CDD card at all for a wired done task → fail-closed.""" + # Don't call _setup_base; build minimum structure manually. + atlas = tmp_path / ".atlas-ai" + (atlas / "cdd").mkdir(parents=True) + tasks = [{"id": 6, "status": "done", "tier": "wired"}] + mod = _load() + ok, failures = mod.gate_reachability(tmp_path, tasks) + assert ok is False + assert any("6" in f and "no CDD card" in f for f in failures), failures + + def test_live_task_with_wired_verdict_passes(self, tmp_path): + tasks = [{"id": 7, "status": "done", "tier": "live"}] + atlas = _setup_base(tmp_path, tasks) + _add_reachability(atlas, 7, "WIRED") + mod = _load() + ok, failures = mod.gate_reachability(tmp_path, tasks) + assert ok is True, failures + + def test_live_task_with_orphan_verdict_blocks(self, tmp_path): + tasks = [{"id": 8, "status": "done", "tier": "live"}] + atlas = _setup_base(tmp_path, tasks) + _add_reachability(atlas, 8, "ORPHAN") + mod = _load() + ok, failures = mod.gate_reachability(tmp_path, tasks) + assert ok is False + assert any("8" in f and "ORPHAN" in f for f in failures), failures + + def test_domain_model_task_ignored_even_with_orphan(self, tmp_path): + """domain-model tier: Gate 6 skips entirely — ORPHAN is not a blocker.""" + tasks = [{"id": 9, "status": "done", "tier": "domain-model"}] + atlas = _setup_base(tmp_path, tasks) + _add_reachability(atlas, 9, "ORPHAN") + mod = _load() + ok, failures = mod.gate_reachability(tmp_path, tasks) + assert ok is True, failures + assert failures == [] + + def test_domain_model_task_ignored_with_no_reachability(self, tmp_path): + """domain-model tier + no reachability block: skipped, no failure.""" + tasks = [{"id": 10, "status": "done", "tier": "domain-model"}] + _setup_base(tmp_path, tasks) + mod = _load() + ok, failures = mod.gate_reachability(tmp_path, tasks) + assert ok is True, failures + + def test_spike_task_ignored(self, tmp_path): + tasks = [{"id": 11, "status": "done", "tier": "spike"}] + _setup_base(tmp_path, tasks) + mod = _load() + ok, failures = mod.gate_reachability(tmp_path, tasks) + assert ok is True, failures + + def test_untiered_task_ignored(self, tmp_path): + """A task with no tier field defaults to domain-model → skipped.""" + tasks = [{"id": 12, "status": "done"}] + _setup_base(tmp_path, tasks) + mod = _load() + ok, failures = mod.gate_reachability(tmp_path, tasks) + assert ok is True, failures + + def test_non_done_wired_task_ignored(self, tmp_path): + """pending wired task: Gate 6 skips non-done tasks.""" + tasks = [{"id": 13, "status": "pending", "tier": "wired"}] + _setup_base(tmp_path, tasks) # no card written for non-done + mod = _load() + ok, failures = mod.gate_reachability(tmp_path, tasks) + assert ok is True, failures + + def test_unknown_verdict_blocks_fail_closed(self, tmp_path): + """A verdict string that is not WIRED/EXEMPT/ORPHAN/ERROR → fail-closed.""" + tasks = [{"id": 14, "status": "done", "tier": "wired"}] + atlas = _setup_base(tmp_path, tasks) + _add_reachability(atlas, 14, "UNKNOWN_GARBAGE") + mod = _load() + ok, failures = mod.gate_reachability(tmp_path, tasks) + assert ok is False + assert any("14" in f for f in failures), failures + + def test_phaseconfig_tier_respected(self, tmp_path): + """tier nested in phaseConfig.tier is honoured.""" + tasks = [{"id": 15, "status": "done", "phaseConfig": {"tier": "wired"}}] + atlas = _setup_base(tmp_path, tasks) + _add_reachability(atlas, 15, "WIRED") + mod = _load() + ok, failures = mod.gate_reachability(tmp_path, tasks) + assert ok is True, failures + + def test_combined_card_is_found(self, tmp_path): + """A combined task-15-16-17.json card covers task 16.""" + atlas = tmp_path / ".atlas-ai" + (atlas / "cdd").mkdir(parents=True) + combined = atlas / "cdd" / "task-15-16-17.json" + combined.write_text(json.dumps({ + "id": "15-16-17", + "grading": {"command": ["sh", "grade.sh"]}, + "reachability": {"verdict": "WIRED"}, + })) + tasks = [{"id": 16, "status": "done", "tier": "wired"}] + mod = _load() + ok, failures = mod.gate_reachability(tmp_path, tasks) + assert ok is True, failures + + def test_modules_listed_in_orphan_failure_message(self, tmp_path): + """ORPHAN failure message includes the module names from the card.""" + tasks = [{"id": 20, "status": "done", "tier": "wired"}] + atlas = _setup_base(tmp_path, tasks) + _add_reachability(atlas, 20, "ORPHAN", modules=["myapp.dead_code", "myapp.other"]) + mod = _load() + ok, failures = mod.gate_reachability(tmp_path, tasks) + assert ok is False + assert any("myapp.dead_code" in f for f in failures), failures + + +# ═══════════════════════════════════════════════════════════════════════════════ +# Integration tests via run_all_gates (importlib, monkeypatched oracle) +# ═══════════════════════════════════════════════════════════════════════════════ + + +class TestGate6ViaRunAllGates: + """run_all_gates integration: Gate 6 is wired after Gate 5.""" + + def test_wired_ships_when_reachability_is_wired(self, tmp_path, monkeypatch, capsys): + tasks_data = [{"id": 1, "status": "done", "tier": "wired"}] + atlas = _setup_base(tmp_path, tasks_data) + _add_reachability(atlas, 1, "WIRED") + mod = _load() + monkeypatch.setattr(mod.subprocess, "run", _fake_run_oracle_pass) + ok, failures = mod.run_all_gates(tmp_path) + assert ok is True, f"failures={failures!r}" + + def test_orphan_blocks_run_all_gates(self, tmp_path, monkeypatch): + tasks_data = [{"id": 2, "status": "done", "tier": "wired"}] + atlas = _setup_base(tmp_path, tasks_data) + _add_reachability(atlas, 2, "ORPHAN", modules=["myapp.route"]) + mod = _load() + monkeypatch.setattr(mod.subprocess, "run", _fake_run_oracle_pass) + ok, failures = mod.run_all_gates(tmp_path) + assert ok is False + assert any("2" in f and "ORPHAN" in f for f in failures), failures + + def test_missing_reachability_blocks_run_all_gates(self, tmp_path, monkeypatch): + tasks_data = [{"id": 3, "status": "done", "tier": "live"}] + _setup_base(tmp_path, tasks_data) # card has no reachability key + mod = _load() + monkeypatch.setattr(mod.subprocess, "run", _fake_run_oracle_pass) + ok, failures = mod.run_all_gates(tmp_path) + assert ok is False + assert any("3" in f for f in failures), failures + + def test_domain_model_does_not_block_run_all_gates(self, tmp_path, monkeypatch): + """domain-model done task with NO reachability block: whole suite passes.""" + tasks_data = [{"id": 4, "status": "done", "tier": "domain-model"}] + _setup_base(tmp_path, tasks_data) + mod = _load() + monkeypatch.setattr(mod.subprocess, "run", _fake_run_oracle_pass) + ok, failures = mod.run_all_gates(tmp_path) + assert ok is True, f"failures={failures!r}" + + def test_untiered_done_task_does_not_block_run_all_gates(self, tmp_path, monkeypatch): + tasks_data = [{"id": 5, "status": "done"}] + _setup_base(tmp_path, tasks_data) + mod = _load() + monkeypatch.setattr(mod.subprocess, "run", _fake_run_oracle_pass) + ok, failures = mod.run_all_gates(tmp_path) + assert ok is True, f"failures={failures!r}" + + def test_non_done_wired_task_does_not_block_run_all_gates(self, tmp_path, monkeypatch): + # All tasks must be done for Gate 2 to pass; use a done untiered task + # plus a pending wired task and verify the pending one doesn't trigger Gate 6. + # NOTE: Gate 2 requires ALL tasks done, so we can't mix done/pending here. + # Test Gate 6 isolation: directly call gate_reachability with pending task. + tasks_data = [{"id": 6, "status": "pending", "tier": "wired"}] + _setup_base(tmp_path, []) # cdd dir created, no cards needed + mod = _load() + ok, failures = mod.gate_reachability(tmp_path, tasks_data) + assert ok is True, failures + + +# ═══════════════════════════════════════════════════════════════════════════════ +# End-to-end subprocess tests (main() → SHIP_CHECK_OK / exit 1) +# ═══════════════════════════════════════════════════════════════════════════════ + + +class TestGate6Subprocess: + """Drive the actual script as a subprocess; oracle stubbed via ATLAS_ORACLE_CMD.""" + + def test_wired_wired_verdict_ships(self, tmp_path): + """Scenario 1: done wired task with WIRED verdict → SHIP_CHECK_OK, exit 0.""" + tasks_data = [{"id": 1, "status": "done", "tier": "wired"}] + atlas = _setup_base(tmp_path, tasks_data) + _add_reachability(atlas, 1, "WIRED") + r = _run_subprocess(tmp_path) + assert r.returncode == 0, f"stderr={r.stderr!r}" + assert "SHIP_CHECK_OK" in r.stdout + + def test_orphan_blocks_main(self, tmp_path): + """Scenario 2: done wired task ORPHAN → nothing on stdout, exit 1.""" + tasks_data = [{"id": 2, "status": "done", "tier": "wired"}] + atlas = _setup_base(tmp_path, tasks_data) + _add_reachability(atlas, 2, "ORPHAN") + r = _run_subprocess(tmp_path) + assert r.returncode == 1 + assert "SHIP_CHECK_OK" not in r.stdout + assert "ORPHAN" in r.stderr + + def test_missing_reachability_block_blocks_main(self, tmp_path): + """Scenario 3: wired done task, card has no 'reachability' key → exit 1.""" + tasks_data = [{"id": 3, "status": "done", "tier": "wired"}] + _setup_base(tmp_path, tasks_data) # card has grading but no reachability + r = _run_subprocess(tmp_path) + assert r.returncode == 1 + assert "SHIP_CHECK_OK" not in r.stdout + assert "reachability" in r.stderr.lower() + + def test_error_verdict_blocks_main(self, tmp_path): + """Scenario 4: done wired task ERROR verdict → exit 1.""" + tasks_data = [{"id": 4, "status": "done", "tier": "wired"}] + atlas = _setup_base(tmp_path, tasks_data) + _add_reachability(atlas, 4, "ERROR") + r = _run_subprocess(tmp_path) + assert r.returncode == 1 + assert "SHIP_CHECK_OK" not in r.stdout + assert "ERROR" in r.stderr + + def test_domain_model_not_blocked_even_without_reachability(self, tmp_path): + """Scenario 5: domain-model done task, no reachability → ships fine.""" + tasks_data = [{"id": 5, "status": "done", "tier": "domain-model"}] + _setup_base(tmp_path, tasks_data) + r = _run_subprocess(tmp_path) + assert r.returncode == 0, f"stderr={r.stderr!r}" + assert "SHIP_CHECK_OK" in r.stdout + + def test_live_tier_with_orphan_blocks(self, tmp_path): + """live tier is also subject to Gate 6.""" + tasks_data = [{"id": 6, "status": "done", "tier": "live"}] + atlas = _setup_base(tmp_path, tasks_data) + _add_reachability(atlas, 6, "ORPHAN") + r = _run_subprocess(tmp_path) + assert r.returncode == 1 + assert "SHIP_CHECK_OK" not in r.stdout + + def test_live_tier_with_exempt_ships(self, tmp_path): + """live tier + EXEMPT verdict → ships.""" + tasks_data = [{"id": 7, "status": "done", "tier": "live"}] + atlas = _setup_base(tmp_path, tasks_data) + _add_reachability(atlas, 7, "EXEMPT") + r = _run_subprocess(tmp_path) + assert r.returncode == 0, f"stderr={r.stderr!r}" + assert "SHIP_CHECK_OK" in r.stdout + + def test_remediation_hint_in_orphan_failure(self, tmp_path): + """ORPHAN failure message must mention remediation (wire or re-status).""" + tasks_data = [{"id": 8, "status": "done", "tier": "wired"}] + atlas = _setup_base(tmp_path, tasks_data) + _add_reachability(atlas, 8, "ORPHAN") + r = _run_subprocess(tmp_path) + assert r.returncode == 1 + assert "wire" in r.stderr.lower() or "re-status" in r.stderr.lower(), r.stderr From 3c9b69f745c6e61230da116da6c0700ff9c6276a Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 16 Jun 2026 19:56:44 +0800 Subject: [PATCH 06/13] feat(engine): validate tier-gated reachableVia + promise>evidence detector + warnings channel - Add warnings channel to run_validate_tasks (alongside problems); hard-block only for wired/live tiers; spike/domain-model get advisory warnings only. - Add reachableVia to TASKS_SCHEMA_HINT, parse prompt, and _task_summaries so it survives expansion; wired/live empty reachableVia is a hard-block, untiered/lower tiers get a warning; bare-word (unscoped) values get an advisory warning. - Add _promise_evidence_mismatch() detector: maps high-altitude claim terms (prisma, cli, client, api, connector/adapter/sync/webhook/integration/endpoint/route) with word-boundary anchored regexes against testStrategy altitude (fixture-only vs live); mismatches are hard-blocked for wired/live, warned for spike/domain-model; includes down-rank suggested_title map. - 27 new tests in tests/core/test_validate_reachability.py; 0 existing tests broken. Co-Authored-By: Claude Sonnet 4.6 --- prd_taskmaster/backend.py | 11 +- prd_taskmaster/validation.py | 150 ++++++- tests/core/test_validate_reachability.py | 480 +++++++++++++++++++++++ 3 files changed, 638 insertions(+), 3 deletions(-) create mode 100644 tests/core/test_validate_reachability.py diff --git a/prd_taskmaster/backend.py b/prd_taskmaster/backend.py index f2e8601..a9051f6 100644 --- a/prd_taskmaster/backend.py +++ b/prd_taskmaster/backend.py @@ -43,6 +43,7 @@ def rate(self, tag=None, research=True) -> dict: ... "dependencies": [], "priority": "high", "tier": "domain-model", + "reachableVia": "", "subtasks": [ { "id": 1, @@ -70,7 +71,9 @@ def rate(self, tag=None, research=True) -> dict: ... do not include placeholders, generic tasks, or empty testStrategy fields; tier ∈ {spike|domain-model|wired|live}: the altitude of the claim — spike=research, domain-model=pure logic, wired=integration, live=user-visible; wired/live require -reachability evidence (the deterministic enrich step will set this if omitted).""" +reachability evidence (the deterministic enrich step will set this if omitted); +reachableVia names the existing route/component/CLI/tool/API the new code wires into; +required for wired/live tasks (a task naming no consumer is an orphan by design).""" PARALLEL_RESULT_SCHEMA_HINT = """{ @@ -241,6 +244,7 @@ def _task_summaries(tasks: list[dict]) -> list[dict]: "dependencies": task.get("dependencies") or [], "status": task.get("status", "pending"), "subtask_count": len(task.get("subtasks") or []), + "reachableVia": task.get("reachableVia", ""), }) return summaries @@ -364,7 +368,10 @@ def parse_prd(self, prd_path, num_tasks, tag=None) -> dict: prompt = ( f"Parse this PRD into exactly {num_tasks} TaskMaster-compatible tasks.\n" f"Target tag: {tag or parallel.current_tag(None)}.\n" - "Return only the tasks JSON object.\n\n" + "Return only the tasks JSON object.\n" + "For wired/live tier tasks, set reachableVia to the existing route, component, CLI, " + "tool, or API that this task's code wires into (e.g. 'route:/api/v1/orders', " + "'cli:prd-taskmaster', 'component:OrdersTable').\n\n" f"PRD PATH: {path}\n" f"PRD:\n{prd_text}" ) diff --git a/prd_taskmaster/validation.py b/prd_taskmaster/validation.py index d9f6765..030c279 100644 --- a/prd_taskmaster/validation.py +++ b/prd_taskmaster/validation.py @@ -338,6 +338,101 @@ def cmd_validate_prd(args: argparse.Namespace) -> None: fail(e.message, **e.extra) +# ─── Reachability / promise-evidence helpers ───────────────────────────────── + +# Claim terms that imply live/integration-level evidence (word-boundary anchored). +# More-specific patterns come FIRST so they take priority over generic ones. +_CLAIM_TERMS: list[tuple[re.Pattern, str]] = [ + (re.compile(r'\b(prisma|database|persist|store|migration|orm)\b', re.IGNORECASE), "db"), + (re.compile(r'\bcli\b', re.IGNORECASE), "cli"), + (re.compile(r'\bclient\b', re.IGNORECASE), "client"), + (re.compile(r'\bapi\b', re.IGNORECASE), "api"), + (re.compile(r'\b(connector|adapter|sync|webhook|integration|endpoint|route)\b', re.IGNORECASE), "integration"), +] + +# Signal that satisfies the live/integration claim requirement +_LIVE_SIGNAL_RE = re.compile( + r'http|request|server|e2e|integration|curl|port|subprocess|invoke|entrypoint|endpoint', + re.IGNORECASE, +) + +# Signal that satisfies the DB claim requirement (real connection, not just fixture) +_DB_SIGNAL_RE = re.compile( + r'database|db|connection|driver|postgres|mysql|sqlite|mongo|redis|supabase|prisma\s+client', + re.IGNORECASE, +) + +# "Fixture-only" pattern: mentions mocks/stubs without live signals +_FIXTURE_ONLY_RE = re.compile( + r'fixture|mock|stub|sample|parses|unit\s+test', + re.IGNORECASE, +) + +# Down-rank map: claim term → suggested replacement title fragment +_DOWNRANK: dict[str, str] = { + "connector": "parser", + "client": "parser", + "prisma": "file adapter", + "database": "file adapter", + "integration": "handler", + "sync": "reader", +} + + +def _classify_test_strategy(test_strategy: str) -> str: + """Return 'fixture-only' if testStrategy has no live signal, else 'live'.""" + ts = test_strategy.strip() + if not ts: + return "fixture-only" + has_live = bool(_LIVE_SIGNAL_RE.search(ts) or _DB_SIGNAL_RE.search(ts)) + has_fixture = bool(_FIXTURE_ONLY_RE.search(ts)) + if has_fixture and not has_live: + return "fixture-only" + return "live" + + +def _suggested_title(claim_term: str, original_title: str) -> str: + """Produce a down-ranked title suggestion.""" + term_lower = claim_term.lower() + # Find the most specific key in _DOWNRANK that is a substring of the claim term + replacement = _DOWNRANK.get(term_lower) + if replacement is None: + # Default: strip the claim term from the title + stripped = re.sub(r'\b' + re.escape(claim_term) + r'\b', '', original_title, flags=re.IGNORECASE).strip() + return stripped or original_title + # Replace first occurrence of the claim term in title + suggested = re.sub(r'\b' + re.escape(claim_term) + r'\b', replacement, original_title, count=1, flags=re.IGNORECASE) + return suggested.strip() + + +def _promise_evidence_mismatch(task: dict) -> dict | None: + """Detect high-altitude claim in title/description vs fixture-only testStrategy. + + Returns a mismatch dict {task_id, claim_term, evidence_altitude, suggested_title} + or None if no mismatch detected. + """ + title = str(task.get("title") or "") + description = str(task.get("description") or "") + test_strategy = str(task.get("testStrategy") or "") + combined_text = f"{title} {description}" + + altitude = _classify_test_strategy(test_strategy) + if altitude != "fixture-only": + return None + + for pattern, claim_key in _CLAIM_TERMS: + mo = pattern.search(combined_text) + if mo: + matched_term = mo.group(0) + return { + "task_id": task.get("id"), + "claim_term": matched_term, + "evidence_altitude": "fixture-only", + "suggested_title": _suggested_title(matched_term, title), + } + return None + + def run_validate_tasks(input_path: str | None, allow_empty_subtasks: bool, require_phase_config: bool) -> dict: """Validate a manually-authored TaskMaster-compatible tasks.json file.""" tasks_path = Path(input_path) if input_path else TASKMASTER_TASKS / "tasks.json" @@ -360,6 +455,7 @@ def run_validate_tasks(input_path: str | None, allow_empty_subtasks: bool, requi allowed_statuses = {"pending", "in-progress", "review", "done", "deferred", "cancelled"} allowed_priorities = {"high", "medium", "low"} problems = [] + warnings = [] ids = [] placeholder_re = re.compile( @@ -455,6 +551,50 @@ def check_text(label: str, field: str, value: object, *, required: bool = True) if dep not in sub_ids: problems.append(f"{label} subtask {sub_id}: dependency {dep!r} does not exist in sibling subtasks") + # ── Reachability checks (per-task) ───────────────────────────────────── + tier = ( + task.get("phaseConfig", {}).get("tier") + or task.get("tier") + or "domain-model" + ) + tier = str(tier).strip().lower() + hard_tiers = {"wired", "live"} + soft_tiers = {"spike", "domain-model"} + + # 1. reachableVia presence check + reachable_via = str(task.get("reachableVia") or "").strip() + if not reachable_via: + if tier in hard_tiers: + problems.append( + f"{label}: tier={tier} requires reachableVia " + f"(name the route/component/CLI/API this wires into)" + ) + else: + warnings.append( + f"{label}: tier={tier} — reachableVia is empty " + f"(advisory: name the route/component/CLI/API this connects to)" + ) + else: + # reachableVia is present — check it looks scoped (has : / . or -) + if not re.search(r'[:/.\-]', reachable_via): + warnings.append( + f"{label}: reachableVia={reachable_via!r} looks unscoped " + f"(no ':' '/' '.' or '-'; prefer e.g. 'route:/x', 'cli:cmd', 'component.Name')" + ) + + # 2. Promise-evidence mismatch check + mismatch = _promise_evidence_mismatch(task) + if mismatch: + msg = ( + f"{label}: title/description claims '{mismatch['claim_term']}' " + f"but testStrategy is fixture-only — " + f"suggested title: {mismatch['suggested_title']!r}" + ) + if tier in hard_tiers: + problems.append(msg) + else: + warnings.append(msg) + real_ids = [task_id for task_id in ids if task_id is not None] duplicate_ids = sorted({task_id for task_id in real_ids if real_ids.count(task_id) > 1}, key=str) for task_id in duplicate_ids: @@ -479,6 +619,7 @@ def check_text(label: str, field: str, value: object, *, required: bool = True) "tasks_path": str(tasks_path), "task_count": len(tasks), "problems": problems, + "warnings": warnings, }, ) @@ -487,12 +628,19 @@ def check_text(label: str, field: str, value: object, *, required: bool = True) "tasks_path": str(tasks_path), "task_count": len(tasks), "subtask_count": sum(len(t.get("subtasks", []) or []) for t in tasks if isinstance(t, dict)), + "warnings": warnings, "message": "Task file is valid for manual prd-taskmaster mode", } def cmd_validate_tasks(args: argparse.Namespace) -> None: try: - emit(run_validate_tasks(args.input, args.allow_empty_subtasks, args.require_phase_config)) + result = run_validate_tasks(args.input, args.allow_empty_subtasks, args.require_phase_config) + # Surface warnings non-fatally before emitting JSON + if result.get("warnings"): + import sys as _sys + for w in result["warnings"]: + print(f"WARNING: {w}", file=_sys.stderr) + emit(result) except CommandError as e: fail(e.message, **e.extra) diff --git a/tests/core/test_validate_reachability.py b/tests/core/test_validate_reachability.py new file mode 100644 index 0000000..3db2cbe --- /dev/null +++ b/tests/core/test_validate_reachability.py @@ -0,0 +1,480 @@ +"""Tests for tier-gated reachableVia + promise>evidence detector + warnings channel. + +All tests call run_validate_tasks() directly or _promise_evidence_mismatch() directly. +No subprocess, no mocking of the load-bearing checks. +""" + +from __future__ import annotations + +import json +import pytest + +from prd_taskmaster.lib import CommandError +from prd_taskmaster.validation import run_validate_tasks, _promise_evidence_mismatch + + +# ─── Helpers ───────────────────────────────────────────────────────────────── + + +def _make_task( + task_id: int = 1, + title: str = "Implement feature X", + description: str = "A concrete feature.", + details: str = "Implementation details here.", + test_strategy: str = "Run pytest tests/core/ -q", + tier: str | None = None, + reachable_via: str | None = None, + phase_config: dict | None = None, + priority: str = "medium", + status: str = "pending", +) -> dict: + """Build a minimal valid task dict with optional tier/reachableVia overrides.""" + task: dict = { + "id": task_id, + "title": title, + "description": description, + "details": details, + "testStrategy": test_strategy, + "priority": priority, + "status": status, + "dependencies": [], + "subtasks": [ + {"id": 1, "title": "First checkpoint", "description": "First step", "status": "pending", "dependencies": []}, + {"id": 2, "title": "Second checkpoint", "description": "Second step", "status": "pending", "dependencies": [1]}, + ], + } + if tier is not None: + task["tier"] = tier + if reachable_via is not None: + task["reachableVia"] = reachable_via + if phase_config is not None: + task["phaseConfig"] = phase_config + return task + + +def _write_tasks_file(tmp_path, tasks: list[dict]) -> str: + """Write a flat tasks.json to tmp_path and return the path string.""" + tasks_file = tmp_path / "tasks.json" + tasks_file.write_text(json.dumps({"tasks": tasks})) + return str(tasks_file) + + +# ─── 1. wired-tier task with empty reachableVia → hard-block ───────────────── + + +class TestReachableViaHardBlock: + def test_wired_empty_reachable_via_raises(self, tmp_path): + """wired-tier task with no reachableVia must raise CommandError (hard-block).""" + task = _make_task(tier="wired", reachable_via=None) + path = _write_tasks_file(tmp_path, [task]) + with pytest.raises(CommandError) as exc_info: + run_validate_tasks(path, allow_empty_subtasks=True, require_phase_config=False) + err = exc_info.value + assert "reachableVia" in " ".join(str(p) for p in err.extra.get("problems", [])) + + def test_live_empty_reachable_via_raises(self, tmp_path): + """live-tier task with no reachableVia must also raise CommandError.""" + task = _make_task(tier="live", reachable_via=None) + path = _write_tasks_file(tmp_path, [task]) + with pytest.raises(CommandError) as exc_info: + run_validate_tasks(path, allow_empty_subtasks=True, require_phase_config=False) + err = exc_info.value + problems_text = " ".join(str(p) for p in err.extra.get("problems", [])) + assert "reachableVia" in problems_text + assert "tier=live" in problems_text + + def test_wired_with_reachable_via_passes(self, tmp_path): + """wired-tier task with a populated reachableVia must NOT raise.""" + task = _make_task(tier="wired", reachable_via="route:/api/v1/orders") + path = _write_tasks_file(tmp_path, [task]) + result = run_validate_tasks(path, allow_empty_subtasks=True, require_phase_config=False) + assert result["ok"] is True + + +# ─── 2. domain-model/untiered task with empty reachableVia → warning only ──── + + +class TestReachableViaWarningsOnly: + def test_domain_model_empty_reachable_via_does_not_raise(self, tmp_path): + """domain-model task with empty reachableVia must not raise — just warn.""" + task = _make_task(tier="domain-model", reachable_via=None) + path = _write_tasks_file(tmp_path, [task]) + # Must NOT raise + result = run_validate_tasks(path, allow_empty_subtasks=True, require_phase_config=False) + assert result["ok"] is True + + def test_domain_model_empty_reachable_via_produces_warning(self, tmp_path): + """domain-model + empty reachableVia must appear in warnings.""" + task = _make_task(tier="domain-model", reachable_via=None) + path = _write_tasks_file(tmp_path, [task]) + result = run_validate_tasks(path, allow_empty_subtasks=True, require_phase_config=False) + assert "warnings" in result + assert any("reachableVia" in w for w in result["warnings"]) + + def test_untiered_task_defaults_to_domain_model(self, tmp_path): + """Task with no tier field defaults to domain-model → warning, no raise.""" + task = _make_task(tier=None) # no tier field in dict + path = _write_tasks_file(tmp_path, [task]) + result = run_validate_tasks(path, allow_empty_subtasks=True, require_phase_config=False) + assert result["ok"] is True + # Warning is expected (defaults to domain-model) + assert "warnings" in result + + +# ─── 3. Promise-evidence mismatch: wired → hard-block ──────────────────────── + + +class TestPromiseEvidenceMismatchHardBlock: + def test_wired_prisma_connector_fixture_only_raises(self, tmp_path): + """wired-tier task titled 'Prisma connector for orders' with fixture-only test → hard-block.""" + task = _make_task( + tier="wired", + reachable_via="route:/api/v1/orders", + title="Prisma connector for orders", + description="Persist order data via Prisma ORM.", + test_strategy="unit test parses a fixture file", + ) + path = _write_tasks_file(tmp_path, [task]) + with pytest.raises(CommandError) as exc_info: + run_validate_tasks(path, allow_empty_subtasks=True, require_phase_config=False) + err = exc_info.value + problems_text = " ".join(str(p) for p in err.extra.get("problems", [])) + # Must mention the claim term or the fixture-only signal + assert "fixture-only" in problems_text or "connector" in problems_text or "prisma" in problems_text.lower() + + def test_promise_evidence_mismatch_direct_suggested_title(self, tmp_path): + """_promise_evidence_mismatch returns suggested_title containing 'file adapter' for Prisma.""" + task = _make_task( + title="Prisma connector for orders", + description="Persist order data.", + test_strategy="unit test parses a fixture", + ) + result = _promise_evidence_mismatch(task) + assert result is not None + assert result["evidence_altitude"] == "fixture-only" + assert "file adapter" in result["suggested_title"].lower() + + def test_promise_evidence_mismatch_client_suggested_title(self): + """_promise_evidence_mismatch: 'client' claim → 'parser' in suggested_title.""" + task = _make_task( + title="HTTP client for external API", + description="Fetch data from vendor.", + test_strategy="unit test parses sample fixture", + ) + result = _promise_evidence_mismatch(task) + assert result is not None + assert "parser" in result["suggested_title"].lower() + + +# ─── 4. Promise-evidence mismatch: domain-model → warning, no raise ────────── + + +class TestPromiseEvidenceMismatchWarningOnly: + def test_domain_model_mismatch_does_not_raise(self, tmp_path): + """domain-model task with claim/fixture mismatch must NOT raise.""" + task = _make_task( + tier="domain-model", + title="Prisma connector for orders", + description="Persist order data.", + test_strategy="unit test parses a fixture", + ) + path = _write_tasks_file(tmp_path, [task]) + result = run_validate_tasks(path, allow_empty_subtasks=True, require_phase_config=False) + assert result["ok"] is True + + def test_domain_model_mismatch_appears_in_warnings(self, tmp_path): + """domain-model mismatch appears in warnings (not problems).""" + task = _make_task( + tier="domain-model", + title="Prisma connector for orders", + description="Persist order data.", + test_strategy="unit test parses a fixture", + ) + path = _write_tasks_file(tmp_path, [task]) + result = run_validate_tasks(path, allow_empty_subtasks=True, require_phase_config=False) + warnings = result.get("warnings", []) + # At least one warning mentioning the mismatch + mismatch_warnings = [w for w in warnings if "fixture-only" in w or "connector" in w or "prisma" in w.lower()] + assert mismatch_warnings, f"Expected mismatch warning in warnings; got: {warnings}" + + def test_domain_model_mismatch_suggested_title_in_warning(self, tmp_path): + """domain-model mismatch warning must include suggested_title.""" + task = _make_task( + tier="domain-model", + title="Prisma connector for orders", + description="Persist order data.", + test_strategy="unit test parses a fixture", + ) + path = _write_tasks_file(tmp_path, [task]) + result = run_validate_tasks(path, allow_empty_subtasks=True, require_phase_config=False) + warnings = result.get("warnings", []) + # Suggested title "file adapter" must appear in the warning text + assert any("file adapter" in w.lower() for w in warnings), ( + f"Expected 'file adapter' in warning text; got: {warnings}" + ) + + +# ─── 5. Live test strategy → no flag ───────────────────────────────────────── + + +class TestLiveTestStrategyNoFlag: + def test_wired_integration_test_no_flag(self, tmp_path): + """wired task with integration-test testStrategy → NO promise-evidence flag.""" + task = _make_task( + tier="wired", + reachable_via="route:/api/v1/orders", + title="API connector for orders endpoint", + description="Wire the orders REST integration.", + test_strategy="integration test hits the /orders endpoint via http request", + ) + path = _write_tasks_file(tmp_path, [task]) + result = run_validate_tasks(path, allow_empty_subtasks=True, require_phase_config=False) + assert result["ok"] is True + # No mismatch warning should appear + warnings = result.get("warnings", []) + mismatch_warnings = [w for w in warnings if "fixture-only" in w] + assert not mismatch_warnings, f"Unexpected mismatch warning: {mismatch_warnings}" + + def test_promise_evidence_mismatch_returns_none_for_live_strategy(self): + """_promise_evidence_mismatch returns None when testStrategy has live signal.""" + task = _make_task( + title="Prisma connector for orders", + description="Persist order data.", + test_strategy="run integration test against real postgres connection", + ) + result = _promise_evidence_mismatch(task) + assert result is None + + +# ─── 6. run_validate_tasks returns 'warnings' key on success ───────────────── + + +class TestWarningsKeyOnSuccess: + def test_success_result_has_warnings_key(self, tmp_path): + """run_validate_tasks always returns a 'warnings' list on success.""" + task = _make_task( + tier="wired", + reachable_via="route:/api/v1/x", + title="Clean task", + description="No suspicious claims.", + test_strategy="run pytest tests/ -q", + ) + path = _write_tasks_file(tmp_path, [task]) + result = run_validate_tasks(path, allow_empty_subtasks=True, require_phase_config=False) + assert "warnings" in result + assert isinstance(result["warnings"], list) + + def test_clean_task_has_empty_warnings(self, tmp_path): + """A fully clean wired task with live test → warnings list is empty.""" + task = _make_task( + tier="wired", + reachable_via="route:/api/v1/x", + title="Add order endpoint handler", + description="Handle POST /orders via REST.", + test_strategy="integration test hits the /orders endpoint via http", + ) + path = _write_tasks_file(tmp_path, [task]) + result = run_validate_tasks(path, allow_empty_subtasks=True, require_phase_config=False) + assert result["warnings"] == [] + + +# ─── 7. domain-model mismatch in warnings not problems ─────────────────────── + + +class TestDomainModelMismatchInWarningsOnly: + def test_domain_model_mismatch_not_in_problems(self, tmp_path): + """domain-model mismatch must go to warnings, never to problems.""" + task = _make_task( + tier="domain-model", + title="Prisma connector for orders", + description="Pure domain logic.", + test_strategy="unit test parses a fixture", + ) + path = _write_tasks_file(tmp_path, [task]) + result = run_validate_tasks(path, allow_empty_subtasks=True, require_phase_config=False) + assert result["ok"] is True + assert "warnings" in result + # The warning should mention fixture-only or the claim term + warnings = result["warnings"] + assert any("fixture-only" in w or "connector" in w or "prisma" in w.lower() for w in warnings) + + +# ─── 8. Anchoring: client substring does NOT trigger hard-block ────────────── + + +class TestClientSubstringAnchoring: + def test_client_side_hyphenated_does_not_trigger(self): + """'client-side' (hyphenated) should NOT match the \\bclient\\b pattern.""" + # The word "client" in "client-side" has a hyphen immediately after, + # so \bclient\b won't match at the 't' boundary (hyphen is a word boundary). + # However, \bclient\b DOES match 'client' in 'client-side' because '-' is + # a non-word char that forms a boundary. Let's test via _promise_evidence_mismatch + # that even if it matches, domain-model never produces a hard-block. + task = _make_task( + tier="domain-model", + title="client-side state model", + description="Pure domain-model for UI state.", + test_strategy="unit test parses a fixture", + ) + # domain-model never hard-blocks regardless of match + result = _promise_evidence_mismatch(task) + # Even if there IS a mismatch, domain-model only warns + # This test verifies via run_validate_tasks that it doesn't raise + # (direct function call already tested above, so here we test the key property) + if result is not None: + # The mismatch exists but domain-model means it is only advisory + assert result["evidence_altitude"] == "fixture-only" + + def test_api_substring_does_not_match_rapid(self): + """'rapid' should NOT trigger the \\bapi\\b pattern.""" + task = _make_task( + title="Rapid prototyping for UI layer", + description="Quick iteration cycle.", + test_strategy="unit test parses a fixture", + ) + result = _promise_evidence_mismatch(task) + # If a mismatch fires, it should NOT be for 'api' (rapid does not contain \bapi\b) + if result is not None: + assert result["claim_term"].lower() != "api", ( + f"'rapid' should not match \\bapi\\b, but got claim_term={result['claim_term']!r}" + ) + + def test_api_standalone_word_does_trigger(self): + """'API' as a standalone word in the title DOES trigger the pattern.""" + task = _make_task( + title="API client for vendor integration", + description="Fetch from vendor API.", + test_strategy="unit test parses a sample fixture file", + ) + result = _promise_evidence_mismatch(task) + assert result is not None + assert result["claim_term"].lower() in {"api", "client", "integration"} + + def test_cli_does_not_match_client(self): + """'client' should not trigger the \\bcli\\b pattern.""" + task = _make_task( + title="HTTP client wrapper", + description="Wrap HTTP calls.", + test_strategy="unit test parses a fixture", + ) + result = _promise_evidence_mismatch(task) + if result is not None: + # It might match 'client' but must NOT report 'cli' as the matched term + # (because 'client' does not match \bcli\b — 'client' has extra chars after 'cli') + # Actually \bcli\b won't match inside 'client' because 'e' follows 'i' + # \b is at word boundary: cli-ent, 'i'→'e' are both word chars, so no boundary + assert result["claim_term"].lower() != "cli", ( + f"'client' should not match \\bcli\\b, but got claim_term={result['claim_term']!r}" + ) + + +# ─── 9. Hand-authored vs AI-authored: checks fire regardless ───────────────── + + +class TestHandAuthoredCoverage: + def test_hand_authored_task_triggers_reachability_check(self, tmp_path): + """Hand-crafted wired task without reachableVia must still hard-block.""" + # This simulates a task authored manually (not by AI) + raw_task = { + "id": 1, + "title": "Hand-built route handler", + "description": "Manually written task.", + "details": "This was written by a human developer.", + "testStrategy": "pytest tests/test_handler.py", + "priority": "high", + "status": "pending", + "tier": "wired", + "dependencies": [], + "subtasks": [ + {"id": 1, "title": "First", "description": "Step one", "status": "pending", "dependencies": []}, + {"id": 2, "title": "Second", "description": "Step two", "status": "pending", "dependencies": [1]}, + ], + # No reachableVia — intentionally omitted + } + tasks_file = tmp_path / "tasks.json" + tasks_file.write_text(json.dumps({"tasks": [raw_task]})) + with pytest.raises(CommandError) as exc_info: + run_validate_tasks(str(tasks_file), allow_empty_subtasks=True, require_phase_config=False) + assert "reachableVia" in " ".join(str(p) for p in exc_info.value.extra.get("problems", [])) + + def test_hand_authored_domain_model_no_hard_block(self, tmp_path): + """Hand-crafted domain-model task without reachableVia → warn, not hard-block.""" + raw_task = { + "id": 1, + "title": "Domain model for orders", + "description": "Pure business logic.", + "details": "No integration needed.", + "testStrategy": "pytest tests/ -q", + "priority": "low", + "status": "pending", + "tier": "domain-model", + "dependencies": [], + "subtasks": [ + {"id": 1, "title": "Model class", "description": "Define model", "status": "pending", "dependencies": []}, + {"id": 2, "title": "Validators", "description": "Add validators", "status": "pending", "dependencies": [1]}, + ], + } + tasks_file = tmp_path / "tasks.json" + tasks_file.write_text(json.dumps({"tasks": [raw_task]})) + result = run_validate_tasks(str(tasks_file), allow_empty_subtasks=True, require_phase_config=False) + assert result["ok"] is True + assert "warnings" in result + + +# ─── 10. Unscoped reachableVia → advisory warning ──────────────────────────── + + +class TestUnscopedReachableVia: + def test_bare_word_reachable_via_warns(self, tmp_path): + """reachableVia with no colon, slash, dot, or dash → advisory warning.""" + task = _make_task( + tier="wired", + reachable_via="ordersroute", # no scope marker + ) + path = _write_tasks_file(tmp_path, [task]) + result = run_validate_tasks(path, allow_empty_subtasks=True, require_phase_config=False) + assert result["ok"] is True + warnings = result.get("warnings", []) + assert any("unscoped" in w or "ordersroute" in w for w in warnings) + + def test_scoped_reachable_via_no_warning(self, tmp_path): + """reachableVia with a colon (e.g. 'route:/x') → no unscoped warning.""" + task = _make_task( + tier="wired", + reachable_via="route:/api/v1/orders", + ) + path = _write_tasks_file(tmp_path, [task]) + result = run_validate_tasks(path, allow_empty_subtasks=True, require_phase_config=False) + assert result["ok"] is True + warnings = result.get("warnings", []) + unscoped_warnings = [w for w in warnings if "unscoped" in w] + assert not unscoped_warnings + + +# ─── 11. Spike tier follows soft rules ─────────────────────────────────────── + + +class TestSpikeTierSoftRules: + def test_spike_empty_reachable_via_warns_not_blocks(self, tmp_path): + """spike-tier task with empty reachableVia → warning, not hard-block.""" + task = _make_task(tier="spike", reachable_via=None) + path = _write_tasks_file(tmp_path, [task]) + result = run_validate_tasks(path, allow_empty_subtasks=True, require_phase_config=False) + assert result["ok"] is True + warnings = result.get("warnings", []) + assert any("reachableVia" in w for w in warnings) + + def test_spike_mismatch_warns_not_blocks(self, tmp_path): + """spike-tier task with promise-evidence mismatch → warning, not hard-block.""" + task = _make_task( + tier="spike", + title="Prisma connector spike", + description="Research Prisma integration.", + test_strategy="unit test parses a fixture", + ) + path = _write_tasks_file(tmp_path, [task]) + result = run_validate_tasks(path, allow_empty_subtasks=True, require_phase_config=False) + assert result["ok"] is True + warnings = result.get("warnings", []) + # There should be a mismatch warning + assert any("fixture-only" in w or "connector" in w or "prisma" in w.lower() for w in warnings) From 8455009c382325ec52de9b4bf1e0346c2a958050 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 16 Jun 2026 20:11:17 +0800 Subject: [PATCH 07/13] =?UTF-8?q?fix(engine):=20split=20promise-mismatch?= =?UTF-8?q?=20claim=20terms=20=E2=80=94=20precise=3Dhard-block,=20ambiguou?= =?UTF-8?q?s(store/route/sync/client/api)=3Dwarn-only?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-Authored-By: Claude Opus 4.8 --- prd_taskmaster/validation.py | 53 ++++-- tests/core/test_validate_reachability.py | 211 ++++++++++++++++++++--- 2 files changed, 233 insertions(+), 31 deletions(-) diff --git a/prd_taskmaster/validation.py b/prd_taskmaster/validation.py index 030c279..24b7965 100644 --- a/prd_taskmaster/validation.py +++ b/prd_taskmaster/validation.py @@ -340,14 +340,23 @@ def cmd_validate_prd(args: argparse.Namespace) -> None: # ─── Reachability / promise-evidence helpers ───────────────────────────────── -# Claim terms that imply live/integration-level evidence (word-boundary anchored). -# More-specific patterns come FIRST so they take priority over generic ones. -_CLAIM_TERMS: list[tuple[re.Pattern, str]] = [ - (re.compile(r'\b(prisma|database|persist|store|migration|orm)\b', re.IGNORECASE), "db"), +# Precise claim terms that unambiguously imply live/integration-level evidence. +# A fixture-only testStrategy paired with one of these is a HARD-BLOCK on +# wired/live tasks (and a warning on spike/domain-model), same as before. +# Word-boundary anchored; more-specific patterns come first. +_HARD_CLAIM_TERMS: list[tuple[re.Pattern, str]] = [ + (re.compile(r'\b(prisma|database|persist|migration|orm)\b', re.IGNORECASE), "db"), (re.compile(r'\bcli\b', re.IGNORECASE), "cli"), + (re.compile(r'\b(connector|adapter|webhook|integration|endpoint)\b', re.IGNORECASE), "integration"), +] + +# Ambiguous claim terms — common in legitimate UI/state work (Redux store, +# React Router route, client-side hydration, API types/config, sync props). +# These NEVER produce a hard-block; they add an advisory warning at every tier. +_SOFT_CLAIM_TERMS: list[tuple[re.Pattern, str]] = [ (re.compile(r'\bclient\b', re.IGNORECASE), "client"), (re.compile(r'\bapi\b', re.IGNORECASE), "api"), - (re.compile(r'\b(connector|adapter|sync|webhook|integration|endpoint|route)\b', re.IGNORECASE), "integration"), + (re.compile(r'\b(route|sync|store)\b', re.IGNORECASE), "integration"), ] # Signal that satisfies the live/integration claim requirement @@ -364,7 +373,7 @@ def cmd_validate_prd(args: argparse.Namespace) -> None: # "Fixture-only" pattern: mentions mocks/stubs without live signals _FIXTURE_ONLY_RE = re.compile( - r'fixture|mock|stub|sample|parses|unit\s+test', + r'fixture|mock|stub|sample|parses|\bunit\s+tests?\b', re.IGNORECASE, ) @@ -408,8 +417,13 @@ def _suggested_title(claim_term: str, original_title: str) -> str: def _promise_evidence_mismatch(task: dict) -> dict | None: """Detect high-altitude claim in title/description vs fixture-only testStrategy. - Returns a mismatch dict {task_id, claim_term, evidence_altitude, suggested_title} + Returns a mismatch dict {task_id, claim_term, evidence_altitude, suggested_title, soft} or None if no mismatch detected. + + ``soft=True`` means the matched term is ambiguous (store/route/sync/client/api) — + the caller must treat this as an advisory warning at every tier, never a hard-block. + ``soft=False`` means the matched term is a precise integration promise — the caller + hard-blocks wired/live tasks and warns on spike/domain-model. """ title = str(task.get("title") or "") description = str(task.get("description") or "") @@ -420,7 +434,21 @@ def _promise_evidence_mismatch(task: dict) -> dict | None: if altitude != "fixture-only": return None - for pattern, claim_key in _CLAIM_TERMS: + # Check precise (hard) terms first + for pattern, claim_key in _HARD_CLAIM_TERMS: + mo = pattern.search(combined_text) + if mo: + matched_term = mo.group(0) + return { + "task_id": task.get("id"), + "claim_term": matched_term, + "evidence_altitude": "fixture-only", + "suggested_title": _suggested_title(matched_term, title), + "soft": False, + } + + # Check ambiguous (soft) terms — warn-only regardless of tier + for pattern, claim_key in _SOFT_CLAIM_TERMS: mo = pattern.search(combined_text) if mo: matched_term = mo.group(0) @@ -429,7 +457,9 @@ def _promise_evidence_mismatch(task: dict) -> dict | None: "claim_term": matched_term, "evidence_altitude": "fixture-only", "suggested_title": _suggested_title(matched_term, title), + "soft": True, } + return None @@ -559,7 +589,6 @@ def check_text(label: str, field: str, value: object, *, required: bool = True) ) tier = str(tier).strip().lower() hard_tiers = {"wired", "live"} - soft_tiers = {"spike", "domain-model"} # 1. reachableVia presence check reachable_via = str(task.get("reachableVia") or "").strip() @@ -590,7 +619,11 @@ def check_text(label: str, field: str, value: object, *, required: bool = True) f"but testStrategy is fixture-only — " f"suggested title: {mismatch['suggested_title']!r}" ) - if tier in hard_tiers: + if mismatch.get("soft"): + # Ambiguous term (store/route/sync/client/api): advisory warning at every tier, + # never a hard-block even for wired/live tasks. + warnings.append(msg) + elif tier in hard_tiers: problems.append(msg) else: warnings.append(msg) diff --git a/tests/core/test_validate_reachability.py b/tests/core/test_validate_reachability.py index 3db2cbe..1ace035 100644 --- a/tests/core/test_validate_reachability.py +++ b/tests/core/test_validate_reachability.py @@ -303,27 +303,26 @@ def test_domain_model_mismatch_not_in_problems(self, tmp_path): class TestClientSubstringAnchoring: - def test_client_side_hyphenated_does_not_trigger(self): - """'client-side' (hyphenated) should NOT match the \\bclient\\b pattern.""" - # The word "client" in "client-side" has a hyphen immediately after, - # so \bclient\b won't match at the 't' boundary (hyphen is a word boundary). - # However, \bclient\b DOES match 'client' in 'client-side' because '-' is - # a non-word char that forms a boundary. Let's test via _promise_evidence_mismatch - # that even if it matches, domain-model never produces a hard-block. + def test_client_side_warns_not_blocks(self, tmp_path): + """'client-side' matches \\bclient\\b (hyphen is a word boundary), but 'client' + is now a soft/ambiguous term — it produces an advisory warning, never a hard-block + even for wired/live tasks.""" task = _make_task( - tier="domain-model", - title="client-side state model", - description="Pure domain-model for UI state.", - test_strategy="unit test parses a fixture", + tier="wired", + reachable_via="component.HydrationLayer", + title="client-side hydration layer", + description="Pure client-side rendering with React.", + test_strategy="write unit tests for component props", + ) + path = _write_tasks_file(tmp_path, [task]) + # Must NOT raise — client is soft/warn-only + result = run_validate_tasks(path, allow_empty_subtasks=True, require_phase_config=False) + assert result["ok"] is True + # But a warning should be present + warnings = result.get("warnings", []) + assert any("client" in w.lower() for w in warnings), ( + f"Expected advisory warning mentioning 'client'; got: {warnings}" ) - # domain-model never hard-blocks regardless of match - result = _promise_evidence_mismatch(task) - # Even if there IS a mismatch, domain-model only warns - # This test verifies via run_validate_tasks that it doesn't raise - # (direct function call already tested above, so here we test the key property) - if result is not None: - # The mismatch exists but domain-model means it is only advisory - assert result["evidence_altitude"] == "fixture-only" def test_api_substring_does_not_match_rapid(self): """'rapid' should NOT trigger the \\bapi\\b pattern.""" @@ -340,7 +339,9 @@ def test_api_substring_does_not_match_rapid(self): ) def test_api_standalone_word_does_trigger(self): - """'API' as a standalone word in the title DOES trigger the pattern.""" + """Title containing 'integration' (a hard term) plus 'API' and 'client' (soft + terms) still triggers a mismatch because hard terms are checked first. + 'integration' is precise/hard so it fires as the matched term.""" task = _make_task( title="API client for vendor integration", description="Fetch from vendor API.", @@ -348,7 +349,10 @@ def test_api_standalone_word_does_trigger(self): ) result = _promise_evidence_mismatch(task) assert result is not None - assert result["claim_term"].lower() in {"api", "client", "integration"} + # 'integration' is the first hard term to match; api/client are soft (checked after hard) + assert result["claim_term"].lower() in {"integration", "api", "client"} + # The result must not be soft — 'integration' is a hard claim term + assert not result.get("soft"), "Expected hard mismatch (integration is a precise term)" def test_cli_does_not_match_client(self): """'client' should not trigger the \\bcli\\b pattern.""" @@ -478,3 +482,168 @@ def test_spike_mismatch_warns_not_blocks(self, tmp_path): warnings = result.get("warnings", []) # There should be a mismatch warning assert any("fixture-only" in w or "connector" in w or "prisma" in w.lower() for w in warnings) + + +# ─── 12. Soft claim terms: no false hard-block on wired/live ────────────────── + + +class TestSoftClaimTermsNoFalseHardBlock: + """Ambiguous claim terms (store/route/sync/client/api) must NEVER hard-block + wired or live tasks — only produce advisory warnings.""" + + def test_wired_redux_store_unit_tests_warns_not_blocks(self, tmp_path): + """wired 'Redux store' + 'write unit tests' → ok:True, warning present.""" + task = _make_task( + tier="wired", + reachable_via="component.ReduxStore", + title="Redux store for cart state", + description="Manage cart state with Redux.", + test_strategy="write unit tests for reducers", + ) + path = _write_tasks_file(tmp_path, [task]) + result = run_validate_tasks(path, allow_empty_subtasks=True, require_phase_config=False) + assert result["ok"] is True, ( + "wired 'Redux store' with unit tests should not hard-block — 'store' is a soft term" + ) + warnings = result.get("warnings", []) + assert any("store" in w.lower() or "fixture-only" in w for w in warnings), ( + f"Expected advisory warning for 'store'; got: {warnings}" + ) + + def test_wired_api_route_handler_unit_tests_warns_not_blocks(self, tmp_path): + """wired 'API route handler' + 'write unit tests' → ok:True, warning present.""" + task = _make_task( + tier="wired", + reachable_via="route:/api/v1/orders", + title="API route handler for orders", + description="Handle order requests via the API route.", + test_strategy="write unit tests for the handler function", + ) + path = _write_tasks_file(tmp_path, [task]) + result = run_validate_tasks(path, allow_empty_subtasks=True, require_phase_config=False) + assert result["ok"] is True, ( + "wired 'API route handler' with unit tests should not hard-block — 'api'/'route' are soft terms" + ) + warnings = result.get("warnings", []) + assert any("api" in w.lower() or "route" in w.lower() or "fixture-only" in w for w in warnings), ( + f"Expected advisory warning for 'api'/'route'; got: {warnings}" + ) + + def test_wired_client_side_hydration_unit_tests_warns_not_blocks(self, tmp_path): + """wired 'client-side hydration' + 'unit tests' → ok:True, warning present.""" + task = _make_task( + tier="wired", + reachable_via="component.HydrationShell", + title="client-side hydration shell", + description="Implement React client-side hydration for SSR pages.", + test_strategy="unit tests for hydration props", + ) + path = _write_tasks_file(tmp_path, [task]) + result = run_validate_tasks(path, allow_empty_subtasks=True, require_phase_config=False) + assert result["ok"] is True, ( + "wired 'client-side hydration' with unit tests should not hard-block — 'client' is a soft term" + ) + warnings = result.get("warnings", []) + assert any("client" in w.lower() or "fixture-only" in w for w in warnings), ( + f"Expected advisory warning for 'client'; got: {warnings}" + ) + + def test_wired_sync_props_unit_tests_warns_not_blocks(self, tmp_path): + """wired 'sync props/state' + 'write unit tests' → ok:True, warning present.""" + task = _make_task( + tier="wired", + reachable_via="component.SyncedPanel", + title="Sync props between parent and child components", + description="Use useEffect to sync state props in React.", + test_strategy="write unit tests for the sync hook", + ) + path = _write_tasks_file(tmp_path, [task]) + result = run_validate_tasks(path, allow_empty_subtasks=True, require_phase_config=False) + assert result["ok"] is True, ( + "wired 'sync props' with unit tests should not hard-block — 'sync' is a soft term" + ) + warnings = result.get("warnings", []) + assert any("sync" in w.lower() or "fixture-only" in w for w in warnings), ( + f"Expected advisory warning for 'sync'; got: {warnings}" + ) + + def test_soft_term_mismatch_is_marked_soft(self): + """_promise_evidence_mismatch sets soft=True for ambiguous claim terms.""" + task = _make_task( + title="Redux store reducer", + description="Manage UI state store.", + test_strategy="unit tests for reducer pure functions", + ) + result = _promise_evidence_mismatch(task) + assert result is not None + assert result.get("soft") is True, ( + f"Expected soft=True for 'store' claim term; got: {result}" + ) + + def test_hard_term_mismatch_is_not_soft(self): + """_promise_evidence_mismatch sets soft=False for precise claim terms.""" + task = _make_task( + title="Prisma connector for orders", + description="Persist data via Prisma ORM.", + test_strategy="unit test parses a fixture", + ) + result = _promise_evidence_mismatch(task) + assert result is not None + assert result.get("soft") is False, ( + f"Expected soft=False for 'prisma'/'connector' claim term; got: {result}" + ) + + +# ─── 13. Precise terms still hard-block wired/live ─────────────────────────── + + +class TestPreciseTermsStillHardBlock: + """Precise/unambiguous terms (prisma, webhook, connector, endpoint, etc.) + must continue to hard-block wired/live tasks.""" + + def test_wired_webhook_integration_fixture_only_raises(self, tmp_path): + """wired 'webhook integration' + fixture-only test → hard-block.""" + task = _make_task( + tier="wired", + reachable_via="route:/webhooks/stripe", + title="webhook integration for Stripe events", + description="Handle Stripe webhook payloads.", + test_strategy="unit test parses a fixture payload", + ) + path = _write_tasks_file(tmp_path, [task]) + with pytest.raises(CommandError) as exc_info: + run_validate_tasks(path, allow_empty_subtasks=True, require_phase_config=False) + err = exc_info.value + problems_text = " ".join(str(p) for p in err.extra.get("problems", [])) + assert "fixture-only" in problems_text or "webhook" in problems_text, ( + f"Expected 'webhook' or 'fixture-only' in problems; got: {problems_text}" + ) + + def test_wired_prisma_connector_fixture_still_hard_blocks(self, tmp_path): + """wired 'Prisma connector' + fixture-only test → hard-block (unchanged).""" + task = _make_task( + tier="wired", + reachable_via="route:/api/v1/orders", + title="Prisma connector for order persistence", + description="Persist orders to the database via Prisma.", + test_strategy="unit test parses a fixture file", + ) + path = _write_tasks_file(tmp_path, [task]) + with pytest.raises(CommandError): + run_validate_tasks(path, allow_empty_subtasks=True, require_phase_config=False) + + def test_wired_database_migration_fixture_still_hard_blocks(self, tmp_path): + """wired 'database migration' + fixture-only test → hard-block.""" + task = _make_task( + tier="wired", + reachable_via="cli:migrate", + title="database migration for user schema", + description="Add migration to update the users table.", + test_strategy="unit tests parse fixture SQL", + ) + path = _write_tasks_file(tmp_path, [task]) + with pytest.raises(CommandError) as exc_info: + run_validate_tasks(path, allow_empty_subtasks=True, require_phase_config=False) + err = exc_info.value + problems_text = " ".join(str(p) for p in err.extra.get("problems", [])) + assert "fixture-only" in problems_text or "database" in problems_text or "migration" in problems_text From a1f077cc7f4c96d762a2f76730425ff7ae821655 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 16 Jun 2026 20:19:11 +0800 Subject: [PATCH 08/13] feat(engine): set-status done captures evidence + tier-gated reachability gate (wired/live require WIRED/EXEMPT verdict) - run_set_status gains evidence_ref + reachability params (signature backward-compatible: tag/non-done paths unchanged) - wired/live tasks marked done without a WIRED/EXEMPT reachability dict now raise CommandError (fail-closed gate) - ORPHAN/ERROR/unknown/absent verdicts all block; domain-model/spike/untiered tasks keep bare flip - doneEvidence (evidence_ref + timestamp) and reachability dict persisted additively on the task object when provided - set_task_status MCP wrapper passes both new params through - 16 new TDD tests green; full suite 517 passed 3 skipped --- mcp-server/server.py | 25 +- prd_taskmaster/task_state.py | 53 +++- tests/core/test_set_status_evidence.py | 342 +++++++++++++++++++++++++ 3 files changed, 415 insertions(+), 5 deletions(-) create mode 100644 tests/core/test_set_status_evidence.py diff --git a/mcp-server/server.py b/mcp-server/server.py index 4330502..f25e5c1 100644 --- a/mcp-server/server.py +++ b/mcp-server/server.py @@ -142,10 +142,29 @@ def claim_task(tag: str = "") -> dict: @mcp.tool() -def set_task_status(id: str, status: str, tag: str = "") -> dict: - """Set a task or subtask status without terminating the MCP host.""" +def set_task_status( + id: str, + status: str, + tag: str = "", + evidence_ref: str | None = None, + reachability: dict | None = None, +) -> dict: + """Set a task or subtask status without terminating the MCP host. + + For status != "done": evidence_ref and reachability are ignored. + For status == "done" on a wired/live task: reachability must be provided + with verdict in {WIRED, EXEMPT}. Pass the dict returned by the + reachability sweep (mcp__atlas-engine__check_gate / sweep_task). + Evidence is persisted on the task when provided (any tier). + """ try: - return TS.run_set_status(id_str=id, status=status, tag=tag or None) + return TS.run_set_status( + id_str=id, + status=status, + tag=tag or None, + evidence_ref=evidence_ref, + reachability=reachability, + ) except LIB.CommandError as exc: return {"ok": False, "error": exc.message, **exc.extra} diff --git a/prd_taskmaster/task_state.py b/prd_taskmaster/task_state.py index 14eb6de..507a02b 100644 --- a/prd_taskmaster/task_state.py +++ b/prd_taskmaster/task_state.py @@ -4,11 +4,17 @@ import argparse import json +from datetime import datetime, timezone from typing import Any from prd_taskmaster import fleet, parallel from prd_taskmaster.lib import CommandError, emit, fail, locked_update +# Tiers that require a reachability verdict before done is accepted. +_GATED_TIERS = {"wired", "live"} +# Reachability verdicts that allow done. +_PASSING_VERDICTS = {"WIRED", "EXEMPT"} + VALID_STATUSES = { "pending", "in-progress", @@ -227,8 +233,23 @@ def _split_id(id_str: str) -> tuple[str, str | None]: raise CommandError(f"unknown id: {id_str}") -def run_set_status(id_str: str, status: str, tag: str | None = None) -> dict: - """Set a parent task or subtask status under a file lock.""" +def run_set_status( + id_str: str, + status: str, + tag: str | None = None, + evidence_ref: str | None = None, + reachability: dict | None = None, +) -> dict: + """Set a parent task or subtask status under a file lock. + + For status != "done": evidence_ref and reachability are accepted but ignored. + For status == "done" on a wired/live task: a reachability dict with verdict + in {WIRED, EXEMPT} is required; absence or a blocking verdict (ORPHAN, ERROR) + raises CommandError. + + When evidence_ref or reachability is provided (any tier), the proof is + persisted on the task object as doneEvidence / reachability fields. + """ if status not in VALID_STATUSES: raise CommandError(f"unknown status: {status}") @@ -258,7 +279,35 @@ def transform(current: str) -> str: if str(task.get("id")) != parent_id: continue if subtask_id is None: + # Tier-gated reachability check for parent tasks marked done. + if status == "done": + tier = ( + (task.get("phaseConfig") or {}).get("tier") + or task.get("tier") + or "domain-model" + ) + if tier in _GATED_TIERS: + if reachability is None: + raise CommandError( + f"cannot mark task {id_str} (tier={tier}) done without a" + f" reachability verdict — run the reachability sweep" + ) + verdict = reachability.get("verdict") + if verdict not in _PASSING_VERDICTS: + raise CommandError( + f"cannot mark task {id_str} done: reachability {verdict}" + f" — wire the module(s) into the running system or" + f" re-status deferred/scaffold" + ) task["status"] = status + # Persist evidence additively when provided (any tier). + if evidence_ref is not None: + task["doneEvidence"] = { + "evidence_ref": evidence_ref, + "at": datetime.now(timezone.utc).isoformat(), + } + if reachability is not None: + task["reachability"] = reachability result.update({ "ok": True, "tag": resolved_tag, diff --git a/tests/core/test_set_status_evidence.py b/tests/core/test_set_status_evidence.py new file mode 100644 index 0000000..162fdeb --- /dev/null +++ b/tests/core/test_set_status_evidence.py @@ -0,0 +1,342 @@ +"""TDD: set-status done — tier-gated reachability + evidence persistence. + +Design: +- status != "done": evidence_ref / reachability ignored → unchanged behavior. +- status == "done" on wired/live task: requires reachability dict w/ WIRED or EXEMPT verdict. + - absent reachability → CommandError + - ORPHAN / ERROR verdict → CommandError + - WIRED / EXEMPT verdict → ok, evidence persisted +- status == "done" on domain-model / untiered task: bare flip (backward-compat). +- evidence_ref / reachability persisted on the task object when provided (any tier). +""" + +from __future__ import annotations + +import json +from pathlib import Path + +import pytest + +from prd_taskmaster.lib import CommandError +from prd_taskmaster.task_state import run_set_status + +REPO = Path(__file__).resolve().parents[2] + + +# --------------------------------------------------------------------------- +# Helpers +# --------------------------------------------------------------------------- + + +def _task(task_id, *, tier=None, status="pending"): + t = { + "id": task_id, + "title": f"Task {task_id}", + "description": f"Description {task_id}", + "details": f"Details {task_id}", + "testStrategy": f"Test {task_id}", + "status": status, + "priority": "medium", + "dependencies": [], + "subtasks": [], + } + if tier is not None: + t["phaseConfig"] = {"tier": tier} + return t + + +def _write_project(tmp_path, tasks, *, tag="master"): + payload = { + tag: { + "tasks": tasks, + "metadata": { + "created": "2026-01-01T00:00:00.000Z", + "updated": "2026-01-01T00:00:00.000Z", + "description": f"Tasks for {tag}", + }, + } + } + tasks_dir = tmp_path / ".taskmaster" / "tasks" + tasks_dir.mkdir(parents=True) + f = tasks_dir / "tasks.json" + f.write_text(json.dumps(payload, indent=2)) + (tmp_path / ".taskmaster" / "state.json").write_text( + json.dumps({"currentTag": tag, "migrationNoticeShown": True}) + ) + return f + + +def _reload(tasks_file, tag="master"): + raw = json.loads(tasks_file.read_text()) + return {str(t["id"]): t for t in raw[tag]["tasks"]} + + +# --------------------------------------------------------------------------- +# 1. Backward-compat: untiered / domain-model task done without evidence → ok +# --------------------------------------------------------------------------- + + +def test_untiered_done_no_evidence_ok(tmp_path, monkeypatch): + """Untiered task (falls back to domain-model) → bare flip, no raise.""" + tasks_file = _write_project(tmp_path, [_task(1)]) + monkeypatch.chdir(tmp_path) + + result = run_set_status("1", "done") + + assert result["ok"] is True + assert result["status"] == "done" + assert result["kind"] == "task" + tasks = _reload(tasks_file) + assert tasks["1"]["status"] == "done" + # No doneEvidence persisted when none provided + assert "doneEvidence" not in tasks["1"] + + +def test_domain_model_done_no_evidence_ok(tmp_path, monkeypatch): + """Explicit domain-model tier → bare flip, no raise.""" + tasks_file = _write_project(tmp_path, [_task(1, tier="domain-model")]) + monkeypatch.chdir(tmp_path) + + result = run_set_status("1", "done") + + assert result["ok"] is True + tasks = _reload(tasks_file) + assert tasks["1"]["status"] == "done" + + +def test_spike_done_no_evidence_ok(tmp_path, monkeypatch): + """spike tier → bare flip (same as domain-model), no raise.""" + tasks_file = _write_project(tmp_path, [_task(1, tier="spike")]) + monkeypatch.chdir(tmp_path) + + result = run_set_status("1", "done") + + assert result["ok"] is True + tasks = _reload(tasks_file) + assert tasks["1"]["status"] == "done" + + +# --------------------------------------------------------------------------- +# 2. wired requires reachability +# --------------------------------------------------------------------------- + + +def test_wired_done_without_reachability_raises(tmp_path, monkeypatch): + """wired task + done + reachability=None → CommandError.""" + _write_project(tmp_path, [_task(1, tier="wired")]) + monkeypatch.chdir(tmp_path) + + with pytest.raises(CommandError) as exc_info: + run_set_status("1", "done", reachability=None) + + msg = exc_info.value.message + assert "reachability verdict" in msg + assert "1" in msg + + +def test_live_done_without_reachability_raises(tmp_path, monkeypatch): + """live tier + done + reachability=None → CommandError.""" + _write_project(tmp_path, [_task(1, tier="live")]) + monkeypatch.chdir(tmp_path) + + with pytest.raises(CommandError): + run_set_status("1", "done") + + +# --------------------------------------------------------------------------- +# 3. wired ORPHAN verdict blocks +# --------------------------------------------------------------------------- + + +def test_wired_orphan_verdict_raises(tmp_path, monkeypatch): + """wired task + ORPHAN verdict → CommandError.""" + _write_project(tmp_path, [_task(1, tier="wired")]) + monkeypatch.chdir(tmp_path) + + with pytest.raises(CommandError) as exc_info: + run_set_status("1", "done", reachability={"verdict": "ORPHAN"}) + + msg = exc_info.value.message + assert "ORPHAN" in msg + + +def test_wired_error_verdict_raises(tmp_path, monkeypatch): + """wired task + ERROR verdict → CommandError.""" + _write_project(tmp_path, [_task(1, tier="wired")]) + monkeypatch.chdir(tmp_path) + + with pytest.raises(CommandError) as exc_info: + run_set_status("1", "done", reachability={"verdict": "ERROR"}) + + msg = exc_info.value.message + assert "ERROR" in msg + + +def test_wired_unknown_verdict_raises(tmp_path, monkeypatch): + """wired task + unrecognized verdict → CommandError (fail closed).""" + _write_project(tmp_path, [_task(1, tier="wired")]) + monkeypatch.chdir(tmp_path) + + with pytest.raises(CommandError): + run_set_status("1", "done", reachability={"verdict": "UNKNOWN_FUTURE_VERDICT"}) + + +# --------------------------------------------------------------------------- +# 4. wired WIRED verdict passes + evidence persisted +# --------------------------------------------------------------------------- + + +def test_wired_wired_verdict_passes_and_persists(tmp_path, monkeypatch): + """wired task + WIRED verdict + evidence_ref → ok, both persisted.""" + tasks_file = _write_project(tmp_path, [_task(1, tier="wired")]) + monkeypatch.chdir(tmp_path) + + sweep = {"verdict": "WIRED", "tier": "wired", "modules": [], "start_commit": "abc123"} + result = run_set_status("1", "done", evidence_ref="card.json", reachability=sweep) + + assert result["ok"] is True + assert result["status"] == "done" + + tasks = _reload(tasks_file) + task = tasks["1"] + assert task["status"] == "done" + assert task["doneEvidence"]["evidence_ref"] == "card.json" + assert "at" in task["doneEvidence"] + assert task["reachability"]["verdict"] == "WIRED" + assert task["reachability"]["start_commit"] == "abc123" + + +# --------------------------------------------------------------------------- +# 5. EXEMPT verdict passes +# --------------------------------------------------------------------------- + + +def test_wired_exempt_verdict_passes(tmp_path, monkeypatch): + """wired task + EXEMPT verdict → ok.""" + tasks_file = _write_project(tmp_path, [_task(1, tier="wired")]) + monkeypatch.chdir(tmp_path) + + result = run_set_status( + "1", "done", reachability={"verdict": "EXEMPT", "reason": "entrypoint"} + ) + + assert result["ok"] is True + tasks = _reload(tasks_file) + assert tasks["1"]["status"] == "done" + assert tasks["1"]["reachability"]["verdict"] == "EXEMPT" + + +def test_live_exempt_verdict_passes(tmp_path, monkeypatch): + """live tier + EXEMPT verdict → ok.""" + tasks_file = _write_project(tmp_path, [_task(1, tier="live")]) + monkeypatch.chdir(tmp_path) + + result = run_set_status("1", "done", reachability={"verdict": "EXEMPT"}) + + assert result["ok"] is True + tasks = _reload(tasks_file) + assert tasks["1"]["status"] == "done" + + +# --------------------------------------------------------------------------- +# 6. Non-done transitions on wired task → no gate, no raise +# --------------------------------------------------------------------------- + + +def test_wired_in_progress_no_gate(tmp_path, monkeypatch): + """set-status in-progress on wired task → ok without reachability.""" + tasks_file = _write_project(tmp_path, [_task(1, tier="wired")]) + monkeypatch.chdir(tmp_path) + + result = run_set_status("1", "in-progress") + + assert result["ok"] is True + tasks = _reload(tasks_file) + assert tasks["1"]["status"] == "in-progress" + # No reachability persisted + assert "reachability" not in tasks["1"] + + +def test_wired_blocked_no_gate(tmp_path, monkeypatch): + """set-status blocked on wired task → ok without reachability.""" + tasks_file = _write_project(tmp_path, [_task(1, tier="wired")]) + monkeypatch.chdir(tmp_path) + + result = run_set_status("1", "blocked") + + assert result["ok"] is True + tasks = _reload(tasks_file) + assert tasks["1"]["status"] == "blocked" + + +def test_wired_review_no_gate(tmp_path, monkeypatch): + """set-status review on wired task → ok without reachability.""" + tasks_file = _write_project(tmp_path, [_task(1, tier="wired")]) + monkeypatch.chdir(tmp_path) + + result = run_set_status("1", "review") + + assert result["ok"] is True + + +# --------------------------------------------------------------------------- +# 7. Subtask path unbroken (subtask done never goes through tier gate) +# --------------------------------------------------------------------------- + + +def test_subtask_done_unaffected(tmp_path, monkeypatch): + """Subtask done flip is not tier-gated (subtasks have no tier field).""" + tasks_file = _write_project( + tmp_path, + [ + { + "id": 1, + "title": "Wired parent", + "description": "", + "details": "", + "testStrategy": "", + "status": "in-progress", + "priority": "medium", + "dependencies": [], + "phaseConfig": {"tier": "wired"}, + "subtasks": [ + { + "id": 1, + "title": "Subtask 1", + "description": "", + "details": "", + "status": "pending", + "dependencies": [], + } + ], + } + ], + ) + monkeypatch.chdir(tmp_path) + + # Subtask done should NOT raise even though parent is wired + result = run_set_status("1.1", "done") + + assert result["ok"] is True + assert result["kind"] == "subtask" + raw = json.loads(tasks_file.read_text()) + subtask = raw["master"]["tasks"][0]["subtasks"][0] + assert subtask["status"] == "done" + + +# --------------------------------------------------------------------------- +# 8. evidence_ref persisted on domain-model task (non-gated but evidence stored) +# --------------------------------------------------------------------------- + + +def test_domain_model_evidence_persisted(tmp_path, monkeypatch): + """evidence_ref is stored even for non-gated tiers when provided.""" + tasks_file = _write_project(tmp_path, [_task(1, tier="domain-model")]) + monkeypatch.chdir(tmp_path) + + result = run_set_status("1", "done", evidence_ref="proof.json") + + assert result["ok"] is True + tasks = _reload(tasks_file) + assert tasks["1"]["doneEvidence"]["evidence_ref"] == "proof.json" + assert "at" in tasks["1"]["doneEvidence"] From 562edb325359c87784e88d7135b20df4d05773f8 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 16 Jun 2026 20:37:10 +0800 Subject: [PATCH 09/13] feat(engine): reachability-sweep CLI writes CDD verdict; set-status auto-reads it; execute-task sweeps + auto-downgrades orphans MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Part A: new `prd-taskmaster reachability-sweep --task --start-commit ` subcommand (prd_taskmaster/reachability_cmd.py + cli.py wiring). Loads task, runs sweep_task(), writes verdict dict into .atlas-ai/cdd/task-.json under "reachability" key (atomic, additive). exit 0 for WIRED/EXEMPT, exit 1 for ORPHAN/ERROR. - Part B: `set-status` gains --evidence-ref and --reachability flags. Auto-read fallback in run_set_status(): when marking done with no explicit reachability, reads the verdict from the CDD card — so Step 9b sweep → Step 10 mark-done works without extra flags. ORPHAN/ERROR from auto-read still blocks (RA5 gate preserved). Untiered/domain-model tasks unchanged. - Part C: execute-task/SKILL.md updated — documents task_start_sha capture, Step 9b reachability sweep (mandatory for wired/live), and Step 10 branching: WIRED/EXEMPT → done, ORPHAN/ERROR → auto-downgrade to scaffold + continue loop. Throughline: "a green test on a module imported by nothing is not done — wire it or it ships as scaffold." - 27 new tests in tests/core/test_reachability_sweep_cmd.py; full suite 544 passed. Co-Authored-By: Claude Opus 4.8 --- prd_taskmaster/cli.py | 33 ++ prd_taskmaster/reachability_cmd.py | 190 ++++++++ prd_taskmaster/task_state.py | 104 ++++- skills/execute-task/SKILL.md | 93 +++- tests/core/test_reachability_sweep_cmd.py | 523 ++++++++++++++++++++++ 5 files changed, 938 insertions(+), 5 deletions(-) create mode 100644 prd_taskmaster/reachability_cmd.py create mode 100644 tests/core/test_reachability_sweep_cmd.py diff --git a/prd_taskmaster/cli.py b/prd_taskmaster/cli.py index 6e149ab..6a06492 100644 --- a/prd_taskmaster/cli.py +++ b/prd_taskmaster/cli.py @@ -18,6 +18,7 @@ from prd_taskmaster.feedback import HARNESS_CHOICES, cmd_feedback_add, cmd_feedback_report from prd_taskmaster.context_pack import build_context_pack from prd_taskmaster import fleet, parallel, task_state, tm_parallel +from prd_taskmaster.reachability_cmd import cmd_reachability_sweep def _backend_source() -> str: @@ -322,6 +323,37 @@ def build_parser() -> argparse.ArgumentParser: p.add_argument("--id", required=True) p.add_argument("--status", required=True) p.add_argument("--tag") + p.add_argument( + "--evidence-ref", + default=None, + help="Path or ref to the CDD evidence card for this task", + ) + p.add_argument( + "--reachability", + default=None, + help=( + "Reachability verdict: bare string (WIRED|EXEMPT|ORPHAN) or a JSON dict. " + "When omitted and marking done, the verdict is auto-read from the task's " + "CDD card .atlas-ai/cdd/task-.json if present." + ), + ) + + # reachability-sweep + p = sub.add_parser( + "reachability-sweep", + help="Run the reachability sweep for a task and write the verdict into its CDD card", + ) + p.add_argument("--task", required=True, help="Task id (e.g. 1 or 1.2)") + p.add_argument( + "--start-commit", + required=True, + help="Git SHA recorded when work on this task began (git rev-parse HEAD at task start)", + ) + p.add_argument( + "--cwd", + default=None, + help="Explicit repo root (defaults to the current working directory)", + ) # status — render progress panels p = sub.add_parser("status", help="Render Atlas progress panels for the current phase") @@ -379,6 +411,7 @@ def cmd_status(args) -> None: "next-task": task_state.cmd_next_task, "claim-task": task_state.cmd_claim_task, "set-status": task_state.cmd_set_status, + "reachability-sweep": cmd_reachability_sweep, "economy-report": cmd_economy_report, "context-pack": cmd_context_pack, "feedback-add": cmd_feedback_add, diff --git a/prd_taskmaster/reachability_cmd.py b/prd_taskmaster/reachability_cmd.py new file mode 100644 index 0000000..2283f7e --- /dev/null +++ b/prd_taskmaster/reachability_cmd.py @@ -0,0 +1,190 @@ +"""CLI command core for `reachability-sweep`. + +run_reachability_sweep(task_id, start_commit, cwd=None) -> dict + - Loads the task from .taskmaster/tasks/tasks.json + - Runs reachability.sweep_task(repo_root, task, start_commit) + - Writes the verdict dict into the task's CDD card + .atlas-ai/cdd/task-.json under the "reachability" key + (atomic, additive — preserves all other card keys) + - Returns the full sweep verdict dict + +Exit code convention (enforced by cmd_reachability_sweep): + 0 → verdict in {WIRED, EXEMPT} + 1 → verdict in {ORPHAN, ERROR} or any CommandError + +CDD card format written: + { + ...existing card keys..., + "reachability": { + "verdict": "WIRED" | "ORPHAN" | "EXEMPT" | "ERROR", + "tier": str, + "modules": [...], + "checked_at": str (ISO-8601), + "start_commit": str, + ... + } + } +""" + +from __future__ import annotations + +import json +import os +from pathlib import Path +from typing import Any + +from prd_taskmaster import parallel +from prd_taskmaster.lib import CommandError, atomic_write +from prd_taskmaster.reachability import sweep_task + +# Verdicts that map to exit 0. +_PASS_VERDICTS = {"WIRED", "EXEMPT"} + + +def _cdd_dir(repo_root: Path) -> Path: + return repo_root / ".atlas-ai" / "cdd" + + +def _card_path(repo_root: Path, task_id: str) -> "Path | None": + """Return the CDD card path for *task_id*, or None if it doesn't exist. + + Mirrors ship-check._card_path_for: prefers task-.json; falls back to + combined cards whose hyphen-separated id-list contains the id. + """ + cdd = _cdd_dir(repo_root) + if not cdd.exists(): + return None + + direct = cdd / f"task-{task_id}.json" + if direct.exists(): + return direct + + for card in cdd.glob("task-*.json"): + stem = card.stem + if not stem.startswith("task-"): + continue + ids = stem[len("task-"):].split("-") + if task_id in ids: + return card + + return None + + +def _load_task(repo_root: Path, task_id: str) -> dict: + """Load the task dict from .taskmaster/tasks/tasks.json. + + Looks for task_id in every tag's tasks list (and the flat-tasks fallback). + Raises CommandError if not found. + """ + tasks_path = repo_root / ".taskmaster" / "tasks" / "tasks.json" + if not tasks_path.exists(): + raise CommandError(f"tasks.json not found at {tasks_path}") + + try: + raw = json.loads(tasks_path.read_text()) + except json.JSONDecodeError as exc: + raise CommandError(f"tasks.json is invalid JSON: {exc}") from exc + + if not isinstance(raw, dict): + raise CommandError("tasks.json root must be an object") + + # Search all tag namespaces (plus flat "tasks" list). + candidates: list[dict] = [] + if isinstance(raw.get("tasks"), list): + candidates.extend(raw["tasks"]) + for value in raw.values(): + if isinstance(value, dict) and isinstance(value.get("tasks"), list): + candidates.extend(value["tasks"]) + + for task in candidates: + if str(task.get("id")) == task_id: + return task + + raise CommandError(f"task {task_id!r} not found in tasks.json") + + +def run_reachability_sweep( + task_id: str, + start_commit: str, + cwd: "str | None" = None, +) -> dict: + """Run the reachability sweep for *task_id* and write the result into its CDD card. + + Parameters + ---------- + task_id: + The task id (string, may be "1" or "1.2" — only parent IDs supported for sweep). + start_commit: + The git sha to compare HEAD against (the sha recorded when work started on the task). + cwd: + Optional explicit repo root. Defaults to the current working directory. + + Returns + ------- + The sweep verdict dict from reachability.sweep_task. + + Raises + ------ + CommandError + If the task is not found, or if the CDD card cannot be updated. + """ + repo_root = Path(cwd).resolve() if cwd else Path.cwd().resolve() + + # 1. Load the task. + task = _load_task(repo_root, task_id) + + # 2. Run the sweep. + verdict = sweep_task(repo_root, task, start_commit) + + # 3. Write the verdict into the CDD card, additively. + card_path = _card_path(repo_root, task_id) + if card_path is None: + # Attempt to use the direct path if it doesn't exist yet — but only if + # the cdd directory itself exists (the task must have a card already). + cdd = _cdd_dir(repo_root) + if not cdd.exists(): + raise CommandError( + f"task {task_id}: .atlas-ai/cdd/ directory does not exist — " + f"generate the CDD card (execute-task Step 5) before running the sweep" + ) + # No existing card: raise with a clear message. + raise CommandError( + f"task {task_id}: no CDD card found in .atlas-ai/cdd/ — " + f"generate the CDD card (execute-task Step 5) before running the sweep" + ) + + # Read existing card, add "reachability", write back atomically. + try: + existing: dict[str, Any] = json.loads(card_path.read_text()) + except (OSError, json.JSONDecodeError) as exc: + raise CommandError( + f"task {task_id}: cannot read CDD card at {card_path}: {exc}" + ) from exc + + existing["reachability"] = verdict + atomic_write(card_path, json.dumps(existing, indent=2, default=str)) + + return verdict + + +# ─── CLI wrapper ────────────────────────────────────────────────────────────── + +def cmd_reachability_sweep(args) -> None: + """CLI entry point for `prd-taskmaster reachability-sweep`.""" + import json as _json + import sys as _sys + + from prd_taskmaster.lib import fail + + try: + verdict = run_reachability_sweep( + task_id=args.task, + start_commit=args.start_commit, + cwd=getattr(args, "cwd", None), + ) + except CommandError as exc: + fail(exc.message, **exc.extra) + return # never reached; fail() exits + + print(_json.dumps(verdict, indent=2, default=str)) + _sys.exit(0 if verdict.get("verdict") in _PASS_VERDICTS else 1) diff --git a/prd_taskmaster/task_state.py b/prd_taskmaster/task_state.py index 507a02b..625b2c9 100644 --- a/prd_taskmaster/task_state.py +++ b/prd_taskmaster/task_state.py @@ -257,6 +257,12 @@ def run_set_status( resolved_tag = parallel.current_tag(tag) result: dict[str, Any] = {} + # Auto-read reachability from CDD card when marking done without an explicit verdict. + # This allows `set-status done` to work transparently after the sweep has run and + # written the verdict into the card (execute-task Step 9 → Step 10 flow). + if reachability is None and status == "done" and subtask_id is None: + reachability = _read_cdd_reachability(parent_id) + def transform(current: str) -> str: if not current.strip(): raise CommandError(f"{parallel.TASKS} not found") @@ -350,8 +356,104 @@ def cmd_claim_task(args: argparse.Namespace) -> None: fail(exc.message, **exc.extra) +def _parse_reachability_arg(value: "str | None") -> "dict | None": + """Parse the --reachability CLI argument. + + Accepts: + - None → None (not provided) + - "WIRED" → {"verdict": "WIRED"} + - "EXEMPT" → {"verdict": "EXEMPT"} + - "ORPHAN" → {"verdict": "ORPHAN"} + - '{"verdict":…}' → parsed JSON dict + + Raises CommandError on invalid input. + """ + if value is None: + return None + value = value.strip() + if value.startswith("{"): + try: + parsed = json.loads(value) + except json.JSONDecodeError as exc: + raise CommandError(f"--reachability: invalid JSON: {exc}") from exc + if not isinstance(parsed, dict): + raise CommandError("--reachability: JSON value must be an object") + return parsed + # Bare verdict string. + if value in ("WIRED", "EXEMPT", "ORPHAN", "ERROR"): + return {"verdict": value} + raise CommandError( + f"--reachability: expected WIRED, EXEMPT, ORPHAN, or a JSON dict; got {value!r}" + ) + + +def _read_cdd_reachability(task_id: str) -> "dict | None": + """Attempt to read the reachability block from the task's CDD card. + + Looks for .atlas-ai/cdd/task-.json (direct) or a combined card + whose hyphen-separated id-list contains the id (matching ship-check logic). + Returns the dict under the "reachability" key, or None if unavailable. + """ + from pathlib import Path as _Path + + cdd_dir = _Path(".atlas-ai") / "cdd" + if not cdd_dir.exists(): + return None + + tid_str = str(task_id) + # Direct card. + direct = cdd_dir / f"task-{tid_str}.json" + card_path = None + if direct.exists(): + card_path = direct + else: + # Combined card fallback. + for card in cdd_dir.glob("task-*.json"): + stem = card.stem + if not stem.startswith("task-"): + continue + ids = stem[len("task-"):].split("-") + if tid_str in ids: + card_path = card + break + + if card_path is None: + return None + + try: + card = json.loads(card_path.read_text()) + except (OSError, json.JSONDecodeError): + return None + + reach = card.get("reachability") + return reach if isinstance(reach, dict) else None + + def cmd_set_status(args: argparse.Namespace) -> None: + # Parse --reachability flag (bare verdict string or JSON dict). + try: + reachability = _parse_reachability_arg(getattr(args, "reachability", None)) + except CommandError as exc: + fail(exc.message, **exc.extra) + return + + # Auto-read fallback: if marking done and --reachability not given, try the CDD card. + if reachability is None and args.status == "done": + # Only the parent task id matters for CDD card lookup. + parent_id = str(args.id).split(".")[0] + reachability = _read_cdd_reachability(parent_id) + + evidence_ref = getattr(args, "evidence_ref", None) + try: - emit(run_set_status(args.id, args.status, getattr(args, "tag", None))) + emit( + run_set_status( + args.id, + args.status, + getattr(args, "tag", None), + evidence_ref=evidence_ref, + reachability=reachability, + ) + ) except CommandError as exc: fail(exc.message, **exc.extra) diff --git a/skills/execute-task/SKILL.md b/skills/execute-task/SKILL.md index 138b061..079eb1d 100644 --- a/skills/execute-task/SKILL.md +++ b/skills/execute-task/SKILL.md @@ -63,6 +63,18 @@ orchestrator's job. Each pass through this cycle moves exactly one TaskMaster task from `pending` to `done`. Do the 13 steps in order. Do not skip. +> **Task-start SHA** — at the very beginning of each iteration (before step 2), +> capture the current git HEAD: +> +> ```bash +> task_start_sha=$(git rev-parse HEAD) +> ``` +> +> Record `$task_start_sha` in the execute-log row for this iteration. It is the +> oracle of truth for every reachability sweep in step 9b below: "what modules +> did THIS task add?" is `diff $task_start_sha..HEAD`. The oracle flow already +> issues per-task start commits; this surfaces the same value in the loop prose. + 1. **Heartbeat check**: verify the execute-task heartbeat timer is running. If missing, register one via `CronCreate("execute-task-heartbeat", "* * * * *", "echo heartbeat")`. Abort the iteration if the timer cannot be created — a missing heartbeat @@ -180,7 +192,37 @@ to `done`. Do the 13 steps in order. Do not skip. in ai-human-tasker was marked DONE while `pnpm test` exited 1 with 11 failing tests.) - The three checks (run only if the hard gate passes): + **9b. Reachability sweep (MANDATORY for wired/live tasks).** After the + hard exit-code gate passes, run the reachability sweep for this task: + + ```bash + python3 script.py reachability-sweep \ + --task \ + --start-commit + ``` + + This command: + - Inspects every source module added between `$task_start_sha` and `HEAD`. + - Computes a per-task verdict: `WIRED`, `EXEMPT`, `ORPHAN`, or `ERROR`. + - **Writes the verdict dict** into the task's CDD card + `.atlas-ai/cdd/task-.json` under the `"reachability"` key (atomic, + additive — existing card keys are preserved). + + The sweep exit code encodes the verdict: + - `exit 0` → WIRED or EXEMPT (pass; proceed to the three checkers). + - `exit 1` → ORPHAN or ERROR (see step 10 for the auto-downgrade path). + + For spike/domain-model tasks the sweep returns EXEMPT automatically (no + importer search is performed for those tiers). + + > **Why sweep before the triple check?** A green test on a module + > imported by nothing is not "done" — it is scaffolded. The triple check + > can pass for an ORPHAN module (all tests pass; doubt and validate agree). + > The reachability gate closes that gap: `done` means the module is + > reachable from real production callsites, not just reachable from tests. + > Wire it or it ships as scaffold. + + The three checks (run only if the hard gate AND the reachability sweep both pass): - Plugin-native check: evidence file count vs declared subtask count (from the CDD card in step 5). Missing evidence = fail. @@ -193,9 +235,21 @@ to `done`. Do the 13 steps in order. Do not skip. 3+ agree pass -> task passes. Disagreement -> halt this iteration, surface to inbox. -10. **Mark done + propagate state**: - a. Run backend op `set-status` for the parent task: - `python3 script.py set-status --id --status done`. +10. **Mark done + propagate state** — branch on the sweep verdict from step 9b: + + **WIRED or EXEMPT** (sweep exit 0) → proceed normally: + + a. Run backend op `set-status` for the parent task. Because the sweep + already wrote the `reachability` block into the CDD card, the + `set-status` CLI auto-reads it — no `--reachability` flag needed: + + ```bash + python3 script.py set-status --id --status done + ``` + + If you want to be explicit (e.g. for logging), you may pass: + `--reachability WIRED` or `--reachability EXEMPT`. + b. **Subtask writeback**: for each subtask `S` in `task.subtasks` whose evidence file (per the CDD card from step 5) exists, run `python3 script.py set-status --id . --status done`. Subtasks left @@ -203,6 +257,7 @@ to `done`. Do the 13 steps in order. Do not skip. that breaks any tool computing progress from subtask state. (Codified 2026-06-04 — yesterday's run left all 39 subtasks `pending` despite 13/13 parent tasks `done`.) + c. Update `.atlas-ai/state/pipeline.json` per-task: call `mcp__plugin_prd_go__update_pipeline_task_status(task_id=, status="done")` if the MCP tool is available. If not, fall back to @@ -214,6 +269,36 @@ to `done`. Do the 13 steps in order. Do not skip. SKILL.md but never executed it. pipeline.json froze at HANDOFF transition through all 85 minutes of execution.) + **ORPHAN or ERROR** (sweep exit 1) → **auto-downgrade to scaffold**: + + Do NOT mark the task `done`. Instead: + + ```bash + python3 script.py set-status --id --status scaffold + ``` + + Then: + - Log to `execute-log.jsonl`: `"reachability_verdict": "ORPHAN"` (or + `"ERROR"`), `"auto_downgraded": true`, and a plain-English note of + which modules are unwired (from the sweep's `modules` list in the + CDD card). + - **Do NOT halt the loop** — continue to the next task (step 1). + An ORPHAN module is scaffolded work, not blocked work. The ship + gate (Gate 6, RA3) will report it honestly as `scaffold`, not `done`. + - If you need to wire the module, create a follow-up task + (`title: "Wire into "`) and append it via + `python3 script.py expand --id ` or the MCP equivalent. + + > **Throughline:** a green test on a module imported by nothing is not + > done — wire it or it ships as scaffold. The auto-downgrade ensures + > the task graph stays honest: Gate 6 will block the ship until every + > wired/live task's reachability block reads WIRED or EXEMPT. If all + > wired/live tasks auto-downgraded to scaffold, the ship check will + > block at Gate 2 ("not every task is done") and the developer must + > choose: wire the modules, re-tier them (spike/domain-model), or mark + > them explicitly exempt (`reachableVia: cli:...`). There is no silent + > path to SHIP_CHECK_OK with an unwired module at a wired/live tier. + 11. **Check stepback triggers**: if 15 minutes have passed with no task moving to done, OR 5 consecutive iterations have failed on the same task class, the recon escalation ladder is MANDATORY. Climb the ladder diff --git a/tests/core/test_reachability_sweep_cmd.py b/tests/core/test_reachability_sweep_cmd.py new file mode 100644 index 0000000..e404e20 --- /dev/null +++ b/tests/core/test_reachability_sweep_cmd.py @@ -0,0 +1,523 @@ +"""TDD: reachability-sweep CLI (Part A) + cmd_set_status auto-read (Part B). + +All tests that exercise WIRED vs ORPHAN build real git repos so that the +grep / git logic runs against actual source content. No mocking of the +load-bearing sweep calls. + +Coverage: + Part A — run_reachability_sweep + A1. WIRED task → writes reachability.verdict==WIRED into CDD card, returns dict. + A2. ORPHAN wired task → writes ORPHAN into card; caller should exit 1 (verdict check). + A3. CDD card additive: other card keys preserved after sweep writes reachability. + A4. No CDD card → CommandError (clear message). + A5. No tasks.json → CommandError. + A6. EXEMPT task (tier-exempt) → writes EXEMPT into card. + + Part B — cmd_set_status auto-read + B1. wired task whose CDD card has verdict==WIRED → set-status done succeeds (no flag). + B2. wired task whose CDD card has verdict==ORPHAN → raises CommandError. + B3. --reachability WIRED explicit → passes. + B4. --reachability JSON dict explicit → passes. + B5. Backward-compat: untiered set-status done via CLI still works with no flags. + B6. wired task, no CDD card, no --reachability → CommandError (RA5 gate, no card to auto-read). +""" + +from __future__ import annotations + +import json +import subprocess +from pathlib import Path + +import pytest + +from prd_taskmaster.lib import CommandError +from prd_taskmaster.reachability_cmd import run_reachability_sweep +from prd_taskmaster.task_state import ( + _parse_reachability_arg, + _read_cdd_reachability, + run_set_status, +) + + +# ─── Git repo helpers (mirrors test_reachability.py) ───────────────────────── + + +def _git(repo: Path, *args: str) -> str: + result = subprocess.run( + ["git", "-C", str(repo), *args], + capture_output=True, + text=True, + check=True, + ) + return result.stdout.strip() + + +def _init_repo(tmp_path: Path) -> Path: + repo = tmp_path / "repo" + repo.mkdir() + _git(repo, "init") + _git(repo, "config", "user.email", "test@example.com") + _git(repo, "config", "user.name", "Test") + return repo + + +def _commit_all(repo: Path, message: str) -> str: + _git(repo, "add", "-A") + _git(repo, "commit", "-m", message) + return _git(repo, "rev-parse", "HEAD") + + +def _make_wired_py_repo(tmp_path: Path) -> tuple[Path, str, str, str]: + """Build a Python repo where pkg/foo.py is WIRED (imported by pkg/app.py). + + Commit 1 (start): pyproject.toml + pkg/__init__.py + pkg/app.py (importer) + Commit 2 (head): pkg/foo.py added (new module) + + Returns (repo, start_sha, head_sha, task_id). + """ + repo = _init_repo(tmp_path) + (repo / "pyproject.toml").write_text("[project]\nname = 'mypkg'\n") + pkg = repo / "pkg" + pkg.mkdir() + (pkg / "__init__.py").write_text("") + (pkg / "app.py").write_text("from pkg import foo\n") + tests_dir = repo / "tests" + tests_dir.mkdir() + (tests_dir / "__init__.py").write_text("") + start = _commit_all(repo, "initial") + + (pkg / "foo.py").write_text("def hello():\n return 'hello'\n") + (tests_dir / "test_foo.py").write_text( + "from pkg.foo import hello\ndef test_hello():\n assert hello() == 'hello'\n" + ) + head = _commit_all(repo, "add foo") + return repo, start, head, "1" + + +def _make_orphan_py_repo(tmp_path: Path) -> tuple[Path, str, str, str]: + """Build a Python repo where pkg/foo.py is ORPHAN (imported by nothing non-test). + + Commit 1 (start): pyproject.toml + pkg/__init__.py + Commit 2 (head): pkg/foo.py added (new module, not imported by any non-test file) + """ + repo = _init_repo(tmp_path) + (repo / "pyproject.toml").write_text("[project]\nname = 'mypkg'\n") + pkg = repo / "pkg" + pkg.mkdir() + (pkg / "__init__.py").write_text("") + tests_dir = repo / "tests" + tests_dir.mkdir() + (tests_dir / "__init__.py").write_text("") + start = _commit_all(repo, "initial") + + (pkg / "foo.py").write_text("def hello():\n return 'hello'\n") + (tests_dir / "test_foo.py").write_text( + "from pkg.foo import hello\ndef test_hello():\n assert hello() == 'hello'\n" + ) + head = _commit_all(repo, "add orphan foo") + return repo, start, head, "1" + + +def _write_tasks(repo: Path, tasks: list, *, tag: str = "master") -> Path: + """Write a tagged tasks.json under .taskmaster/tasks/.""" + payload = { + tag: { + "tasks": tasks, + "metadata": {"created": "2026-01-01T00:00:00Z", "updated": "2026-01-01T00:00:00Z"}, + } + } + tasks_dir = repo / ".taskmaster" / "tasks" + tasks_dir.mkdir(parents=True, exist_ok=True) + (repo / ".taskmaster" / "state.json").write_text( + json.dumps({"currentTag": tag, "migrationNoticeShown": True}) + ) + f = tasks_dir / "tasks.json" + f.write_text(json.dumps(payload, indent=2)) + return f + + +def _make_task(task_id: int | str, *, tier: str | None = None) -> dict: + t: dict = { + "id": task_id, + "title": f"Task {task_id}", + "description": "desc", + "details": "details", + "testStrategy": "test", + "status": "in-progress", + "priority": "medium", + "dependencies": [], + "subtasks": [], + } + if tier is not None: + t["phaseConfig"] = {"tier": tier} + return t + + +def _write_cdd_card(repo: Path, task_id: str, extra: dict | None = None) -> Path: + """Write a minimal CDD card for *task_id*, optionally merging *extra* fields.""" + cdd_dir = repo / ".atlas-ai" / "cdd" + cdd_dir.mkdir(parents=True, exist_ok=True) + card: dict = { + "task_id": task_id, + "title": f"CDD card for task {task_id}", + "testing_plan": [], + } + if extra: + card.update(extra) + path = cdd_dir / f"task-{task_id}.json" + path.write_text(json.dumps(card, indent=2)) + return path + + +# ─── Part A: run_reachability_sweep ────────────────────────────────────────── + + +class TestRunReachabilitySweepWired: + def test_wired_module_writes_wired_verdict_to_card(self, tmp_path): + """WIRED task: sweep returns WIRED and writes it into the CDD card.""" + repo, start, head, tid = _make_wired_py_repo(tmp_path) + _write_tasks(repo, [_make_task(1, tier="wired")]) + card_path = _write_cdd_card(repo, tid) + + result = run_reachability_sweep(tid, start, cwd=str(repo)) + + assert result["verdict"] == "WIRED", f"expected WIRED, got: {result}" + + # Card must have the reachability block. + card = json.loads(card_path.read_text()) + assert "reachability" in card + assert card["reachability"]["verdict"] == "WIRED" + assert card["reachability"]["start_commit"] == start + # Original card keys preserved. + assert card["task_id"] == tid + assert card["title"] == f"CDD card for task {tid}" + + def test_wired_sweep_returns_full_dict(self, tmp_path): + """run_reachability_sweep return value has all expected keys.""" + repo, start, _head, tid = _make_wired_py_repo(tmp_path) + _write_tasks(repo, [_make_task(1, tier="wired")]) + _write_cdd_card(repo, tid) + + result = run_reachability_sweep(tid, start, cwd=str(repo)) + + assert "verdict" in result + assert "tier" in result + assert "modules" in result + assert "checked_at" in result + assert "start_commit" in result + + +class TestRunReachabilitySweepOrphan: + def test_orphan_module_writes_orphan_verdict_to_card(self, tmp_path): + """ORPHAN task: sweep writes ORPHAN to CDD card.""" + repo, start, _head, tid = _make_orphan_py_repo(tmp_path) + _write_tasks(repo, [_make_task(1, tier="wired")]) + card_path = _write_cdd_card(repo, tid) + + result = run_reachability_sweep(tid, start, cwd=str(repo)) + + assert result["verdict"] == "ORPHAN", f"expected ORPHAN, got: {result}" + + card = json.loads(card_path.read_text()) + assert card["reachability"]["verdict"] == "ORPHAN" + + def test_orphan_verdict_is_not_pass(self, tmp_path): + """Confirm ORPHAN is not in the passing set (exit-1 contract).""" + from prd_taskmaster.reachability_cmd import _PASS_VERDICTS + assert "ORPHAN" not in _PASS_VERDICTS + assert "ERROR" not in _PASS_VERDICTS + + +class TestRunReachabilitySweepAdditiveCard: + def test_existing_card_keys_preserved(self, tmp_path): + """Sweep write is additive: other card keys survive.""" + repo, start, _head, tid = _make_wired_py_repo(tmp_path) + _write_tasks(repo, [_make_task(1, tier="wired")]) + card_path = _write_cdd_card(repo, tid, extra={ + "testing_plan": [{"check": "unit tests pass", "evidence": "pytest"}], + "grading": {"grade": "A", "score": 10}, + "custom_field": "preserve-me", + }) + + run_reachability_sweep(tid, start, cwd=str(repo)) + + card = json.loads(card_path.read_text()) + # All pre-existing keys still present. + assert card["task_id"] == tid + assert card["custom_field"] == "preserve-me" + assert card["grading"]["grade"] == "A" + assert len(card["testing_plan"]) == 1 + # Reachability block written. + assert "reachability" in card + + +class TestRunReachabilitySweepExempt: + def test_tier_exempt_writes_exempt_verdict(self, tmp_path): + """spike/domain-model tier → EXEMPT verdict written to CDD card.""" + repo = _init_repo(tmp_path) + (repo / "pyproject.toml").write_text("[project]\nname = 'mypkg'\n") + _commit_all(repo, "initial") + start = _git(repo, "rev-parse", "HEAD") + + _write_tasks(repo, [_make_task(1, tier="spike")]) + card_path = _write_cdd_card(repo, "1") + + result = run_reachability_sweep("1", start, cwd=str(repo)) + + assert result["verdict"] == "EXEMPT" + card = json.loads(card_path.read_text()) + assert card["reachability"]["verdict"] == "EXEMPT" + + +class TestRunReachabilitySweepErrors: + def test_no_cdd_card_raises_command_error(self, tmp_path): + """If no CDD card exists, raise CommandError with a clear message.""" + repo, start, _head, tid = _make_wired_py_repo(tmp_path) + _write_tasks(repo, [_make_task(1, tier="wired")]) + # Create the cdd dir but not the card. + (repo / ".atlas-ai" / "cdd").mkdir(parents=True) + + with pytest.raises(CommandError) as exc_info: + run_reachability_sweep(tid, start, cwd=str(repo)) + + assert "no CDD card" in exc_info.value.message or "CDD card" in exc_info.value.message + + def test_no_tasks_json_raises_command_error(self, tmp_path): + """If tasks.json is missing, raise CommandError.""" + repo = _init_repo(tmp_path) + (repo / "pyproject.toml").write_text("[project]\nname='x'\n") + _commit_all(repo, "init") + start = _git(repo, "rev-parse", "HEAD") + _write_cdd_card(repo, "1") + + with pytest.raises(CommandError) as exc_info: + run_reachability_sweep("1", start, cwd=str(repo)) + + assert "tasks.json" in exc_info.value.message + + def test_task_not_found_raises_command_error(self, tmp_path): + """If task id not in tasks.json, raise CommandError.""" + repo, start, _head, _tid = _make_wired_py_repo(tmp_path) + _write_tasks(repo, [_make_task(99, tier="wired")]) + _write_cdd_card(repo, "1") + + with pytest.raises(CommandError) as exc_info: + run_reachability_sweep("1", start, cwd=str(repo)) + + assert "not found" in exc_info.value.message + + +# ─── Part B: cmd_set_status auto-read ──────────────────────────────────────── + + +def _write_project(tmp_path: Path, tasks: list, *, tag: str = "master") -> Path: + """Write .taskmaster/tasks/tasks.json and state.json.""" + payload = { + tag: { + "tasks": tasks, + "metadata": {"created": "2026-01-01T00:00:00Z"}, + } + } + tasks_dir = tmp_path / ".taskmaster" / "tasks" + tasks_dir.mkdir(parents=True) + f = tasks_dir / "tasks.json" + f.write_text(json.dumps(payload, indent=2)) + (tmp_path / ".taskmaster" / "state.json").write_text( + json.dumps({"currentTag": tag, "migrationNoticeShown": True}) + ) + return f + + +def _reload(tasks_file: Path, tag: str = "master") -> dict[str, dict]: + raw = json.loads(tasks_file.read_text()) + return {str(t["id"]): t for t in raw[tag]["tasks"]} + + +class TestParseReachabilityArg: + def test_none_returns_none(self): + assert _parse_reachability_arg(None) is None + + def test_bare_wired_returns_dict(self): + r = _parse_reachability_arg("WIRED") + assert r == {"verdict": "WIRED"} + + def test_bare_exempt_returns_dict(self): + r = _parse_reachability_arg("EXEMPT") + assert r == {"verdict": "EXEMPT"} + + def test_bare_orphan_returns_dict(self): + r = _parse_reachability_arg("ORPHAN") + assert r == {"verdict": "ORPHAN"} + + def test_json_dict_parsed(self): + r = _parse_reachability_arg('{"verdict": "WIRED", "tier": "wired"}') + assert r == {"verdict": "WIRED", "tier": "wired"} + + def test_invalid_bare_raises(self): + with pytest.raises(CommandError): + _parse_reachability_arg("UNKNOWN_VERDICT") + + def test_invalid_json_raises(self): + with pytest.raises(CommandError): + _parse_reachability_arg("{bad json}") + + +class TestReadCddReachability: + def test_reads_reachability_from_card(self, tmp_path, monkeypatch): + monkeypatch.chdir(tmp_path) + cdd_dir = tmp_path / ".atlas-ai" / "cdd" + cdd_dir.mkdir(parents=True) + card = {"task_id": "1", "reachability": {"verdict": "WIRED", "tier": "wired"}} + (cdd_dir / "task-1.json").write_text(json.dumps(card)) + + result = _read_cdd_reachability("1") + assert result == {"verdict": "WIRED", "tier": "wired"} + + def test_returns_none_when_no_card(self, tmp_path, monkeypatch): + monkeypatch.chdir(tmp_path) + (tmp_path / ".atlas-ai" / "cdd").mkdir(parents=True) + result = _read_cdd_reachability("42") + assert result is None + + def test_returns_none_when_no_reachability_block(self, tmp_path, monkeypatch): + monkeypatch.chdir(tmp_path) + cdd_dir = tmp_path / ".atlas-ai" / "cdd" + cdd_dir.mkdir(parents=True) + (cdd_dir / "task-1.json").write_text(json.dumps({"task_id": "1"})) + result = _read_cdd_reachability("1") + assert result is None + + +class TestSetStatusAutoRead: + def test_wired_task_auto_reads_wired_from_card(self, tmp_path, monkeypatch): + """wired task, CDD card has WIRED verdict → set-status done succeeds with no --reachability.""" + tasks_file = _write_project(tmp_path, [_make_task(1, tier="wired")]) + # Write CDD card with WIRED reachability already in place (as if sweep ran). + cdd_dir = tmp_path / ".atlas-ai" / "cdd" + cdd_dir.mkdir(parents=True) + card = { + "task_id": "1", + "reachability": {"verdict": "WIRED", "tier": "wired", "start_commit": "abc123"}, + } + (cdd_dir / "task-1.json").write_text(json.dumps(card)) + monkeypatch.chdir(tmp_path) + + # No --reachability flag → auto-reads WIRED from card. + result = run_set_status("1", "done") + + assert result["ok"] is True + assert result["status"] == "done" + tasks = _reload(tasks_file) + assert tasks["1"]["status"] == "done" + # run_set_status persists the auto-read reachability dict. + assert tasks["1"]["reachability"]["verdict"] == "WIRED" + + def test_wired_task_auto_reads_orphan_raises(self, tmp_path, monkeypatch): + """wired task, CDD card has ORPHAN verdict → set-status done raises. + + When the CDD card has ORPHAN, auto-read provides the ORPHAN dict to + run_set_status, which then fires the blocking-verdict gate. The error + message includes 'ORPHAN'. + """ + _write_project(tmp_path, [_make_task(1, tier="wired")]) + cdd_dir = tmp_path / ".atlas-ai" / "cdd" + cdd_dir.mkdir(parents=True) + card = { + "task_id": "1", + "reachability": {"verdict": "ORPHAN", "tier": "wired"}, + } + (cdd_dir / "task-1.json").write_text(json.dumps(card)) + monkeypatch.chdir(tmp_path) + + with pytest.raises(CommandError) as exc_info: + run_set_status("1", "done") + + # The gate reads the ORPHAN from the card and blocks with a message + # that references the blocking verdict. + msg = exc_info.value.message + assert "ORPHAN" in msg or "reachability" in msg.lower() + + def test_explicit_reachability_wired_passes(self, tmp_path, monkeypatch): + """--reachability WIRED explicit → passes (no card required).""" + tasks_file = _write_project(tmp_path, [_make_task(1, tier="wired")]) + monkeypatch.chdir(tmp_path) + + result = run_set_status("1", "done", reachability={"verdict": "WIRED"}) + + assert result["ok"] is True + tasks = _reload(tasks_file) + assert tasks["1"]["status"] == "done" + + def test_explicit_reachability_json_dict_passes(self, tmp_path, monkeypatch): + """Explicit reachability as a full dict → ok.""" + tasks_file = _write_project(tmp_path, [_make_task(1, tier="wired")]) + monkeypatch.chdir(tmp_path) + + sweep = {"verdict": "EXEMPT", "reason": "entrypoint", "tier": "wired"} + result = run_set_status("1", "done", reachability=sweep) + + assert result["ok"] is True + tasks = _reload(tasks_file) + assert tasks["1"]["reachability"]["verdict"] == "EXEMPT" + + def test_untiered_done_no_flags_still_works(self, tmp_path, monkeypatch): + """Backward-compat: untiered task done with no --reachability → ok.""" + tasks_file = _write_project(tmp_path, [_make_task(1)]) # no tier + monkeypatch.chdir(tmp_path) + + result = run_set_status("1", "done") + + assert result["ok"] is True + tasks = _reload(tasks_file) + assert tasks["1"]["status"] == "done" + + def test_wired_task_no_card_no_reachability_raises(self, tmp_path, monkeypatch): + """wired task, no CDD card, no --reachability → CommandError (RA5 gate).""" + _write_project(tmp_path, [_make_task(1, tier="wired")]) + monkeypatch.chdir(tmp_path) + # No CDD card written — auto-read returns None → gate fires. + + with pytest.raises(CommandError) as exc_info: + run_set_status("1", "done") + + assert "reachability" in exc_info.value.message.lower() + + def test_wired_task_card_missing_reachability_block_raises(self, tmp_path, monkeypatch): + """wired task, CDD card exists but has no reachability key → CommandError.""" + _write_project(tmp_path, [_make_task(1, tier="wired")]) + cdd_dir = tmp_path / ".atlas-ai" / "cdd" + cdd_dir.mkdir(parents=True) + # Card without reachability block. + (cdd_dir / "task-1.json").write_text(json.dumps({"task_id": "1"})) + monkeypatch.chdir(tmp_path) + + with pytest.raises(CommandError) as exc_info: + run_set_status("1", "done") + + assert "reachability" in exc_info.value.message.lower() + + +class TestEndToEnd: + """Integration: sweep writes WIRED → set-status auto-reads → done.""" + + def test_sweep_then_set_status_done_e2e(self, tmp_path): + """Full end-to-end: sweep writes WIRED into card, then set-status reads it.""" + repo, start, _head, tid = _make_wired_py_repo(tmp_path) + _write_tasks(repo, [_make_task(1, tier="wired")]) + _write_cdd_card(repo, tid) + + # Step 1: sweep writes verdict. + sweep_result = run_reachability_sweep(tid, start, cwd=str(repo)) + assert sweep_result["verdict"] == "WIRED" + + # Step 2: set-status done (no explicit reachability) → auto-reads from card. + import os + old_cwd = os.getcwd() + try: + os.chdir(str(repo)) + result = run_set_status(tid, "done") + finally: + os.chdir(old_cwd) + + assert result["ok"] is True + assert result["status"] == "done" From dc10e5341535d2f505675730d1faf360d90a3614 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 16 Jun 2026 20:56:35 +0800 Subject: [PATCH 10/13] fix(engine): make scaffold a first-class status (auto-downgrade target); scaffold blocks ship gate MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Add "scaffold" to VALID_STATUSES in task_state.py so set-status --status scaffold no longer raises "unknown status" (the auto-downgrade path in execute-task Step 10 now works). - scaffold is NOT done, NOT deferred — a deliberate incomplete state for orphan modules; Gate 2 (gate_tasks requires every task done) already blocks any scaffold task from SHIP_CHECK_OK. - Count scaffold tasks in _count_tasks and surface them in execute_panel with a ⚠ line ("N scaffolded — orphan, blocks ship gate") for honest operator visibility. - Tests: set-status scaffold succeeds (API + CLI); scaffold task verified to block run_all_gates; VALID_STATUSES membership assertion; pipeline_state tag_counts dict updated for scaffold key. Co-Authored-By: Claude Opus 4.8 --- prd_taskmaster/pipeline.py | 2 + prd_taskmaster/render.py | 6 +++ prd_taskmaster/task_state.py | 2 + tests/core/test_pipeline_state.py | 2 +- tests/core/test_task_state.py | 66 +++++++++++++++++++++++++++++++ 5 files changed, 77 insertions(+), 1 deletion(-) diff --git a/prd_taskmaster/pipeline.py b/prd_taskmaster/pipeline.py index 10b746a..ecdaad8 100644 --- a/prd_taskmaster/pipeline.py +++ b/prd_taskmaster/pipeline.py @@ -148,10 +148,12 @@ def _tag_task_lists(tasks: dict) -> dict[str, list[dict]]: def _count_tasks(items: list[dict]) -> dict[str, int]: total = len(items) done = sum(1 for item in items if item.get("status") == "done") + scaffold = sum(1 for item in items if item.get("status") == "scaffold") return { "total": total, "pending": total - done, "done": done, + "scaffold": scaffold, } diff --git a/prd_taskmaster/render.py b/prd_taskmaster/render.py index e1c675d..b8b5d12 100644 --- a/prd_taskmaster/render.py +++ b/prd_taskmaster/render.py @@ -284,10 +284,16 @@ def _gate_tokens(i: int) -> list[str]: def execute_panel(task_counts: dict, *, ascii_mode: bool = False) -> str: total = task_counts.get("total", 0) done = task_counts.get("done", 0) + scaffold = task_counts.get("scaffold", 0) g = bar(done, total or 1, ascii_mode=ascii_mode) running = _g("running", ascii_mode) + warn = _g("warn", ascii_mode) lines = [ f"Progress {g} {done}/{total} tasks done", + ] + if scaffold: + lines.append(f"{warn} {scaffold} scaffolded (orphan — not wired, blocks ship gate)") + lines += [ "", f"{running} executing — evidence required before a task counts done.", ] diff --git a/prd_taskmaster/task_state.py b/prd_taskmaster/task_state.py index 625b2c9..3e6254f 100644 --- a/prd_taskmaster/task_state.py +++ b/prd_taskmaster/task_state.py @@ -23,6 +23,8 @@ "deferred", "cancelled", "blocked", + "scaffold", # auto-downgraded orphan: code exists but not wired into the system; + # NOT done, NOT deferred (deferred = deliberate); blocks the ship gate. } _PRIORITY_RANK = {"high": 0, "medium": 1, "low": 2} diff --git a/tests/core/test_pipeline_state.py b/tests/core/test_pipeline_state.py index bd6cfe8..0777c2c 100644 --- a/tests/core/test_pipeline_state.py +++ b/tests/core/test_pipeline_state.py @@ -103,7 +103,7 @@ def test_preflight_reads_standard_taskmaster_state_and_recommends_pending_tag(pr assert result["current_tag"] == "production-agent" assert result["task_count"] == 1 assert result["pending_task_count"] == 0 - assert result["tag_counts"]["master"] == {"total": 2, "pending": 1, "done": 1} + assert result["tag_counts"]["master"] == {"total": 2, "pending": 1, "done": 1, "scaffold": 0} assert result["recommended_tag"] == "master" assert result["recommended_action"] == "select_taskmaster_tag" diff --git a/tests/core/test_task_state.py b/tests/core/test_task_state.py index cdba2a2..250e815 100644 --- a/tests/core/test_task_state.py +++ b/tests/core/test_task_state.py @@ -440,3 +440,69 @@ def test_run_next_task_matches_live_taskmaster_in_progress_subtask(tmp_path, mon assert local["task"]["status"] == live["task"]["status"] assert local["task"]["parent_id"] == live["task"]["parentId"] assert local["task"]["parent_title"] == "Resume parent" + + +# ─── scaffold status tests ──────────────────────────────────────────────────── + + +def test_set_status_scaffold_succeeds(tmp_path, monkeypatch): + """set-status scaffold is accepted (auto-downgrade path no longer halts).""" + from prd_taskmaster.task_state import run_set_status + + tasks_file = _write_project(tmp_path, _tagged_payload([_task(1, status="done")])) + monkeypatch.chdir(tmp_path) + + result = run_set_status("1", "scaffold") + assert result["ok"] is True + assert result["status"] == "scaffold" + written = json.loads(tasks_file.read_text()) + assert written["master"]["tasks"][0]["status"] == "scaffold" + + +def test_set_status_scaffold_via_cli(tmp_path): + """CLI set-status --status scaffold exits 0 and writes the status.""" + tasks_file = _write_project(tmp_path, _tagged_payload([_task(1, status="done")])) + + code, out, err = _run_cli(tmp_path, "set-status", "--id", "1", "--status", "scaffold") + assert code == 0, f"stderr={err!r}" + data = json.loads(out) + assert data["status"] == "scaffold" + written = json.loads(tasks_file.read_text()) + assert written["master"]["tasks"][0]["status"] == "scaffold" + + +def test_scaffold_task_blocks_ship_gate(tmp_path): + """A project with one scaffold task must NOT yield SHIP_CHECK_OK (Gate 2 blocks).""" + from prd_taskmaster.shipcheck import run_all_gates + + # Write minimal project: Gates 1, 3, 4, 6 satisfied; only Gate 2 is the issue. + atlas = tmp_path / ".atlas-ai" + (atlas / "state").mkdir(parents=True) + (atlas / "state" / "pipeline.json").write_text( + json.dumps({"current_phase": "EXECUTE"}) + ) + tm = tmp_path / ".taskmaster" / "tasks" + tm.mkdir(parents=True) + # One scaffold task — status != "done" → Gate 2 must fail. + (tm / "tasks.json").write_text( + json.dumps({"master": {"tasks": [{"id": 1, "title": "Orphan", "status": "scaffold"}]}}) + ) + docs = tmp_path / ".taskmaster" / "docs" + docs.mkdir(parents=True) + (docs / "plan.md").write_text("# Plan\n") + # No CDD card needed — Gate 2 fires first for the scaffold task. + + ok, failures = run_all_gates(tmp_path) + assert ok is False, "scaffold task must block the ship gate" + assert any("scaffold" in f or "not done" in f for f in failures), ( + f"expected a Gate 2 failure mentioning scaffold or not done; got: {failures}" + ) + + +def test_scaffold_not_in_done_set(tmp_path, monkeypatch): + """scaffold is not treated as done — it must not satisfy the 'all done' ship gate.""" + from prd_taskmaster.task_state import VALID_STATUSES + + assert "scaffold" in VALID_STATUSES, "scaffold must be a valid status" + # Confirm it is NOT the 'done' status. + assert "scaffold" != "done" From 1750d99e8cc2eed5017cc0eeb346e8b801398350 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 16 Jun 2026 21:11:21 +0800 Subject: [PATCH 11/13] =?UTF-8?q?test(engine):=20e2e=20reachability=20loop?= =?UTF-8?q?=20=E2=80=94=20orphan=20blocks=20ship,=20wiring=20ships,=20scaf?= =?UTF-8?q?fold=20honest;=20orthogonal=20to=20oracle?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adds 11 genuine end-to-end tests spanning the full reachability lifecycle: real git repo, real reachability-sweep CLI, real ship-check subprocess, fake oracle stub via ATLAS_ORACLE_CMD so Gate 5 passes and Gate 6 is the deciding gate. Scenarios covered: - A: orphan module → sweep writes ORPHAN → ship-check exits 1, stderr names ORPHAN + task id (non-vacuous acceptance criterion) - B: oracle/test orthogonality — task's own test passes (oracle PASS) while reachability is ORPHAN, proving the two axes are independent (the user's exact bug: 10 of 19 'done' tasks were orphan dead code) - C: wire module (edit existing app.py to import Widget) → re-sweep writes WIRED → set-status done auto-reads it → ship-check prints SHIP_CHECK_OK - D: set-status scaffold → Gate 2 blocks honestly (not Gate 6), proving the scaffold path is an honest "not done" exit Bug fixed during e2e development (uncovered by real sweep): gate_reachability in skel/ship-check.py joined modules with ', '.join() but sweep_task writes module dicts, not strings — fixed to extract m["module"] from each dict entry. Co-Authored-By: Claude Opus 4.8 --- skel/ship-check.py | 8 +- tests/core/test_reachability_e2e.py | 580 ++++++++++++++++++++++++++++ 2 files changed, 587 insertions(+), 1 deletion(-) create mode 100644 tests/core/test_reachability_e2e.py diff --git a/skel/ship-check.py b/skel/ship-check.py index f7ed5b7..1066979 100755 --- a/skel/ship-check.py +++ b/skel/ship-check.py @@ -253,7 +253,13 @@ def gate_reachability(repo_root: Path, tasks: list) -> Tuple[bool, List[str]]: if verdict in _PASS_VERDICTS: continue elif verdict in _FAIL_VERDICTS: - modules = reach.get("modules", []) if isinstance(reach, dict) else [] + raw_modules = reach.get("modules", []) if isinstance(reach, dict) else [] + # modules may be plain strings (test fixtures / legacy) or dicts written + # by reachability.sweep_task with keys {verdict, module, importers, ...}. + modules = [ + m["module"] if isinstance(m, dict) else str(m) + for m in raw_modules + ] mod_str = f" ({', '.join(modules)})" if modules else "" failures.append( f"task {tid}: reachability {verdict}{mod_str} — wire the module(s)" diff --git a/tests/core/test_reachability_e2e.py b/tests/core/test_reachability_e2e.py new file mode 100644 index 0000000..942bd05 --- /dev/null +++ b/tests/core/test_reachability_e2e.py @@ -0,0 +1,580 @@ +"""E2E: Full reachability loop — orphan blocks ship, wiring ships, scaffold honest. + +This test drives the COMPLETE lifecycle end-to-end: + + 1. A real git project is constructed with Gates 1-4 satisfied and the oracle + stubbed PASS via ATLAS_ORACLE_CMD (Gate 5 always passes). + 2. A 'wired'-tier task exists. Its module is committed but imported only by + its own test — no production importer. + +Scenario A — Orphan blocks ship (the headline acceptance criterion): + * reachability-sweep → CDD card verdict == ORPHAN + * ship-check.py subprocess → exits 1, no SHIP_CHECK_OK, failure names the + ORPHAN and the task (non-vacuous: the task is wired-tier, so Gate 6 fires) + +Scenario B — Oracle / test orthogonality: + * The task's own test is GREEN (oracle stubbed PASS) — proving that a task + can pass its unit test yet still be ORPHAN: the two axes are independent. + * Gate 5 (oracle) passes; Gate 6 (reachability) is the sole blocker. + +Scenario C — Wire it → ships: + * A production importer is committed, sweep re-run → verdict WIRED + * set-status done succeeds (auto-reads WIRED from card) + * ship-check subprocess → SHIP_CHECK_OK, exit 0 + +Scenario D — Or scaffold → honest block: + * Starting from the ORPHAN state (task still in-progress), set-status scaffold + * ship-check subprocess → exits 1 (Gate 2: task not done), no SHIP_CHECK_OK + * No Gate 6 involvement — honest "not done" message + +ALL paths use real subprocess for ship-check. The reachability-sweep is called +via its Python API (run_reachability_sweep) — no mock of sweep internals. +""" +from __future__ import annotations + +import json +import os +import stat +import subprocess +from pathlib import Path + +import pytest + +from prd_taskmaster.lib import CommandError +from prd_taskmaster.reachability_cmd import run_reachability_sweep +from prd_taskmaster.task_state import run_set_status + +# Path to the real ship-check script (stdlib-only, ships into user projects). +REPO_ROOT = Path(__file__).resolve().parents[2] +SHIP = REPO_ROOT / "skel" / "ship-check.py" + + +# ─── Git repo helpers ───────────────────────────────────────────────────────── + +def _git(repo: Path, *args: str) -> str: + result = subprocess.run( + ["git", "-C", str(repo), *args], + capture_output=True, + text=True, + check=True, + ) + return result.stdout.strip() + + +def _init_repo(tmp_path: Path) -> Path: + repo = tmp_path / "repo" + repo.mkdir() + _git(repo, "init") + _git(repo, "config", "user.email", "test@example.com") + _git(repo, "config", "user.name", "Test") + return repo + + +def _commit_all(repo: Path, message: str) -> str: + _git(repo, "add", "-A") + _git(repo, "commit", "-m", message) + return _git(repo, "rev-parse", "HEAD") + + +# ─── Project scaffolding ────────────────────────────────────────────────────── + +TASK_ID = "42" +TASK_TITLE = "Implement widget module" + + +def _make_task(tier: str = "wired", status: str = "in-progress") -> dict: + """Build a minimal task dict for tasks.json.""" + return { + "id": int(TASK_ID), + "title": TASK_TITLE, + "description": "Implement the widget module", + "details": "details here", + "testStrategy": "unit test", + "status": status, + "priority": "medium", + "dependencies": [], + "subtasks": [], + "phaseConfig": {"tier": tier}, + } + + +def _write_tasks(repo: Path, task: dict, tag: str = "master") -> Path: + """Write .taskmaster/tasks/tasks.json and state.json.""" + payload = { + tag: { + "tasks": [task], + "metadata": {"created": "2026-01-01T00:00:00Z", "updated": "2026-01-01T00:00:00Z"}, + } + } + tasks_dir = repo / ".taskmaster" / "tasks" + tasks_dir.mkdir(parents=True, exist_ok=True) + (repo / ".taskmaster" / "state.json").write_text( + json.dumps({"currentTag": tag, "migrationNoticeShown": True}) + ) + path = tasks_dir / "tasks.json" + path.write_text(json.dumps(payload, indent=2)) + return path + + +def _write_cdd_card(repo: Path, task_id: str = TASK_ID) -> Path: + """Write the CDD card (with grading block so Gate 5 can run).""" + cdd_dir = repo / ".atlas-ai" / "cdd" + cdd_dir.mkdir(parents=True, exist_ok=True) + card = { + "task_id": task_id, + "title": TASK_TITLE, + "testing_plan": [{"check": "widget test passes", "evidence": "pytest"}], + "grading": {"command": ["sh", "grade.sh"]}, + } + path = cdd_dir / f"task-{task_id}.json" + path.write_text(json.dumps(card, indent=2)) + return path + + +def _write_gates_1_4(repo: Path) -> None: + """Write the Gates 1-4 supporting files.""" + # Gate 1: pipeline.json current_phase == EXECUTE + state_dir = repo / ".atlas-ai" / "state" + state_dir.mkdir(parents=True, exist_ok=True) + (state_dir / "pipeline.json").write_text( + json.dumps({"current_phase": "EXECUTE"}) + ) + # Gate 4: plan file + docs = repo / ".taskmaster" / "docs" + docs.mkdir(parents=True, exist_ok=True) + (docs / "plan.md").write_text("# Plan\nImplement the widget module.\n") + + +def _fake_oracle_script(tmp_path: Path) -> str: + """Return ATLAS_ORACLE_CMD string for a fake oracle that always emits PASS.""" + script = tmp_path / "fake_atlas.sh" + script.write_text( + "#!/bin/sh\n" + 'for a in "$@"; do\n' + ' if [ "$a" = "grade" ]; then\n' + ' printf \'{"verdict":"PASS"}\'\n' + " exit 0\n" + " fi\n" + "done\n" + "exit 0\n" + ) + script.chmod(script.stat().st_mode | stat.S_IXUSR | stat.S_IXGRP | stat.S_IXOTH) + return f"sh {script}" + + +def _run_ship_check(repo: Path, oracle_cmd: str) -> subprocess.CompletedProcess: + """Run ship-check.py as a subprocess against repo with a fake oracle.""" + env = dict(os.environ) + env["ATLAS_ORACLE_CMD"] = oracle_cmd + return subprocess.run( + ["python3", str(SHIP)], + cwd=str(repo), + capture_output=True, + text=True, + check=False, + env=env, + ) + + +# ─── Fixture: base orphan project ───────────────────────────────────────────── + +@pytest.fixture() +def orphan_project(tmp_path): + """Build a real git project where pkg/widget.py is an orphan. + + Layout: + Commit 1 (start): pyproject.toml + pkg/__init__.py + pkg/app.py (production + entry point — empty placeholder, imports nothing yet) + Commit 2 (head): pkg/widget.py added (new module); only imported by its + own test, NOT by app.py → ORPHAN + + Key design rationale: app.py exists BEFORE start_sha so the sweep only + counts widget.py as a new module (not app.py). This means: + - ORPHAN state: app.py does NOT import widget.py (widget is orphan) + - WIRED state: editing app.py to import widget.py wires it (app.py is + already counted as "existing production code", not a new module) + + The project has: + - Gates 1-4 satisfied (pipeline.json, tasks.json, CDD card, plan.md) + - Task 42, tier=wired, status=in-progress + - Oracle stubbed PASS via ATLAS_ORACLE_CMD + """ + repo = _init_repo(tmp_path) + + # Commit 1: base project with a production entrypoint (app.py) that does NOT + # import widget yet. This commit is start_sha. + (repo / "pyproject.toml").write_text("[project]\nname = 'myapp'\n") + pkg = repo / "pkg" + pkg.mkdir() + (pkg / "__init__.py").write_text("") + # app.py exists BEFORE start_sha so it is not counted as a new module. + (pkg / "app.py").write_text("# Production entrypoint — widget import not wired yet\n") + # Gate files + _write_gates_1_4(repo) + # tasks.json (task in-progress) + task = _make_task(tier="wired", status="in-progress") + tasks_file = _write_tasks(repo, task) + # CDD card + card_path = _write_cdd_card(repo) + start_sha = _commit_all(repo, "initial: base project with app.py entrypoint") + + # Commit 2: widget.py added — only imported by its own test (orphan). + # app.py is NOT modified, so widget.py has no production importer. + (pkg / "widget.py").write_text( + "class Widget:\n" + " def render(self):\n" + " return ''\n" + ) + tests_dir = repo / "tests" + tests_dir.mkdir() + (tests_dir / "__init__.py").write_text("") + (tests_dir / "test_widget.py").write_text( + "from pkg.widget import Widget\n\n" + "def test_render():\n" + " assert Widget().render() == ''\n" + ) + head_sha = _commit_all(repo, "feat: add widget module (test only, not wired to production)") + + oracle_cmd = _fake_oracle_script(tmp_path) + + return { + "repo": repo, + "start_sha": start_sha, + "head_sha": head_sha, + "tasks_file": tasks_file, + "card_path": card_path, + "oracle_cmd": oracle_cmd, + "tmp_path": tmp_path, + } + + +# ─── Scenario A: Orphan blocks ship ────────────────────────────────────────── + +class TestOrphanBlocksShip: + """Scenario A: orphan module → sweep writes ORPHAN → ship-check blocks.""" + + def test_sweep_writes_orphan_verdict_to_cdd_card(self, orphan_project): + """reachability-sweep on an orphan module writes ORPHAN to the CDD card.""" + p = orphan_project + result = run_reachability_sweep(TASK_ID, p["start_sha"], cwd=str(p["repo"])) + + assert result["verdict"] == "ORPHAN", ( + f"Expected ORPHAN but got {result['verdict']!r}. " + f"Full result: {result}" + ) + + # Verify the card was updated. + card = json.loads(p["card_path"].read_text()) + assert "reachability" in card + assert card["reachability"]["verdict"] == "ORPHAN" + # start_commit is recorded + assert card["reachability"]["start_commit"] == p["start_sha"] + + def test_ship_check_exits_nonzero_for_orphan(self, orphan_project): + """ship-check subprocess exits 1 when the wired task is ORPHAN.""" + p = orphan_project + # First run sweep to write the ORPHAN verdict. + run_reachability_sweep(TASK_ID, p["start_sha"], cwd=str(p["repo"])) + + # Mark task as done in tasks.json so Gate 2 passes and Gate 6 runs. + # (We mark done without going through run_set_status — we want the card + # to have ORPHAN so Gate 6 blocks, not set-status.) + tasks = json.loads(p["tasks_file"].read_text()) + for t in tasks["master"]["tasks"]: + if str(t["id"]) == TASK_ID: + t["status"] = "done" + p["tasks_file"].write_text(json.dumps(tasks, indent=2)) + + r = _run_ship_check(p["repo"], p["oracle_cmd"]) + + # Must NOT print SHIP_CHECK_OK. + assert "SHIP_CHECK_OK" not in r.stdout, ( + f"Expected no SHIP_CHECK_OK but stdout was: {r.stdout!r}" + ) + # Must exit non-zero. + assert r.returncode != 0, ( + f"Expected non-zero exit but got {r.returncode}. stderr={r.stderr!r}" + ) + + def test_ship_check_stderr_names_orphan_and_task(self, orphan_project): + """ship-check failure message names ORPHAN and the task id (non-vacuous).""" + p = orphan_project + run_reachability_sweep(TASK_ID, p["start_sha"], cwd=str(p["repo"])) + # Mark done so Gate 6 fires. + tasks = json.loads(p["tasks_file"].read_text()) + for t in tasks["master"]["tasks"]: + if str(t["id"]) == TASK_ID: + t["status"] = "done" + p["tasks_file"].write_text(json.dumps(tasks, indent=2)) + + r = _run_ship_check(p["repo"], p["oracle_cmd"]) + + assert r.returncode == 1 + # Gate 6 failure message must mention ORPHAN. + assert "ORPHAN" in r.stderr, f"Expected 'ORPHAN' in stderr, got: {r.stderr!r}" + # Failure message must identify the task. + assert TASK_ID in r.stderr, f"Expected task id {TASK_ID!r} in stderr, got: {r.stderr!r}" + + +# ─── Scenario B: Oracle / test orthogonality ────────────────────────────────── + +class TestOracleOrthogonality: + """Scenario B: the task's own test passes (oracle PASS) while reachability is ORPHAN. + + This is the user's exact bug: 'done' was optimised on 'tasks marked done', + which broke when a task passed its unit test but the module was unreachable. + + Assertion: Gate 5 (oracle) == PASS AND Gate 6 (reachability) == ORPHAN → blocked. + The two axes are independent. + """ + + def test_oracle_passes_while_reachability_is_orphan(self, orphan_project, tmp_path): + """Gate 5 (oracle) passes; Gate 6 (reachability) blocks — two independent axes.""" + p = orphan_project + # Write ORPHAN verdict into card. + sweep_result = run_reachability_sweep(TASK_ID, p["start_sha"], cwd=str(p["repo"])) + assert sweep_result["verdict"] == "ORPHAN" + + # Confirm card has ORPHAN. + card = json.loads(p["card_path"].read_text()) + assert card["reachability"]["verdict"] == "ORPHAN" + + # The oracle is stubbed PASS — Gate 5 would pass for this task. + # Verify the fake oracle script emits PASS when called with 'grade'. + oracle_result = subprocess.run( + ["sh", str(p["tmp_path"] / "fake_atlas.sh"), "oracle", "grade"], + capture_output=True, + text=True, + ) + oracle_out = json.loads(oracle_result.stdout) + assert oracle_out["verdict"] == "PASS", ( + f"Oracle stub should emit PASS but got: {oracle_result.stdout!r}" + ) + + # Now run the full ship-check: Gate 5 passes, Gate 6 blocks. + tasks = json.loads(p["tasks_file"].read_text()) + for t in tasks["master"]["tasks"]: + if str(t["id"]) == TASK_ID: + t["status"] = "done" + p["tasks_file"].write_text(json.dumps(tasks, indent=2)) + + r = _run_ship_check(p["repo"], p["oracle_cmd"]) + + # Ship-check is blocked even though oracle says PASS. + assert r.returncode == 1, ( + "Expected ship-check to block but it passed. " + "Oracle says PASS, yet reachability is ORPHAN — Gate 6 should block." + ) + assert "SHIP_CHECK_OK" not in r.stdout + # Gate 6 is responsible — not Gate 5. + assert "ORPHAN" in r.stderr, f"Expected Gate 6 to mention ORPHAN. stderr={r.stderr!r}" + + def test_set_status_done_raises_for_orphan(self, orphan_project): + """set-status done raises for the orphan task; oracle has no bearing.""" + p = orphan_project + run_reachability_sweep(TASK_ID, p["start_sha"], cwd=str(p["repo"])) + + old_cwd = os.getcwd() + try: + os.chdir(str(p["repo"])) + with pytest.raises(CommandError) as exc_info: + run_set_status(TASK_ID, "done") + finally: + os.chdir(old_cwd) + + msg = exc_info.value.message + assert "ORPHAN" in msg or "reachability" in msg.lower(), ( + f"Expected ORPHAN or 'reachability' in error message, got: {msg!r}" + ) + + +# ─── Scenario C: Wire it → ships ────────────────────────────────────────────── + +class TestWiredShips: + """Scenario C: wire the module (edit existing app.py) → re-sweep → WIRED → ships. + + Design: app.py exists BEFORE start_sha (not counted as a new module). + Wiring = modifying app.py to import Widget (an edit to an existing file, not + a new module). The sweep then finds only widget.py as a new module, and + widget.py now has a production importer (app.py) → WIRED. + """ + + def test_wire_module_then_sweep_gives_wired(self, orphan_project): + """After editing app.py to import Widget, sweep returns WIRED.""" + p = orphan_project + repo = p["repo"] + + # Confirm we start with ORPHAN. + result = run_reachability_sweep(TASK_ID, p["start_sha"], cwd=str(repo)) + assert result["verdict"] == "ORPHAN" + + # Wire: edit the existing production entrypoint to import Widget. + # (app.py was committed before start_sha, so it's not a new module.) + (repo / "pkg" / "app.py").write_text( + "from pkg.widget import Widget\n\n" + "def main():\n" + " w = Widget()\n" + " return w.render()\n" + ) + _commit_all(repo, "wire: import Widget from pkg.widget in app.py") + + # Re-run sweep against the same start sha. + result2 = run_reachability_sweep(TASK_ID, p["start_sha"], cwd=str(repo)) + assert result2["verdict"] == "WIRED", ( + f"Expected WIRED after wiring but got {result2['verdict']!r}. Full: {result2}" + ) + + # CDD card is updated. + card = json.loads(p["card_path"].read_text()) + assert card["reachability"]["verdict"] == "WIRED" + + def test_wire_then_set_status_done_succeeds(self, orphan_project): + """After wiring + sweep, set-status done auto-reads WIRED from card → ok.""" + p = orphan_project + repo = p["repo"] + + # Wire: edit existing app.py to import Widget. + (repo / "pkg" / "app.py").write_text( + "from pkg.widget import Widget\n\n" + "def main():\n" + " return Widget().render()\n" + ) + _commit_all(repo, "wire: import Widget in app.py") + + # Sweep → WIRED (only widget.py is a new module; app.py existed before start_sha). + sweep_result = run_reachability_sweep(TASK_ID, p["start_sha"], cwd=str(repo)) + assert sweep_result["verdict"] == "WIRED", f"Expected WIRED but got: {sweep_result}" + + # set-status done — auto-reads WIRED from card. + old_cwd = os.getcwd() + try: + os.chdir(str(repo)) + result = run_set_status(TASK_ID, "done") + finally: + os.chdir(old_cwd) + + assert result["ok"] is True + assert result["status"] == "done" + + # tasks.json is updated. + tasks = json.loads(p["tasks_file"].read_text()) + updated = {str(t["id"]): t for t in tasks["master"]["tasks"]} + assert updated[TASK_ID]["status"] == "done" + assert updated[TASK_ID]["reachability"]["verdict"] == "WIRED" + + def test_wire_then_ship_check_passes(self, orphan_project): + """After wiring, set-status done, ship-check prints SHIP_CHECK_OK exit 0.""" + p = orphan_project + repo = p["repo"] + + # Wire: edit existing app.py to import Widget. + (repo / "pkg" / "app.py").write_text( + "from pkg.widget import Widget\n\n" + "def main():\n" + " return Widget().render()\n" + ) + _commit_all(repo, "wire: import Widget in app.py") + + # Sweep → WIRED. + sweep_result = run_reachability_sweep(TASK_ID, p["start_sha"], cwd=str(repo)) + assert sweep_result["verdict"] == "WIRED", f"Expected WIRED but got: {sweep_result}" + + # set-status done (auto-reads WIRED from card). + old_cwd = os.getcwd() + try: + os.chdir(str(repo)) + run_set_status(TASK_ID, "done") + finally: + os.chdir(old_cwd) + + # ship-check subprocess should now pass. + r = _run_ship_check(repo, p["oracle_cmd"]) + + assert r.returncode == 0, ( + f"Expected ship-check to pass (exit 0) but got {r.returncode}. " + f"stderr={r.stderr!r}" + ) + assert "SHIP_CHECK_OK" in r.stdout, ( + f"Expected SHIP_CHECK_OK in stdout but got: {r.stdout!r}" + ) + + +# ─── Scenario D: Scaffold → honest block ───────────────────────────────────── + +class TestScaffoldHonestBlock: + """Scenario D: instead of wiring, re-status to scaffold → Gate 2 blocks honestly.""" + + def test_scaffold_status_blocks_ship_gate2(self, orphan_project): + """set-status scaffold → task not done → ship-check blocks at Gate 2.""" + p = orphan_project + repo = p["repo"] + + # Run sweep (ORPHAN) first so reachability is recorded. + run_reachability_sweep(TASK_ID, p["start_sha"], cwd=str(repo)) + + # Set status to scaffold (no reachability gate on non-done statuses). + old_cwd = os.getcwd() + try: + os.chdir(str(repo)) + result = run_set_status(TASK_ID, "scaffold") + finally: + os.chdir(old_cwd) + + assert result["ok"] is True + assert result["status"] == "scaffold" + + # Verify tasks.json has the new status. + tasks = json.loads(p["tasks_file"].read_text()) + updated = {str(t["id"]): t for t in tasks["master"]["tasks"]} + assert updated[TASK_ID]["status"] == "scaffold" + + # ship-check: Gate 2 blocks (task is not done). + r = _run_ship_check(repo, p["oracle_cmd"]) + + assert r.returncode != 0, ( + f"Expected ship-check to block (scaffold is not done) but it passed. " + f"stdout={r.stdout!r}" + ) + assert "SHIP_CHECK_OK" not in r.stdout + + def test_scaffold_block_is_gate2_not_gate6(self, orphan_project): + """Scaffold block comes from Gate 2 (not done), not Gate 6 (reachability).""" + p = orphan_project + repo = p["repo"] + + run_reachability_sweep(TASK_ID, p["start_sha"], cwd=str(repo)) + + old_cwd = os.getcwd() + try: + os.chdir(str(repo)) + run_set_status(TASK_ID, "scaffold") + finally: + os.chdir(old_cwd) + + r = _run_ship_check(repo, p["oracle_cmd"]) + + assert r.returncode == 1 + # Gate 2 message: task is "not done" / status check. + stderr = r.stderr.lower() + assert "not done" in stderr or "scaffold" in stderr or "status" in stderr, ( + f"Expected Gate 2 'not done' message in stderr, got: {r.stderr!r}" + ) + # Gate 6 (ORPHAN) should NOT fire because the task is not 'done'. + # The scaffold task is caught by Gate 2 first. + # (Note: Gate 6 skips non-done tasks per the contract.) + + def test_scaffold_set_status_does_not_require_reachability(self, orphan_project): + """set-status scaffold works without a reachability verdict (non-done bypass).""" + p = orphan_project + repo = p["repo"] + + # No sweep run — no reachability in card. Scaffold still works. + old_cwd = os.getcwd() + try: + os.chdir(str(repo)) + result = run_set_status(TASK_ID, "scaffold") + finally: + os.chdir(old_cwd) + + assert result["ok"] is True + assert result["status"] == "scaffold" From 5c3d2a8b5fc0b0c36a0bf444be8e68f6e41c5583 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 16 Jun 2026 21:29:40 +0800 Subject: [PATCH 12/13] fix(engine): twin shipcheck gate_reachability dict-modules crash; add skel/.npmignore (pyc excluded from pack) Co-Authored-By: Claude Opus 4.8 --- prd_taskmaster/shipcheck.py | 8 +- skel/.npmignore | 2 + tests/core/test_ship_check_reachability.py | 100 +++++++++++++++++++++ 3 files changed, 109 insertions(+), 1 deletion(-) create mode 100644 skel/.npmignore diff --git a/prd_taskmaster/shipcheck.py b/prd_taskmaster/shipcheck.py index 821f142..78cbd68 100644 --- a/prd_taskmaster/shipcheck.py +++ b/prd_taskmaster/shipcheck.py @@ -215,7 +215,13 @@ def gate_reachability(repo_root: Path, tasks: list) -> Tuple[bool, List[str]]: if verdict in _PASS_VERDICTS: continue elif verdict in _FAIL_VERDICTS: - modules = reach.get("modules", []) if isinstance(reach, dict) else [] + raw_modules = reach.get("modules", []) if isinstance(reach, dict) else [] + # modules may be plain strings (test fixtures / legacy) or dicts written + # by reachability.sweep_task with keys {verdict, module, importers, ...}. + modules = [ + m["module"] if isinstance(m, dict) else str(m) + for m in raw_modules + ] mod_str = f" ({', '.join(modules)})" if modules else "" failures.append( f"task {tid}: reachability {verdict}{mod_str} — wire the module(s)" diff --git a/skel/.npmignore b/skel/.npmignore new file mode 100644 index 0000000..8d35cb3 --- /dev/null +++ b/skel/.npmignore @@ -0,0 +1,2 @@ +__pycache__ +*.pyc diff --git a/tests/core/test_ship_check_reachability.py b/tests/core/test_ship_check_reachability.py index 727a39c..359ccde 100644 --- a/tests/core/test_ship_check_reachability.py +++ b/tests/core/test_ship_check_reachability.py @@ -447,3 +447,103 @@ def test_remediation_hint_in_orphan_failure(self, tmp_path): r = _run_subprocess(tmp_path) assert r.returncode == 1 assert "wire" in r.stderr.lower() or "re-status" in r.stderr.lower(), r.stderr + + +# ═══════════════════════════════════════════════════════════════════════════════ +# Regression tests: dict-shaped modules (real sweep_task / reachability-sweep output) +# Both copies of gate_reachability must handle this without crashing. +# ═══════════════════════════════════════════════════════════════════════════════ + +_DICT_MODULES_REACHABILITY = { + "verdict": "ORPHAN", + "modules": [ + {"verdict": "ORPHAN", "module": "pkg/widget.py", "importers": []}, + ], +} + + +def _add_reachability_raw(atlas: Path, tid, reach: dict) -> None: + """Patch a CDD card to set the reachability block to a raw dict.""" + card_path = atlas / "cdd" / f"task-{tid}.json" + card = json.loads(card_path.read_text()) + card["reachability"] = reach + card_path.write_text(json.dumps(card)) + + +class TestGateReachabilityDictModules: + """Regression: gate_reachability must NOT crash when modules are dicts. + + Both copies — skel/ship-check.py (loaded via importlib) and + prd_taskmaster.shipcheck — are exercised with the exact shape that + reachability.sweep_task / reachability-sweep writes: + + {"verdict": "ORPHAN", "module": "pkg/widget.py", "importers": []} + """ + + def test_skel_dict_modules_no_crash_and_module_name_in_failure(self, tmp_path): + """skel copy: dict-shaped modules → no TypeError, module name appears in failure.""" + tasks = [{"id": 50, "status": "done", "tier": "wired"}] + atlas = _setup_base(tmp_path, tasks) + _add_reachability_raw(atlas, 50, _DICT_MODULES_REACHABILITY) + mod = _load() + # Must not raise TypeError + ok, failures = mod.gate_reachability(tmp_path, tasks) + assert ok is False, "ORPHAN verdict should block ship" + assert failures, "expected at least one failure message" + assert any("pkg/widget.py" in f for f in failures), ( + f"module name missing from failure messages: {failures!r}" + ) + + def test_twin_dict_modules_no_crash_and_module_name_in_failure(self, tmp_path): + """prd_taskmaster.shipcheck twin: dict-shaped modules → no TypeError, module name in failure.""" + from prd_taskmaster import shipcheck as twin + + tasks = [{"id": 51, "status": "done", "tier": "wired"}] + atlas = _setup_base(tmp_path, tasks) + _add_reachability_raw(atlas, 51, _DICT_MODULES_REACHABILITY) + # Must not raise TypeError + ok, failures = twin.gate_reachability(tmp_path, tasks) + assert ok is False, "ORPHAN verdict should block ship" + assert failures, "expected at least one failure message" + assert any("pkg/widget.py" in f for f in failures), ( + f"module name missing from failure messages: {failures!r}" + ) + + def test_skel_dict_modules_multi_module(self, tmp_path): + """skel copy: multiple dict-shaped modules all appear in the failure message.""" + reach = { + "verdict": "ORPHAN", + "modules": [ + {"verdict": "ORPHAN", "module": "pkg/alpha.py", "importers": []}, + {"verdict": "ORPHAN", "module": "pkg/beta.py", "importers": []}, + ], + } + tasks = [{"id": 52, "status": "done", "tier": "wired"}] + atlas = _setup_base(tmp_path, tasks) + _add_reachability_raw(atlas, 52, reach) + mod = _load() + ok, failures = mod.gate_reachability(tmp_path, tasks) + assert ok is False + combined = " ".join(failures) + assert "pkg/alpha.py" in combined, failures + assert "pkg/beta.py" in combined, failures + + def test_twin_dict_modules_multi_module(self, tmp_path): + """twin: multiple dict-shaped modules all appear in the failure message.""" + from prd_taskmaster import shipcheck as twin + + reach = { + "verdict": "ORPHAN", + "modules": [ + {"verdict": "ORPHAN", "module": "pkg/alpha.py", "importers": []}, + {"verdict": "ORPHAN", "module": "pkg/beta.py", "importers": []}, + ], + } + tasks = [{"id": 53, "status": "done", "tier": "wired"}] + atlas = _setup_base(tmp_path, tasks) + _add_reachability_raw(atlas, 53, reach) + ok, failures = twin.gate_reachability(tmp_path, tasks) + assert ok is False + combined = " ".join(failures) + assert "pkg/alpha.py" in combined, failures + assert "pkg/beta.py" in combined, failures From e0e319f23f84c7af0f9feb03d8a4abe576635fe9 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 16 Jun 2026 21:49:03 +0800 Subject: [PATCH 13/13] fix(engine): exclude .spec/.cy test files from reachability importers (close JS/TS false-WIRED) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Extend _EXCLUDE_NAME_RE to match *.spec. and *.cy. for all supported language extensions (py/ts/js/jsx/tsx/go/cjs/mjs). This prevents co-located Jest/Vitest/Angular spec files and Cypress .cy files from being counted as production importers in find_importers, and from being swept as new modules in new_modules_for_task. Without this fix a module imported only by src/bar.spec.ts would return WIRED (false pass). Also aligns the find_importers grep globs to include *.cjs alongside the existing *.mjs so a .cjs-only importer is detected correctly. Five new regression tests added in TestSpecCyImporterIsOrphan confirming: - bar.spec.ts sole importer → ORPHAN (was WIRED pre-fix) - bar.cy.ts sole importer → ORPHAN (was WIRED pre-fix) - bar.cy.js sole importer → ORPHAN (was WIRED pre-fix) - spec + real production importer → still WIRED - new foo.spec.ts excluded from new_modules_for_task Full suite: 568 passed, 3 skipped, 0 failures. --- prd_taskmaster/reachability.py | 12 ++-- tests/core/test_reachability.py | 123 ++++++++++++++++++++++++++++++++ 2 files changed, 130 insertions(+), 5 deletions(-) diff --git a/prd_taskmaster/reachability.py b/prd_taskmaster/reachability.py index eac9fec..965c290 100644 --- a/prd_taskmaster/reachability.py +++ b/prd_taskmaster/reachability.py @@ -78,11 +78,13 @@ class ReachabilityError(Exception): _EXCLUDE_NAME_RE = re.compile( r"(^|/)(" - r"test_[^/]+\.(py|ts|js|go)" # test_*.py / test_*.ts … - r"|[^/]+_test\.(py|ts|js|go)" # *_test.py … - r"|[^/]+\.test\.(py|ts|js|go)" # *.test.py / *.test.ts … + r"test_[^/]+\.(py|ts|js|jsx|tsx|go|cjs|mjs)" # test_*.py / test_*.ts … + r"|[^/]+_test\.(py|ts|js|jsx|tsx|go|cjs|mjs)" # *_test.py … + r"|[^/]+\.test\.(py|ts|js|jsx|tsx|go|cjs|mjs)" # *.test.py / *.test.ts … + r"|[^/]+\.spec\.(py|ts|js|jsx|tsx|go|cjs|mjs)" # *.spec.ts / *.spec.js (Jest/Vitest/Angular) + r"|[^/]+\.cy\.(ts|js|jsx|tsx|cjs|mjs)" # *.cy.ts / *.cy.js (Cypress) r"|index\.[^/]+" # index.* barrels - r"|[^/]+migration[^/]*\.(py|ts|js|go)" # migration files + r"|[^/]+migration[^/]*\.(py|ts|js|jsx|tsx|go|cjs|mjs)" # migration files r")$" ) @@ -232,7 +234,7 @@ def find_importers(repo_root: Path, module: Path, lang: str) -> list[Path]: raw = _grep_patterns(repo_root, patterns, "*.py") elif lang in ("ts", "js"): patterns = _ts_import_patterns(module) - globs = ["*.ts", "*.tsx", "*.js", "*.jsx", "*.mjs"] + globs = ["*.ts", "*.tsx", "*.js", "*.jsx", "*.mjs", "*.cjs"] raw = [] for g in globs: raw.extend(_grep_patterns(repo_root, patterns, g)) diff --git a/tests/core/test_reachability.py b/tests/core/test_reachability.py index 686407b..2a6c6a0 100644 --- a/tests/core/test_reachability.py +++ b/tests/core/test_reachability.py @@ -672,3 +672,126 @@ def test_ts_substring_full_sweep_orphan(self, tmp_path): f"got {verdict['verdict']!r} with importers {verdict['importers']!r}. " "Pre-fix code would return WIRED." ) + + +# ─── 10. Regression: .spec/.cy importer → ORPHAN (JS/TS false-WIRED fix) ───── + +class TestSpecCyImporterIsOrphan: + """.spec.ts / .cy.ts importers must be excluded from reachability importers. + + PRE-FIX BEHAVIOR: *.spec.ts / *.cy.ts were not in _EXCLUDE_NAME_RE → + they appeared as production importers → WIRED (false pass). + POST-FIX BEHAVIOR: _EXCLUDE_NAME_RE matches *.spec. and *.cy. → + excluded from find_importers → ORPHAN (correct blocking verdict). + """ + + def _make_ts_repo_with_spec( + self, tmp_path: Path, spec_suffix: str + ) -> tuple[Path, Path]: + """Create a minimal TS repo where src/bar.ts is imported ONLY by a .spec file. + + Returns (repo, spec_path). + """ + repo = _init_repo(tmp_path) + (repo / "package.json").write_text('{"name": "test"}') + (repo / "tsconfig.json").write_text("{}") + src = repo / "src" + src.mkdir() + (src / "bar.ts").write_text("export const bar = 42;\n") + spec_file = src / f"bar{spec_suffix}" + spec_file.write_text("import { bar } from './bar';\n") + _commit_all(repo, f"add bar.ts + {spec_file.name}") + return repo, spec_file + + def test_spec_ts_importer_yields_orphan(self, tmp_path): + """src/bar.spec.ts is the ONLY importer of src/bar.ts → ORPHAN. + + PRE-FIX: bar.spec.ts not excluded → counted as importer → WIRED. + POST-FIX: bar.spec.ts excluded by _EXCLUDE_NAME_RE → ORPHAN. + """ + repo, spec_file = self._make_ts_repo_with_spec(tmp_path, ".spec.ts") + verdict = reachability_verdict(repo, Path("src/bar.ts"), "ts") + assert verdict["verdict"] == "ORPHAN", ( + f"Expected ORPHAN — only importer is {spec_file.name} (.spec.ts), " + f"got {verdict['verdict']!r} with importers {verdict['importers']!r}. " + "Pre-fix code would return WIRED (false pass)." + ) + assert verdict["importers"] == [] + + def test_cy_ts_importer_yields_orphan(self, tmp_path): + """src/bar.cy.ts is the ONLY importer of src/bar.ts → ORPHAN. + + PRE-FIX: bar.cy.ts not excluded → counted as importer → WIRED. + POST-FIX: bar.cy.ts excluded by _EXCLUDE_NAME_RE → ORPHAN. + """ + repo, spec_file = self._make_ts_repo_with_spec(tmp_path, ".cy.ts") + verdict = reachability_verdict(repo, Path("src/bar.ts"), "ts") + assert verdict["verdict"] == "ORPHAN", ( + f"Expected ORPHAN — only importer is {spec_file.name} (.cy.ts), " + f"got {verdict['verdict']!r} with importers {verdict['importers']!r}. " + "Pre-fix code would return WIRED (false pass)." + ) + assert verdict["importers"] == [] + + def test_spec_ts_plus_real_importer_yields_wired(self, tmp_path): + """When BOTH src/bar.spec.ts AND src/app.ts import src/bar.ts → WIRED. + + The .spec file is excluded but the real production importer still wires it. + """ + repo = _init_repo(tmp_path) + (repo / "package.json").write_text('{"name": "test"}') + (repo / "tsconfig.json").write_text("{}") + src = repo / "src" + src.mkdir() + (src / "bar.ts").write_text("export const bar = 42;\n") + (src / "bar.spec.ts").write_text("import { bar } from './bar';\n") + (src / "app.ts").write_text("import { bar } from './bar';\n") + _commit_all(repo, "add bar.ts + spec + app") + + verdict = reachability_verdict(repo, Path("src/bar.ts"), "ts") + assert verdict["verdict"] == "WIRED", ( + "Expected WIRED — src/app.ts is a genuine production importer alongside bar.spec.ts" + ) + importer_names = [Path(p).name for p in verdict["importers"]] + assert "app.ts" in importer_names, "app.ts must appear as importer" + assert "bar.spec.ts" not in importer_names, "bar.spec.ts must NOT appear as importer" + + def test_spec_file_not_swept_as_new_module(self, tmp_path): + """A newly-added src/foo.spec.ts must NOT appear in new_modules_for_task. + + It's a test file, not a production module — should be excluded by _EXCLUDE_NAME_RE. + """ + repo = _init_repo(tmp_path) + (repo / "package.json").write_text('{"name": "test"}') + (repo / "tsconfig.json").write_text("{}") + src = repo / "src" + src.mkdir() + start = _commit_all(repo, "init") + + (src / "foo.ts").write_text("export const foo = 1;\n") + (src / "foo.spec.ts").write_text("import { foo } from './foo';\n") + head = _commit_all(repo, "add foo + spec") + + modules = new_modules_for_task(repo, start, head) + names = [m.name for m in modules] + assert "foo.spec.ts" not in names, ( + "foo.spec.ts is a test file and must be excluded from new_modules_for_task" + ) + assert "foo.ts" in names, "foo.ts (production module) must be included" + + def test_cy_js_importer_yields_orphan(self, tmp_path): + """src/bar.cy.js is the ONLY importer of src/bar.ts → ORPHAN. + + Verifies .cy. exclusion works for JS as well as TS. + """ + repo, spec_file = self._make_ts_repo_with_spec(tmp_path, ".cy.js") + # bar.cy.js imports './bar' — but since it's a JS file we need a JS-repo + # detection for find_importers to sweep it. The repo has tsconfig.json + # so it's detected as 'ts', which includes *.js globs — correct. + verdict = reachability_verdict(repo, Path("src/bar.ts"), "ts") + assert verdict["verdict"] == "ORPHAN", ( + f"Expected ORPHAN — only importer is {spec_file.name} (.cy.js), " + f"got {verdict['verdict']!r} with importers {verdict['importers']!r}. " + "Pre-fix code would return WIRED (false pass)." + ) + assert verdict["importers"] == []