From cd47f754ce3f2c9b0bb6aa584d7fd233fb99e511 Mon Sep 17 00:00:00 2001 From: Timothy Wayne Gregg Date: Sun, 5 Jul 2026 20:48:37 -0400 Subject: [PATCH 1/2] feat(headless): emit structured review results (#119) --- docs/headless-contract.md | 323 +++++++++++++++--- src-rust/crates/cli/src/headless.rs | 225 +++++++++++- .../headless_contract/result.example.json | 19 +- .../headless_contract/result.schema.json | 96 +++++- .../session-brief.example.json | 2 +- .../session-brief.schema.json | 4 +- 6 files changed, 599 insertions(+), 70 deletions(-) diff --git a/docs/headless-contract.md b/docs/headless-contract.md index 231b6db..a25bc5f 100644 --- a/docs/headless-contract.md +++ b/docs/headless-contract.md @@ -1,67 +1,288 @@ -# Headless execution contract (coven-github integration) +# coven-code Headless Execution Contract -`coven-code` runs as the **execution runtime** behind the -[`coven-github`](https://github.com/OpenCoven/coven-github) GitHub App. When the -App's worker picks up a task it spawns: +**Contract version: `2`** - Status: **Locked** (V2 / structured review output) + +This document is the single source of truth for the interface between +`coven-github` (the GitHub ingress adapter) and `coven-code` (the execution +runtime) when the runtime is invoked in headless mode. + +It is normative. Where the prose in [`COVEN-GITHUB.md`](../COVEN-GITHUB.md) or any +other doc disagrees with this file, **this file wins**. Both repositories MUST +implement exactly what is specified here, and changes require a contract version +bump (see [Versioning](#versioning)). + +The contract is enforced on the `coven-github` side by golden fixtures in +[`docs/contracts/`](contracts/) and a conformance test +(`crates/github/tests/contract.rs`) that round-trips those fixtures through the +Rust types. `coven-code` MUST validate its emitted `result.json` against +[`docs/contracts/result.schema.json`](contracts/result.schema.json) and its +accepted `session-brief.json` against +[`docs/contracts/session-brief.schema.json`](contracts/session-brief.schema.json). + +The key words MUST, MUST NOT, SHOULD, and MAY are used as in RFC 2119. + +--- + +## 1. Invocation + +The adapter spawns the runtime as a child process: ``` coven-code --headless --context --output ``` -The wire interface between the two repos is **locked** and normative. Its single -source of truth lives in `coven-github`: +| Flag | Meaning | +|---|---| +| `--headless` | Disables the ratatui TUI entirely. All human-facing output is suppressed; the process is non-interactive and reads no stdin. | +| `--context ` | Path to a `session-brief.json` file the adapter has already written. Read-only input. | +| `--output ` | Path the runtime MUST write `result.json` to before exiting `0`, `1`, or `3`. | + +The runtime MUST NOT require a TTY. It MUST NOT block on interactive prompts. -- Contract doc: `docs/headless-contract.md` (contract version **`1`**) -- JSON Schemas + golden fixtures: `docs/contracts/` +### 1.1 Environment -This document describes how `coven-code` **conforms** to that contract. Where -this file disagrees with the canonical contract, the canonical contract wins. +| Variable | Required | Meaning | +|---|---|---| +| `COVEN_GIT_TOKEN` | yes (for any push) | GitHub App **installation access token** used to authenticate `git push` over HTTPS. The runtime MUST use this token for git write operations and MUST NOT use ambient user credentials. | -## Invocation +The token is passed **only** through the environment. It MUST NOT appear in the +session brief, the result envelope, the clone URL, logs, or any durable +artifact. The 1-hour token TTL is the adapter's concern; the runtime treats the +token as opaque and valid for the session. -| Flag | Meaning | +> **Drift note (supersedes `COVEN-GITHUB.md`):** earlier spec prose referenced +> `GIT_ASKPASS` / `GIT_TOKEN` and an `auth.token` field embedded in the brief. +> Those are **removed**. The brief is tokenless (issue #4) and the only git +> credential channel is `COVEN_GIT_TOKEN`. + +--- + +## 2. Input — `session-brief.json` + +The adapter is the **producer**; the runtime is the **consumer**. The brief is +**tokenless**: it carries read context only. + +```json +{ + "contract_version": "2", + "trigger": "issue_assigned", + "repo": { + "owner": "OpenCoven", + "name": "coven-code", + "clone_url": "https://github.com/OpenCoven/coven-code.git", + "default_branch": "main" + }, + "task": { + "kind": "fix_issue", + "issue_number": 42, + "issue_title": "Fix OAuth token refresh", + "issue_body": "The refresh path ignores clock skew…" + }, + "familiar": { + "id": "cody", + "display_name": "Cody", + "model": "anthropic/claude-sonnet-4-6", + "skills": ["systematic-debugging"] + }, + "workspace": { + "root": "/tmp/task-abc123" + } +} +``` + +### 2.1 Fields + +| Field | Type | Notes | +|---|---|---| +| `contract_version` | string | MUST be `"2"`. Consumers MUST reject a brief whose major version they do not implement. | +| `trigger` | string enum | `issue_assigned` \| `pr_review_comment` \| `issue_mention`. | +| `repo.owner` | string | | +| `repo.name` | string | | +| `repo.clone_url` | string | HTTPS clone URL **without** embedded credentials. The runtime supplies auth via `COVEN_GIT_TOKEN`. | +| `repo.default_branch` | string | Resolved from live GitHub metadata, not assumed to be `main` (issue #9). | +| `task` | object | Tagged union discriminated by `kind`. See [2.2](#22-task-kinds). | +| `familiar.id` | string | Stable familiar identifier (e.g. `cody`). | +| `familiar.display_name` | string | Human label used in familiar-voice output. | +| `familiar.model` | string \| null | BYOM model id; `null`/absent means runtime default. | +| `familiar.skills` | string[] | Skill ids to load for the session. MAY be empty. | +| `workspace.root` | string | Absolute path to the pre-cloned, isolated workspace. The runtime operates **inside** this directory and MUST NOT write outside it. | + +### 2.2 Task kinds + +The `task` object is discriminated by a `kind` string (serde +`#[serde(tag = "kind", rename_all = "snake_case")]`). + +| `kind` | Paired `trigger` | Fields | +|---|---|---| +| `fix_issue` | `issue_assigned` | `issue_number: u64`, `issue_title: string`, `issue_body: string` | +| `address_review_comment` | `pr_review_comment` | `pr_number: u64`, `comment_body: string`, `diff_hunk: string \| null` | +| `respond_to_mention` | `issue_mention` | `issue_number: u64`, `comment_body: string` | + +--- + +## 3. Output — `result.json` + +The runtime is the **producer**; the adapter is the **consumer**. The runtime +MUST write this file before exiting `0`, `1`, or `3`. On exit `2` (infra error) +the file MAY be absent. + +```json +{ + "contract_version": "2", + "status": "success", + "branch": "cody/fix-issue-42", + "commits": [ + { "sha": "a1b2c3d", "message": "Add clock-skew buffer to refresh path" } + ], + "files_changed": ["src/auth/refresh.rs"], + "summary": "Fixed OAuth token refresh by adding a 60-second clock skew buffer.", + "pr_body": "## Hey, I'm Cody\n\nI looked at issue #42...", + "review": { + "mode": "pull_request", + "evidence_status": "complete", + "reviewed_files": ["src/auth/refresh.rs"], + "findings": [], + "tests_run": [], + "no_findings_reason": "Reviewed the supplied PR file and found no blocking issues.", + "limitations": [] + }, + "exit_reason": null +} +``` + +### 3.1 Fields + +| Field | Type | Notes | +|---|---|---| +| `contract_version` | string | MUST be `"2"`. Producers MUST emit it. | +| `status` | string enum | `success` \| `failure` \| `partial` \| `needs_input`. See [3.3](#33-status). | +| `branch` | string \| null | Branch the runtime pushed. `null` when no branch was created. The adapter only opens a PR when `branch` is set **and** `commits` is non-empty. | +| `commits` | array | `{ "sha": string, "message": string }`. MAY be empty. | +| `files_changed` | string[] | Workspace-relative paths. MAY be empty. | +| `summary` | string | One-line familiar-voice summary. Used in the Check Run and PR title. | +| `pr_body` | string | Full PR body, **authored by the familiar** in its own voice — not a template. | +| `review` | object | Structured review evidence and findings. Required even when `mode` is `none`. See [3.2](#32-review). | +| `exit_reason` | string enum \| null | `null` on success; otherwise the terminal cause. See [3.4](#34-exit_reason). | + +> **Drift note (supersedes `COVEN-GITHUB.md`):** the prose result envelope listed +> an `events` array. Progress/event streaming is **not** part of the v2 result +> envelope — it is deferred to M2 and will travel over a separate channel. The +> v2 envelope carries terminal task state only. Producers MUST NOT rely on +> `events` being read. + +### 3.2 `review` + +`review` is the machine-readable proof that a hosted review actually examined +the intended code. It is required on every result. Non-review tasks MUST set +`mode: "none"` and `evidence_status: "not_applicable"`. + +| Field | Type | Notes | +|---|---|---| +| `mode` | string enum | `none`, `pull_request`, or `review_comment`. | +| `evidence_status` | string enum | `not_applicable`, `complete`, `partial`, or `missing`. PR review modes MUST NOT use `not_applicable`. | +| `reviewed_files` | string[] | Workspace-relative files supplied to or inspected by the runtime. PR review modes MUST include at least one file unless `evidence_status` is `missing`. | +| `findings` | array | Structured findings. Empty is allowed only when `no_findings_reason` is a non-empty string. | +| `tests_run` | array | Commands run while reviewing, with `passed`, `failed`, `not_run`, or `unknown` status. | +| `no_findings_reason` | string \| null | Required when `mode` is a review mode and `findings` is empty. | +| `limitations` | string[] | Evidence gaps, skipped checks, or other caveats. | + +Each finding carries `severity`, `file`, optional `line`, `title`, `body`, and +optional `recommendation`. Valid severities are `info`, `low`, `medium`, `high`, +and `critical`. + +### 3.3 `status` + +| Value | Meaning | |---|---| -| `--headless` | Disables the ratatui TUI entirely; non-interactive, structured output. Accepted as the canonical headless entry point (also implied by `--context`, `--print`, or a positional prompt). | -| `--context ` | Reads a tokenless session brief (contract §2). Overrides model + working directory from the brief and forces bypass-permissions. A brief whose major `contract_version` this build does not implement is **rejected**. | -| `--output ` | Writes the terminal result envelope (contract §3) before exiting `0`/`1`/`3`. | +| `success` | Work complete; commits made; ready for a PR. | +| `partial` | Some progress committed but the task is not fully done (e.g. tests still failing after the retry budget). The adapter still opens a PR if there are commits. | +| `failure` | The agent gave up; no usable result. | +| `needs_input` | The agent needs human clarification and has posted (or expects the adapter to surface) a question. Pairs with exit code `3`. | + +The adapter treats `success` and `partial` as PR-opening outcomes; `failure` +and `needs_input` do not open a PR by themselves. + +### 3.4 `exit_reason` -## Environment +`null` on success. Otherwise one of: -| Variable | Meaning | +| Value | Meaning | |---|---| -| `COVEN_GIT_TOKEN` | GitHub App installation access token. The **only** git credential channel. On a `--context` run the runtime installs a *local, env-backed* git credential helper in the workspace so `git push` authenticates over HTTPS. The token stays in the environment — it is never written to the brief, the result envelope, `.git/config`, or logs. | +| `test_failure` | Tests could not be made to pass within the retry budget. | +| `ambiguous_spec` | The request is underspecified; the agent chose to ask rather than guess. | +| `git_conflict` | A git conflict the agent could not safely resolve. | +| `infra_error` | Workspace, git, or tool failure. Retry-safe. | -## Exit codes (authoritative — contract §4) +--- -| Code | Meaning | `result.json` | -|---|---|---| -| `0` | success / partial (commits made) | present | -| `1` | failure (agent finished with no usable diff on a change task) | present | -| `2` | infra error (model/tool/workspace failure) — **retry-safe** | best-effort | -| `3` | needs input (reserved; wired for M2) | present | - -The runtime maps its terminal run outcome to these codes: - -- clean finish **with** commits → `success` / exit `0` -- truncation or budget stop **with** commits → `partial` / exit `0` -- clean finish with **no** diff on a change task → `failure` / exit `1` -- reply-only task (`respond_to_mention`) with no diff → `success` / exit `0` -- model / tool / workspace error → `infra_error` / exit `2` - -## Conformance tests - -`crates/cli/src/headless.rs` carries the runtime's contract types and a -`#[cfg(test)]` conformance suite pinned to **vendored** golden fixtures in -`crates/cli/tests/headless_contract/` (verbatim copies of the coven-github -`docs/contracts/` artifacts). The suite asserts: - -- every brief that validates against `session-brief.schema.json` is accepted - (tokenless, version-defaulting, forward-compatible with unknown fields); -- an unsupported major version is rejected; -- every emitted `result.json` validates against `result.schema.json`; -- the exit-code mapping matches §4; -- `COVEN_GIT_TOKEN` never leaks into `.git/config`. - -If a test drifts, the runtime broke the contract **or** the contract changed — -fix one deliberately and bump `contract_version` on both sides. Do not re-bless -fixtures casually. +## 4. Exit codes + +The exit code is the authoritative signal; `status` is advisory detail. The +adapter's dispatch logic (`crates/worker`) keys on the exit code: + +| Code | Name | `result.json` | Adapter behavior | +|---|---|---|---| +| `0` | success | MUST be present | Read result; open draft PR if `branch` + `commits` present; complete Check Run `success` (or `failure` if `status` is `failure`/`needs_input`). | +| `1` | failure | MUST be present | Agent gave up. Mark Check Run `failure` with `summary`. **Not** retried. | +| `2` | infra error | MAY be absent | Retry-safe. Adapter retries up to its configured `max_retries`, then marks `failure`. | +| `3` | needs input | MUST be present (`status: needs_input`) | Agent posted a clarifying question and exited cleanly. Adapter surfaces it; does not retry. | + +A process **killed by signal**, or one that **times out** (the adapter enforces +`worker.timeout_secs`), is treated as a retry-safe failure equivalent to exit +`2`. + +> **Drift note (supersedes `COVEN-GITHUB.md`):** earlier prose described exit `1` +> as "failure: result.json written with exit_reason" and exit `3` as +> "ambiguous". That intent is preserved, but the **locked** semantics are the +> table above: `2` = infra/retry-safe, `3` = needs-input/clean. The adapter's +> retry boundary is exit `2` (and timeout/signal), never exit `1`. + +--- + +## 5. Security invariants + +These are non-negotiable for v2: + +1. The session brief is **tokenless**. Serializing a brief MUST NOT produce an + `auth` field, a `"token"` field, or a credential-bearing `clone_url` + (enforced by `brief_serialization_never_contains_token_or_auth_fields`). +2. The only git credential channel is the `COVEN_GIT_TOKEN` environment + variable. It MUST NOT be persisted to the brief, the result, or logs. +3. GitHub **write authority** (comments, Check Runs, branches, PRs) stays with + the adapter behind its publication gate. The runtime's only direct GitHub + write is `git push` of its working branch, authenticated by the installation + token. +4. The runtime confines all filesystem writes to `workspace.root`. + +--- + +## 6. Versioning + +The contract is versioned by the single `contract_version` string, which tracks +**major** compatibility only. + +- A change that adds an **optional** field, a new enum variant a consumer can + ignore, or clarifies prose is **backward-compatible** and does **not** bump + the version. +- A change that adds a **required** field, removes a field, renames a field, + changes a type, or changes exit-code/status semantics is **breaking** and MUST + bump `contract_version` to `"2"`, update both schemas and fixtures, and ship a + migration note here. +- Consumers MUST reject a payload whose major version they do not implement, + rather than silently mis-parsing it. + +--- + +## 7. Conformance artifacts + +| Artifact | Purpose | +|---|---| +| [`docs/contracts/session-brief.schema.json`](contracts/session-brief.schema.json) | JSON Schema for the input brief. | +| [`docs/contracts/result.schema.json`](contracts/result.schema.json) | JSON Schema for the output envelope. | +| [`docs/contracts/session-brief.example.json`](contracts/session-brief.example.json) | Golden input fixture. | +| [`docs/contracts/result.example.json`](contracts/result.example.json) | Golden output fixture. | +| `crates/github/tests/contract.rs` | Round-trips the golden fixtures through the Rust types — fails the build if the adapter drifts from this contract. | + +A `coven-code` change is contract-conformant when its emitted `result.json` +validates against `result.schema.json` and it accepts any brief that validates +against `session-brief.schema.json`. diff --git a/src-rust/crates/cli/src/headless.rs b/src-rust/crates/cli/src/headless.rs index 47f1501..d005279 100644 --- a/src-rust/crates/cli/src/headless.rs +++ b/src-rust/crates/cli/src/headless.rs @@ -1,4 +1,4 @@ -//! coven-github headless execution contract (contract version `1`). +//! coven-github headless execution contract (contract version `2`). //! //! This module is the **coven-code side** of the interface locked in the //! `coven-github` repo (`docs/headless-contract.md` + `docs/contracts/`). The @@ -31,7 +31,7 @@ use serde::{Deserialize, Serialize}; use std::path::{Path, PathBuf}; /// Major contract version this build implements (contract §6). -pub const CONTRACT_VERSION: &str = "1"; +pub const CONTRACT_VERSION: &str = "2"; /// Environment variable carrying the GitHub App **installation access token** /// used to authenticate `git push`. This is the ONLY git credential channel; it @@ -65,6 +65,8 @@ pub struct SessionBrief { pub task: TaskBrief, pub familiar: FamiliarBrief, pub workspace: WorkspaceBrief, + #[serde(default)] + pub review_context: Option, } #[derive(Debug, Clone, Deserialize)] @@ -111,6 +113,19 @@ pub struct WorkspaceBrief { pub root: String, } +#[derive(Debug, Clone, Deserialize)] +pub struct ReviewContext { + #[serde(default)] + pub kind: Option, + #[serde(default)] + pub files: Vec, +} + +#[derive(Debug, Clone, Deserialize)] +pub struct ReviewContextFile { + pub filename: String, +} + impl SessionBrief { /// Read + parse a brief from a `--context` path. Returns `Ok(None)` when no /// path is given. Rejects a brief whose major contract version this build @@ -158,6 +173,21 @@ impl SessionBrief { matches!(self.task, TaskBrief::RespondToMention { .. }) } + pub fn review_mode(&self) -> ReviewMode { + if matches!(self.task, TaskBrief::AddressReviewComment { .. }) { + return ReviewMode::ReviewComment; + } + if self + .review_context + .as_ref() + .and_then(|context| context.kind.as_deref()) + == Some("pull_request") + { + return ReviewMode::PullRequest; + } + ReviewMode::None + } + /// Build the first-turn user prompt injected into the headless session. pub fn to_prompt(&self) -> String { let mut lines = vec![ @@ -409,7 +439,7 @@ pub enum Status { not(test), expect( dead_code, - reason = "contract v1 reserves needs_input for the M2 clarification path" + reason = "contract v2 reserves needs_input for the M2 clarification path" ) )] NeedsInput, @@ -424,7 +454,7 @@ pub enum ExitReason { not(test), expect( dead_code, - reason = "contract v1 reserves test_failure for future verifier integration" + reason = "contract v2 reserves test_failure for future verifier integration" ) )] TestFailure, @@ -433,7 +463,7 @@ pub enum ExitReason { not(test), expect( dead_code, - reason = "contract v1 reserves git_conflict for future git conflict detection" + reason = "contract v2 reserves git_conflict for future git conflict detection" ) )] GitConflict, @@ -457,9 +487,143 @@ pub struct ResultEnvelope { pub files_changed: Vec, pub summary: String, pub pr_body: String, + pub review: ReviewResult, pub exit_reason: Option, } +#[derive(Debug, Clone, PartialEq, Eq, Serialize)] +pub struct ReviewResult { + pub mode: ReviewMode, + pub evidence_status: ReviewEvidenceStatus, + pub reviewed_files: Vec, + pub findings: Vec, + pub tests_run: Vec, + pub no_findings_reason: Option, + pub limitations: Vec, +} + +impl ReviewResult { + pub fn none() -> Self { + Self { + mode: ReviewMode::None, + evidence_status: ReviewEvidenceStatus::NotApplicable, + reviewed_files: Vec::new(), + findings: Vec::new(), + tests_run: Vec::new(), + no_findings_reason: None, + limitations: Vec::new(), + } + } + + pub fn from_brief(brief: Option<&SessionBrief>) -> Self { + let Some(brief) = brief else { + return Self::none(); + }; + let mode = brief.review_mode(); + if mode == ReviewMode::None { + return Self::none(); + } + + let reviewed_files = brief + .review_context + .as_ref() + .map(|context| { + context + .files + .iter() + .map(|file| file.filename.clone()) + .collect::>() + }) + .unwrap_or_default(); + + let mut limitations = Vec::new(); + let evidence_status = if reviewed_files.is_empty() { + limitations.push("No PR file evidence was supplied in the session brief.".to_string()); + ReviewEvidenceStatus::Missing + } else { + ReviewEvidenceStatus::Complete + }; + + Self { + mode, + evidence_status, + reviewed_files, + findings: Vec::new(), + tests_run: Vec::new(), + no_findings_reason: Some( + "The run completed review mode without returning structured findings; see pr_body for the narrative review." + .to_string(), + ), + limitations, + } + } +} + +#[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize)] +#[serde(rename_all = "snake_case")] +pub enum ReviewMode { + None, + PullRequest, + ReviewComment, +} + +#[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize)] +#[serde(rename_all = "snake_case")] +pub enum ReviewEvidenceStatus { + NotApplicable, + Complete, + #[allow( + dead_code, + reason = "contract v2 reserves partial review evidence for future adapters" + )] + Partial, + Missing, +} + +#[derive(Debug, Clone, PartialEq, Eq, Serialize)] +pub struct ReviewFinding { + pub severity: ReviewSeverity, + pub file: String, + pub line: Option, + pub title: String, + pub body: String, + pub recommendation: Option, +} + +#[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize)] +#[serde(rename_all = "snake_case")] +#[allow( + dead_code, + reason = "contract v2 reserves structured findings for the review parser" +)] +pub enum ReviewSeverity { + Info, + Low, + Medium, + High, + Critical, +} + +#[derive(Debug, Clone, PartialEq, Eq, Serialize)] +pub struct ReviewTestRun { + pub command: String, + pub status: ReviewTestStatus, + pub output_summary: Option, +} + +#[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize)] +#[serde(rename_all = "snake_case")] +#[allow( + dead_code, + reason = "contract v2 reserves structured test evidence for verifier integration" +)] +pub enum ReviewTestStatus { + Passed, + Failed, + NotRun, + Unknown, +} + /// How the headless run terminated, decoupled from the query crate so this /// module stays independently testable. #[derive(Debug, Clone, Copy, PartialEq, Eq)] @@ -544,6 +708,7 @@ pub fn build_result( files_changed: git.files_changed.clone(), summary, pr_body, + review: ReviewResult::from_brief(brief), exit_reason, }; (envelope, code) @@ -567,6 +732,7 @@ pub fn infra_error_result( pr_body: format!( "## {name}\n\nThe headless session failed before completing the task:\n\n```\n{message}\n```" ), + review: ReviewResult::from_brief(brief), exit_reason: Some(ExitReason::InfraError), }; (envelope, 2) @@ -714,7 +880,7 @@ mod tests { .. } )); - brief.ensure_supported_version().expect("v1 is supported"); + brief.ensure_supported_version().expect("v2 is supported"); } #[test] @@ -733,7 +899,7 @@ mod tests { // The adapter emits a tokenless brief; the runtime must NOT require an // `auth`/`token` field to parse it. let raw = r#"{ - "contract_version": "1", + "contract_version": "2", "trigger": "issue_assigned", "repo": { "owner": "o", "name": "r", "clone_url": "https://github.com/o/r.git", "default_branch": "main" }, "task": { "kind": "fix_issue", "issue_number": 1, "issue_title": "t", "issue_body": "b" }, @@ -765,7 +931,7 @@ mod tests { // Contract §6: additive fields within a major version are backward // compatible; the consumer must not choke on them. let raw = r#"{ - "contract_version": "1", + "contract_version": "2", "trigger": "issue_assigned", "repo": { "owner": "o", "name": "r", "clone_url": "https://github.com/o/r.git", "default_branch": "main", "topics": ["x"] }, "task": { "kind": "fix_issue", "issue_number": 1, "issue_title": "t", "issue_body": "b" }, @@ -780,10 +946,10 @@ mod tests { #[test] fn rejects_unsupported_major_version() { let mut brief = sample_brief(); - brief.contract_version = "2".to_string(); + brief.contract_version = "3".to_string(); assert!( brief.ensure_supported_version().is_err(), - "a v2 brief must be rejected by a v1 runtime" + "a v3 brief must be rejected by a v2 runtime" ); } @@ -844,7 +1010,7 @@ mod tests { "result has key `{key}` not permitted by schema (additionalProperties:false)" ); } - assert_eq!(obj["contract_version"], json!("1")); + assert_eq!(obj["contract_version"], json!("2")); let status_enum = props["status"]["enum"].as_array().unwrap(); assert!(status_enum.contains(&obj["status"]), "status out of enum"); @@ -875,11 +1041,48 @@ mod tests { ); assert_eq!(code, 0); assert_eq!(env.status, Status::Success); + assert_eq!(env.review.mode, ReviewMode::None); assert!(env.exit_reason.is_none()); let value = serde_json::to_value(&env).unwrap(); assert_matches_result_schema(&value); } + #[test] + fn review_context_produces_structured_pr_review_evidence() { + let raw = r#"{ + "contract_version": "2", + "trigger": "issue_mention", + "repo": { "owner": "o", "name": "r", "clone_url": "https://github.com/o/r.git", "default_branch": "main" }, + "task": { "kind": "respond_to_mention", "issue_number": 7, "comment_body": "review this" }, + "familiar": { "id": "cody", "display_name": "Cody", "skills": [] }, + "workspace": { "root": "/tmp/ws" }, + "review_context": { + "kind": "pull_request", + "files": [ + { "filename": "src/lib.rs" }, + { "filename": "README.md" } + ] + } + }"#; + let brief: SessionBrief = serde_json::from_str(raw).expect("brief parses"); + let (env, code) = build_result( + Some(&brief), + &GitSummary::default(), + RunOutcome::Completed, + "Reviewed the PR.", + ); + + assert_eq!(code, 0); + assert_eq!(env.review.mode, ReviewMode::PullRequest); + assert_eq!(env.review.evidence_status, ReviewEvidenceStatus::Complete); + assert_eq!( + env.review.reviewed_files, + vec!["src/lib.rs".to_string(), "README.md".to_string()] + ); + assert!(env.review.findings.is_empty()); + assert!(env.review.no_findings_reason.is_some()); + } + #[test] fn infra_error_envelope_validates_and_is_retry_safe() { let (env, code) = infra_error_result(None, &GitSummary::default(), "workspace vanished"); diff --git a/src-rust/crates/cli/tests/headless_contract/result.example.json b/src-rust/crates/cli/tests/headless_contract/result.example.json index 53fd6ac..c6afd86 100644 --- a/src-rust/crates/cli/tests/headless_contract/result.example.json +++ b/src-rust/crates/cli/tests/headless_contract/result.example.json @@ -1,5 +1,5 @@ { - "contract_version": "1", + "contract_version": "2", "status": "success", "branch": "cody/fix-issue-42", "commits": [ @@ -7,6 +7,21 @@ ], "files_changed": ["src/auth/refresh.rs"], "summary": "Fixed OAuth token refresh by adding a 60-second clock skew buffer.", - "pr_body": "## Hey, I'm Cody 🦄\n\nIssue #42 traced to the OAuth refresh path not accounting for clock skew. I added a 60-second buffer to the expiry check.\n\n**Changed:** `src/auth/refresh.rs` (+12 / -3)\n**Tests:** 8/8 passing (added 2 regression cases)\n", + "pr_body": "## Hey, I'm Cody\n\nIssue #42 traced to the OAuth refresh path not accounting for clock skew. I added a 60-second buffer to the expiry check.\n\n**Changed:** `src/auth/refresh.rs` (+12 / -3)\n**Tests:** 8/8 passing (added 2 regression cases)\n", + "review": { + "mode": "none", + "evidence_status": "not_applicable", + "reviewed_files": [], + "findings": [], + "tests_run": [ + { + "command": "cargo test -p auth refresh", + "status": "passed", + "output_summary": "8/8 passing" + } + ], + "no_findings_reason": null, + "limitations": [] + }, "exit_reason": null } diff --git a/src-rust/crates/cli/tests/headless_contract/result.schema.json b/src-rust/crates/cli/tests/headless_contract/result.schema.json index 0a86c4c..434c6e4 100644 --- a/src-rust/crates/cli/tests/headless_contract/result.schema.json +++ b/src-rust/crates/cli/tests/headless_contract/result.schema.json @@ -2,14 +2,14 @@ "$schema": "https://json-schema.org/draft/2020-12/schema", "$id": "https://github.com/OpenCoven/coven-github/docs/contracts/result.schema.json", "title": "coven-code headless result envelope", - "description": "Terminal task state coven-code --headless writes to --output. Contract version 1.", + "description": "Terminal task state coven-code --headless writes to --output. Contract version 2.", "type": "object", "additionalProperties": false, - "required": ["status", "commits", "files_changed", "summary", "pr_body"], + "required": ["contract_version", "status", "commits", "files_changed", "summary", "pr_body", "review"], "properties": { "contract_version": { "type": "string", - "const": "1" + "const": "2" }, "status": { "type": "string", @@ -36,6 +36,96 @@ }, "summary": { "type": "string" }, "pr_body": { "type": "string" }, + "review": { + "type": "object", + "additionalProperties": false, + "required": [ + "mode", + "evidence_status", + "reviewed_files", + "findings", + "tests_run", + "no_findings_reason", + "limitations" + ], + "properties": { + "mode": { + "type": "string", + "enum": ["none", "pull_request", "review_comment"] + }, + "evidence_status": { + "type": "string", + "enum": ["not_applicable", "complete", "partial", "missing"] + }, + "reviewed_files": { + "type": "array", + "items": { "type": "string" } + }, + "findings": { + "type": "array", + "items": { + "type": "object", + "additionalProperties": false, + "required": ["severity", "file", "line", "title", "body", "recommendation"], + "properties": { + "severity": { + "type": "string", + "enum": ["info", "low", "medium", "high", "critical"] + }, + "file": { "type": "string" }, + "line": { "type": ["integer", "null"], "minimum": 1 }, + "title": { "type": "string" }, + "body": { "type": "string" }, + "recommendation": { "type": ["string", "null"] } + } + } + }, + "tests_run": { + "type": "array", + "items": { + "type": "object", + "additionalProperties": false, + "required": ["command", "status", "output_summary"], + "properties": { + "command": { "type": "string" }, + "status": { + "type": "string", + "enum": ["passed", "failed", "not_run", "unknown"] + }, + "output_summary": { "type": ["string", "null"] } + } + } + }, + "no_findings_reason": { "type": ["string", "null"] }, + "limitations": { + "type": "array", + "items": { "type": "string" } + } + }, + "allOf": [ + { + "if": { + "properties": { "mode": { "enum": ["pull_request", "review_comment"] } }, + "required": ["mode"] + }, + "then": { + "anyOf": [ + { "properties": { "reviewed_files": { "minItems": 1 } }, "required": ["reviewed_files"] }, + { "properties": { "evidence_status": { "const": "missing" } }, "required": ["evidence_status"] } + ], + "oneOf": [ + { "properties": { "findings": { "minItems": 1 } }, "required": ["findings"] }, + { + "properties": { + "no_findings_reason": { "type": "string", "minLength": 1 } + }, + "required": ["no_findings_reason"] + } + ] + } + } + ] + }, "exit_reason": { "type": ["string", "null"], "enum": ["test_failure", "ambiguous_spec", "git_conflict", "infra_error", null] diff --git a/src-rust/crates/cli/tests/headless_contract/session-brief.example.json b/src-rust/crates/cli/tests/headless_contract/session-brief.example.json index 8ecbc9a..92719bd 100644 --- a/src-rust/crates/cli/tests/headless_contract/session-brief.example.json +++ b/src-rust/crates/cli/tests/headless_contract/session-brief.example.json @@ -1,5 +1,5 @@ { - "contract_version": "1", + "contract_version": "2", "trigger": "issue_assigned", "repo": { "owner": "OpenCoven", diff --git a/src-rust/crates/cli/tests/headless_contract/session-brief.schema.json b/src-rust/crates/cli/tests/headless_contract/session-brief.schema.json index 613a352..850c31e 100644 --- a/src-rust/crates/cli/tests/headless_contract/session-brief.schema.json +++ b/src-rust/crates/cli/tests/headless_contract/session-brief.schema.json @@ -2,14 +2,14 @@ "$schema": "https://json-schema.org/draft/2020-12/schema", "$id": "https://github.com/OpenCoven/coven-github/docs/contracts/session-brief.schema.json", "title": "coven-code headless session brief", - "description": "Tokenless read-context payload the adapter writes for coven-code --headless --context. Contract version 1.", + "description": "Tokenless read-context payload the adapter writes for coven-code --headless --context. Contract version 2.", "type": "object", "additionalProperties": false, "required": ["trigger", "repo", "task", "familiar", "workspace"], "properties": { "contract_version": { "type": "string", - "const": "1" + "const": "2" }, "trigger": { "type": "string", From 9e2e44e26a8febd17b43b471849ab97939d79907 Mon Sep 17 00:00:00 2001 From: Timothy Wayne Gregg Date: Sun, 5 Jul 2026 22:22:38 -0400 Subject: [PATCH 2/2] feat(headless): expose supporting review evidence gap (#119) Signed-off-by: Timothy Wayne Gregg --- docs/headless-contract.md | 2 ++ src-rust/crates/cli/src/headless.rs | 13 +++++++++++++ .../cli/tests/headless_contract/result.example.json | 1 + .../cli/tests/headless_contract/result.schema.json | 5 +++++ 4 files changed, 21 insertions(+) diff --git a/docs/headless-contract.md b/docs/headless-contract.md index a25bc5f..23f3fc6 100644 --- a/docs/headless-contract.md +++ b/docs/headless-contract.md @@ -141,6 +141,7 @@ the file MAY be absent. "mode": "pull_request", "evidence_status": "complete", "reviewed_files": ["src/auth/refresh.rs"], + "supporting_files": ["src/auth/mod.rs", "tests/auth_refresh.rs"], "findings": [], "tests_run": [], "no_findings_reason": "Reviewed the supplied PR file and found no blocking issues.", @@ -181,6 +182,7 @@ the intended code. It is required on every result. Non-review tasks MUST set | `mode` | string enum | `none`, `pull_request`, or `review_comment`. | | `evidence_status` | string enum | `not_applicable`, `complete`, `partial`, or `missing`. PR review modes MUST NOT use `not_applicable`. | | `reviewed_files` | string[] | Workspace-relative files supplied to or inspected by the runtime. PR review modes MUST include at least one file unless `evidence_status` is `missing`. | +| `supporting_files` | string[] | Workspace-relative files beyond the changed-file list that the runtime can prove were inspected for context. Empty means no broader-codebase inspection was proven, not that none was needed. | | `findings` | array | Structured findings. Empty is allowed only when `no_findings_reason` is a non-empty string. | | `tests_run` | array | Commands run while reviewing, with `passed`, `failed`, `not_run`, or `unknown` status. | | `no_findings_reason` | string \| null | Required when `mode` is a review mode and `findings` is empty. | diff --git a/src-rust/crates/cli/src/headless.rs b/src-rust/crates/cli/src/headless.rs index d005279..73bc3b2 100644 --- a/src-rust/crates/cli/src/headless.rs +++ b/src-rust/crates/cli/src/headless.rs @@ -496,6 +496,7 @@ pub struct ReviewResult { pub mode: ReviewMode, pub evidence_status: ReviewEvidenceStatus, pub reviewed_files: Vec, + pub supporting_files: Vec, pub findings: Vec, pub tests_run: Vec, pub no_findings_reason: Option, @@ -508,6 +509,7 @@ impl ReviewResult { mode: ReviewMode::None, evidence_status: ReviewEvidenceStatus::NotApplicable, reviewed_files: Vec::new(), + supporting_files: Vec::new(), findings: Vec::new(), tests_run: Vec::new(), no_findings_reason: None, @@ -543,11 +545,16 @@ impl ReviewResult { } else { ReviewEvidenceStatus::Complete }; + limitations.push( + "Supporting-code inspection is not yet trace-backed; supporting_files is empty until tool-read tracing is wired." + .to_string(), + ); Self { mode, evidence_status, reviewed_files, + supporting_files: Vec::new(), findings: Vec::new(), tests_run: Vec::new(), no_findings_reason: Some( @@ -1079,8 +1086,14 @@ mod tests { env.review.reviewed_files, vec!["src/lib.rs".to_string(), "README.md".to_string()] ); + assert!(env.review.supporting_files.is_empty()); assert!(env.review.findings.is_empty()); assert!(env.review.no_findings_reason.is_some()); + assert!(env + .review + .limitations + .iter() + .any(|item| item.contains("Supporting-code inspection"))); } #[test] diff --git a/src-rust/crates/cli/tests/headless_contract/result.example.json b/src-rust/crates/cli/tests/headless_contract/result.example.json index c6afd86..7796d31 100644 --- a/src-rust/crates/cli/tests/headless_contract/result.example.json +++ b/src-rust/crates/cli/tests/headless_contract/result.example.json @@ -12,6 +12,7 @@ "mode": "none", "evidence_status": "not_applicable", "reviewed_files": [], + "supporting_files": [], "findings": [], "tests_run": [ { diff --git a/src-rust/crates/cli/tests/headless_contract/result.schema.json b/src-rust/crates/cli/tests/headless_contract/result.schema.json index 434c6e4..6b7167b 100644 --- a/src-rust/crates/cli/tests/headless_contract/result.schema.json +++ b/src-rust/crates/cli/tests/headless_contract/result.schema.json @@ -43,6 +43,7 @@ "mode", "evidence_status", "reviewed_files", + "supporting_files", "findings", "tests_run", "no_findings_reason", @@ -61,6 +62,10 @@ "type": "array", "items": { "type": "string" } }, + "supporting_files": { + "type": "array", + "items": { "type": "string" } + }, "findings": { "type": "array", "items": {