From c14c5f5fdbee91a1e279eae310840353ea57a1fd Mon Sep 17 00:00:00 2001 From: datacas <4690232+datacas@users.noreply.github.com> Date: Thu, 29 Oct 2026 18:10:21 +0100 Subject: [PATCH 1/3] feat: define verification plans and per-check results (#290) --- .github/ISSUE_TEMPLATE/work-item.md | 16 ++ docs/execution-plan.md | 43 +++- docs/verification.md | 61 +++++ scripts/execution_plan.py | 42 +++- scripts/native_prerequisites.py | 14 +- scripts/run_cycle.py | 88 ++++++- scripts/runtime.manifest | 1 + scripts/verification_plan.py | 359 ++++++++++++++++++++++++++++ scripts/windows-test-shards.json | 1 + skills/cc-implement-issue/SKILL.md | 30 +++ skills/cc-rereview/SKILL.md | 30 +++ skills/cc-resolve-comments/SKILL.md | 30 +++ skills/cc-verify/SKILL.md | 30 +++ tests/test_execution_plan.py | 10 +- tests/test_run_cycle.py | 4 + tests/test_stage_dispatch.py | 11 +- tests/test_verification_plan.py | 267 +++++++++++++++++++++ 17 files changed, 1014 insertions(+), 23 deletions(-) create mode 100644 .github/ISSUE_TEMPLATE/work-item.md create mode 100644 scripts/verification_plan.py create mode 100644 tests/test_verification_plan.py diff --git a/.github/ISSUE_TEMPLATE/work-item.md b/.github/ISSUE_TEMPLATE/work-item.md new file mode 100644 index 0000000..7cdee48 --- /dev/null +++ b/.github/ISSUE_TEMPLATE/work-item.md @@ -0,0 +1,16 @@ +--- +name: Work item +about: Specify a change with executable acceptance criteria +--- + +## Problem + +Describe the observable problem and the intended outcome. + +## Acceptance criteria + +- Replace this item with an observable acceptance criterion. + +## Verification + +Describe the commands or boundary checks that can prove the criteria. diff --git a/docs/execution-plan.md b/docs/execution-plan.md index 9a83be5..c1d95c4 100644 --- a/docs/execution-plan.md +++ b/docs/execution-plan.md @@ -13,10 +13,10 @@ views `cycle.ROLE_CONTRACTS`, `run_cycle.SKILL_FOR_ROLE`, and keys and values. In particular, `SKILL_FOR_ROLE` remains the five-role adapter used by existing callers; catalog entries for other skills do not expand it. -## ExecutionPlan version 2 +## ExecutionPlan version 3 `ExecutionPlan` serializes the plan contract without copying the existing -findings, verification checks, code-host checks, or evidence contracts. Its +findings or code-host checks. Its identity includes `plan_id`, `revision`, repository, and work-item ID. It also stores the objective, execution profile, effective configuration and policy, plan provenance, review metadata, base/head references, and a metadata extension @@ -73,7 +73,7 @@ rereview decision is one rule for every profile invalidate the review's evidence, and open findings without fixes keep their blocks rather than being reviewed again. -**Version negotiation.** `from_dict()` reads versions 1 and 2 and rejects every +**Version negotiation.** `from_dict()` reads versions 1, 2 and 3 and rejects every other value, including `"1"` and `true`; a version 1 payload is upgraded, never reinterpreted, and may not carry version 2 fields. `to_dict(contract_version=1)` writes a lossless projection for a version 1 reader: it refuses while any @@ -83,6 +83,43 @@ obligations, controls, and stage policies under envelope is validated and its typed fields restored. A version 1 payload read and projected back is unchanged. +Version 3 (#290) adds `verification`, a `VerificationPlan` from +`scripts/verification_plan.py`. It belongs to the enclosing plan's `plan_id` +and `revision`; it has no parallel identity or store. Older plans upgrade with +`verification: null`. Versions 1 and 2 may still be written when verification +is absent; projecting a plan that carries verification to either version is +rejected instead of dropping obligations or history. + +The verification contract contains: + +- `criteria`: provider, content SHA-256, evaluation status, ordered criterion + texts and a reason. Runtime parsing assigns `AC-1`, `AC-2`, etc. within this + owning revision. Changed content requires an explicit revision; identifiers + from a previous revision must not be silently assigned new meanings. +- `checks`: stable ID, exact command, executor, profile, execution context, + required flag, criterion IDs, command level, provenance and activation reason. + The configured test command is `configured-test` and required. Every check + linked to a criterion is required. An agent can add checks and promote an + obligation; it cannot remove or demote an existing one. +- `results`: append-only check ID, verified HEAD, `pass|fail|unverified`, + evidence, cause and attempts. Unexecuted results carry an explicit cause; + a null HEAD is permitted only for an unverified result whose target is unknown. + Executed evidence requires integer exit codes and nonnegative counts or null. + +`extend_verification()` returns a new owning revision and preserves result +history. `VerificationPlan.extend()` freezes command, executor, profile, +context and provenance, allows additions/promotions, and retains omitted +entries. `record()` appends results; `current()` invalidates an old HEAD without +deleting history. `cap()` never raises the reported conclusion: required fail +caps at `failed`; required unverified, unavailable criteria or an unlinked +criterion cap at `not_verified`. Code-host CI never substitutes for these +results in #290. + +The contract and in-cycle normalization are implemented here. Durable revision +and result persistence, execution of the plan, the end-of-cycle +`verification_unverified` gate, and verification-only resume belong to #329. +No provisional store or telemetry stop-reason schema is introduced by #290. + The contract reserves identity, profile, revision, provenance, effective configuration, policy, and stage activation/omission reasons for later analysis such as #127. `scripts/execution_profiles.py` builds the plan from the diff --git a/docs/verification.md b/docs/verification.md index 65cac1c..d000b0b 100644 --- a/docs/verification.md +++ b/docs/verification.md @@ -184,6 +184,67 @@ layers: This section decides whether output is complete. [Evidence levels](#evidence-levels) decides who produced it; the two don't overlap. +## Verification plan and tests version 2 + +New reports use `tests.version: 2` and `tests.checks`, replacing the legacy +top-level `tests.evidence` list. This is separate from the unchanged code-host +`checks.items` object. The enclosing result supplies `head_sha`. + +```json +{ + "version": 2, + "conclusion": "verified", + "boundary": "not_required", + "checks": [{ + "id": "configured-test", "command": "python3 -m unittest", + "executor": "codex", "profile": "deep_coder", "context": "project", + "required": true, "criteria": ["AC-1"], "level": "test", + "provenance": "project", "reason": "configured test command", + "head_sha": "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa", + "result": "pass", "cause": "", "attempts": [], + "evidence": [{"exit_code": 0, "executed": 14}] + }] +} +``` + +Freeze each check before execution. A result names the same check ID, command, +executor/profile and context. Required checks originate in project policy and +criterion links; an agent may promote, never demote. Record an unexecuted check +as `result: unverified`, with `cause`, evidence and actual recovery `attempts`; +do not invent an exit code. Results on another HEAD stay in history but cannot +prove the current change. An uncovered criterion remains open. Required failure +caps `tests.conclusion` at `failed`; required unverified or open criteria cap it +at `not_verified`. These caps never improve an agent conclusion. + +Before implementation, the runtime reads the GitHub work-item body once per +cycle and parses one explicit criteria section. Accepted headings are exactly +`Acceptance criteria` and `Criterios de aceptación`, case insensitive, as a +Markdown heading (`#` through `######`) or bold label. A trailing colon or +period is allowed inside a bold label or after it. The section is a flat list +at column zero using `-`, `+`, `*`, `1.` or `1)` markers and optional Markdown +checkboxes; another heading or bold label ends it. Empty, missing, duplicate, +placeholder or unparseable sections produce deterministic `needs_refinement` +before any implementation, even with issue review off or difficulty 1. They +are insufficient specification, never a failed test. Existing issues are not +migrated. `.github/ISSUE_TEMPLATE/work-item.md` supplies the grammar for new work. +IDs are `AC-n` in order, scoped to the execution-plan revision and content hash. +Plane and Jira return `not_evaluable` with reason `criteria reader unavailable`, +distinct from parsed criteria that remain unmet; neither counts as satisfied. + +For one release, omitted `tests.version` or version 1 accepts `tests.evidence`. +The compatibility interval starts with the first published release supporting +version 2 and ends at the next published release. With runtime obligations, +the adapter matches exact commands and preserves missing required checks as +explicit unverified results; absence never creates a pass. Legacy standalone +reports retain their historical evidence classification. Version 2 evidence +is normalized into a private compatibility view for `tests_basis`, +`boundary_verified`, telemetry and stats; this view is not a second result +contract. Malformed or unsupported reports cap at `not_verified`. + +Plan persistence, the final verification gate and verification-only resume are +deferred to #329; see [Execution plan](execution-plan.md). CI substitution is +out of scope: `CI_PASSED` does not turn a local unverified result into pass. + ## Evidence levels Every recorded test outcome has one `tests_basis` token. The functional diff --git a/scripts/execution_plan.py b/scripts/execution_plan.py index d25ed7a..e620d8d 100644 --- a/scripts/execution_plan.py +++ b/scripts/execution_plan.py @@ -9,17 +9,21 @@ from dataclasses import dataclass, field, replace from enum import Enum -from typing import Any, Mapping +from typing import Any, Mapping, TYPE_CHECKING from executors import WorkspacePolicy from issue_review import ROLE as ISSUE_REVIEW_ROLE, SKILL as ISSUE_REVIEW_SKILL +if TYPE_CHECKING: + from .verification_plan import VerificationPlan, VerificationError, Check +else: + from verification_plan import VerificationPlan, VerificationError, Check -CONTRACT_VERSION = 2 +CONTRACT_VERSION = 3 #: Versions `ExecutionPlan.from_dict` reads and `to_dict` can write. Version 1 #: is read by upgrading it and written only as a lossless projection. -SUPPORTED_CONTRACT_VERSIONS = (1, 2) +SUPPORTED_CONTRACT_VERSIONS = (1, 2, 3) class ContractError(ValueError): @@ -414,8 +418,16 @@ class ExecutionPlan: decisions: tuple[PendingDecision, ...] = () obligations: tuple[ConditionalObligation, ...] = () controls: tuple[PlannedControl, ...] = () + verification: VerificationPlan | None = None def validate(self) -> None: + if self.verification is not None: + if not isinstance(self.verification, VerificationPlan): + raise ContractError("verification must be a verification plan") + try: + self.verification.validate() + except VerificationError as exc: + raise ContractError(str(exc)) from exc if type(self.contract_version) is not int: raise ContractError("contract version must be an integer") if self.contract_version != CONTRACT_VERSION: @@ -661,6 +673,18 @@ def _validate_obligations(self, stages: dict[str, PlannedStage], # -- queries ------------------------------------------------------------ + def extend_verification(self, checks: tuple[Check, ...]) -> ExecutionPlan: + """A new owning-plan revision; never remove a frozen check or its history.""" + if self.verification is None: + raise ContractError("initialize verification with runtime criteria first") + try: + verification = self.verification.extend(checks) + except VerificationError as exc: + raise ContractError(str(exc)) from exc + result = replace(self, revision=self.revision + 1, verification=verification) + result.validate() + return result + def stage(self, stage_id: str) -> PlannedStage: for item in self.stages: if item.stage_id == stage_id: @@ -769,12 +793,16 @@ def to_dict(self, contract_version: int = CONTRACT_VERSION) -> dict[str, Any]: self.validate() if type(contract_version) is not int: raise ContractError(f"unsupported execution-plan contract version: {contract_version!r}") - if contract_version == CONTRACT_VERSION: + if contract_version in (2, CONTRACT_VERSION): + if contract_version == 2 and self.verification is not None: + raise ContractError("version 2 cannot represent verification; negotiate version 3") return { **self._common_dict(dict(self.effective_policy), contract_version), "decisions": [item.to_dict() for item in self.decisions], "obligations": [item.to_dict() for item in self.obligations], "controls": [item.to_dict() for item in self.controls], + **({"verification": self.verification.to_dict() if self.verification else None} + if contract_version >= 3 else {}), } if contract_version == 1: return self._project_v1() @@ -808,6 +836,8 @@ def _project_v1(self) -> dict[str, Any]: projection; settled obligations, controls, and stage policies travel under `effective_policy.contract_v2` rather than disappearing. """ + if self.verification is not None: + raise ContractError("version 1 cannot represent verification; negotiate version 3") pending = [f"decision for {item.subject}" for item in self.decisions] pending += [f"obligation {item.obligation_id}" for item in self.obligations if item.state is ObligationState.PENDING] @@ -840,6 +870,8 @@ def from_dict(cls, payload: Mapping[str, Any]) -> ExecutionPlan: version = payload["contract_version"] if type(version) is not int or version not in SUPPORTED_CONTRACT_VERSIONS: raise ContractError(f"unsupported execution-plan contract version: {version!r}") + if version < 3 and "verification" in payload: + raise ContractError("verification requires execution-plan version 3") if version == 1 and _V2_PLAN_KEYS & set(payload): raise ContractError("a version 1 plan cannot carry version 2 fields") if version == 1: @@ -961,6 +993,8 @@ def entries(name: str) -> list[Mapping[str, Any]]: decisions=decisions, obligations=obligations, controls=controls, + verification=(VerificationPlan.from_dict(payload["verification"]) + if payload.get("verification") is not None else None), ) plan.validate() except (KeyError, TypeError, ValueError, AttributeError, IndexError) as exc: diff --git a/scripts/native_prerequisites.py b/scripts/native_prerequisites.py index e6764e4..0045e7a 100644 --- a/scripts/native_prerequisites.py +++ b/scripts/native_prerequisites.py @@ -384,10 +384,16 @@ def static_result(spoken: str, commands, evidence: dict[str, Any]) -> str: return spoken if not isinstance(payload, dict): return spoken - payload["tests"] = {"ran": False, "conclusion": "not_verified", - "evidence": [{"command": command, "exit_code": None, - "executed": None, "reason": "native isolation unavailable"} - for command in commands]} + payload["tests"] = {"version": 2, "ran": False, "conclusion": "not_verified", + "checks": [{"id": f"native-{index}", "command": command, + "executor": "native", "profile": "selected", + "context": "native", "required": True, "criteria": [], + "level": "test", "provenance": "runtime", + "reason": "configured verification", + "head_sha": payload.get("head_sha"), "result": "unverified", + "evidence": [], "cause": "native isolation unavailable", + "attempts": []} + for index, command in enumerate(commands, 1)]} payload["native_sandbox_preflight"] = evidence return (spoken[:begin] + "ORCHESTRATION_RESULT\n" + json.dumps(payload) + "\n" + spoken[end:]) diff --git a/scripts/run_cycle.py b/scripts/run_cycle.py index 5881036..b0f83cf 100644 --- a/scripts/run_cycle.py +++ b/scripts/run_cycle.py @@ -58,7 +58,12 @@ from dataclasses import dataclass, field, replace from datetime import datetime, timezone from pathlib import Path, PureWindowsPath -from typing import Any +from typing import Any, TYPE_CHECKING + +if TYPE_CHECKING: + from . import verification_plan as verification_contract +else: + import verification_plan as verification_contract import ci_gate try: @@ -2450,6 +2455,7 @@ def run_cycle( repair_approval: str | None = None, published_change_request: Callable[[], str | None] | None = None, publication_auditor: PublicationAuditor | None = None, + criteria_reader: Callable[..., verification_contract.Criteria] | None = None, ) -> CycleReport: """[issue_review ->] implement -> review -> (resolve -> rereview)*, recorded. @@ -2603,6 +2609,8 @@ def progress_stage_started(role, decision) -> None: policy_host = code_host or policy_code_host policy_provider = work_item_provider or ( "github" if policy_host == "github" and str(task_id).isdecimal() else None) + verification_plan: verification_contract.VerificationPlan | None = None + criteria_context = "" def stage_policy(role: str, request: str | None) -> StagePolicy: """The closed command table this stage receives (#142).""" @@ -2881,7 +2889,7 @@ def run(role: str, instruction: str = "", *, journal_context = (journal.summary() if redispatched or journal.has_history() else "") outcome = recorder.stage( - role, compose(role, repo_id, task_id, instruction, + role, compose(role, repo_id, task_id, instruction + criteria_context, change_request_id=change_request_id, local_only=local_only, evidence=evidence, worktree_dir=worktree_dir, @@ -3222,7 +3230,7 @@ def advance(role: str, instruction: str = "", *, missing from the fourth. """ nonlocal pending_recovery, previous_resolve_target, previous_resolve_used_fallback - nonlocal review_attempts + nonlocal review_attempts, verification_plan if pending_recovery is not None: ids = [item["id"] for item in pending_recovery.get("finding_outcomes", []) if item["status"] in {"resolved", "not_applicable"}] @@ -3373,6 +3381,26 @@ def advance(role: str, instruction: str = "", *, # agent said it did; the row already says the call returned. The prose # beside it is not recorded: the store holds references and counts. if reported.status: + test_payload = reported.payload + if (verification_plan is None and allowed_test_command + and role in {"implement", "resolve", "rereview"}): + # Resumes cannot recover criteria revisions until #329. Keep + # them explicitly open and retain the project's obligation. + verification_plan = verification_contract.VerificationPlan( + verification_contract.Criteria( + policy_provider or "unknown", "", "not_evaluable", + reason="runtime criteria revision unavailable")) + if role in {"implement", "resolve", "rereview"} and verification_plan is not None: + # Mandatory commands come from project policy, never the report. + if allowed_test_command and not any( + check.id == "configured-test" for check in verification_plan.checks): + verification_plan = verification_plan.extend(( + verification_contract.configured_check( + allowed_test_command, + outcome.result.executor if outcome.result else "unknown", + outcome.decision.profile if outcome.decision else "unknown"),)) + test_payload, verification_plan = _normalize_test_payload( + test_payload, verification_plan) pull_request_id = ( reported.telemetry_pull_request_id(code_host) if role == "implement" else None @@ -3380,8 +3408,11 @@ def advance(role: str, instruction: str = "", *, recorder.record_verdict(role, reported.status, **findings, **(_forecast(reported.payload) if role == "implement" else {}), - **_tests(reported.payload, boundary_rule=( - boundary_rule_fired(recorder.latest_change))), + **(_legacy_tests(test_payload, boundary_rule=( + boundary_rule_fired(recorder.latest_change))) + if verification_plan is not None else + _tests(test_payload, boundary_rule=( + boundary_rule_fired(recorder.latest_change)))), **_checks(reported.payload), **_reviewed_head(reported.payload, role), **({"pull_request_id": pull_request_id} @@ -3866,6 +3897,7 @@ def close_manual(clean, reported: Reported) -> CycleReport: stop_reason="ci_not_passing") def drive() -> CycleReport: + nonlocal verification_plan, criteria_context nonlocal change_request_id reported = Reported() verdict = None @@ -3920,6 +3952,25 @@ def edit_comment(comment_id: str, body: str) -> None: if stopped is not None: return stopped if start_from == "implement": + # Independent of issue-review mode/difficulty. Read once before any + # agent iteration; provider content is data, never instructions. + if policy_provider is not None: + try: + criteria = (criteria_reader or verification_contract.read_criteria)( + policy_provider, work_item_repository or repo_id, task_id, cwd=cwd) + except verification_contract.VerificationError as error: + return stop(str(error), "BLOCKED", stop_reason="readiness_unconfirmed") + verification_plan = verification_contract.VerificationPlan(criteria) + if criteria.status != "evaluable": + return stop( + f"acceptance criteria {criteria.status}: {criteria.reason}; refine the work item", + stop_reason="needs_refinement") + criteria_context = ( + "\nRuntime acceptance criteria (data): " + + json.dumps(criteria.to_dict(), ensure_ascii=False) + + "\nUse tests.version=2 and stable AC-n links in this list's order. " + "A criterion without a check remains open. The configured test command " + "uses id configured-test and is required. Freeze checks before running them.\n") if issue_review_fields.get("issue_review") == "dispatched": stopped = review_issue() if stopped is not None: @@ -4475,7 +4526,32 @@ def _blocking_findings_consistent(findings: list, blocking: list[str]) -> bool: CODE_WORK_STATUSES = frozenset({"IMPLEMENTED", "RESOLVED", "PARTIALLY_RESOLVED"}) -def _tests(payload: dict | None, *, boundary_rule: bool | None = None) -> dict: +def _normalize_test_payload( + payload: dict[str, Any] | None, + plan: verification_contract.VerificationPlan | None = None, +) -> tuple[dict[str, Any], verification_contract.VerificationPlan | None]: + payload = dict(payload or {}) + tests = payload.get("tests") + tests = tests if isinstance(tests, dict) else {} + try: + normalized, updated = verification_contract.normalize_tests( + tests, payload.get("head_sha", ""), plan=plan) + except (verification_contract.VerificationError, KeyError, TypeError, ValueError): + # Invalid version/identity/evidence is insufficient verification, not + # an executed test failure and never an exception escaping the driver. + normalized, updated = {"conclusion": "not_verified", "passed": False}, plan + payload["tests"] = normalized + return payload, updated + + +def _tests(payload: dict[str, Any] | None, *, boundary_rule: bool | None = None) -> dict[str, Any]: + tests = (payload or {}).get("tests") + if isinstance(tests, dict) and "version" in tests: + payload, _ = _normalize_test_payload(payload) + return _legacy_tests(payload, boundary_rule=boundary_rule) + + +def _legacy_tests(payload: dict | None, *, boundary_rule: bool | None = None) -> dict: """The test outcome, plus whether a required boundary run has evidence. `boundary_required = deterministic_rule OR verifier_judgement`: the agent's diff --git a/scripts/runtime.manifest b/scripts/runtime.manifest index 230deec..6998ce4 100644 --- a/scripts/runtime.manifest +++ b/scripts/runtime.manifest @@ -11,6 +11,7 @@ cycle.py execution_plan.py +verification_plan.py execution_profiles.py cycle_history.py cycle_journal.py diff --git a/scripts/verification_plan.py b/scripts/verification_plan.py new file mode 100644 index 0000000..779b8f1 --- /dev/null +++ b/scripts/verification_plan.py @@ -0,0 +1,359 @@ +"""Verification contracts; data only, never a command executor or CI substitute. + +Plan persistence and verification-only resume belong to the cycle history driver +(#329). These values share their owning ExecutionPlan's identity and revision. +""" +from __future__ import annotations + +from dataclasses import asdict, dataclass, replace +import hashlib +import json +import re +import subprocess +from collections.abc import Callable, Mapping +from typing import Any + + +TESTS_VERSION = 2 +CONCLUSIONS = ("failed", "not_verified", "verified_with_reservations", "verified") +LEVELS = {"static", "test", "boundary"} + + +class VerificationError(ValueError): + """Invalid criteria, check identity, or result representation.""" + + +@dataclass(frozen=True) +class Criteria: + provider: str + content_sha256: str + status: str # evaluable | needs_refinement | not_evaluable + items: tuple[str, ...] = () + reason: str = "" + + def to_dict(self) -> dict[str, Any]: + return {**asdict(self), "items": list(self.items)} + + @classmethod + def from_dict(cls, value: Mapping[str, Any]) -> Criteria: + items = value.get("items", ()) + if not isinstance(items, (list, tuple)): + raise VerificationError("criteria items must be an array") + result = cls(value["provider"], value["content_sha256"], value["status"], + tuple(items), value.get("reason", "")) + if (result.status not in {"evaluable", "needs_refinement", "not_evaluable"} + or not isinstance(result.provider, str) + or not isinstance(result.content_sha256, str) + or not isinstance(result.reason, str) + or any(not isinstance(item, str) or not item.strip() for item in result.items) + or (result.status == "evaluable") != bool(result.items)): + raise VerificationError("invalid criteria source") + return result + + +# Closed grammar: a section consists of one flat list, not arbitrary prose. +_LABEL = r"(?:Acceptance criteria|Criterios de aceptación)" +_HEADING = re.compile(rf"^\s*(?:#{{1,6}}\s+({_LABEL})\s*[:.]?\s*#*|\*\*({_LABEL})[:.]?\*\*[:.]?)\s*$", re.I) +_ITEM = re.compile(r"^(?:[-+*]|\d+[.)])\s+(?:\[[ xX]\]\s+)?(\S.*)$") +_SECTION = re.compile(r"^\s*(?:#{1,6}\s+|\*\*[^*]+\*\*[:.]?\s*$)") + + +def parse_criteria(provider: str, body: str | None) -> Criteria: + """Provider-independent interface; only GitHub has a body reader today.""" + if provider != "github": + return Criteria(provider, "", "not_evaluable", reason="criteria reader unavailable") + if not isinstance(body, str): + raise VerificationError("GitHub work-item body is unreadable") + digest = hashlib.sha256(body.encode()).hexdigest() + lines = body.splitlines() + starts = [index for index, line in enumerate(lines) if _HEADING.fullmatch(line)] + if len(starts) != 1: + return Criteria(provider, digest, "needs_refinement", reason="one acceptance-criteria section required") + items: list[str] = [] + for line in lines[starts[0] + 1:]: + if _SECTION.match(line): + break + if not line.strip(): + continue + match = _ITEM.fullmatch(line) + if not match: + return Criteria(provider, digest, "needs_refinement", reason="criteria must be a flat Markdown list") + text = match.group(1).strip() + if text in {"...", "TODO", "TBD"}: + return Criteria(provider, digest, "needs_refinement", reason="placeholder criterion") + items.append(text) + return Criteria(provider, digest, "evaluable" if items else "needs_refinement", + tuple(items), "" if items else "acceptance-criteria list is empty") + + +def read_criteria(provider: str, repository: str, work_item: str, *, + cwd: str | None = None, + runner: Callable[..., subprocess.CompletedProcess[str]] = subprocess.run) -> Criteria: + if provider != "github": + return parse_criteria(provider, None) + try: + result = runner(["gh", "issue", "view", work_item, "--repo", repository, + "--json", "body"], cwd=cwd, capture_output=True, text=True, timeout=30) + if result.returncode: + raise VerificationError("GitHub criteria read failed") + return parse_criteria(provider, json.loads(result.stdout)["body"]) + except (OSError, subprocess.TimeoutExpired, KeyError, TypeError, json.JSONDecodeError) as exc: + raise VerificationError("GitHub criteria read unavailable or invalid") from exc + + +@dataclass(frozen=True) +class Check: + id: str + command: str + executor: str + profile: str + context: str + required: bool + criteria: tuple[str, ...] = () + level: str = "test" + provenance: str = "agent" + reason: str = "proposed verification" + + def to_dict(self) -> dict[str, Any]: + return {**asdict(self), "criteria": list(self.criteria)} + + @classmethod + def from_dict(cls, item: Mapping[str, Any]) -> Check: + return cls(item["id"], item["command"], item["executor"], item["profile"], + item["context"], item["required"], tuple(item.get("criteria", ())), + item.get("level", "test"), item.get("provenance", "agent"), + item.get("reason", "proposed verification")) + + def validate(self, criterion_ids: set[str]) -> None: + if (any(not isinstance(v, str) or not v.strip() for v in ( + self.id, self.command, self.executor, self.profile, self.context, + self.provenance, self.reason)) or type(self.required) is not bool + or self.level not in LEVELS or len(set(self.criteria)) != len(self.criteria) + or not set(self.criteria) <= criterion_ids): + raise VerificationError("invalid check or criterion link") + if self.criteria and not self.required: + raise VerificationError("a criterion-dependent check must be required") + + +@dataclass(frozen=True) +class CheckResult: + id: str + head_sha: str | None + result: str + evidence: tuple[Mapping[str, Any], ...] = () + cause: str = "" + attempts: tuple[Mapping[str, Any], ...] = () + + def to_dict(self) -> dict[str, Any]: + return {**asdict(self), "evidence": [dict(v) for v in self.evidence], + "attempts": [dict(v) for v in self.attempts]} + + @classmethod + def from_dict(cls, item: Mapping[str, Any]) -> CheckResult: + return cls(item["id"], item["head_sha"], item["result"], + tuple(item.get("evidence", ())), item.get("cause", ""), + tuple(item.get("attempts", ()))) + + def validate(self) -> None: + if (not isinstance(self.id, str) or not self.id.strip() + or (self.head_sha is None and self.result != "unverified") + or (self.head_sha is not None and (not isinstance(self.head_sha, str) + or not re.fullmatch(r"[a-fA-F0-9]{40}", self.head_sha))) + or self.result not in {"pass", "fail", "unverified"} + or not isinstance(self.cause, str) + or any(not isinstance(v, Mapping) for v in (*self.evidence, *self.attempts))): + raise VerificationError("invalid check result") + if self.result == "unverified": + if not self.cause.strip(): + raise VerificationError("unverified result requires a cause") + return + if not self.evidence: + raise VerificationError("executed result requires evidence") + codes = [] + for item in self.evidence: + code, executed = item.get("exit_code"), item.get("executed") + if (type(code) is not int or "executed" not in item + or (executed is not None and (type(executed) is not int or executed < 0))): + raise VerificationError("executed evidence requires integer exit_code and count") + codes.append(code) + if (self.result == "pass") != all(code == 0 for code in codes): + raise VerificationError("check result contradicts evidence") + + +@dataclass(frozen=True) +class VerificationPlan: + criteria: Criteria + checks: tuple[Check, ...] = () + results: tuple[CheckResult, ...] = () # append-only history, including stale HEADs + + @property + def criterion_ids(self) -> set[str]: + return {f"AC-{i}" for i in range(1, len(self.criteria.items) + 1)} + + def validate(self) -> None: + Criteria.from_dict(self.criteria.to_dict()) + ids: set[str] = set() + for check in self.checks: + check.validate(self.criterion_ids) + if check.id in ids: + raise VerificationError("duplicate check identity") + ids.add(check.id) + for result in self.results: + result.validate() + if result.id not in ids: + raise VerificationError("result for unknown check") + command = next(check.command for check in self.checks if check.id == result.id) + if any(item.get("command", command) != command for item in result.evidence): + raise VerificationError("result evidence command differs from check") + + def extend(self, additions: tuple[Check, ...]) -> VerificationPlan: + """Freeze identity and obligations. Promotion and additions are allowed.""" + checks = {check.id: check for check in self.checks} + for check in additions: + old = checks.get(check.id) + if old is not None: + if replace(check, required=old.required, criteria=old.criteria) != old: + raise VerificationError("cannot rewrite frozen check identity") + if (old.required and not check.required) or not set(old.criteria) <= set(check.criteria): + raise VerificationError("cannot demote a frozen obligation") + checks[check.id] = check + plan = replace(self, checks=tuple(checks.values())) + plan.validate() + return plan + + def record(self, result: CheckResult) -> VerificationPlan: + plan = replace(self, results=(*self.results, result)) + plan.validate() + return plan + + def current(self, check_id: str, head_sha: str) -> str: + latest = next((result for result in reversed(self.results) if result.id == check_id), None) + return latest.result if latest is not None and latest.head_sha == head_sha else "unverified" + + def cap(self, conclusion: str, head_sha: str) -> str: + """Deterministic caps never raise the agent's conclusion.""" + if not isinstance(conclusion, str) or conclusion not in CONCLUSIONS: + raise VerificationError("unknown verification conclusion") + required = [check for check in self.checks if check.required] + states = [self.current(check.id, head_sha) for check in required] + cap = "verified" + if "fail" in states: + cap = "failed" + elif ("unverified" in states or self.criteria.status != "evaluable" + or any(not any(cid in check.criteria for check in required) + for cid in self.criterion_ids)): + cap = "not_verified" + return CONCLUSIONS[min(CONCLUSIONS.index(conclusion), CONCLUSIONS.index(cap))] + + def to_dict(self) -> dict[str, Any]: + self.validate() + return {"criteria": self.criteria.to_dict(), + "checks": [check.to_dict() for check in self.checks], + "results": [result.to_dict() for result in self.results]} + + @classmethod + def from_dict(cls, value: Mapping[str, Any]) -> VerificationPlan: + try: + plan = cls(Criteria.from_dict(value["criteria"]), + tuple(Check.from_dict(item) for item in value.get("checks", ())), + tuple(CheckResult.from_dict(item) for item in value.get("results", ()))) + plan.validate() + return plan + except (KeyError, TypeError, ValueError, AttributeError) as exc: + raise VerificationError(f"invalid verification plan: {exc}") from exc + + +def configured_check(command: str, executor: str, profile: str) -> Check: + return Check("configured-test", command, executor, profile, "project", True, + provenance="project", reason="configured test command") + + +def normalize_tests(tests: Mapping[str, Any], head_sha: str, *, + plan: VerificationPlan | None = None) -> tuple[dict[str, Any], VerificationPlan | None]: + """Normalize v2 (or the one-release v1 adapter) against runtime obligations. + + The normalized evidence is only a compatibility view for telemetry consumers; + explicit check states decide caps. No code-host check can substitute for one. + """ + version = tests.get("version", 1) + if type(version) is not int or version not in {1, TESTS_VERSION}: + raise VerificationError("unsupported tests.version") + if version == 1 and plan is None: + return dict(tests), None # legacy consumers retain their historical contract + if not re.fullmatch(r"[a-fA-F0-9]{40}", head_sha): + raise VerificationError("per-check results require the verified HEAD") + if plan is None: + # Standalone v2 reports still need a real criterion list to claim coverage. + plan = VerificationPlan(Criteria.from_dict(tests.get("criteria", { + "provider": "unknown", "content_sha256": "", "status": "not_evaluable", + "reason": "runtime criteria unavailable"}))) + entries = tests.get("checks", []) if version == 2 else tests.get("evidence", []) + if not isinstance(entries, list): + raise VerificationError("checks/evidence must be an array") + if version == 1: + # Only match an exact known command. Missing evidence leaves it unverified. + converted = [] + known_commands = {check.command for check in plan.checks} + for check in plan.checks: + evidence = [v for v in entries if isinstance(v, dict) and v.get("command") == check.command] + if evidence: + converted.append({**check.to_dict(), "head_sha": head_sha, + "result": "pass" if all(v.get("exit_code") == 0 for v in evidence) else "fail", + "evidence": evidence}) + # Keep other executed commands too: an unrelated failing legacy entry + # must not disappear while converting the report to per-check results. + for entry in entries: + if not isinstance(entry, dict) or not isinstance(entry.get("command"), str): + raise VerificationError("invalid legacy evidence") + command = entry["command"] + if command in known_commands: + continue + spec = Check("legacy-" + hashlib.sha256(command.encode()).hexdigest()[:16], + command, "legacy", "legacy", "legacy", False, + level=entry.get("level", "test"), provenance="legacy", + reason="version 1 evidence adapter") + plan = plan.extend((spec,)) + converted.append({**spec.to_dict(), "head_sha": head_sha, + "result": "pass" if entry.get("exit_code") == 0 else "fail", + "evidence": [entry]}) + entries = converted + seen: set[str] = set() + evidence_view: list[dict[str, Any]] = [] + for entry in entries: + if not isinstance(entry, dict): + raise VerificationError("check must be an object") + check = Check.from_dict(entry) + if check.id in seen: + raise VerificationError("duplicate reported check") + seen.add(check.id) + # Runtime criteria links promote agent obligations rather than accepting a demotion. + if check.criteria: + check = replace(check, required=True) + plan = plan.extend((check,)) + result = CheckResult(check.id, entry["head_sha"], entry["result"], + tuple(entry.get("evidence", ())), entry.get("cause", ""), + tuple(entry.get("attempts", ()))) + for item in result.evidence: + if item.get("command", check.command) != check.command: + raise VerificationError("evidence command does not match frozen check") + plan = plan.record(result) + if result.head_sha == head_sha and result.result != "unverified": + evidence_view.extend({**item, "command": check.command, "level": check.level} + for item in result.evidence) + for check in plan.checks: + if not any(result.id == check.id for result in plan.results): + plan = plan.record(CheckResult(check.id, head_sha, "unverified", + cause="check has no execution result")) + conclusion = plan.cap(tests.get("conclusion", "not_verified"), head_sha) + normalized = {**tests, "version": 2, "conclusion": conclusion, + "passed": conclusion in {"verified", "verified_with_reservations"}, + "evidence": evidence_view, + "criteria": plan.criteria.to_dict(), + "checks": [{**check.to_dict(), **next( + result.to_dict() for result in reversed(plan.results) if result.id == check.id)} + for check in plan.checks]} + # An unexecuted required check still records not_verified; ran:false is not + # allowed to suppress deterministic caps in telemetry. + if conclusion in {"not_verified", "failed"}: + normalized.pop("ran", None) + return normalized, plan diff --git a/scripts/windows-test-shards.json b/scripts/windows-test-shards.json index 625e21e..901e17b 100644 --- a/scripts/windows-test-shards.json +++ b/scripts/windows-test-shards.json @@ -94,6 +94,7 @@ "test_dispatch_failures", "test_execution_plan", "test_execution_profiles", + "test_verification_plan", "test_stats", "test_support", "test_validate_package" diff --git a/skills/cc-implement-issue/SKILL.md b/skills/cc-implement-issue/SKILL.md index df1f457..ab5087f 100644 --- a/skills/cc-implement-issue/SKILL.md +++ b/skills/cc-implement-issue/SKILL.md @@ -5,6 +5,36 @@ description: Use this skill when asked to implement a work item from GitHub Issu # Implement Issue +## Verification result version 2 + +Emit new test reports with `tests.version: 2` and `tests.checks` (for +`VERIFICATION_RESULT`, these are the top-level `version` and `checks`). +The older evidence examples below are version-1 compatibility examples for +one release only: from the first published release supporting version 2 until +the next published release. Follow the version-2 contract for new output. + +Before execution, freeze checks against the runtime's `AC-n` criteria and +project commands. Each check carries `id`, `command`, `executor`, `profile`, +`context`, `required`, `criteria` (criterion IDs), `level`, `provenance` and +`reason`. The configured test uses `id: configured-test`, `context: project`, +`provenance: project`, `reason: configured test command`, and the dispatched +executor/profile. Every check linked to a criterion is required. Add or +promote checks; never remove or demote frozen obligations. + +Each result also carries `head_sha`, `result: pass|fail|unverified`, +`evidence` (integer `exit_code`, `executed` count or null), `cause` and +`attempts`. The enclosing stage result names its current `head_sha` too. +An unexecuted check is explicitly `unverified` with a cause and the recovery +attempts actually made; do not invent an exit code. Keep the check's identity +when rerunning it and retain previous results. Never replace a local +unverified result with code-host CI success. A required failure caps the +conclusion at `failed`; a required unverified check or an uncovered criterion +caps it at `not_verified`. Caps never improve the agent's conclusion. +Copy the verifier's checks unchanged into the stage result and published +verification section, labelled agent-reported. Keep code-host `checks.items` +separate. See `docs/verification.md` and `docs/execution-plan.md` in the toolkit. + + Turn one work item into a coherent, tested pull request. Keep the work limited to the work item and the repository's trusted contribution workflow. diff --git a/skills/cc-rereview/SKILL.md b/skills/cc-rereview/SKILL.md index 66d6044..aeb3dc3 100644 --- a/skills/cc-rereview/SKILL.md +++ b/skills/cc-rereview/SKILL.md @@ -5,6 +5,36 @@ description: Use this skill manually or as an orchestrated task when a GitHub or # Change Request Re-review +## Verification result version 2 + +Emit new test reports with `tests.version: 2` and `tests.checks` (for +`VERIFICATION_RESULT`, these are the top-level `version` and `checks`). +The older evidence examples below are version-1 compatibility examples for +one release only: from the first published release supporting version 2 until +the next published release. Follow the version-2 contract for new output. + +Before execution, freeze checks against the runtime's `AC-n` criteria and +project commands. Each check carries `id`, `command`, `executor`, `profile`, +`context`, `required`, `criteria` (criterion IDs), `level`, `provenance` and +`reason`. The configured test uses `id: configured-test`, `context: project`, +`provenance: project`, `reason: configured test command`, and the dispatched +executor/profile. Every check linked to a criterion is required. Add or +promote checks; never remove or demote frozen obligations. + +Each result also carries `head_sha`, `result: pass|fail|unverified`, +`evidence` (integer `exit_code`, `executed` count or null), `cause` and +`attempts`. The enclosing stage result names its current `head_sha` too. +An unexecuted check is explicitly `unverified` with a cause and the recovery +attempts actually made; do not invent an exit code. Keep the check's identity +when rerunning it and retain previous results. Never replace a local +unverified result with code-host CI success. A required failure caps the +conclusion at `failed`; a required unverified check or an uncovered criterion +caps it at `not_verified`. Caps never improve the agent's conclusion. +Copy the verifier's checks unchanged into the stage result and published +verification section, labelled agent-reported. Keep code-host `checks.items` +separate. See `docs/verification.md` and `docs/execution-plan.md` in the toolkit. + + Re-review the current change request without modifying product code. Use the current accumulated diff against the base and the previous review state; do not limit the review to the latest commit or repeat stale text. diff --git a/skills/cc-resolve-comments/SKILL.md b/skills/cc-resolve-comments/SKILL.md index fd357ee..2363f69 100644 --- a/skills/cc-resolve-comments/SKILL.md +++ b/skills/cc-resolve-comments/SKILL.md @@ -5,6 +5,36 @@ description: Use this skill manually or as an orchestrated task to recover stabl # Resolve Change Request Review Comments +## Verification result version 2 + +Emit new test reports with `tests.version: 2` and `tests.checks` (for +`VERIFICATION_RESULT`, these are the top-level `version` and `checks`). +The older evidence examples below are version-1 compatibility examples for +one release only: from the first published release supporting version 2 until +the next published release. Follow the version-2 contract for new output. + +Before execution, freeze checks against the runtime's `AC-n` criteria and +project commands. Each check carries `id`, `command`, `executor`, `profile`, +`context`, `required`, `criteria` (criterion IDs), `level`, `provenance` and +`reason`. The configured test uses `id: configured-test`, `context: project`, +`provenance: project`, `reason: configured test command`, and the dispatched +executor/profile. Every check linked to a criterion is required. Add or +promote checks; never remove or demote frozen obligations. + +Each result also carries `head_sha`, `result: pass|fail|unverified`, +`evidence` (integer `exit_code`, `executed` count or null), `cause` and +`attempts`. The enclosing stage result names its current `head_sha` too. +An unexecuted check is explicitly `unverified` with a cause and the recovery +attempts actually made; do not invent an exit code. Keep the check's identity +when rerunning it and retain previous results. Never replace a local +unverified result with code-host CI success. A required failure caps the +conclusion at `failed`; a required unverified check or an uncovered criterion +caps it at `not_verified`. Caps never improve the agent's conclusion. +Copy the verifier's checks unchanged into the stage result and published +verification section, labelled agent-reported. Keep code-host `checks.items` +separate. See `docs/verification.md` and `docs/execution-plan.md` in the toolkit. + + Resolve change-request review feedback deliberately. The user's request to fix valid comments authorizes targeted code changes, not unrelated rewrites. diff --git a/skills/cc-verify/SKILL.md b/skills/cc-verify/SKILL.md index c19f858..7786381 100644 --- a/skills/cc-verify/SKILL.md +++ b/skills/cc-verify/SKILL.md @@ -5,6 +5,36 @@ description: Use this skill to verify that code changes actually work by running # Verify +## Verification result version 2 + +Emit new test reports with `tests.version: 2` and `tests.checks` (for +`VERIFICATION_RESULT`, these are the top-level `version` and `checks`). +The older evidence examples below are version-1 compatibility examples for +one release only: from the first published release supporting version 2 until +the next published release. Follow the version-2 contract for new output. + +Before execution, freeze checks against the runtime's `AC-n` criteria and +project commands. Each check carries `id`, `command`, `executor`, `profile`, +`context`, `required`, `criteria` (criterion IDs), `level`, `provenance` and +`reason`. The configured test uses `id: configured-test`, `context: project`, +`provenance: project`, `reason: configured test command`, and the dispatched +executor/profile. Every check linked to a criterion is required. Add or +promote checks; never remove or demote frozen obligations. + +Each result also carries `head_sha`, `result: pass|fail|unverified`, +`evidence` (integer `exit_code`, `executed` count or null), `cause` and +`attempts`. The enclosing stage result names its current `head_sha` too. +An unexecuted check is explicitly `unverified` with a cause and the recovery +attempts actually made; do not invent an exit code. Keep the check's identity +when rerunning it and retain previous results. Never replace a local +unverified result with code-host CI success. A required failure caps the +conclusion at `failed`; a required unverified check or an uncovered criterion +caps it at `not_verified`. Caps never improve the agent's conclusion. +Copy the verifier's checks unchanged into the stage result and published +verification section, labelled agent-reported. Keep code-host `checks.items` +separate. See `docs/verification.md` and `docs/execution-plan.md` in the toolkit. + + Verify that the changes really work. Do not stop at inspecting the code. **Reading code does not count as verification. Only executed commands with an diff --git a/tests/test_execution_plan.py b/tests/test_execution_plan.py index 148bf6a..a197f9c 100644 --- a/tests/test_execution_plan.py +++ b/tests/test_execution_plan.py @@ -213,18 +213,18 @@ def standard(self) -> plan.ExecutionPlan: def test_v2_round_trip_keeps_decisions_obligations_and_controls(self) -> None: sample = self.standard() - payload = sample.to_dict() + payload = sample.to_dict(contract_version=2) self.assertEqual(payload["contract_version"], 2) self.assertEqual({item["control_id"] for item in payload["controls"]}, {"ci", "merge"}) self.assertTrue(payload["decisions"] and payload["obligations"]) self.assertEqual(plan.ExecutionPlan.from_dict(payload), sample) self.assertEqual(plan.ExecutionPlan.from_dict( - json.loads(json.dumps(payload))).to_dict(), payload) + json.loads(json.dumps(payload))).to_dict(contract_version=2), payload) def test_v1_payload_is_read_and_projected_back_unchanged(self) -> None: payload = self.v1_payload() upgraded = plan.ExecutionPlan.from_dict(payload) - self.assertEqual(upgraded.contract_version, 2) + self.assertEqual(upgraded.contract_version, plan.CONTRACT_VERSION) self.assertEqual(upgraded.stages[0].resolution, plan.Resolution.RESOLVED) self.assertIsNone(upgraded.stages[0].policy) self.assertEqual(upgraded.to_dict(contract_version=1), payload) @@ -262,14 +262,14 @@ def test_v1_projection_envelope_is_validated_on_upgrade(self) -> None: plan.ExecutionPlan.from_dict(payload) def test_unsupported_versions_are_rejected_not_read_as_v1(self) -> None: - for version in (0, 3, "1", None, True): + for version in (0, 4, "1", None, True): with self.subTest(version=version): payload = self.v1_payload() payload["contract_version"] = version with self.assertRaisesRegex(plan.ContractError, "unsupported"): plan.ExecutionPlan.from_dict(payload) with self.assertRaisesRegex(plan.ContractError, "unsupported"): - self.standard().to_dict(contract_version=3) + self.standard().to_dict(contract_version=4) for version in (True, False, 1.0, 2.0, "2"): with self.subTest(output_version=version): with self.assertRaisesRegex(plan.ContractError, "unsupported"): diff --git a/tests/test_run_cycle.py b/tests/test_run_cycle.py index ce42e09..3e68c0b 100644 --- a/tests/test_run_cycle.py +++ b/tests/test_run_cycle.py @@ -181,6 +181,10 @@ def run_cycle(self, implementer, reviewer, **kw): # The original flow, which these tests describe; the issue-review stage # has its own tests, which ask for it. kw.setdefault("issue_review", "off") + # Provider reads are explicit fixtures; driver tests never contact GitHub. + kw.setdefault("criteria_reader", lambda provider, *_args, **_kw: + rc.verification_contract.parse_criteria( + provider, "## Acceptance criteria\n- Fixture behaviour works")) if kw.get("start_from", "implement") != "implement": # A resume rebuilds the claimed-fix history from the change # request's comments; these tests describe a request without any. diff --git a/tests/test_stage_dispatch.py b/tests/test_stage_dispatch.py index 7395458..117f2ce 100644 --- a/tests/test_stage_dispatch.py +++ b/tests/test_stage_dispatch.py @@ -144,7 +144,9 @@ def runner(argv, **kw): self.assertEqual("IMPLEMENTED", parsed["status"]) self.assertEqual("not_verified", parsed["tests"]["conclusion"]) self.assertNotIn("passed", parsed["tests"]) - self.assertEqual("pytest", parsed["tests"]["evidence"][0]["command"]) + self.assertEqual(2, parsed["tests"]["version"]) + self.assertEqual("pytest", parsed["tests"]["checks"][0]["command"]) + self.assertEqual("unverified", parsed["tests"]["checks"][0]["result"]) def test_preparation_role_gating_and_evidence_reach_each_native_prompt(self) -> None: for executor, adapter_type, target in (("codex", ex.CodexAdapter, CODEX), @@ -877,6 +879,13 @@ def test_the_prompt_states_the_table_and_how_to_stop(self) -> None: class RuntimeStopTests(unittest.TestCase): def setUp(self) -> None: + # These tests isolate command-policy stops from provider availability. + reader = mock.patch.object(rc.verification_contract, "read_criteria", side_effect=( + lambda provider, *_args, **_kw: replace( + rc.verification_contract.parse_criteria( + "github", "## Acceptance criteria\n- Fixture behaviour works"), provider=provider))) + reader.start() + self.addCleanup(reader.stop) temporary = tempfile.TemporaryDirectory() self.addCleanup(temporary.cleanup) import telemetry as tm diff --git a/tests/test_verification_plan.py b/tests/test_verification_plan.py new file mode 100644 index 0000000..8a083a9 --- /dev/null +++ b/tests/test_verification_plan.py @@ -0,0 +1,267 @@ +"""Verification contracts and their runtime/telemetry boundaries.""" +from __future__ import annotations + +from dataclasses import replace +import json +from pathlib import Path +import subprocess +import sys +import unittest +from unittest.mock import Mock + +ROOT = Path(__file__).resolve().parent.parent +sys.path.insert(0, str(ROOT / "scripts")) +sys.path.insert(0, str(ROOT / "tests")) +import execution_plan as ep +import executors as ex +import execution_profiles as profiles +import run_cycle as rc +import stats +import verification_plan as vp +from test_run_cycle import RunCycleTestCase, Talker, block + +HEAD = "a" * 40 +OTHER = "b" * 40 + + +def criteria(body="## Acceptance criteria\n- Works\n"): + return vp.parse_criteria("github", body) + + +def check(**kw): + return vp.Check(**{**dict(id="check-1", command="python3 -m unittest", + executor="codex", profile="deep_coder", context="local", + required=True, criteria=("AC-1",)), **kw}) + + +def result(state="pass", head=HEAD, id="check-1"): + return vp.CheckResult(id, head, state, + ({"exit_code": 0 if state == "pass" else 1, + "executed": 2},) if state != "unverified" else (), + "sandbox unavailable" if state == "unverified" else "") + + +def report(state="pass", **kw): + return {**check().to_dict(), **result(state).to_dict(), **kw} + + +class CriteriaTests(unittest.TestCase): + def test_closed_grammar_english_spanish_heading_bold_lists(self): + for heading in ("## Acceptance criteria", "### Criterios de aceptación", + "**Acceptance criteria:**", "**Criterios de aceptación.**"): + for bullet in ("-", "*", "+", "1.", "2)"): + with self.subTest(heading=heading, bullet=bullet): + parsed = criteria(f"{heading}\n{bullet} [ ] Works\n\n## Tests\nOther prose") + self.assertEqual(parsed.status, "evaluable") + self.assertEqual(parsed.items, ("Works",)) + + def test_missing_empty_duplicate_unparseable_and_placeholder_stay_open(self): + for body in ("## Problem\nWorks", "## Acceptance criteria\n", "## Acceptance criteria\nprose", + "## Acceptance criteria\n- Works\n - nested", + "## Acceptance criteria\n- TODO", "## Acceptance criteria\n- A\n## Acceptance criteria\n- B"): + with self.subTest(body=body): + self.assertEqual(criteria(body).status, "needs_refinement") + + def test_provider_independent_unavailable_readers_never_satisfied(self): + runner = Mock() + for provider in ("plane", "jira"): + parsed = vp.read_criteria(provider, "project", "work", runner=runner) + self.assertEqual(parsed.status, "not_evaluable") + self.assertEqual(vp.VerificationPlan(parsed).cap("verified", HEAD), "not_verified") + runner.assert_not_called() + + def test_github_reader_failure_and_body_change(self): + runner = Mock(return_value=subprocess.CompletedProcess([], 0, json.dumps({"body": "## Acceptance criteria\n- A"}))) + parsed = vp.read_criteria("github", "o/r", "1", runner=runner) + self.assertEqual(parsed.items, ("A",)) + self.assertNotEqual(parsed.content_sha256, criteria().content_sha256) + self.assertEqual(runner.call_args.args[0], ["gh", "issue", "view", "1", "--repo", "o/r", "--json", "body"]) + for value in (subprocess.CompletedProcess([], 1, ""), subprocess.CompletedProcess([], 0, "{}")): + runner.return_value = value + with self.assertRaises(vp.VerificationError): + vp.read_criteria("github", "o/r", "1", runner=runner) + + def test_reader_crosses_real_process_boundary_without_live_credentials(self): + def local_provider(_command, **kwargs): + return subprocess.run([ + sys.executable, "-c", + "import json; print(json.dumps({'body': '## Acceptance criteria\\n- Observable result'}))" + ], **kwargs) + + parsed = vp.read_criteria("github", "o/r", "290", runner=local_provider) + self.assertEqual(parsed.status, "evaluable") + self.assertEqual(parsed.items, ("Observable result",)) + + +class PlanTests(unittest.TestCase): + def plan(self): + return vp.VerificationPlan(criteria()).extend((check(),)) + + def test_freeze_allows_addition_promotion_but_never_rewrite_or_demotion(self): + plan = self.plan() + for changed in (check(required=False), check(command="true"), check(executor="claude"), + check(profile="cheap_coder"), check(context="ci"), check(criteria=())): + with self.subTest(changed=changed), self.assertRaises(vp.VerificationError): + plan.extend((changed,)) + optional = check(id="optional", required=False, criteria=()) + plan = plan.extend((optional,)).extend((replace(optional, required=True),)) + self.assertTrue(plan.checks[-1].required) + self.assertEqual(plan.extend(()), plan) + + def test_results_preserve_history_and_invalidate_stale_head(self): + plan = self.plan().record(result()) + self.assertEqual(plan.cap("verified", HEAD), "verified") + self.assertEqual(plan.cap("verified", OTHER), "not_verified") + newer = plan.record(result("unverified", OTHER)) + self.assertEqual(len(newer.results), 2) + self.assertEqual(newer.current("check-1", HEAD), "unverified") + newer = newer.record(result("pass", OTHER)) + self.assertEqual(newer.cap("verified", OTHER), "verified") + self.assertEqual(vp.VerificationPlan.from_dict(newer.to_dict()), newer) + + def test_caps_required_fail_unverified_open_and_never_raise(self): + for state, cap in (("pass", "verified"), ("fail", "failed"), ("unverified", "not_verified")): + plan = self.plan().record(result(state)) + self.assertEqual(plan.cap("verified", HEAD), cap) + self.assertEqual(plan.cap("failed", HEAD), "failed") + self.assertEqual(vp.VerificationPlan(criteria()).cap("verified", HEAD), "not_verified") + optional = vp.VerificationPlan(criteria()).extend((check(), check(id="optional", required=False, criteria=()))) + optional = optional.record(result()).record(result("unverified", id="optional")) + self.assertEqual(optional.cap("verified", HEAD), "verified") + + def test_result_rejects_invalid_counts_exit_codes_contradictions_and_unknown_id(self): + for value in (replace(result(), evidence=()), replace(result(), evidence=({"exit_code": True, "executed": 0},)), + replace(result(), evidence=({"exit_code": 1, "executed": 0},)), + replace(result(), head_sha="short"), replace(result("unverified"), cause=""), + result(id="unknown")): + with self.subTest(value=value), self.assertRaises(vp.VerificationError): + self.plan().record(value) + + def test_execution_plan_versions_and_owner_revision(self): + owner = profiles.build_plan(profiles.resolve_execution(profiles.ExecutionSettings(), profiles.ExecutionSettings()), + repository="o/r", work_item="290") + owner = replace(owner, verification=self.plan().record(result())) + self.assertEqual(owner.to_dict()["contract_version"], 3) + self.assertEqual(ep.ExecutionPlan.from_dict(owner.to_dict()), owner) + for version in (1, 2): + with self.assertRaises(ep.ContractError): + owner.to_dict(version) + legacy_owner = profiles.build_plan(profiles.resolve_execution( + profiles.ExecutionSettings(), profiles.invocation_settings(execution_profile="prototype")), + repository="o/r", work_item="290") + older = legacy_owner.to_dict(version) + self.assertIsNone(ep.ExecutionPlan.from_dict(older).verification) + extended = owner.extend_verification((check(id="new", required=False, criteria=()),)) + self.assertEqual(extended.revision, owner.revision + 1) + self.assertEqual(extended.verification.results, owner.verification.results) + + +class ResultTests(unittest.TestCase): + def test_v2_records_caps_and_boundary_without_touching_host_checks(self): + payload = {"status": "IMPLEMENTED", "head_sha": HEAD, "tests": { + "version": 2, "conclusion": "verified", "criteria": criteria().to_dict(), + "boundary": "required", "checks": [report(level="boundary")]}, + "checks": {"head_sha": HEAD, "passed": 3, "failed": 0, "pending": 1}} + before = dict(payload["checks"]) + fields = rc._tests(payload) + # A boundary-only report cannot prove that tests ran. + self.assertFalse(fields["tests_passed"]) + payload["tests"]["checks"].append(report(id="test")) + fields = rc._tests(payload) + self.assertTrue(fields["tests_passed"]) + self.assertTrue(fields["boundary_verified"]) + self.assertEqual(fields["tests_basis"], "agent_reported") + self.assertEqual(payload["checks"], before) + self.assertEqual(rc._checks(payload), {"checks_passed": 3, "checks_failed": 0, "checks_pending": 1, + "checks_head_sha": HEAD}) + + def test_legacy_adapter_does_not_invent_missing_required_pass(self): + plan = vp.VerificationPlan(criteria()).extend((check(), vp.configured_check("required", "codex", "deep_coder"))) + legacy = {"conclusion": "verified", "passed": True, + "evidence": [{"command": check().command, "level": "test", "exit_code": 0, "executed": 2}]} + tests, updated = vp.normalize_tests(legacy, HEAD, plan=plan) + self.assertEqual(tests["conclusion"], "not_verified") + self.assertEqual(updated.current("configured-test", HEAD), "unverified") + self.assertEqual(updated.current("check-1", HEAD), "pass") + + def test_omitted_failure_cannot_be_erased_by_later_report(self): + plan = vp.VerificationPlan(criteria()).extend((check(),)).record(result("fail")) + tests, updated = vp.normalize_tests( + {"version": 2, "conclusion": "verified", "checks": []}, HEAD, plan=plan) + self.assertEqual(tests["conclusion"], "failed") + self.assertEqual(updated.results, plan.results) + + def test_legacy_unmatched_failures_are_retained(self): + plan = vp.VerificationPlan(criteria()).extend((check(),)) + payload, updated = rc._normalize_test_payload({"status": "IMPLEMENTED", "head_sha": HEAD, + "tests": {"passed": True, "conclusion": "verified", "evidence": [ + {"level": "test", "command": check().command, "exit_code": 0, "executed": 2}, + {"level": "static", "command": "lint", "exit_code": 1, "executed": None}]}}, plan) + self.assertFalse(rc._legacy_tests(payload)["tests_passed"]) + self.assertEqual(len(updated.results), 2) + + def test_invalid_and_stale_reports_fail_closed_and_ran_false_cannot_hide_caps(self): + for changed in ({"head_sha": OTHER}, {"result": "unverified", "evidence": [], "cause": "no sandbox"}, + {"evidence": [{"exit_code": False, "executed": 1}]}, {"criteria": ["AC-99"]}): + fields = rc._tests({"status": "IMPLEMENTED", "head_sha": HEAD, "tests": { + "version": 2, "criteria": criteria().to_dict(), "ran": False, "passed": True, + "conclusion": "verified", "checks": [report(**changed)]}}) + self.assertEqual(fields["verification"], "not_verified") + self.assertFalse(fields["tests_passed"]) + self.assertEqual(rc._tests({"tests": {"version": 9, "passed": True}})["verification"], "not_verified") + + +class CriteriaGateTests(RunCycleTestCase): + def test_gate_runs_for_every_issue_review_mode_and_difficulty_before_dispatch(self): + for mode in ("off", "auto"): + for provider in ("github", "plane", "jira"): + for difficulty in (1, 2, 3): + reader = Mock(return_value=vp.parse_criteria(provider, "## Problem\nNo section")) + implementer, reviewer = Talker("codex"), Talker("claude") + report_ = rc.run_cycle( + "owner/api", "290", rc.TaskSignals(difficulty=difficulty), self.store, + registry=rc.Registry([implementer, reviewer]), + availability={"codex": ex.Availability.READY, "claude": ex.Availability.READY}, + work_item_provider=provider, work_item_repository="owner/issues", + issue_review=mode, criteria_reader=reader) + self.assertEqual(report_.status, "HUMAN_INTERVENTION") + self.assertIn("acceptance criteria", report_.stopped_because) + self.assertEqual(implementer.dispatched, []) + reader.assert_called_once() + self.assertEqual(self.rows()[-1]["payload"]["stop_reason"], "needs_refinement") + self.assertNotIn("verification", self.rows()[-1]["payload"]) + + def test_valid_criteria_read_once_and_context_reaches_implementation(self): + reader = Mock(return_value=criteria()) + implementer, reviewer = Talker("codex"), Talker("claude") + self.run_cycle(implementer, reviewer, work_item_provider="github", + work_item_repository="owner/issues", criteria_reader=reader) + reader.assert_called_once() + self.assertIn("Runtime acceptance criteria", implementer.dispatched[0]) + + def test_v2_conclusion_reaches_telemetry_and_stats_as_agent_reported(self): + tests = {"version": 2, "conclusion": "verified", "checks": [report("unverified")]} + implementer = Talker("codex", block("IMPLEMENTED", head_sha=HEAD, tests=tests)) + self.run_cycle(implementer, Talker("claude"), work_item_provider="github", + work_item_repository="owner/issues", + criteria_reader=lambda *_a, **_kw: criteria()) + verdict = next(row["payload"] for row in self.rows() if row["role"] == "implement" + and row["payload"].get("record_kind") == "verdict") + self.assertEqual(verdict["verification"], "not_verified") + self.assertFalse(verdict["tests_passed"]) + # No executed command: it stays a claim, not runtime-observed evidence. + self.assertEqual(verdict["tests_basis"], "claimed") + aggregate = stats.aggregate(self.rows(), repo_id="owner/api", days=None) + self.assertEqual(aggregate["summary"]["verification"]["conclusions"]["not_verified"], 1) + + def test_reported_passes_keep_their_basis_through_telemetry_and_stats(self): + tests = {"version": 2, "conclusion": "verified", "checks": [report()]} + self.run_cycle(Talker("codex", block("IMPLEMENTED", head_sha=HEAD, tests=tests)), + Talker("claude"), work_item_provider="github", work_item_repository="o/r", + criteria_reader=lambda *_a, **_kw: criteria()) + aggregate = stats.aggregate(self.rows(), repo_id="owner/api", days=None) + self.assertEqual(aggregate["summary"]["verification"]["by_basis"]["agent_reported"]["passed"], 1) + + +if __name__ == "__main__": + unittest.main() From 9070ed2d332451f71151a2c1b6aec5490da8b475 Mon Sep 17 00:00:00 2001 From: datacas <4690232+datacas@users.noreply.github.com> Date: Thu, 29 Oct 2026 18:12:58 +0100 Subject: [PATCH 2/3] fix: preserve failed verification caps on malformed follow-ups --- scripts/run_cycle.py | 5 ++++- tests/test_verification_plan.py | 7 +++++++ 2 files changed, 11 insertions(+), 1 deletion(-) diff --git a/scripts/run_cycle.py b/scripts/run_cycle.py index b0f83cf..4986d71 100644 --- a/scripts/run_cycle.py +++ b/scripts/run_cycle.py @@ -4539,7 +4539,10 @@ def _normalize_test_payload( except (verification_contract.VerificationError, KeyError, TypeError, ValueError): # Invalid version/identity/evidence is insufficient verification, not # an executed test failure and never an exception escaping the driver. - normalized, updated = {"conclusion": "not_verified", "passed": False}, plan + conclusion = "failed" if tests.get("conclusion") == "failed" else "not_verified" + if plan is not None: + conclusion = plan.cap(conclusion, payload.get("head_sha", "")) + normalized, updated = {"conclusion": conclusion, "passed": False}, plan payload["tests"] = normalized return payload, updated diff --git a/tests/test_verification_plan.py b/tests/test_verification_plan.py index 8a083a9..31ded39 100644 --- a/tests/test_verification_plan.py +++ b/tests/test_verification_plan.py @@ -191,6 +191,13 @@ def test_omitted_failure_cannot_be_erased_by_later_report(self): self.assertEqual(tests["conclusion"], "failed") self.assertEqual(updated.results, plan.results) + def test_invalid_followup_never_raises_failed_agent_or_frozen_conclusion(self): + for plan, conclusion in ((None, "failed"), ( + vp.VerificationPlan(criteria()).extend((check(),)).record(result("fail")), "verified")): + payload, _ = rc._normalize_test_payload({"head_sha": HEAD, "tests": { + "version": 99, "conclusion": conclusion, "passed": True}}, plan) + self.assertEqual(rc._legacy_tests(payload)["verification"], "failed") + def test_legacy_unmatched_failures_are_retained(self): plan = vp.VerificationPlan(criteria()).extend((check(),)) payload, updated = rc._normalize_test_payload({"status": "IMPLEMENTED", "head_sha": HEAD, From 77db5dc7c7639aafdccaf06b40d248110e852f1d Mon Sep 17 00:00:00 2001 From: datacas <4690232+datacas@users.noreply.github.com> Date: Thu, 29 Oct 2026 18:39:50 +0100 Subject: [PATCH 3/3] fix(verification): preserve frozen checks and prior test outcomes --- docs/verification.md | 20 ++++++- scripts/run_cycle.py | 25 +++++++- skills/cc-implement-issue/SKILL.md | 8 ++- skills/cc-rereview/SKILL.md | 8 ++- skills/cc-resolve-comments/SKILL.md | 8 ++- skills/cc-verify/SKILL.md | 8 ++- tests/test_verification_plan.py | 91 ++++++++++++++++++++++++++++- 7 files changed, 153 insertions(+), 15 deletions(-) diff --git a/docs/verification.md b/docs/verification.md index d000b0b..192b012 100644 --- a/docs/verification.md +++ b/docs/verification.md @@ -208,7 +208,11 @@ top-level `tests.evidence` list. This is separate from the unchanged code-host ``` Freeze each check before execution. A result names the same check ID, command, -executor/profile and context. Required checks originate in project policy and +executor/profile and context. The runtime supplies frozen check identities to +later code stages. Copy that identity, including its owning executor/profile, +even when resolve or rereview is dispatched with another profile or executor. +A check requiring a different execution context gets a new ID and does not +replace the existing obligation. Required checks originate in project policy and criterion links; an agent may promote, never demote. Record an unexecuted check as `result: unverified`, with `cause`, evidence and actual recovery `attempts`; do not invent an exit code. Results on another HEAD stay in history but cannot @@ -228,8 +232,13 @@ before any implementation, even with issue review off or difficulty 1. They are insufficient specification, never a failed test. Existing issues are not migrated. `.github/ISSUE_TEMPLATE/work-item.md` supplies the grammar for new work. IDs are `AC-n` in order, scoped to the execution-plan revision and content hash. -Plane and Jira return `not_evaluable` with reason `criteria reader unavailable`, -distinct from parsed criteria that remain unmet; neither counts as satisfied. +Plane and Jira return `not_evaluable` with reason `criteria reader unavailable`. +Their cycles stop as `BLOCKED` with `stop_reason: readiness_unconfirmed` before +any agent dispatch, regardless of issue-review mode or difficulty. Editing the +work item cannot clear this stop: a provider reader must become available +(Plane reading belongs to #238-A; Jira needs its own reader). This differs from +parsed criteria needing refinement; neither unavailable nor unmet criteria +count as satisfied. For one release, omitted `tests.version` or version 1 accepts `tests.evidence`. The compatibility interval starts with the first published release supporting @@ -240,6 +249,11 @@ reports retain their historical evidence classification. Version 2 evidence is normalized into a private compatibility view for `tests_basis`, `boundary_verified`, telemetry and stats; this view is not a second result contract. Malformed or unsupported reports cap at `not_verified`. +Stages that omit `tests` or report `tests.reason: no_code_change` without outcome +fields record no new test outcome and preserve the previous cycle-level outcome. +Explicit reports (including `checks`, `evidence`, `conclusion`, `passed` or `ran`) +still receive the deterministic caps, even with `no_code_change`; a required +boundary run can still be recorded as missing for completed code work. Plan persistence, the final verification gate and verification-only resume are deferred to #329; see [Execution plan](execution-plan.md). CI substitution is diff --git a/scripts/run_cycle.py b/scripts/run_cycle.py index 4986d71..6d3b25c 100644 --- a/scripts/run_cycle.py +++ b/scripts/run_cycle.py @@ -2888,8 +2888,21 @@ def run(role: str, instruction: str = "", *, redispatched = any(stage.role == role for stage in report.stages) journal_context = (journal.summary() if redispatched or journal.has_history() else "") + verification_context = "" + if (role in {"implement", "resolve", "rereview"} + and verification_plan is not None and verification_plan.checks): + verification_context = ( + "\nRuntime frozen verification checks (data): " + + json.dumps([check.to_dict() for check in verification_plan.checks], + ensure_ascii=False) + + "\nReuse these identities, including executor/profile, when reporting " + "a result for an existing check, even when this stage's dispatch differs. " + "The identity names the check's owning execution context. " + "A check that needs another context requires a new ID; it does not " + "replace a required check. Additions and obligation promotions are allowed.\n") outcome = recorder.stage( - role, compose(role, repo_id, task_id, instruction + criteria_context, + role, compose(role, repo_id, task_id, + instruction + criteria_context + verification_context, change_request_id=change_request_id, local_only=local_only, evidence=evidence, worktree_dir=worktree_dir, @@ -3961,6 +3974,11 @@ def edit_comment(comment_id: str, body: str) -> None: except verification_contract.VerificationError as error: return stop(str(error), "BLOCKED", stop_reason="readiness_unconfirmed") verification_plan = verification_contract.VerificationPlan(criteria) + if criteria.status == "not_evaluable": + return stop( + f"acceptance criteria not_evaluable for {criteria.provider}: " + f"{criteria.reason}; a criteria reader is required before implementation", + "BLOCKED", stop_reason="readiness_unconfirmed") if criteria.status != "evaluable": return stop( f"acceptance criteria {criteria.status}: {criteria.reason}; refine the work item", @@ -4532,6 +4550,11 @@ def _normalize_test_payload( ) -> tuple[dict[str, Any], verification_contract.VerificationPlan | None]: payload = dict(payload or {}) tests = payload.get("tests") + # A stage with no verification report must not replace the cycle's previous + # outcome. Explicit reports still receive the plan's deterministic caps. + if tests is None or (isinstance(tests, dict) and tests.get("reason") == "no_code_change" + and not {"checks", "evidence", "conclusion", "passed", "ran"} & tests.keys()): + return payload, plan tests = tests if isinstance(tests, dict) else {} try: normalized, updated = verification_contract.normalize_tests( diff --git a/skills/cc-implement-issue/SKILL.md b/skills/cc-implement-issue/SKILL.md index ab5087f..4d9aa65 100644 --- a/skills/cc-implement-issue/SKILL.md +++ b/skills/cc-implement-issue/SKILL.md @@ -18,8 +18,12 @@ project commands. Each check carries `id`, `command`, `executor`, `profile`, `context`, `required`, `criteria` (criterion IDs), `level`, `provenance` and `reason`. The configured test uses `id: configured-test`, `context: project`, `provenance: project`, `reason: configured test command`, and the dispatched -executor/profile. Every check linked to a criterion is required. Add or -promote checks; never remove or demote frozen obligations. +executor/profile when first introduced. For an existing check, copy the runtime's +frozen identity, including its owning executor/profile, even when this dispatch +uses another executor/profile. A different required execution context needs a +new check ID and cannot replace an existing obligation. Every check linked to a +criterion is required. Add or promote checks; never remove or demote frozen +obligations. Each result also carries `head_sha`, `result: pass|fail|unverified`, `evidence` (integer `exit_code`, `executed` count or null), `cause` and diff --git a/skills/cc-rereview/SKILL.md b/skills/cc-rereview/SKILL.md index aeb3dc3..9d03d58 100644 --- a/skills/cc-rereview/SKILL.md +++ b/skills/cc-rereview/SKILL.md @@ -18,8 +18,12 @@ project commands. Each check carries `id`, `command`, `executor`, `profile`, `context`, `required`, `criteria` (criterion IDs), `level`, `provenance` and `reason`. The configured test uses `id: configured-test`, `context: project`, `provenance: project`, `reason: configured test command`, and the dispatched -executor/profile. Every check linked to a criterion is required. Add or -promote checks; never remove or demote frozen obligations. +executor/profile when first introduced. For an existing check, copy the runtime's +frozen identity, including its owning executor/profile, even when this dispatch +uses another executor/profile. A different required execution context needs a +new check ID and cannot replace an existing obligation. Every check linked to a +criterion is required. Add or promote checks; never remove or demote frozen +obligations. Each result also carries `head_sha`, `result: pass|fail|unverified`, `evidence` (integer `exit_code`, `executed` count or null), `cause` and diff --git a/skills/cc-resolve-comments/SKILL.md b/skills/cc-resolve-comments/SKILL.md index 2363f69..9c568e9 100644 --- a/skills/cc-resolve-comments/SKILL.md +++ b/skills/cc-resolve-comments/SKILL.md @@ -18,8 +18,12 @@ project commands. Each check carries `id`, `command`, `executor`, `profile`, `context`, `required`, `criteria` (criterion IDs), `level`, `provenance` and `reason`. The configured test uses `id: configured-test`, `context: project`, `provenance: project`, `reason: configured test command`, and the dispatched -executor/profile. Every check linked to a criterion is required. Add or -promote checks; never remove or demote frozen obligations. +executor/profile when first introduced. For an existing check, copy the runtime's +frozen identity, including its owning executor/profile, even when this dispatch +uses another executor/profile. A different required execution context needs a +new check ID and cannot replace an existing obligation. Every check linked to a +criterion is required. Add or promote checks; never remove or demote frozen +obligations. Each result also carries `head_sha`, `result: pass|fail|unverified`, `evidence` (integer `exit_code`, `executed` count or null), `cause` and diff --git a/skills/cc-verify/SKILL.md b/skills/cc-verify/SKILL.md index 7786381..db6b661 100644 --- a/skills/cc-verify/SKILL.md +++ b/skills/cc-verify/SKILL.md @@ -18,8 +18,12 @@ project commands. Each check carries `id`, `command`, `executor`, `profile`, `context`, `required`, `criteria` (criterion IDs), `level`, `provenance` and `reason`. The configured test uses `id: configured-test`, `context: project`, `provenance: project`, `reason: configured test command`, and the dispatched -executor/profile. Every check linked to a criterion is required. Add or -promote checks; never remove or demote frozen obligations. +executor/profile when first introduced. For an existing check, copy the runtime's +frozen identity, including its owning executor/profile, even when this dispatch +uses another executor/profile. A different required execution context needs a +new check ID and cannot replace an existing obligation. Every check linked to a +criterion is required. Add or promote checks; never remove or demote frozen +obligations. Each result also carries `head_sha`, `result: pass|fail|unverified`, `evidence` (integer `exit_code`, `executed` count or null), `cause` and diff --git a/tests/test_verification_plan.py b/tests/test_verification_plan.py index 31ded39..f2bcaa8 100644 --- a/tests/test_verification_plan.py +++ b/tests/test_verification_plan.py @@ -18,7 +18,7 @@ import run_cycle as rc import stats import verification_plan as vp -from test_run_cycle import RunCycleTestCase, Talker, block +from test_run_cycle import RunCycleTestCase, Talker, Sequence, block HEAD = "a" * 40 OTHER = "b" * 40 @@ -157,6 +157,26 @@ def test_execution_plan_versions_and_owner_revision(self): class ResultTests(unittest.TestCase): + def test_no_test_report_preserves_plan_and_does_not_invent_an_outcome(self): + plan = vp.VerificationPlan(criteria()).extend((check(),)).record(result()) + for omitted in ({}, {"tests": {"reason": "no_code_change"}}, + {"tests": {"version": 2, "reason": "no_code_change"}}): + with self.subTest(payload=omitted): + original = {"status": "RESOLVED", "head_sha": HEAD, **omitted} + payload, updated = rc._normalize_test_payload(original, plan) + self.assertEqual(payload, original) + self.assertEqual(updated, plan) + self.assertEqual(rc._legacy_tests(payload), {}) + + def test_no_code_change_cannot_hide_an_explicit_required_failure(self): + plan = vp.VerificationPlan(criteria()).extend((check(),)) + payload, updated = rc._normalize_test_payload({"status": "RESOLVED", "head_sha": HEAD, + "tests": {"version": 2, "reason": "no_code_change", "conclusion": "verified", + "passed": True, "checks": [report("fail")]}}, plan) + self.assertEqual(updated.current("check-1", HEAD), "fail") + self.assertEqual(rc._legacy_tests(payload)["verification"], "failed") + self.assertFalse(rc._legacy_tests(payload)["tests_passed"]) + def test_v2_records_caps_and_boundary_without_touching_host_checks(self): payload = {"status": "IMPLEMENTED", "head_sha": HEAD, "tests": { "version": 2, "conclusion": "verified", "criteria": criteria().to_dict(), @@ -231,11 +251,15 @@ def test_gate_runs_for_every_issue_review_mode_and_difficulty_before_dispatch(se availability={"codex": ex.Availability.READY, "claude": ex.Availability.READY}, work_item_provider=provider, work_item_repository="owner/issues", issue_review=mode, criteria_reader=reader) - self.assertEqual(report_.status, "HUMAN_INTERVENTION") + self.assertEqual(report_.status, "HUMAN_INTERVENTION" if provider == "github" else "BLOCKED") self.assertIn("acceptance criteria", report_.stopped_because) self.assertEqual(implementer.dispatched, []) reader.assert_called_once() - self.assertEqual(self.rows()[-1]["payload"]["stop_reason"], "needs_refinement") + self.assertEqual(self.rows()[-1]["payload"]["stop_reason"], + "needs_refinement" if provider == "github" else "readiness_unconfirmed") + if provider != "github": + self.assertIn("criteria reader unavailable", report_.stopped_because) + self.assertNotIn("refine the work item", report_.stopped_because) self.assertNotIn("verification", self.rows()[-1]["payload"]) def test_valid_criteria_read_once_and_context_reaches_implementation(self): @@ -269,6 +293,67 @@ def test_reported_passes_keep_their_basis_through_telemetry_and_stats(self): aggregate = stats.aggregate(self.rows(), repo_id="owner/api", days=None) self.assertEqual(aggregate["summary"]["verification"]["by_basis"]["agent_reported"]["passed"], 1) + def test_no_test_resolution_and_rereview_keep_implementation_outcome(self): + for tests in (None, {"reason": "no_code_change"}): + with self.subTest(tests=tests): + initial = {"version": 2, "conclusion": "verified", "checks": [report(), { + **vp.configured_check(check().command, "codex", "cheap_coder").to_dict(), + **result(id="configured-test").to_dict()}]} + resolution = {} if tests is None else {"tests": tests} + implementer = Sequence("codex", [ + block("IMPLEMENTED", head_sha=HEAD, tests=initial), + block("RESOLVED", head_sha=HEAD, **resolution)]) + reviewer = Sequence("claude", [block("CHANGES_REQUESTED"), + block("APPROVED", head_sha=HEAD, **resolution)]) + run = self.run_cycle(implementer, reviewer, allowed_test_command=check().command, + work_item_provider="github", work_item_repository="owner/issues") + self.assertEqual([stage.role for stage in run.stages], + ["implement", "review", "resolve", "rereview"]) + verdicts = [row["payload"] for row in self.rows() + if row["payload"].get("record_kind") == "verdict" + and row["role"] in {"resolve", "rereview"}] + for verdict in verdicts: + self.assertNotIn("verification", verdict) + self.assertNotIn("tests_passed", verdict) + closing = self.rows()[-1]["payload"] + self.assertEqual(closing["verification"], "verified") + self.assertTrue(closing["tests_passed"]) + + def test_later_profiles_receive_and_reuse_frozen_check_identity(self): + configured = replace(vp.configured_check(check().command, "codex", "cheap_coder"), + criteria=("AC-1",)) + tests = {"version": 2, "conclusion": "verified", "checks": [ + {**configured.to_dict(), **result(id="configured-test").to_dict()}]} + prompts = [] + + class FollowingReporter(Talker): + def spoken(self, task): + if "cc-implement-issue" in task: + return block("IMPLEMENTED", head_sha=HEAD, tests=tests) + if "cc-initial-review" in task: + return block("CHANGES_REQUESTED") + prompts.append(task) + marker = "Runtime frozen verification checks (data): " + frozen = json.loads(task.split(marker, 1)[1].splitlines()[0]) + checks = [{**item, **result(id=item["id"]).to_dict()} for item in frozen] + return block("RESOLVED" if "cc-resolve-comments" in task else "APPROVED", + head_sha=HEAD, tests={"version": 2, "conclusion": "verified", "checks": checks}) + + run = self.run_cycle(FollowingReporter("codex"), FollowingReporter("claude"), + allowed_test_command=check().command, work_item_provider="github", + work_item_repository="owner/issues") + self.assertEqual([stage.role for stage in run.stages], + ["implement", "review", "resolve", "rereview"]) + self.assertNotEqual(run.stages[0].decision.profile, run.stages[-1].decision.profile) + assert run.stages[0].result is not None + assert run.stages[-1].result is not None + self.assertNotEqual(run.stages[0].result.executor, run.stages[-1].result.executor) + self.assertEqual(len(prompts), 2) + for row in self.rows(): + if row["payload"].get("record_kind") == "verdict" and row["role"] in {"resolve", "rereview"}: + self.assertEqual(row["payload"]["verification"], "verified") + self.assertTrue(row["payload"]["tests_passed"]) + if __name__ == "__main__": unittest.main()