Skip to content

Commit b2c52e6

Browse files
authored
feat(worker): headless contract + worker resilience (retries, preflight, checks) (#27)
* Introduce headless contract, webhook parsing, and repo client Add a locked headless execution contract (v1) with doc, JSON schemas, and golden fixtures. Make session briefs tokenless and enforce COVEN_GIT_TOKEN as the git-auth channel. Add HEADLESS_CONTRACT_VERSION, contract_version fields to brief/result types, and conformance tests. Implement repo metadata client to resolve branch/PR SHAs and wire it into worker task execution (CheckRun target, brief default_branch, PR base ref). Enhance webhook parsing/routing: PR review events, PR comment detection, ping handling, and robust mention matching. Several tests and fixtures added; update docs (security, self-hosting, README). * Harden worker preflight, retries, and checks Refactors worker task execution so failures before check creation are still recorded in TaskStore, and so check runs are always finalized while workspace cleanup always happens. Moves publish/comment/check-progress steps to best-effort behavior, adds status-based disposition mapping (including neutral/action_required), and only opens PRs when appropriate. Reworks coven-code execution into contract-aware attempt types so only retry-safe failures (exit 2, timeout, signal, spawn/read issues) are retried, while exit 1 and exit 3 are terminal. Adds targeted tests for failed-task registration, disposition mapping, and retry/exit-code behavior, plus small README/COVEN formatting fixes. * test(worker): fix flaky exit-code tests racing the kill timer test_config hardcoded timeout_secs=1 for every caller, so the exit-code tests (0/1/2/3) raced the kill timer under load — the exit-3 needs-input test intermittently saw RetrySafe(timeout) instead of the terminal NeedsInput disposition. Default test_config to a generous 30s timeout so exit-code tests never race the kill path; the dedicated timeout test overrides it back to 1s inline (it exercises sleep 5 and asserts elapsed < 3s).
1 parent 9d34745 commit b2c52e6

26 files changed

Lines changed: 2205 additions & 206 deletions

COVEN-GITHUB.md

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -116,6 +116,17 @@ The same lifecycle in operational prose:
116116

117117
## coven-code Delta: Headless Mode
118118

119+
> **The normative, locked contract lives in [`docs/headless-contract.md`](docs/headless-contract.md)
120+
> (contract version `1`).** That document is the single source of truth, with
121+
> machine-checkable JSON Schemas and golden fixtures in [`docs/contracts/`](docs/contracts/)
122+
> and a conformance test (`crates/worker/tests/contract.rs`). The summary below
123+
> is illustrative; where it disagrees with the contract, the contract wins. In
124+
> particular the contract supersedes this section on three points: the session
125+
> brief is **tokenless** (git auth flows via the `COVEN_GIT_TOKEN` env var, not
126+
> `GIT_ASKPASS`/an embedded `auth.token`), the brief carries a `task` tagged
127+
> union (not an `issue` object), and the v1 `result.json` envelope does **not**
128+
> include an `events` array (progress streaming is deferred to M2).
129+
119130
The execution runtime needs the following additions to work as a GitHub App backend:
120131

121132
### `--headless` flag
@@ -205,7 +216,8 @@ The PR body and issue comments are written in the familiar's voice:
205216
I looked at issue #42 and here's what I found:
206217

207218
The OAuth token refresh path in `src/auth/refresh.rs` wasn't accounting for
208-
clock skew between the client and the auth server. I added a 60-second buffer\nto the expiry check.
219+
clock skew between the client and the auth server. I added a 60-second buffer
220+
to the expiry check.
209221

210222
**Changed:** `src/auth/refresh.rs` (+12 / -3)
211223
**Tests:** 8/8 passing (added 2 regression cases)

README.md

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -112,9 +112,10 @@ For deeper system, sequence, state, security-boundary, and hosted deployment dia
112112
| Label trigger | Implemented | Routes configured `trigger_labels` such as `coven:fix`. |
113113
| Issue / PR mention trigger | Implemented | Ignores familiar bot self-comments to avoid loops. |
114114
| GitHub App installation tokens | Implemented | Mints installation access tokens from the App private key. |
115-
| Check Run creation and completion | Partial | Creates and updates Check Runs; branch/SHA resolution still needs production hardening. |
116-
| `coven-code --headless` execution | Partial | Worker spawns headless sessions and enforces task timeouts; result quality depends on the runtime. |
117-
| Pull request creation | Partial | Opens draft PRs from session results; base branch is still hardcoded to `main`. |
115+
| Check Run creation and completion | Partial | Creates and updates Check Runs against the resolved target head SHA; stale-ref revalidation before publish is still planned. |
116+
| Headless execution contract | Locked (v1) | Brief, result envelope, exit codes, and git-auth channel are pinned in [`docs/headless-contract.md`](docs/headless-contract.md) with JSON Schemas, golden fixtures, and a conformance test. |
117+
| `coven-code --headless` execution | Partial | Worker spawns headless sessions with a tokenless session brief and enforces task timeouts; result quality depends on the runtime. |
118+
| Pull request creation | Partial | Opens draft PRs from session results against the repository's resolved default/base branch. |
118119
| CovenCave task polling | Partial | In-memory task API exists for local oversight; hosted control-plane auth and persistence are planned. |
119120
| Durable queue / task store | Planned | Required for hosted reliability and restarts. |
120121
| Hosted tier | Planned | See [Hosted vs self-hosted](docs/hosted-vs-self-hosted.md). |
@@ -131,7 +132,9 @@ cd coven-github
131132
cargo build --release
132133

133134
# Configure (see config/example.toml)
134-
cp config/example.toml config/local.toml\n# Set: github_app_id, private_key_path, webhook_secret, familiar config
135+
cp config/example.toml config/local.toml
136+
# Then set in config/local.toml: github.app_id, github.private_key_path,
137+
# github.webhook_secret, worker.coven_code_bin, and a [[familiars]] block.
135138

136139
# Run
137140
./target/release/coven-github serve --config config/local.toml

crates/github/src/lib.rs

Lines changed: 45 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -5,9 +5,18 @@ use serde::{Deserialize, Serialize};
55
pub mod check_run;
66
pub mod installation;
77
pub mod pr;
8+
pub mod repo;
89
pub mod tasks;
910

1011
pub const DEFAULT_API_BASE_URL: &str = "https://api.github.com";
12+
13+
/// Major version of the coven-code headless execution contract this adapter
14+
/// speaks. See `docs/headless-contract.md`. Bump only on breaking changes.
15+
pub const HEADLESS_CONTRACT_VERSION: &str = "1";
16+
17+
fn default_contract_version() -> String {
18+
HEADLESS_CONTRACT_VERSION.to_string()
19+
}
1120
const GITHUB_API_VERSION: &str = "2026-03-10";
1221

1322
#[derive(Debug, Clone, PartialEq, Eq)]
@@ -35,14 +44,16 @@ async fn send_json(
3544
request: GitHubRequest,
3645
) -> anyhow::Result<reqwest::Response> {
3746
let method = reqwest::Method::from_bytes(request.method.as_bytes())?;
38-
let response = client
47+
let mut builder = client
3948
.request(method, api_url(base_url, &request.path))
4049
.bearer_auth(token)
4150
.header("Accept", "application/vnd.github+json")
42-
.header("X-GitHub-Api-Version", GITHUB_API_VERSION)
43-
.json(&request.body)
44-
.send()
45-
.await?;
51+
.header("X-GitHub-Api-Version", GITHUB_API_VERSION);
52+
// GET/metadata requests carry no body; only attach JSON for mutations.
53+
if !request.body.is_null() {
54+
builder = builder.json(&request.body);
55+
}
56+
let response = builder.send().await?;
4657

4758
if !response.status().is_success() {
4859
let status = response.status();
@@ -60,7 +71,10 @@ pub enum GitHubEvent {
6071
IssueAssigned(IssueAssignedEvent),
6172
IssueLabeled(IssueLabeledEvent),
6273
IssueComment(IssueCommentEvent),
74+
PullRequestReview(PrReviewEvent),
6375
PullRequestReviewComment(PrReviewCommentEvent),
76+
/// `ping` delivery GitHub sends when a webhook is first configured.
77+
Ping,
6478
Unsupported { name: String },
6579
}
6680

@@ -94,6 +108,27 @@ pub struct IssueCommentEvent {
94108
pub issue_number: u64,
95109
pub comment_body: String,
96110
pub commenter_login: String,
111+
/// `issue_comment` fires for pull-request conversation comments as well as
112+
/// issue comments. GitHub flags the former with an `issue.pull_request`
113+
/// object; this lets routing send PR comments through PR iteration rather
114+
/// than issue-mention handling.
115+
pub on_pull_request: bool,
116+
}
117+
118+
/// Top-level pull request review submission (`pull_request_review` → `submitted`).
119+
///
120+
/// Distinct from [`PrReviewCommentEvent`], which is a single inline comment on a
121+
/// diff hunk. This carries the review summary body and verdict (state).
122+
#[derive(Debug, Clone, Deserialize, Serialize)]
123+
pub struct PrReviewEvent {
124+
pub installation_id: u64,
125+
pub repo_owner: String,
126+
pub repo_name: String,
127+
pub pr_number: u64,
128+
pub review_body: String,
129+
/// Review verdict: `approved`, `changes_requested`, or `commented`.
130+
pub review_state: String,
131+
pub reviewer_login: String,
97132
}
98133

99134
#[derive(Debug, Clone, Deserialize, Serialize)]
@@ -139,6 +174,11 @@ pub enum TaskKind {
139174
/// Structured result envelope written by coven-code --headless.
140175
#[derive(Debug, Clone, Deserialize, Serialize)]
141176
pub struct SessionResult {
177+
/// Contract major version. Defaults to the current version when a runtime
178+
/// omits it, but conformant producers MUST emit it. See
179+
/// `docs/headless-contract.md`.
180+
#[serde(default = "default_contract_version")]
181+
pub contract_version: String,
142182
pub status: SessionStatus,
143183
pub branch: Option<String>,
144184
pub commits: Vec<CommitInfo>,

crates/github/src/repo.rs

Lines changed: 225 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,225 @@
1+
//! GitHub repository, branch, and pull request metadata client.
2+
//!
3+
//! Used to resolve the target refs a task operates on instead of relying on
4+
//! placeholders like `"HEAD"` or a hardcoded `"main"` base branch.
5+
6+
use anyhow::Result;
7+
use serde::Deserialize;
8+
9+
use crate::{client, send_json, GitHubRequest, DEFAULT_API_BASE_URL};
10+
11+
/// Repository metadata we care about for routing and publication.
12+
#[derive(Debug, Clone, PartialEq, Eq, Deserialize)]
13+
pub struct RepoMetadata {
14+
pub default_branch: String,
15+
}
16+
17+
/// Pull request refs needed to attach checks and open/update PRs correctly.
18+
#[derive(Debug, Clone, PartialEq, Eq)]
19+
pub struct PullRequestRefs {
20+
pub head_ref: String,
21+
pub head_sha: String,
22+
pub base_ref: String,
23+
pub base_sha: String,
24+
}
25+
26+
#[derive(Debug, Deserialize)]
27+
struct BranchResponse {
28+
commit: CommitRef,
29+
}
30+
31+
#[derive(Debug, Deserialize)]
32+
struct CommitRef {
33+
sha: String,
34+
}
35+
36+
#[derive(Debug, Deserialize)]
37+
struct PullRequestMetaResponse {
38+
head: PrRef,
39+
base: PrRef,
40+
}
41+
42+
#[derive(Debug, Deserialize)]
43+
struct PrRef {
44+
#[serde(rename = "ref")]
45+
ref_name: String,
46+
sha: String,
47+
}
48+
49+
/// Fetches repository metadata (default branch, etc.).
50+
pub async fn get_repo(installation_token: &str, owner: &str, name: &str) -> Result<RepoMetadata> {
51+
get_repo_with_base_url(DEFAULT_API_BASE_URL, installation_token, owner, name).await
52+
}
53+
54+
pub async fn get_repo_with_base_url(
55+
api_base_url: &str,
56+
installation_token: &str,
57+
owner: &str,
58+
name: &str,
59+
) -> Result<RepoMetadata> {
60+
let client = client()?;
61+
let response = send_json(
62+
&client,
63+
api_base_url,
64+
installation_token,
65+
get_repo_request(owner, name),
66+
)
67+
.await?;
68+
Ok(response.json().await?)
69+
}
70+
71+
/// Resolves the current commit SHA at the tip of a branch.
72+
pub async fn get_branch_sha(
73+
installation_token: &str,
74+
owner: &str,
75+
name: &str,
76+
branch: &str,
77+
) -> Result<String> {
78+
get_branch_sha_with_base_url(
79+
DEFAULT_API_BASE_URL,
80+
installation_token,
81+
owner,
82+
name,
83+
branch,
84+
)
85+
.await
86+
}
87+
88+
pub async fn get_branch_sha_with_base_url(
89+
api_base_url: &str,
90+
installation_token: &str,
91+
owner: &str,
92+
name: &str,
93+
branch: &str,
94+
) -> Result<String> {
95+
let client = client()?;
96+
let response = send_json(
97+
&client,
98+
api_base_url,
99+
installation_token,
100+
get_branch_request(owner, name, branch),
101+
)
102+
.await?;
103+
let body: BranchResponse = response.json().await?;
104+
Ok(body.commit.sha)
105+
}
106+
107+
/// Fetches the head/base refs and SHAs for a pull request.
108+
pub async fn get_pull_request_refs(
109+
installation_token: &str,
110+
owner: &str,
111+
name: &str,
112+
pr_number: u64,
113+
) -> Result<PullRequestRefs> {
114+
get_pull_request_refs_with_base_url(
115+
DEFAULT_API_BASE_URL,
116+
installation_token,
117+
owner,
118+
name,
119+
pr_number,
120+
)
121+
.await
122+
}
123+
124+
pub async fn get_pull_request_refs_with_base_url(
125+
api_base_url: &str,
126+
installation_token: &str,
127+
owner: &str,
128+
name: &str,
129+
pr_number: u64,
130+
) -> Result<PullRequestRefs> {
131+
let client = client()?;
132+
let response = send_json(
133+
&client,
134+
api_base_url,
135+
installation_token,
136+
get_pull_request_request(owner, name, pr_number),
137+
)
138+
.await?;
139+
let body: PullRequestMetaResponse = response.json().await?;
140+
Ok(PullRequestRefs {
141+
head_ref: body.head.ref_name,
142+
head_sha: body.head.sha,
143+
base_ref: body.base.ref_name,
144+
base_sha: body.base.sha,
145+
})
146+
}
147+
148+
fn get_repo_request(owner: &str, name: &str) -> GitHubRequest {
149+
GitHubRequest {
150+
method: "GET",
151+
path: format!("/repos/{owner}/{name}"),
152+
body: serde_json::Value::Null,
153+
}
154+
}
155+
156+
fn get_branch_request(owner: &str, name: &str, branch: &str) -> GitHubRequest {
157+
GitHubRequest {
158+
method: "GET",
159+
path: format!("/repos/{owner}/{name}/branches/{branch}"),
160+
body: serde_json::Value::Null,
161+
}
162+
}
163+
164+
fn get_pull_request_request(owner: &str, name: &str, pr_number: u64) -> GitHubRequest {
165+
GitHubRequest {
166+
method: "GET",
167+
path: format!("/repos/{owner}/{name}/pulls/{pr_number}"),
168+
body: serde_json::Value::Null,
169+
}
170+
}
171+
172+
#[cfg(test)]
173+
mod tests {
174+
use super::*;
175+
use serde_json::json;
176+
177+
#[test]
178+
fn get_repo_request_targets_repo_endpoint() {
179+
let request = get_repo_request("octo", "repo");
180+
assert_eq!(request.method, "GET");
181+
assert_eq!(request.path, "/repos/octo/repo");
182+
}
183+
184+
#[test]
185+
fn get_branch_request_targets_branch_endpoint() {
186+
let request = get_branch_request("octo", "repo", "develop");
187+
assert_eq!(request.method, "GET");
188+
assert_eq!(request.path, "/repos/octo/repo/branches/develop");
189+
}
190+
191+
#[test]
192+
fn get_pull_request_request_targets_pulls_endpoint() {
193+
let request = get_pull_request_request("octo", "repo", 7);
194+
assert_eq!(request.method, "GET");
195+
assert_eq!(request.path, "/repos/octo/repo/pulls/7");
196+
}
197+
198+
#[test]
199+
fn repo_metadata_deserializes_default_branch() {
200+
let meta: RepoMetadata =
201+
serde_json::from_value(json!({ "default_branch": "master", "id": 99 })).unwrap();
202+
assert_eq!(meta.default_branch, "master");
203+
}
204+
205+
#[test]
206+
fn branch_response_extracts_commit_sha() {
207+
let body: BranchResponse =
208+
serde_json::from_value(json!({ "name": "main", "commit": { "sha": "abc123" } }))
209+
.unwrap();
210+
assert_eq!(body.commit.sha, "abc123");
211+
}
212+
213+
#[test]
214+
fn pull_request_meta_extracts_head_and_base_refs() {
215+
let body: PullRequestMetaResponse = serde_json::from_value(json!({
216+
"head": { "ref": "feature", "sha": "headsha" },
217+
"base": { "ref": "develop", "sha": "basesha" }
218+
}))
219+
.unwrap();
220+
assert_eq!(body.head.ref_name, "feature");
221+
assert_eq!(body.head.sha, "headsha");
222+
assert_eq!(body.base.ref_name, "develop");
223+
assert_eq!(body.base.sha, "basesha");
224+
}
225+
}

0 commit comments

Comments
 (0)