From 34b5a0ccbc793af791e393549174563002f563ee Mon Sep 17 00:00:00 2001 From: datacas <4690232+datacas@users.noreply.github.com> Date: Fri, 30 Oct 2026 16:59:06 +0100 Subject: [PATCH 1/2] feat(runtime): execute native cycles from policy plans --- docs/configuration.md | 10 +- docs/execution-plan.md | 38 ++- docs/review-cycle.md | 11 + docs/telemetry.md | 13 + docs/verification.md | 32 +- docs/workflows.md | 16 +- scripts/cycle_cli.py | 5 +- scripts/cycle_dispatch.py | 21 +- scripts/cycle_history.py | 128 +++++++- scripts/cycle_loop.py | 55 +++- scripts/cycle_plan.py | 489 +++++++++++++++++++++++++++++++ scripts/cycle_session.py | 20 +- scripts/review_contract.py | 9 + scripts/run_cycle.py | 24 +- scripts/runtime.manifest | 1 + scripts/stage_permissions.py | 4 +- scripts/telemetry.py | 20 +- scripts/verification_plan.py | 16 +- tests/fakes/fake_agent.py | 4 + tests/test_execution_profiles.py | 15 +- tests/test_installed_cycle.py | 22 +- tests/test_run_cycle.py | 230 +++++++++++++++ tests/test_verification_plan.py | 4 +- tests/test_work_item_outcomes.py | 2 +- 24 files changed, 1124 insertions(+), 65 deletions(-) create mode 100644 scripts/cycle_plan.py diff --git a/docs/configuration.md b/docs/configuration.md index 6d96abc..b43a86a 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -224,7 +224,15 @@ An objective adds obligations: publishing a `prototype` with `--objective merge_ The configurable stages are `issue_review`, `implement`, `verify`, `review`, `security`, `resolve`, and `rereview`. `ci` and `merge` are controls, not stages: `--stage ci=…` is refused, and nothing makes a merge automatic. An unknown key, stage, profile, objective, or policy stops the run by name. -**Before plan-driven execution (#329).** The driver follows the plan only from #329 on. Until then, a run without `--dry-run` refuses, before any effect, any execution configuration whose behaviour differs from `standard`, naming the setting and its source. No new configuration, or an explicit configuration that resolves to `standard` (including `custom` without a difference, `issue_review` off, and `local_change` through the local-only run), runs the current flow; `local_change` from `--objective` or the file applies the local-only run's own requirements (`--cwd` linked worktree, `--workspace current`). A requested profile is never silently replaced by `standard`. `--dry-run` shows the plan for every profile and override. +The native driver executes the resolved plan for all five profiles. A requested +profile is never replaced by `standard`. Stage overrides cannot remove mandatory +checks, security requirements, publication guarantees or human review. +`local_change` from `--objective` or the file applies the local-only workspace +requirements (`--cwd` linked worktree and `--workspace current`) and stops before +merge readiness. `verified_change` stops after verification and a clean review; +`merge_ready` also requires the reviewed SHA's CI gate. `--dry-run` prints the +same plan without effects. No additional configuration key or resume start point +is needed for verification-only recovery; see [Verification](verification.md). ### Issue review diff --git a/docs/execution-plan.md b/docs/execution-plan.md index c1d95c4..46aa7c7 100644 --- a/docs/execution-plan.md +++ b/docs/execution-plan.md @@ -115,16 +115,42 @@ 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 native driver consumes the plan before dispatch. It resolves conditions +from the observed diff and review outcome, records revisions after stage results +and HEAD changes, and refuses a disabled stage when a mandatory obligation +applies. Agent reports can add checks or request security; they cannot withdraw +requirements. Security remains embedded in review and verification in code work. + +Schema 25 stores append-only revisions in `cycle_execution_plans` in the history +database. The version-1 metadata projection removes prompts, criterion prose, +reasoning, commands and output. Commands become SHA-256 bindings; criteria become +stable `AC-n` identities. Check histories retain HEAD, results, closed cause +tokens and evidence fingerprints. Canonical JSON uses sorted keys, compact +separators and ASCII escaping; its SHA-256 is `digest`. Every stage receives +`plan_id`, `revision` and `digest` for its schema-2 receipt. A receipt supplements +the existing attempt, head and `REV-*` contracts; it grants no execution rights. + +On resume the runtime reconciles the current configuration, source criteria, +remote receipt and local revisions. Required checks are retained across HEAD +changes; results from another SHA cannot satisfy them. Changed criteria add new +identities rather than reusing old coverage. Command hashes can be rebound only +to currently authorized exact commands. Without recoverable local history the +runtime replans with `partial` provenance and accepts no prior check as passed. +This is partial recovery, not a fully verified continuation. Analysis such as +#127 can read this metadata without reading prompts. + +The work-item journal may retain redacted command proposals, separately from +the metadata-only history projection. A resume can use a proposal to recover a +binding only after the current configured verification policy authorizes it +and its hash matches the frozen check. Journal text never grants permission. +If redaction or cleanup removed the command, the check remains unsatisfied until +an authorized binding is supplied; it is never dropped. 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 execution profile, objective, and stage policies described in [Configuration → Execution profiles](configuration.md#execution-profiles); -`run_cycle.py --dry-run [--json]` prints it. The driver follows the plan from -#329 on; routing, metrics, and plugin loading stay out of this contract. #330 +`run_cycle.py --dry-run [--json]` prints it. Routing, metrics, and plugin loading +stay out of this contract. #330 can consume the same contract in supervised Orca cycles. diff --git a/docs/review-cycle.md b/docs/review-cycle.md index 2ee0f57..61f035d 100644 --- a/docs/review-cycle.md +++ b/docs/review-cycle.md @@ -1,5 +1,16 @@ # Review lifecycle +Native cycles select stages from the current execution plan. Resolution remains +bounded by the existing iteration budget. After a resolution, identical explicit +review and resolution HEADs stop with `no_progress`: open findings retain their +blocks, and no rereview runs, including under Full. A changed HEAD invalidates +incompatible review and verification evidence and activates the applicable +controls again. A clean trusted receipt for the unchanged current HEAD can be +reused on resume under either CI policy. Final readiness still requires valid +`REV-*` history, required verification and the checks of that SHA; merge remains +a human action. Verification-only recovery uses `cc-verify`, not another review; +see [Verification](verification.md). + How findings are created, tracked across rounds, and closed, and why the pull-request comment is the record the next run reads back. **On this page:** [Issue review before implementation](#issue-review-before-implementation) · [The loop](#the-loop) · [Resume after a failed upload](#resume-after-a-failed-upload) · [CI policy](#ci-policy) · [Anatomy of a review comment](#anatomy-of-a-review-comment) · [Finding fields](#finding-fields) · [Trusted authors](#trusted-authors) · [Security triage](#security-triage) · [Structured result](#the-structured-result-is-opt-in) · [Exit conditions](#exit-conditions) diff --git a/docs/telemetry.md b/docs/telemetry.md index bedbf3e..0422e99 100644 --- a/docs/telemetry.md +++ b/docs/telemetry.md @@ -1,5 +1,18 @@ # Telemetry and `cc-stats` +Schema 25 adds `verification_unverified` to the closed stop reasons and the +append-only `cycle_execution_plans` history table. Each revision records cycle, +repository, work item, change request, plan identity, revision, current HEAD, +canonical digest and provenance (`initial`, `observed`, `recovered`, `partial`, +`stopped`), plus a versioned metadata projection. Existing tables and history +remain intact. The projection excludes prompts, criterion prose, command text +and command output; it retains command/output/test/environment hashes, check +identities, required flags, criterion links, results and cause tokens. Each +check can retain results for several SHAs. Generic CI success does not update +those results. Journal events correlate plan ID, revision and digest with the +cycle. Published schema-2 receipts carry the same three references; see +[Execution plan](execution-plan.md) for canonicalization and recovery rules. + When the runtime drives a cycle, every stage writes rows to a local SQLite database. `cc-stats` turns those rows into a per-repository report. This page covers what is recorded, what never is, and how to read it. **On this page:** [Production delivery events](#production-delivery-events-335) · [When rows are written](#when-rows-are-written) · [Where it lives](#where-it-lives) · [What is recorded](#what-is-recorded) · [Work-item outcomes](#work-item-outcomes) · [What is never recorded](#what-is-never-recorded) · [What it is used for](#what-it-is-used-for) · [Reading it with cc-stats](#reading-it-with-cc-stats) · [Limits](#limits) diff --git a/docs/verification.md b/docs/verification.md index ac76594..e253050 100644 --- a/docs/verification.md +++ b/docs/verification.md @@ -284,9 +284,35 @@ 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 -out of scope: `CI_PASSED` does not turn a local unverified result into pass. +The native driver's final barrier prevents `READY_FOR_MANUAL_MERGE` while a +required check is `unverified`, criteria remain uncovered, or required security +verification was omitted. It stops once with `HUMAN_INTERVENTION` and +`stop_reason: verification_unverified`. Failed checks retain their failure; +blocked, omitted, missing-tool, inconclusive and environment causes remain +distinguishable metadata. `CI_PASSED` never turns an unverified local check into +pass. See [Execution plan](execution-plan.md) for persistence and receipt identity. + +Resume with `--from rereview --pr `. When the current HEAD, recoverable plan +and trusted receipt agree on a verification stop, the runtime dispatches only +`cc-verify`, once, in a detached disposable clone without remotes. It passes only +pending required checks and retains passing results. It does not dispatch the +rereview agent or republish a stage comment. A still-unavailable environment +stops again without a retry loop. A moved HEAD, changed criteria, unbound command +hash or unrecoverable plan takes ordinary reconciliation instead. This does not +add a CLI start point or change `START_ORDER`/`CYCLE_STARTS`. + +Hotfix requires `hotfix-reproduction` and `hotfix-regression`, both `required`, +with distinct focused commands proposed by the implementer. The configured +general suite satisfies neither identity. A missing proposal stops with +`needs_refinement`; a defined check that cannot execute stays `unverified`. +The implementer commits the reproduction before the fix. Under the reproduction +check's `history`, it reports the defect failure's SHA, command, environment and +`cause: defect`, preserving its output artifact. Evidence carries `test_sha256`, +`context_sha256` and `output_sha256`; the passing result on the fixed HEAD must +have the same test and environment fingerprints. An import or environment +failure is not a defect reproduction. Multiple results share one check ID; +only evidence for the current HEAD determines its current result. Changing the +test requires a new justified and revalidated proof, not replacing the old one. ## Evidence levels diff --git a/docs/workflows.md b/docs/workflows.md index 7e03e1d..a027161 100644 --- a/docs/workflows.md +++ b/docs/workflows.md @@ -295,7 +295,21 @@ From a toolkit checkout: `python3 scripts/run_cycle.py …` **Requires:** the runtime and an authenticated Codex and/or Claude CLI. For publishing stages, it also needs `gh` authenticated with push access, Codex CLI 0.138.0 or later, and a working tree on a named branch. Before any publishing dispatch, a readiness check verifies this and records `missing_capability=publication_access` when it fails. -**Dry run:** `--dry-run` resolves the execution profile, objective, and stage policies and prints the plan without selecting a worktree, opening the telemetry database, preparing dependencies, asking for a grant, or starting an agent. Conditions that need the future change stay `pending`. A conflict, such as `--issue-review off` under `full`, is refused before anything runs. Until the driver follows the plan (#329), a run without `--dry-run` refuses any execution configuration whose behaviour differs from `standard`, naming each setting and its source. See [Execution profiles](configuration.md#execution-profiles). +**Dry run:** `--dry-run` resolves the execution profile, objective, and stage +policies and prints the plan without selecting a worktree, opening the telemetry +database, preparing dependencies, asking for a grant, or starting an agent. +Conditions that need the future change stay `pending`. A conflict, such as +`--issue-review off` under `full`, is refused before anything runs. Live native +cycles consume the same plan and record revisions as those conditions become +known. See [Execution profiles](configuration.md#execution-profiles). + +A required unverified check stops the cycle with `verification_unverified`. +`--from rereview --pr ` recovers the plan and, when HEAD and receipt match, +runs only pending required checks through one isolated `cc-verify` dispatch. +It reuses an unchanged clean review. It does not repeat publications or loop +when the environment still cannot verify. Changed inputs use ordinary plan +reconciliation. Hotfix additionally requires test-first reproduction and focused +regression proof; see [Verification](verification.md). **Local-only rehearsal:** `--cwd /path/to/linked-worktree --workspace current --local-only` refuses the live repository. The explicit `current` policy is required so workspace selection cannot create another task worktree. It runs implementation only, adds a no-publish boundary to the prompt, marks each row `local_only`, and ends with `HUMAN_INTERVENTION` because there is no PR to review. It is a policy, not a network sandbox. diff --git a/scripts/cycle_cli.py b/scripts/cycle_cli.py index 0376613..70113d1 100644 --- a/scripts/cycle_cli.py +++ b/scripts/cycle_cli.py @@ -123,8 +123,7 @@ def main(api: Any, argv: list[str] | None = None) -> int: repo, profiles, routing_strategy = api.plan_with_strategy(args) config = api.resolve_config(args, report=print) execution = api.execution_of(args, config) - api.execution_plan_of(args, config, repo, provenance="run") - api.check_current_flow(execution) + execution_plan = api.execution_plan_of(args, config, repo, provenance="run") if execution.objective.value == "local_change" and not args.local_only: # The current flow reaches local_change only as a local-only run, # with the same explicit worktree it requires. @@ -258,6 +257,8 @@ def main(api: Any, argv: list[str] | None = None) -> int: ci_timeout=ci_timeout, repair_approval=args.repair_approval, consent=consent, + execution_plan=execution_plan, + security_rule=api.GateRule.from_config(config), ) except (api.CycleDriverError, api.GrantError) as error: parser.error(str(error)) diff --git a/scripts/cycle_dispatch.py b/scripts/cycle_dispatch.py index 40b9055..10263e5 100644 --- a/scripts/cycle_dispatch.py +++ b/scripts/cycle_dispatch.py @@ -315,8 +315,13 @@ def review_issue(self: Any) -> CycleReport | None: """ escalated = False while True: + try: + self.select_plan_stage(self.api.ISSUE_REVIEW_ROLE) + except ValueError as error: + return self.stop(str(error), "BLOCKED") outcome, reported, tampered, _ = self.run( - self.api.ISSUE_REVIEW_ROLE, self.api.ESCALATED_ISSUE_REVIEW if escalated else "", + self.api.ISSUE_REVIEW_ROLE, + (self.api.ESCALATED_ISSUE_REVIEW if escalated else "") + self.plan_context(), escalated=escalated) stopped = self.unfinished(outcome, reported, tampered) if stopped is not None: @@ -324,6 +329,7 @@ def review_issue(self: Any) -> CycleReport | None: readiness = self.api.assess_readiness(reported.payload, self.task_id) self.report.readiness = readiness if reported.status: + self.record_plan_stage(self.api.ISSUE_REVIEW_ROLE, reported, outcome) self.recorder.record_verdict(self.api.ISSUE_REVIEW_ROLE, reported.status, **readiness.telemetry_fields()) if readiness.gate == self.api.READINESS_CONTINUE: @@ -391,6 +397,11 @@ def advance(self: Any, role: str, instruction: str = "", *, component="runtime", status="worktree_synchronized", reason="clean worktree fast-forwarded to the open pull request head", correlation={"old_head_sha": old_head, "new_head_sha": new_head}) + try: + self.select_plan_stage(role) + except ValueError as error: + return self.api.Reported(), self.stop(str(error), "BLOCKED") + instruction += self.plan_context() audited = None if self.publication_auditor is not None and role in self.api.PUBLISHING_ROLES: try: @@ -523,6 +534,10 @@ def advance(self: Any, role: str, instruction: str = "", *, outcome.decision.profile if outcome.decision else "unknown"),)) test_payload, self.state.verification_plan = self.api._normalize_test_payload( test_payload, self.state.verification_plan) + try: + self.record_plan_stage(role, reported, outcome, test_payload) + except ValueError as error: + return reported, self.stop(str(error), "BLOCKED", reported=reported) pull_request_id = ( reported.telemetry_pull_request_id(self.code_host) if role == "implement" else None @@ -547,6 +562,10 @@ def advance(self: Any, role: str, instruction: str = "", *, return reported, self.stop(because, "BLOCKED" if (reported.payload or {}).get("publication_pending") is True else self.api.UNRESOLVED_END, reported=reported) + if (role in {"review", "rereview"} and self.execution_plan.stage("security").selected + and self.plan_managed and (reported.payload or {}).get("security_audit_ran") is not True): + return reported, self.stop("the required security audit was omitted or unverified", + reported=reported, stop_reason="verification_unverified") if recover and role in self.api.COMMENT_STAGES and change_request_id: stopped = self.after_publication( role, change_request_id, diff --git a/scripts/cycle_history.py b/scripts/cycle_history.py index ce19b2d..2db8ebe 100644 --- a/scripts/cycle_history.py +++ b/scripts/cycle_history.py @@ -34,6 +34,7 @@ from __future__ import annotations import hashlib +import json import os import re import sqlite3 @@ -42,7 +43,10 @@ from dataclasses import dataclass from datetime import datetime, timedelta, timezone from pathlib import Path -from typing import Iterable, Protocol +from typing import Any, Iterable, Protocol, TYPE_CHECKING + +if TYPE_CHECKING: + from .execution_plan import ExecutionPlan from review_contract import ( CLAIM_STATUSES, @@ -214,6 +218,82 @@ def _process_alive(pid: int) -> bool | None: return True +def command_digest(command: str) -> str: + if re.fullmatch(r"sha256:[a-f0-9]{64}", command): + return command + return "sha256:" + hashlib.sha256(command.encode()).hexdigest() + + +def plan_digest(payload: dict[str, Any]) -> str: + return hashlib.sha256(json.dumps(payload, sort_keys=True, separators=(",", ":"), + ensure_ascii=True).encode()).hexdigest() + + +def plan_projection(plan: ExecutionPlan) -> dict[str, Any]: + """Versioned metadata, without criterion prose, commands, output or prompts.""" + def clean(value, key=""): + if key in {"reason", "condition", "source", "policy_source", "provenance", + "plan_provenance"}: + return "runtime" + if key == "command": + return command_digest(value) + if key in {"id", "executor", "profile", "context"} and isinstance(value, str): + return (value if re.fullmatch(r"[A-Za-z0-9._:-]{1,128}", value) + else "ref-" + hashlib.sha256(value.encode()).hexdigest()) + if key == "items": + return [f"AC-{i}" for i in range(1, len(value) + 1)] + if key == "cause": + if value in {"defect", "blocked", "omitted", "missing_tool", "inconclusive", + "environment", "stale", "unverified", ""}: + return value + lowered = value.lower() + for token, phrases in {"omitted": ("no execution result", "omitted"), + "missing_tool": ("not found", "missing tool"), + "blocked": ("denied", "blocked", "permission"), + "inconclusive": ("inconclusive",), + "environment": ("sandbox", "environment",)}.items(): + if any(phrase in lowered for phrase in phrases): + return token + return "unverified" + if key == "metadata": + conclusion = value.get("verification_conclusion") + return ({"verification_conclusion": conclusion} if conclusion in { + "verified", "verified_with_reservations", "not_verified", "failed"} else {}) + if key in {"attempts", "review"}: + return [] if key == "attempts" else {} + if key == "evidence": + return [{k: clean(v, k) for k, v in item.items() + if k in {"command", "level", "exit_code", "executed", "test_sha256", + "output_sha256", "context_sha256"}} for item in value] + if isinstance(value, dict): + return {k: clean(v, k) for k, v in value.items()} + if isinstance(value, (list, tuple)): + return [clean(v) for v in value] + return value + return {"projection_version": 1, "plan": clean(plan.to_dict())} + + +def restore_plan(record: dict[str, Any], commands: tuple[str, ...]) -> ExecutionPlan: + """Recover only checks whose command is still explicitly authorized.""" + if not TYPE_CHECKING: + from execution_plan import ExecutionPlan + payload = record["payload"] + if payload.get("projection_version") != 1 or plan_digest(payload) != record["digest"]: + raise HistoryError("unsupported or corrupt execution plan") + bindings = {command_digest(command): command for command in commands} + def bind(value, key=""): + if key == "command": + if value not in bindings: + raise HistoryError("stored check has no currently authorized command binding") + return bindings[value] + if isinstance(value, dict): + return {k: bind(v, k) for k, v in value.items()} + if isinstance(value, list): + return [bind(v) for v in value] + return value + return ExecutionPlan.from_dict(bind(payload["plan"])) + + class CycleHistoryStore: """The cycle-history tables of a telemetry database. @@ -229,6 +309,52 @@ def _connect(self) -> sqlite3.Connection: connection.row_factory = sqlite3.Row return connection + def save_plan(self, plan: ExecutionPlan, *, cycle_id: str, change_request_id: str | None, + provenance: str, stop_reason: str | None = None) -> dict[str, Any]: + """Append a privacy-safe revision. Commands and prose stay out of SQLite. + + Digests use canonical JSON projection version 1, not a prompt or a + platform-dependent rendering. Rebinding command hashes on recovery + requires a currently authorized command; history grants no execution. + """ + if provenance not in {"initial", "observed", "recovered", "partial", "stopped"}: + raise HistoryError("unknown plan provenance") + payload = plan_projection(plan) + digest = plan_digest(payload) + with closing(self._connect()) as connection: + connection.execute( + "INSERT INTO cycle_execution_plans " + "(repo_id,task_id,change_request_id,cycle_id,plan_id,revision,digest," + "head_sha,provenance,stop_reason,payload) VALUES (?,?,?,?,?,?,?,?,?,?,?)", + (plan.repository, plan.work_item, change_request_id, cycle_id, + plan.plan_id, plan.revision, digest, plan.head_sha, provenance, + stop_reason, json.dumps(payload, sort_keys=True, separators=(",", ":")))) + connection.commit() + return {"plan_id": plan.plan_id, "revision": plan.revision, "digest": digest} + + def latest_plan(self, repo_id: str, task_id: str, change_request_id: str | None) -> dict[str, Any] | None: + with closing(self._connect()) as connection: + row = connection.execute( + "SELECT * FROM cycle_execution_plans WHERE repo_id=? AND task_id=? " + "AND change_request_id IS ? ORDER BY sequence DESC LIMIT 1", + (repo_id, task_id, change_request_id)).fetchone() + if row is None: + return None + value = dict(row) + value["payload"] = json.loads(value["payload"]) + if plan_digest(value["payload"]) != value["digest"]: + raise HistoryError("execution plan digest mismatch") + return value + + def has_plan_receipt(self, repo_id: str, receipt: dict[str, Any]) -> bool: + with closing(self._connect()) as connection: + row = connection.execute( + "SELECT 1 FROM cycle_execution_plans WHERE repo_id=? AND plan_id=? " + "AND revision=? AND digest=? AND head_sha=?", + (repo_id, receipt.get("plan_id"), receipt.get("revision"), + receipt.get("digest"), receipt.get("head_sha"))).fetchone() + return row is not None + # -- writer lock ------------------------------------------------------- def acquire(self, repo_id: str, change_request_id: str) -> str: diff --git a/scripts/cycle_loop.py b/scripts/cycle_loop.py index 38b0f4c..dd3f6f2 100644 --- a/scripts/cycle_loop.py +++ b/scripts/cycle_loop.py @@ -3,6 +3,9 @@ from typing import TYPE_CHECKING, Any if TYPE_CHECKING: from .run_cycle import CycleReport, ci_gate + from .cycle_plan import hotfix_problem +else: + from cycle_plan import hotfix_problem from dataclasses import dataclass, field from enum import Enum @@ -15,6 +18,7 @@ class ControlState(str, Enum): RESOLVE = "resolve" REREVIEW = "rereview" CI = "ci" + VERIFY = "verify" STOPPED = "stopped" INTERRUPTED = "interrupted" @@ -31,9 +35,10 @@ class ControlState(str, Enum): ControlState.RESOLVE: {"resolve", "rereview"}, ControlState.REREVIEW: {"rereview", "resolve", "ci"}, ControlState.CI: {"ci", "resolve", "rereview"}, + ControlState.VERIFY: {"ci", "review", "rereview"}, } TRANSITIONS = { - state: {**{event: ControlState(event) for event in events}, + state: {**{event: ControlState(event) for event in events | {"verify"}}, "stop": ControlState.STOPPED, "interrupt": ControlState.INTERRUPTED} for state, events in _SUCCESSORS.items() } @@ -105,16 +110,37 @@ def edit_comment(comment_id: str, body: str) -> None: stopped = self.seed_history() if stopped is not None: return stopped + try: + self.prepare_plan() + except ValueError as error: + return self.stop(str(error), "BLOCKED", stop_reason="readiness_unconfirmed") + if self.verification_resume: + stopped = self.verify_pending() + if stopped is not None: + return stopped + clean, stopped = self.reusable_review() + if stopped is not None: + return stopped + if clean is None: + return self.stop("pending checks completed; no reusable clean review holds", + stop_reason="verification_unverified") + role, verdict, head, attempts = clean + self.report.verdict = verdict + outcome = self.gate(role, head, attempts, reported) + if isinstance(outcome, self.api.CycleReport): + return outcome + if outcome[0] == "approved": + return self.stop("", self.api.APPROVED_END, stop_reason="approved") + return self.stop("verification completed but the current head's CI is not passing", + stop_reason="ci_not_passing") if self.start_from == "implement": # Independent of issue-review mode/difficulty. Read once before any # agent iteration; provider content is data, never instructions. if self.policy_provider is not None: try: - criteria = (self.criteria_reader or self.api.verification_contract.read_criteria)( - self.policy_provider, self.work_item_repository or self.repo_id, self.task_id, cwd=self.cwd) + criteria = self.state.verification_plan.criteria except self.api.verification_contract.VerificationError as error: return self.stop(str(error), "BLOCKED", stop_reason="readiness_unconfirmed") - self.state.verification_plan = self.api.verification_contract.VerificationPlan(criteria) if criteria.status == "not_evaluable": return self.stop( f"acceptance criteria not_evaluable for {criteria.provider}: " @@ -158,6 +184,10 @@ def reader() -> str | None: f"{reported.change_request_id}, but #{published.lstrip('#')} already " "carries this work; a person decides which one continues", "BLOCKED", reported=reported) + if self.execution_plan.execution_profile == "hotfix": + if hotfix_problem(self.state.verification_plan, self.plan_head or "") == "missing_checks": + return self.stop("Hotfix requires reproduction and focused regression commands", + reported=reported, stop_reason="needs_refinement") if self.local_only: return self.stop( "local-only run stops after implementation; no change request was created for review", @@ -194,7 +224,8 @@ def reader() -> str | None: return stopped if manual is not None: return self.close_manual(manual, reported) - if self.parallel and self.start_from == "rereview": + if ((self.parallel or self.plan_managed) and self.start_from == "rereview" + and not (self.plan_managed and self.plan_partial)): clean, stopped = self.reusable_review() if stopped is not None: return stopped @@ -227,7 +258,10 @@ def reader() -> str | None: ci_failure: ci_gate.GateResult | None = None conflicting_round_done = False while True: - if self.parallel and verdict in self.api.CLEAN_REVIEW_STATUSES: + if (self.parallel or self.plan_managed) and verdict in self.api.CLEAN_REVIEW_STATUSES: + stopped = self.verification_barrier() + if stopped is not None: + return stopped # A clean review is no approval yet: the gate waits for its checks. if clean is None: head, stopped = self.clean_head(role, reported) @@ -235,6 +269,9 @@ def reader() -> str | None: return stopped clean = (role, verdict, head, set(self.state.review_attempts)) head = clean[2] + if self.execution_plan.objective == "verified_change": + return self.stop("verified_change completed with a clean review; merge is a human action", + "IN_PROGRESS", reported=reported, stop_reason="local_only") outcome = self.gate(clean[0], head, clean[3], reported) clean = None if isinstance(outcome, self.api.CycleReport): @@ -349,6 +386,7 @@ def reader() -> str | None: if (self.resolve_effort is None and count >= 1 and open_findings.get(finding) == "open") )[:16]) + review_head = (reported.payload or {}).get("head_sha") resolved, stopped = self.advance( "resolve", instruction, change_request_id=self.state.change_request_id, resolve_target=self.state.previous_resolve_target if escalation_ids else None, @@ -358,6 +396,11 @@ def reader() -> str | None: escalation_finding_ids=escalation_ids) if stopped is not None: return stopped + after_head = (resolved.payload or {}).get("head_sha") + if review_head is not None and after_head is not None and after_head == review_head: + return self.stop("resolution left the reviewed HEAD unchanged; open findings retain " + "their blocks without another review", reported=resolved, + stop_reason="no_progress") if ci_failure is not None and ci_failure.status == self.api.ci_gate.CI_CONFLICTING: conflicting_round_done = True if ci_failure is not None: diff --git a/scripts/cycle_plan.py b/scripts/cycle_plan.py new file mode 100644 index 0000000..692de1d --- /dev/null +++ b/scripts/cycle_plan.py @@ -0,0 +1,489 @@ +"""Plan revisions and verification barriers for the existing native stage loop.""" +from __future__ import annotations + +import json +import re +import hashlib +from dataclasses import replace +from typing import Any, TYPE_CHECKING + +if TYPE_CHECKING: + from .cycle_journal import events as journal_events + from .cycle_history import HistoryError, restore_plan + from .execution_plan import ContractError, ExecutionPlan, ExecutionState, Obligation, Resolution, StagePolicy + from .execution_profiles import ExecutionSettings, PlanSignals, build_plan, invocation_settings, resolve_execution + from .security_gate import GateRule, evaluate + from .stage_permissions import StagePolicy as CommandPolicy, _is_verification_command + from .verification_plan import CheckResult, VerificationError, VerificationPlan, configured_check +else: + from cycle_journal import events as journal_events + from cycle_history import HistoryError, restore_plan + from execution_plan import ContractError, ExecutionPlan, ExecutionState, Obligation, Resolution, StagePolicy + from execution_profiles import ExecutionSettings, PlanSignals, build_plan, invocation_settings, resolve_execution + from security_gate import GateRule, evaluate + from stage_permissions import StagePolicy as CommandPolicy, _is_verification_command + from verification_plan import CheckResult, VerificationError, VerificationPlan, configured_check + + +HOTFIX_IDS = ("hotfix-reproduction", "hotfix-regression") + + +def hotfix_problem(verification: VerificationPlan | None, head: str) -> str | None: + """A generic suite cannot stand in for test-first proof of a defect.""" + if verification is None or not set(HOTFIX_IDS) <= { + check.id for check in verification.checks if check.required}: + return "missing_checks" + commands = {check.id: check.command for check in verification.checks} + if (commands[HOTFIX_IDS[0]] == commands[HOTFIX_IDS[1]] + or any(commands[item] == commands.get("configured-test") for item in HOTFIX_IDS)): + return "missing_checks" + previous = [result for result in verification.results + if result.id == HOTFIX_IDS[0] and result.head_sha != head + and result.result == "fail" and result.cause == "defect"] + current = [result for result in verification.results + if result.id == HOTFIX_IDS[0] and result.head_sha == head + and result.result == "pass"] + if not previous or not current: + return "missing_proof" + before, after = previous[-1], current[-1] + # An environment/import failure and an edited reproduction are not proof. + for key in ("test_sha256", "context_sha256"): + old = {entry.get(key) for entry in before.evidence} + new = {entry.get(key) for entry in after.evidence} + if (len(old) != 1 or None in old or old != new + or any(not isinstance(value, str) or not re.fullmatch(r"[a-f0-9]{64}", value) + for value in old)): + return "incompatible_proof" + if not all(isinstance(entry.get("output_sha256"), str) + and re.fullmatch(r"[a-f0-9]{64}", entry["output_sha256"]) for entry in before.evidence): + return "missing_output" + return None + + +class PlanRuntime: + security_rule: GateRule = GateRule() + plan_managed: bool = False + execution_plan: Any = None + plan_receipt: dict[str, Any] | None = None + verification_resume: bool = False + plan_partial: bool = False + plan_head: str | None = None + plan_store: Any = None + recorded_commands: Any = None + plan_conclusion: str | None = None + + def initialize_plan(self: Any, supplied, security_rule) -> None: + self.security_rule = security_rule or GateRule() + self.plan_managed = supplied is not None + resolved = resolve_execution(ExecutionSettings(), invocation_settings( + local_only=self.local_only, + issue_review="off" if self.issue_review.value == "off" else "auto")) + self.execution_plan = supplied or build_plan( + resolved, repository=self.repo_id, work_item=self.task_id, + signals=PlanSignals(self.signals.difficulty, self.signals.security_sensitive), + start_from=self.start_from, continue_work=self.continue_work, + change_request=self.state.change_request_id, + plan_id=f"plan-{self.recorder.cycle_id}", provenance="run") + # A dry-run identity is deterministic; every real run has its own identity. + self.execution_plan = replace(self.execution_plan, + plan_id=f"plan-{self.recorder.cycle_id}", revision=1, + head_sha=self.api._git_line(self.cwd, ["rev-parse", "HEAD"], 30) if self.cwd else None) + self.execution_plan.validate() + self.plan_receipt = None + self.verification_resume = False + self.plan_partial = False + self.plan_head = self.execution_plan.head_sha + self.plan_store = self.api.CycleHistoryStore(self.telemetry.path) + self.recorded_commands = set() + self.plan_conclusion = None + if self.start_from == "implement" and not self.continue_work: + if self.execution_plan.stage("issue_review").selected: + self.issue_review_fields["issue_review"] = "dispatched" + + def save_plan(self: Any, provenance: str = "observed", stop_reason=None) -> None: + if self.state.verification_plan is not None: + conclusion = self.state.verification_plan.cap(self.plan_conclusion or "verified", self.plan_head or "") + self.execution_plan = replace(self.execution_plan, stages=tuple( + replace(item, + state=ExecutionState.COMPLETED if conclusion in {"verified", "verified_with_reservations"} + else ExecutionState.PLANNED, + conclusion=conclusion if conclusion in {"verified", "verified_with_reservations"} else None) + if item.stage_id == "verify" and item.selected else item + for item in self.execution_plan.stages)) + self.execution_plan = replace(self.execution_plan, + revision=self.execution_plan.revision + 1, + verification=self.state.verification_plan, + head_sha=self.plan_head, + metadata={"verification_conclusion": self.plan_conclusion}) + self.execution_plan.validate() + self.plan_receipt = self.plan_store.save_plan( + self.execution_plan, cycle_id=self.recorder.cycle_id, + change_request_id=self.state.change_request_id, + provenance=provenance, stop_reason=stop_reason) + self.report.execution_plan = self.plan_receipt + if self.state.verification_plan is not None: + for check in self.state.verification_plan.checks: + if check.command not in self.recorded_commands and not check.command.startswith("sha256:"): + self.journal.record("stage_result", component="verification_command", command=check.command, + status="proposed") + self.recorded_commands.add(check.command) + self.journal.record("stage_result", component="execution_plan", status=provenance, + correlation={"plan_id": self.execution_plan.plan_id, + "revision": self.execution_plan.revision, + "digest": self.plan_receipt["digest"], "head_sha": self.plan_head}) + + def prepare_plan(self: Any): + """Reconcile authorized inputs and local history before any dispatch.""" + record = None + if self.start_from in self.api.RESUMES or self.continue_work: + try: + record = self.plan_store.latest_plan( + self.repo_id, self.task_id, self.state.change_request_id) + except (HistoryError, ValueError, TypeError): + self.plan_partial = True + if record is not None: + try: + commands = self.authorized_bindings() + recovered = restore_plan(record, commands) + # New policy can add obligations. It cannot erase the old ones. + same_policy = (recovered.objective == self.execution_plan.objective + and recovered.execution_profile == self.execution_plan.execution_profile + and recovered.control("ci").timing == self.execution_plan.control("ci").timing + and all(old.policy == self.execution_plan.stage(old.stage_id).policy + for old in recovered.stages)) + if not same_policy: + self.plan_partial = True + for old in recovered.stages: + new = self.execution_plan.stage(old.stage_id) + if (old.policy is StagePolicy.REQUIRED + and new.policy is StagePolicy.DISABLED): + raise ContractError("resume would withdraw a required stage") + old_security = recovered.stage("security") + if old_security.selected and old_security.obligation.value == "required": + security = self.execution_plan.stage("security") + if security.resolution is Resolution.PENDING: + self.execution_plan = self.execution_plan.resolve_decision( + "security", True, "retained security obligation", "runtime") + self.execution_plan = replace(self.execution_plan, + plan_id=recovered.plan_id, revision=recovered.revision, + verification=recovered.verification) + self.state.verification_plan = recovered.verification + self.plan_conclusion = recovered.metadata.get("verification_conclusion") + self.verification_resume = ( + self.start_from == "rereview" and record["stop_reason"] == "verification_unverified" + and same_policy + and record["head_sha"] == self.plan_head and self.plan_receipt_matches(record) + and (not recovered.stage("security").selected + or recovered.stage("security").state is ExecutionState.COMPLETED)) + if self.verification_resume: + self.execution_plan = recovered + except HistoryError: + self.plan_partial = True + self.plan_conclusion = "not_verified" + if record is not None: + # Keep unavailable command identities as obligations. A + # fingerprint is not executable and grants no permission. + # Re-authorizing its exact command permits a later resume. + try: + retained = ExecutionPlan.from_dict(record["payload"]["plan"]) + except (KeyError, TypeError, ContractError): + retained = None + self.state.verification_plan = retained.verification if retained is not None else None + if retained is not None and retained.verification is not None: + verification = retained.verification + for check in retained.verification.checks: + verification = verification.record( + CheckResult(check.id, self.plan_head, "unverified", cause="unverified")) + self.state.verification_plan = verification + elif self.start_from in self.api.RESUMES: + self.plan_partial = True + if (self.state.verification_plan is None and self.policy_provider is not None + and (self.start_from == "implement" or self.plan_managed)): + criteria = (self.criteria_reader or self.api.verification_contract.read_criteria)( + self.policy_provider, self.work_item_repository or self.repo_id, + self.task_id, cwd=self.cwd) + self.state.verification_plan = VerificationPlan(criteria) + elif (self.plan_managed and self.start_from in self.api.RESUMES + and self.policy_provider is not None and self.state.verification_plan is not None): + criteria = (self.criteria_reader or self.api.verification_contract.read_criteria)( + self.policy_provider, self.work_item_repository or self.repo_id, + self.task_id, cwd=self.cwd) + old = self.state.verification_plan.criteria + if criteria.content_sha256 != old.content_sha256: + # Existing AC identities remain obligations. New provider input + # gets new identities, so old coverage cannot satisfy it. + criteria = replace(criteria, items=(*old.items, *criteria.items)) + self.state.verification_plan = replace(self.state.verification_plan, criteria=criteria) + self.verification_resume = False + self.plan_partial = True + elif len(criteria.items) == len(old.items): + self.state.verification_plan = replace(self.state.verification_plan, criteria=criteria) + if self.start_from != "implement" and self.state.verification_plan is not None and self.allowed_test_command: + existing = self.state.verification_plan.checks + if not any(check.command == self.allowed_test_command and check.required for check in existing): + spec = configured_check(self.allowed_test_command, "runtime", "runtime") + if any(check.id == spec.id for check in existing): + spec = replace(spec, id="configured-test-" + hashlib.sha256( + self.allowed_test_command.encode()).hexdigest()[:16]) + self.state.verification_plan = self.state.verification_plan.extend(( + spec,)) + if self.state.verification_plan is not None: + self.state.criteria_context = ("\nRuntime acceptance criteria (data): " + + json.dumps(self.state.verification_plan.criteria.to_dict(), ensure_ascii=False) + + "\nUse tests.version=2 and stable AC-n links. Criteria without a check remain open.\n") + self.save_plan("partial" if self.plan_partial else "recovered" if record else "initial") + + def authorized_bindings(self: Any) -> tuple[str, ...]: + """Journal text proposes bindings; today's policy alone authorizes them.""" + commands = list(filter(None, (self.allowed_test_command, *self.allowed_verify_commands))) + policy = CommandPolicy(role="verify", test_command=self.allowed_test_command, + verify_prefixes=tuple(self.allowed_verify_commands)) + for row in journal_events(self.telemetry.path, self.repo_id, task_id=self.task_id, + kinds=("stage_result",)): + command = row.get("command") + if (row.get("component") == "verification_command" + and isinstance(command, str) and _is_verification_command(command, policy)): + commands.append(command) + return tuple(dict.fromkeys(commands)) + + def plan_receipt_matches(self: Any, record) -> bool: + trusted = self.api.trusted_logins(self.trusted_authors) + if not trusted or not self.state.change_request_id: + return False + try: + comments = self.remote.comments(self.repo_id, self.state.change_request_id) + receipts = [receipt for comment in comments + if isinstance(comment.author, str) and comment.author.casefold() in trusted + for receipt in self.api.stage_evidence(comment.body) + if receipt["repo"] == self.repo_id + and receipt["change_request_id"] == self.state.change_request_id] + except (self.api.CycleDriverError, self.api.ContractError): + return False + if not receipts: + return False + receipt = receipts[-1] + return (receipt["head_sha"] == record["head_sha"] + and receipt.get("plan_id") == record["plan_id"] + and self.plan_store.has_plan_receipt(self.repo_id, receipt)) + + def plan_context(self: Any) -> str: + receipt = self.plan_receipt or {} + security = self.execution_plan.stage("security") + context = ("\nRuntime execution plan (data): " + json.dumps({ + **receipt, "objective": self.execution_plan.objective, + "execution_profile": self.execution_plan.execution_profile, + "security_required": security.selected, + "verify_selected": self.execution_plan.stage("verify").selected}) + + "\nInclude plan_id, revision and digest in the schema-2 stage receipt. " + "Required controls cannot be removed by an agent. " + "When security_required, run cc-security-review and report security_audit_ran=true.\n") + if self.execution_plan.execution_profile == "hotfix": + context += ("Hotfix requires tests.version=2 checks hotfix-reproduction and " + "hotfix-regression, both required. Propose authorized focused commands. " + "Create the reproduction in a test-first commit, fail for the defect, " + "then pass the unchanged test on the fixed HEAD. Report the prior fail " + "under that check's history with its full SHA, cause=defect and evidence " + "including command, exit_code, executed, test_sha256, context_sha256 and " + "output_sha256. Retain the pre-fix output artifact. Never count an import " + "or environment failure as reproduction.\n") + return context + + def select_plan_stage(self: Any, role: str) -> None: + observed = self.api._git_line(self.cwd, ["rev-parse", "HEAD"], 30) if self.cwd else None + if observed is not None and observed != self.plan_head: + self.plan_head = observed + self.plan_conclusion = "not_verified" + self.execution_plan = replace(self.execution_plan, stages=tuple( + replace(item, state=ExecutionState.PLANNED, conclusion=None) + if item.stage_id in {"verify", "security"} and item.selected else item + for item in self.execution_plan.stages)) + plan = self.execution_plan + stage = plan.stage(role) + if role == "resolve" and stage.resolution is Resolution.NOT_APPLICABLE: + # A resume entry skipped earlier work; a later review can create + # fresh findings that activate resolution within the same budget. + if stage.policy is StagePolicy.DISABLED: + raise ContractError("resolution is disabled; findings retain their blocks") + plan = replace(plan, stages=tuple(replace(item, selected=True, + resolution=Resolution.RESOLVED, state=ExecutionState.PLANNED) + if item.stage_id == role else item for item in plan.stages)) + if stage.resolution is Resolution.PENDING: + if role in {"resolve", "rereview"}: + plan = plan.resolve_decision(role, True, "runtime condition observed", "runtime") + if role in {"review", "rereview"}: + paths = None + if self.cwd and self.change_bases: + for base in self.change_bases: + raw = self.api._git_line(self.cwd, ["diff", "--name-only", f"{base}...HEAD"], 30) + if raw is not None: + paths = raw.splitlines() + break + # Unknown diff is conservative. Agent requests only add controls. + required = paths is None or evaluate(paths, rule=self.security_rule).required + security = plan.stage("security") + required = required or self.signals.security_sensitive or security.policy in { + StagePolicy.REQUIRED, StagePolicy.ENABLED} or security.selected + if required: + plan = self.require_security(plan, "runtime security gate") + elif security.resolution is Resolution.PENDING: + plan = plan.resolve_decision("security", False, "runtime security gate", "runtime") + self.execution_plan = plan + plan.check_executable(role) + self.save_plan() + + def require_security(self: Any, plan, reason: str): + security = plan.stage("security") + if security.resolution is Resolution.PENDING: + return plan.resolve_decision("security", True, reason, "runtime") + if security.policy is StagePolicy.DISABLED: + # Existing obligation validation explains the policy conflict. + return plan.settle_obligation("security_policy", True, reason) + plan = replace(plan, stages=tuple( + replace(item, selected=True, obligation=Obligation.REQUIRED, + state=ExecutionState.PLANNED if not item.selected else item.state, + resolution=Resolution.RESOLVED) + if item.stage_id == "security" else item for item in plan.stages)) + return plan.settle_obligation("security_policy", True, reason) + + def record_plan_stage(self: Any, role: str, reported, outcome, test_payload=None) -> None: + payload = reported.payload or {} + head = payload.get("head_sha") + if isinstance(head, str) and self.api.FULL_SHA.fullmatch(head): + if self.plan_managed and self.cwd: + actual = self.api._git_line(self.cwd, ["rev-parse", "HEAD"], 30) + if actual is not None and actual != head: + raise ContractError("reported evidence does not match the workspace HEAD") + if self.plan_head and head != self.plan_head: + self.plan_conclusion = "not_verified" + self.execution_plan = replace(self.execution_plan, stages=tuple( + replace(item, state=ExecutionState.PLANNED, conclusion=None) + if item.stage_id in {"verify", "security"} and item.selected else item + for item in self.execution_plan.stages)) + self.plan_head = head + conclusion = self.api._legacy_tests(test_payload).get("verification") + if conclusion is not None: + self.plan_conclusion = conclusion + stage = self.execution_plan.stage(role) + # Functional outcomes remain separate from transport completion. + stages = tuple(replace(item, state=ExecutionState.COMPLETED, + conclusion=reported.status) + if item.stage_id == role else item for item in self.execution_plan.stages) + if stage.selected and reported.status: + self.execution_plan = replace(self.execution_plan, stages=stages) + if role in {"review", "rereview"} and payload.get("security_audit_ran") is True: + self.execution_plan = replace(self.execution_plan, stages=tuple( + replace(item, state=ExecutionState.COMPLETED, conclusion="completed") + if item.stage_id == "security" and item.selected else item + for item in self.execution_plan.stages)) + if role in {"implement", "resolve"} and payload.get("security_required") is True: + self.execution_plan = self.require_security(self.execution_plan, "agent added a control") + self.save_plan() + + def verification_barrier(self: Any, head: str | None = None): + plan = self.state.verification_plan + security = self.execution_plan.stage("security") + if (self.plan_managed and security.selected + and security.state is not ExecutionState.COMPLETED): + return self.stop("required security verification remains unverified", + stop_reason="verification_unverified") + current_head: str = head or self.plan_head or "" + if self.execution_plan.execution_profile == "hotfix": + problem = hotfix_problem(plan, current_head) + if problem is None and self.cwd: + before = next(result for result in reversed(plan.results) + if result.id == HOTFIX_IDS[0] and result.head_sha != current_head + and result.result == "fail" and result.cause == "defect") + if self.api._git_line(self.cwd, ["merge-base", before.head_sha, current_head], 30) != before.head_sha: + problem = "test_first_commit_unverified" + if problem: + return self.stop(f"Hotfix proof is incomplete: {problem}", + stop_reason="needs_refinement" if problem == "missing_checks" else "verification_unverified") + if plan is None: + if self.plan_managed and self.execution_plan.objective != "local_change": + return self.stop("required verification has no recoverable check plan", + stop_reason="verification_unverified") + return None + required = [check for check in plan.checks if check.required] + if self.plan_conclusion == "failed" or any(plan.current(check.id, current_head) == "fail" for check in required): + return self.stop("a required verification check failed", stop_reason="stage_not_completed") + if plan.cap(self.plan_conclusion or "verified", current_head) == "not_verified": + return self.stop("required verification remains unverified on the current HEAD; " + "resume with --from rereview --pr to run only pending checks", + stop_reason="verification_unverified") + return None + + def verify_pending(self: Any): + """A single isolated cc-verify dispatch; no publication or review loop.""" + from executors import DisposableWorkspace, _git_output + plan = self.state.verification_plan + pending = [check for check in plan.checks if check.required + and plan.current(check.id, self.plan_head) == "unverified"] if plan else [] + if not pending: + return self.verification_barrier() + source = self.cwd + if not source or self.api._git_line(source, ["rev-parse", "HEAD"], 30) != self.plan_head: + return self.stop("verification resume HEAD changed", stop_reason="head_moved") + directory = self.api.tempfile.mkdtemp(prefix="code-cycle-verify-") + try: + source_status = _git_output(source, "status", "--porcelain", "--untracked-files=all") + source_branch = _git_output(source, "rev-parse", "--symbolic-full-name", "HEAD") + self.api.Path(directory).rmdir() + _git_output(source, "clone", "--no-hardlinks", "--no-checkout", source, directory, timeout=300) + _git_output(directory, "remote", "remove", "origin") + _git_output(directory, "checkout", "--detach", self.plan_head) + instruction = (f"Run cc-verify on HEAD {self.plan_head}. Run only these pending required checks " + + json.dumps([check.to_dict() for check in pending]) + + ". Do not review, change source, commit or publish. Return VERIFICATION_RESULT " + "with version=2, head_sha and checks, including unverified causes. Run every command " + "in the foreground and allow at least 600 seconds for each configured check. " + "No general CI result satisfies a check.") + kwargs = {**self.dispatch_kwargs, "cwd": directory, + "workspace": DisposableWorkspace(directory, source), "writable_dirs": (), + "command_policy": replace(self.stage_policy("verify", self.state.change_request_id), + test_command=None, verify_prefixes=tuple(check.command for check in pending))} + self.control.move("verify") + outcome = self.recorder.stage("verify", instruction, **kwargs) + self.report.stages.append(outcome) + self.report.stage_details.append(self.api.StageReportDetails( + self.recorder.iteration, None)) + if not outcome.succeeded or outcome.result is None or outcome.result.asynchronous: + return self.stop("pending verification could not execute", stop_reason="verification_unverified") + if (_git_output(directory, "rev-parse", "HEAD") != self.plan_head + or _git_output(directory, "diff", "--name-only", "HEAD")): + raise VerificationError("verifier changed the tracked source under verification") + spoken = outcome.result.agent_output + end = spoken.rfind("END_VERIFICATION_RESULT") + begin = spoken.rfind("VERIFICATION_RESULT", 0, end) + if begin < 0 or end < 0: + raise VerificationError("verifier returned no structured result") + result = json.loads(spoken[begin + len("VERIFICATION_RESULT"):end].strip()) + if (not isinstance(result, dict) or not isinstance(result.get("checks"), list) + or any(not isinstance(item, dict) for item in result["checks"])): + raise VerificationError("verifier returned malformed check results") + if result.get("head_sha") != self.plan_head: + raise VerificationError("verifier returned another HEAD") + if any(item.get("id") not in {check.id for check in pending} + or item.get("head_sha") != self.plan_head or item.get("history") + for item in result.get("checks", [])): + raise VerificationError("verifier reported checks outside the pending set") + if self.api._git_line(source, ["rev-parse", "HEAD"], 30) != self.plan_head: + raise VerificationError("source HEAD changed during verification") + if (_git_output(source, "status", "--porcelain", "--untracked-files=all") != source_status + or _git_output(source, "rev-parse", "--symbolic-full-name", "HEAD") != source_branch): + raise VerificationError("source workspace changed during verification") + tests, self.state.verification_plan = self.api.verification_contract.normalize_tests( + result, self.plan_head, plan=plan) + self.plan_conclusion = tests["conclusion"] + self.recorder.record_verdict("verify", "READY" if tests["passed"] else "BLOCKED", + **self.api._legacy_tests({"tests": tests})) + self.report.stage_details[-1].status = "READY" if tests["passed"] else "BLOCKED" + self.status_writer.stage_finished(outcome.result, self.report.stage_details[-1].status) + self.journal.record("stage_result", role="verify", component="execution_plan", + status=self.report.stage_details[-1].status, correlation={"head_sha": self.plan_head}) + self.save_plan() + return self.verification_barrier() + except (OSError, ValueError, RuntimeError, self.api.subprocess.SubprocessError) as error: + return self.stop(f"pending verification is unverified: {error}", + stop_reason="verification_unverified") + finally: + self.api._remove_workspace(directory) diff --git a/scripts/cycle_session.py b/scripts/cycle_session.py index 47e2c4d..c92ae4d 100644 --- a/scripts/cycle_session.py +++ b/scripts/cycle_session.py @@ -13,6 +13,7 @@ from .cycle_review_gate import ReviewGate from .cycle_loop import CycleControl, StageLoop from .workspace_setup import commit_setup, own_file + from .cycle_plan import PlanRuntime else: from cycle_state import SessionState from cycle_dispatch import StageDispatch @@ -20,8 +21,9 @@ from cycle_review_gate import ReviewGate from cycle_loop import CycleControl, StageLoop from workspace_setup import commit_setup, own_file + from cycle_plan import PlanRuntime -class CycleSession(StageDispatch, PublicationHistory, ReviewGate, StageLoop): +class CycleSession(PlanRuntime, StageDispatch, PublicationHistory, ReviewGate, StageLoop): def __init__(self, api: Any, options: dict[str, Any]): # The facade supplies dependencies so existing integrations keep # the same public names and injection points. @@ -188,6 +190,7 @@ def __init__(self, api: Any, options: dict[str, Any]): self.issue_review_fields["issue_review"] = self.api.issue_review_decision( self.issue_review, self.signals) self.checks_provider = self.ci_checks if self.ci_checks is not None else self.api.ci_gate.GitHubChecks(self.cwd) + self.initialize_plan(options.get("execution_plan"), options.get("security_rule")) def progress_stage_started(self, role, decision) -> None: iteration = self.recorder.iteration if role in {"review", "resolve", "rereview"} else None @@ -234,6 +237,15 @@ def journal_result(self, role, outcome, reported, attempt_id) -> None: def stop(self, because: str, status: str = "HUMAN_INTERVENTION", reported: Reported | None = None, *, stop_reason: str = "stage_not_completed") -> CycleReport: + if status == self.api.APPROVED_END: + barrier = self.verification_barrier() + if barrier is not None: + return barrier + if self.execution_plan.objective != "merge_ready": + status = self.api.UNRESOLVED_END + stop_reason = "local_only" + because = f"objective {self.execution_plan.objective} completed before merge readiness" + self.save_plan("stopped", stop_reason=stop_reason) self.control.move("stop") if self.report.stages and self.report.stage_details: # The exact command a permission stopped, in the stop line, so a @@ -302,6 +314,12 @@ def execute(self): if self.policy_host == "github" else None) except Exception as error: self.report.note(f" journal cleanup failed ({type(error).__name__}); detail retained") + try: + if self.start_from not in self.api.RESUMES: + self.prepare_plan() + except (self.api.verification_contract.VerificationError, + self.api.HistoryError, ValueError) as error: + return self.stop(str(error), "BLOCKED", stop_reason="readiness_unconfirmed") return self.drive() except self.api.CycleInterrupted as interruption: return self.interrupted(interruption) diff --git a/scripts/review_contract.py b/scripts/review_contract.py index f6b46b2..9035902 100644 --- a/scripts/review_contract.py +++ b/scripts/review_contract.py @@ -111,6 +111,15 @@ def stage_evidence(text: str) -> list[dict]: raise ContractError(f"stage evidence requires {key}") if not _FULL_SHA_RE.fullmatch(value["head_sha"]): raise ContractError("stage evidence requires a full head SHA") + plan_keys = {"plan_id", "revision", "digest"} + if plan_keys & value.keys(): + if (not plan_keys <= value.keys() + or not isinstance(value["plan_id"], str) + or re.fullmatch(r"[A-Za-z0-9._:#/-]{1,200}", value["plan_id"]) is None + or type(value["revision"]) is not int or value["revision"] < 1 + or not isinstance(value["digest"], str) + or re.fullmatch(r"[a-f0-9]{64}", value["digest"]) is None): + raise ContractError("invalid execution plan receipt") outcomes = value.get("finding_outcomes", []) if not isinstance(outcomes, list): raise ContractError("stage evidence outcomes must be a list") diff --git a/scripts/run_cycle.py b/scripts/run_cycle.py index 64f72ca..ef06289 100644 --- a/scripts/run_cycle.py +++ b/scripts/run_cycle.py @@ -66,6 +66,7 @@ import verification_plan as verification_contract import ci_gate +from security_gate import GateRule try: from gh_environment import apply_gh_environment_defaults except ModuleNotFoundError: @@ -87,7 +88,6 @@ ResolvedExecution, Setting, build_plan, - current_flow_differences, invocation_settings, render_text as render_plan, repository_settings, @@ -907,6 +907,7 @@ class CycleReport: #: The last publication audit's evidence (#261): refs and change-request #: states before and after the stage, and every breach. SHAs and names only. publication_audit: dict | None = None + execution_plan: dict[str, Any] | None = None @property def approved(self) -> bool: @@ -2358,6 +2359,8 @@ def run_cycle( published_change_request: Callable[[], str | None] | None = None, publication_auditor: PublicationAuditor | None = None, criteria_reader: Callable[..., verification_contract.Criteria] | None = None, + execution_plan: ExecutionPlan | None = None, + security_rule=None, ) -> CycleReport: """[issue_review ->] implement -> review -> (resolve -> rereview)*, recorded. @@ -3063,25 +3066,6 @@ def dry_run(args) -> ExecutionPlan: return execution_plan_of(args, config, repo) -#: Until the driver follows the plan (#329), it runs the current flow only. -PLAN_DRIVEN_EXECUTION = "#329" - - -def check_current_flow(resolved: ResolvedExecution) -> None: - """Refuse an execution configuration the current flow would not honour. - - A requested profile is never replaced by `standard` in silence: the run - stops before any effect, naming each setting and its source. - """ - differences = current_flow_differences(resolved) - if differences: - raise CycleDriverError( - "this execution configuration differs from the standard flow: " - + "; ".join(differences) - + f". Plan-driven execution needs {PLAN_DRIVEN_EXECUTION}; until then a " - "run follows only the standard flow. Inspect the plan with --dry-run.") - - def _main_worktree_root(cwd: Path, run=subprocess.run) -> Path | None: """Return the main worktree root seen from this worktree, when available. diff --git a/scripts/runtime.manifest b/scripts/runtime.manifest index f71b860..d0867e2 100644 --- a/scripts/runtime.manifest +++ b/scripts/runtime.manifest @@ -59,3 +59,4 @@ cycle_cli.py cycle_workspace.py workspace_setup.py cycle_state.py +cycle_plan.py diff --git a/scripts/stage_permissions.py b/scripts/stage_permissions.py index 3beff53..aaffc24 100644 --- a/scripts/stage_permissions.py +++ b/scripts/stage_permissions.py @@ -80,7 +80,7 @@ def credential_read_denials(*, publishes: bool = False, GIT_REVIEW_READ = frozenset({"merge-base", "ls-files", "grep", "blame"}) #: Stages that search the repository with `GIT_REVIEW_READ`. The hook and the #: `--allowedTools` patterns both read this set, so the two barriers agree. -GIT_SEARCH_ROLES = WRITING_ROLES | REVIEW_ROLES | {ISSUE_REVIEW_ROLE} +GIT_SEARCH_ROLES = WRITING_ROLES | REVIEW_ROLES | {ISSUE_REVIEW_ROLE, "verify"} GIT_WRITE = frozenset({"add", "commit", "push", "fetch", "checkout", "switch", "pull", "rm", "mv"}) #: Options that make Git run another program, on any subcommand. Git also @@ -239,7 +239,7 @@ def issue_number(self) -> str | None: @property def verification_commands(self) -> tuple[str, ...]: """The configured test and verify commands this role receives.""" - if self.static_only or self.role not in WRITING_ROLES | REVIEW_ROLES: + if self.static_only or self.role not in WRITING_ROLES | REVIEW_ROLES | {"verify"}: return () commands = (self.test_command,) if self.test_command else () return commands + tuple(self.verify_prefixes) diff --git a/scripts/telemetry.py b/scripts/telemetry.py index 71dc60f..4cf3424 100644 --- a/scripts/telemetry.py +++ b/scripts/telemetry.py @@ -107,7 +107,9 @@ #: `publication_violation`, an invariant on an audited ref or change request #: failed during a publishing stage, or `publication_unverified`, the audited #: state could not be read. Both payload-only. -SCHEMA_VERSION = 24 +#: 25: execution-plan revisions and check histories; verification_unverified +#: is a distinct stop and cannot be cleared by generic CI success. +SCHEMA_VERSION = 25 APP_DIRNAME = "code-cycle-toolkit" DATABASE_NAME = "telemetry.sqlite" @@ -155,6 +157,7 @@ def is_jev_model(value) -> bool: "manual_rereview_rejected", # A publication invariant failed, or could not be read (schema 24). "publication_violation", "publication_unverified", + "verification_unverified", }) #: Who ran the review a cycle closed on (schema 22); `review_contract` defines them. @@ -761,6 +764,21 @@ def started_at_implement(row: dict) -> bool: return payload.get("started_from", "implement") == "implement" SCHEMA = """ +CREATE TABLE IF NOT EXISTS cycle_execution_plans ( + sequence INTEGER PRIMARY KEY AUTOINCREMENT, + repo_id TEXT NOT NULL, + task_id TEXT NOT NULL, + change_request_id TEXT, + cycle_id TEXT NOT NULL, + plan_id TEXT NOT NULL, + revision INTEGER NOT NULL, + digest TEXT NOT NULL, + head_sha TEXT, + provenance TEXT NOT NULL, + stop_reason TEXT, + payload TEXT NOT NULL, + UNIQUE(repo_id, plan_id, revision) +); CREATE TABLE IF NOT EXISTS stages ( id INTEGER PRIMARY KEY AUTOINCREMENT, schema_version INTEGER NOT NULL, diff --git a/scripts/verification_plan.py b/scripts/verification_plan.py index 779b8f1..cf28e7e 100644 --- a/scripts/verification_plan.py +++ b/scripts/verification_plan.py @@ -172,6 +172,10 @@ def validate(self) -> None: codes = [] for item in self.evidence: code, executed = item.get("exit_code"), item.get("executed") + if any(key in item and (not isinstance(item[key], str) + or re.fullmatch(r"[a-f0-9]{64}", item[key]) is None) + for key in ("test_sha256", "context_sha256", "output_sha256")): + raise VerificationError("evidence fingerprints must be SHA-256 digests") 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") @@ -227,8 +231,9 @@ def record(self, result: CheckResult) -> VerificationPlan: 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" + latest = next((result for result in reversed(self.results) + if result.id == check_id and result.head_sha == head_sha), None) + return latest.result if latest is not None else "unverified" def cap(self, conclusion: str, head_sha: str) -> str: """Deterministic caps never raise the agent's conclusion.""" @@ -330,6 +335,13 @@ def normalize_tests(tests: Mapping[str, Any], head_sha: str, *, if check.criteria: check = replace(check, required=True) plan = plan.extend((check,)) + history = entry.get("history", []) + if not isinstance(history, list): + raise VerificationError("check history must be an array") + for previous in history: + prior = CheckResult.from_dict({**previous, "id": check.id}) + if prior not in plan.results: + plan = plan.record(prior) result = CheckResult(check.id, entry["head_sha"], entry["result"], tuple(entry.get("evidence", ())), entry.get("cause", ""), tuple(entry.get("attempts", ()))) diff --git a/tests/fakes/fake_agent.py b/tests/fakes/fake_agent.py index c013406..25a22dd 100755 --- a/tests/fakes/fake_agent.py +++ b/tests/fakes/fake_agent.py @@ -307,6 +307,10 @@ def writable(path: Path) -> bool: ) if skill in prompt), ("fake", None)) _, role = selected payload = result_for_role(role or "unknown", status) + if role in {"review", "rereview"} and os.environ.get("FAKE_SECURITY_AUDIT") == "1": + # Explicit fixture evidence: omission is the default, and must never + # produce merge readiness. This flag lets loop tests reach resolution. + payload["security_audit_ran"] = True if os.environ.get("FAKE_SANDBOX") == NAME and role == "implement": payload = result_for_role(role, "BLOCKED") payload["environment_blocker"] = { diff --git a/tests/test_execution_profiles.py b/tests/test_execution_profiles.py index fc35bf0..5e951ca 100644 --- a/tests/test_execution_profiles.py +++ b/tests/test_execution_profiles.py @@ -455,12 +455,15 @@ def test_flag_combinations(self) -> None: code, _, err, _ = self.run_main(self.BASE + ["--dry-run", "--detach"]) self.assertEqual(code, 2) - def test_a_run_refuses_a_non_standard_configuration_before_any_effect(self) -> None: - code, _, err, mocks = self.run_main(self.BASE + ["--execution-profile", "full"]) - self.assertEqual(code, 2) - self.assertIn("execution profile full (--execution-profile)", err) - self.assertIn("#329", err) - self.assertFalse(any(spy.called for spy in mocks.values())) + def test_a_full_run_reaches_workspace_selection(self) -> None: + stack, mocks = self.spies() + mocks["select_workspace"].side_effect = run_cycle.CycleDriverError("reached full workspace") + stderr = io.StringIO() + with stack, contextlib.redirect_stdout(io.StringIO()), contextlib.redirect_stderr(stderr): + with self.assertRaises(SystemExit): + run_cycle.main(self.BASE + ["--execution-profile", "full"]) + self.assertIn("reached full workspace", stderr.getvalue()) + self.assertFalse(mocks["run_cycle"].called) def test_a_standard_run_reaches_the_current_flow(self) -> None: stack, mocks = self.spies() diff --git a/tests/test_installed_cycle.py b/tests/test_installed_cycle.py index 72b4ff8..27d04b8 100644 --- a/tests/test_installed_cycle.py +++ b/tests/test_installed_cycle.py @@ -161,18 +161,20 @@ def all_rows(self) -> list[dict]: def test_a_cycle_run_from_the_installation_records_every_stage(self) -> None: result = self.run_cycle() - self.assertEqual(0, result.returncode, result.stderr) + self.assertEqual(1, result.returncode, result.stderr) rows = self.rows() self.assertEqual( ["implement", "implement", "review", "review", "coordinate"], [row["role"] for row in rows], ) - self.assertEqual("READY_FOR_MANUAL_MERGE", rows[-1]["status"]) + self.assertEqual("HUMAN_INTERVENTION", rows[-1]["status"]) + self.assertEqual("verification_unverified", json.loads(rows[-1]["payload"])["stop_reason"]) + self.assertIn("security audit was omitted", result.stdout) def test_the_default_cycle_reviews_the_work_item_before_implementing(self) -> None: result = self.run_cycle() - self.assertEqual(0, result.returncode, result.stderr) + self.assertEqual(1, result.returncode, result.stderr) rows = self.rows(issue_review=True) self.assertEqual( ["issue_review", "issue_review", "implement", "implement", @@ -196,10 +198,10 @@ def test_a_work_item_that_needs_refinement_is_not_implemented(self) -> None: self.assertEqual("needs_refinement", json.loads(rows[-1]["payload"])["stop_reason"]) self.assertIn("IR-001", result.stdout) - def test_off_runs_the_original_cycle(self) -> None: + def test_off_skips_only_the_work_item_review(self) -> None: result = self.run_cycle("--issue-review", "off") - self.assertEqual(0, result.returncode, result.stderr) + self.assertEqual(1, result.returncode, result.stderr) rows = self.rows(issue_review=True) self.assertEqual( ["implement", "implement", "review", "review", "coordinate"], @@ -229,7 +231,7 @@ def test_the_rows_say_which_executor_and_model_actually_ran(self) -> None: def test_agent_reported_test_evidence_survives_an_installed_cycle(self) -> None: result = self.run_cycle(FAKE_TEST_EVIDENCE="1") - self.assertEqual(0, result.returncode, result.stderr) + self.assertEqual(1, result.returncode, result.stderr) rows = self.rows() implement_verdict = next(row for row in rows if row["role"] == "implement" and row["status"]) @@ -244,9 +246,9 @@ def test_agent_reported_test_evidence_survives_an_installed_cycle(self) -> None: def test_a_second_round_is_recorded_as_a_second_round(self) -> None: """The review asks for changes once, so the cycle resolves and re-reviews.""" - result = self.run_cycle(FAKE_ROUNDS=str(self.root / "rounds")) + result = self.run_cycle(FAKE_ROUNDS=str(self.root / "rounds"), FAKE_SECURITY_AUDIT="1") - self.assertEqual(0, result.returncode, result.stderr) + self.assertEqual(1, result.returncode, result.stderr) rows = self.rows() self.assertEqual( ["implement", "implement", "review", "review", @@ -351,7 +353,7 @@ def test_the_block_is_read_out_of_the_real_envelope(self) -> None: verdicts = [row["status"] for row in self.rows() if row["status"]] - self.assertEqual(["IMPLEMENTED", "APPROVED", "READY_FOR_MANUAL_MERGE"], + self.assertEqual(["IMPLEMENTED", "APPROVED", "HUMAN_INTERVENTION"], verdicts) def test_a_stopped_cycle_records_where_it_stopped_and_stops_its_agent(self) -> None: @@ -427,7 +429,7 @@ def test_an_interrupted_implementation_continues_from_its_checkout(self) -> None (self.project / "partial.txt").write_text("half done\n", encoding="utf-8") continued = self.run_cycle("--continue", "--workspace", "current") - self.assertEqual(0, continued.returncode, continued.stderr) + self.assertEqual(1, continued.returncode, continued.stderr) closing = [json.loads(row["payload"]) for row in self.all_rows() if row["role"] == "coordinate"] self.assertEqual([True], [payload.get("continued") for payload in closing]) diff --git a/tests/test_run_cycle.py b/tests/test_run_cycle.py index 3e68c0b..56348cf 100644 --- a/tests/test_run_cycle.py +++ b/tests/test_run_cycle.py @@ -209,6 +209,236 @@ def rows(self): return self.store.rows("owner/api") +class NativeExecutionPlanTests(RunCycleTestCase): + """The native loop consumes policy and recovers check evidence, not verdicts.""" + + def plan(self, profile="standard", start="implement", objective="merge_ready"): + import execution_profiles as ep + resolved = ep.resolve_execution(ep.ExecutionSettings(), ep.invocation_settings( + execution_profile=profile, objective=objective, issue_review="off")) + return ep.build_plan(resolved, repository="owner/api", work_item="API-7", + start_from=start, change_request="4" if start != "implement" else None) + + def test_cli_boundary_prints_full_plan_and_refuses_withdrawn_review(self): + argv = [sys.executable, str(ROOT / "scripts" / "run_cycle.py"), "--no-config", + "--repo", "owner/api", "--task", "API-7", "--execution-profile", "full", + "--dry-run", "--json"] + good = subprocess.run(argv, capture_output=True, text=True, timeout=600) + self.assertEqual(good.returncode, 0, good.stderr) + value = json.loads(good.stdout) + self.assertEqual(value["execution_profile"], "full") + self.assertTrue(next(stage for stage in value["stages"] + if stage["stage_id"] == "issue_review")["selected"]) + bad = subprocess.run([*argv, "--stage", "review=disabled"], + capture_output=True, text=True, timeout=600) + self.assertEqual(bad.returncode, 2) + self.assertIn("requires an independent review", bad.stderr) + + def test_all_five_profiles_retain_blocks_without_an_identical_rereview(self): + for profile in ("prototype", "standard", "full", "hotfix", "custom"): + with self.subTest(profile=profile): + # Full requires readiness only on a new implementation. + review = block("CHANGES_REQUESTED", head_sha="a" * 40, + security_audit_ran=True, findings=[{ + "id": "REV-001", "status": "open", "severity": "high", + "blocks_approval": True}]) + report = self.run_cycle(Talker("codex", block("RESOLVED", head_sha="a" * 40)), + Talker("claude", review), start_from="review", change_request_id="4", + execution_plan=self.plan(profile, "review")) + self.assertEqual([stage.role for stage in report.stages], ["review", "resolve"]) + self.assertEqual(report.stop_reason, "no_progress") + self.assertFalse(report.approved) + + def test_unverified_required_check_stops_before_ci_under_both_policies(self): + import verification_plan as vp + spec = vp.Check("focused", "python3 -m unittest tests.focused", "codex", "cheap_coder", + "project", True, criteria=("AC-1",)) + for policy in ("strict", "parallel"): + with self.subTest(policy=policy): + report = self.run_cycle(Talker("codex", block("IMPLEMENTED", head_sha="a" * 40, + tests={"version": 2, "conclusion": "not_verified", "checks": [{ + **spec.to_dict(), "head_sha": "a" * 40, "result": "unverified", + "cause": "missing_tool"}]})), Talker("claude", block("APPROVED", + head_sha="a" * 40, security_audit_ran=True)), + work_item_provider="github", work_item_repository="owner/api", + execution_plan=self.plan(), ci_policy=policy) + self.assertEqual(report.stop_reason, "verification_unverified") + self.assertEqual(report.status, "HUMAN_INTERVENTION") + self.assertEqual([stage.role for stage in report.stages], ["implement", "review"]) + record = rc.CycleHistoryStore(self.store.path).latest_plan("owner/api", "API-7", "4") + self.assertEqual(record["stop_reason"], "verification_unverified") + + def test_missing_hotfix_commands_need_refinement(self): + report = self.run_cycle(Talker("codex", block("IMPLEMENTED", head_sha="a" * 40)), + Talker("claude", block("APPROVED", head_sha="a" * 40, security_audit_ran=True)), + execution_plan=self.plan("hotfix"), work_item_provider="github", work_item_repository="owner/api") + self.assertEqual(report.stop_reason, "needs_refinement") + + def test_passing_checks_do_not_raise_the_agents_not_verified_conclusion(self): + import verification_plan as vp + spec = vp.Check("focused", "python3 -m unittest tests.focused", "codex", "cheap_coder", + "project", True, criteria=("AC-1",)) + report = self.run_cycle(Talker("codex", block("IMPLEMENTED", head_sha="a" * 40, + tests={"version": 2, "conclusion": "not_verified", "checks": [{**spec.to_dict(), + "head_sha": "a" * 40, "result": "pass", "evidence": [{"command": spec.command, + "exit_code": 0, "executed": 1}]}]})), + Talker("claude", block("APPROVED", head_sha="a" * 40, security_audit_ran=True)), + execution_plan=self.plan(), work_item_provider="github", work_item_repository="owner/api") + self.assertEqual(report.stop_reason, "verification_unverified") + self.assertFalse(report.approved) + + def test_store_keeps_metadata_and_command_bindings_without_prose(self): + import cycle_history as ch + import verification_plan as vp + from dataclasses import replace + command = "python3 -m unittest tests.focused" + check = vp.Check("focused", command, "codex", "cheap_coder", "project", True, + criteria=("AC-1",), reason="private reasoning") + verification = vp.VerificationPlan(vp.Criteria("github", "b" * 64, "evaluable", + ("sensitive acceptance text",))).extend((check,)).record(vp.CheckResult( + "focused", "a" * 40, "unverified", cause="missing_tool")) + plan = replace(self.plan(), plan_id="plan-fixture", head_sha="a" * 40, + verification=verification) + store = ch.CycleHistoryStore(self.store.path) + receipt = store.save_plan(plan, cycle_id="cycle-test", change_request_id="4", provenance="initial") + record = store.latest_plan("owner/api", "API-7", "4") + serialized = json.dumps(record) + for private in (command, "private reasoning", "sensitive acceptance text"): + self.assertNotIn(private, serialized) + self.assertEqual(receipt["digest"], ch.plan_digest(record["payload"])) + with self.assertRaises(ch.HistoryError): + ch.restore_plan(record, ()) + restored = ch.restore_plan(record, (command,)) + self.assertEqual(restored.verification.checks[0].command, command) + self.assertEqual(restored.verification.results[0].cause, "missing_tool") + + def test_journal_command_proposals_require_current_authorization(self): + from cycle_plan import PlanRuntime + from cycle_journal import Journal + from types import SimpleNamespace + journal = Journal(self.store.path, "owner/api", "API-7", "cycle-bindings") + safe = "python3 -m unittest tests.focused" + unsafe = "python3 -m unittest tests.focused; echo secret" + withdrawn = "pytest tests/focused.py" + for command in (safe, unsafe, withdrawn): + journal.record("stage_result", component="verification_command", command=command) + session = SimpleNamespace(telemetry=self.store, repo_id="owner/api", task_id="API-7", + allowed_test_command=None, allowed_verify_commands=("python3 -m unittest",)) + candidates = PlanRuntime.authorized_bindings(session) + self.assertIn(safe, candidates) + self.assertNotIn(unsafe, candidates) + self.assertNotIn(withdrawn, candidates) + + def test_new_sensitive_diff_promotes_a_previously_omitted_audit(self): + from cycle_plan import PlanRuntime + import execution_plan as ep + old = self.plan(start="review").resolve_decision("security", False, "old diff", "runtime") + updated = PlanRuntime().require_security(old, "new diff requires security") + self.assertTrue(updated.stage("security").selected) + self.assertIs(updated.stage("security").obligation, ep.Obligation.REQUIRED) + updated.validate() + + def test_hotfix_requires_same_test_and_context_and_retains_prior_failure(self): + from cycle_plan import HOTFIX_IDS, hotfix_problem + import verification_plan as vp + proof = {"exit_code": 1, "executed": 1, "test_sha256": "c" * 64, + "context_sha256": "d" * 64, "output_sha256": "e" * 64} + checks = tuple(vp.Check(identifier, f"python3 -m unittest tests.{identifier.replace('-', '_')}", + "codex", "cheap_coder", "project", True, criteria=("AC-1",)) for identifier in HOTFIX_IDS) + verification = vp.VerificationPlan(vp.Criteria("github", "b" * 64, "evaluable", ("works",)), checks) + before = vp.CheckResult(HOTFIX_IDS[0], "a" * 40, "fail", (proof,), cause="defect") + after = vp.CheckResult(HOTFIX_IDS[0], "b" * 40, "pass", ({**proof, "exit_code": 0},)) + verification = verification.record(before).record(after) + self.assertIsNone(hotfix_problem(verification, "b" * 40)) + self.assertEqual(verification.current(HOTFIX_IDS[0], "b" * 40), "pass") + self.assertEqual(verification.current(HOTFIX_IDS[0], "a" * 40), "fail") + changed = replace(after, evidence=({**proof, "exit_code": 0, "test_sha256": "f" * 64},)) + self.assertEqual(hotfix_problem(verification.record(changed), "b" * 40), "incompatible_proof") + environment = replace(before, cause="environment") + self.assertEqual(hotfix_problem(replace(verification, results=(environment, after)), + "b" * 40), "missing_proof") + + def test_verification_only_resume_uses_disposable_clone_and_only_pending_checks(self): + self.verification_resume_fixture("pass") + + def test_verification_only_unavailable_environment_stops_once(self): + self.verification_resume_fixture("unverified") + + def test_verifier_cannot_pass_after_editing_its_clone(self): + self.verification_resume_fixture("pass", tamper=True) + + def test_new_profile_obligations_prevent_verification_only_shortcut(self): + self.verification_resume_fixture("pass", policy_change=True) + + def verification_resume_fixture(self, result, tamper=False, policy_change=False): + import cycle_history as ch + import cycle_session + import verification_plan as vp + from types import SimpleNamespace + fixture = self.home / "checkout" + fixture.mkdir(parents=True) + repo = change_repository(fixture) + head = git(repo, "rev-parse", "HEAD").decode().strip() + commands = ("python3 -m unittest tests.first", "python3 -m unittest tests.pending") + specs = tuple(vp.Check(f"check-{i}", command, "codex", "cheap_coder", "project", True, + criteria=("AC-1",)) for i, command in enumerate(commands)) + verification = vp.VerificationPlan(vp.Criteria("github", "b" * 64, "evaluable", ("works",)), specs) + verification = verification.record(vp.CheckResult("check-0", head, "pass", ({ + "command": commands[0], "exit_code": 0, "executed": 1},))) + verification = verification.record(vp.CheckResult("check-1", head, "unverified", cause="environment")) + old = replace(self.plan(), plan_id="plan-resume", head_sha=head, verification=verification) + receipt = ch.CycleHistoryStore(self.store.path).save_plan(old, cycle_id="cycle-before", + change_request_id="4", provenance="stopped", stop_reason="verification_unverified") + comment = contract.ProviderComment(author="reviewer", id="1", body=contract.format_stage_evidence({ + "schema": 2, "repo": "owner/api", "change_request_id": "4", "stage": "review", + "attempt_id": "attempt-review", "head_sha": head, "status": "APPROVED", + "finding_outcomes": [], **receipt})) + seen = [] + + class Verifier(Talker): + def dispatch(inner, target, task, **kw): + seen.append((task, kw)) + if policy_change: + self.assertIn("cc-rereview", task) + return ex.DispatchResult(ex.DispatchOutcome.SUCCEEDED, inner.name, target, + agent_output=block("BLOCKED"), model_resolved=target.model, process_exited=True) + self.assertNotIn(commands[0], task) + self.assertIn(commands[1], task) + self.assertNotEqual(Path(kw["cwd"]), repo) + self.assertEqual(git(Path(kw["cwd"]), "rev-parse", "HEAD").decode().strip(), head) + self.assertEqual(git(Path(kw["cwd"]), "remote"), b"") + if tamper: + tracked = git(Path(kw["cwd"]), "ls-files").decode().splitlines()[0] + (Path(kw["cwd"]) / tracked).write_text("changed by verifier\n") + spoken = "VERIFICATION_RESULT\n" + json.dumps({"version": 2, "head_sha": head, + "conclusion": "verified", "checks": [{**specs[1].to_dict(), + "reason": "runtime", "provenance": "runtime", "head_sha": head, "result": result, + "cause": "environment", "evidence": [{"command": commands[1], "exit_code": 0, + "executed": 1}] if result == "pass" else []}]}) + "\nEND_VERIFICATION_RESULT" + return ex.DispatchResult(ex.DispatchOutcome.SUCCEEDED, inner.name, target, + agent_output=spoken, model_resolved=target.model, process_exited=True) + + checks = SimpleNamespace(pull=lambda *_: rc.ci_gate.PullState("OPEN", head, "main", "task", True, "clean")) + with mock.patch.object(cycle_session.CycleSession, "gate", return_value=("approved", None)): + report = self.run_cycle(Verifier("codex"), Verifier("claude"), start_from="rereview", + change_request_id="4", execution_plan=self.plan( + "full" if policy_change else "standard", start="rereview"), + allowed_verify_commands=commands, cwd=str(repo), ci_checks=checks, + comment_reader=lambda *_: [comment]) + if policy_change: + self.assertEqual([stage.role for stage in report.stages], ["rereview"]) + self.assertFalse(report.approved) + return + self.assertEqual([stage.role for stage in report.stages], ["verify"]) + self.assertEqual(report.status, rc.APPROVED_END if result == "pass" and not tamper else "HUMAN_INTERVENTION") + if result == "unverified" or tamper: + self.assertEqual(report.stop_reason, "verification_unverified") + self.assertEqual(len(seen), 1) + self.assertFalse(Path(seen[0][1]["cwd"]).exists()) + self.assertFalse(seen[0][1]["publishes"]) + self.assertEqual(git(repo, "rev-parse", "HEAD").decode().strip(), head) + + class PublicationCompletionTests(RunCycleTestCase): def test_completion_normalizes_supported_change_request_references(self): class PublishingTalker(Talker): diff --git a/tests/test_verification_plan.py b/tests/test_verification_plan.py index f2bcaa8..4f2dde1 100644 --- a/tests/test_verification_plan.py +++ b/tests/test_verification_plan.py @@ -114,7 +114,9 @@ def test_results_preserve_history_and_invalidate_stale_head(self): 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") + # The caller supplies the current HEAD. History for another SHA never + # erases proof retained for this SHA (including Hotfix test-first runs). + self.assertEqual(newer.current("check-1", HEAD), "pass") newer = newer.record(result("pass", OTHER)) self.assertEqual(newer.cap("verified", OTHER), "verified") self.assertEqual(vp.VerificationPlan.from_dict(newer.to_dict()), newer) diff --git a/tests/test_work_item_outcomes.py b/tests/test_work_item_outcomes.py index 6c8c77c..38130e8 100644 --- a/tests/test_work_item_outcomes.py +++ b/tests/test_work_item_outcomes.py @@ -70,7 +70,7 @@ def test_schema_fourteen_records_transitions_and_deduplicates_snapshots(self) -> self.assertEqual("unchanged", repeated["reason"]) self.assertFalse(stale["created"]) self.assertEqual("stale", stale["reason"]) - self.assertEqual(24, tm.SCHEMA_VERSION) + self.assertEqual(25, tm.SCHEMA_VERSION) self.assertEqual(1, len(self.store.work_item_disposition_events("owner/repo"))) event = self.store.latest_work_item_disposition("owner/repo", "github", "owner/repo", "120") self.assertEqual(["cycle-1"], event["cycle_ids"]) From dcd36fc270c62c0988418bb8c4a1aa2e8df9443f Mon Sep 17 00:00:00 2001 From: datacas <4690232+datacas@users.noreply.github.com> Date: Fri, 30 Oct 2026 17:30:51 +0100 Subject: [PATCH 2/2] fix(runtime): resolve policy-plan review findings --- docs/configuration.md | 4 ++- docs/instrumentation.md | 4 ++- docs/review-cycle.md | 50 +++++++++++++++++++++------- docs/telemetry.md | 28 ++++++++-------- scripts/cycle_loop.py | 11 +++---- scripts/cycle_session.py | 3 +- scripts/execution_profiles.py | 25 -------------- scripts/validate-package.py | 11 ++++--- skills/cc-initial-review/SKILL.md | 12 ++++++- skills/cc-rereview/SKILL.md | 12 ++++++- tests/test_execution_profiles.py | 29 ---------------- tests/test_run_cycle.py | 55 ++++++++++++++++++++++++++++--- tests/test_validate_package.py | 25 +++++++------- 13 files changed, 158 insertions(+), 111 deletions(-) diff --git a/docs/configuration.md b/docs/configuration.md index b43a86a..543b4e1 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -229,7 +229,9 @@ profile is never replaced by `standard`. Stage overrides cannot remove mandatory checks, security requirements, publication guarantees or human review. `local_change` from `--objective` or the file applies the local-only workspace requirements (`--cwd` linked worktree and `--workspace current`) and stops before -merge readiness. `verified_change` stops after verification and a clean review; +merge readiness. `verified_change` stops after verification and a clean review +with terminal status `HUMAN_INTERVENTION` and `stop_reason: approved`, including +after verification-only resume. It does not run the merge-readiness CI gate; `merge_ready` also requires the reviewed SHA's CI gate. `--dry-run` prints the same plan without effects. No additional configuration key or resume start point is needed for verification-only recovery; see [Verification](verification.md). diff --git a/docs/instrumentation.md b/docs/instrumentation.md index e5c92d3..c8cb285 100644 --- a/docs/instrumentation.md +++ b/docs/instrumentation.md @@ -710,7 +710,9 @@ manual rereview a resume cannot close on; see *Manual rereview* in `review-cycle.md`), `publication_violation` (schema 24: a publication invariant failed during a publishing stage, whoever caused it), `publication_unverified` (schema 24: the audited refs or change-request state could not be read; see -*Publication audit* in `role-workspace-policy.md`), `worktree_dirty`, or +*Publication audit* in `role-workspace-policy.md`), `verification_unverified` +(schema 25: required check evidence, criteria coverage, or security audit +completion is unverified; generic CI success cannot clear it), `worktree_dirty`, or `worktree_diverged`. The last two stop before changing files when a dirty tree or non-fast-forward prevents syncing an open pull request head. A closing row from an earlier version has none, and `cc-stats` reads it as unknown. A stage row's `reroute_reason` may then be diff --git a/docs/review-cycle.md b/docs/review-cycle.md index 61f035d..d46ed4a 100644 --- a/docs/review-cycle.md +++ b/docs/review-cycle.md @@ -1,16 +1,5 @@ # Review lifecycle -Native cycles select stages from the current execution plan. Resolution remains -bounded by the existing iteration budget. After a resolution, identical explicit -review and resolution HEADs stop with `no_progress`: open findings retain their -blocks, and no rereview runs, including under Full. A changed HEAD invalidates -incompatible review and verification evidence and activates the applicable -controls again. A clean trusted receipt for the unchanged current HEAD can be -reused on resume under either CI policy. Final readiness still requires valid -`REV-*` history, required verification and the checks of that SHA; merge remains -a human action. Verification-only recovery uses `cc-verify`, not another review; -see [Verification](verification.md). - How findings are created, tracked across rounds, and closed, and why the pull-request comment is the record the next run reads back. **On this page:** [Issue review before implementation](#issue-review-before-implementation) · [The loop](#the-loop) · [Resume after a failed upload](#resume-after-a-failed-upload) · [CI policy](#ci-policy) · [Anatomy of a review comment](#anatomy-of-a-review-comment) · [Finding fields](#finding-fields) · [Trusted authors](#trusted-authors) · [Security triage](#security-triage) · [Structured result](#the-structured-result-is-opt-in) · [Exit conditions](#exit-conditions) @@ -49,6 +38,17 @@ A delegated stage returns its questions as a `questions` list in its structured ## The loop +Native cycles select stages from the current execution plan. Resolution remains +bounded by the existing iteration budget. After a resolution, identical explicit +review and resolution HEADs stop with `no_progress`: open findings retain their +blocks, and no rereview runs, including under Full. A changed HEAD invalidates +incompatible review and verification evidence and activates the applicable +controls again. A clean trusted receipt for the unchanged current HEAD can be +reused on resume under either CI policy. Final readiness still requires valid +`REV-*` history, required verification and the checks of that SHA; merge remains +a human action. Verification-only recovery uses `cc-verify`, not another review; +see [Verification](verification.md). + ```mermaid sequenceDiagram participant R as cc-initial-review @@ -334,6 +334,10 @@ security_required = deterministic_rule OR reviewer_requests_security `scripts/security_gate.py` is the reference implementation. +The native execution plan can also require security (for example, under Full +or when diff paths are unavailable). Its `security_required` obligation is +mandatory even when the rule and reviewer judgement would otherwise skip it. + ## The structured result is opt-in `ORCHESTRATION_RESULT`, a strict JSON block with IDs, statuses, and SHAs, is emitted only when: @@ -344,6 +348,15 @@ security_required = deterministic_rule OR reviewer_requests_security When it is emitted alongside a published comment, it is collapsed in a `
` element there, not repeated in the response. Otherwise the published comment, with the headers above, is the record. See [Instrumentation → The comment is the record](instrumentation.md#the-comment-is-the-record). +Review and rereview results include the boolean `security_audit_ran`. Use `true` +only when the `cc-security-review` pass completed for `reviewed_head_sha`, even +if it found vulnerabilities. Use `false` when triage skipped it or execution +could not complete, and explain why in the comment. A clean review status does +not imply an audit. If the execution plan requires security, an omitted or +false value stops the native cycle with `verification_unverified`; prose that +says an audit ran does not substitute for the structured field. Both review +skills' result examples declare the field, including its use in blocked results. + ## Exit conditions An orchestrator reports `READY_FOR_MANUAL_MERGE` only when **all** of these hold on the code host: @@ -357,12 +370,19 @@ The last review may be a confirmed [manual rereview](#manual-rereview) of the cu A human still owns the merge. +The `verified_change` objective completes after required verification and a +clean review with `HUMAN_INTERVENTION` and `stop_reason: approved`. This records +objective completion before merge readiness, including after verification-only +resume; it does not run the merge-readiness CI gate or report +`READY_FOR_MANUAL_MERGE`. `local_only` remains the stop for `local_change`. + The loop stops early with `HUMAN_INTERVENTION`, before another resolution is dispatched, on any of these: | Stop | When | Recorded as | |---|---|---| | Iteration limit | the resolve+rereview budget is spent: `max_iterations`, default `3` in `run_cycle.py` and `6` in the orchestrator skills | `iteration_limit` | -| No progress | a rereview reports the same `head_sha` and the same open finding set as the review before it | `no_progress` | +| No progress | a resolution reports the same explicit `head_sha` as the review it addresses; no rereview is dispatched and open findings retain their blocks | `no_progress` | +| Required verification unavailable | a required check is unverified, acceptance criteria remain uncovered, or a required security audit was omitted or could not be verified | `verification_unverified` | | Repeated findings | one `REV-xxx` ID survived a claimed fix twice | `repeated_findings` | | Manual rereview not accepted | a resume's latest receipt is a [manual rereview](#manual-rereview) it cannot close on | `manual_rereview_rejected` | | Publication invariant failed | during any publishing stage, a change request was merged or closed, the cycle branch moved non-fast-forward or was deleted, or the default or a protected branch moved ([Publication audit](role-workspace-policy.md#publication-audit)) | `publication_violation` | @@ -371,6 +391,12 @@ Before the loop, an [issue review](#issue-review-before-implementation) can stop **A finding survives a claimed fix** when a resolution publishes its ID as `resolved` or `not_applicable` and the next review reopens it as `open` (`still_open` in a rereview's result). The claim is the status, never the disposition: a finding triaged `valid` and left `open` is no claim; one triaged `incorrect` and published `not_applicable` is one. Each claim is judged by the next review only, and within one run the last header for an ID is its position. These do not count: +An ordinary comment-only resolution leaves HEAD unchanged, so the native loop +stops with `no_progress` before another review can judge its claims. Publishing +`not_applicable` after triaging a finding `incorrect` does not bypass that guard +or remove the reviewed blocks. A person must decide how to continue; no survival +is counted without a subsequent readable review rejection. + | Sequence | Why not | |---|---| | `open@CCR → open@CCT → open@CCR` | the resolver never claimed it; the no-progress guard and the limit cover it | diff --git a/docs/telemetry.md b/docs/telemetry.md index 0422e99..dbf5cf1 100644 --- a/docs/telemetry.md +++ b/docs/telemetry.md @@ -1,18 +1,5 @@ # Telemetry and `cc-stats` -Schema 25 adds `verification_unverified` to the closed stop reasons and the -append-only `cycle_execution_plans` history table. Each revision records cycle, -repository, work item, change request, plan identity, revision, current HEAD, -canonical digest and provenance (`initial`, `observed`, `recovered`, `partial`, -`stopped`), plus a versioned metadata projection. Existing tables and history -remain intact. The projection excludes prompts, criterion prose, command text -and command output; it retains command/output/test/environment hashes, check -identities, required flags, criterion links, results and cause tokens. Each -check can retain results for several SHAs. Generic CI success does not update -those results. Journal events correlate plan ID, revision and digest with the -cycle. Published schema-2 receipts carry the same three references; see -[Execution plan](execution-plan.md) for canonicalization and recovery rules. - When the runtime drives a cycle, every stage writes rows to a local SQLite database. `cc-stats` turns those rows into a per-repository report. This page covers what is recorded, what never is, and how to read it. **On this page:** [Production delivery events](#production-delivery-events-335) · [When rows are written](#when-rows-are-written) · [Where it lives](#where-it-lives) · [What is recorded](#what-is-recorded) · [Work-item outcomes](#work-item-outcomes) · [What is never recorded](#what-is-never-recorded) · [What it is used for](#what-it-is-used-for) · [Reading it with cc-stats](#reading-it-with-cc-stats) · [Limits](#limits) @@ -449,6 +436,21 @@ GitHub has no atomic check-and-publish, so this reduces conflicts between machines without excluding them. Each publishing stage costs one freshness check and, when something changed, one filtered comment read. +### Execution plan history (schema 25) + +Schema 25 adds `verification_unverified` to the closed stop reasons and the +append-only `cycle_execution_plans` history table. Each revision records cycle, +repository, work item, change request, plan identity, revision, current HEAD, +canonical digest and provenance (`initial`, `observed`, `recovered`, `partial`, +`stopped`), plus a versioned metadata projection. Existing tables and history +remain intact. The projection excludes prompts, criterion prose, command text +and command output; it retains command/output/test/environment hashes, check +identities, required flags, criterion links, results and cause tokens. Each +check can retain results for several SHAs. Generic CI success does not update +those results. Journal events correlate plan ID, revision and digest with the +cycle. Published schema-2 receipts carry the same three references; see +[Execution plan](execution-plan.md) for canonicalization and recovery rules. + ### CI gate (schema 19) Under the `parallel` [CI policy](review-cycle.md#ci-policy) the review outcome and diff --git a/scripts/cycle_loop.py b/scripts/cycle_loop.py index dd3f6f2..175e9b7 100644 --- a/scripts/cycle_loop.py +++ b/scripts/cycle_loop.py @@ -126,6 +126,8 @@ def edit_comment(comment_id: str, body: str) -> None: stop_reason="verification_unverified") role, verdict, head, attempts = clean self.report.verdict = verdict + if self.execution_plan.objective == "verified_change": + return self.stop("", self.api.APPROVED_END, stop_reason="approved") outcome = self.gate(role, head, attempts, reported) if isinstance(outcome, self.api.CycleReport): return outcome @@ -137,10 +139,7 @@ def edit_comment(comment_id: str, body: str) -> None: # Independent of issue-review mode/difficulty. Read once before any # agent iteration; provider content is data, never instructions. if self.policy_provider is not None: - try: - criteria = self.state.verification_plan.criteria - except self.api.verification_contract.VerificationError as error: - return self.stop(str(error), "BLOCKED", stop_reason="readiness_unconfirmed") + criteria = self.state.verification_plan.criteria if criteria.status == "not_evaluable": return self.stop( f"acceptance criteria not_evaluable for {criteria.provider}: " @@ -270,8 +269,8 @@ def reader() -> str | None: clean = (role, verdict, head, set(self.state.review_attempts)) head = clean[2] if self.execution_plan.objective == "verified_change": - return self.stop("verified_change completed with a clean review; merge is a human action", - "IN_PROGRESS", reported=reported, stop_reason="local_only") + return self.stop("", self.api.APPROVED_END, reported=reported, + stop_reason="approved") outcome = self.gate(clean[0], head, clean[3], reported) clean = None if isinstance(outcome, self.api.CycleReport): diff --git a/scripts/cycle_session.py b/scripts/cycle_session.py index c92ae4d..a735cc3 100644 --- a/scripts/cycle_session.py +++ b/scripts/cycle_session.py @@ -243,7 +243,8 @@ def stop(self, because: str, status: str = "HUMAN_INTERVENTION", return barrier if self.execution_plan.objective != "merge_ready": status = self.api.UNRESOLVED_END - stop_reason = "local_only" + stop_reason = ("approved" if self.execution_plan.objective == "verified_change" + else "local_only") because = f"objective {self.execution_plan.objective} completed before merge readiness" self.save_plan("stopped", stop_reason=stop_reason) self.control.move("stop") diff --git a/scripts/execution_profiles.py b/scripts/execution_profiles.py index feb8693..de47a8d 100644 --- a/scripts/execution_profiles.py +++ b/scripts/execution_profiles.py @@ -336,31 +336,6 @@ def resolve_execution(repository: ExecutionSettings, invocation: ExecutionSettin ci_policy or Setting("strict", "built-in default", "default")) -def current_flow_differences(resolved: ResolvedExecution) -> list[str]: - """What the driver cannot honour until it follows the plan (#329). - - The current flow is `standard`. It already honours an issue review that is - off, and the local-only run as objective `local_change`. Anything else is - named with its source. - """ - differences = [] - if resolved.profile.value not in {"standard", "custom"}: - differences.append( - f"execution profile {resolved.profile.value} ({resolved.profile.source})") - if resolved.objective.value == "verified_change": - differences.append( - f"objective verified_change ({resolved.objective.source})") - for stage in PLANNED_STAGES: - setting = resolved.stages[stage] - if setting.policy is _STANDARD[stage]: - continue - if stage == "issue_review" and setting.policy is StagePolicy.DISABLED: - continue - if setting.layer in EXPLICIT_LAYERS: - differences.append(f"{stage}={setting.policy.value} ({setting.source})") - return differences - - # -- plan construction ------------------------------------------------------- @dataclass diff --git a/scripts/validate-package.py b/scripts/validate-package.py index dfc62df..a54d621 100755 --- a/scripts/validate-package.py +++ b/scripts/validate-package.py @@ -522,17 +522,18 @@ def check_asking_contract(root: Path, errors: list[str]) -> None: runtime refuses. """ check_shared_sections(root, ASKING_SKILLS, (ASKING_SECTION,), errors) - # A result that carries identifiers and status only would forbid the very - # prose a delegated stage's questions consist of. + # Restricted result prose must still allow delegated questions, including + # review contracts that also declare the security completion boolean. for skill in ASKING_SKILLS: skill_path = root / "skills" / skill / "SKILL.md" if not skill_path.is_file(): continue flattened = " ".join(skill_path.read_text(encoding="utf-8").split()) - if ("identifiers and status only" in flattened + if (("identifiers and status only" in flattened + or "The serialised block carries identifiers, status" in flattened) and "The one exception is `questions`" not in flattened): - errors.append(f"skills/{skill}/SKILL.md: its result allows identifiers " - "and status only, with no exception for `questions`") + errors.append(f"skills/{skill}/SKILL.md: its result restricts prose, " + "with no exception for `questions`") path = root / "skills" / ASKING_SKILLS[0] / "SKILL.md" if not path.is_file(): return diff --git a/skills/cc-initial-review/SKILL.md b/skills/cc-initial-review/SKILL.md index 9b01fb9..39a8781 100644 --- a/skills/cc-initial-review/SKILL.md +++ b/skills/cc-initial-review/SKILL.md @@ -719,7 +719,16 @@ strict JSON, not a Markdown code fence, and do not include chain-of-thought. `reviewed_head_sha` is mandatory and `blocking_findings` contains the IDs of all open findings with `blocks_approval: true`. -The serialised block carries identifiers and status only. It must not repeat +The serialised block carries identifiers, status, and the boolean +`security_audit_ran`. Set it to `true` only when the required `cc-security-review` +pass actually completed for `reviewed_head_sha`; findings do not make it false. +Set it to `false` when triage skipped the audit or it could not complete, and +explain that state in the published comment. Never infer completion from a clean +verdict. When the runtime plan says `security_required`, the audit is mandatory: +an omitted, false, or unverified value stops the native cycle with +`verification_unverified`. Include the field in every structured result, +including `BLOCKED` and `FAILED`; it does not replace the audit evidence in prose. +It must not repeat narrative already published in the comment: no `title`, no `description`, no `summary`, no `reason`, no quoted evidence. `comment_url` is the pointer to that prose and is mandatory whenever a comment was published. The one @@ -742,6 +751,7 @@ ORCHESTRATION_RESULT "comment_url": "https://github.com/owner/repo/pull/123#issuecomment-1234567890", "head_sha": "0123456789abcdef0123456789abcdef01234567", "reviewed_head_sha": "0123456789abcdef0123456789abcdef01234567", + "security_audit_ran": true, "review_run_id": "CCR-20260918-001", "reviewer": { "profile": "senior_reviewer", diff --git a/skills/cc-rereview/SKILL.md b/skills/cc-rereview/SKILL.md index 9d03d58..5b301b4 100644 --- a/skills/cc-rereview/SKILL.md +++ b/skills/cc-rereview/SKILL.md @@ -711,7 +711,16 @@ strict JSON without a Markdown code fence or chain-of-thought. `blocking_findings` contains every previous or new open finding whose `blocks_approval` is `true`. -The serialised block carries identifiers and status only. It must not repeat +The serialised block carries identifiers, status, and the boolean +`security_audit_ran`. Set it to `true` only when the required `cc-security-review` +pass actually completed for `reviewed_head_sha`; findings do not make it false. +Set it to `false` when triage skipped the audit or it could not complete, and +explain that state in the published comment. Never infer completion from a clean +verdict. When the runtime plan says `security_required`, the audit is mandatory: +an omitted, false, or unverified value stops the native cycle with +`verification_unverified`. Include the field in every structured result, +including `BLOCKED` and `FAILED`; it does not replace the audit evidence in prose. +It must not repeat narrative already published in the comment: no `title`, no `description`, no `summary`, no `reason`, no quoted evidence. `comment_url` is the pointer to that prose and is mandatory whenever a comment was published. The one @@ -734,6 +743,7 @@ ORCHESTRATION_RESULT "comment_url": "https://github.com/owner/repo/pull/123#issuecomment-1234567890", "previous_reviewed_head_sha": "0123456789abcdef0123456789abcdef01234567", "reviewed_head_sha": "89abcdef0123456789abcdef0123456789abcdef", + "security_audit_ran": true, "head_sha": "89abcdef0123456789abcdef0123456789abcdef", "review_run_id": "CCR-20260918-002", "reviewer": { diff --git a/tests/test_execution_profiles.py b/tests/test_execution_profiles.py index 5e951ca..dc69fed 100644 --- a/tests/test_execution_profiles.py +++ b/tests/test_execution_profiles.py @@ -66,7 +66,6 @@ def test_no_configuration_records_the_compatible_standard_flow(self) -> None: merge = built.control("merge") self.assertEqual(merge.requirement, plan.ControlRequirement.REQUIRED) self.assertFalse(merge.automatic) - self.assertEqual(profiles.current_flow_differences(resolved()), []) def test_each_profile_resolves_with_its_default_objective(self) -> None: expected_objective = {"prototype": "local_change", "standard": "merge_ready", @@ -364,34 +363,6 @@ def test_disabled_rereview_makes_changes_after_review_invalid(self) -> None: open_findings=False)) -class CurrentFlowTests(unittest.TestCase): - def test_standard_equivalents_run_the_current_flow(self) -> None: - for invocation in ({}, {"issue_review": "off"}, {"local_only": True}, - {"execution_profile": "standard"}, - {"execution_profile": "custom"}, - {"stages": ["issue_review=disabled", "review=required"]}, - {"objective": "merge_ready"}): - with self.subTest(invocation=invocation): - self.assertEqual(profiles.current_flow_differences(resolved(**invocation)), []) - - def test_anything_else_is_named_with_its_source(self) -> None: - cases = ( - ({"execution_profile": "full"}, "execution profile full (--execution-profile)"), - ({"execution_profile": "hotfix"}, "execution profile hotfix"), - ({"objective": "verified_change"}, "objective verified_change (--objective)"), - ({"execution_profile": "custom", "stages": ["security=required"]}, - "security=required (--stage security=required)"), - ({"stages": ["issue_review=required"]}, "issue_review=required"), - ) - for invocation, text in cases: - with self.subTest(invocation=invocation): - differences = profiles.current_flow_differences(resolved(**invocation)) - self.assertTrue(any(text in item for item in differences), differences) - config = {"code_cycle": {"execution": {"profile": "prototype"}}} - self.assertIn("execution profile prototype (code_cycle.execution.profile in " - ".code-cycle.yml)", profiles.current_flow_differences(resolved(config))) - - class DryRunTests(unittest.TestCase): BASE = ["--task", "328", "--repo", "datacas/code-cycle-toolkit", "--no-config"] diff --git a/tests/test_run_cycle.py b/tests/test_run_cycle.py index 56348cf..b772d9c 100644 --- a/tests/test_run_cycle.py +++ b/tests/test_run_cycle.py @@ -274,6 +274,40 @@ def test_missing_hotfix_commands_need_refinement(self): execution_plan=self.plan("hotfix"), work_item_provider="github", work_item_repository="owner/api") self.assertEqual(report.stop_reason, "needs_refinement") + def test_verified_change_completes_without_merge_readiness_or_ci(self): + import cycle_session + import verification_plan as vp + spec = vp.Check("focused", "python3 -m unittest tests.focused", "codex", "cheap_coder", + "project", True, criteria=("AC-1",)) + for policy in ("strict", "parallel"): + with self.subTest(policy=policy), mock.patch.object( + cycle_session.CycleSession, "gate") as gate, mock.patch.object( + cycle_session.CycleSession, "receipt_problem", return_value=None): + report = self.run_cycle(Talker("codex", block("IMPLEMENTED", head_sha="a" * 40, + tests={"version": 2, "conclusion": "verified", "checks": [{ + **spec.to_dict(), "head_sha": "a" * 40, "result": "pass", + "evidence": [{"command": spec.command, "exit_code": 0, "executed": 1}]}]})), + Talker("claude", block("APPROVED", head_sha="a" * 40, + reviewed_head_sha="a" * 40, findings=[], blocking_findings=[], + security_audit_ran=True)), + execution_plan=self.plan(objective="verified_change"), ci_policy=policy, + work_item_provider="github", work_item_repository="owner/api") + self.assertEqual(report.status, "HUMAN_INTERVENTION") + self.assertEqual(report.stop_reason, "approved") + self.assertFalse(report.approved) + self.assertEqual([stage.role for stage in report.stages], ["implement", "review"]) + self.assertIn("objective verified_change completed", report.stopped_because) + gate.assert_not_called() + + def test_required_security_needs_explicit_completion_from_the_reviewer(self): + for fields in ({}, {"security_audit_ran": False}): + with self.subTest(fields=fields): + report = self.run_cycle(Talker("codex"), Talker("claude", block( + "APPROVED", head_sha="a" * 40, **fields)), start_from="review", + change_request_id="4", execution_plan=self.plan("full", "review")) + self.assertEqual(report.stop_reason, "verification_unverified") + self.assertIn("security audit was omitted", report.stopped_because) + def test_passing_checks_do_not_raise_the_agents_not_verified_conclusion(self): import verification_plan as vp spec = vp.Check("focused", "python3 -m unittest tests.focused", "codex", "cheap_coder", @@ -364,13 +398,17 @@ def test_verification_only_resume_uses_disposable_clone_and_only_pending_checks( def test_verification_only_unavailable_environment_stops_once(self): self.verification_resume_fixture("unverified") + def test_verified_change_resume_completes_without_merge_readiness_or_ci(self): + self.verification_resume_fixture("pass", objective="verified_change") + def test_verifier_cannot_pass_after_editing_its_clone(self): self.verification_resume_fixture("pass", tamper=True) def test_new_profile_obligations_prevent_verification_only_shortcut(self): self.verification_resume_fixture("pass", policy_change=True) - def verification_resume_fixture(self, result, tamper=False, policy_change=False): + def verification_resume_fixture(self, result, tamper=False, policy_change=False, + objective="merge_ready"): import cycle_history as ch import cycle_session import verification_plan as vp @@ -386,7 +424,8 @@ def verification_resume_fixture(self, result, tamper=False, policy_change=False) verification = verification.record(vp.CheckResult("check-0", head, "pass", ({ "command": commands[0], "exit_code": 0, "executed": 1},))) verification = verification.record(vp.CheckResult("check-1", head, "unverified", cause="environment")) - old = replace(self.plan(), plan_id="plan-resume", head_sha=head, verification=verification) + old = replace(self.plan(objective=objective), plan_id="plan-resume", head_sha=head, + verification=verification) receipt = ch.CycleHistoryStore(self.store.path).save_plan(old, cycle_id="cycle-before", change_request_id="4", provenance="stopped", stop_reason="verification_unverified") comment = contract.ProviderComment(author="reviewer", id="1", body=contract.format_stage_evidence({ @@ -419,10 +458,10 @@ def dispatch(inner, target, task, **kw): agent_output=spoken, model_resolved=target.model, process_exited=True) checks = SimpleNamespace(pull=lambda *_: rc.ci_gate.PullState("OPEN", head, "main", "task", True, "clean")) - with mock.patch.object(cycle_session.CycleSession, "gate", return_value=("approved", None)): + with mock.patch.object(cycle_session.CycleSession, "gate", return_value=("approved", None)) as gate: report = self.run_cycle(Verifier("codex"), Verifier("claude"), start_from="rereview", change_request_id="4", execution_plan=self.plan( - "full" if policy_change else "standard", start="rereview"), + "full" if policy_change else "standard", start="rereview", objective=objective), allowed_verify_commands=commands, cwd=str(repo), ci_checks=checks, comment_reader=lambda *_: [comment]) if policy_change: @@ -430,7 +469,13 @@ def dispatch(inner, target, task, **kw): self.assertFalse(report.approved) return self.assertEqual([stage.role for stage in report.stages], ["verify"]) - self.assertEqual(report.status, rc.APPROVED_END if result == "pass" and not tamper else "HUMAN_INTERVENTION") + completed = result == "pass" and not tamper + self.assertEqual(report.status, rc.APPROVED_END + if completed and objective == "merge_ready" else "HUMAN_INTERVENTION") + if completed and objective == "verified_change": + self.assertEqual(report.stop_reason, "approved") + self.assertFalse(report.approved) + gate.assert_not_called() if result == "unverified" or tamper: self.assertEqual(report.stop_reason, "verification_unverified") self.assertEqual(len(seen), 1) diff --git a/tests/test_validate_package.py b/tests/test_validate_package.py index 076187c..8026d6c 100644 --- a/tests/test_validate_package.py +++ b/tests/test_validate_package.py @@ -260,17 +260,20 @@ def test_rejects_an_asking_rule_that_allows_a_wall_of_text(self) -> None: def test_rejects_a_result_that_forbids_the_prose_of_questions(self) -> None: package = self.copy_package() - path = package / "skills" / "cc-rereview" / "SKILL.md" - text = path.read_text(encoding="utf-8") - self.assertIn("The one\nexception is `questions`", text) - path.write_text(text.replace("The one\nexception is `questions`", - "No\nexception is `questions`", 1), encoding="utf-8") - - errors = VALIDATOR.validate_package(package) - - self.assert_error_contains( - errors, "skills/cc-rereview/SKILL.md: its result allows identifiers and " - "status only, with no exception for `questions`") + for skill in VALIDATOR.REVIEW_CYCLE_SKILLS: + with self.subTest(skill=skill): + path = package / "skills" / skill / "SKILL.md" + text = path.read_text(encoding="utf-8") + self.assertIn("The one\nexception is `questions`", text) + path.write_text(text.replace("The one\nexception is `questions`", + "No\nexception is `questions`", 1), encoding="utf-8") + + errors = VALIDATOR.validate_package(package) + + self.assert_error_contains( + errors, f"skills/{skill}/SKILL.md: its result restricts prose, " + "with no exception for `questions`") + path.write_text(text, encoding="utf-8") def test_rejects_a_questions_example_the_runtime_refuses(self) -> None: package = self.copy_package()