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 <noreply@anthropic.com> Assistant: claude-code Assistant-Model: opus Assistant-Process: 692292@bnt-lap001 Assistant-Session: 89c7d8e8-e1b3-47eb-9411-a0ab227fca2f
This commit is contained in:
parent
a0083ab78c
commit
150cb0b201
6 changed files with 284 additions and 1 deletions
|
|
@ -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.
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
|
|
|||
|
|
@ -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,
|
||||
|
|
|
|||
155
tests/test_c25_source_revision.py
Normal file
155
tests/test_c25_source_revision.py
Normal file
|
|
@ -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")
|
||||
|
|
@ -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)
|
||||
|
|
|
|||
|
|
@ -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.
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue