feat(worker): headless contract + worker resilience (retries, preflight, checks)#27
Merged
Conversation
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).
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_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).
There was a problem hiding this comment.
Pull request overview
This PR consolidates the “headless execution contract” and “worker resilience” work by locking a v1 adapter↔runtime interface (docs + JSON Schemas + golden fixtures + conformance tests) and hardening the worker’s execution/publication lifecycle (preflight ref resolution, retry-safe exit-code handling, and deterministic process tests).
Changes:
- Introduces a locked v1 headless contract (normative doc, JSON Schemas, golden fixtures, and a conformance test).
- Hardens the worker loop with preflight (token + ref resolution + Check Run creation), retry budget/backoff based on exit code semantics, and “always finalize Check Run / always cleanup workspace” behavior.
- Expands webhook support/fixtures (ping handling + PR review submissions + more robust mention routing) and updates docs for self-hosting/security.
Reviewed changes
Copilot reviewed 26 out of 26 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| README.md | Updates capability/status table and self-hosting config instructions to reflect the new contract and default-branch behavior. |
| docs/self-hosting.md | Adds app-manifest-based setup path and documents additional webhook triggers/events. |
| docs/security.md | Updates token handling description to match the tokenless brief + env-injected git token model. |
| docs/headless-contract.md | Adds the normative v1 contract spec for headless execution (inputs, outputs, exit codes, invariants). |
| docs/contracts/session-brief.schema.json | Adds the session brief JSON Schema (tokenless, versioned). |
| docs/contracts/session-brief.example.json | Adds a golden session-brief fixture. |
| docs/contracts/result.schema.json | Adds the result envelope JSON Schema (versioned). |
| docs/contracts/result.example.json | Adds a golden result fixture. |
| docs/app-manifest.json | Adds a prefilled GitHub App manifest for simpler registration. |
| crates/worker/tests/contract.rs | Adds a conformance test that round-trips golden fixtures through Rust wire types. |
| crates/worker/src/lib.rs | Implements worker preflight, ref resolution, retry-safe execution policy, always-finalize Check Runs, and hardened process tests. |
| crates/worker/src/brief.rs | Makes the session brief tokenless, versioned, and default-branch aware; adds token-leak regression tests. |
| crates/webhook/src/routes.rs | Acknowledges ping explicitly; adds boundary-aware mention parsing; routes PR conversation comments to PR iteration. |
| crates/webhook/src/events.rs | Extends webhook parsing for pull_request_review and flags issue_comment events that are actually PR comments. |
| crates/webhook/tests/parse_fixtures.rs | Adds fixture-driven parsing tests for all supported webhook payloads (including ping/reviews). |
| crates/webhook/tests/fixtures/*.json | Adds webhook payload fixtures for parsing coverage. |
| crates/github/src/tasks.rs | Adds register_failed to persist preflight failures; updates tests for contract_version field. |
| crates/github/src/repo.rs | Adds a GitHub metadata client for repo default branch + branch SHA + PR refs/SHAs. |
| crates/github/src/lib.rs | Adds HEADLESS_CONTRACT_VERSION, adds Ping/PullRequestReview events and contract_version on SessionResult, and avoids sending JSON bodies for GETs. |
| COVEN-GITHUB.md | Clarifies that docs/headless-contract.md is normative and corrects/aligns illustrative prose. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+14
to
+16
| The contract is enforced on the `coven-github` side by golden fixtures in | ||
| [`docs/contracts/`](contracts/) and a conformance test | ||
| (`crates/github/tests/contract.rs`) that round-trips those fixtures through the |
Comment on lines
+252
to
+254
| | [`docs/contracts/session-brief.example.json`](contracts/session-brief.example.json) | Golden input fixture. | | ||
| | [`docs/contracts/result.example.json`](contracts/result.example.json) | Golden output fixture. | | ||
| | `crates/github/tests/contract.rs` | Round-trips the golden fixtures through the Rust types — fails the build if the adapter drifts from this contract. | |
Comment on lines
+7
to
+10
| "additionalProperties": false, | ||
| "required": ["trigger", "repo", "task", "familiar", "workspace"], | ||
| "properties": { | ||
| "contract_version": { |
Comment on lines
+15
to
+17
| /// The brief is intentionally tokenless: the agent receives read context only. | ||
| /// Git authentication is injected out-of-band (env / GIT_ASKPASS) and GitHub | ||
| /// write authority (comments, Check Runs, branches, PRs) stays with the adapter |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
Consolidates the headless-contract + worker-resilience work into one landable PR. This is a superset of the
feat/github-correctness-issues-8-9-4branch (shares commitce23755), so landing this delivers both.Layer 1 — headless contract (
ce23755)docs/headless-contract.md,docs/contracts/*.schema.json+ examples,docs/app-manifest.jsoncrates/worker/tests/contract.rs)briefuses the resolved default branch (not hardcodedmain); brief serialization never leaks token/auth fieldsLayer 2 — worker resilience (
8af77c8)Layer 3 — test hardening (
5a0870a)test_confignow defaults to a generous 30s timeout; the dedicated timeout test overrides to 1s inline.Verification (local)
cargo check --workspace→ cleancargo test --workspace→ all green (worker: 14 lib + 1 contract; config 11; webhook 7; api 5)exit_three_needs_input_is_completed_not_retriednow passes deterministically.Consolidation note
Once this lands,
feat/github-correctness-issues-8-9-4is fully subsumed (its commitce23755is an ancestor here) and can be deleted.