[codex] Require structured hosted reviews and repair loop

.gitignore

@@ -4,3 +4,6 @@ keys/
 *.pem
 .env
 .DS_Store
+deploy/coven-github/.coven-github-private-key.pem
+deploy/coven-github/coven-github-policy.json
+deploy/coven-github/coven-github-state/

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<String>,
     pub summary: String,
     pub pr_body: String,
+    pub review: ReviewResult,
     pub exit_reason: Option<ExitReason>,
 }
 
@@ -205,6 +206,86 @@ 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<String>,
+    pub supporting_files: Vec<String>,
+    pub findings: Vec<ReviewFinding>,
+    pub tests_run: Vec<ReviewTestRun>,
+    pub no_findings_reason: Option<String>,
+    pub limitations: Vec<String>,
+}
+
+impl ReviewResult {
+    pub fn none() -> Self {
+        Self {
+            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,
+            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<u64>,
+    pub title: String,
+    pub body: String,
+    pub recommendation: Option<String>,
+}
+
+#[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<String>,
+}
+
+#[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 {

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),

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_json::Value>,
+    #[serde(default, skip_serializing_if = "Option::is_none")]
+    pub audit_instruction: Option<String>,
 }
 
 #[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,
     }
 }
 

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};
 
@@ -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<SessionResult>),
     /// 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}"
             )),
@@ -506,9 +506,31 @@ async fn read_result(result_path: &Path) -> Result<SessionResult> {
         .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 {}",
+            result.contract_version,
+            coven_github_api::HEADLESS_CONTRACT_VERSION
+        );
+    }
+    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 {
     config
         .server
@@ -622,10 +644,62 @@ fn task_issue_number(kind: &TaskKind) -> Option<u64> {
     }
 }
 
+#[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);
+    }
+
+    #[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)]
 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 +717,7 @@ mod disposition_tests {
             files_changed: vec![],
             summary: "summary".to_string(),
             pr_body: "body".to_string(),
+            review: ReviewResult::none(),
             exit_reason: None,
         }
     }
@@ -828,7 +903,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":[],"supporting_files":[],"findings":[],"tests_run":[],"no_findings_reason":null,"limitations":[]}},"exit_reason":{exit_reason}}}"#
         )
     }
 

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 {
@@ -35,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");
@@ -43,6 +97,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 +122,16 @@ 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": [],
+            "supporting_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");