From a6619b261c9430b4cf6a518641727cd4e9bb6a54 Mon Sep 17 00:00:00 2001 From: Sinity Date: Fri, 31 Jul 2026 07:07:23 +0200 Subject: [PATCH 1/4] feat(devtools): add bead-landing-check, empty-diff evidence verification Problem: five open beads (o4j2, hiu, 0jf4, pbuh, cijx.4) each cost a dispatched agent a full investigation cycle (30-60+ min) on 2026-07-30/31 before turning out to describe work already completed on master. A one-off manual sweep of the 606-open-bead backlog is not a repeatable fix. Solution: devtools workspace bead-landing-check extracts commit hashes, PR numbers, and file paths cited in a bead's description/design/AC/notes/ close_reason/comments, then verifies them two ways: cherry-picks cited commits onto master in a single reused throwaway worktree (empty diff, or a per-file content-equivalence fallback when the raw cherry-pick conflicts, both prove landing regardless of squash-merge id rewriting -- more reliable than `git log --is-ancestor` or grepping for issue ids), and checks cited PR merge state via `gh pr view` (disk-cached, MERGED results reused permanently). Emits LIKELY-STALE / LIKELY-LIVE / UNDETERMINED per bead with a confidence tier and the evidence, never auto-closing anything. Beads with no cited evidence are reported UNDETERMINED, not guessed -- verified against o4j2 (closed, correctly flagged LIKELY-STALE from its cited merged PRs) and against hiu/0jf4/pbuh/cijx.4 (still open, correctly UNDETERMINED because the resolving evidence was never written back into the bead text itself -- exactly why they cost a fresh investigation cycle). Registered as a CommandSpec in devtools/command_catalog.py per the documented devtools-command pattern; docs/devtools.md regenerated via `devtools render devtools-reference`. Verification: ruff check/format, mypy --strict (clean), devtools render all --check (no drift). Manual runs against the five motivating beads confirm correct verdicts including a real does-not-apply/content-divergence case (polylogue-0jf4's cited commits landed but were further edited afterward -- tool correctly reports UNDETERMINED rather than a false empty-diff match). Co-Authored-By: Claude --- devtools/bead_landing_check.py | 758 +++++++++++++++++++++++++++++++++ devtools/command_catalog.py | 24 ++ docs/devtools.md | 1 + 3 files changed, 783 insertions(+) create mode 100644 devtools/bead_landing_check.py diff --git a/devtools/bead_landing_check.py b/devtools/bead_landing_check.py new file mode 100644 index 0000000000..96b4f3c431 --- /dev/null +++ b/devtools/bead_landing_check.py @@ -0,0 +1,758 @@ +"""bead-landing-check: verify whether a bead's cited implementation evidence +already landed on master, before dispatching a full investigation cycle. + +Motivating incident (2026-07-30/31): five separate open beads (o4j2, hiu, +0jf4, pbuh, cijx.4) each cost a dispatched agent a full investigation cycle +before discovering the described work was already done on master. The +technique that worked, twice: take the commit(s) a bead cites as evidence, +cherry-pick them onto current master in a throwaway worktree, and check for +an EMPTY DIFF. An empty diff proves the change already exists regardless of +squash-merge id rewriting -- it is cheaper and more reliable than +`git log --is-ancestor` (fails under squash merges) or grepping for issue ids +(false-positives on prose mentions). + +This tool automates that technique plus a cheap gh-based PR merge-state check, +scaled to run over the whole open backlog without one investigation cycle per +bead: + + - extracts cited commit hashes / PR numbers / file paths from a bead's + title, description, design, acceptance_criteria, notes, close_reason, + and comment text + - resolves each cited commit against a SINGLE reused throwaway worktree + (not one worktree per bead) and classifies it: unknown-revision, + already-on-master, empty-diff, non-empty-diff, does-not-apply + - resolves each cited PR number via `gh pr view --json state,mergedAt,...`, + cached to disk so a re-run is nearly free + - emits a per-bead verdict: LIKELY-STALE / LIKELY-LIVE / UNDETERMINED, with + the evidence and a confidence tier (a commit empty-diff is "strong" + evidence; a merged-PR citation alone is "weak" -- it proves the PR + landed, not that this bead's full scope is subsumed) + +Honesty rule: a bead with no cited commits/PRs cannot be verified by this +tool and MUST be reported UNDETERMINED, never guessed either way. This tool +never auto-closes anything -- it only reports; a human or a follow-up bead +decides. + +Usage: + # Sweep every open + in_progress bead (the default): + devtools workspace bead-landing-check + + # Check specific beads: + devtools workspace bead-landing-check polylogue-o4j2 polylogue-hiu + + # Machine-readable JSON, only the beads it can actually vouch for: + devtools workspace bead-landing-check --json --stale-only + + # Cheap pass with no network calls, reusing whatever is cached: + devtools workspace bead-landing-check --offline + + # Skip the (slower) cherry-pick path, PR checks only: + devtools workspace bead-landing-check --no-commits + +Safety: the throwaway worktree defaults to +`.cache/bead-landing-check/worktree` (gitignored, disposable) and is REUSED +across beads and across runs -- never created per-bead. `--remove-worktree` +tears it down explicitly at the end of a run: it verifies no live process has +its cwd inside the worktree before removing it, and never passes `--force` to +`git worktree remove`. By default the worktree is left in place for the next +run. +""" + +from __future__ import annotations + +import argparse +import json +import re +import subprocess +import sys +import time +from collections import Counter +from collections.abc import Sequence +from dataclasses import dataclass, field +from pathlib import Path +from typing import Any + +BeadDict = dict[str, Any] + +# --------------------------------------------------------------------------- +# Evidence extraction +# --------------------------------------------------------------------------- + +# Bare "#N" only -- this repo's own PR/commit-subject convention cites PRs +# this way (e.g. "(#3390)"), so it is the dominant valid signal. A +# `github.com/.../pull/N` URL pattern was deliberately dropped: bead text +# frequently *quotes* sample payloads containing unrelated PR-linked URLs +# (e.g. a `pr-link` record's sample JSON), and matching those produced +# false PR citations unconnected to the bead's own resolution. +_PR_PATTERN = re.compile(r"(?:^|[^\w/])#(\d{2,6})\b") +_COMMIT_PATTERN = re.compile(r"\b[0-9a-f]{7,40}\b") +_FILE_PATTERN = re.compile( + r"(?:polylogue|tests|devtools|docs|storage|pipeline|daemon|cli|mcp" + r"|browser[_-]extension|browser_capture|coordination|archive|insights" + r"|context|core|hooks|maintenance|schemas)/[\w./\-]+\.(?:py|ts|js|yaml|yml|md|json|sql)", + re.IGNORECASE, +) + +_CACHE_DIR_NAME = ".cache/bead-landing-check" +_DEFAULT_WORKTREE_SUBDIR = "worktree" +_DEFAULT_CACHE_TTL_DAYS = 7 +_DEFAULT_REPO_SLUG = "Sinity/polylogue" +_DEFAULT_STATUSES = frozenset({"open", "in_progress"}) +_MAX_FILE_CHECKS_PER_BEAD = 6 + + +@dataclass +class Evidence: + pr_numbers: list[int] = field(default_factory=list) + commit_candidates: list[str] = field(default_factory=list) + file_paths: list[str] = field(default_factory=list) + + +def _bead_text(bead: BeadDict) -> str: + parts = [ + bead.get("title") or "", + bead.get("description") or "", + bead.get("design") or "", + bead.get("acceptance_criteria") or "", + bead.get("notes") or "", + bead.get("close_reason") or "", + ] + for comment in bead.get("comments") or []: + parts.append(comment.get("text") or "") + return "\n".join(parts) + + +def extract_evidence(bead: BeadDict) -> Evidence: + text = _bead_text(bead) + + prs: set[int] = set() + for m in _PR_PATTERN.finditer(text): + prs.add(int(m.group(1))) + + commits: set[str] = set() + for m in _COMMIT_PATTERN.finditer(text): + token = m.group(0) + # Require a hex token to mix digits and letters -- a pure-digit run is + # a plain number (record count, port, etc.), not a commit-ish hash. + if any(c.isdigit() for c in token) and any(c.isalpha() for c in token): + commits.add(token) + + files = list(dict.fromkeys(_FILE_PATTERN.findall(text))) + + return Evidence(pr_numbers=sorted(prs), commit_candidates=sorted(commits), file_paths=files) + + +# --------------------------------------------------------------------------- +# git plumbing +# --------------------------------------------------------------------------- + + +def _run(cmd: list[str], *, cwd: Path | None = None, timeout: float | None = 30) -> subprocess.CompletedProcess[str]: + return subprocess.run(cmd, cwd=cwd, capture_output=True, text=True, timeout=timeout) + + +class CommitChecker: + """Resolves cited commits against a single reused throwaway worktree.""" + + def __init__(self, repo_root: Path, worktree_dir: Path, base_ref: str = "origin/master") -> None: + if worktree_dir.resolve() == repo_root.resolve(): + raise ValueError("throwaway worktree must not be the repo root") + self.repo_root = repo_root + self.worktree_dir = worktree_dir + self.base_ref = base_ref + self._ensured = False + self._fetched = False + + def _ensure_fetch(self) -> None: + if not self._fetched: + _run(["git", "fetch", "origin", "--quiet"], cwd=self.repo_root, timeout=60) + self._fetched = True + + def _is_real_commit(self, commit: str) -> bool: + return _run(["git", "cat-file", "-e", f"{commit}^{{commit}}"], cwd=self.repo_root).returncode == 0 + + def _is_ancestor(self, commit: str) -> bool: + self._ensure_fetch() + return _run(["git", "merge-base", "--is-ancestor", commit, self.base_ref], cwd=self.repo_root).returncode == 0 + + def _ensure_worktree(self) -> None: + if self._ensured: + return + self._ensure_fetch() + git_marker = self.worktree_dir / ".git" + if git_marker.exists(): + pass # ours already, reuse + elif self.worktree_dir.exists(): + if any(self.worktree_dir.iterdir()): + raise RuntimeError( + f"{self.worktree_dir} exists and is not a git worktree -- refusing to reuse or clobber it" + ) + self.worktree_dir.rmdir() + self._create_worktree() + else: + self._create_worktree() + self._ensured = True + + def _create_worktree(self) -> None: + self.worktree_dir.parent.mkdir(parents=True, exist_ok=True) + result = _run( + ["git", "worktree", "add", "--detach", str(self.worktree_dir), self.base_ref], + cwd=self.repo_root, + timeout=60, + ) + if result.returncode != 0: + raise RuntimeError(f"failed to create throwaway worktree at {self.worktree_dir}: {result.stderr.strip()}") + + def _reset_worktree(self) -> None: + _run(["git", "cherry-pick", "--abort"], cwd=self.worktree_dir) + _run(["git", "reset", "--hard", self.base_ref], cwd=self.worktree_dir) + _run(["git", "clean", "-fdq"], cwd=self.worktree_dir) + + def _commit_touched_files(self, commit: str) -> list[str]: + result = _run(["git", "diff-tree", "--no-commit-id", "--name-only", "-r", commit], cwd=self.repo_root) + return [line for line in result.stdout.splitlines() if line.strip()] + + def _content_equivalent_to_base(self, commit: str) -> bool | None: + """Fallback for a conflicted cherry-pick: compare each touched file's + content at *commit* against ``self.base_ref`` directly. + + A squash-merge can rewrite surrounding history enough that a raw + cherry-pick of the *original* pre-squash commit conflicts, even + though the change is fully present on master under a different + commit id. If every file the commit touched already has identical + content on ``self.base_ref``, that is direct proof of landing -- + stronger than the conflict result alone would suggest. + + Returns None when there is nothing to compare (inconclusive). + """ + files = self._commit_touched_files(commit) + if not files: + return None + for path in files: + base = _run(["git", "show", f"{self.base_ref}:{path}"], cwd=self.repo_root) + theirs = _run(["git", "show", f"{commit}:{path}"], cwd=self.repo_root) + if (base.returncode == 0) != (theirs.returncode == 0): + return False + if base.returncode == 0 and base.stdout != theirs.stdout: + return False + return True + + def check(self, commit: str) -> str: + """Classify *commit* relative to ``self.base_ref``. + + Returns one of: unknown-revision, already-on-master, empty-diff, + content-equivalent, non-empty-diff, does-not-apply. + """ + if not self._is_real_commit(commit): + return "unknown-revision" + if self._is_ancestor(commit): + return "already-on-master" + + self._ensure_worktree() + self._reset_worktree() + result = _run(["git", "cherry-pick", "--no-commit", "--allow-empty", commit], cwd=self.worktree_dir) + if result.returncode != 0: + self._reset_worktree() + if self._content_equivalent_to_base(commit): + return "content-equivalent" + return "does-not-apply" + diff = _run(["git", "diff", "--cached", "--stat"], cwd=self.worktree_dir) + empty = not diff.stdout.strip() + self._reset_worktree() + return "empty-diff" if empty else "non-empty-diff" + + +def check_file_paths(repo_root: Path, paths: list[str], base_ref: str) -> list[dict[str, Any]]: + out: list[dict[str, Any]] = [] + for p in paths[:_MAX_FILE_CHECKS_PER_BEAD]: + exists = _run(["git", "cat-file", "-e", f"{base_ref}:{p}"], cwd=repo_root).returncode == 0 + entry: dict[str, Any] = {"path": p, "exists_on_master": exists} + if exists: + log = _run(["git", "log", "-1", "--format=%H|%cI", base_ref, "--", p], cwd=repo_root) + if log.stdout.strip(): + sha, _, date = log.stdout.strip().partition("|") + entry["last_commit"] = sha + entry["last_commit_date"] = date + out.append(entry) + return out + + +def _worktree_has_live_process(path: Path) -> bool: + """Conservative check: any process whose cwd resolves inside *path*.""" + proc_dir = Path("/proc") + if not proc_dir.exists(): + return True # can't verify -- block rather than risk it + target = str(path.resolve()) + for entry in proc_dir.iterdir(): + if not entry.name.isdigit(): + continue + try: + resolved = str((entry / "cwd").resolve()) + except OSError: + continue + if resolved == target or resolved.startswith(target + "/"): + return True + return False + + +def remove_worktree(repo_root: Path, worktree_dir: Path) -> str: + """Remove *worktree_dir* iff it's ours, clean, and has no live occupant. + + Never passes --force to `git worktree remove`. + """ + if not worktree_dir.exists(): + return "not-present" + if _worktree_has_live_process(worktree_dir): + return "blocked-live-process" + dirty = _run(["git", "status", "--porcelain"], cwd=worktree_dir).stdout.strip() + if dirty: + return "blocked-dirty" + result = _run(["git", "worktree", "remove", str(worktree_dir)], cwd=repo_root) + if result.returncode != 0: + return f"failed: {result.stderr.strip()[:200]}" + _run(["git", "worktree", "prune"], cwd=repo_root) + return "removed" + + +# --------------------------------------------------------------------------- +# PR checking (gh, disk-cached) +# --------------------------------------------------------------------------- + + +@dataclass +class PrResult: + number: int + found: bool + state: str | None = None + merged_at: str | None = None + merge_commit: str | None = None + title: str | None = None + error: str | None = None + + +class PrChecker: + def __init__( + self, + repo_slug: str, + cache_path: Path, + ttl_days: int, + *, + refresh: bool = False, + offline: bool = False, + ) -> None: + self.repo_slug = repo_slug + self.cache_path = cache_path + self.ttl_seconds = ttl_days * 86400 + self.refresh = refresh + self.offline = offline + self._cache: dict[str, Any] = self._load_cache() + self._dirty = False + + def _load_cache(self) -> dict[str, Any]: + if self.cache_path.exists(): + try: + return dict(json.loads(self.cache_path.read_text())) + except json.JSONDecodeError: + return {} + return {} + + def flush(self) -> None: + if not self._dirty: + return + self.cache_path.parent.mkdir(parents=True, exist_ok=True) + self.cache_path.write_text(json.dumps(self._cache, indent=2, sort_keys=True)) + self._dirty = False + + def _from_cache(self, number: int, cached: dict[str, Any]) -> PrResult: + return PrResult( + number=number, + found=bool(cached.get("found")), + state=cached.get("state"), + merged_at=cached.get("merged_at"), + merge_commit=cached.get("merge_commit"), + title=cached.get("title"), + error=cached.get("error"), + ) + + def check(self, number: int) -> PrResult: + key = str(number) + cached = self._cache.get(key) + now = time.time() + # A MERGED result is permanent -- always safe to reuse. Anything else + # (open, not-found, error) is only reused within the TTL. + if ( + cached is not None + and not self.refresh + and (cached.get("state") == "MERGED" or (now - cached.get("checked_at", 0)) < self.ttl_seconds) + ): + return self._from_cache(number, cached) + + if self.offline: + if cached is not None: + return self._from_cache(number, cached) + return PrResult(number=number, found=False, error="offline: not cached") + + result = self._fetch(number) + self._cache[key] = { + "checked_at": now, + "found": result.found, + "state": result.state, + "merged_at": result.merged_at, + "merge_commit": result.merge_commit, + "title": result.title, + "error": result.error, + } + self._dirty = True + return result + + def _fetch(self, number: int) -> PrResult: + result = _run( + ["gh", "pr", "view", str(number), "--repo", self.repo_slug, "--json", "state,mergedAt,mergeCommit,title"], + timeout=20, + ) + if result.returncode != 0: + return PrResult(number=number, found=False, error=result.stderr.strip()[:200]) + try: + data = json.loads(result.stdout) + except json.JSONDecodeError: + return PrResult(number=number, found=False, error="gh returned non-JSON output") + merge_commit = (data.get("mergeCommit") or {}).get("oid") + return PrResult( + number=number, + found=True, + state=data.get("state"), + merged_at=data.get("mergedAt"), + merge_commit=merge_commit, + title=data.get("title"), + ) + + +# --------------------------------------------------------------------------- +# Verdicts +# --------------------------------------------------------------------------- + +_LANDED_COMMIT_STATES = frozenset({"empty-diff", "already-on-master", "content-equivalent"}) + + +@dataclass +class BeadVerdict: + bead_id: str + status: str + priority: int + title: str + verdict: str + confidence: str + reasons: list[str] + evidence: dict[str, Any] + + +def verdict_for_bead( + bead: BeadDict, + evidence: Evidence, + commit_results: dict[str, str], + pr_results: dict[int, PrResult], +) -> BeadVerdict: + reasons: list[str] = [] + + landed = sorted(c for c, s in commit_results.items() if s in _LANDED_COMMIT_STATES) + live = sorted(c for c, s in commit_results.items() if s == "non-empty-diff") + unresolved = sorted(c for c, s in commit_results.items() if s == "unknown-revision") + inapplicable = sorted(c for c, s in commit_results.items() if s == "does-not-apply") + + merged_prs = sorted(n for n, r in pr_results.items() if r.state == "MERGED") + open_prs = sorted(n for n, r in pr_results.items() if r.found and r.state != "MERGED") + missing_prs = sorted(n for n, r in pr_results.items() if not r.found) + + if landed: + verdict, confidence = "LIKELY-STALE", "strong" + detail = ", ".join(f"{c} ({commit_results[c]})" for c in landed) + reasons.append(f"cited commit(s) already on master: {detail}") + if live: + reasons.append( + f"NOTE: commit(s) {', '.join(live)} still produce a non-empty diff -- " + "possibly a partial landing, verify remaining scope before closing" + ) + elif live: + verdict, confidence = "LIKELY-LIVE", "strong" + reasons.append( + f"cited commit(s) cherry-pick with a real (non-empty) diff, not yet on master: {', '.join(live)}" + ) + elif merged_prs and not open_prs: + verdict, confidence = "LIKELY-STALE", "weak" + reasons.append( + f"cited PR(s) merged: {', '.join(f'#{n}' for n in merged_prs)} -- " + "weak evidence: proves the PR landed, not that this bead's full scope is subsumed; verify AC by AC" + ) + elif open_prs: + verdict, confidence = "LIKELY-LIVE", "weak" + reasons.append(f"cited PR(s) not yet merged: {', '.join(f'#{n}' for n in open_prs)}") + else: + verdict, confidence = "UNDETERMINED", "none" + if unresolved: + reasons.append( + f"{len(unresolved)} cited hex token(s) are not real commits in this repo " + f"(likely a false-positive extraction, e.g. a worktree/session id): {', '.join(unresolved)}" + ) + if inapplicable: + reasons.append( + f"commit(s) could not be cherry-picked cleanly onto master (conflict) -- ambiguous, needs human review: " + f"{', '.join(inapplicable)}" + ) + if missing_prs: + reasons.append( + f"PR number(s) not found via gh (deleted, mistyped, or actually an issue number, not a PR): " + f"{', '.join(f'#{n}' for n in missing_prs)}" + ) + if not reasons: + reasons.append("no cited commit hash or PR number found in bead text -- not verifiable by this tool") + + evidence_payload: dict[str, Any] = { + "pr_numbers": evidence.pr_numbers, + "commit_candidates": evidence.commit_candidates, + "file_paths": evidence.file_paths, + "commit_checks": dict(commit_results), + "pr_checks": { + str(n): { + "found": r.found, + "state": r.state, + "merged_at": r.merged_at, + "merge_commit": r.merge_commit, + "title": r.title, + "error": r.error, + } + for n, r in pr_results.items() + }, + } + + return BeadVerdict( + bead_id=bead["id"], + status=bead.get("status", "?"), + priority=bead.get("priority", 4), + title=bead.get("title", ""), + verdict=verdict, + confidence=confidence, + reasons=reasons, + evidence=evidence_payload, + ) + + +# --------------------------------------------------------------------------- +# Loading + rendering +# --------------------------------------------------------------------------- + + +def load_beads_from_jsonl(path: Path) -> list[BeadDict]: + beads: list[BeadDict] = [] + with path.open() as f: + for line in f: + line = line.strip() + if not line: + continue + beads.append(json.loads(line)) + return beads + + +def _short(s: str, n: int = 62) -> str: + return s[:n] + "..." if len(s) > n else s + + +def _render_human(results: list[BeadVerdict], stats: dict[str, Any]) -> None: + print("=" * 100) + print("BEAD LANDING CHECK") + print("=" * 100) + print() + for r in results: + print(f"{r.bead_id:24s} P{r.priority} [{r.status:11s}] {r.verdict:15s} ({r.confidence:6s}) {_short(r.title)}") + for reason in r.reasons: + print(f" - {reason}") + print() + counts = Counter(r.verdict for r in results) + print( + f"Checked {stats['beads_checked']} beads in {stats['elapsed_seconds']}s " + f"({stats['commits_checked']} commit checks, {stats['prs_checked']} PR checks)" + ) + print("Verdicts: " + " ".join(f"{k}={v}" for k, v in sorted(counts.items()))) + removal_note = ( + f" (removed: {stats['worktree_removed']})" if stats.get("worktree_removed") else " (persisted for reuse)" + ) + print(f"Throwaway worktree: {stats['worktree_dir']}{removal_note}") + + +# --------------------------------------------------------------------------- +# Main +# --------------------------------------------------------------------------- + + +def main(argv: Sequence[str] | None = None) -> int: + parser = argparse.ArgumentParser( + prog="devtools workspace bead-landing-check", + description=__doc__, + formatter_class=argparse.RawDescriptionHelpFormatter, + ) + parser.add_argument( + "beads", + nargs="*", + metavar="BEAD_ID", + help="Specific bead id(s) to check (default: all open + in_progress beads)", + ) + parser.add_argument( + "--beads-file", + type=Path, + default=None, + help="Path to .beads/issues.jsonl (default: /.beads/issues.jsonl)", + ) + parser.add_argument( + "--status", + action="append", + default=None, + help="Bead status to include when no bead ids are given (repeatable; default: open, in_progress)", + ) + parser.add_argument( + "--limit", type=int, default=0, help="Cap the number of beads processed after filtering (0 = no cap)" + ) + parser.add_argument("--no-commits", action="store_true", help="Skip cherry-pick commit verification") + parser.add_argument("--no-prs", action="store_true", help="Skip gh PR merge-state checks") + parser.add_argument("--offline", action="store_true", help="Reuse only cached gh results; never call gh") + parser.add_argument("--refresh-cache", action="store_true", help="Ignore cached PR results and re-fetch every one") + parser.add_argument( + "--repo-slug", + default=_DEFAULT_REPO_SLUG, + help=f"GitHub repo slug for gh PR checks (default: {_DEFAULT_REPO_SLUG})", + ) + parser.add_argument( + "--worktree-dir", + type=Path, + default=None, + help="Throwaway worktree path (default: /.cache/bead-landing-check/worktree, reused across runs)", + ) + parser.add_argument( + "--remove-worktree", + action="store_true", + help="Remove the throwaway worktree after this run (checks for a live occupant first; never --force)", + ) + parser.add_argument("--json", action="store_true", help="Emit machine-readable JSON") + parser.add_argument("--stale-only", action="store_true", help="Only report LIKELY-STALE verdicts") + parser.add_argument( + "--only-evidenced", + action="store_true", + help="Skip beads that cite no commit/PR evidence at all (pure UNDETERMINED noise)", + ) + args = parser.parse_args(argv) + + repo_root_result = _run(["git", "rev-parse", "--show-toplevel"]) + if repo_root_result.returncode != 0: + print("ERROR: not inside a git repository", file=sys.stderr) + return 1 + repo_root = Path(repo_root_result.stdout.strip()) + + beads_file = args.beads_file or (repo_root / ".beads" / "issues.jsonl") + if not beads_file.exists(): + print(f"ERROR: beads file not found: {beads_file}", file=sys.stderr) + return 1 + + all_beads = load_beads_from_jsonl(beads_file) + by_id = {b["id"]: b for b in all_beads} + + if args.beads: + missing = [b for b in args.beads if b not in by_id] + if missing: + print(f"ERROR: unknown bead id(s): {', '.join(missing)}", file=sys.stderr) + return 1 + selected = [by_id[b] for b in args.beads] + else: + statuses = set(args.status) if args.status else set(_DEFAULT_STATUSES) + selected = [b for b in all_beads if b.get("status") in statuses] + selected.sort(key=lambda b: (b.get("priority", 4), b["id"])) + if args.limit: + selected = selected[: args.limit] + + cache_dir = repo_root / _CACHE_DIR_NAME + worktree_dir = args.worktree_dir or (cache_dir / _DEFAULT_WORKTREE_SUBDIR) + + commit_checker = None if args.no_commits else CommitChecker(repo_root, worktree_dir) + pr_checker = ( + None + if args.no_prs + else PrChecker( + args.repo_slug, + cache_dir / "pr-cache.json", + _DEFAULT_CACHE_TTL_DAYS, + refresh=args.refresh_cache, + offline=args.offline, + ) + ) + + t_start = time.time() + results: list[BeadVerdict] = [] + n_commits_checked = 0 + n_prs_checked = 0 + + for bead in selected: + evidence = extract_evidence(bead) + + commit_results: dict[str, str] = {} + if commit_checker is not None: + for commit in evidence.commit_candidates: + commit_results[commit] = commit_checker.check(commit) + n_commits_checked += 1 + + pr_results: dict[int, PrResult] = {} + if pr_checker is not None: + for number in evidence.pr_numbers: + pr_results[number] = pr_checker.check(number) + n_prs_checked += 1 + + results.append(verdict_for_bead(bead, evidence, commit_results, pr_results)) + + if pr_checker is not None: + pr_checker.flush() + + elapsed = time.time() - t_start + + removal_status = None + if args.remove_worktree: + removal_status = remove_worktree(repo_root, worktree_dir) + + if args.stale_only: + results = [r for r in results if r.verdict == "LIKELY-STALE"] + if args.only_evidenced: + results = [r for r in results if r.evidence["pr_numbers"] or r.evidence["commit_candidates"]] + + stats = { + "beads_checked": len(selected), + "commits_checked": n_commits_checked, + "prs_checked": n_prs_checked, + "elapsed_seconds": round(elapsed, 2), + "worktree_dir": str(worktree_dir), + "worktree_removed": removal_status, + } + + if args.json: + counts = Counter(r.verdict for r in results) + payload = { + "stats": stats, + "verdict_counts": dict(counts), + "results": [ + { + "bead_id": r.bead_id, + "status": r.status, + "priority": r.priority, + "title": r.title, + "verdict": r.verdict, + "confidence": r.confidence, + "reasons": r.reasons, + "evidence": r.evidence, + } + for r in results + ], + } + json.dump(payload, sys.stdout, indent=2, default=str) + print() + else: + _render_human(results, stats) + + return 0 + + +if __name__ == "__main__": + raise SystemExit(main()) diff --git a/devtools/command_catalog.py b/devtools/command_catalog.py index 6f1cdf30d9..218755644b 100644 --- a/devtools/command_catalog.py +++ b/devtools/command_catalog.py @@ -617,6 +617,30 @@ def to_dict(self) -> dict[str, object]: "devtools workspace bead-cluster --input ready.json --validate-roster", ), ), + CommandSpec( + "workspace bead-landing-check", + "workspace", + "Verify whether a bead's cited commits/PRs already landed on master before dispatching an investigation.", + "devtools.bead_landing_check", + use_when=( + "Before claiming or dispatching a bead, or for a full-backlog sweep: extracts commit hashes, PR " + "numbers, and file paths cited in a bead's description/design/AC/notes/close_reason/comments, " + "cherry-picks cited commits onto master in ONE reused throwaway worktree (empty diff = already " + "landed, proof against squash-merge id rewriting), checks cited PR merge state via `gh pr view` " + "(disk-cached), and emits LIKELY-STALE / LIKELY-LIVE / UNDETERMINED per bead. Never guesses when " + "no evidence is cited -- UNDETERMINED is reported honestly rather than defaulting either way. " + "Never auto-closes anything; it only reports. Five open beads (o4j2, hiu, 0jf4, pbuh, cijx.4) each " + "cost a full dispatched-agent investigation cycle 2026-07-30/31 before turning out to already be " + "done on master -- this is the repeatable, cheap check that replaces hand-auditing 600+ beads." + ), + examples=( + "devtools workspace bead-landing-check", + "devtools workspace bead-landing-check --json --stale-only", + "devtools workspace bead-landing-check polylogue-o4j2 polylogue-hiu", + "devtools workspace bead-landing-check --offline --only-evidenced", + "devtools workspace bead-landing-check --no-commits # PR checks only, fastest pass", + ), + ), CommandSpec( "workspace bead-reimport-guard", "workspace", diff --git a/docs/devtools.md b/docs/devtools.md index ebf606b15b..2b7f583201 100644 --- a/docs/devtools.md +++ b/docs/devtools.md @@ -216,6 +216,7 @@ These are the commands worth remembering during normal repo work: | `devtools workspace basic-usage-demo-check` | Re-run the basic-usage demo suite's commands and assert output shape. | | `devtools workspace bead-batch-show` | Batch-show beads: id, status, prio, title, desc head, deps, notes tail. | | `devtools workspace bead-cluster` | Footprint/overlap/contention clustering of ready Beads (execution frontier). | +| `devtools workspace bead-landing-check` | Verify whether a bead's cited commits/PRs already landed on master before dispatching an investigation. | | `devtools workspace bead-reimport-guard` | Monotonic, receipted guard/reconcile/export for bd's JSONL synchronization. | | `devtools workspace claim-vs-evidence` | Build a structured failure follow-up claim-vs-evidence demo. | | `devtools workspace cli-surface-audit` | Capture a current-curated CLI surface audit demo. | From 39b69e96da51d0ec592fb077757ae97adf9c1384 Mon Sep 17 00:00:00 2001 From: Sinity Date: Fri, 31 Jul 2026 07:11:01 +0200 Subject: [PATCH 2/4] test(devtools): cover bead-landing-check evidence, verdicts, and worktree safety Covers: PR/commit/file evidence extraction (including the pull-URL false-positive fix), the LIKELY-STALE/LIKELY-LIVE/UNDETERMINED verdict matrix (empty-diff, already-on-master, content-equivalent, non-empty-diff, merged/open PR, unresolvable/conflicted evidence -- each must not be guessed into a confident verdict), CommitChecker against a real throwaway git repo (unknown-revision, already-on-master, non-empty-diff, and an empty-diff case built from independently-converging history to model a squash-merge equivalent), worktree reuse across checks, remove_worktree's live-process guard (spawns a real subprocess with cwd inside the worktree and confirms removal is blocked and the directory survives), and PrChecker disk-cache/offline/refresh semantics with `_run` mocked out (no live gh calls in tests). Verification: devtools test tests/unit/devtools/test_bead_landing_check.py -- 30 passed. ruff check/format and mypy --strict clean on the new file. Co-Authored-By: Claude --- .../unit/devtools/test_bead_landing_check.py | 424 ++++++++++++++++++ 1 file changed, 424 insertions(+) create mode 100644 tests/unit/devtools/test_bead_landing_check.py diff --git a/tests/unit/devtools/test_bead_landing_check.py b/tests/unit/devtools/test_bead_landing_check.py new file mode 100644 index 0000000000..4d0fdc5128 --- /dev/null +++ b/tests/unit/devtools/test_bead_landing_check.py @@ -0,0 +1,424 @@ +"""Tests for ``devtools workspace bead-landing-check``.""" + +from __future__ import annotations + +import json +import subprocess +import time +from pathlib import Path +from typing import Any + +import pytest + +from devtools.bead_landing_check import ( + CommitChecker, + Evidence, + PrChecker, + PrResult, + extract_evidence, + load_beads_from_jsonl, + remove_worktree, + verdict_for_bead, +) + +# --------------------------------------------------------------------------- +# helpers +# --------------------------------------------------------------------------- + + +def _run_git(args: list[str], *, cwd: Path) -> subprocess.CompletedProcess[str]: + result = subprocess.run(["git", *args], cwd=cwd, capture_output=True, text=True) + result.check_returncode() + return result + + +def _make_repo(path: Path) -> Path: + path.mkdir(parents=True, exist_ok=True) + _run_git(["init", "-b", "master"], cwd=path) + _run_git(["config", "user.email", "test@test"], cwd=path) + _run_git(["config", "user.name", "Test"], cwd=path) + (path / "file.txt").write_text("A\n") + _run_git(["add", "file.txt"], cwd=path) + _run_git(["commit", "-m", "initial"], cwd=path) + return path + + +def _bead( + bead_id: str = "polylogue-x", + *, + status: str = "open", + priority: int = 2, + title: str = "untitled", + description: str = "", + design: str = "", + acceptance_criteria: str = "", + notes: str = "", + close_reason: str = "", + comments: list[dict[str, str]] | None = None, +) -> dict[str, Any]: + return { + "id": bead_id, + "status": status, + "priority": priority, + "title": title, + "description": description, + "design": design, + "acceptance_criteria": acceptance_criteria, + "notes": notes, + "close_reason": close_reason, + "comments": comments or [], + } + + +# --------------------------------------------------------------------------- +# evidence extraction +# --------------------------------------------------------------------------- + + +def test_extract_evidence_finds_pr_numbers() -> None: + bead = _bead(description="Fixed via PR #3414 (branch feature/x, commit e9e7a7245).") + ev = extract_evidence(bead) + assert ev.pr_numbers == [3414] + + +def test_extract_evidence_ignores_pull_slash_url_to_avoid_sample_payload_false_positives() -> None: + # A bead quoting a *sample payload* containing an unrelated PR URL must not + # be read as citing that PR as its own resolving evidence (this was a real + # false positive against polylogue-pbuh, which quotes a pr-link sample + # payload referencing https://github.com/Sinity/polylogue/pull/3126). + bead = _bead( + description='Sample payload: {"prUrl": "https://github.com/Sinity/polylogue/pull/3126"}', + ) + ev = extract_evidence(bead) + assert ev.pr_numbers == [] + + +def test_extract_evidence_finds_commit_hashes_but_not_plain_numbers() -> None: + bead = _bead(description="commit e9e7a7245 fixed this. Also saw 850678 records and port 4000.") + ev = extract_evidence(bead) + assert "e9e7a7245" in ev.commit_candidates + assert "850678" not in ev.commit_candidates + assert "4000" not in ev.commit_candidates + + +def test_extract_evidence_reads_comments_too() -> None: + bead = _bead(comments=[{"text": "Landed in #3390."}]) + ev = extract_evidence(bead) + assert ev.pr_numbers == [3390] + + +def test_extract_evidence_finds_file_paths() -> None: + bead = _bead(description="See polylogue/sources/parsers/claude/code_parser.py:87 for the skip list.") + ev = extract_evidence(bead) + assert "polylogue/sources/parsers/claude/code_parser.py" in ev.file_paths + + +def test_extract_evidence_empty_when_no_signal() -> None: + ev = extract_evidence(_bead(description="This is a plain description with no citations.")) + assert ev == Evidence() + + +# --------------------------------------------------------------------------- +# verdict logic (pure function, no subprocess) +# --------------------------------------------------------------------------- + + +def test_verdict_empty_diff_commit_is_likely_stale_strong() -> None: + bead = _bead("polylogue-a") + ev = extract_evidence(_bead(description="commit abc1234ff already did this")) + v = verdict_for_bead(bead, ev, {"abc1234ff": "empty-diff"}, {}) + assert v.verdict == "LIKELY-STALE" + assert v.confidence == "strong" + + +def test_verdict_already_on_master_is_likely_stale_strong() -> None: + bead = _bead("polylogue-a") + ev = Evidence(commit_candidates=["abc1234ff"]) + v = verdict_for_bead(bead, ev, {"abc1234ff": "already-on-master"}, {}) + assert v.verdict == "LIKELY-STALE" + assert v.confidence == "strong" + + +def test_verdict_content_equivalent_is_likely_stale_strong() -> None: + bead = _bead("polylogue-a") + ev = Evidence(commit_candidates=["abc1234ff"]) + v = verdict_for_bead(bead, ev, {"abc1234ff": "content-equivalent"}, {}) + assert v.verdict == "LIKELY-STALE" + assert v.confidence == "strong" + + +def test_verdict_non_empty_diff_is_likely_live_strong() -> None: + bead = _bead("polylogue-a") + ev = Evidence(commit_candidates=["abc1234ff"]) + v = verdict_for_bead(bead, ev, {"abc1234ff": "non-empty-diff"}, {}) + assert v.verdict == "LIKELY-LIVE" + assert v.confidence == "strong" + + +def test_verdict_merged_pr_only_is_likely_stale_weak() -> None: + bead = _bead("polylogue-a") + ev = Evidence(pr_numbers=[3390]) + pr_results = {3390: PrResult(number=3390, found=True, state="MERGED")} + v = verdict_for_bead(bead, ev, {}, pr_results) + assert v.verdict == "LIKELY-STALE" + assert v.confidence == "weak" + + +def test_verdict_open_pr_is_likely_live_weak() -> None: + bead = _bead("polylogue-a") + ev = Evidence(pr_numbers=[3390]) + pr_results = {3390: PrResult(number=3390, found=True, state="OPEN")} + v = verdict_for_bead(bead, ev, {}, pr_results) + assert v.verdict == "LIKELY-LIVE" + assert v.confidence == "weak" + + +def test_verdict_no_evidence_is_undetermined() -> None: + bead = _bead("polylogue-a") + v = verdict_for_bead(bead, Evidence(), {}, {}) + assert v.verdict == "UNDETERMINED" + assert v.confidence == "none" + assert "not verifiable" in v.reasons[0] + + +def test_verdict_unresolvable_commit_is_undetermined_not_guessed() -> None: + # A bogus hex token (e.g. a worktree/session id) must never be silently + # treated as proof of anything -- this is the core honesty requirement. + bead = _bead("polylogue-a") + ev = Evidence(commit_candidates=["ad682bc849a1cd0f0"]) + v = verdict_for_bead(bead, ev, {"ad682bc849a1cd0f0": "unknown-revision"}, {}) + assert v.verdict == "UNDETERMINED" + + +def test_verdict_conflicted_commit_is_undetermined_not_guessed() -> None: + bead = _bead("polylogue-a") + ev = Evidence(commit_candidates=["abc1234ff"]) + v = verdict_for_bead(bead, ev, {"abc1234ff": "does-not-apply"}, {}) + assert v.verdict == "UNDETERMINED" + + +def test_verdict_pr_not_found_is_undetermined_not_guessed() -> None: + bead = _bead("polylogue-a") + ev = Evidence(pr_numbers=[99999]) + pr_results = {99999: PrResult(number=99999, found=False, error="not found")} + v = verdict_for_bead(bead, ev, {}, pr_results) + assert v.verdict == "UNDETERMINED" + + +def test_verdict_mixed_empty_and_non_empty_notes_partial_landing() -> None: + bead = _bead("polylogue-a") + ev = Evidence(commit_candidates=["aaa1111ff", "bbb2222ff"]) + v = verdict_for_bead(bead, ev, {"aaa1111ff": "empty-diff", "bbb2222ff": "non-empty-diff"}, {}) + assert v.verdict == "LIKELY-STALE" + assert any("partial" in r.lower() for r in v.reasons) + + +# --------------------------------------------------------------------------- +# CommitChecker against a real throwaway repo +# --------------------------------------------------------------------------- + + +def test_commit_checker_unknown_revision(tmp_path: Path) -> None: + repo = _make_repo(tmp_path / "repo") + checker = CommitChecker(repo, tmp_path / "wt", base_ref="master") + assert checker.check("deadbeef00") == "unknown-revision" + + +def test_commit_checker_already_on_master(tmp_path: Path) -> None: + repo = _make_repo(tmp_path / "repo") + tip = _run_git(["rev-parse", "HEAD"], cwd=repo).stdout.strip() + checker = CommitChecker(repo, tmp_path / "wt", base_ref="master") + assert checker.check(tip) == "already-on-master" + + +def test_commit_checker_non_empty_diff_for_unmerged_commit(tmp_path: Path) -> None: + repo = _make_repo(tmp_path / "repo") + _run_git(["checkout", "-b", "feature"], cwd=repo) + (repo / "other.txt").write_text("new file\n") + _run_git(["add", "other.txt"], cwd=repo) + _run_git(["commit", "-m", "add other.txt"], cwd=repo) + feature_sha = _run_git(["rev-parse", "HEAD"], cwd=repo).stdout.strip() + _run_git(["checkout", "master"], cwd=repo) + + checker = CommitChecker(repo, tmp_path / "wt", base_ref="master") + assert checker.check(feature_sha) == "non-empty-diff" + + +def test_commit_checker_empty_diff_when_change_already_present_via_different_history(tmp_path: Path) -> None: + repo = _make_repo(tmp_path / "repo") + # master: file.txt A -> B + (repo / "file.txt").write_text("B\n") + _run_git(["add", "file.txt"], cwd=repo) + _run_git(["commit", "-m", "A to B on master"], cwd=repo) + + # A divergent branch from the *original* commit that makes the exact same + # A -> B change independently (simulating a squash-merged equivalent). + initial_sha = _run_git(["rev-list", "--max-parents=0", "HEAD"], cwd=repo).stdout.strip() + _run_git(["checkout", "-b", "other-lane", initial_sha], cwd=repo) + (repo / "file.txt").write_text("B\n") + _run_git(["add", "file.txt"], cwd=repo) + _run_git(["commit", "-m", "same A to B change, different lineage"], cwd=repo) + other_sha = _run_git(["rev-parse", "HEAD"], cwd=repo).stdout.strip() + _run_git(["checkout", "master"], cwd=repo) + + checker = CommitChecker(repo, tmp_path / "wt", base_ref="master") + assert checker.check(other_sha) == "empty-diff" + + +def test_commit_checker_reuses_one_worktree_across_checks(tmp_path: Path) -> None: + repo = _make_repo(tmp_path / "repo") + wt_dir = tmp_path / "wt" + checker = CommitChecker(repo, wt_dir, base_ref="master") + + checker.check("deadbeef00") + assert not wt_dir.exists() # unknown-revision never needs the worktree + + tip = _run_git(["rev-parse", "HEAD"], cwd=repo).stdout.strip() + checker.check(tip) + assert not wt_dir.exists() # already-on-master short-circuits too + + _run_git(["checkout", "-b", "feature"], cwd=repo) + (repo / "other.txt").write_text("x\n") + _run_git(["add", "other.txt"], cwd=repo) + _run_git(["commit", "-m", "add"], cwd=repo) + feature_sha = _run_git(["rev-parse", "HEAD"], cwd=repo).stdout.strip() + _run_git(["checkout", "master"], cwd=repo) + + checker.check(feature_sha) + assert wt_dir.exists() + marker_mtime = (wt_dir / ".git").stat().st_mtime + + # A second, distinct commit must reuse the same worktree directory rather + # than creating a fresh one. + (repo / "third.txt").write_text("y\n") + _run_git(["add", "third.txt"], cwd=repo) + _run_git(["commit", "-m", "add third"], cwd=repo) + third_sha = _run_git(["rev-parse", "HEAD"], cwd=repo).stdout.strip() + _run_git(["checkout", "master~1"], cwd=repo) # detach so third_sha isn't already master's tip + _run_git(["checkout", "master"], cwd=repo) + + checker.check(third_sha) + assert wt_dir.exists() + assert (wt_dir / ".git").stat().st_mtime == marker_mtime + + +def test_commit_checker_rejects_worktree_dir_equal_to_repo_root(tmp_path: Path) -> None: + repo = _make_repo(tmp_path / "repo") + with pytest.raises(ValueError): + CommitChecker(repo, repo, base_ref="master") + + +# --------------------------------------------------------------------------- +# worktree removal safety +# --------------------------------------------------------------------------- + + +def test_remove_worktree_not_present(tmp_path: Path) -> None: + repo = _make_repo(tmp_path / "repo") + assert remove_worktree(repo, tmp_path / "does-not-exist") == "not-present" + + +def test_remove_worktree_blocks_on_live_process(tmp_path: Path) -> None: + repo = _make_repo(tmp_path / "repo") + wt = tmp_path / "wt" + _run_git(["worktree", "add", "--detach", str(wt), "master"], cwd=repo) + + proc = subprocess.Popen(["sleep", "5"], cwd=wt) + try: + # Give the OS a moment to publish /proc//cwd. + for _ in range(50): + if Path(f"/proc/{proc.pid}/cwd").exists(): + break + time.sleep(0.05) + assert remove_worktree(repo, wt) == "blocked-live-process" + assert wt.exists() # never removed while occupied + finally: + proc.kill() + proc.wait() + + +def test_remove_worktree_removes_clean_unoccupied_worktree(tmp_path: Path) -> None: + repo = _make_repo(tmp_path / "repo") + wt = tmp_path / "wt" + _run_git(["worktree", "add", "--detach", str(wt), "master"], cwd=repo) + assert remove_worktree(repo, wt) == "removed" + assert not wt.exists() + + +# --------------------------------------------------------------------------- +# PrChecker caching (subprocess mocked out) +# --------------------------------------------------------------------------- + + +def test_pr_checker_caches_merged_result_permanently(tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> None: + calls = {"n": 0} + + def fake_run(cmd: list[str], **kwargs: Any) -> subprocess.CompletedProcess[str]: + calls["n"] += 1 + return subprocess.CompletedProcess( + cmd, + 0, + stdout=json.dumps({"state": "MERGED", "mergedAt": "t", "mergeCommit": {"oid": "abc"}, "title": "x"}), + stderr="", + ) + + monkeypatch.setattr("devtools.bead_landing_check._run", fake_run) + cache_path = tmp_path / "cache.json" + + checker = PrChecker("Sinity/polylogue", cache_path, ttl_days=7) + r1 = checker.check(100) + assert r1.state == "MERGED" + assert calls["n"] == 1 + checker.flush() + + # Fresh PrChecker instance reading the persisted cache must not re-fetch. + checker2 = PrChecker("Sinity/polylogue", cache_path, ttl_days=7) + r2 = checker2.check(100) + assert r2.state == "MERGED" + assert calls["n"] == 1 + + +def test_pr_checker_offline_uses_cache_only(tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> None: + def fake_run(cmd: list[str], **kwargs: Any) -> subprocess.CompletedProcess[str]: + raise AssertionError("gh must not be called in offline mode") + + monkeypatch.setattr("devtools.bead_landing_check._run", fake_run) + cache_path = tmp_path / "cache.json" + checker = PrChecker("Sinity/polylogue", cache_path, ttl_days=7, offline=True) + r = checker.check(200) + assert r.found is False + assert r.error == "offline: not cached" + + +def test_pr_checker_refresh_ignores_cache(tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> None: + calls = {"n": 0} + + def fake_run(cmd: list[str], **kwargs: Any) -> subprocess.CompletedProcess[str]: + calls["n"] += 1 + return subprocess.CompletedProcess( + cmd, 0, stdout=json.dumps({"state": "OPEN", "mergedAt": None, "mergeCommit": None, "title": "x"}), stderr="" + ) + + monkeypatch.setattr("devtools.bead_landing_check._run", fake_run) + cache_path = tmp_path / "cache.json" + checker = PrChecker("Sinity/polylogue", cache_path, ttl_days=7) + checker.check(300) + assert calls["n"] == 1 + checker.check(300) + assert calls["n"] == 1 # OPEN is TTL-cached, reused within TTL + + checker_refresh = PrChecker("Sinity/polylogue", cache_path, ttl_days=7, refresh=True) + checker_refresh.check(300) + assert calls["n"] == 2 + + +# --------------------------------------------------------------------------- +# jsonl loading +# --------------------------------------------------------------------------- + + +def test_load_beads_from_jsonl(tmp_path: Path) -> None: + p = tmp_path / "issues.jsonl" + p.write_text('{"id": "a"}\n\n{"id": "b"}\n') + beads = load_beads_from_jsonl(p) + assert [b["id"] for b in beads] == ["a", "b"] From d7cd95d52a9217e305370e8a0c077274ddc6bf99 Mon Sep 17 00:00:00 2001 From: Sinity Date: Fri, 31 Jul 2026 07:20:10 +0200 Subject: [PATCH 3/4] fix(devtools): exclude snapshot-anchor citations from bead-landing-check Problem: the first full sweep over the live backlog (629 open/in_progress beads) flagged 174 "strong" LIKELY-STALE verdicts, but spot-checking them found systematic false positives. This repo's prework-packet convention writes "Generated from master @ " (185+ occurrences) and verification-pass notes write "re-checked ... against current master ()" -- both record what master looked like when a note was written, not that the cited hash is the bead's own resolving commit. Confirmed false positive: polylogue-lkrc cites its own verification-pass master snapshot this way in a note that explicitly says the named gaps are STILL open -- the tool nonetheless flagged it LIKELY-STALE (strong) because the snapshot hash is, trivially, an ancestor of current master. Fix: extract_evidence() now excludes a hex token when the ~24 characters immediately preceding it match "master @", "master (", "master@", or bare "master " (case-insensitive) -- this repo's snapshot-anchor idiom. Re-run over the same 629 beads: strong LIKELY-STALE dropped from 174 to 71, UNDETERMINED rose from 351 to 435 (favoring "not verifiable" over a false positive, per the anti-vacuity requirement that a wrong LIKELY-STALE risks dropping real work). Also added a standing CAVEAT reason line on every LIKELY-STALE verdict: proving a cited commit/PR exists on master is not proof every acceptance criterion is satisfied. Verification: devtools test tests/unit/devtools/test_bead_landing_check.py -- 33 passed (3 new regression tests reproducing the lkrc/2qx false positives and a still-genuine self-citation case). ruff check/format and mypy --strict clean. Co-Authored-By: Claude --- devtools/bead_landing_check.py | 25 +++++++++++-- .../unit/devtools/test_bead_landing_check.py | 35 +++++++++++++++++++ 2 files changed, 58 insertions(+), 2 deletions(-) diff --git a/devtools/bead_landing_check.py b/devtools/bead_landing_check.py index 96b4f3c431..32222c5f11 100644 --- a/devtools/bead_landing_check.py +++ b/devtools/bead_landing_check.py @@ -86,6 +86,9 @@ # false PR citations unconnected to the bead's own resolution. _PR_PATTERN = re.compile(r"(?:^|[^\w/])#(\d{2,6})\b") _COMMIT_PATTERN = re.compile(r"\b[0-9a-f]{7,40}\b") +# Matches immediately before a hash that reads "master @", "master (", +# "master@", or bare "master " -- this repo's snapshot-anchor idiom. +_SNAPSHOT_ANCHOR_PATTERN = re.compile(r"master\s*[@(]?\s*$", re.IGNORECASE) _FILE_PATTERN = re.compile( r"(?:polylogue|tests|devtools|docs|storage|pipeline|daemon|cli|mcp" r"|browser[_-]extension|browser_capture|coordination|archive|insights" @@ -134,8 +137,19 @@ def extract_evidence(bead: BeadDict) -> Evidence: token = m.group(0) # Require a hex token to mix digits and letters -- a pure-digit run is # a plain number (record count, port, etc.), not a commit-ish hash. - if any(c.isdigit() for c in token) and any(c.isalpha() for c in token): - commits.add(token) + if not (any(c.isdigit() for c in token) and any(c.isalpha() for c in token)): + continue + if _SNAPSHOT_ANCHOR_PATTERN.search(text[max(0, m.start() - 24) : m.start()]): + # "Generated from master @ " / "against current master + # ()" is this repo's prework-packet/verification-pass + # snapshot-anchor idiom (185+ occurrences in the live backlog): + # it records what master looked like when a note was written, + # not that the cited hash is this bead's own resolving commit. + # Confirmed false positive: polylogue-lkrc cites its own + # verification-pass master snapshot this way in a note that + # explicitly says the named gaps are STILL open. + continue + commits.add(token) files = list(dict.fromkeys(_FILE_PATTERN.findall(text))) @@ -506,6 +520,13 @@ def verdict_for_bead( if not reasons: reasons.append("no cited commit hash or PR number found in bead text -- not verifiable by this tool") + if verdict == "LIKELY-STALE": + reasons.append( + "CAVEAT: this proves the cited commit(s)/PR(s) exist on master, not that every acceptance " + "criterion is satisfied -- a bead can cite a landed prerequisite, a partial-progress note, or a " + "verification-pass anchor while real remaining scope stays open. Read the bead's AC list before closing." + ) + evidence_payload: dict[str, Any] = { "pr_numbers": evidence.pr_numbers, "commit_candidates": evidence.commit_candidates, diff --git a/tests/unit/devtools/test_bead_landing_check.py b/tests/unit/devtools/test_bead_landing_check.py index 4d0fdc5128..f0c5ebcab5 100644 --- a/tests/unit/devtools/test_bead_landing_check.py +++ b/tests/unit/devtools/test_bead_landing_check.py @@ -113,6 +113,41 @@ def test_extract_evidence_finds_file_paths() -> None: assert "polylogue/sources/parsers/claude/code_parser.py" in ev.file_paths +def test_extract_evidence_ignores_snapshot_anchor_citations() -> None: + # "Generated from master @ " is this repo's prework-packet snapshot + # header (185+ occurrences in the live backlog) -- it records what master + # looked like when the note was written, not that the hash is the bead's + # own resolving commit. Confirmed false positive against polylogue-2qx. + bead = _bead( + description=( + "Generated from master @ 8a975a40 2026-07-06 -- verify source anchors before coding; " + "line numbers are snapshot-relative." + ) + ) + ev = extract_evidence(bead) + assert ev.commit_candidates == [] + + +def test_extract_evidence_ignores_verification_pass_master_anchor() -> None: + # Confirmed false positive against polylogue-lkrc: the note explicitly + # says the named gaps are STILL open as of this master snapshot. + bead = _bead( + description=( + "2026-07-14 code-verification pass: re-checked the residual gaps against " + "current master (031d8d183) source. All 3 named residual gaps are still open." + ) + ) + ev = extract_evidence(bead) + assert ev.commit_candidates == [] + + +def test_extract_evidence_still_finds_genuine_self_citation_commit() -> None: + bead = _bead(description="Foundation phase merged via PR #2915 as d6501ac4615efa30cb0e2413c97614a4bf44b253.") + ev = extract_evidence(bead) + assert ev.commit_candidates == ["d6501ac4615efa30cb0e2413c97614a4bf44b253"] + assert ev.pr_numbers == [2915] + + def test_extract_evidence_empty_when_no_signal() -> None: ev = extract_evidence(_bead(description="This is a plain description with no citations.")) assert ev == Evidence() From c94edc0375f3ebb3da179c8db02ea6b5181e7294 Mon Sep 17 00:00:00 2001 From: Sinity Date: Fri, 31 Jul 2026 08:16:40 +0200 Subject: [PATCH 4/4] fix(devtools): require a live consumer before bead-landing-check calls LIKELY-STALE Problem: five independent human reviewers checked all 190 LIKELY-STALE verdicts from the first full sweep against 114 real beads. Measured precision was catastrophic -- roughly 6 of 114 (~5%) were genuinely safe to close; ~95% were false positives. The root cause, found independently by two reviewers: the heuristic keyed off "a cited commit/PR exists on master" without checking whether the shipped code has any live production consumer -- exactly the "code shipped, nothing reads it" defect class this repo has spent the night finding elsewhere. Concrete confirmed false positives: polylogue-rxdo.9.6 (blind_items() has zero callers outside its own test), polylogue-rxdo.6 (ReferenceQueryPipeline has zero CLI/MCP/daemon references and still hard-errors), polylogue-rxdo.9.7 (ClaimWithControls has zero callers outside its own test), polylogue-hg97 (cost_outlook absent from polylogue/mcp/, its contract test still xfail), polylogue-dcz5 (3.14t live in prod, but daemon_parse_stage_split is still False), polylogue-yp0 (EventBus core landed, notes explicitly say "NOT wired to a live producer/consumer"). A second failure mode: epic/parent beads flagged because their own commit landed while most dotted-id/parent-child dependents remain open (polylogue-2qx: 9 open dependents; polylogue-3tl: 17; polylogue-rxdo.9: 7 of 9 children). Solution -- three corrective checks, all downgrade-only (never upgrade a verdict, per the standing rule that a wrong LIKELY-STALE is worse than an honest UNDETERMINED): 1. LIVE-CONSUMER CHECK (CommitChecker.added_symbols/has_live_consumer/ consumer_check): a landed commit only counts as strong evidence if at least one top-level symbol it added has a `git grep` hit outside the commit's own touched files and outside tests/. Landed-but-unconsumed or landed-with-inconclusive-symbols both downgrade to UNDETERMINED. 2. OPEN-DEPENDENTS CHECK (build_open_parent_child_dependents_index): a bead with unresolved `parent-child` dependents (open/in_progress) is never LIKELY-STALE regardless of its own evidence. 3. SUPPRESSION-PHRASE CHECK (find_suppression_signal): if the bead's own text already says "deferred"/"xfail"/"not wired"/"is NOT satisfied"/etc, that overrides commit/PR evidence and forces UNDETERMINED. Re-run over the same 629 open+in_progress beads: LIKELY-STALE dropped from 190 to 84 (UNDETERMINED rose from 435 to 541), runtime 14s warm (was 8s -- consumer checks add `git show`/`git grep` per landed commit, still far below the cold-sweep baseline). Known residual gap: polylogue-a7xr.16 remains a weak-confidence LIKELY-STALE from a merged-PR citation alone -- its own note claims "WORK COMPLETE" while only the INSERT half of the column-spec refactor actually landed (SELECT's 14 methods/503 accessors untouched); no cited commit and no suppression phrase exists in its text for this tool to catch, which is exactly why the CAVEAT reason on every LIKELY-STALE verdict says to read the AC list, not just the tool's verdict. Verification: devtools test tests/unit/devtools/test_bead_landing_check.py -- 54 passed, including a new labelled-evaluation regression block that runs the real pipeline (real git history, real .beads/issues.jsonl, no network) against the six named false positives and asserts none regress back to a strong-confidence LIKELY-STALE verdict. ruff check/format and mypy --strict clean. devtools verify --quick exit 0. devtools render all --check: no drift. Co-Authored-By: Claude --- devtools/bead_landing_check.py | 238 +++++++++++- .../unit/devtools/test_bead_landing_check.py | 358 +++++++++++++++++- 2 files changed, 579 insertions(+), 17 deletions(-) diff --git a/devtools/bead_landing_check.py b/devtools/bead_landing_check.py index 32222c5f11..e22afef599 100644 --- a/devtools/bead_landing_check.py +++ b/devtools/bead_landing_check.py @@ -33,6 +33,38 @@ never auto-closes anything -- it only reports; a human or a follow-up bead decides. +CORRECTION (2026-07-31, five independent human reviewers checked all 190 +LIKELY-STALE verdicts from the first sweep against the live archive): "a +cited commit exists on master" is not "the bead is done" -- roughly 6 of +every 114 checked were genuinely safe to close. The dominant failure mode is +exactly the defect class this repo has spent the night finding elsewhere: +code SHIPPED with no live production consumer (an unreferenced function, an +unwired EventBus, a contract test still `xfail`). A second failure mode is +epic/parent beads whose own commit landed while most of their dotted-id +children or `parent-child` dependents are still open. A third is beads whose +own latest note already says "remaining" / "deferred" / "not attempted" in +plain language, contradicted only by an earlier landed-prerequisite citation. + +Three corrective checks now run before a verdict is allowed to reach +LIKELY-STALE: + + 1. LIVE-CONSUMER CHECK: a landed commit only counts as strong evidence if + at least one top-level symbol it added has a `git grep` hit outside the + commit's own touched files and outside tests/. A landed commit with no + provable consumer (or no extractable symbols at all -- non-Python + changes) downgrades the bead to UNDETERMINED instead of LIKELY-STALE. + 2. OPEN-DEPENDENTS CHECK: a bead with unresolved `parent-child` dependents + (open or in_progress) is never LIKELY-STALE regardless of its own + evidence -- an epic landing a foundational commit does not mean its + children are done. + 3. SUPPRESSION-PHRASE CHECK: if the bead's own text contains an explicit + remaining/deferred/not-implemented/no-consumer phrase, that overrides + any commit/PR evidence and forces UNDETERMINED. + +None of these upgrade a verdict -- they only ever downgrade toward +UNDETERMINED, in keeping with the rule that a wrong LIKELY-STALE is worse +than an honest "not verifiable". + Usage: # Sweep every open + in_progress bead (the default): devtools workspace bead-landing-check @@ -96,6 +128,25 @@ re.IGNORECASE, ) +# A line added by a diff that defines a top-level-ish function/class -- used +# by the live-consumer check to find what a landed commit actually shipped. +_ADDED_SYMBOL_PATTERN = re.compile(r"^\+\s*(?:async\s+)?(?:def|class)\s+(\w+)") + +# Phrases indicating a bead's own text already says the work is unfinished -- +# a strong suppression signal against a stale verdict built on commit/PR +# citation alone. Case-insensitive; matched anywhere in the bead's combined +# text, not only its most recent note (notes are an unstructured blob here). +_SUPPRESSION_PATTERN = re.compile( + r"\b(?:" + r"not attempted|no code written|not implemented|not yet implemented|" + r"not wired|zero callers|no callers|no live (?:producer|consumer)|" + r"remains? open|still open|still not|deferred(?: to)?|explicitly deferred|" + r"not yet wired|xfail|" + r"is NOT satisfied|not satisfied|not attempted" + r")\b", + re.IGNORECASE, +) + _CACHE_DIR_NAME = ".cache/bead-landing-check" _DEFAULT_WORKTREE_SUBDIR = "worktree" _DEFAULT_CACHE_TTL_DAYS = 7 @@ -156,15 +207,58 @@ def extract_evidence(bead: BeadDict) -> Evidence: return Evidence(pr_numbers=sorted(prs), commit_candidates=sorted(commits), file_paths=files) +def find_suppression_signal(bead: BeadDict) -> str | None: + """Return a short excerpt if the bead's own text already says the work is + unfinished (fix 3: weight the bead's own words above commit existence). + """ + text = _bead_text(bead) + m = _SUPPRESSION_PATTERN.search(text) + if not m: + return None + start = max(0, m.start() - 40) + end = min(len(text), m.end() + 40) + return " ".join(text[start:end].split()) + + +def build_open_parent_child_dependents_index(all_beads: list[BeadDict]) -> dict[str, list[dict[str, str]]]: + """Map ``parent_bead_id -> [{"id": ..., "status": ...}, ...]`` for every + still-open/in_progress bead whose ``dependencies`` list records a + ``parent-child`` edge onto that parent (fix 2: an epic's own commit + landing does not mean its children are done). + """ + index: dict[str, list[dict[str, str]]] = {} + for bead in all_beads: + if bead.get("status") not in ("open", "in_progress"): + continue + for dep in bead.get("dependencies") or []: + if dep.get("type") != "parent-child": + continue + parent_id = dep.get("depends_on_id") + if not parent_id: + continue + index.setdefault(parent_id, []).append({"id": bead["id"], "status": bead["status"]}) + return index + + # --------------------------------------------------------------------------- # git plumbing # --------------------------------------------------------------------------- +_MAX_CONSUMER_SYMBOLS_PER_COMMIT = 4 + + def _run(cmd: list[str], *, cwd: Path | None = None, timeout: float | None = 30) -> subprocess.CompletedProcess[str]: return subprocess.run(cmd, cwd=cwd, capture_output=True, text=True, timeout=timeout) +def _is_test_path(path: str) -> bool: + if not path: + return True # empty/unparseable -- treat conservatively as not a production consumer + parts = path.split("/") + return "tests" in parts or parts[-1].startswith("test_") or parts[-1].endswith("_test.py") + + class CommitChecker: """Resolves cited commits against a single reused throwaway worktree.""" @@ -176,6 +270,7 @@ def __init__(self, repo_root: Path, worktree_dir: Path, base_ref: str = "origin/ self.base_ref = base_ref self._ensured = False self._fetched = False + self._consumer_cache: dict[str, bool | None] = {} def _ensure_fetch(self) -> None: if not self._fetched: @@ -275,6 +370,60 @@ def check(self, commit: str) -> str: self._reset_worktree() return "empty-diff" if empty else "non-empty-diff" + def added_symbols(self, commit: str) -> list[str]: + """Top-level function/class names *commit* added, per its diff. + + Best-effort text extraction, not an AST diff -- good enough to seed + `git grep` lookups for the live-consumer check. + """ + result = _run(["git", "show", "--unified=0", "--no-color", commit], cwd=self.repo_root, timeout=30) + if result.returncode != 0: + return [] + symbols: list[str] = [] + for line in result.stdout.splitlines(): + if line.startswith("+++") or line.startswith("---"): + continue + m = _ADDED_SYMBOL_PATTERN.match(line) + if not m: + continue + name = m.group(1) + if name.startswith("test_") or name in ("__init__", "__post_init__", "__repr__", "__str__"): + continue + if name not in symbols: + symbols.append(name) + return symbols[:_MAX_CONSUMER_SYMBOLS_PER_COMMIT] + + def has_live_consumer(self, commit: str, symbols: list[str]) -> bool | None: + """Return True if any *symbols* has a reference outside the commit's + own touched files and outside tests/, False if none do, None if there + was nothing checkable (e.g. a non-Python change with no symbols). + """ + if not symbols: + return None + touched = set(self._commit_touched_files(commit)) + for symbol in symbols: + result = _run( + ["git", "grep", "--no-color", "-w", "-l", symbol, self.base_ref, "--", "*.py"], + cwd=self.repo_root, + timeout=15, + ) + if result.returncode not in (0, 1): + continue # grep error on this symbol -- skip, don't crash the sweep + for line in result.stdout.splitlines(): + _, _, path = line.partition(":") + if path in touched or _is_test_path(path): + continue + return True + return False + + def consumer_check(self, commit: str) -> bool | None: + """Combined added_symbols + has_live_consumer, cached per commit.""" + if commit in self._consumer_cache: + return self._consumer_cache[commit] + result = self.has_live_consumer(commit, self.added_symbols(commit)) + self._consumer_cache[commit] = result + return result + def check_file_paths(repo_root: Path, paths: list[str], base_ref: str) -> list[dict[str, Any]]: out: list[dict[str, Any]] = [] @@ -465,10 +614,19 @@ def verdict_for_bead( evidence: Evidence, commit_results: dict[str, str], pr_results: dict[int, PrResult], + *, + commit_consumer: dict[str, bool | None] | None = None, + open_dependents: list[dict[str, str]] | None = None, + suppression_signal: str | None = None, ) -> BeadVerdict: reasons: list[str] = [] + commit_consumer = commit_consumer or {} + open_dependents = open_dependents or [] - landed = sorted(c for c, s in commit_results.items() if s in _LANDED_COMMIT_STATES) + landed_all = sorted(c for c, s in commit_results.items() if s in _LANDED_COMMIT_STATES) + landed_verified = sorted(c for c in landed_all if commit_consumer.get(c) is True) + landed_unconsumed = sorted(c for c in landed_all if commit_consumer.get(c) is False) + landed_unknown_consumer = sorted(c for c in landed_all if c not in landed_verified and c not in landed_unconsumed) live = sorted(c for c, s in commit_results.items() if s == "non-empty-diff") unresolved = sorted(c for c, s in commit_results.items() if s == "unknown-revision") inapplicable = sorted(c for c, s in commit_results.items() if s == "does-not-apply") @@ -477,10 +635,10 @@ def verdict_for_bead( open_prs = sorted(n for n, r in pr_results.items() if r.found and r.state != "MERGED") missing_prs = sorted(n for n, r in pr_results.items() if not r.found) - if landed: + if landed_verified: verdict, confidence = "LIKELY-STALE", "strong" - detail = ", ".join(f"{c} ({commit_results[c]})" for c in landed) - reasons.append(f"cited commit(s) already on master: {detail}") + detail = ", ".join(f"{c} ({commit_results[c]})" for c in landed_verified) + reasons.append(f"cited commit(s) already on master AND reachable from a live caller: {detail}") if live: reasons.append( f"NOTE: commit(s) {', '.join(live)} still produce a non-empty diff -- " @@ -491,6 +649,24 @@ def verdict_for_bead( reasons.append( f"cited commit(s) cherry-pick with a real (non-empty) diff, not yet on master: {', '.join(live)}" ) + elif landed_unconsumed or landed_unknown_consumer: + # Landed on master, but the live-consumer check (fix 1, 2026-07-31) + # could not prove the shipped code is reachable from a production + # caller. Measured against 114 human-verified verdicts, "cited commit + # exists on master" alone was right about 6 times in 114 -- this is + # exactly the "shipped, nothing reads it" defect class the check + # exists to catch, so it does NOT count as LIKELY-STALE on its own. + verdict, confidence = "UNDETERMINED", "none" + if landed_unconsumed: + reasons.append( + "commit(s) already on master but NO live production consumer found outside tests -- " + f"shipped without being wired in, treat as still open: {', '.join(landed_unconsumed)}" + ) + if landed_unknown_consumer: + reasons.append( + "commit(s) already on master but consumer-reachability could not be checked (non-Python " + f"change or no top-level symbols added) -- not enough to confirm wiring: {', '.join(landed_unknown_consumer)}" + ) elif merged_prs and not open_prs: verdict, confidence = "LIKELY-STALE", "weak" reasons.append( @@ -520,11 +696,30 @@ def verdict_for_bead( if not reasons: reasons.append("no cited commit hash or PR number found in bead text -- not verifiable by this tool") + # Cross-cutting downgrades (fix 2, fix 3, 2026-07-31): applied after the + # evidence-shape branch above. Both only ever pull a verdict DOWN toward + # UNDETERMINED -- neither can upgrade LIKELY-LIVE or UNDETERMINED to STALE. + if verdict == "LIKELY-STALE" and open_dependents: + verdict, confidence = "UNDETERMINED", "none" + examples = ", ".join(d["id"] for d in open_dependents[:5]) + more = f" (+{len(open_dependents) - 5} more)" if len(open_dependents) > 5 else "" + reasons.append( + f"{len(open_dependents)} open parent-child dependent bead(s) still unresolved -- an epic/parent " + f"commit landing does not mean its children are done: {examples}{more}" + ) + + if verdict == "LIKELY-STALE" and suppression_signal: + verdict, confidence = "UNDETERMINED", "none" + reasons.append( + "bead's own text contains an explicit remaining/deferred/not-done phrase that contradicts a stale " + f'verdict built on commit/PR citation alone: "...{suppression_signal}..."' + ) + if verdict == "LIKELY-STALE": reasons.append( - "CAVEAT: this proves the cited commit(s)/PR(s) exist on master, not that every acceptance " - "criterion is satisfied -- a bead can cite a landed prerequisite, a partial-progress note, or a " - "verification-pass anchor while real remaining scope stays open. Read the bead's AC list before closing." + "CAVEAT: this proves the cited commit(s)/PR(s) exist on master AND (for commit evidence) that the " + "shipped code has a live caller -- it is still not proof every acceptance criterion is satisfied. " + "Read the bead's AC list before closing." ) evidence_payload: dict[str, Any] = { @@ -532,6 +727,9 @@ def verdict_for_bead( "commit_candidates": evidence.commit_candidates, "file_paths": evidence.file_paths, "commit_checks": dict(commit_results), + "commit_consumer_checks": dict(commit_consumer), + "open_parent_child_dependents": open_dependents, + "suppression_signal": suppression_signal, "pr_checks": { str(n): { "found": r.found, @@ -590,7 +788,8 @@ def _render_human(results: list[BeadVerdict], stats: dict[str, Any]) -> None: counts = Counter(r.verdict for r in results) print( f"Checked {stats['beads_checked']} beads in {stats['elapsed_seconds']}s " - f"({stats['commits_checked']} commit checks, {stats['prs_checked']} PR checks)" + f"({stats['commits_checked']} commit checks, {stats.get('consumer_checks', 0)} consumer checks, " + f"{stats['prs_checked']} PR checks)" ) print("Verdicts: " + " ".join(f"{k}={v}" for k, v in sorted(counts.items()))) removal_note = ( @@ -703,19 +902,27 @@ def main(argv: Sequence[str] | None = None) -> int: ) ) + dependents_index = build_open_parent_child_dependents_index(all_beads) + t_start = time.time() results: list[BeadVerdict] = [] n_commits_checked = 0 n_prs_checked = 0 + n_consumer_checks = 0 for bead in selected: evidence = extract_evidence(bead) commit_results: dict[str, str] = {} + commit_consumer: dict[str, bool | None] = {} if commit_checker is not None: for commit in evidence.commit_candidates: - commit_results[commit] = commit_checker.check(commit) + status = commit_checker.check(commit) + commit_results[commit] = status n_commits_checked += 1 + if status in _LANDED_COMMIT_STATES: + commit_consumer[commit] = commit_checker.consumer_check(commit) + n_consumer_checks += 1 pr_results: dict[int, PrResult] = {} if pr_checker is not None: @@ -723,7 +930,17 @@ def main(argv: Sequence[str] | None = None) -> int: pr_results[number] = pr_checker.check(number) n_prs_checked += 1 - results.append(verdict_for_bead(bead, evidence, commit_results, pr_results)) + results.append( + verdict_for_bead( + bead, + evidence, + commit_results, + pr_results, + commit_consumer=commit_consumer, + open_dependents=dependents_index.get(bead["id"], []), + suppression_signal=find_suppression_signal(bead), + ) + ) if pr_checker is not None: pr_checker.flush() @@ -742,6 +959,7 @@ def main(argv: Sequence[str] | None = None) -> int: stats = { "beads_checked": len(selected), "commits_checked": n_commits_checked, + "consumer_checks": n_consumer_checks, "prs_checked": n_prs_checked, "elapsed_seconds": round(elapsed, 2), "worktree_dir": str(worktree_dir), diff --git a/tests/unit/devtools/test_bead_landing_check.py b/tests/unit/devtools/test_bead_landing_check.py index f0c5ebcab5..a545df137e 100644 --- a/tests/unit/devtools/test_bead_landing_check.py +++ b/tests/unit/devtools/test_bead_landing_check.py @@ -3,6 +3,7 @@ from __future__ import annotations import json +import os import subprocess import time from pathlib import Path @@ -15,7 +16,9 @@ Evidence, PrChecker, PrResult, + build_open_parent_child_dependents_index, extract_evidence, + find_suppression_signal, load_beads_from_jsonl, remove_worktree, verdict_for_bead, @@ -158,30 +161,164 @@ def test_extract_evidence_empty_when_no_signal() -> None: # --------------------------------------------------------------------------- -def test_verdict_empty_diff_commit_is_likely_stale_strong() -> None: +def test_verdict_empty_diff_commit_is_likely_stale_strong_when_consumed() -> None: bead = _bead("polylogue-a") ev = extract_evidence(_bead(description="commit abc1234ff already did this")) - v = verdict_for_bead(bead, ev, {"abc1234ff": "empty-diff"}, {}) + v = verdict_for_bead(bead, ev, {"abc1234ff": "empty-diff"}, {}, commit_consumer={"abc1234ff": True}) assert v.verdict == "LIKELY-STALE" assert v.confidence == "strong" -def test_verdict_already_on_master_is_likely_stale_strong() -> None: +def test_verdict_already_on_master_is_likely_stale_strong_when_consumed() -> None: bead = _bead("polylogue-a") ev = Evidence(commit_candidates=["abc1234ff"]) - v = verdict_for_bead(bead, ev, {"abc1234ff": "already-on-master"}, {}) + v = verdict_for_bead(bead, ev, {"abc1234ff": "already-on-master"}, {}, commit_consumer={"abc1234ff": True}) assert v.verdict == "LIKELY-STALE" assert v.confidence == "strong" -def test_verdict_content_equivalent_is_likely_stale_strong() -> None: +def test_verdict_content_equivalent_is_likely_stale_strong_when_consumed() -> None: bead = _bead("polylogue-a") ev = Evidence(commit_candidates=["abc1234ff"]) - v = verdict_for_bead(bead, ev, {"abc1234ff": "content-equivalent"}, {}) + v = verdict_for_bead(bead, ev, {"abc1234ff": "content-equivalent"}, {}, commit_consumer={"abc1234ff": True}) assert v.verdict == "LIKELY-STALE" assert v.confidence == "strong" +# --------------------------------------------------------------------------- +# fix 1 (2026-07-31): live-consumer check -- landed without a live caller +# must NOT be LIKELY-STALE. Measured against 114 human-verified beads: ~95% +# of "cited commit exists on master" verdicts were false positives, and the +# dominant cause was exactly this -- shipped code with zero production +# callers (e.g. polylogue-rxdo.9.6's blind_items(), polylogue-yp0's EventBus). +# --------------------------------------------------------------------------- + + +def test_verdict_landed_but_unconsumed_commit_is_undetermined_not_stale() -> None: + bead = _bead("polylogue-a") + ev = Evidence(commit_candidates=["abc1234ff"]) + v = verdict_for_bead(bead, ev, {"abc1234ff": "empty-diff"}, {}, commit_consumer={"abc1234ff": False}) + assert v.verdict == "UNDETERMINED" + assert any("no live production consumer" in r.lower() for r in v.reasons) + + +def test_verdict_landed_but_consumer_unknown_is_undetermined_not_stale() -> None: + # No symbols extracted (e.g. a non-Python change) -- inconclusive, and + # inconclusive must never default to LIKELY-STALE. + bead = _bead("polylogue-a") + ev = Evidence(commit_candidates=["abc1234ff"]) + v = verdict_for_bead(bead, ev, {"abc1234ff": "already-on-master"}, {}, commit_consumer={"abc1234ff": None}) + assert v.verdict == "UNDETERMINED" + assert any("could not be checked" in r for r in v.reasons) + + +def test_verdict_missing_commit_consumer_entry_defaults_to_unknown_not_stale() -> None: + # If the caller forgets to populate commit_consumer for a landed commit, + # the safe default is "unknown", not "assume consumed". + bead = _bead("polylogue-a") + ev = Evidence(commit_candidates=["abc1234ff"]) + v = verdict_for_bead(bead, ev, {"abc1234ff": "empty-diff"}, {}, commit_consumer={}) + assert v.verdict == "UNDETERMINED" + + +# --------------------------------------------------------------------------- +# fix 2 (2026-07-31): open parent-child dependents suppress a stale verdict +# --------------------------------------------------------------------------- + + +def test_verdict_suppressed_by_open_parent_child_dependents() -> None: + bead = _bead("polylogue-epic") + ev = Evidence(commit_candidates=["abc1234ff"]) + open_dependents = [{"id": "polylogue-epic.1", "status": "open"}, {"id": "polylogue-epic.2", "status": "open"}] + v = verdict_for_bead( + bead, + ev, + {"abc1234ff": "empty-diff"}, + {}, + commit_consumer={"abc1234ff": True}, + open_dependents=open_dependents, + ) + assert v.verdict == "UNDETERMINED" + assert any("open parent-child dependent" in r for r in v.reasons) + + +def test_verdict_open_dependents_do_not_suppress_a_live_verdict() -> None: + # The downgrade only applies to LIKELY-STALE -- it must not mask genuine + # LIKELY-LIVE evidence. + bead = _bead("polylogue-epic") + ev = Evidence(commit_candidates=["abc1234ff"]) + open_dependents = [{"id": "polylogue-epic.1", "status": "open"}] + v = verdict_for_bead(bead, ev, {"abc1234ff": "non-empty-diff"}, {}, open_dependents=open_dependents) + assert v.verdict == "LIKELY-LIVE" + + +def test_build_open_parent_child_dependents_index() -> None: + beads = [ + _bead("polylogue-parent"), + { + **_bead("polylogue-parent.1"), + "status": "open", + "dependencies": [{"depends_on_id": "polylogue-parent", "type": "parent-child"}], + }, + { + **_bead("polylogue-parent.2"), + "status": "closed", + "dependencies": [{"depends_on_id": "polylogue-parent", "type": "parent-child"}], + }, + { + **_bead("polylogue-parent.3"), + "status": "open", + "dependencies": [{"depends_on_id": "polylogue-parent", "type": "related"}], + }, + ] + index = build_open_parent_child_dependents_index(beads) + assert [d["id"] for d in index["polylogue-parent"]] == ["polylogue-parent.1"] + + +# --------------------------------------------------------------------------- +# fix 3 (2026-07-31): the bead's own words suppress a stale verdict +# --------------------------------------------------------------------------- + + +def test_verdict_suppressed_by_bead_own_not_done_note() -> None: + bead = _bead( + "polylogue-a", + notes="Landed the core in commit abc1234ff. AC4 is NOT satisfied: the DSL still lacks float literals.", + ) + ev = Evidence(commit_candidates=["abc1234ff"]) + v = verdict_for_bead( + bead, + ev, + {"abc1234ff": "empty-diff"}, + {}, + commit_consumer={"abc1234ff": True}, + suppression_signal=find_suppression_signal(bead), + ) + assert v.verdict == "UNDETERMINED" + assert any("contradicts a stale verdict" in r for r in v.reasons) + + +def test_find_suppression_signal_detects_common_phrasings() -> None: + assert ( + find_suppression_signal(_bead(notes="EventBus core landed. NOT wired to a live producer/consumer.")) is not None + ) + assert find_suppression_signal(_bead(notes="14 SELECT methods / 503 accessors untouched, still open.")) is not None + assert find_suppression_signal(_bead(notes="Everything here is done and fully wired end to end.")) is None + + +def test_find_suppression_signal_catches_deferred_and_xfail() -> None: + # Confirmed false positive against polylogue-hg97: the PR merged, but the + # bead's own text says the follow-on work was "Explicitly deferred this + # session" and the regression test is "Marked xfail" pending it. + bead = _bead( + notes=( + "Explicitly deferred this session (2026-07-18) per operator scoping choice. " + "Marked xfail (strict=False, reason references this bead) rather than deleted." + ) + ) + assert find_suppression_signal(bead) is not None + + def test_verdict_non_empty_diff_is_likely_live_strong() -> None: bead = _bead("polylogue-a") ev = Evidence(commit_candidates=["abc1234ff"]) @@ -243,7 +380,13 @@ def test_verdict_pr_not_found_is_undetermined_not_guessed() -> None: def test_verdict_mixed_empty_and_non_empty_notes_partial_landing() -> None: bead = _bead("polylogue-a") ev = Evidence(commit_candidates=["aaa1111ff", "bbb2222ff"]) - v = verdict_for_bead(bead, ev, {"aaa1111ff": "empty-diff", "bbb2222ff": "non-empty-diff"}, {}) + v = verdict_for_bead( + bead, + ev, + {"aaa1111ff": "empty-diff", "bbb2222ff": "non-empty-diff"}, + {}, + commit_consumer={"aaa1111ff": True}, + ) assert v.verdict == "LIKELY-STALE" assert any("partial" in r.lower() for r in v.reasons) @@ -343,6 +486,105 @@ def test_commit_checker_rejects_worktree_dir_equal_to_repo_root(tmp_path: Path) CommitChecker(repo, repo, base_ref="master") +# --------------------------------------------------------------------------- +# fix 1 (2026-07-31): live-consumer check against a real throwaway repo +# --------------------------------------------------------------------------- + + +def test_added_symbols_extracts_new_function_and_class_names(tmp_path: Path) -> None: + repo = _make_repo(tmp_path / "repo") + (repo / "module.py").write_text("def existing():\n pass\n") + _run_git(["add", "module.py"], cwd=repo) + _run_git(["commit", "-m", "add module"], cwd=repo) + + (repo / "module.py").write_text( + "def existing():\n pass\n\n\ndef new_helper():\n pass\n\n\nclass NewThing:\n pass\n" + ) + _run_git(["add", "module.py"], cwd=repo) + _run_git(["commit", "-m", "add new_helper and NewThing"], cwd=repo) + sha = _run_git(["rev-parse", "HEAD"], cwd=repo).stdout.strip() + + checker = CommitChecker(repo, tmp_path / "wt", base_ref="master") + symbols = checker.added_symbols(sha) + assert "new_helper" in symbols + assert "NewThing" in symbols + assert "existing" not in symbols # only NEW definitions, not pre-existing ones + + +def test_has_live_consumer_true_when_symbol_used_elsewhere(tmp_path: Path) -> None: + repo = _make_repo(tmp_path / "repo") + (repo / "lib.py").write_text("def blind_items():\n return []\n") + _run_git(["add", "lib.py"], cwd=repo) + _run_git(["commit", "-m", "add lib"], cwd=repo) + sha = _run_git(["rev-parse", "HEAD"], cwd=repo).stdout.strip() + + # The caller lands in a LATER, separate commit -- has_live_consumer must + # find it via base_ref, not just the cited commit's own touched files. + (repo / "caller.py").write_text("from lib import blind_items\n\nblind_items()\n") + _run_git(["add", "caller.py"], cwd=repo) + _run_git(["commit", "-m", "wire up the real caller"], cwd=repo) + + checker = CommitChecker(repo, tmp_path / "wt", base_ref="master") + assert checker.has_live_consumer(sha, ["blind_items"]) is True + + +def test_has_live_consumer_false_when_symbol_unreferenced(tmp_path: Path) -> None: + # Models polylogue-rxdo.9.6: blind_items() shipped, nothing outside its + # own file (and no test) calls it. + repo = _make_repo(tmp_path / "repo") + (repo / "lib.py").write_text("def blind_items():\n return []\n") + _run_git(["add", "lib.py"], cwd=repo) + _run_git(["commit", "-m", "add lib, unconsumed"], cwd=repo) + sha = _run_git(["rev-parse", "HEAD"], cwd=repo).stdout.strip() + + checker = CommitChecker(repo, tmp_path / "wt", base_ref="master") + assert checker.has_live_consumer(sha, ["blind_items"]) is False + + +def test_has_live_consumer_ignores_test_only_references(tmp_path: Path) -> None: + repo = _make_repo(tmp_path / "repo") + (repo / "lib.py").write_text("def blind_items():\n return []\n") + (repo / "tests").mkdir() + (repo / "tests" / "test_lib.py").write_text("from lib import blind_items\n\ndef test_x():\n blind_items()\n") + _run_git(["add", "lib.py", "tests/test_lib.py"], cwd=repo) + _run_git(["commit", "-m", "add lib, only referenced from its own test"], cwd=repo) + sha = _run_git(["rev-parse", "HEAD"], cwd=repo).stdout.strip() + + checker = CommitChecker(repo, tmp_path / "wt", base_ref="master") + # lib.py and tests/test_lib.py were BOTH touched by this commit, so the + # only reference is inside the commit's own touched files -- no external + # production consumer. + assert checker.has_live_consumer(sha, ["blind_items"]) is False + + +def test_has_live_consumer_none_when_no_symbols(tmp_path: Path) -> None: + # No extractable symbols (e.g. a non-Python change) is inconclusive, not + # a "no consumer" verdict -- the two must stay distinguishable so the + # verdict layer can report them with different reasons. + repo = _make_repo(tmp_path / "repo") + checker = CommitChecker(repo, tmp_path / "wt", base_ref="master") + assert checker.has_live_consumer("deadbeef", []) is None + + +def test_consumer_check_is_cached(tmp_path: Path) -> None: + repo = _make_repo(tmp_path / "repo") + (repo / "lib.py").write_text("def blind_items():\n return []\n") + _run_git(["add", "lib.py"], cwd=repo) + _run_git(["commit", "-m", "add lib"], cwd=repo) + sha = _run_git(["rev-parse", "HEAD"], cwd=repo).stdout.strip() + + (repo / "caller.py").write_text("from lib import blind_items\n\nblind_items()\n") + _run_git(["add", "caller.py"], cwd=repo) + _run_git(["commit", "-m", "wire up the real caller"], cwd=repo) + + checker = CommitChecker(repo, tmp_path / "wt", base_ref="master") + first = checker.consumer_check(sha) + assert first is True + assert sha in checker._consumer_cache + # Second call must reuse the cache rather than re-run git show/grep. + assert checker.consumer_check(sha) is True + + # --------------------------------------------------------------------------- # worktree removal safety # --------------------------------------------------------------------------- @@ -457,3 +699,105 @@ def test_load_beads_from_jsonl(tmp_path: Path) -> None: p.write_text('{"id": "a"}\n\n{"id": "b"}\n') beads = load_beads_from_jsonl(p) assert [b["id"] for b in beads] == ["a", "b"] + + +# --------------------------------------------------------------------------- +# Labelled-evaluation regression: five independent human reviewers checked +# all 190 LIKELY-STALE verdicts from the pre-fix sweep against 114 real +# beads (2026-07-31) and found ~95% were false positives. These tests pin +# the fixes against the ACTUAL live archive and repo history so precision +# cannot silently regress -- no synthetic fixtures, real .beads/issues.jsonl +# and real git objects. Network-free: PR merge state is not queried here, so +# only the commit-consumer and text-based checks are exercised. +# --------------------------------------------------------------------------- + + +def _test_worktree_dir(repo_root: Path) -> Path: + # Worker-scoped so `-n auto` distribution can't race two xdist workers + # onto the same throwaway worktree path. + worker = os.environ.get("PYTEST_XDIST_WORKER", "main") + return repo_root / ".cache" / "bead-landing-check" / f"test-worktree-{worker}" + + +def _repo_root() -> Path: + result = subprocess.run(["git", "rev-parse", "--show-toplevel"], capture_output=True, text=True, check=True) + return Path(result.stdout.strip()) + + +@pytest.fixture(scope="module", autouse=True) +def _cleanup_live_repo_test_worktree() -> Any: + """These regression tests exercise CommitChecker against the real repo + (not tmp_path) so they see real production evidence -- but that means + they create a real, gitignored throwaway worktree under the checkout's + own .cache/. Remove it after this module's tests finish so it doesn't + linger as a stray artifact (same safety contract as --remove-worktree: + only ever removes a path this fixture created, via remove_worktree()). + """ + yield + repo_root = _repo_root() + test_worktree = _test_worktree_dir(repo_root) + if test_worktree.exists(): + remove_worktree(repo_root, test_worktree) + + +def _live_bead(bead_id: str) -> dict[str, Any] | None: + repo_root = _repo_root() + beads_file = repo_root / ".beads" / "issues.jsonl" + if not beads_file.exists(): + return None + for bead in load_beads_from_jsonl(beads_file): + if bead["id"] == bead_id: + return bead + return None + + +@pytest.mark.parametrize( + "bead_id", + [ + "polylogue-rxdo.9.6", # blind_items() has zero callers outside its own test + "polylogue-rxdo.6", # ReferenceQueryPipeline has zero CLI/MCP/daemon references, still hard-errors + "polylogue-rxdo.9.7", # ClaimWithControls has zero callers outside its own test + "polylogue-dcz5", # 3.14t live in prod, but daemon_parse_stage_split is still False + "polylogue-hg97", # cost_outlook absent from polylogue/mcp/, contract test still xfail + "polylogue-yp0", # EventBus core landed but notes say "NOT wired to a live producer/consumer" + ], +) +def test_known_false_positive_no_longer_verifies_as_strong_likely_stale(bead_id: str) -> None: + """Five independent human reviewers checked all 190 LIKELY-STALE verdicts + from the pre-fix sweep against 114 real beads (2026-07-31) and found + roughly 95% were false positives -- these six were named explicitly. Run + the REAL pipeline (evidence extraction, real git history for commit/ + consumer checks, real .beads/issues.jsonl for dependents/suppression) + against each one and require the fixed verdict logic to no longer call + it a strong-confidence LIKELY-STALE. PR-merge state is not queried here + (network-free); PR numbers this bead cites are treated as unverified, + which only ever pushes a verdict TOWARD UNDETERMINED, never masks a + regression back to LIKELY-STALE. + """ + bead = _live_bead(bead_id) + if bead is None or bead.get("status") not in ("open", "in_progress"): + pytest.skip(f"{bead_id} no longer open in the live backlog -- regression target moved on") + + repo_root = _repo_root() + beads_file = repo_root / ".beads" / "issues.jsonl" + all_beads = load_beads_from_jsonl(beads_file) + dependents_index = build_open_parent_child_dependents_index(all_beads) + + evidence = extract_evidence(bead) + checker = CommitChecker(repo_root, _test_worktree_dir(repo_root)) + commit_results = {c: checker.check(c) for c in evidence.commit_candidates} + landed_states = {"empty-diff", "already-on-master", "content-equivalent"} + commit_consumer = {c: checker.consumer_check(c) for c, s in commit_results.items() if s in landed_states} + + verdict = verdict_for_bead( + bead, + evidence, + commit_results, + {}, # PR state unverified (network-free) -- can only push toward UNDETERMINED, never mask a regression + commit_consumer=commit_consumer, + open_dependents=dependents_index.get(bead_id, []), + suppression_signal=find_suppression_signal(bead), + ) + assert verdict.verdict != "LIKELY-STALE" or verdict.confidence != "strong", ( + f"{bead_id} regressed back to a strong-confidence LIKELY-STALE verdict: {verdict.reasons}" + )