feat(store): durable claims + restart recovery — the tasks table is the queue (#2)#46
Merged
Merged
Conversation
…t recovery (#2 phases 2+3) Closes #2. The in-process mpsc channel and its silent try-send drop path are gone: the webhook's durable insert IS the enqueue, and workers claim the oldest queued row atomically (UPDATE .. RETURNING) with a Notify wake-up plus a 5s poll backstop. Capacity is held before claiming, so a claimed task is never parked behind a saturated pool. Startup recovery closes orphaned attempts and requeues any 'running' rows left by a dead process, failing tasks whose claim attempts are spent so a crash-looping task cannot poison the queue. The in-memory TaskStore is retired: supersession tombstones live in the store (insert-time for newer reviews, the cancel command via cancel_queued), the Cave /api/github/tasks list and the status command read SQLite (adapter replies filtered out; new 'queued' projection state), and every terminal transition closes its attempt record with redacted detail. Truth pass folded in: README durable-queue row -> Implemented, HOSTED comparison updated, design doc marked implemented. Signed-off-by: Val Alexander <bunsthedev@gmail.com>
Contributor
There was a problem hiding this comment.
Pull request overview
This PR completes phases 2–3 of the durable task store design by making SQLite (tasks table) the durable queue, removing the in-process mpsc dispatch path, and adding startup recovery so accepted webhook work survives restarts. It also retires the in-memory Cave task store, serving /api/github/tasks directly from SQLite and updating docs to reflect the capability as implemented.
Changes:
- Replace the in-process task channel with durable FIFO task claiming (
UPDATE … RETURNING) from SQLite and aNotify-based wake-up + polling backstop. - Add restart recovery that closes open attempts, requeues orphaned
runningtasks, and fails crash-looping tasks once their claim-attempt budget is exhausted. - Move Cave task polling/status projection to SQLite (including a new
queuedstate) and update documentation/status matrices accordingly.
Reviewed changes
Copilot reviewed 10 out of 11 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| README.md | Updates capability table to mark durable queue/task store as implemented and clarifies Cave polling persistence. |
| HOSTED.md | Updates self-hosted vs hosted capability table to reflect built-in durable SQLite queue/state. |
| docs/durable-task-store.md | Marks the durable task store design as fully implemented (phases 1–3). |
| crates/worker/src/lib.rs | Replaces channel receive loop with durable claim_next loop; records terminal outcomes into the store; removes in-memory TaskStore supersession logic. |
| crates/worker/Cargo.toml | Adds dependency on coven-github-store. |
| crates/webhook/src/routes.rs | Removes mpsc enqueue/drop path; records tasks durably and notifies worker; Cave task list served from SQLite; command cancel/status now read/write via store. |
| crates/store/src/lib.rs | Implements durable queue primitives (claim_next, finish, recover_interrupted, cancel_queued, cave_list), adds schema v2 column for projection. |
| crates/server/src/main.rs | Opens store before serving, runs recovery on boot, wires Notify into worker + routes. |
| crates/github/src/tasks.rs | Retires in-memory TaskStore; keeps Cave-facing types + surface_of mapping; adds queued status. |
| crates/github/Cargo.toml | Removes unused chrono dependency. |
| Cargo.lock | Reflects dependency graph updates (remove chrono from api, add store to worker). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+250
to
+260
| store | ||
| .finish( | ||
| &task.id, | ||
| Terminal { | ||
| state: TerminalState::Failed, | ||
| detail: Some(redact::redact(&format!("{e:#}"), &[])), | ||
| ..Terminal::default() | ||
| }, | ||
| ) | ||
| .await | ||
| .ok(); |
Comment on lines
+297
to
+310
| store | ||
| .finish( | ||
| &task.id, | ||
| Terminal { | ||
| state: TerminalState::Superseded, | ||
| summary: Some(format!( | ||
| "head moved {} -> {} mid-review", | ||
| stale.reviewed_sha, stale.current_sha | ||
| )), | ||
| ..Terminal::default() | ||
| }, | ||
| ) | ||
| .await | ||
| .ok(); |
Comment on lines
+359
to
+362
| store | ||
| .finish(&task.id, terminal_of(&published)) | ||
| .await | ||
| .ok(); |
Comment on lines
+404
to
+414
| store | ||
| .finish( | ||
| &task.id, | ||
| Terminal { | ||
| state: TerminalState::Failed, | ||
| detail: Some(redact::redact(&format!("{e:#}"), &[&orchestration])), | ||
| ..Terminal::default() | ||
| }, | ||
| ) | ||
| .await | ||
| .ok(); |
Comment on lines
+479
to
+486
| let cancelled = state | ||
| .store | ||
| .cancel_queued(&format!("{repo}#{}", s.number)) | ||
| .await | ||
| .unwrap_or_else(|e| { | ||
| warn!("cancel could not reach the store: {e:#}"); | ||
| 0 | ||
| }); |
Comment on lines
+223
to
+228
| let conn = conn.lock().expect("store mutex poisoned"); | ||
| conn.execute( | ||
| "UPDATE tasks SET check_run_url = ?1, updated_at = ?2 WHERE id = ?3", | ||
| params![url, now_rfc3339(), task_id], | ||
| )?; | ||
| Ok(()) |
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.
Phases 2+3 of the durable task store (design, phase 1 in #44). Closes #2.
The queue is now the
taskstablempscchannel — and thetask queue full — dropping taskpath that acknowledged work to GitHub and then lost it — is removed. The webhook's durable insert is the enqueue; it just pings atokio::sync::Notify.queuedrow atomically (UPDATE … RETURNINGunder SQLite's writer lock), holding semaphore capacity before claiming so a claimed task is never parked asrunningbehind a saturated pool. A 5s poll backstops missed wake-ups.Restart recovery
At boot, before serving: open attempts are closed as
interrupted,runningorphans requeue, and tasks whose claim attempts exceedworker.max_retries + 1landfailed— a crash-looping task cannot poison the queue. The marker-backed status comment (#13) keeps the user surface deduplicated across restarts.In-memory
TaskStoreretiredcancelcommand now callscancel_queued(counts reported in the reply); in-flight staleness stays with the Resolve Check Run head SHA and target ref correctly #8 re-fetch gate/api/github/tasksand thestatuscommand read SQLite — task history survives restarts (the Cave continuity criterion); adapter replies are filtered out; a newqueuedprojection state is exposedcompleted, decline summary) instead of vanishingAcceptance criteria of #2, all landed
X-GitHub-Deliveryrefusedmissing_delivery_id_is_rejected…restart_recovery_requeues_or_fails_by_attempt_budgetclaims_are_fifo…received/ignoredrouting; tasks:queued/running/completed/failed/supersededVerification
-D warningsclean · python gates green