From 61909e8b5d0d7d8a2c12f662c05ec676d7f029fa Mon Sep 17 00:00:00 2001 From: Timothy Wayne Gregg Date: Sun, 5 Jul 2026 20:42:35 -0400 Subject: [PATCH 01/19] feat(contract): require structured review results (#119) --- crates/github/src/lib.rs | 81 ++++++++++++++++++- crates/github/src/tasks.rs | 1 + crates/worker/src/lib.rs | 5 +- crates/worker/tests/contract.rs | 19 ++++- docs/contracts/result.example.json | 19 ++++- docs/contracts/result.schema.json | 96 ++++++++++++++++++++++- docs/contracts/session-brief.example.json | 2 +- docs/contracts/session-brief.schema.json | 4 +- docs/headless-contract.md | 56 ++++++++++--- 9 files changed, 258 insertions(+), 25 deletions(-) diff --git a/crates/github/src/lib.rs b/crates/github/src/lib.rs index 5e05f32..bd19597 100644 --- a/crates/github/src/lib.rs +++ b/crates/github/src/lib.rs @@ -12,7 +12,7 @@ pub const DEFAULT_API_BASE_URL: &str = "https://api.github.com"; /// Major version of the coven-code headless execution contract this adapter /// speaks. See `docs/headless-contract.md`. Bump only on breaking changes. -pub const HEADLESS_CONTRACT_VERSION: &str = "1"; +pub const HEADLESS_CONTRACT_VERSION: &str = "2"; fn default_contract_version() -> String { HEADLESS_CONTRACT_VERSION.to_string() @@ -187,6 +187,7 @@ pub struct SessionResult { pub files_changed: Vec, pub summary: String, pub pr_body: String, + pub review: ReviewResult, pub exit_reason: Option, } @@ -205,6 +206,84 @@ pub struct CommitInfo { pub message: String, } +#[derive(Debug, Clone, Deserialize, 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(), + } + } +} + +#[derive(Debug, Clone, Deserialize, Serialize, PartialEq, Eq)] +#[serde(rename_all = "snake_case")] +pub enum ReviewMode { + None, + PullRequest, + ReviewComment, +} + +#[derive(Debug, Clone, Deserialize, Serialize, PartialEq, Eq)] +#[serde(rename_all = "snake_case")] +pub enum ReviewEvidenceStatus { + NotApplicable, + Complete, + Partial, + Missing, +} + +#[derive(Debug, Clone, Deserialize, 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, Deserialize, Serialize, PartialEq, Eq)] +#[serde(rename_all = "snake_case")] +pub enum ReviewSeverity { + Info, + Low, + Medium, + High, + Critical, +} + +#[derive(Debug, Clone, Deserialize, Serialize)] +pub struct ReviewTestRun { + pub command: String, + pub status: ReviewTestStatus, + pub output_summary: Option, +} + +#[derive(Debug, Clone, Deserialize, Serialize, PartialEq, Eq)] +#[serde(rename_all = "snake_case")] +pub enum ReviewTestStatus { + Passed, + Failed, + NotRun, + Unknown, +} + #[derive(Debug, Clone, Deserialize, Serialize, PartialEq, Eq)] #[serde(rename_all = "snake_case")] pub enum ExitReason { diff --git a/crates/github/src/tasks.rs b/crates/github/src/tasks.rs index 4b1aff8..b35bf6c 100644 --- a/crates/github/src/tasks.rs +++ b/crates/github/src/tasks.rs @@ -191,6 +191,7 @@ mod tests { files_changed: vec![], summary: "Done".to_string(), pr_body: "Body".to_string(), + review: crate::ReviewResult::none(), exit_reason: None, }, Some(9), diff --git a/crates/worker/src/lib.rs b/crates/worker/src/lib.rs index c26f7da..e99c107 100644 --- a/crates/worker/src/lib.rs +++ b/crates/worker/src/lib.rs @@ -625,7 +625,7 @@ fn task_issue_number(kind: &TaskKind) -> Option { #[cfg(test)] mod disposition_tests { use super::*; - use coven_github_api::{CommitInfo, HEADLESS_CONTRACT_VERSION}; + use coven_github_api::{CommitInfo, ReviewResult, HEADLESS_CONTRACT_VERSION}; use coven_github_config::{GitHubAppConfig, ServerConfig, WorkerConfig}; use std::path::PathBuf; @@ -643,6 +643,7 @@ mod disposition_tests { files_changed: vec![], summary: "summary".to_string(), pr_body: "body".to_string(), + review: ReviewResult::none(), exit_reason: None, } } @@ -828,7 +829,7 @@ mod process_tests { /// A minimal contract-valid result.json with the given status/exit_reason. fn result_json(status: &str, exit_reason: &str) -> String { format!( - r#"{{"contract_version":"1","status":"{status}","branch":null,"commits":[],"files_changed":[],"summary":"s","pr_body":"","exit_reason":{exit_reason}}}"# + r#"{{"contract_version":"2","status":"{status}","branch":null,"commits":[],"files_changed":[],"summary":"s","pr_body":"","review":{{"mode":"none","evidence_status":"not_applicable","reviewed_files":[],"findings":[],"tests_run":[],"no_findings_reason":null,"limitations":[]}},"exit_reason":{exit_reason}}}"# ) } diff --git a/crates/worker/tests/contract.rs b/crates/worker/tests/contract.rs index b5cfd91..238b03a 100644 --- a/crates/worker/tests/contract.rs +++ b/crates/worker/tests/contract.rs @@ -5,7 +5,10 @@ //! If these fail, either the runtime contract drifted or the docs are stale โ€” //! fix one of them, do not just bless the test. -use coven_github_api::{ExitReason, SessionResult, SessionStatus, HEADLESS_CONTRACT_VERSION}; +use coven_github_api::{ + ExitReason, ReviewEvidenceStatus, ReviewMode, SessionResult, SessionStatus, + HEADLESS_CONTRACT_VERSION, +}; use coven_github_worker::brief::SessionBrief; fn fixture(name: &str) -> String { @@ -43,6 +46,11 @@ fn golden_result_deserializes_into_adapter_type() { assert_eq!(result.contract_version, HEADLESS_CONTRACT_VERSION); assert_eq!(result.status, SessionStatus::Success); + assert_eq!(result.review.mode, ReviewMode::None); + assert_eq!( + result.review.evidence_status, + ReviewEvidenceStatus::NotApplicable + ); assert_eq!(result.branch.as_deref(), Some("cody/fix-issue-42")); assert_eq!(result.commits.len(), 1); assert_eq!( @@ -63,6 +71,15 @@ fn result_without_contract_version_defaults_to_current() { "files_changed": [], "summary": "Could not reproduce.", "pr_body": "", + "review": { + "mode": "none", + "evidence_status": "not_applicable", + "reviewed_files": [], + "findings": [], + "tests_run": [], + "no_findings_reason": null, + "limitations": [] + }, "exit_reason": "ambiguous_spec" }"#; let result: SessionResult = serde_json::from_str(raw).expect("legacy result must parse"); diff --git a/docs/contracts/result.example.json b/docs/contracts/result.example.json index 53fd6ac..c6afd86 100644 --- a/docs/contracts/result.example.json +++ b/docs/contracts/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/docs/contracts/result.schema.json b/docs/contracts/result.schema.json index 0a86c4c..434c6e4 100644 --- a/docs/contracts/result.schema.json +++ b/docs/contracts/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/docs/contracts/session-brief.example.json b/docs/contracts/session-brief.example.json index 8ecbc9a..92719bd 100644 --- a/docs/contracts/session-brief.example.json +++ b/docs/contracts/session-brief.example.json @@ -1,5 +1,5 @@ { - "contract_version": "1", + "contract_version": "2", "trigger": "issue_assigned", "repo": { "owner": "OpenCoven", diff --git a/docs/contracts/session-brief.schema.json b/docs/contracts/session-brief.schema.json index 613a352..850c31e 100644 --- a/docs/contracts/session-brief.schema.json +++ b/docs/contracts/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", diff --git a/docs/headless-contract.md b/docs/headless-contract.md index f71eb54..a25bc5f 100644 --- a/docs/headless-contract.md +++ b/docs/headless-contract.md @@ -1,6 +1,6 @@ # coven-code Headless Execution Contract -**Contract version: `1`** ยท Status: **Locked** (V1 / M1) +**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 @@ -64,7 +64,7 @@ The adapter is the **producer**; the runtime is the **consumer**. The brief is ```json { - "contract_version": "1", + "contract_version": "2", "trigger": "issue_assigned", "repo": { "owner": "OpenCoven", @@ -94,7 +94,7 @@ The adapter is the **producer**; the runtime is the **consumer**. The brief is | Field | Type | Notes | |---|---|---| -| `contract_version` | string | MUST be `"1"`. Consumers MUST reject a brief whose major version they do not implement. | +| `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 | | @@ -128,7 +128,7 @@ the file MAY be absent. ```json { - "contract_version": "1", + "contract_version": "2", "status": "success", "branch": "cody/fix-issue-42", "commits": [ @@ -136,7 +136,16 @@ the file MAY be absent. ], "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โ€ฆ", + "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 } ``` @@ -145,22 +154,43 @@ the file MAY be absent. | Field | Type | Notes | |---|---|---| -| `contract_version` | string | MUST be `"1"`. If absent, the consumer assumes `"1"` for backward compatibility, but producers MUST emit it. | -| `status` | string enum | `success` \| `failure` \| `partial` \| `needs_input`. See [3.2](#32-status). | +| `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. | -| `exit_reason` | string enum \| null | `null` on success; otherwise the terminal cause. See [3.3](#33-exit_reason). | +| `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 v1 result +> 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 -> v1 envelope carries terminal task state only. Producers MUST NOT rely on +> v2 envelope carries terminal task state only. Producers MUST NOT rely on > `events` being read. -### 3.2 `status` +### 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 | |---|---| @@ -172,7 +202,7 @@ the file MAY be absent. The adapter treats `success` and `partial` as PR-opening outcomes; `failure` and `needs_input` do not open a PR by themselves. -### 3.3 `exit_reason` +### 3.4 `exit_reason` `null` on success. Otherwise one of: @@ -211,7 +241,7 @@ A process **killed by signal**, or one that **times out** (the adapter enforces ## 5. Security invariants -These are non-negotiable for v1: +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` From fdc255f0cb1357b0faa2cef543a00ae17bbeeac0 Mon Sep 17 00:00:00 2001 From: Timothy Wayne Gregg Date: Sun, 5 Jul 2026 22:19:24 -0400 Subject: [PATCH 02/19] feat(contract): record supporting review files (#119) Signed-off-by: Timothy Wayne Gregg --- crates/github/src/lib.rs | 2 ++ crates/worker/src/lib.rs | 2 +- crates/worker/tests/contract.rs | 1 + docs/contracts/result.example.json | 1 + docs/contracts/result.schema.json | 5 +++++ docs/headless-contract.md | 2 ++ 6 files changed, 12 insertions(+), 1 deletion(-) diff --git a/crates/github/src/lib.rs b/crates/github/src/lib.rs index bd19597..97d04cf 100644 --- a/crates/github/src/lib.rs +++ b/crates/github/src/lib.rs @@ -211,6 +211,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, @@ -223,6 +224,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, diff --git a/crates/worker/src/lib.rs b/crates/worker/src/lib.rs index e99c107..bc8372c 100644 --- a/crates/worker/src/lib.rs +++ b/crates/worker/src/lib.rs @@ -829,7 +829,7 @@ mod process_tests { /// A minimal contract-valid result.json with the given status/exit_reason. fn result_json(status: &str, exit_reason: &str) -> String { format!( - r#"{{"contract_version":"2","status":"{status}","branch":null,"commits":[],"files_changed":[],"summary":"s","pr_body":"","review":{{"mode":"none","evidence_status":"not_applicable","reviewed_files":[],"findings":[],"tests_run":[],"no_findings_reason":null,"limitations":[]}},"exit_reason":{exit_reason}}}"# + r#"{{"contract_version":"2","status":"{status}","branch":null,"commits":[],"files_changed":[],"summary":"s","pr_body":"","review":{{"mode":"none","evidence_status":"not_applicable","reviewed_files":[],"supporting_files":[],"findings":[],"tests_run":[],"no_findings_reason":null,"limitations":[]}},"exit_reason":{exit_reason}}}"# ) } diff --git a/crates/worker/tests/contract.rs b/crates/worker/tests/contract.rs index 238b03a..45387e1 100644 --- a/crates/worker/tests/contract.rs +++ b/crates/worker/tests/contract.rs @@ -75,6 +75,7 @@ fn result_without_contract_version_defaults_to_current() { "mode": "none", "evidence_status": "not_applicable", "reviewed_files": [], + "supporting_files": [], "findings": [], "tests_run": [], "no_findings_reason": null, diff --git a/docs/contracts/result.example.json b/docs/contracts/result.example.json index c6afd86..7796d31 100644 --- a/docs/contracts/result.example.json +++ b/docs/contracts/result.example.json @@ -12,6 +12,7 @@ "mode": "none", "evidence_status": "not_applicable", "reviewed_files": [], + "supporting_files": [], "findings": [], "tests_run": [ { diff --git a/docs/contracts/result.schema.json b/docs/contracts/result.schema.json index 434c6e4..6b7167b 100644 --- a/docs/contracts/result.schema.json +++ b/docs/contracts/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": { 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. | From 44d266323fc52c91a4aee17ed068bae289ecebe0 Mon Sep 17 00:00:00 2001 From: Timothy Wayne Gregg Date: Sun, 5 Jul 2026 22:30:49 -0400 Subject: [PATCH 03/19] chore(hosted): track CompleteTech dogfood adapter (#119) Signed-off-by: Timothy Wayne Gregg --- .gitignore | 3 + deploy/complete-tech/README.md | 36 + deploy/complete-tech/coven_github_adapter.py | 955 +++++++++++++++++++ 3 files changed, 994 insertions(+) create mode 100644 deploy/complete-tech/README.md create mode 100644 deploy/complete-tech/coven_github_adapter.py diff --git a/.gitignore b/.gitignore index 1e3c192..a8407f6 100644 --- a/.gitignore +++ b/.gitignore @@ -4,3 +4,6 @@ keys/ *.pem .env .DS_Store +deploy/complete-tech/.coven-github-private-key.pem +deploy/complete-tech/coven-github-policy.json +deploy/complete-tech/coven-github-state/ diff --git a/deploy/complete-tech/README.md b/deploy/complete-tech/README.md new file mode 100644 index 0000000..5c695ae --- /dev/null +++ b/deploy/complete-tech/README.md @@ -0,0 +1,36 @@ +# CompleteTech hosted dogfood adapter + +This directory tracks the Python adapter currently deployed for the +CompleteTech-hosted dogfood GitHub App at `webhook.complete.tech`. + +The adapter is intentionally deployment-specific. It is not the canonical Rust +worker implementation; it exists so hosted dogfood behavior can be reviewed, +reproduced, and changed through PRs instead of invisible server edits. + +## Files + +- `coven_github_adapter.py` - webhook handler, task runner, PR evidence capture, + Codex-backed headless runtime invocation, and dogfood comment publisher. + +## Runtime inputs + +The deployment expects secrets and mutable state to be supplied outside git: + +- `GITHUB_APP_PRIVATE_KEY_PATH` or `.coven-github-private-key.pem` +- `GITHUB_APP_ID` +- `COVEN_GITHUB_STATE_DIR` +- `COVEN_GITHUB_POLICY_PATH` +- `COVEN_CODE_BIN` +- Codex OAuth tokens under the deployed account's `.coven-code` directory + +Do not commit private keys, webhook secrets, OAuth tokens, generated task state, +workspaces, or attempt artifacts. + +## Current dogfood behavior + +- Emits headless contract v2 session briefs. +- Captures PR checkout metadata and changed-file patches before invoking + `coven-code`. +- Publishes visible structured review evidence, including `reviewed_files`, + `supporting_files`, findings, test evidence, no-findings rationale, and + limitations. diff --git a/deploy/complete-tech/coven_github_adapter.py b/deploy/complete-tech/coven_github_adapter.py new file mode 100644 index 0000000..2d1b6ef --- /dev/null +++ b/deploy/complete-tech/coven_github_adapter.py @@ -0,0 +1,955 @@ +import base64 +import hashlib +import json +import os +import subprocess +import tempfile +import time +import traceback +from datetime import datetime, timezone +from pathlib import Path +from urllib.error import HTTPError +from urllib.request import Request, urlopen + + +ROOT_DIR = Path(__file__).resolve().parent +STATE_DIR = Path(os.environ.get("COVEN_GITHUB_STATE_DIR", ROOT_DIR / "coven-github-state")) +DELIVERIES_DIR = STATE_DIR / "deliveries" +TASKS_DIR = STATE_DIR / "tasks" +WORKSPACES_DIR = STATE_DIR / "workspaces" +ATTEMPTS_DIR = STATE_DIR / "attempts" +POLICY_PATH = Path(os.environ.get("COVEN_GITHUB_POLICY_PATH", ROOT_DIR / "coven-github-policy.json")) +PRIVATE_KEY_PATH = Path( + os.environ.get("GITHUB_APP_PRIVATE_KEY_PATH", ROOT_DIR / ".coven-github-private-key.pem") +) +APP_ID = os.environ.get("GITHUB_APP_ID", "4224630").strip() +COVEN_CODE_BIN = os.environ.get("COVEN_CODE_BIN", "coven-code").strip() or "coven-code" +COVEN_CODE_MODEL = os.environ.get("COVEN_CODE_MODEL", "gpt-5.5").strip() + + +def account_home(): + try: + import pwd + + return Path(pwd.getpwuid(os.getuid()).pw_dir) + except Exception: + return Path.home() + + +def configured_codex_tokens_path(): + configured = os.environ.get("COVEN_CODE_CODEX_TOKENS_PATH", "").strip() + if configured: + return Path(configured).expanduser() + return account_home() / ".coven-code" / "codex_tokens.json" + + +CODEX_TOKENS_PATH = configured_codex_tokens_path() + +for directory in (DELIVERIES_DIR, TASKS_DIR, WORKSPACES_DIR, ATTEMPTS_DIR): + directory.mkdir(parents=True, exist_ok=True) + + +DEFAULT_POLICY = { + "version": 1, + "installations": { + "144638200": { + "repositories": { + "1290307745": { + "full_name": "CompleteTech-LLC-AI-Research/coven-code", + "default_branch": "main", + "enabled_triggers": [ + "issue_mention", + "issue_label", + "pr_review_comment", + "pull_request", + "push", + ], + "trigger_labels": ["coven", "coven:fix", "coven:review"], + "bot_usernames": ["coven-github", "cody"], + "familiar": { + "id": "cody", + "display_name": "Cody", + "model": None, + "skills": ["code-review"], + }, + "publication": {"mode": "comment"}, + } + } + } + }, +} + + +def utc_now(): + return datetime.now(timezone.utc).isoformat() + + +def read_json(path, default): + try: + with open(path, "r", encoding="utf-8") as handle: + return json.load(handle) + except FileNotFoundError: + return default + + +def write_json_atomic(path, value): + path.parent.mkdir(parents=True, exist_ok=True) + fd, tmp_name = tempfile.mkstemp(prefix=path.name + ".", suffix=".tmp", dir=str(path.parent)) + try: + with os.fdopen(fd, "w", encoding="utf-8") as handle: + json.dump(value, handle, sort_keys=True, indent=2) + handle.write("\n") + os.replace(tmp_name, str(path)) + finally: + if os.path.exists(tmp_name): + os.unlink(tmp_name) + + +def b64url(raw): + return base64.urlsafe_b64encode(raw).rstrip(b"=").decode("ascii") + + +def sign_rs256(message): + from cryptography.hazmat.primitives import hashes, serialization + from cryptography.hazmat.primitives.asymmetric import padding + + key_data = PRIVATE_KEY_PATH.read_bytes() + private_key = serialization.load_pem_private_key(key_data, password=None) + return private_key.sign(message, padding.PKCS1v15(), hashes.SHA256()) + + +def github_app_jwt(): + now = int(time.time()) + header = {"alg": "RS256", "typ": "JWT"} + payload = {"iat": now - 60, "exp": now + 540, "iss": APP_ID} + signing_input = ( + b64url(json.dumps(header, separators=(",", ":")).encode("utf-8")) + + "." + + b64url(json.dumps(payload, separators=(",", ":")).encode("utf-8")) + ).encode("ascii") + return signing_input.decode("ascii") + "." + b64url(sign_rs256(signing_input)) + + +def github_request(method, url, token, body=None): + headers = { + "Accept": "application/vnd.github+json", + "Authorization": "Bearer " + token, + "User-Agent": "coven-github-hosted-prototype", + "X-GitHub-Api-Version": "2022-11-28", + } + data = None + if body is not None: + data = json.dumps(body).encode("utf-8") + headers["Content-Type"] = "application/json" + request = Request(url, data=data, headers=headers, method=method) + try: + with urlopen(request, timeout=30) as response: + raw = response.read().decode("utf-8") + return json.loads(raw) if raw else {} + except HTTPError as exc: + raw = exc.read().decode("utf-8", errors="replace") + raise RuntimeError("GitHub API {} {} failed: {}".format(method, url, raw)) + + +def installation_token(installation_id): + app_token = github_app_jwt() + response = github_request( + "POST", + "https://api.github.com/app/installations/{}/access_tokens".format(installation_id), + app_token, + {}, + ) + token = response.get("token") + if not token: + raise RuntimeError("GitHub installation token response did not include token") + return token + + +def load_policy(): + if not POLICY_PATH.exists(): + write_json_atomic(POLICY_PATH, DEFAULT_POLICY) + return read_json(POLICY_PATH, DEFAULT_POLICY) + + +def repo_policy(payload): + policy = load_policy() + installation_id = str((payload.get("installation") or {}).get("id") or "") + repository = payload.get("repository") or {} + repo_id = str(repository.get("id") or "") + installation = (policy.get("installations") or {}).get(installation_id) or {} + repo = (installation.get("repositories") or {}).get(repo_id) + return installation_id, repo_id, repo + + +def delivery_path(delivery_id): + return DELIVERIES_DIR / (delivery_id + ".json") + + +def task_path(task_id): + return TASKS_DIR / (task_id + ".json") + + +def payload_hash(payload): + raw = json.dumps(payload, sort_keys=True, separators=(",", ":")).encode("utf-8") + return hashlib.sha256(raw).hexdigest() + + +def delivery_record(delivery_id, event_name, payload): + repository = payload.get("repository") or {} + installation = payload.get("installation") or {} + return { + "delivery_id": delivery_id, + "event": event_name, + "action": payload.get("action"), + "installation_id": installation.get("id"), + "repository_id": repository.get("id"), + "repository": repository.get("full_name"), + "payload_hash": payload_hash(payload), + "received_at": utc_now(), + "state": "received", + "issue_refs": ["OpenCoven/coven-github#2"], + } + + +def mentioned(text, policy): + normalized = (text or "").lower() + for username in policy.get("bot_usernames") or []: + if "@" + username.lower() in normalized: + return True + return False + + +def labels_include_trigger(labels, policy): + wanted = set((policy.get("trigger_labels") or [])) + for label in labels or []: + name = (label.get("name") if isinstance(label, dict) else str(label)).strip() + if name in wanted: + return True + return False + + +def build_task_from_event(event_name, delivery_id, payload, policy): + repository = payload.get("repository") or {} + installation = payload.get("installation") or {} + familiar = policy.get("familiar") or DEFAULT_POLICY["installations"]["144638200"]["repositories"]["1290307745"]["familiar"] + base = { + "task_id": delivery_id, + "delivery_id": delivery_id, + "created_at": utc_now(), + "updated_at": utc_now(), + "state": "queued", + "attempts": 0, + "installation_id": installation.get("id"), + "repository_id": repository.get("id"), + "repository": repository.get("full_name"), + "clone_url": repository.get("clone_url") + or "https://github.com/{}.git".format(repository.get("full_name")), + "default_branch": repository.get("default_branch") or policy.get("default_branch") or "main", + "familiar": familiar, + "publication": policy.get("publication") or {"mode": "record_only"}, + "issue_refs": ["OpenCoven/coven-github#2", "OpenCoven/coven-github#7"], + } + + if event_name == "issue_comment": + issue = payload.get("issue") or {} + comment = payload.get("comment") or {} + if not mentioned(comment.get("body"), policy): + return ignored(base, "issue_comment_without_mention") + if issue.get("pull_request"): + base.update( + { + "trigger": "pr_mention", + "target": { + "kind": "pull_request", + "pr_number": int(issue.get("number") or 0), + }, + "task": { + "kind": "respond_to_mention", + "issue_number": int(issue.get("number") or 0), + "comment_body": comment.get("body") or "", + }, + "issue_refs": base["issue_refs"] + ["OpenCoven/coven-github#4"], + } + ) + return base + base.update( + { + "trigger": "issue_mention", + "task": { + "kind": "respond_to_mention", + "issue_number": int(issue.get("number") or 0), + "comment_body": comment.get("body") or "", + }, + "issue_refs": base["issue_refs"] + ["OpenCoven/coven-github#4"], + } + ) + return base + + if event_name == "pull_request_review_comment": + comment = payload.get("comment") or {} + pull_request = payload.get("pull_request") or {} + if not mentioned(comment.get("body"), policy): + return ignored(base, "pr_review_comment_without_mention") + base.update( + { + "trigger": "pr_review_comment", + "task": { + "kind": "address_review_comment", + "pr_number": int(pull_request.get("number") or 0), + "comment_body": comment.get("body") or "", + "diff_hunk": comment.get("diff_hunk"), + "path": comment.get("path"), + "line": comment.get("line"), + "side": comment.get("side"), + "commit_id": comment.get("commit_id"), + "html_url": comment.get("html_url"), + }, + "issue_refs": base["issue_refs"] + ["OpenCoven/coven-github#4"], + } + ) + return base + + if event_name == "issues": + issue = payload.get("issue") or {} + action = payload.get("action") + if action not in ("assigned", "labeled", "opened"): + return ignored(base, "unsupported_issue_action") + if action == "labeled" and not labels_include_trigger(issue.get("labels"), policy): + return ignored(base, "issue_label_not_enabled") + base.update( + { + "trigger": "issue_assigned" if action == "assigned" else "issue_mention", + "task": { + "kind": "fix_issue", + "issue_number": int(issue.get("number") or 0), + "issue_title": issue.get("title") or "", + "issue_body": issue.get("body") or "", + }, + "issue_refs": base["issue_refs"] + ["OpenCoven/coven-github#4"], + } + ) + return base + + if event_name == "pull_request": + pull_request = payload.get("pull_request") or {} + base.update( + { + "state": "ignored", + "ignored_reason": "pull_request_review_task_not_in_headless_contract_v1", + "trigger": "pull_request", + "target": { + "action": payload.get("action"), + "number": pull_request.get("number"), + "head_sha": (pull_request.get("head") or {}).get("sha"), + "head_ref": (pull_request.get("head") or {}).get("ref"), + "base_ref": (pull_request.get("base") or {}).get("ref"), + }, + "issue_refs": base["issue_refs"] + ["OpenCoven/coven-github#10"], + } + ) + return base + + if event_name == "push": + base.update( + { + "state": "ignored", + "ignored_reason": "push_review_task_not_in_headless_contract_v1", + "trigger": "push", + "target": { + "ref": payload.get("ref"), + "before": payload.get("before"), + "after": payload.get("after"), + "commit_count": len(payload.get("commits") or []), + }, + "issue_refs": base["issue_refs"] + ["OpenCoven/coven-github#10"], + } + ) + return base + + return ignored(base, "unsupported_event") + + +def ignored(base, reason): + base["state"] = "ignored" + base["ignored_reason"] = reason + return base + + +def route_delivery(event_name, delivery_id, payload, debug): + delivery_file = delivery_path(delivery_id) + if delivery_file.exists(): + existing = read_json(delivery_file, {}) + return { + "ok": True, + "action": "duplicate_ignored", + "delivery_id": delivery_id, + "task_id": existing.get("task_id"), + "state": existing.get("state"), + } + + delivery = delivery_record(delivery_id, event_name, payload) + installation_id, repo_id, policy = repo_policy(payload) + if not policy: + delivery["state"] = "ignored" + delivery["routing_result"] = "no_policy_for_installation_repo" + delivery["installation_id"] = installation_id or delivery.get("installation_id") + delivery["repository_id"] = repo_id or delivery.get("repository_id") + write_json_atomic(delivery_file, delivery) + return { + "ok": True, + "action": "ignored", + "delivery_id": delivery_id, + "reason": "no_policy_for_installation_repo", + } + + task = build_task_from_event(event_name, delivery_id, payload, policy) + task["policy_snapshot"] = { + "enabled_triggers": policy.get("enabled_triggers") or [], + "publication": policy.get("publication") or {"mode": "record_only"}, + } + write_json_atomic(task_path(task["task_id"]), task) + + delivery["task_id"] = task["task_id"] + delivery["state"] = task["state"] + delivery["routing_result"] = task.get("ignored_reason") or "queued" + write_json_atomic(delivery_file, delivery) + + if task["state"] == "queued": + try: + run_task(task["task_id"], debug) + except Exception: + debug("COVEN GITHUB TASK RUN FAIL task_id={} {}".format(task["task_id"], traceback.format_exc())) + + return { + "ok": True, + "action": "accepted" if task["state"] != "ignored" else "ignored", + "delivery_id": delivery_id, + "task_id": task["task_id"], + "state": read_json(task_path(task["task_id"]), task).get("state"), + "reason": task.get("ignored_reason"), + "queued": task["state"] == "queued", + } + + +def run_command(args, cwd=None, env=None, timeout=300): + proc = subprocess.run( + args, + cwd=cwd, + env=env, + stdout=subprocess.PIPE, + stderr=subprocess.PIPE, + timeout=timeout, + text=True, + ) + return { + "args": args, + "returncode": proc.returncode, + "stdout": proc.stdout[-8000:], + "stderr": proc.stderr[-8000:], + } + + +def write_askpass(work_dir): + script = work_dir / "git-askpass.sh" + script.write_text("#!/bin/sh\nprintf '%s\\n' \"$COVEN_GIT_TOKEN\"\n", encoding="utf-8") + script.chmod(0o700) + return script + + +def session_brief(task, workspace, review_context=None): + owner, name = (task["repository"] or "/").split("/", 1) + brief = { + "contract_version": "2", + "trigger": task["trigger"], + "repo": { + "owner": owner, + "name": name, + "clone_url": task["clone_url"], + "default_branch": task["default_branch"], + }, + "task": task["task"], + "familiar": task["familiar"], + "workspace": {"root": str(workspace)}, + } + if review_context: + brief["review_context"] = review_context + brief["audit_instruction"] = ( + "This run is evidence-backed. Review the supplied PR metadata and " + "changed-file patches in review_context before responding. Cite the " + "specific changed files you inspected in the result summary." + ) + return brief + + +def run_task(task_id, debug): + path = task_path(task_id) + task = read_json(path, {}) + if task.get("state") != "queued": + return task + + task["state"] = "running" + task["attempts"] = int(task.get("attempts") or 0) + 1 + task["updated_at"] = utc_now() + write_json_atomic(path, task) + + attempt_dir = ATTEMPTS_DIR / task_id / str(task["attempts"]) + attempt_dir.mkdir(parents=True, exist_ok=True) + workspace = WORKSPACES_DIR / task_id / "repo" + + try: + token = installation_token(task["installation_id"]) + askpass = write_askpass(attempt_dir) + env = os.environ.copy() + env["GIT_ASKPASS"] = str(askpass) + env["GIT_TERMINAL_PROMPT"] = "0" + env["COVEN_GIT_TOKEN"] = token + env["COVEN_CODE_PROVIDER"] = "codex" + env["COVEN_CODE_HOSTED_REVIEW"] = "1" + env["HOME"] = str(CODEX_TOKENS_PATH.parent.parent) + codex_access_token = load_codex_access_token() + if not codex_access_token: + return fail_task( + path, + task, + "codex_auth_missing", + "Missing Codex access token at {}".format(CODEX_TOKENS_PATH), + ) + env["OPENAI_API_KEY"] = codex_access_token + + if not workspace.exists(): + clone = run_command( + [ + "git", + "clone", + "--depth", + "1", + "--branch", + task["default_branch"], + task["clone_url"], + str(workspace), + ], + env=env, + timeout=180, + ) + write_json_atomic(attempt_dir / "clone.json", redacted_command_result(clone)) + if clone["returncode"] != 0: + return fail_task(path, task, "clone_failed", clone["stderr"]) + + review_context = prepare_review_context(task, workspace, token, env, attempt_dir) + if review_context: + review_context_path = attempt_dir / "review-context.json" + write_json_atomic(review_context_path, review_context) + task["review_context_path"] = str(review_context_path) + task["review_context_sha256"] = file_sha256(review_context_path) + task["review_evidence"] = review_evidence(review_context, review_context_path, task) + write_json_atomic(path, task) + + brief_path = attempt_dir / "session-brief.json" + result_path = attempt_dir / "result.json" + write_json_atomic(brief_path, session_brief(task, workspace, review_context)) + task["session_brief_path"] = str(brief_path) + task["session_brief_sha256"] = file_sha256(brief_path) + write_json_atomic(path, task) + + if not command_exists(COVEN_CODE_BIN): + return fail_task( + path, + task, + "runtime_missing", + "COVEN_CODE_BIN is not available on the host: {}".format(COVEN_CODE_BIN), + ) + + run = run_command( + [ + COVEN_CODE_BIN, + "--headless", + "--hosted-review", + "--provider", + "codex", + "--model", + COVEN_CODE_MODEL, + "--context", + str(brief_path), + "--output", + str(result_path), + ], + cwd=str(workspace), + env=env, + timeout=1800, + ) + write_json_atomic(attempt_dir / "run.json", redacted_command_result(run)) + task["runtime_exit_code"] = run["returncode"] + task["result_path"] = str(result_path) + if not result_path.exists(): + return fail_task( + path, + task, + "result_missing", + "coven-code exited {} without writing result.json: {}".format( + run["returncode"], run["stderr"] + ), + ) + task["state"] = "completed" if run["returncode"] in (0, 1, 3) else "failed" + task["updated_at"] = utc_now() + publish_result_if_configured(task, result_path, token) + write_json_atomic(path, task) + return task + except Exception as exc: + return fail_task(path, task, "infra_error", repr(exc)) + + +def command_exists(command): + probe = run_command(["/bin/sh", "-lc", "command -v {}".format(shell_quote(command))], timeout=10) + return probe["returncode"] == 0 + + +def shell_quote(value): + return "'" + str(value).replace("'", "'\"'\"'") + "'" + + +def pr_number_for_task(task): + task_data = task.get("task") or {} + target = task.get("target") or {} + if target.get("kind") == "pull_request" and target.get("pr_number"): + return int(target.get("pr_number")) + value = task_data.get("pr_number") + if value: + return int(value) + return None + + +def prepare_review_context(task, workspace, token, env, attempt_dir): + pr_number = pr_number_for_task(task) + if not pr_number: + return None + + repo = task.get("repository") + pr = github_request( + "GET", + "https://api.github.com/repos/{}/pulls/{}".format(repo, pr_number), + token, + ) + files = github_request( + "GET", + "https://api.github.com/repos/{}/pulls/{}/files?per_page=100".format(repo, pr_number), + token, + ) + + fetch = run_command( + ["git", "fetch", "--depth", "1", "origin", "pull/{}/head".format(pr_number)], + cwd=str(workspace), + env=env, + timeout=180, + ) + write_json_atomic(attempt_dir / "fetch-pr.json", redacted_command_result(fetch)) + if fetch["returncode"] != 0: + return { + "kind": "pull_request", + "pr_number": pr_number, + "fetch_error": fetch["stderr"], + "metadata": summarize_pr(pr), + "files": summarize_pr_files(files), + } + + checkout = run_command(["git", "checkout", "--detach", "FETCH_HEAD"], cwd=str(workspace), env=env) + write_json_atomic(attempt_dir / "checkout-pr.json", redacted_command_result(checkout)) + + head = run_command(["git", "rev-parse", "HEAD"], cwd=str(workspace), env=env) + status = run_command(["git", "status", "--short", "--branch"], cwd=str(workspace), env=env) + write_json_atomic(attempt_dir / "workspace-git.json", redacted_command_result({ + "args": ["git evidence"], + "returncode": 0 if head["returncode"] == 0 and status["returncode"] == 0 else 1, + "stdout": "HEAD={}\n{}".format(head["stdout"].strip(), status["stdout"].strip()), + "stderr": head["stderr"] + status["stderr"], + })) + + return { + "kind": "pull_request", + "pr_number": pr_number, + "metadata": summarize_pr(pr), + "files": summarize_pr_files(files), + "checkout": { + "fetch_returncode": fetch["returncode"], + "checkout_returncode": checkout["returncode"], + "workspace_head_sha": head["stdout"].strip(), + "workspace_status": status["stdout"].strip(), + }, + } + + +def summarize_pr(pr): + return { + "number": pr.get("number"), + "title": pr.get("title"), + "state": pr.get("state"), + "html_url": pr.get("html_url"), + "base_ref": (pr.get("base") or {}).get("ref"), + "base_sha": (pr.get("base") or {}).get("sha"), + "head_ref": (pr.get("head") or {}).get("ref"), + "head_sha": (pr.get("head") or {}).get("sha"), + "merge_commit_sha": pr.get("merge_commit_sha"), + } + + +def summarize_pr_files(files): + summarized = [] + for item in files or []: + patch = item.get("patch") or "" + summarized.append( + { + "filename": item.get("filename"), + "status": item.get("status"), + "additions": item.get("additions"), + "deletions": item.get("deletions"), + "changes": item.get("changes"), + "sha": item.get("sha"), + "patch": patch[:12000], + "patch_truncated": len(patch) > 12000, + } + ) + return summarized + + +def review_evidence(review_context, review_context_path, task): + metadata = review_context.get("metadata") or {} + files = review_context.get("files") or [] + checkout = review_context.get("checkout") or {} + return { + "pr_number": review_context.get("pr_number"), + "base_ref": metadata.get("base_ref"), + "base_sha": metadata.get("base_sha"), + "head_ref": metadata.get("head_ref"), + "head_sha": metadata.get("head_sha"), + "workspace_head_sha": checkout.get("workspace_head_sha"), + "changed_file_count": len(files), + "changed_files": [f.get("filename") for f in files], + "review_context_path": str(review_context_path), + "review_context_sha256": file_sha256(review_context_path), + } + + +def file_sha256(path): + digest = hashlib.sha256() + with open(path, "rb") as handle: + for chunk in iter(lambda: handle.read(65536), b""): + digest.update(chunk) + return digest.hexdigest() + + +def publish_result_if_configured(task, result_path, token): + publication = task.get("publication") or {} + mode = publication.get("mode") or "record_only" + if mode != "comment": + task["publication_state"] = "held_for_issue_11_publication_gates" + return + + result = read_json(result_path, {}) + number = ( + (task.get("task") or {}).get("issue_number") + or (task.get("task") or {}).get("pr_number") + ) + if not number: + task["publication_state"] = "publication_skipped_no_issue_or_pr_number" + return + + body = publication_comment_body(task, result) + repo = task.get("repository") + url = "https://api.github.com/repos/{}/issues/{}/comments".format(repo, int(number)) + try: + response = github_request("POST", url, token, {"body": body}) + task["publication_state"] = "published_comment" + task["publication_url"] = response.get("html_url") + task["publication_comment_id"] = response.get("id") + except Exception as exc: + task["publication_state"] = "publication_failed" + task["publication_error"] = redact_tokenish(repr(exc)) + + +def publication_comment_body(task, result): + status = result.get("status") or "unknown" + summary = result.get("summary") or "No summary returned." + pr_body = result.get("pr_body") or "" + files_changed = result.get("files_changed") or [] + commits = result.get("commits") or [] + task_id = task.get("task_id") or "" + evidence = task.get("review_evidence") or {} + review = result.get("review") or {} + + parts = [ + "## Cody dogfood result", + "", + "**Status:** {}".format(status), + "", + summary.strip(), + ] + if pr_body.strip() and pr_body.strip() != summary.strip(): + parts.extend(["", pr_body.strip()]) + parts.extend(["", "### Evidence"]) + if evidence: + changed_files = evidence.get("changed_files") or [] + parts.extend( + [ + "- PR: #{}".format(evidence.get("pr_number")), + "- Base: `{}` @ `{}`".format(evidence.get("base_ref"), evidence.get("base_sha")), + "- Head: `{}` @ `{}`".format(evidence.get("head_ref"), evidence.get("head_sha")), + "- Checked-out workspace HEAD: `{}`".format(evidence.get("workspace_head_sha")), + "- Changed files supplied to agent: {}".format(evidence.get("changed_file_count")), + "- Review context SHA-256: `{}`".format(evidence.get("review_context_sha256")), + ] + ) + if changed_files: + parts.append("- Files: {}".format(", ".join("`{}`".format(f) for f in changed_files[:20]))) + else: + parts.append("- No PR review evidence was captured for this run.") + parts.extend(structured_review_lines(review)) + parts.extend( + [ + "", + "**Files changed:** {}".format(len(files_changed)), + "**Commits:** {}".format(len(commits)), + "", + "_Task `{}`. Publication is enabled on the hosted test adapter only._".format(task_id), + ] + ) + return "\n".join(parts) + + +def structured_review_lines(review): + if not review: + return ["", "### Structured review", "- No structured review result was emitted."] + + lines = [ + "", + "### Structured review", + "- Mode: `{}`".format(review.get("mode") or "unknown"), + "- Evidence status: `{}`".format(review.get("evidence_status") or "unknown"), + ] + + reviewed_files = review.get("reviewed_files") or [] + lines.append("- Reviewed files: {}".format(len(reviewed_files))) + if reviewed_files: + lines.append( + "- Reviewed file list: {}".format( + ", ".join("`{}`".format(path) for path in reviewed_files[:20]) + ) + ) + if len(reviewed_files) > 20: + lines.append("- Reviewed file list truncated after 20 entries.") + + supporting_files = review.get("supporting_files") or [] + lines.append("- Supporting files inspected: {}".format(len(supporting_files))) + if supporting_files: + lines.append( + "- Supporting file list: {}".format( + ", ".join("`{}`".format(path) for path in supporting_files[:20]) + ) + ) + if len(supporting_files) > 20: + lines.append("- Supporting file list truncated after 20 entries.") + + findings = review.get("findings") or [] + lines.append("- Findings: {}".format(len(findings))) + for index, finding in enumerate(findings[:10], start=1): + location = finding.get("file") or "unknown file" + if finding.get("line") is not None: + location = "{}:{}".format(location, finding.get("line")) + lines.append( + " {}. `{}` {} - {}".format( + index, + finding.get("severity") or "unknown", + location, + finding.get("title") or "Untitled finding", + ) + ) + if len(findings) > 10: + lines.append("- Findings truncated after 10 entries.") + + no_findings_reason = review.get("no_findings_reason") + if no_findings_reason: + lines.append("- No-findings reason: {}".format(no_findings_reason)) + + tests_run = review.get("tests_run") or [] + lines.append("- Tests reported by runtime: {}".format(len(tests_run))) + for test in tests_run[:10]: + summary = test.get("output_summary") + suffix = " - {}".format(summary) if summary else "" + lines.append( + " - `{}`: `{}`{}".format( + test.get("command") or "unknown command", + test.get("status") or "unknown", + suffix, + ) + ) + if len(tests_run) > 10: + lines.append("- Test list truncated after 10 entries.") + + limitations = review.get("limitations") or [] + lines.append("- Limitations: {}".format(len(limitations))) + for limitation in limitations[:10]: + lines.append(" - {}".format(limitation)) + if len(limitations) > 10: + lines.append("- Limitation list truncated after 10 entries.") + + return lines + + +def load_codex_access_token(): + for path in codex_token_candidates(): + try: + data = read_json(path, {}) + token = str(data.get("access_token") or "").strip() + if token: + return token + except Exception: + continue + return None + + +def codex_token_candidates(): + yield CODEX_TOKENS_PATH + + coven_home = CODEX_TOKENS_PATH.parent + registry = read_json(coven_home / "accounts.json", {}) + active = ( + registry.get("providers", {}) + .get("codex", {}) + .get("active") + ) + if active: + yield coven_home / "accounts" / "codex" / str(active) / "codex_tokens.json" + + accounts_root = coven_home / "accounts" / "codex" + if accounts_root.exists(): + for path in accounts_root.glob("*/codex_tokens.json"): + yield path + + +def redacted_command_result(result): + redacted = dict(result) + for key in ("stdout", "stderr"): + redacted[key] = redact_tokenish(redacted.get(key) or "") + return redacted + + +def redact_tokenish(text): + if not text: + return text + markers = ["ghs_", "ghu_", "github_pat_", "x-access-token:"] + redacted = text + for marker in markers: + while marker in redacted: + index = redacted.find(marker) + end = index + len(marker) + while end < len(redacted) and redacted[end] not in " \n\r\t'\"": + end += 1 + redacted = redacted[:index] + marker + "[redacted]" + redacted[end:] + return redacted + + +def fail_task(path, task, reason, detail): + task["state"] = "failed" + task["failure_category"] = reason + task["failure_detail"] = redact_tokenish(str(detail))[-4000:] + task["updated_at"] = utc_now() + write_json_atomic(path, task) + return task From 3cd400a27053f519411965f7f821dc31bfa910b1 Mon Sep 17 00:00:00 2001 From: Timothy Wayne Gregg Date: Mon, 6 Jul 2026 01:16:02 -0400 Subject: [PATCH 04/19] feat(hosted): add bounded review fix loop (#119) Signed-off-by: Timothy Wayne Gregg --- crates/worker/src/lib.rs | 6 +- deploy/complete-tech/README.md | 5 + deploy/complete-tech/coven_github_adapter.py | 239 ++++++++++++++++--- 3 files changed, 218 insertions(+), 32 deletions(-) diff --git a/crates/worker/src/lib.rs b/crates/worker/src/lib.rs index bc8372c..35bcee1 100644 --- a/crates/worker/src/lib.rs +++ b/crates/worker/src/lib.rs @@ -391,7 +391,7 @@ fn disposition(result: &SessionResult) -> Disposition { enum Attempt { /// The runtime exited 0/1/3 and wrote a parseable `result.json`. Terminal: /// the adapter acts on `status`/`exit_reason` and MUST NOT retry. - Completed(SessionResult), + Completed(Box), /// Exit 2, timeout, kill-by-signal, an unexpected exit code, or a spawn/read /// failure. Retry-safe per the contract. RetrySafe(anyhow::Error), @@ -430,7 +430,7 @@ async fn run_session_with_backoff( let mut attempts = 0u32; loop { match run_coven_code(config, brief_path, result_path, git_token).await { - Attempt::Completed(result) => return Ok(result), + Attempt::Completed(result) => return Ok(*result), Attempt::RetrySafe(e) if attempts < max_retries => { attempts += 1; warn!("coven-code attempt {attempts} hit a retry-safe failure ({e:#}), retryingโ€ฆ"); @@ -488,7 +488,7 @@ async fn run_coven_code( // exit codes; if it isn't, the runtime misbehaved โ€” fall back to a // retry-safe failure rather than silently losing the task. Some(code @ (0 | 1 | 3)) => match read_result(result_path).await { - Ok(result) => Attempt::Completed(result), + Ok(result) => Attempt::Completed(Box::new(result)), Err(e) => Attempt::RetrySafe(anyhow::anyhow!( "coven-code exited {code} but result.json was unusable: {e}" )), diff --git a/deploy/complete-tech/README.md b/deploy/complete-tech/README.md index 5c695ae..64484b2 100644 --- a/deploy/complete-tech/README.md +++ b/deploy/complete-tech/README.md @@ -21,6 +21,8 @@ The deployment expects secrets and mutable state to be supplied outside git: - `COVEN_GITHUB_STATE_DIR` - `COVEN_GITHUB_POLICY_PATH` - `COVEN_CODE_BIN` +- `COVEN_REVIEW_FIX_LOOPS` - optional bounded review-fix loop count, clamped + between `0` and `5`; defaults to `0` so hosted repair loops are opt-in - Codex OAuth tokens under the deployed account's `.coven-code` directory Do not commit private keys, webhook secrets, OAuth tokens, generated task state, @@ -34,3 +36,6 @@ workspaces, or attempt artifacts. - Publishes visible structured review evidence, including `reviewed_files`, `supporting_files`, findings, test evidence, no-findings rationale, and limitations. +- When `COVEN_REVIEW_FIX_LOOPS` is greater than `0`, reruns `coven-code` with + prior structured review findings as explicit repair instructions until no + findings remain or the configured loop count is exhausted. diff --git a/deploy/complete-tech/coven_github_adapter.py b/deploy/complete-tech/coven_github_adapter.py index 2d1b6ef..0277d7b 100644 --- a/deploy/complete-tech/coven_github_adapter.py +++ b/deploy/complete-tech/coven_github_adapter.py @@ -27,6 +27,20 @@ COVEN_CODE_MODEL = os.environ.get("COVEN_CODE_MODEL", "gpt-5.5").strip() +def env_int(name, default, minimum=0, maximum=10): + raw = os.environ.get(name, "").strip() + if not raw: + return default + try: + value = int(raw) + except ValueError: + return default + return max(minimum, min(maximum, value)) + + +MAX_REVIEW_FIX_LOOPS = env_int("COVEN_REVIEW_FIX_LOOPS", 0, minimum=0, maximum=5) + + def account_home(): try: import pwd @@ -456,7 +470,7 @@ def write_askpass(work_dir): return script -def session_brief(task, workspace, review_context=None): +def session_brief(task, workspace, review_context=None, extra_audit_instruction=None): owner, name = (task["repository"] or "/").split("/", 1) brief = { "contract_version": "2", @@ -473,14 +487,122 @@ def session_brief(task, workspace, review_context=None): } if review_context: brief["review_context"] = review_context - brief["audit_instruction"] = ( + instruction = ( "This run is evidence-backed. Review the supplied PR metadata and " "changed-file patches in review_context before responding. Cite the " "specific changed files you inspected in the result summary." ) + if extra_audit_instruction: + instruction = instruction + "\n\n" + extra_audit_instruction + brief["audit_instruction"] = instruction return brief +def run_coven_code_cycle( + task, + workspace, + review_context, + attempt_dir, + env, + cycle, + extra_audit_instruction=None, +): + suffix = "" if cycle == 0 else "-repair-{}".format(cycle) + brief_path = attempt_dir / "session-brief{}.json".format(suffix) + result_path = attempt_dir / "result{}.json".format(suffix) + run_path = attempt_dir / "run{}.json".format(suffix) + + write_json_atomic( + brief_path, + session_brief(task, workspace, review_context, extra_audit_instruction), + ) + run = run_command( + [ + COVEN_CODE_BIN, + "--headless", + "--hosted-review", + "--provider", + "codex", + "--model", + COVEN_CODE_MODEL, + "--context", + str(brief_path), + "--output", + str(result_path), + ], + cwd=str(workspace), + env=env, + timeout=1800, + ) + write_json_atomic(run_path, redacted_command_result(run)) + result = read_json(result_path, None) if result_path.exists() else None + return { + "cycle": cycle, + "brief_path": brief_path, + "result_path": result_path, + "run_path": run_path, + "run": run, + "result": result, + } + + +def review_findings(result): + if not result: + return [] + review = result.get("review") or {} + mode = review.get("mode") + if mode not in ("pull_request", "review_comment"): + return [] + return review.get("findings") or [] + + +def review_fix_instruction(findings, iteration, max_iterations): + lines = [ + "Autofix review loop iteration {}/{}.".format(iteration, max_iterations), + "The previous hosted review returned structured findings. Fix the findings below, run the relevant checks you can run safely, then perform another bounded review of the updated code using the required review sections.", + "If a finding cannot be fixed safely, leave a clear limitation and explain the remaining blocker. Do not merely restate the findings.", + "", + "Findings to fix:", + ] + for index, finding in enumerate(findings[:10], start=1): + location = finding.get("file") or "unknown file" + if finding.get("line") is not None: + location = "{}:{}".format(location, finding.get("line")) + lines.append( + "{}. [{}] `{}` {}".format( + index, + finding.get("severity") or "unknown", + location, + finding.get("title") or "Untitled finding", + ) + ) + body = (finding.get("body") or "").strip() + if body: + lines.append(" Body: {}".format(body[:1200])) + recommendation = (finding.get("recommendation") or "").strip() + if recommendation: + lines.append(" Recommendation: {}".format(recommendation[:1200])) + if len(findings) > 10: + lines.append("Only the first 10 findings are listed; inspect the prior result for the full set.") + return "\n".join(lines) + + +def task_with_repair_request(task, instruction): + copy = json.loads(json.dumps(task)) + task_data = copy.get("task") or {} + explicit_request = ( + "\n\nPlease fix the review findings from the previous hosted review cycle. " + "After fixing them, rerun relevant checks and produce another structured review.\n\n" + + instruction + ) + if "comment_body" in task_data: + task_data["comment_body"] = (task_data.get("comment_body") or "") + explicit_request + elif "issue_body" in task_data: + task_data["issue_body"] = (task_data.get("issue_body") or "") + explicit_request + copy["task"] = task_data + return copy + + def run_task(task_id, debug): path = task_path(task_id) task = read_json(path, {}) @@ -544,13 +666,6 @@ def run_task(task_id, debug): task["review_evidence"] = review_evidence(review_context, review_context_path, task) write_json_atomic(path, task) - brief_path = attempt_dir / "session-brief.json" - result_path = attempt_dir / "result.json" - write_json_atomic(brief_path, session_brief(task, workspace, review_context)) - task["session_brief_path"] = str(brief_path) - task["session_brief_sha256"] = file_sha256(brief_path) - write_json_atomic(path, task) - if not command_exists(COVEN_CODE_BIN): return fail_task( path, @@ -559,28 +674,17 @@ def run_task(task_id, debug): "COVEN_CODE_BIN is not available on the host: {}".format(COVEN_CODE_BIN), ) - run = run_command( - [ - COVEN_CODE_BIN, - "--headless", - "--hosted-review", - "--provider", - "codex", - "--model", - COVEN_CODE_MODEL, - "--context", - str(brief_path), - "--output", - str(result_path), - ], - cwd=str(workspace), - env=env, - timeout=1800, - ) - write_json_atomic(attempt_dir / "run.json", redacted_command_result(run)) + cycle_result = run_coven_code_cycle(task, workspace, review_context, attempt_dir, env, 0) + brief_path = cycle_result["brief_path"] + result_path = cycle_result["result_path"] + run = cycle_result["run"] + task["session_brief_path"] = str(brief_path) + task["session_brief_sha256"] = file_sha256(brief_path) task["runtime_exit_code"] = run["returncode"] task["result_path"] = str(result_path) - if not result_path.exists(): + write_json_atomic(path, task) + + if cycle_result["result"] is None: return fail_task( path, task, @@ -589,6 +693,58 @@ def run_task(task_id, debug): run["returncode"], run["stderr"] ), ) + + final_cycle = cycle_result + loop_records = [] + for iteration in range(1, MAX_REVIEW_FIX_LOOPS + 1): + findings = review_findings(final_cycle["result"]) + if not findings: + break + instruction = review_fix_instruction(findings, iteration, MAX_REVIEW_FIX_LOOPS) + repair_task = task_with_repair_request(task, instruction) + repair_cycle = run_coven_code_cycle( + repair_task, + workspace, + review_context, + attempt_dir, + env, + iteration, + instruction, + ) + remaining = review_findings(repair_cycle["result"]) + loop_records.append( + { + "iteration": iteration, + "input_findings": len(findings), + "runtime_exit_code": repair_cycle["run"]["returncode"], + "result_path": str(repair_cycle["result_path"]), + "result_status": (repair_cycle["result"] or {}).get("status"), + "remaining_findings": len(remaining), + } + ) + task["review_fix_loops"] = loop_records + task["runtime_exit_code"] = repair_cycle["run"]["returncode"] + task["result_path"] = str(repair_cycle["result_path"]) + task["updated_at"] = utc_now() + write_json_atomic(path, task) + + if repair_cycle["result"] is None: + return fail_task( + path, + task, + "result_missing", + "review repair loop {} exited {} without writing result.json: {}".format( + iteration, + repair_cycle["run"]["returncode"], + repair_cycle["run"]["stderr"], + ), + ) + final_cycle = repair_cycle + + result_path = final_cycle["result_path"] + run = final_cycle["run"] + task["runtime_exit_code"] = run["returncode"] + task["result_path"] = str(result_path) task["state"] = "completed" if run["returncode"] in (0, 1, 3) else "failed" task["updated_at"] = utc_now() publish_result_if_configured(task, result_path, token) @@ -784,6 +940,7 @@ def publication_comment_body(task, result): ] if pr_body.strip() and pr_body.strip() != summary.strip(): parts.extend(["", pr_body.strip()]) + parts.extend(review_fix_loop_lines(task)) parts.extend(["", "### Evidence"]) if evidence: changed_files = evidence.get("changed_files") or [] @@ -814,6 +971,30 @@ def publication_comment_body(task, result): return "\n".join(parts) +def review_fix_loop_lines(task): + loops = task.get("review_fix_loops") or [] + if not loops: + return [] + + lines = ["", "### Review fix loop"] + for loop in loops: + lines.append( + "- Iteration {iteration}: input findings {input_findings}, result `{result_status}`, remaining findings {remaining_findings}.".format( + iteration=loop.get("iteration"), + input_findings=loop.get("input_findings"), + result_status=loop.get("result_status") or "unknown", + remaining_findings=loop.get("remaining_findings"), + ) + ) + if loops and loops[-1].get("remaining_findings", 0): + lines.append( + "- Loop stopped after {} configured iteration(s); unresolved findings remain.".format( + len(loops) + ) + ) + return lines + + def structured_review_lines(review): if not review: return ["", "### Structured review", "- No structured review result was emitted."] From 669fca69b993acfcce90e5170c8456163aa04d40 Mon Sep 17 00:00:00 2001 From: Timothy Wayne Gregg Date: Mon, 6 Jul 2026 03:09:20 -0400 Subject: [PATCH 05/19] fix(hosted): address review gating and evidence gaps (#119) Signed-off-by: Timothy Wayne Gregg --- deploy/{complete-tech => coven-github}/README.md | 0 deploy/{complete-tech => coven-github}/coven_github_adapter.py | 0 2 files changed, 0 insertions(+), 0 deletions(-) rename deploy/{complete-tech => coven-github}/README.md (100%) rename deploy/{complete-tech => coven-github}/coven_github_adapter.py (100%) diff --git a/deploy/complete-tech/README.md b/deploy/coven-github/README.md similarity index 100% rename from deploy/complete-tech/README.md rename to deploy/coven-github/README.md diff --git a/deploy/complete-tech/coven_github_adapter.py b/deploy/coven-github/coven_github_adapter.py similarity index 100% rename from deploy/complete-tech/coven_github_adapter.py rename to deploy/coven-github/coven_github_adapter.py From a6a794d643c342162072ec7b72377f1f6ae75d9c Mon Sep 17 00:00:00 2001 From: Timothy Wayne Gregg Date: Mon, 6 Jul 2026 03:09:32 -0400 Subject: [PATCH 06/19] fix(hosted): apply review hardening (#119) Signed-off-by: Timothy Wayne Gregg --- .gitignore | 6 +- crates/worker/src/lib.rs | 36 +++++ deploy/coven-github/README.md | 12 +- deploy/coven-github/coven_github_adapter.py | 151 ++++++++++++++------ docs/contracts/session-brief.schema.json | 2 +- 5 files changed, 155 insertions(+), 52 deletions(-) diff --git a/.gitignore b/.gitignore index a8407f6..27e947b 100644 --- a/.gitignore +++ b/.gitignore @@ -4,6 +4,6 @@ keys/ *.pem .env .DS_Store -deploy/complete-tech/.coven-github-private-key.pem -deploy/complete-tech/coven-github-policy.json -deploy/complete-tech/coven-github-state/ +deploy/coven-github/.coven-github-private-key.pem +deploy/coven-github/coven-github-policy.json +deploy/coven-github/coven-github-state/ diff --git a/crates/worker/src/lib.rs b/crates/worker/src/lib.rs index 35bcee1..08412f3 100644 --- a/crates/worker/src/lib.rs +++ b/crates/worker/src/lib.rs @@ -506,6 +506,13 @@ async fn read_result(result_path: &Path) -> Result { .await .map_err(|_| anyhow::anyhow!("result.json not written by coven-code"))?; let result: SessionResult = serde_json::from_slice(&bytes)?; + if result.contract_version != coven_github_api::HEADLESS_CONTRACT_VERSION { + anyhow::bail!( + "unsupported result contract_version {}; expected {}", + result.contract_version, + coven_github_api::HEADLESS_CONTRACT_VERSION + ); + } Ok(result) } @@ -622,6 +629,35 @@ fn task_issue_number(kind: &TaskKind) -> Option { } } +#[cfg(test)] +mod result_tests { + use super::*; + use std::fs; + + #[tokio::test] + async fn read_result_rejects_unsupported_contract_version() { + let path = std::env::temp_dir().join(format!( + "coven-github-result-version-{}.json", + uuid::Uuid::new_v4() + )); + fs::write( + &path, + r#"{"contract_version":"1","status":"success","branch":null,"commits":[],"files_changed":[],"summary":"s","pr_body":"","review":{"mode":"none","evidence_status":"not_applicable","reviewed_files":[],"supporting_files":[],"findings":[],"tests_run":[],"no_findings_reason":null,"limitations":[]},"exit_reason":null}"#, + ) + .expect("result fixture should be written"); + + let error = read_result(&path) + .await + .expect_err("v1 result must be rejected"); + assert!( + format!("{error:#}").contains("unsupported result contract_version 1"), + "unexpected error: {error:#}" + ); + + let _ = fs::remove_file(path); + } +} + #[cfg(test)] mod disposition_tests { use super::*; diff --git a/deploy/coven-github/README.md b/deploy/coven-github/README.md index 64484b2..ece56de 100644 --- a/deploy/coven-github/README.md +++ b/deploy/coven-github/README.md @@ -1,7 +1,7 @@ -# CompleteTech hosted dogfood adapter +# coven-github hosted dogfood adapter -This directory tracks the Python adapter currently deployed for the -CompleteTech-hosted dogfood GitHub App at `webhook.complete.tech`. +This directory tracks the Python adapter used by the hosted coven-github +dogfood GitHub App. The adapter is intentionally deployment-specific. It is not the canonical Rust worker implementation; it exists so hosted dogfood behavior can be reviewed, @@ -9,8 +9,9 @@ reproduced, and changed through PRs instead of invisible server edits. ## Files -- `coven_github_adapter.py` - webhook handler, task runner, PR evidence capture, - Codex-backed headless runtime invocation, and dogfood comment publisher. +- `coven_github_adapter.py` - webhook routing helpers, task runner, PR evidence + capture, Codex-backed headless runtime invocation, and dogfood comment + publisher. ## Runtime inputs @@ -18,6 +19,7 @@ The deployment expects secrets and mutable state to be supplied outside git: - `GITHUB_APP_PRIVATE_KEY_PATH` or `.coven-github-private-key.pem` - `GITHUB_APP_ID` +- `GITHUB_WEBHOOK_SECRET` - `COVEN_GITHUB_STATE_DIR` - `COVEN_GITHUB_POLICY_PATH` - `COVEN_CODE_BIN` diff --git a/deploy/coven-github/coven_github_adapter.py b/deploy/coven-github/coven_github_adapter.py index 0277d7b..4195086 100644 --- a/deploy/coven-github/coven_github_adapter.py +++ b/deploy/coven-github/coven_github_adapter.py @@ -1,5 +1,6 @@ import base64 import hashlib +import hmac import json import os import subprocess @@ -22,9 +23,10 @@ PRIVATE_KEY_PATH = Path( os.environ.get("GITHUB_APP_PRIVATE_KEY_PATH", ROOT_DIR / ".coven-github-private-key.pem") ) -APP_ID = os.environ.get("GITHUB_APP_ID", "4224630").strip() +APP_ID = os.environ.get("GITHUB_APP_ID", "").strip() COVEN_CODE_BIN = os.environ.get("COVEN_CODE_BIN", "coven-code").strip() or "coven-code" COVEN_CODE_MODEL = os.environ.get("COVEN_CODE_MODEL", "gpt-5.5").strip() +WEBHOOK_SECRET = os.environ.get("GITHUB_WEBHOOK_SECRET", "").strip() def env_int(name, default, minimum=0, maximum=10): @@ -63,34 +65,17 @@ def configured_codex_tokens_path(): directory.mkdir(parents=True, exist_ok=True) +DEFAULT_FAMILIAR = { + "id": "cody", + "display_name": "Cody", + "model": None, + "skills": ["code-review"], +} + + DEFAULT_POLICY = { "version": 1, - "installations": { - "144638200": { - "repositories": { - "1290307745": { - "full_name": "CompleteTech-LLC-AI-Research/coven-code", - "default_branch": "main", - "enabled_triggers": [ - "issue_mention", - "issue_label", - "pr_review_comment", - "pull_request", - "push", - ], - "trigger_labels": ["coven", "coven:fix", "coven:review"], - "bot_usernames": ["coven-github", "cody"], - "familiar": { - "id": "cody", - "display_name": "Cody", - "model": None, - "skills": ["code-review"], - }, - "publication": {"mode": "comment"}, - } - } - } - }, + "installations": {}, } @@ -133,6 +118,8 @@ def sign_rs256(message): def github_app_jwt(): + if not APP_ID: + raise RuntimeError("GITHUB_APP_ID is required") now = int(time.time()) header = {"alg": "RS256", "typ": "JWT"} payload = {"iat": now - 60, "exp": now + 540, "iss": APP_ID} @@ -242,10 +229,17 @@ def labels_include_trigger(labels, policy): return False +def trigger_enabled(policy, trigger): + enabled = policy.get("enabled_triggers") + if enabled is None: + return True + return trigger in set(enabled or []) + + def build_task_from_event(event_name, delivery_id, payload, policy): repository = payload.get("repository") or {} installation = payload.get("installation") or {} - familiar = policy.get("familiar") or DEFAULT_POLICY["installations"]["144638200"]["repositories"]["1290307745"]["familiar"] + familiar = policy.get("familiar") or DEFAULT_FAMILIAR base = { "task_id": delivery_id, "delivery_id": delivery_id, @@ -269,10 +263,12 @@ def build_task_from_event(event_name, delivery_id, payload, policy): comment = payload.get("comment") or {} if not mentioned(comment.get("body"), policy): return ignored(base, "issue_comment_without_mention") + if not trigger_enabled(policy, "issue_mention"): + return ignored(base, "issue_mention_not_enabled") if issue.get("pull_request"): base.update( { - "trigger": "pr_mention", + "trigger": "issue_mention", "target": { "kind": "pull_request", "pr_number": int(issue.get("number") or 0), @@ -304,6 +300,8 @@ def build_task_from_event(event_name, delivery_id, payload, policy): pull_request = payload.get("pull_request") or {} if not mentioned(comment.get("body"), policy): return ignored(base, "pr_review_comment_without_mention") + if not trigger_enabled(policy, "pr_review_comment"): + return ignored(base, "pr_review_comment_not_enabled") base.update( { "trigger": "pr_review_comment", @@ -328,8 +326,14 @@ def build_task_from_event(event_name, delivery_id, payload, policy): action = payload.get("action") if action not in ("assigned", "labeled", "opened"): return ignored(base, "unsupported_issue_action") + if action == "assigned" and not trigger_enabled(policy, "issue_assigned"): + return ignored(base, "issue_assigned_not_enabled") if action == "labeled" and not labels_include_trigger(issue.get("labels"), policy): return ignored(base, "issue_label_not_enabled") + if action == "labeled" and not trigger_enabled(policy, "issue_label"): + return ignored(base, "issue_label_not_enabled") + if action == "opened" and not trigger_enabled(policy, "issue_mention"): + return ignored(base, "issue_mention_not_enabled") base.update( { "trigger": "issue_assigned" if action == "assigned" else "issue_mention", @@ -346,10 +350,12 @@ def build_task_from_event(event_name, delivery_id, payload, policy): if event_name == "pull_request": pull_request = payload.get("pull_request") or {} + if not trigger_enabled(policy, "pull_request"): + return ignored(base, "pull_request_not_enabled") base.update( { "state": "ignored", - "ignored_reason": "pull_request_review_task_not_in_headless_contract_v1", + "ignored_reason": "pull_request_review_task_not_in_headless_contract_v2", "trigger": "pull_request", "target": { "action": payload.get("action"), @@ -364,10 +370,12 @@ def build_task_from_event(event_name, delivery_id, payload, policy): return base if event_name == "push": + if not trigger_enabled(policy, "push"): + return ignored(base, "push_not_enabled") base.update( { "state": "ignored", - "ignored_reason": "push_review_task_not_in_headless_contract_v1", + "ignored_reason": "push_review_task_not_in_headless_contract_v2", "trigger": "push", "target": { "ref": payload.get("ref"), @@ -389,6 +397,53 @@ def ignored(base, reason): return base +def header_value(headers, name): + wanted = name.lower() + for key, value in (headers or {}).items(): + if str(key).lower() == wanted: + return value + return None + + +def verify_webhook_signature(secret, body, signature): + if not secret: + raise RuntimeError("GITHUB_WEBHOOK_SECRET is required") + if not signature or not str(signature).startswith("sha256="): + return False + expected = "sha256=" + hmac.new(secret.encode("utf-8"), body, hashlib.sha256).hexdigest() + return hmac.compare_digest(expected, str(signature)) + + +def route_signed_delivery(headers, body, debug, webhook_secret=None): + raw_body = body.encode("utf-8") if isinstance(body, str) else body + if raw_body is None: + raw_body = b"" + + signature = header_value(headers, "x-hub-signature-256") + if not verify_webhook_signature(webhook_secret or WEBHOOK_SECRET, raw_body, signature): + return {"ok": False, "status": 401, "error": "invalid signature"} + + event_name = header_value(headers, "x-github-event") + if not event_name: + return {"ok": False, "status": 400, "error": "missing event type"} + + delivery_id = header_value(headers, "x-github-delivery") + if not delivery_id: + return {"ok": False, "status": 400, "error": "missing delivery id"} + + try: + payload = json.loads(raw_body.decode("utf-8")) + except json.JSONDecodeError: + return {"ok": False, "status": 400, "error": "invalid json"} + + if event_name == "ping": + return {"ok": True, "status": 200, "pong": True, "delivery_id": delivery_id} + + result = route_delivery(str(event_name), str(delivery_id), payload, debug) + result["status"] = 200 + return result + + def route_delivery(event_name, delivery_id, payload, debug): delivery_file = delivery_path(delivery_id) if delivery_file.exists(): @@ -785,11 +840,7 @@ def prepare_review_context(task, workspace, token, env, attempt_dir): "https://api.github.com/repos/{}/pulls/{}".format(repo, pr_number), token, ) - files = github_request( - "GET", - "https://api.github.com/repos/{}/pulls/{}/files?per_page=100".format(repo, pr_number), - token, - ) + files = paginated_pr_files(repo, pr_number, token) fetch = run_command( ["git", "fetch", "--depth", "1", "origin", "pull/{}/head".format(pr_number)], @@ -799,16 +850,12 @@ def prepare_review_context(task, workspace, token, env, attempt_dir): ) write_json_atomic(attempt_dir / "fetch-pr.json", redacted_command_result(fetch)) if fetch["returncode"] != 0: - return { - "kind": "pull_request", - "pr_number": pr_number, - "fetch_error": fetch["stderr"], - "metadata": summarize_pr(pr), - "files": summarize_pr_files(files), - } + raise RuntimeError("failed to fetch PR #{} head: {}".format(pr_number, fetch["stderr"])) checkout = run_command(["git", "checkout", "--detach", "FETCH_HEAD"], cwd=str(workspace), env=env) write_json_atomic(attempt_dir / "checkout-pr.json", redacted_command_result(checkout)) + if checkout["returncode"] != 0: + raise RuntimeError("failed to checkout PR #{} head: {}".format(pr_number, checkout["stderr"])) head = run_command(["git", "rev-parse", "HEAD"], cwd=str(workspace), env=env) status = run_command(["git", "status", "--short", "--branch"], cwd=str(workspace), env=env) @@ -833,6 +880,24 @@ def prepare_review_context(task, workspace, token, env, attempt_dir): } +def paginated_pr_files(repo, pr_number, token): + files = [] + page = 1 + while True: + page_items = github_request( + "GET", + "https://api.github.com/repos/{}/pulls/{}/files?per_page=100&page={}".format( + repo, pr_number, page + ), + token, + ) + files.extend(page_items or []) + if len(page_items or []) < 100: + break + page += 1 + return files + + def summarize_pr(pr): return { "number": pr.get("number"), diff --git a/docs/contracts/session-brief.schema.json b/docs/contracts/session-brief.schema.json index 850c31e..f8abd31 100644 --- a/docs/contracts/session-brief.schema.json +++ b/docs/contracts/session-brief.schema.json @@ -5,7 +5,7 @@ "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"], + "required": ["contract_version", "trigger", "repo", "task", "familiar", "workspace"], "properties": { "contract_version": { "type": "string", From 7cfc1bed57ccbdc125e6eff4bd8b00c4093a356f Mon Sep 17 00:00:00 2001 From: Timothy Wayne Gregg Date: Mon, 6 Jul 2026 03:19:45 -0400 Subject: [PATCH 07/19] fix(hosted): align review context and routing contract (#119) Signed-off-by: Timothy Wayne Gregg --- crates/worker/src/brief.rs | 6 +++ crates/worker/src/lib.rs | 44 ++++++++++++++++-- crates/worker/tests/contract.rs | 51 +++++++++++++++++++++ deploy/coven-github/coven_github_adapter.py | 26 +++++++++-- docs/contracts/result.schema.json | 5 ++ docs/contracts/session-brief.schema.json | 9 ++++ docs/headless-contract.md | 2 + 7 files changed, 136 insertions(+), 7 deletions(-) diff --git a/crates/worker/src/brief.rs b/crates/worker/src/brief.rs index 2dce3d7..f03074d 100644 --- a/crates/worker/src/brief.rs +++ b/crates/worker/src/brief.rs @@ -27,6 +27,10 @@ pub struct SessionBrief { pub task: TaskBrief, pub familiar: FamiliarBrief, pub workspace: WorkspaceBrief, + #[serde(default, skip_serializing_if = "Option::is_none")] + pub review_context: Option, + #[serde(default, skip_serializing_if = "Option::is_none")] + pub audit_instruction: Option, } #[derive(Debug, Serialize, Deserialize)] @@ -136,6 +140,8 @@ pub fn build( workspace: WorkspaceBrief { root: workspace.to_string_lossy().to_string(), }, + review_context: None, + audit_instruction: None, } } diff --git a/crates/worker/src/lib.rs b/crates/worker/src/lib.rs index 08412f3..c833d01 100644 --- a/crates/worker/src/lib.rs +++ b/crates/worker/src/lib.rs @@ -7,8 +7,8 @@ use tokio::process::Command; use tracing::{error, info, warn}; use coven_github_api::{ - check_run, installation, pr, repo, tasks::TaskStore, SessionResult, SessionStatus, Task, - TaskKind, DEFAULT_API_BASE_URL, + check_run, installation, pr, repo, tasks::TaskStore, ReviewEvidenceStatus, ReviewMode, + SessionResult, SessionStatus, Task, TaskKind, DEFAULT_API_BASE_URL, }; use coven_github_config::{Config, FamiliarConfig}; @@ -506,6 +506,11 @@ async fn read_result(result_path: &Path) -> Result { .await .map_err(|_| anyhow::anyhow!("result.json not written by coven-code"))?; let result: SessionResult = serde_json::from_slice(&bytes)?; + validate_result_contract(&result)?; + Ok(result) +} + +fn validate_result_contract(result: &SessionResult) -> Result<()> { if result.contract_version != coven_github_api::HEADLESS_CONTRACT_VERSION { anyhow::bail!( "unsupported result contract_version {}; expected {}", @@ -513,7 +518,17 @@ async fn read_result(result_path: &Path) -> Result { coven_github_api::HEADLESS_CONTRACT_VERSION ); } - Ok(result) + if matches!( + result.review.mode, + ReviewMode::PullRequest | ReviewMode::ReviewComment + ) && result.review.evidence_status == ReviewEvidenceStatus::NotApplicable + { + anyhow::bail!( + "review evidence_status not_applicable is invalid for {:?}", + result.review.mode + ); + } + Ok(()) } fn cave_base_url(config: &Config) -> &str { @@ -656,6 +671,29 @@ mod result_tests { let _ = fs::remove_file(path); } + + #[tokio::test] + async fn read_result_rejects_not_applicable_evidence_for_review_modes() { + let path = std::env::temp_dir().join(format!( + "coven-github-result-review-evidence-{}.json", + uuid::Uuid::new_v4() + )); + fs::write( + &path, + r#"{"contract_version":"2","status":"success","branch":null,"commits":[],"files_changed":[],"summary":"s","pr_body":"","review":{"mode":"pull_request","evidence_status":"not_applicable","reviewed_files":["src/lib.rs"],"supporting_files":[],"findings":[],"tests_run":[],"no_findings_reason":"reviewed supplied file","limitations":[]},"exit_reason":null}"#, + ) + .expect("result fixture should be written"); + + let error = read_result(&path) + .await + .expect_err("review result must reject not_applicable evidence"); + assert!( + format!("{error:#}").contains("review evidence_status not_applicable is invalid"), + "unexpected error: {error:#}" + ); + + let _ = fs::remove_file(path); + } } #[cfg(test)] diff --git a/crates/worker/tests/contract.rs b/crates/worker/tests/contract.rs index 45387e1..8e74365 100644 --- a/crates/worker/tests/contract.rs +++ b/crates/worker/tests/contract.rs @@ -38,6 +38,57 @@ fn golden_session_brief_deserializes_into_adapter_type() { assert_eq!(reserialized, original, "brief did not round-trip cleanly"); } +#[test] +fn hosted_review_session_brief_deserializes_optional_context() { + let raw = r#"{ + "contract_version": "2", + "trigger": "pr_review_comment", + "repo": { + "owner": "OpenCoven", + "name": "coven-github", + "clone_url": "https://github.com/OpenCoven/coven-github.git", + "default_branch": "main" + }, + "task": { + "kind": "address_review_comment", + "pr_number": 31, + "comment_body": "review this", + "diff_hunk": null + }, + "familiar": { + "id": "cody", + "display_name": "Cody", + "model": null, + "skills": [] + }, + "workspace": { + "root": "/tmp/coven" + }, + "review_context": { + "pr_number": 31, + "files": [{ "path": "src/lib.rs" }] + }, + "audit_instruction": "Inspect supplied changed-file patches." + }"#; + + let brief: SessionBrief = + serde_json::from_str(raw).expect("hosted review brief must match SessionBrief"); + + assert_eq!(brief.trigger, "pr_review_comment"); + assert_eq!( + brief + .review_context + .as_ref() + .and_then(|context| context.get("pr_number")) + .and_then(serde_json::Value::as_u64), + Some(31) + ); + assert_eq!( + brief.audit_instruction.as_deref(), + Some("Inspect supplied changed-file patches.") + ); +} + #[test] fn golden_result_deserializes_into_adapter_type() { let raw = fixture("result.example.json"); diff --git a/deploy/coven-github/coven_github_adapter.py b/deploy/coven-github/coven_github_adapter.py index 4195086..e8e832b 100644 --- a/deploy/coven-github/coven_github_adapter.py +++ b/deploy/coven-github/coven_github_adapter.py @@ -229,6 +229,24 @@ def labels_include_trigger(labels, policy): return False +def issue_assigned_to_bot(issue, policy): + wanted = {str(username).lower() for username in (policy.get("bot_usernames") or [])} + if not wanted: + return False + + candidates = [] + assignee = issue.get("assignee") + if assignee: + candidates.append(assignee) + candidates.extend(issue.get("assignees") or []) + + for candidate in candidates: + login = candidate.get("login") if isinstance(candidate, dict) else str(candidate) + if str(login).lower() in wanted: + return True + return False + + def trigger_enabled(policy, trigger): enabled = policy.get("enabled_triggers") if enabled is None: @@ -324,19 +342,19 @@ def build_task_from_event(event_name, delivery_id, payload, policy): if event_name == "issues": issue = payload.get("issue") or {} action = payload.get("action") - if action not in ("assigned", "labeled", "opened"): + if action not in ("assigned", "labeled"): return ignored(base, "unsupported_issue_action") if action == "assigned" and not trigger_enabled(policy, "issue_assigned"): return ignored(base, "issue_assigned_not_enabled") + if action == "assigned" and not issue_assigned_to_bot(issue, policy): + return ignored(base, "issue_assigned_to_unmanaged_user") if action == "labeled" and not labels_include_trigger(issue.get("labels"), policy): return ignored(base, "issue_label_not_enabled") if action == "labeled" and not trigger_enabled(policy, "issue_label"): return ignored(base, "issue_label_not_enabled") - if action == "opened" and not trigger_enabled(policy, "issue_mention"): - return ignored(base, "issue_mention_not_enabled") base.update( { - "trigger": "issue_assigned" if action == "assigned" else "issue_mention", + "trigger": "issue_assigned", "task": { "kind": "fix_issue", "issue_number": int(issue.get("number") or 0), diff --git a/docs/contracts/result.schema.json b/docs/contracts/result.schema.json index 6b7167b..2ecd61e 100644 --- a/docs/contracts/result.schema.json +++ b/docs/contracts/result.schema.json @@ -114,6 +114,11 @@ "required": ["mode"] }, "then": { + "properties": { + "evidence_status": { + "enum": ["complete", "partial", "missing"] + } + }, "anyOf": [ { "properties": { "reviewed_files": { "minItems": 1 } }, "required": ["reviewed_files"] }, { "properties": { "evidence_status": { "const": "missing" } }, "required": ["evidence_status"] } diff --git a/docs/contracts/session-brief.schema.json b/docs/contracts/session-brief.schema.json index f8abd31..f7f5d47 100644 --- a/docs/contracts/session-brief.schema.json +++ b/docs/contracts/session-brief.schema.json @@ -84,6 +84,15 @@ "properties": { "root": { "type": "string" } } + }, + "review_context": { + "type": "object", + "description": "Optional tokenless hosted-review evidence supplied by the adapter.", + "additionalProperties": true + }, + "audit_instruction": { + "type": "string", + "description": "Optional hosted-review instruction paired with review_context." } } } diff --git a/docs/headless-contract.md b/docs/headless-contract.md index 23f3fc6..86b6b87 100644 --- a/docs/headless-contract.md +++ b/docs/headless-contract.md @@ -106,6 +106,8 @@ The adapter is the **producer**; the runtime is the **consumer**. The brief is | `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. | +| `review_context` | object | Optional tokenless hosted-review evidence supplied by the adapter. When present, it contains PR metadata and changed-file context the runtime MUST inspect before producing review output. | +| `audit_instruction` | string | Optional hosted-review instruction paired with `review_context`. Consumers that do not implement hosted review MAY ignore it. | ### 2.2 Task kinds From 4e19b9367fe74260ed6dfc7aeeab984046b338e2 Mon Sep 17 00:00:00 2001 From: Timothy Wayne Gregg Date: Mon, 6 Jul 2026 03:31:28 -0400 Subject: [PATCH 08/19] fix(hosted): enforce result contract before review loops (#119) Signed-off-by: Timothy Wayne Gregg --- crates/github/src/lib.rs | 7 +- crates/worker/src/lib.rs | 23 ++++ crates/worker/tests/contract.rs | 17 +-- deploy/coven-github/coven_github_adapter.py | 130 +++++++++++++++++--- 4 files changed, 148 insertions(+), 29 deletions(-) diff --git a/crates/github/src/lib.rs b/crates/github/src/lib.rs index 97d04cf..0bca7bd 100644 --- a/crates/github/src/lib.rs +++ b/crates/github/src/lib.rs @@ -14,9 +14,6 @@ pub const DEFAULT_API_BASE_URL: &str = "https://api.github.com"; /// speaks. See `docs/headless-contract.md`. Bump only on breaking changes. pub const HEADLESS_CONTRACT_VERSION: &str = "2"; -fn default_contract_version() -> String { - HEADLESS_CONTRACT_VERSION.to_string() -} const GITHUB_API_VERSION: &str = "2026-03-10"; #[derive(Debug, Clone, PartialEq, Eq)] @@ -176,10 +173,8 @@ pub enum TaskKind { /// Structured result envelope written by coven-code --headless. #[derive(Debug, Clone, Deserialize, Serialize)] pub struct SessionResult { - /// Contract major version. Defaults to the current version when a runtime - /// omits it, but conformant producers MUST emit it. See + /// Contract major version. Conformant producers MUST emit it. See /// `docs/headless-contract.md`. - #[serde(default = "default_contract_version")] pub contract_version: String, pub status: SessionStatus, pub branch: Option, diff --git a/crates/worker/src/lib.rs b/crates/worker/src/lib.rs index c833d01..ed0d34d 100644 --- a/crates/worker/src/lib.rs +++ b/crates/worker/src/lib.rs @@ -672,6 +672,29 @@ mod result_tests { let _ = fs::remove_file(path); } + #[tokio::test] + async fn read_result_rejects_missing_contract_version() { + let path = std::env::temp_dir().join(format!( + "coven-github-result-missing-version-{}.json", + uuid::Uuid::new_v4() + )); + fs::write( + &path, + r#"{"status":"success","branch":null,"commits":[],"files_changed":[],"summary":"s","pr_body":"","review":{"mode":"none","evidence_status":"not_applicable","reviewed_files":[],"supporting_files":[],"findings":[],"tests_run":[],"no_findings_reason":null,"limitations":[]},"exit_reason":null}"#, + ) + .expect("result fixture should be written"); + + let error = read_result(&path) + .await + .expect_err("missing contract_version result must be rejected"); + assert!( + format!("{error:#}").contains("missing field `contract_version`"), + "unexpected error: {error:#}" + ); + + let _ = fs::remove_file(path); + } + #[tokio::test] async fn read_result_rejects_not_applicable_evidence_for_review_modes() { let path = std::env::temp_dir().join(format!( diff --git a/crates/worker/tests/contract.rs b/crates/worker/tests/contract.rs index 8e74365..81e2b9f 100644 --- a/crates/worker/tests/contract.rs +++ b/crates/worker/tests/contract.rs @@ -112,9 +112,7 @@ fn golden_result_deserializes_into_adapter_type() { } #[test] -fn result_without_contract_version_defaults_to_current() { - // Backward compatibility: a runtime that predates the version field still - // parses, and is treated as the current major version. +fn result_without_contract_version_is_rejected() { let raw = r#"{ "status": "failure", "branch": null, @@ -134,10 +132,15 @@ fn result_without_contract_version_defaults_to_current() { }, "exit_reason": "ambiguous_spec" }"#; - let result: SessionResult = serde_json::from_str(raw).expect("legacy result must parse"); - assert_eq!(result.contract_version, HEADLESS_CONTRACT_VERSION); - assert_eq!(result.status, SessionStatus::Failure); - assert_eq!(result.exit_reason, Some(ExitReason::AmbiguousSpec)); + let error = serde_json::from_str::(raw) + .expect_err("v2 result without contract_version must be rejected"); + + assert!( + error + .to_string() + .contains("missing field `contract_version`"), + "unexpected error: {error}" + ); } #[test] diff --git a/deploy/coven-github/coven_github_adapter.py b/deploy/coven-github/coven_github_adapter.py index e8e832b..1a77716 100644 --- a/deploy/coven-github/coven_github_adapter.py +++ b/deploy/coven-github/coven_github_adapter.py @@ -41,6 +41,10 @@ def env_int(name, default, minimum=0, maximum=10): MAX_REVIEW_FIX_LOOPS = env_int("COVEN_REVIEW_FIX_LOOPS", 0, minimum=0, maximum=5) +TERMINAL_RESULT_EXIT_CODES = {0, 1, 3} +RESULT_STATUSES = {"success", "failure", "partial", "needs_input"} +REVIEW_MODES = {"none", "pull_request", "review_comment"} +REVIEW_EVIDENCE_STATUSES = {"not_applicable", "complete", "partial", "missing"} def account_home(): @@ -543,6 +547,66 @@ def write_askpass(work_dir): return script +def validate_result_contract(result): + if not isinstance(result, dict): + raise ValueError("result.json must be a JSON object") + + for field in ("contract_version", "status", "commits", "files_changed", "summary", "pr_body", "review"): + if field not in result: + raise ValueError("result.json missing required field {}".format(field)) + + if result.get("contract_version") != "2": + raise ValueError("unsupported result contract_version {}".format(result.get("contract_version"))) + if result.get("status") not in RESULT_STATUSES: + raise ValueError("unsupported result status {}".format(result.get("status"))) + if not isinstance(result.get("commits"), list): + raise ValueError("result.commits must be an array") + if not isinstance(result.get("files_changed"), list): + raise ValueError("result.files_changed must be an array") + if not isinstance(result.get("summary"), str): + raise ValueError("result.summary must be a string") + if not isinstance(result.get("pr_body"), str): + raise ValueError("result.pr_body must be a string") + + review = result.get("review") + if not isinstance(review, dict): + raise ValueError("result.review must be an object") + for field in ( + "mode", + "evidence_status", + "reviewed_files", + "supporting_files", + "findings", + "tests_run", + "no_findings_reason", + "limitations", + ): + if field not in review: + raise ValueError("result.review missing required field {}".format(field)) + + mode = review.get("mode") + evidence_status = review.get("evidence_status") + if mode not in REVIEW_MODES: + raise ValueError("unsupported review mode {}".format(mode)) + if evidence_status not in REVIEW_EVIDENCE_STATUSES: + raise ValueError("unsupported review evidence_status {}".format(evidence_status)) + + for field in ("reviewed_files", "supporting_files", "findings", "tests_run", "limitations"): + if not isinstance(review.get(field), list): + raise ValueError("result.review.{} must be an array".format(field)) + + if mode in ("pull_request", "review_comment"): + if evidence_status == "not_applicable": + raise ValueError("review evidence_status not_applicable is invalid for {}".format(mode)) + if evidence_status != "missing" and not review.get("reviewed_files"): + raise ValueError("reviewed_files is required for review mode {}".format(mode)) + no_findings_reason = review.get("no_findings_reason") + if not review.get("findings") and not ( + isinstance(no_findings_reason, str) and no_findings_reason.strip() + ): + raise ValueError("no_findings_reason is required when review findings are empty") + + def session_brief(task, workspace, review_context=None, extra_audit_instruction=None): owner, name = (task["repository"] or "/").split("/", 1) brief = { @@ -608,7 +672,15 @@ def run_coven_code_cycle( timeout=1800, ) write_json_atomic(run_path, redacted_command_result(run)) - result = read_json(result_path, None) if result_path.exists() else None + result = None + result_error = None + if result_path.exists(): + try: + result = read_json(result_path, None) + validate_result_contract(result) + except Exception as exc: + result_error = str(exc) + result = None return { "cycle": cycle, "brief_path": brief_path, @@ -616,6 +688,7 @@ def run_coven_code_cycle( "run_path": run_path, "run": run, "result": result, + "result_error": result_error, } @@ -757,15 +830,26 @@ def run_task(task_id, debug): task["result_path"] = str(result_path) write_json_atomic(path, task) - if cycle_result["result"] is None: + if run["returncode"] not in TERMINAL_RESULT_EXIT_CODES: return fail_task( path, task, - "result_missing", - "coven-code exited {} without writing result.json: {}".format( + "runtime_non_terminal", + "coven-code exited {} before a terminal result could be used: {}".format( run["returncode"], run["stderr"] ), ) + if cycle_result["result"] is None: + reason = "result_invalid" if cycle_result.get("result_error") else "result_missing" + detail = cycle_result.get("result_error") or "coven-code exited {} without writing result.json: {}".format( + run["returncode"], run["stderr"] + ) + return fail_task( + path, + task, + reason, + detail, + ) final_cycle = cycle_result loop_records = [] @@ -784,6 +868,32 @@ def run_task(task_id, debug): iteration, instruction, ) + if repair_cycle["run"]["returncode"] not in TERMINAL_RESULT_EXIT_CODES: + return fail_task( + path, + task, + "runtime_non_terminal", + "review repair loop {} exited {} before a terminal result could be used: {}".format( + iteration, + repair_cycle["run"]["returncode"], + repair_cycle["run"]["stderr"], + ), + ) + if repair_cycle["result"] is None: + reason = "result_invalid" if repair_cycle.get("result_error") else "result_missing" + detail = repair_cycle.get("result_error") or ( + "review repair loop {} exited {} without writing result.json: {}".format( + iteration, + repair_cycle["run"]["returncode"], + repair_cycle["run"]["stderr"], + ) + ) + return fail_task( + path, + task, + reason, + detail, + ) remaining = review_findings(repair_cycle["result"]) loop_records.append( { @@ -800,18 +910,6 @@ def run_task(task_id, debug): task["result_path"] = str(repair_cycle["result_path"]) task["updated_at"] = utc_now() write_json_atomic(path, task) - - if repair_cycle["result"] is None: - return fail_task( - path, - task, - "result_missing", - "review repair loop {} exited {} without writing result.json: {}".format( - iteration, - repair_cycle["run"]["returncode"], - repair_cycle["run"]["stderr"], - ), - ) final_cycle = repair_cycle result_path = final_cycle["result_path"] From 9091dc69543410ce58719d2789be716f53cbbd53 Mon Sep 17 00:00:00 2001 From: Timothy Wayne Gregg Date: Mon, 6 Jul 2026 03:48:56 -0400 Subject: [PATCH 09/19] fix(hosted): tighten review result validation (#119) Signed-off-by: Timothy Wayne Gregg --- crates/worker/src/lib.rs | 31 ++++++ deploy/coven-github/coven_github_adapter.py | 110 ++++++++++++++++++-- docs/contracts/result.schema.json | 12 +++ 3 files changed, 146 insertions(+), 7 deletions(-) diff --git a/crates/worker/src/lib.rs b/crates/worker/src/lib.rs index ed0d34d..efc3e3e 100644 --- a/crates/worker/src/lib.rs +++ b/crates/worker/src/lib.rs @@ -528,6 +528,14 @@ fn validate_result_contract(result: &SessionResult) -> Result<()> { result.review.mode ); } + if result.review.mode == ReviewMode::None + && result.review.evidence_status != ReviewEvidenceStatus::NotApplicable + { + anyhow::bail!( + "review evidence_status {:?} is invalid for none mode", + result.review.evidence_status + ); + } Ok(()) } @@ -717,6 +725,29 @@ mod result_tests { let _ = fs::remove_file(path); } + + #[tokio::test] + async fn read_result_rejects_applicable_evidence_for_none_mode() { + let path = std::env::temp_dir().join(format!( + "coven-github-result-none-evidence-{}.json", + uuid::Uuid::new_v4() + )); + fs::write( + &path, + r#"{"contract_version":"2","status":"success","branch":null,"commits":[],"files_changed":[],"summary":"s","pr_body":"","review":{"mode":"none","evidence_status":"complete","reviewed_files":[],"supporting_files":[],"findings":[],"tests_run":[],"no_findings_reason":null,"limitations":[]},"exit_reason":null}"#, + ) + .expect("result fixture should be written"); + + let error = read_result(&path) + .await + .expect_err("none-mode result must reject applicable evidence"); + assert!( + format!("{error:#}").contains("is invalid for none mode"), + "unexpected error: {error:#}" + ); + + let _ = fs::remove_file(path); + } } #[cfg(test)] diff --git a/deploy/coven-github/coven_github_adapter.py b/deploy/coven-github/coven_github_adapter.py index 1a77716..909c328 100644 --- a/deploy/coven-github/coven_github_adapter.py +++ b/deploy/coven-github/coven_github_adapter.py @@ -547,6 +547,90 @@ def write_askpass(work_dir): return script +def require_string(value, path): + if not isinstance(value, str): + raise ValueError("{} must be a string".format(path)) + + +def require_optional_string(value, path): + if value is not None and not isinstance(value, str): + raise ValueError("{} must be a string or null".format(path)) + + +def validate_string_array(values, path): + if not isinstance(values, list): + raise ValueError("{} must be an array".format(path)) + for index, item in enumerate(values): + require_string(item, "{}[{}]".format(path, index)) + + +def validate_commits(commits): + if not isinstance(commits, list): + raise ValueError("result.commits must be an array") + for index, commit in enumerate(commits): + if not isinstance(commit, dict): + raise ValueError("result.commits[{}] must be an object".format(index)) + for field in ("sha", "message"): + if field not in commit: + raise ValueError( + "result.commits[{}] missing required field {}".format(index, field) + ) + require_string(commit.get(field), "result.commits[{}].{}".format(index, field)) + + +def validate_findings(findings): + if not isinstance(findings, list): + raise ValueError("result.review.findings must be an array") + severities = {"info", "low", "medium", "high", "critical"} + for index, finding in enumerate(findings): + if not isinstance(finding, dict): + raise ValueError("result.review.findings[{}] must be an object".format(index)) + for field in ("severity", "file", "line", "title", "body", "recommendation"): + if field not in finding: + raise ValueError( + "result.review.findings[{}] missing required field {}".format(index, field) + ) + severity = finding.get("severity") + if severity not in severities: + raise ValueError("unsupported review finding severity {}".format(severity)) + require_string(finding.get("file"), "result.review.findings[{}].file".format(index)) + line = finding.get("line") + if line is not None and (isinstance(line, bool) or not isinstance(line, int) or line < 1): + raise ValueError( + "result.review.findings[{}].line must be an integer >= 1 or null".format( + index + ) + ) + require_string(finding.get("title"), "result.review.findings[{}].title".format(index)) + require_string(finding.get("body"), "result.review.findings[{}].body".format(index)) + require_optional_string( + finding.get("recommendation"), + "result.review.findings[{}].recommendation".format(index), + ) + + +def validate_tests_run(tests_run): + if not isinstance(tests_run, list): + raise ValueError("result.review.tests_run must be an array") + statuses = {"passed", "failed", "not_run", "unknown"} + for index, test in enumerate(tests_run): + if not isinstance(test, dict): + raise ValueError("result.review.tests_run[{}] must be an object".format(index)) + for field in ("command", "status", "output_summary"): + if field not in test: + raise ValueError( + "result.review.tests_run[{}] missing required field {}".format(index, field) + ) + require_string(test.get("command"), "result.review.tests_run[{}].command".format(index)) + status = test.get("status") + if status not in statuses: + raise ValueError("unsupported review test status {}".format(status)) + require_optional_string( + test.get("output_summary"), + "result.review.tests_run[{}].output_summary".format(index), + ) + + def validate_result_contract(result): if not isinstance(result, dict): raise ValueError("result.json must be a JSON object") @@ -559,14 +643,22 @@ def validate_result_contract(result): raise ValueError("unsupported result contract_version {}".format(result.get("contract_version"))) if result.get("status") not in RESULT_STATUSES: raise ValueError("unsupported result status {}".format(result.get("status"))) - if not isinstance(result.get("commits"), list): - raise ValueError("result.commits must be an array") - if not isinstance(result.get("files_changed"), list): - raise ValueError("result.files_changed must be an array") + validate_commits(result.get("commits")) + validate_string_array(result.get("files_changed"), "result.files_changed") if not isinstance(result.get("summary"), str): raise ValueError("result.summary must be a string") if not isinstance(result.get("pr_body"), str): raise ValueError("result.pr_body must be a string") + if result.get("branch") is not None and not isinstance(result.get("branch"), str): + raise ValueError("result.branch must be a string or null") + if result.get("exit_reason") not in ( + "test_failure", + "ambiguous_spec", + "git_conflict", + "infra_error", + None, + ): + raise ValueError("unsupported result exit_reason {}".format(result.get("exit_reason"))) review = result.get("review") if not isinstance(review, dict): @@ -591,9 +683,11 @@ def validate_result_contract(result): if evidence_status not in REVIEW_EVIDENCE_STATUSES: raise ValueError("unsupported review evidence_status {}".format(evidence_status)) - for field in ("reviewed_files", "supporting_files", "findings", "tests_run", "limitations"): - if not isinstance(review.get(field), list): - raise ValueError("result.review.{} must be an array".format(field)) + validate_string_array(review.get("reviewed_files"), "result.review.reviewed_files") + validate_string_array(review.get("supporting_files"), "result.review.supporting_files") + validate_findings(review.get("findings")) + validate_tests_run(review.get("tests_run")) + validate_string_array(review.get("limitations"), "result.review.limitations") if mode in ("pull_request", "review_comment"): if evidence_status == "not_applicable": @@ -605,6 +699,8 @@ def validate_result_contract(result): isinstance(no_findings_reason, str) and no_findings_reason.strip() ): raise ValueError("no_findings_reason is required when review findings are empty") + if mode == "none" and evidence_status != "not_applicable": + raise ValueError("review evidence_status {} is invalid for none mode".format(evidence_status)) def session_brief(task, workspace, review_context=None, extra_audit_instruction=None): diff --git a/docs/contracts/result.schema.json b/docs/contracts/result.schema.json index 2ecd61e..24a94d6 100644 --- a/docs/contracts/result.schema.json +++ b/docs/contracts/result.schema.json @@ -133,6 +133,18 @@ } ] } + }, + { + "if": { + "properties": { "mode": { "const": "none" } }, + "required": ["mode"] + }, + "then": { + "properties": { + "evidence_status": { "const": "not_applicable" } + }, + "required": ["evidence_status"] + } } ] }, From cacece4b57d1baf83eb7275ee883c5a90c91dacf Mon Sep 17 00:00:00 2001 From: Timothy Wayne Gregg Date: Mon, 6 Jul 2026 03:57:29 -0400 Subject: [PATCH 10/19] fix(worker): enforce v2 review contract invariants (#119) Signed-off-by: Timothy Wayne Gregg --- crates/worker/src/brief.rs | 16 +++++-- crates/worker/src/lib.rs | 81 ++++++++++++++++++++++++++++++--- crates/worker/tests/contract.rs | 76 +++++++++++++++++++++++++++++++ 3 files changed, 163 insertions(+), 10 deletions(-) diff --git a/crates/worker/src/brief.rs b/crates/worker/src/brief.rs index f03074d..761e16d 100644 --- a/crates/worker/src/brief.rs +++ b/crates/worker/src/brief.rs @@ -6,8 +6,18 @@ use std::path::Path; use coven_github_api::{Task, TaskKind, HEADLESS_CONTRACT_VERSION}; use coven_github_config::FamiliarConfig; -fn default_contract_version() -> String { - HEADLESS_CONTRACT_VERSION.to_string() +fn deserialize_contract_version<'de, D>(deserializer: D) -> Result +where + D: serde::Deserializer<'de>, +{ + let version = String::deserialize(deserializer)?; + if version != HEADLESS_CONTRACT_VERSION { + return Err(serde::de::Error::custom(format!( + "unsupported session brief contract_version {}; expected {}", + version, HEADLESS_CONTRACT_VERSION + ))); + } + Ok(version) } /// The session-brief.json schema injected into coven-code --headless. @@ -20,7 +30,7 @@ fn default_contract_version() -> String { pub struct SessionBrief { /// Contract major version this brief is written against. See /// `docs/headless-contract.md`. - #[serde(default = "default_contract_version")] + #[serde(deserialize_with = "deserialize_contract_version")] pub contract_version: String, pub trigger: String, pub repo: RepoBrief, diff --git a/crates/worker/src/lib.rs b/crates/worker/src/lib.rs index efc3e3e..3ea4477 100644 --- a/crates/worker/src/lib.rs +++ b/crates/worker/src/lib.rs @@ -518,15 +518,36 @@ fn validate_result_contract(result: &SessionResult) -> Result<()> { coven_github_api::HEADLESS_CONTRACT_VERSION ); } - if matches!( + let is_review_mode = matches!( result.review.mode, ReviewMode::PullRequest | ReviewMode::ReviewComment - ) && result.review.evidence_status == ReviewEvidenceStatus::NotApplicable - { - anyhow::bail!( - "review evidence_status not_applicable is invalid for {:?}", - result.review.mode - ); + ); + if is_review_mode { + if result.review.evidence_status == ReviewEvidenceStatus::NotApplicable { + anyhow::bail!( + "review evidence_status not_applicable is invalid for {:?}", + result.review.mode + ); + } + if result.review.evidence_status != ReviewEvidenceStatus::Missing + && result.review.reviewed_files.is_empty() + { + anyhow::bail!( + "reviewed_files is required for review mode {:?}", + result.review.mode + ); + } + if result.review.findings.is_empty() + && result + .review + .no_findings_reason + .as_deref() + .map(str::trim) + .unwrap_or_default() + .is_empty() + { + anyhow::bail!("no_findings_reason is required when review findings are empty"); + } } if result.review.mode == ReviewMode::None && result.review.evidence_status != ReviewEvidenceStatus::NotApplicable @@ -726,6 +747,52 @@ mod result_tests { let _ = fs::remove_file(path); } + #[tokio::test] + async fn read_result_rejects_review_without_reviewed_files() { + let path = std::env::temp_dir().join(format!( + "coven-github-result-review-files-{}.json", + uuid::Uuid::new_v4() + )); + fs::write( + &path, + r#"{"contract_version":"2","status":"success","branch":null,"commits":[],"files_changed":[],"summary":"s","pr_body":"","review":{"mode":"pull_request","evidence_status":"complete","reviewed_files":[],"supporting_files":[],"findings":[{"severity":"low","file":"src/lib.rs","line":null,"title":"t","body":"b","recommendation":null}],"tests_run":[],"no_findings_reason":null,"limitations":[]},"exit_reason":null}"#, + ) + .expect("result fixture should be written"); + + let error = read_result(&path) + .await + .expect_err("review result must reject missing reviewed_files"); + assert!( + format!("{error:#}").contains("reviewed_files is required"), + "unexpected error: {error:#}" + ); + + let _ = fs::remove_file(path); + } + + #[tokio::test] + async fn read_result_rejects_empty_review_without_reason() { + let path = std::env::temp_dir().join(format!( + "coven-github-result-review-reason-{}.json", + uuid::Uuid::new_v4() + )); + fs::write( + &path, + r#"{"contract_version":"2","status":"success","branch":null,"commits":[],"files_changed":[],"summary":"s","pr_body":"","review":{"mode":"pull_request","evidence_status":"complete","reviewed_files":["src/lib.rs"],"supporting_files":[],"findings":[],"tests_run":[],"no_findings_reason":" ","limitations":[]},"exit_reason":null}"#, + ) + .expect("result fixture should be written"); + + let error = read_result(&path) + .await + .expect_err("empty review result must require a reason"); + assert!( + format!("{error:#}").contains("no_findings_reason is required"), + "unexpected error: {error:#}" + ); + + let _ = fs::remove_file(path); + } + #[tokio::test] async fn read_result_rejects_applicable_evidence_for_none_mode() { let path = std::env::temp_dir().join(format!( diff --git a/crates/worker/tests/contract.rs b/crates/worker/tests/contract.rs index 81e2b9f..b4429d8 100644 --- a/crates/worker/tests/contract.rs +++ b/crates/worker/tests/contract.rs @@ -89,6 +89,82 @@ fn hosted_review_session_brief_deserializes_optional_context() { ); } +#[test] +fn session_brief_without_contract_version_is_rejected() { + let raw = r#"{ + "trigger": "issue_assigned", + "repo": { + "owner": "OpenCoven", + "name": "coven-github", + "clone_url": "https://github.com/OpenCoven/coven-github.git", + "default_branch": "main" + }, + "task": { + "kind": "fix_issue", + "issue_number": 119, + "issue_title": "Review contract", + "issue_body": "Tighten the contract." + }, + "familiar": { + "id": "cody", + "display_name": "Cody", + "model": null, + "skills": [] + }, + "workspace": { + "root": "/tmp/coven" + } + }"#; + + let error = serde_json::from_str::(raw) + .expect_err("v2 brief without contract_version must be rejected"); + + assert!( + error + .to_string() + .contains("missing field `contract_version`"), + "unexpected error: {error}" + ); +} + +#[test] +fn session_brief_with_unsupported_contract_version_is_rejected() { + let raw = r#"{ + "contract_version": "1", + "trigger": "issue_assigned", + "repo": { + "owner": "OpenCoven", + "name": "coven-github", + "clone_url": "https://github.com/OpenCoven/coven-github.git", + "default_branch": "main" + }, + "task": { + "kind": "fix_issue", + "issue_number": 119, + "issue_title": "Review contract", + "issue_body": "Tighten the contract." + }, + "familiar": { + "id": "cody", + "display_name": "Cody", + "model": null, + "skills": [] + }, + "workspace": { + "root": "/tmp/coven" + } + }"#; + + let error = serde_json::from_str::(raw).expect_err("v1 brief must be rejected"); + + assert!( + error + .to_string() + .contains("unsupported session brief contract_version 1"), + "unexpected error: {error}" + ); +} + #[test] fn golden_result_deserializes_into_adapter_type() { let raw = fixture("result.example.json"); From 6bdfb7bba2da0a6172b4b5fa59417faf4c88d2fc Mon Sep 17 00:00:00 2001 From: Timothy Wayne Gregg Date: Mon, 6 Jul 2026 04:04:41 -0400 Subject: [PATCH 11/19] fix(contract): validate result exit reasons (#119) Signed-off-by: Timothy Wayne Gregg --- crates/worker/src/lib.rs | 55 +++++++++++++++++++++ deploy/coven-github/coven_github_adapter.py | 6 +++ docs/contracts/result.schema.json | 30 ++++++++++- 3 files changed, 90 insertions(+), 1 deletion(-) diff --git a/crates/worker/src/lib.rs b/crates/worker/src/lib.rs index 3ea4477..0420b41 100644 --- a/crates/worker/src/lib.rs +++ b/crates/worker/src/lib.rs @@ -518,6 +518,15 @@ fn validate_result_contract(result: &SessionResult) -> Result<()> { coven_github_api::HEADLESS_CONTRACT_VERSION ); } + if result.status == SessionStatus::Success && result.exit_reason.is_some() { + anyhow::bail!("result exit_reason must be null when status is success"); + } + if result.status != SessionStatus::Success && result.exit_reason.is_none() { + anyhow::bail!( + "result exit_reason is required when status is {:?}", + result.status + ); + } let is_review_mode = matches!( result.review.mode, ReviewMode::PullRequest | ReviewMode::ReviewComment @@ -724,6 +733,52 @@ mod result_tests { let _ = fs::remove_file(path); } + #[tokio::test] + async fn read_result_rejects_success_with_exit_reason() { + let path = std::env::temp_dir().join(format!( + "coven-github-result-success-exit-reason-{}.json", + uuid::Uuid::new_v4() + )); + fs::write( + &path, + r#"{"contract_version":"2","status":"success","branch":null,"commits":[],"files_changed":[],"summary":"s","pr_body":"","review":{"mode":"none","evidence_status":"not_applicable","reviewed_files":[],"supporting_files":[],"findings":[],"tests_run":[],"no_findings_reason":null,"limitations":[]},"exit_reason":"infra_error"}"#, + ) + .expect("result fixture should be written"); + + let error = read_result(&path) + .await + .expect_err("success result must reject non-null exit_reason"); + assert!( + format!("{error:#}").contains("must be null when status is success"), + "unexpected error: {error:#}" + ); + + let _ = fs::remove_file(path); + } + + #[tokio::test] + async fn read_result_rejects_failure_without_exit_reason() { + let path = std::env::temp_dir().join(format!( + "coven-github-result-failure-exit-reason-{}.json", + uuid::Uuid::new_v4() + )); + fs::write( + &path, + r#"{"contract_version":"2","status":"failure","branch":null,"commits":[],"files_changed":[],"summary":"s","pr_body":"","review":{"mode":"none","evidence_status":"not_applicable","reviewed_files":[],"supporting_files":[],"findings":[],"tests_run":[],"no_findings_reason":null,"limitations":[]},"exit_reason":null}"#, + ) + .expect("result fixture should be written"); + + let error = read_result(&path) + .await + .expect_err("non-success result must require exit_reason"); + assert!( + format!("{error:#}").contains("exit_reason is required"), + "unexpected error: {error:#}" + ); + + let _ = fs::remove_file(path); + } + #[tokio::test] async fn read_result_rejects_not_applicable_evidence_for_review_modes() { let path = std::env::temp_dir().join(format!( diff --git a/deploy/coven-github/coven_github_adapter.py b/deploy/coven-github/coven_github_adapter.py index 909c328..c31a6c2 100644 --- a/deploy/coven-github/coven_github_adapter.py +++ b/deploy/coven-github/coven_github_adapter.py @@ -643,6 +643,7 @@ def validate_result_contract(result): raise ValueError("unsupported result contract_version {}".format(result.get("contract_version"))) if result.get("status") not in RESULT_STATUSES: raise ValueError("unsupported result status {}".format(result.get("status"))) + status = result.get("status") validate_commits(result.get("commits")) validate_string_array(result.get("files_changed"), "result.files_changed") if not isinstance(result.get("summary"), str): @@ -659,6 +660,10 @@ def validate_result_contract(result): None, ): raise ValueError("unsupported result exit_reason {}".format(result.get("exit_reason"))) + if status == "success" and result.get("exit_reason") is not None: + raise ValueError("result.exit_reason must be null when status is success") + if status != "success" and result.get("exit_reason") is None: + raise ValueError("result.exit_reason is required when status is {}".format(status)) review = result.get("review") if not isinstance(review, dict): @@ -687,6 +692,7 @@ def validate_result_contract(result): validate_string_array(review.get("supporting_files"), "result.review.supporting_files") validate_findings(review.get("findings")) validate_tests_run(review.get("tests_run")) + require_optional_string(review.get("no_findings_reason"), "result.review.no_findings_reason") validate_string_array(review.get("limitations"), "result.review.limitations") if mode in ("pull_request", "review_comment"): diff --git a/docs/contracts/result.schema.json b/docs/contracts/result.schema.json index 24a94d6..be4409d 100644 --- a/docs/contracts/result.schema.json +++ b/docs/contracts/result.schema.json @@ -152,5 +152,33 @@ "type": ["string", "null"], "enum": ["test_failure", "ambiguous_spec", "git_conflict", "infra_error", null] } - } + }, + "allOf": [ + { + "if": { + "properties": { "status": { "const": "success" } }, + "required": ["status"] + }, + "then": { + "properties": { + "exit_reason": { "const": null } + } + } + }, + { + "if": { + "properties": { "status": { "enum": ["failure", "partial", "needs_input"] } }, + "required": ["status"] + }, + "then": { + "properties": { + "exit_reason": { + "type": "string", + "enum": ["test_failure", "ambiguous_spec", "git_conflict", "infra_error"] + } + }, + "required": ["exit_reason"] + } + } + ] } From 2cc54f708172eda3431cf456b4f7a927e846248c Mon Sep 17 00:00:00 2001 From: Timothy Wayne Gregg Date: Mon, 6 Jul 2026 04:13:49 -0400 Subject: [PATCH 12/19] fix(contract): close result envelope validation (#119) Signed-off-by: Timothy Wayne Gregg --- crates/github/src/lib.rs | 5 ++ crates/worker/src/lib.rs | 70 +++++++++++++++++++++ deploy/coven-github/coven_github_adapter.py | 52 +++++++++++---- docs/contracts/result.schema.json | 2 +- 4 files changed, 115 insertions(+), 14 deletions(-) diff --git a/crates/github/src/lib.rs b/crates/github/src/lib.rs index 0bca7bd..bf711a0 100644 --- a/crates/github/src/lib.rs +++ b/crates/github/src/lib.rs @@ -172,6 +172,7 @@ pub enum TaskKind { /// Structured result envelope written by coven-code --headless. #[derive(Debug, Clone, Deserialize, Serialize)] +#[serde(deny_unknown_fields)] pub struct SessionResult { /// Contract major version. Conformant producers MUST emit it. See /// `docs/headless-contract.md`. @@ -196,12 +197,14 @@ pub enum SessionStatus { } #[derive(Debug, Clone, Deserialize, Serialize)] +#[serde(deny_unknown_fields)] pub struct CommitInfo { pub sha: String, pub message: String, } #[derive(Debug, Clone, Deserialize, Serialize)] +#[serde(deny_unknown_fields)] pub struct ReviewResult { pub mode: ReviewMode, pub evidence_status: ReviewEvidenceStatus, @@ -246,6 +249,7 @@ pub enum ReviewEvidenceStatus { } #[derive(Debug, Clone, Deserialize, Serialize)] +#[serde(deny_unknown_fields)] pub struct ReviewFinding { pub severity: ReviewSeverity, pub file: String, @@ -266,6 +270,7 @@ pub enum ReviewSeverity { } #[derive(Debug, Clone, Deserialize, Serialize)] +#[serde(deny_unknown_fields)] pub struct ReviewTestRun { pub command: String, pub status: ReviewTestStatus, diff --git a/crates/worker/src/lib.rs b/crates/worker/src/lib.rs index 0420b41..977762b 100644 --- a/crates/worker/src/lib.rs +++ b/crates/worker/src/lib.rs @@ -779,6 +779,52 @@ mod result_tests { let _ = fs::remove_file(path); } + #[tokio::test] + async fn read_result_rejects_unknown_root_field() { + let path = std::env::temp_dir().join(format!( + "coven-github-result-extra-root-{}.json", + uuid::Uuid::new_v4() + )); + fs::write( + &path, + r#"{"contract_version":"2","status":"success","branch":null,"commits":[],"files_changed":[],"summary":"s","pr_body":"","review":{"mode":"none","evidence_status":"not_applicable","reviewed_files":[],"supporting_files":[],"findings":[],"tests_run":[],"no_findings_reason":null,"limitations":[]},"exit_reason":null,"extra_root_field":"rejected"}"#, + ) + .expect("result fixture should be written"); + + let error = read_result(&path) + .await + .expect_err("unknown root field must be rejected"); + assert!( + format!("{error:#}").contains("unknown field `extra_root_field`"), + "unexpected error: {error:#}" + ); + + let _ = fs::remove_file(path); + } + + #[tokio::test] + async fn read_result_rejects_unknown_review_field() { + let path = std::env::temp_dir().join(format!( + "coven-github-result-extra-review-{}.json", + uuid::Uuid::new_v4() + )); + fs::write( + &path, + r#"{"contract_version":"2","status":"success","branch":null,"commits":[],"files_changed":[],"summary":"s","pr_body":"","review":{"mode":"none","evidence_status":"not_applicable","reviewed_files":[],"supporting_files":[],"findings":[],"tests_run":[],"no_findings_reason":null,"limitations":[],"extra_review_field":"rejected"},"exit_reason":null}"#, + ) + .expect("result fixture should be written"); + + let error = read_result(&path) + .await + .expect_err("unknown review field must be rejected"); + assert!( + format!("{error:#}").contains("unknown field `extra_review_field`"), + "unexpected error: {error:#}" + ); + + let _ = fs::remove_file(path); + } + #[tokio::test] async fn read_result_rejects_not_applicable_evidence_for_review_modes() { let path = std::env::temp_dir().join(format!( @@ -825,6 +871,30 @@ mod result_tests { let _ = fs::remove_file(path); } + #[tokio::test] + async fn read_result_accepts_review_findings_with_reason() { + let path = std::env::temp_dir().join(format!( + "coven-github-result-findings-reason-{}.json", + uuid::Uuid::new_v4() + )); + fs::write( + &path, + r#"{"contract_version":"2","status":"success","branch":null,"commits":[],"files_changed":[],"summary":"s","pr_body":"","review":{"mode":"pull_request","evidence_status":"complete","reviewed_files":["src/lib.rs"],"supporting_files":[],"findings":[{"severity":"low","file":"src/lib.rs","line":null,"title":"t","body":"b","recommendation":null}],"tests_run":[],"no_findings_reason":"Also reviewed nearby context and found this issue.","limitations":[]},"exit_reason":null}"#, + ) + .expect("result fixture should be written"); + + let result = read_result(&path) + .await + .expect("findings with a reason should remain valid"); + assert_eq!(result.review.findings.len(), 1); + assert_eq!( + result.review.no_findings_reason.as_deref(), + Some("Also reviewed nearby context and found this issue.") + ); + + let _ = fs::remove_file(path); + } + #[tokio::test] async fn read_result_rejects_empty_review_without_reason() { let path = std::env::temp_dir().join(format!( diff --git a/deploy/coven-github/coven_github_adapter.py b/deploy/coven-github/coven_github_adapter.py index c31a6c2..12eca58 100644 --- a/deploy/coven-github/coven_github_adapter.py +++ b/deploy/coven-github/coven_github_adapter.py @@ -45,6 +45,30 @@ def env_int(name, default, minimum=0, maximum=10): RESULT_STATUSES = {"success", "failure", "partial", "needs_input"} REVIEW_MODES = {"none", "pull_request", "review_comment"} REVIEW_EVIDENCE_STATUSES = {"not_applicable", "complete", "partial", "missing"} +RESULT_FIELDS = { + "contract_version", + "status", + "branch", + "commits", + "files_changed", + "summary", + "pr_body", + "review", + "exit_reason", +} +COMMIT_FIELDS = {"sha", "message"} +REVIEW_FIELDS = { + "mode", + "evidence_status", + "reviewed_files", + "supporting_files", + "findings", + "tests_run", + "no_findings_reason", + "limitations", +} +FINDING_FIELDS = {"severity", "file", "line", "title", "body", "recommendation"} +TEST_RUN_FIELDS = {"command", "status", "output_summary"} def account_home(): @@ -557,6 +581,12 @@ def require_optional_string(value, path): raise ValueError("{} must be a string or null".format(path)) +def validate_object_fields(value, allowed, path): + extra = set(value.keys()) - allowed + if extra: + raise ValueError("{} has unsupported field {}".format(path, sorted(extra)[0])) + + def validate_string_array(values, path): if not isinstance(values, list): raise ValueError("{} must be an array".format(path)) @@ -570,7 +600,8 @@ def validate_commits(commits): for index, commit in enumerate(commits): if not isinstance(commit, dict): raise ValueError("result.commits[{}] must be an object".format(index)) - for field in ("sha", "message"): + validate_object_fields(commit, COMMIT_FIELDS, "result.commits[{}]".format(index)) + for field in COMMIT_FIELDS: if field not in commit: raise ValueError( "result.commits[{}] missing required field {}".format(index, field) @@ -585,7 +616,8 @@ def validate_findings(findings): for index, finding in enumerate(findings): if not isinstance(finding, dict): raise ValueError("result.review.findings[{}] must be an object".format(index)) - for field in ("severity", "file", "line", "title", "body", "recommendation"): + validate_object_fields(finding, FINDING_FIELDS, "result.review.findings[{}]".format(index)) + for field in FINDING_FIELDS: if field not in finding: raise ValueError( "result.review.findings[{}] missing required field {}".format(index, field) @@ -616,7 +648,8 @@ def validate_tests_run(tests_run): for index, test in enumerate(tests_run): if not isinstance(test, dict): raise ValueError("result.review.tests_run[{}] must be an object".format(index)) - for field in ("command", "status", "output_summary"): + validate_object_fields(test, TEST_RUN_FIELDS, "result.review.tests_run[{}]".format(index)) + for field in TEST_RUN_FIELDS: if field not in test: raise ValueError( "result.review.tests_run[{}] missing required field {}".format(index, field) @@ -635,6 +668,7 @@ def validate_result_contract(result): if not isinstance(result, dict): raise ValueError("result.json must be a JSON object") + validate_object_fields(result, RESULT_FIELDS, "result.json") for field in ("contract_version", "status", "commits", "files_changed", "summary", "pr_body", "review"): if field not in result: raise ValueError("result.json missing required field {}".format(field)) @@ -668,16 +702,8 @@ def validate_result_contract(result): review = result.get("review") if not isinstance(review, dict): raise ValueError("result.review must be an object") - for field in ( - "mode", - "evidence_status", - "reviewed_files", - "supporting_files", - "findings", - "tests_run", - "no_findings_reason", - "limitations", - ): + validate_object_fields(review, REVIEW_FIELDS, "result.review") + for field in REVIEW_FIELDS: if field not in review: raise ValueError("result.review missing required field {}".format(field)) diff --git a/docs/contracts/result.schema.json b/docs/contracts/result.schema.json index be4409d..6685f81 100644 --- a/docs/contracts/result.schema.json +++ b/docs/contracts/result.schema.json @@ -123,7 +123,7 @@ { "properties": { "reviewed_files": { "minItems": 1 } }, "required": ["reviewed_files"] }, { "properties": { "evidence_status": { "const": "missing" } }, "required": ["evidence_status"] } ], - "oneOf": [ + "anyOf": [ { "properties": { "findings": { "minItems": 1 } }, "required": ["findings"] }, { "properties": { From cec7a28a01e40c7785d562d895e5d9fcd2acbac0 Mon Sep 17 00:00:00 2001 From: Timothy Wayne Gregg Date: Mon, 6 Jul 2026 04:20:03 -0400 Subject: [PATCH 13/19] fix(contract): align session brief validation (#119) Signed-off-by: Timothy Wayne Gregg --- crates/worker/src/brief.rs | 6 +- crates/worker/tests/contract.rs | 163 ++++++++++++++++++++++++++++++ docs/contracts/result.schema.json | 26 +++-- 3 files changed, 184 insertions(+), 11 deletions(-) diff --git a/crates/worker/src/brief.rs b/crates/worker/src/brief.rs index 761e16d..11a27b5 100644 --- a/crates/worker/src/brief.rs +++ b/crates/worker/src/brief.rs @@ -27,6 +27,7 @@ where /// write authority (comments, Check Runs, branches, PRs) stays with the adapter /// behind its publication gate. See issue #4. #[derive(Debug, Serialize, Deserialize)] +#[serde(deny_unknown_fields)] pub struct SessionBrief { /// Contract major version this brief is written against. See /// `docs/headless-contract.md`. @@ -44,6 +45,7 @@ pub struct SessionBrief { } #[derive(Debug, Serialize, Deserialize)] +#[serde(deny_unknown_fields)] pub struct RepoBrief { pub owner: String, pub name: String, @@ -52,7 +54,7 @@ pub struct RepoBrief { } #[derive(Debug, Serialize, Deserialize)] -#[serde(tag = "kind", rename_all = "snake_case")] +#[serde(tag = "kind", rename_all = "snake_case", deny_unknown_fields)] pub enum TaskBrief { FixIssue { issue_number: u64, @@ -71,6 +73,7 @@ pub enum TaskBrief { } #[derive(Debug, Serialize, Deserialize)] +#[serde(deny_unknown_fields)] pub struct FamiliarBrief { pub id: String, pub display_name: String, @@ -79,6 +82,7 @@ pub struct FamiliarBrief { } #[derive(Debug, Serialize, Deserialize)] +#[serde(deny_unknown_fields)] pub struct WorkspaceBrief { pub root: String, } diff --git a/crates/worker/tests/contract.rs b/crates/worker/tests/contract.rs index b4429d8..d0b8dc7 100644 --- a/crates/worker/tests/contract.rs +++ b/crates/worker/tests/contract.rs @@ -165,6 +165,169 @@ fn session_brief_with_unsupported_contract_version_is_rejected() { ); } +#[test] +fn session_brief_rejects_unknown_fields() { + let fixtures = [ + ( + "extra_root", + r#"{ + "contract_version": "2", + "trigger": "issue_assigned", + "repo": { + "owner": "OpenCoven", + "name": "coven-github", + "clone_url": "https://github.com/OpenCoven/coven-github.git", + "default_branch": "main" + }, + "task": { + "kind": "fix_issue", + "issue_number": 119, + "issue_title": "Review contract", + "issue_body": "Tighten the contract." + }, + "familiar": { + "id": "cody", + "display_name": "Cody", + "model": null, + "skills": [] + }, + "workspace": { + "root": "/tmp/coven" + }, + "extra_root": true + }"#, + ), + ( + "extra_repo", + r#"{ + "contract_version": "2", + "trigger": "issue_assigned", + "repo": { + "owner": "OpenCoven", + "name": "coven-github", + "clone_url": "https://github.com/OpenCoven/coven-github.git", + "default_branch": "main", + "extra_repo": true + }, + "task": { + "kind": "fix_issue", + "issue_number": 119, + "issue_title": "Review contract", + "issue_body": "Tighten the contract." + }, + "familiar": { + "id": "cody", + "display_name": "Cody", + "model": null, + "skills": [] + }, + "workspace": { + "root": "/tmp/coven" + } + }"#, + ), + ( + "extra_task", + r#"{ + "contract_version": "2", + "trigger": "issue_assigned", + "repo": { + "owner": "OpenCoven", + "name": "coven-github", + "clone_url": "https://github.com/OpenCoven/coven-github.git", + "default_branch": "main" + }, + "task": { + "kind": "fix_issue", + "issue_number": 119, + "issue_title": "Review contract", + "issue_body": "Tighten the contract.", + "extra_task": true + }, + "familiar": { + "id": "cody", + "display_name": "Cody", + "model": null, + "skills": [] + }, + "workspace": { + "root": "/tmp/coven" + } + }"#, + ), + ( + "extra_familiar", + r#"{ + "contract_version": "2", + "trigger": "issue_assigned", + "repo": { + "owner": "OpenCoven", + "name": "coven-github", + "clone_url": "https://github.com/OpenCoven/coven-github.git", + "default_branch": "main" + }, + "task": { + "kind": "fix_issue", + "issue_number": 119, + "issue_title": "Review contract", + "issue_body": "Tighten the contract." + }, + "familiar": { + "id": "cody", + "display_name": "Cody", + "model": null, + "skills": [], + "extra_familiar": true + }, + "workspace": { + "root": "/tmp/coven" + } + }"#, + ), + ( + "extra_workspace", + r#"{ + "contract_version": "2", + "trigger": "issue_assigned", + "repo": { + "owner": "OpenCoven", + "name": "coven-github", + "clone_url": "https://github.com/OpenCoven/coven-github.git", + "default_branch": "main" + }, + "task": { + "kind": "fix_issue", + "issue_number": 119, + "issue_title": "Review contract", + "issue_body": "Tighten the contract." + }, + "familiar": { + "id": "cody", + "display_name": "Cody", + "model": null, + "skills": [] + }, + "workspace": { + "root": "/tmp/coven", + "extra_workspace": true + } + }"#, + ), + ]; + + for (field, raw) in fixtures { + let error = match serde_json::from_str::(raw) { + Ok(_) => panic!("brief with {field} must be rejected"), + Err(error) => error, + }; + let message = error.to_string(); + assert!( + message.contains("unknown field"), + "unexpected error for {field}: {message}" + ); + } +} + #[test] fn golden_result_deserializes_into_adapter_type() { let raw = fixture("result.example.json"); diff --git a/docs/contracts/result.schema.json b/docs/contracts/result.schema.json index 6685f81..1fc1a47 100644 --- a/docs/contracts/result.schema.json +++ b/docs/contracts/result.schema.json @@ -119,17 +119,23 @@ "enum": ["complete", "partial", "missing"] } }, - "anyOf": [ - { "properties": { "reviewed_files": { "minItems": 1 } }, "required": ["reviewed_files"] }, - { "properties": { "evidence_status": { "const": "missing" } }, "required": ["evidence_status"] } - ], - "anyOf": [ - { "properties": { "findings": { "minItems": 1 } }, "required": ["findings"] }, + "allOf": [ { - "properties": { - "no_findings_reason": { "type": "string", "minLength": 1 } - }, - "required": ["no_findings_reason"] + "anyOf": [ + { "properties": { "reviewed_files": { "minItems": 1 } }, "required": ["reviewed_files"] }, + { "properties": { "evidence_status": { "const": "missing" } }, "required": ["evidence_status"] } + ] + }, + { + "anyOf": [ + { "properties": { "findings": { "minItems": 1 } }, "required": ["findings"] }, + { + "properties": { + "no_findings_reason": { "type": "string", "minLength": 1 } + }, + "required": ["no_findings_reason"] + } + ] } ] } From e5b59c8dbca6d9ccf3dde4adeceb20ee7345c6e7 Mon Sep 17 00:00:00 2001 From: Timothy Wayne Gregg Date: Mon, 6 Jul 2026 04:30:12 -0400 Subject: [PATCH 14/19] fix(hosted): harden review routing evidence (#119) Signed-off-by: Timothy Wayne Gregg --- deploy/coven-github/coven_github_adapter.py | 30 ++++++++- .../coven-github/test_coven_github_adapter.py | 66 +++++++++++++++++++ 2 files changed, 93 insertions(+), 3 deletions(-) create mode 100644 deploy/coven-github/test_coven_github_adapter.py diff --git a/deploy/coven-github/coven_github_adapter.py b/deploy/coven-github/coven_github_adapter.py index 12eca58..3dad945 100644 --- a/deploy/coven-github/coven_github_adapter.py +++ b/deploy/coven-github/coven_github_adapter.py @@ -3,6 +3,7 @@ import hmac import json import os +import re import subprocess import tempfile import time @@ -241,9 +242,15 @@ def delivery_record(delivery_id, event_name, payload): def mentioned(text, policy): - normalized = (text or "").lower() + normalized = text or "" for username in policy.get("bot_usernames") or []: - if "@" + username.lower() in normalized: + login = str(username).strip() + if not login: + continue + pattern = r"(? Date: Mon, 6 Jul 2026 04:37:28 -0400 Subject: [PATCH 15/19] ci: run hosted adapter python tests (#119) Signed-off-by: Timothy Wayne Gregg --- .github/workflows/ci.yml | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 805eee1..7b26b54 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -11,7 +11,7 @@ env: jobs: check: - name: cargo check + clippy + test + name: python + cargo check + clippy + test runs-on: ubuntu-latest steps: - uses: actions/checkout@v4 @@ -19,6 +19,10 @@ jobs: with: components: clippy - uses: Swatinem/rust-cache@v2 + - name: hosted adapter python compile + run: python -B -m py_compile deploy/coven-github/coven_github_adapter.py deploy/coven-github/test_coven_github_adapter.py + - name: hosted adapter python tests + run: python -B -m unittest discover -s deploy/coven-github -p "test_*.py" - name: cargo check run: cargo check --all-targets - name: clippy From 2584b34ef3db88e68bbd089a27edba12feb28401 Mon Sep 17 00:00:00 2001 From: Timothy Wayne Gregg Date: Mon, 6 Jul 2026 04:42:25 -0400 Subject: [PATCH 16/19] fix(hosted): reject team-style bot mentions (#119) --- deploy/coven-github/coven_github_adapter.py | 2 +- deploy/coven-github/test_coven_github_adapter.py | 8 +++++++- 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/deploy/coven-github/coven_github_adapter.py b/deploy/coven-github/coven_github_adapter.py index 3dad945..3400c03 100644 --- a/deploy/coven-github/coven_github_adapter.py +++ b/deploy/coven-github/coven_github_adapter.py @@ -247,7 +247,7 @@ def mentioned(text, policy): login = str(username).strip() if not login: continue - pattern = r"(? Date: Mon, 6 Jul 2026 06:47:06 -0500 Subject: [PATCH 17/19] Potential fix for pull request finding Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> --- deploy/coven-github/coven_github_adapter.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/deploy/coven-github/coven_github_adapter.py b/deploy/coven-github/coven_github_adapter.py index 3400c03..1ed3f3c 100644 --- a/deploy/coven-github/coven_github_adapter.py +++ b/deploy/coven-github/coven_github_adapter.py @@ -908,7 +908,7 @@ def run_task(task_id, debug): env["COVEN_GIT_TOKEN"] = token env["COVEN_CODE_PROVIDER"] = "codex" env["COVEN_CODE_HOSTED_REVIEW"] = "1" - env["HOME"] = str(CODEX_TOKENS_PATH.parent.parent) + env["HOME"] = str(account_home()) codex_access_token = load_codex_access_token() if not codex_access_token: return fail_task( From 089d347ab67df227a85419e41e961bbbbd4603b5 Mon Sep 17 00:00:00 2001 From: Timothy Wayne Gregg Date: Mon, 6 Jul 2026 12:25:42 -0400 Subject: [PATCH 18/19] fix(worker): align v2 review result contract Accepts degraded partial review results from coven-code while preserving complete clean-review validation. Aligns the v2 schemas and contract prose with coven-code PR #132. Refs #11 Refs PR #31 Signed-off-by: Timothy Wayne Gregg --- crates/worker/src/lib.rs | 40 ++++++++++++++++++++---- docs/contracts/result.schema.json | 23 +++----------- docs/contracts/session-brief.schema.json | 30 ++++++++++++++---- docs/headless-contract.md | 14 +++++---- 4 files changed, 70 insertions(+), 37 deletions(-) diff --git a/crates/worker/src/lib.rs b/crates/worker/src/lib.rs index 977762b..a01e644 100644 --- a/crates/worker/src/lib.rs +++ b/crates/worker/src/lib.rs @@ -521,7 +521,11 @@ fn validate_result_contract(result: &SessionResult) -> Result<()> { if result.status == SessionStatus::Success && result.exit_reason.is_some() { anyhow::bail!("result exit_reason must be null when status is success"); } - if result.status != SessionStatus::Success && result.exit_reason.is_none() { + if matches!( + result.status, + SessionStatus::Failure | SessionStatus::NeedsInput + ) && result.exit_reason.is_none() + { anyhow::bail!( "result exit_reason is required when status is {:?}", result.status @@ -546,7 +550,8 @@ fn validate_result_contract(result: &SessionResult) -> Result<()> { result.review.mode ); } - if result.review.findings.is_empty() + if result.review.evidence_status == ReviewEvidenceStatus::Complete + && result.review.findings.is_empty() && result .review .no_findings_reason @@ -555,7 +560,7 @@ fn validate_result_contract(result: &SessionResult) -> Result<()> { .unwrap_or_default() .is_empty() { - anyhow::bail!("no_findings_reason is required when review findings are empty"); + anyhow::bail!("no_findings_reason is required when complete review findings are empty"); } } if result.review.mode == ReviewMode::None @@ -896,11 +901,34 @@ mod result_tests { } #[tokio::test] - async fn read_result_rejects_empty_review_without_reason() { + async fn read_result_accepts_partial_review_without_reason() { let path = std::env::temp_dir().join(format!( "coven-github-result-review-reason-{}.json", uuid::Uuid::new_v4() )); + fs::write( + &path, + r#"{"contract_version":"2","status":"partial","branch":null,"commits":[],"files_changed":[],"summary":"s","pr_body":"","review":{"mode":"pull_request","evidence_status":"partial","reviewed_files":["src/lib.rs"],"supporting_files":[],"findings":[],"tests_run":[],"no_findings_reason":null,"limitations":["Review output was degraded before a clean-review conclusion."]},"exit_reason":null}"#, + ) + .expect("result fixture should be written"); + + let result = read_result(&path) + .await + .expect("partial degraded review result should remain valid"); + assert_eq!(result.status, SessionStatus::Partial); + assert!(result.exit_reason.is_none()); + assert!(result.review.findings.is_empty()); + assert!(result.review.no_findings_reason.is_none()); + + let _ = fs::remove_file(path); + } + + #[tokio::test] + async fn read_result_rejects_complete_review_without_reason() { + let path = std::env::temp_dir().join(format!( + "coven-github-result-complete-review-reason-{}.json", + uuid::Uuid::new_v4() + )); fs::write( &path, r#"{"contract_version":"2","status":"success","branch":null,"commits":[],"files_changed":[],"summary":"s","pr_body":"","review":{"mode":"pull_request","evidence_status":"complete","reviewed_files":["src/lib.rs"],"supporting_files":[],"findings":[],"tests_run":[],"no_findings_reason":" ","limitations":[]},"exit_reason":null}"#, @@ -909,9 +937,9 @@ mod result_tests { let error = read_result(&path) .await - .expect_err("empty review result must require a reason"); + .expect_err("complete empty review result must require a reason"); assert!( - format!("{error:#}").contains("no_findings_reason is required"), + format!("{error:#}").contains("complete review findings are empty"), "unexpected error: {error:#}" ); diff --git a/docs/contracts/result.schema.json b/docs/contracts/result.schema.json index 1fc1a47..4cae129 100644 --- a/docs/contracts/result.schema.json +++ b/docs/contracts/result.schema.json @@ -119,24 +119,9 @@ "enum": ["complete", "partial", "missing"] } }, - "allOf": [ - { - "anyOf": [ - { "properties": { "reviewed_files": { "minItems": 1 } }, "required": ["reviewed_files"] }, - { "properties": { "evidence_status": { "const": "missing" } }, "required": ["evidence_status"] } - ] - }, - { - "anyOf": [ - { "properties": { "findings": { "minItems": 1 } }, "required": ["findings"] }, - { - "properties": { - "no_findings_reason": { "type": "string", "minLength": 1 } - }, - "required": ["no_findings_reason"] - } - ] - } + "anyOf": [ + { "properties": { "reviewed_files": { "minItems": 1 } }, "required": ["reviewed_files"] }, + { "properties": { "evidence_status": { "const": "missing" } }, "required": ["evidence_status"] } ] } }, @@ -173,7 +158,7 @@ }, { "if": { - "properties": { "status": { "enum": ["failure", "partial", "needs_input"] } }, + "properties": { "status": { "enum": ["failure", "needs_input"] } }, "required": ["status"] }, "then": { diff --git a/docs/contracts/session-brief.schema.json b/docs/contracts/session-brief.schema.json index f7f5d47..60d7625 100644 --- a/docs/contracts/session-brief.schema.json +++ b/docs/contracts/session-brief.schema.json @@ -85,14 +85,32 @@ "root": { "type": "string" } } }, - "review_context": { - "type": "object", - "description": "Optional tokenless hosted-review evidence supplied by the adapter.", - "additionalProperties": true - }, "audit_instruction": { - "type": "string", + "type": ["string", "null"], "description": "Optional hosted-review instruction paired with review_context." + }, + "review_context": { + "type": ["object", "null"], + "description": "Optional tokenless hosted-review evidence supplied by the adapter.", + "additionalProperties": false, + "required": ["kind", "files"], + "properties": { + "kind": { + "type": ["string", "null"], + "enum": ["pull_request", null] + }, + "files": { + "type": "array", + "items": { + "type": "object", + "additionalProperties": false, + "required": ["filename"], + "properties": { + "filename": { "type": "string" } + } + } + } + } } } } diff --git a/docs/headless-contract.md b/docs/headless-contract.md index 86b6b87..a81bf87 100644 --- a/docs/headless-contract.md +++ b/docs/headless-contract.md @@ -86,7 +86,9 @@ The adapter is the **producer**; the runtime is the **consumer**. The brief is }, "workspace": { "root": "/tmp/task-abc123" - } + }, + "review_context": null, + "audit_instruction": null } ``` @@ -106,8 +108,8 @@ The adapter is the **producer**; the runtime is the **consumer**. The brief is | `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. | -| `review_context` | object | Optional tokenless hosted-review evidence supplied by the adapter. When present, it contains PR metadata and changed-file context the runtime MUST inspect before producing review output. | -| `audit_instruction` | string | Optional hosted-review instruction paired with `review_context`. Consumers that do not implement hosted review MAY ignore it. | +| `review_context` | object \| null | Optional tokenless hosted-review evidence supplied by the adapter. `kind: "pull_request"` puts the runtime in PR review mode and carries changed-file metadata. | +| `audit_instruction` | string \| null | Optional adapter-authored review instruction appended to the review prompt. | ### 2.2 Task kinds @@ -185,9 +187,9 @@ the intended code. It is required on every result. Non-review tasks MUST set | `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. | +| `findings` | array | Structured findings. Empty is allowed. For complete no-finding reviews, `no_findings_reason` SHOULD explain the clean outcome. | | `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. | +| `no_findings_reason` | string \| null | File-backed explanation for a clean review. MAY be `null` for degraded/partial output when `evidence_status` and `limitations` explain why a substantive clean-review conclusion was not possible. | | `limitations` | string[] | Evidence gaps, skipped checks, or other caveats. | Each finding carries `severity`, `file`, optional `line`, `title`, `body`, and @@ -199,7 +201,7 @@ and `critical`. | Value | Meaning | |---|---| | `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. | +| `partial` | Progress or review evidence is incomplete but usable. `exit_reason` MAY be `null` when the result is partial because of review-evidence limitations rather than a terminal error. 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`. | From 5d22ccee27d53e3e9385d7510a7804b3bd92d1e4 Mon Sep 17 00:00:00 2001 From: Timothy Wayne Gregg Date: Mon, 6 Jul 2026 12:28:35 -0400 Subject: [PATCH 19/19] fix(adapter): handle missing webhook secret Return a structured 500 for missing webhook secret configuration and clarify nullable finding fields in the v2 contract docs. Refs PR #31 Signed-off-by: Timothy Wayne Gregg --- deploy/coven-github/coven_github_adapter.py | 9 ++++++++- deploy/coven-github/test_coven_github_adapter.py | 12 ++++++++++++ docs/headless-contract.md | 6 +++--- 3 files changed, 23 insertions(+), 4 deletions(-) diff --git a/deploy/coven-github/coven_github_adapter.py b/deploy/coven-github/coven_github_adapter.py index 1ed3f3c..10290ee 100644 --- a/deploy/coven-github/coven_github_adapter.py +++ b/deploy/coven-github/coven_github_adapter.py @@ -473,7 +473,14 @@ def route_signed_delivery(headers, body, debug, webhook_secret=None): raw_body = b"" signature = header_value(headers, "x-hub-signature-256") - if not verify_webhook_signature(webhook_secret or WEBHOOK_SECRET, raw_body, signature): + try: + signature_valid = verify_webhook_signature( + webhook_secret or WEBHOOK_SECRET, raw_body, signature + ) + except RuntimeError as exc: + return {"ok": False, "status": 500, "error": str(exc)} + + if not signature_valid: return {"ok": False, "status": 401, "error": "invalid signature"} event_name = header_value(headers, "x-github-event") diff --git a/deploy/coven-github/test_coven_github_adapter.py b/deploy/coven-github/test_coven_github_adapter.py index bde9f03..a466c47 100644 --- a/deploy/coven-github/test_coven_github_adapter.py +++ b/deploy/coven-github/test_coven_github_adapter.py @@ -29,6 +29,18 @@ def test_mentions_are_boundary_aware(self): ): self.assertFalse(adapter.mentioned(text, policy), text) + def test_route_signed_delivery_reports_missing_secret(self): + adapter = load_adapter() + result = adapter.route_signed_delivery( + {"x-hub-signature-256": "sha256=abc"}, + b"{}", + debug=True, + webhook_secret="", + ) + + self.assertEqual(result["status"], 500) + self.assertIn("GITHUB_WEBHOOK_SECRET", result["error"]) + def test_prepare_review_context_rejects_stale_pr_head_evidence(self): adapter = load_adapter() diff --git a/docs/headless-contract.md b/docs/headless-contract.md index a81bf87..3be3dd6 100644 --- a/docs/headless-contract.md +++ b/docs/headless-contract.md @@ -192,9 +192,9 @@ the intended code. It is required on every result. Non-review tasks MUST set | `no_findings_reason` | string \| null | File-backed explanation for a clean review. MAY be `null` for degraded/partial output when `evidence_status` and `limitations` explain why a substantive clean-review conclusion was not possible. | | `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`. +Each finding carries required `severity`, `file`, `line`, `title`, `body`, and +`recommendation` fields. `line` and `recommendation` may be `null`. Valid +severities are `info`, `low`, `medium`, `high`, and `critical`. ### 3.3 `status`