From 150cb0b201ef68e86f508bcaac9e440cc905b27d Mon Sep 17 00:00:00 2001 From: tegwick Date: Sun, 4 Oct 2026 13:33:24 +0200 Subject: [PATCH] Bind C-25 JSON to the scanned source commit (STATE-WP-0094-T03) The checker JSON reports per repository repo_slug, source_revision (full 40-hex HEAD SHA) and source_revision_reason; C-25 findings repeat them so a findings-only consumer can bind. A dirty workplans/ tree, a non-git path, a missing HEAD or a tree changing mid-scan yields null plus a reason, never HEAD or a branch name. plan_owner_returns validates and passes the revision into every candidate and refuses findings bound elsewhere. Co-Authored-By: Claude Opus 5.5 Assistant: claude-code Assistant-Model: opus Assistant-Process: 692292@bnt-lap001 Assistant-Session: 89c7d8e8-e1b3-47eb-9411-a0ab227fca2f --- docs/owner-return-handoff.md | 16 ++ scripts/consistency_check.py | 55 +++++++ scripts/owner_return_handoff.py | 12 +- tests/test_c25_source_revision.py | 155 ++++++++++++++++++ tests/test_owner_return_handoff.py | 37 +++++ ...-WP-0094-routed-gates-and-owner-returns.md | 10 ++ 6 files changed, 284 insertions(+), 1 deletion(-) create mode 100644 tests/test_c25_source_revision.py diff --git a/docs/owner-return-handoff.md b/docs/owner-return-handoff.md index 22612ba..6d2932b 100644 --- a/docs/owner-return-handoff.md +++ b/docs/owner-return-handoff.md @@ -105,3 +105,19 @@ a comment or receipt does not promote it. Terminal predecessors/resolved non-human decisions and elapsed windows can leave an owner-return wait `reviewable`; that disposition permits review preparation only and never clears the wait marker, task status, human gate or any other field. + +## Scanned source revision + +The checker JSON (`--json`) states the commit it scanned, per repository: +`repo_slug`, `source_revision` (full 40-hex lowercase HEAD SHA) and +`source_revision_reason`. Every C-25 finding repeats the same three fields so a +consumer of the findings alone can bind. The revision is claimed only when +`workplans/` has no modified, staged, untracked or ignored change and the tree +did not move while the files were read and C-25 ran; otherwise +`source_revision` is `null` with a reason (`not_a_git_tree`, `no_head_commit`, +`unsupported_object_format`, `dirty_workplans`, `git_status_failed`, +`changed_during_scan`, `not_scanned`). It is never a branch name or a guess. +`plan_owner_returns` validates it (40-hex or null), refuses findings bound to a +different repository/revision and copies it into every candidate; the +deduplication event identity stays revision-independent. Activity Core's +eligibility builder compares it to its `scan_revision`, so `null` refuses. diff --git a/scripts/consistency_check.py b/scripts/consistency_check.py index 11b6bc6..ee080e7 100644 --- a/scripts/consistency_check.py +++ b/scripts/consistency_check.py @@ -312,6 +312,10 @@ class ConsistencyReport: repo_path: str issues: list[Issue] = field(default_factory=list) fixes_applied: list[str] = field(default_factory=list) + # Commit the workplans/ scan read (STATE-WP-0094): a full 40-hex SHA, or + # None with a machine-readable reason. Never a branch name or a guess. + source_revision: str | None = None + source_revision_reason: str | None = "not_scanned" def add(self, **kwargs) -> Issue: issue = Issue(**kwargs) @@ -1465,6 +1469,9 @@ def check_repo( _check_orphan_db(api_base, repo_id, set(), report) return report + # Bind the scan to the commit it reads; re-verified after C-25 below. + report.source_revision, report.source_revision_reason = scan_source_revision(repo_dir) + # Parse workplan files workplan_infos: list[tuple[Path, dict, str]] = [] file_ws_ids: dict[str, tuple[Path, dict, str]] = {} # ws_id → (file, meta, body) @@ -2429,6 +2436,10 @@ def check_repo( _check_ghost_duplicates(api_base, workplan_infos, file_ws_ids, report) _check_blocked_unblock_sweep(api_base, repo_slug, repo_dir, workplan_infos, report) + if report.source_revision is not None: + # The tree must not have moved while the files were read and C-25 ran. + if scan_source_revision(repo_dir) != (report.source_revision, None): + report.source_revision, report.source_revision_reason = None, "changed_during_scan" _check_inbox_hygiene(api_base, repo_slug, report) _check_workplan_prefixes(repo_dir, repo_slug, workplan_infos, report) _check_workplan_id_collisions(api_base, repo_slug, repo_dir, workplan_infos, report) @@ -2786,6 +2797,44 @@ def collect_inbox_hygiene(api_base: str, repo_slug: str) -> dict[str, Any]: } +_FULL_SHA = re.compile(r"^[0-9a-f]{40}$") + + +def scan_source_revision(repo_dir: Path) -> tuple[str | None, str | None]: + """Return ``(sha, None)`` for a clean ``workplans/`` tree, else ``(None, reason)``. + + ``sha`` is the full 40-hex HEAD commit. Any change under ``workplans/`` + (modified, staged, untracked or ignored) means the scanned files are not + that commit, so no revision is claimed. Reasons: ``not_a_git_tree``, + ``no_head_commit``, ``unsupported_object_format``, ``dirty_workplans``, + ``git_status_failed``. + """ + def git(*args: str) -> str | None: + try: + return subprocess.run( + ["git", *args], cwd=repo_dir, capture_output=True, text=True, + timeout=30, check=True, + ).stdout + except (subprocess.CalledProcessError, subprocess.TimeoutExpired, OSError): + return None + + if not Path(repo_dir).is_dir() or git("rev-parse", "--is-inside-work-tree") is None: + return None, "not_a_git_tree" + head = git("rev-parse", "--verify", "--quiet", "HEAD^{commit}") + if head is None or not head.strip(): + return None, "no_head_commit" + sha = head.strip() + if not _FULL_SHA.fullmatch(sha): + return None, "unsupported_object_format" + status = git("status", "--porcelain", "--untracked-files=all", "--ignored", + "--", "workplans") + if status is None: + return None, "git_status_failed" + if status.strip(): + return None, "dirty_workplans" + return sha, None + + C25_PAGE_SIZE = 200 C25_MAX_PAGES = 50 @@ -4360,9 +4409,14 @@ def render_text(report: ConsistencyReport, show_info: bool = True) -> str: def report_to_dict(report: ConsistencyReport) -> dict: + revision = { + "source_revision": report.source_revision, + "source_revision_reason": report.source_revision_reason, + } return { "repo_slug": report.repo_slug, "repo_path": report.repo_path, + **revision, "issues": [ { "severity": i.severity, @@ -4373,6 +4427,7 @@ def report_to_dict(report: ConsistencyReport) -> dict: "file_value": i.file_value, "db_value": i.db_value, "fixable": i.fixable, + **({"repo_slug": report.repo_slug, **revision} if i.check_id == "C-25" else {}), **({"owner_return": i._fix_context} if i.check_id == "C-25" and i._fix_context.get("task_record_id") else {}), } for i in report.issues diff --git a/scripts/owner_return_handoff.py b/scripts/owner_return_handoff.py index 1abea90..a610798 100644 --- a/scripts/owner_return_handoff.py +++ b/scripts/owner_return_handoff.py @@ -15,6 +15,7 @@ from datetime import date _REPO = re.compile(r"^[a-z0-9][a-z0-9-]*$") _TASK = re.compile(r"^[A-Za-z0-9][A-Za-z0-9._-]*$") _STATES = {"open", "claimed", "succeeded", "failed", "expired"} +_SHA = re.compile(r"^[0-9a-f]{40}$") def _uuid(value: object) -> str: @@ -91,6 +92,12 @@ def plan_owner_returns( raise ValueError("invalid repository identity") if report.get("result") == "error": raise ValueError("cannot plan from failed checker collection") + # Passed through unchanged so candidates stay bound to the scanned commit; + # None (unknown/dirty scan) travels as None and the eligibility builder + # refuses it. Never substituted or guessed here. + revision = report.get("source_revision") + if revision is not None and (not isinstance(revision, str) or not _SHA.fullmatch(revision)): + raise ValueError("source_revision must be a full 40-hex commit SHA or null") issues = report.get("issues") if not isinstance(issues, list): raise ValueError("checker issues must be a list") @@ -100,6 +107,8 @@ def plan_owner_returns( raise ValueError("invalid checker issue") if issue.get("check_id") != "C-25" or "owner_return" not in issue: continue + if issue.get("repo_slug", repo) != repo or issue.get("source_revision", revision) != revision: + raise ValueError("C-25 finding is bound to a different repository or revision") context = issue["owner_return"] if not isinstance(context, Mapping): raise ValueError("invalid owner-return context") @@ -118,7 +127,8 @@ def plan_owner_returns( encoded = json.dumps([repo, task, message], separators=(",", ":")).encode() event = "owner-return:" + hashlib.sha256(encoded).hexdigest() candidates[event] = { - "target_repo": repo, "task_record_id": task, "message_id": message, + "target_repo": repo, "source_revision": revision, + "task_record_id": task, "message_id": message, "activity_definition_id": definition, "triggering_event_id": event, "disposition": "stopped" if stop else "reviewable", "stop_reason": stop, "ops_run_id": None, diff --git a/tests/test_c25_source_revision.py b/tests/test_c25_source_revision.py new file mode 100644 index 0000000..8bfa29d --- /dev/null +++ b/tests/test_c25_source_revision.py @@ -0,0 +1,155 @@ +"""C-25 JSON binds to the scanned source commit (STATE-WP-0094).""" +from __future__ import annotations + +import subprocess +import sys +from pathlib import Path + +sys.path.insert(0, str(Path(__file__).parent.parent / "scripts")) + +from consistency_check import ( # noqa: E402 + ConsistencyReport, report_to_dict, scan_source_revision, +) + + +def _git(repo: Path, *args: str) -> str: + return subprocess.run( + ["git", "-c", "user.name=t", "-c", "user.email=t@example.invalid", *args], + cwd=repo, check=True, capture_output=True, text=True, + ).stdout.strip() + + +def _repo(tmp_path: Path) -> Path: + repo = tmp_path / "repo" + (repo / "workplans").mkdir(parents=True) + (repo / "workplans" / "DEMO-WP-0001-x.md").write_text("---\nid: DEMO-WP-0001\n---\n") + (repo / "README.md").write_text("demo\n") + _git(repo, "init", "-q") + _git(repo, "add", ".") + _git(repo, "commit", "-qm", "init") + return repo + + +def test_clean_tree_reports_full_head_sha(tmp_path): + repo = _repo(tmp_path) + sha, reason = scan_source_revision(repo) + assert sha == _git(repo, "rev-parse", "HEAD") + assert len(sha) == 40 and sha == sha.lower() + assert reason is None + + +def test_dirt_outside_workplans_does_not_unbind(tmp_path): + repo = _repo(tmp_path) + (repo / "README.md").write_text("changed\n") + (repo / "scratch.txt").write_text("x\n") + assert scan_source_revision(repo) == (_git(repo, "rev-parse", "HEAD"), None) + + +def test_modified_staged_or_untracked_workplans_are_not_head(tmp_path): + repo = _repo(tmp_path) + plan = repo / "workplans" / "DEMO-WP-0001-x.md" + plan.write_text("---\nid: DEMO-WP-0001\nstatus: active\n---\n") + assert scan_source_revision(repo) == (None, "dirty_workplans") + _git(repo, "add", ".") + assert scan_source_revision(repo) == (None, "dirty_workplans") + _git(repo, "commit", "-qm", "two") + assert scan_source_revision(repo)[1] is None + (repo / "workplans" / "archived").mkdir() + (repo / "workplans" / "archived" / "new.md").write_text("---\n---\n") + assert scan_source_revision(repo) == (None, "dirty_workplans") + + +def test_ignored_workplan_file_is_not_head(tmp_path): + repo = _repo(tmp_path) + (repo / ".gitignore").write_text("*.local.md\n") + _git(repo, "add", ".") + _git(repo, "commit", "-qm", "ignore") + (repo / "workplans" / "secret.local.md").write_text("---\n---\n") + assert scan_source_revision(repo) == (None, "dirty_workplans") + + +def test_non_git_path_and_missing_head(tmp_path): + plain = tmp_path / "plain" + (plain / "workplans").mkdir(parents=True) + assert scan_source_revision(plain) == (None, "not_a_git_tree") + assert scan_source_revision(tmp_path / "absent") == (None, "not_a_git_tree") + empty = tmp_path / "empty" + empty.mkdir() + _git(empty, "init", "-q") + assert scan_source_revision(empty) == (None, "no_head_commit") + + +def _report(sha, reason): + report = ConsistencyReport(repo_slug="demo", repo_path="/x", + source_revision=sha, source_revision_reason=reason) + report.add(severity="WARN", check_id="C-25", message="m", file_path="workplans/p.md#T01", + file_value="message-from:provider", + _fix_context={"task_record_id": "DEMO-WP-0001-T01", "message_ids": ["a"]}) + report.add(severity="WARN", check_id="C-25", message="legacy", file_value="x") + report.add(severity="WARN", check_id="C-24", message="other") + return report_to_dict(report) + + +def test_json_report_and_c25_findings_carry_revision(): + sha = "0123456789abcdef0123456789abcdef01234567" + data = _report(sha, None) + assert (data["repo_slug"], data["source_revision"], data["source_revision_reason"]) == ("demo", sha, None) + c25 = [i for i in data["issues"] if i["check_id"] == "C-25"] + assert len(c25) == 2 + for issue in c25: + assert (issue["repo_slug"], issue["source_revision"], issue["source_revision_reason"]) == ("demo", sha, None) + other = next(i for i in data["issues"] if i["check_id"] == "C-24") + assert "source_revision" not in other and "repo_slug" not in other + + +def test_json_unknown_revision_is_null_with_reason(): + data = _report(None, "dirty_workplans") + assert data["source_revision"] is None + assert data["source_revision_reason"] == "dirty_workplans" + assert all(i["source_revision"] is None for i in data["issues"] if i["check_id"] == "C-25") + + +def test_unscanned_report_defaults_to_null(): + data = report_to_dict(ConsistencyReport(repo_slug="demo", repo_path="(unknown)")) + assert (data["source_revision"], data["source_revision_reason"]) == (None, "not_scanned") + + +def _check(repo, monkeypatch, sweep=None): + import socket + + import consistency_check + + def fake_get(_api_base, path, params=None, **_kwargs): + if path == "/repos/demo": + return {"id": "repo-1", "slug": "demo", "local_path": str(repo), + "host_paths": {socket.gethostname(): str(repo)}, "domain_slug": "infotech"} + return [] + + monkeypatch.setattr("consistency_check.load_classification_file", lambda _d: ({}, [], [])) + monkeypatch.setattr("consistency_check._api_get", fake_get) + if sweep: + monkeypatch.setattr("consistency_check._check_blocked_unblock_sweep", sweep) + return consistency_check.check_repo("http://unused", "demo") + + +def test_check_repo_binds_clean_scan_to_head(tmp_path, monkeypatch): + repo = _repo(tmp_path) + report = _check(repo, monkeypatch) + assert (report.source_revision, report.source_revision_reason) == (_git(repo, "rev-parse", "HEAD"), None) + + +def test_check_repo_dirty_workplans_is_null(tmp_path, monkeypatch): + repo = _repo(tmp_path) + (repo / "workplans" / "DEMO-WP-0001-x.md").write_text("---\nid: DEMO-WP-0001\nstatus: active\n---\n") + report = _check(repo, monkeypatch) + assert (report.source_revision, report.source_revision_reason) == (None, "dirty_workplans") + + +def test_check_repo_tree_changed_during_scan_is_null(tmp_path, monkeypatch): + repo = _repo(tmp_path) + + def sweep(_api, _slug, repo_dir, _infos, _report): + (repo_dir / "workplans" / "late.md").write_text("---\n---\n") + + report = _check(repo, monkeypatch, sweep) + assert (report.source_revision, report.source_revision_reason) == (None, "changed_during_scan") diff --git a/tests/test_owner_return_handoff.py b/tests/test_owner_return_handoff.py index eb8e4c4..733a716 100644 --- a/tests/test_owner_return_handoff.py +++ b/tests/test_owner_return_handoff.py @@ -195,3 +195,40 @@ def test_resolved_decision_does_not_override_human_gate(): assert plan(report(), activity_definition_id=DEF, current_tasks=tasks)[0]["stop_reason"] == "human_gate" row["needs_human"] = False assert plan(report(), activity_definition_id=DEF, current_tasks=tasks)[0]["disposition"] == "reviewable" + + +SHA = "0123456789abcdef0123456789abcdef01234567" + + +def test_source_revision_passes_through_to_every_candidate(): + source = report() + source["source_revision"] = SHA + source["issues"][0].update(repo_slug="state-hub", source_revision=SHA) + rows = plan(source, activity_definition_id=DEF) + assert rows and all(row["source_revision"] == SHA for row in rows) + # Dedup identity is revision-independent, so a rescan does not re-trigger. + assert rows[0]["triggering_event_id"] == plan(report(), activity_definition_id=DEF)[0]["triggering_event_id"] + + +def test_unknown_revision_travels_as_null(): + source = report() + source["source_revision"] = None + assert plan(source, activity_definition_id=DEF)[0]["source_revision"] is None + assert plan(report(), activity_definition_id=DEF)[0]["source_revision"] is None + + +@pytest.mark.parametrize("bad", ["main", SHA[:12], SHA.upper(), 7]) +def test_non_sha_revision_is_refused(bad): + source = report() + source["source_revision"] = bad + with pytest.raises(ValueError, match="source_revision"): + plan(source, activity_definition_id=DEF) + + +@pytest.mark.parametrize("field,value", [("source_revision", "f" * 40), ("repo_slug", "other")]) +def test_finding_bound_elsewhere_is_refused(field, value): + source = report() + source["source_revision"] = SHA + source["issues"][0][field] = value + with pytest.raises(ValueError, match="different repository or revision"): + plan(source, activity_definition_id=DEF) diff --git a/workplans/STATE-WP-0094-routed-gates-and-owner-returns.md b/workplans/STATE-WP-0094-routed-gates-and-owner-returns.md index 73880fa..f0c43ae 100644 --- a/workplans/STATE-WP-0094-routed-gates-and-owner-returns.md +++ b/workplans/STATE-WP-0094-routed-gates-and-owner-returns.md @@ -244,3 +244,13 @@ Helm revision 72, main-5dd3df9. Open T03 source gaps before any trial: C-25 must report the scanned commit, and the pickup caller is not yet wired into the existing runner. Activity Core b2c6c56 aligns its freshness projection with the runner's results[] output. T03/T05 remain progress. + + +2026-10-04 T03 source gap closed (claude-state-hub): the checker JSON now +carries per-repository `repo_slug`, `source_revision` (full 40-hex HEAD SHA, or +null) and `source_revision_reason`; C-25 findings repeat them. A dirty +`workplans/` tree, a non-git path or a tree that moves mid-scan yields null with +a reason, never HEAD or a branch. `plan_owner_returns` passes the revision into +each candidate. Matches Activity Core `build_current_tasks` (`repo_slug`, +`source_revision`). See docs/owner-return-handoff.md. Pickup caller wiring +remains open; T03/T05 remain progress.