Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions config/example.toml
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ max_retries = 2 # Retries for infra errors (exit code 2)
# Durable adapter state: webhook deliveries (idempotency) and task records.
# SQLite file — keep it on a persistent volume; parents are created at start.
path = "data/coven-github.db"
# task_retention_days = 90 # expire terminal task history after N days (issue #12); omit to keep forever

# ── Task API auth (issue #3) ────────────────────────────────────────────────
# Gate GET /api/github/tasks. "open" = unauthenticated (local development
Expand Down
5 changes: 5 additions & 0 deletions crates/config/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -247,12 +247,17 @@ pub struct StorageConfig {
/// SQLite database path; parent directories are created at startup.
#[serde(default = "default_storage_path")]
pub path: PathBuf,
/// Days to retain terminal task history before a periodic sweep deletes it
/// (issue #12). Absent = keep indefinitely. In-flight tasks are never
/// expired.
pub task_retention_days: Option<u32>,
}

impl Default for StorageConfig {
fn default() -> Self {
Self {
path: default_storage_path(),
task_retention_days: None,
}
}
}
Expand Down
20 changes: 20 additions & 0 deletions crates/server/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -132,6 +132,26 @@ async fn main() -> Result<()> {
});
}

// Task-history retention sweep (issue #12): expire terminal tasks
// older than the configured horizon; in-flight work is never touched.
if let Some(retention_days) = config.storage.task_retention_days {
let sweep_store = store.clone();
tokio::spawn(async move {
let mut ticker =
tokio::time::interval(std::time::Duration::from_secs(6 * 3600));
loop {
ticker.tick().await;
match sweep_store.expire_terminal_tasks(retention_days).await {
Ok(0) => {}
Ok(expired) => {
tracing::info!(expired, "expired terminal task rows past retention")
}
Err(e) => tracing::error!("task retention sweep failed: {e:#}"),
}
}
});
}

// Build router.
let state = AppState {
config: config.clone(),
Expand Down
182 changes: 182 additions & 0 deletions crates/store/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -770,6 +770,100 @@ impl Store {
let cutoff = (chrono::Utc::now() - chrono::Duration::days(retention_days as i64)).to_rfc3339();
self.expire_memory_activity_before(&cutoff).await
}

/// Purges all task artifacts for an installation — attempts, tasks, and the
/// delivery records — on uninstall (issue #12). Returns rows removed.
pub async fn delete_tasks_for_installation(&self, installation_id: u64) -> Result<usize> {
let conn = self.conn.clone();
tokio::task::spawn_blocking(move || {
let mut conn = conn.lock().expect("store mutex poisoned");
let tx = conn.transaction()?;
// Defer FK checks to commit so the multi-table cascade (attempts →
// tasks → deliveries) needn't be perfectly ordered; the whole set is
// consistent once committed.
tx.execute_batch("PRAGMA defer_foreign_keys = ON")?;
let attempts = tx.execute(
"DELETE FROM task_attempts
WHERE task_id IN (SELECT id FROM tasks WHERE installation_id = ?1)",
params![installation_id],
)?;
let tasks = tx.execute(
"DELETE FROM tasks WHERE installation_id = ?1",
params![installation_id],
)?;
let deliveries = tx.execute(
"DELETE FROM webhook_deliveries WHERE installation_id = ?1",
params![installation_id],
)?;
Comment on lines +794 to +797
tx.commit()?;
Ok(attempts + tasks + deliveries)
})
.await
.expect("store task panicked")
}

/// Deletes terminal (completed / failed / superseded) tasks and their
/// attempts older than `cutoff` (RFC 3339) — task retention (issue #12).
/// In-flight (`queued` / `running`) tasks are never expired.
pub async fn expire_terminal_tasks_before(&self, cutoff: &str) -> Result<usize> {
let conn = self.conn.clone();
let cutoff = cutoff.to_string();
tokio::task::spawn_blocking(move || {
let mut conn = conn.lock().expect("store mutex poisoned");
let tx = conn.transaction()?;
tx.execute_batch("PRAGMA defer_foreign_keys = ON")?;
let expiring = "SELECT id FROM tasks \
WHERE state IN ('completed','failed','superseded') AND updated_at < ?1";
let attempts = tx.execute(
&format!("DELETE FROM task_attempts WHERE task_id IN ({expiring})"),
params![cutoff],
)?;
let tasks = tx.execute(
"DELETE FROM tasks \
WHERE state IN ('completed','failed','superseded') AND updated_at < ?1",
params![cutoff],
)?;
tx.commit()?;
Ok(attempts + tasks)
})
.await
.expect("store task panicked")
}

/// Expires terminal tasks older than `retention_days` from now.
pub async fn expire_terminal_tasks(&self, retention_days: u32) -> Result<usize> {
let cutoff = (chrono::Utc::now() - chrono::Duration::days(retention_days as i64)).to_rfc3339();
self.expire_terminal_tasks_before(&cutoff).await
}

/// Every adapter-generated free-text value the store durably retains — the
/// scan surface for the redaction guarantee (issue #12): no raw token or
/// secret may survive here. Deliberately excludes user-authored content
/// (issue/comment bodies in `tasks.kind`), which may legitimately quote
/// token-shaped strings and is never the adapter's to redact.
pub async fn all_stored_text(&self) -> Result<Vec<String>> {
let conn = self.conn.clone();
tokio::task::spawn_blocking(move || {
let conn = conn.lock().expect("store mutex poisoned");
let mut out = Vec::new();
for sql in [
"SELECT summary FROM tasks WHERE summary IS NOT NULL",
"SELECT detail FROM task_attempts WHERE detail IS NOT NULL",
"SELECT routing FROM webhook_deliveries",
"SELECT target FROM memory_activity",
"SELECT target FROM memory_revocations",
] {
Comment on lines +849 to +855
let mut stmt = conn.prepare(sql)?;
let rows = stmt
.query_map([], |row| row.get::<_, String>(0))?
.collect::<std::result::Result<Vec<_>, _>>()?;
out.extend(rows);
}
Ok(out)
})
.await
.expect("store task panicked")
}
}

/// One recorded memory operation for the inspect/audit trail (issue #6).
Expand Down Expand Up @@ -1431,6 +1525,94 @@ mod queue_tests {
assert!(store.claim_next(&Default::default()).await.unwrap().is_none());
}

async fn finish_completed(store: &Store, task_id: &str, summary: &str, detail: Option<&str>) {
store
.finish(
task_id,
Terminal {
state: TerminalState::Completed,
result_status: Some("success".to_string()),
branch: None,
pr_number: None,
summary: Some(summary.to_string()),
detail: detail.map(str::to_string),
},
)
.await
.expect("finish");
}

#[tokio::test]
async fn delete_tasks_for_installation_purges_that_tenants_artifacts() {
let store = Store::open_in_memory().expect("open");
enqueue(&store, "d1", &fix_task("t1")).await; // installation 1
// Installation 2: the delivery and task share the installation (as they
// always do from one webhook payload).
let other = Task {
installation_id: 2,
..fix_task("t2")
};
store
.record_delivery(
Delivery {
installation_id: Some(2),
..delivery("d2")
},
Routing::Task(&other),
)
.await
.expect("enqueue");
// Claim + finish t1 so it has an attempt row too.
store.claim_next(&Default::default()).await.unwrap();
finish_completed(&store, "t1", "done", Some("some detail")).await;

let removed = store.delete_tasks_for_installation(1).await.unwrap();
assert!(removed >= 2, "task + delivery (+ attempt) removed: {removed}");

// Installation 1 is gone; installation 2 survives and is still claimable.
assert!(store
.delivery_routing("d1")
.await
.unwrap()
.is_none());
let claimed = store.claim_next(&Default::default()).await.unwrap().expect("t2 survives");
assert_eq!(claimed.id, "t2");
}

#[tokio::test]
async fn task_retention_expires_terminal_tasks_but_not_in_flight() {
let store = Store::open_in_memory().expect("open");
enqueue(&store, "d1", &fix_task("done")).await;
enqueue(&store, "d2", &fix_task("queued")).await;
// Make the first terminal; leave the second queued.
store.claim_next(&Default::default()).await.unwrap();
finish_completed(&store, "done", "s", None).await;

// A cutoff in the future expires the terminal task, never the queued one.
let removed = store
.expire_terminal_tasks_before("2999-01-01T00:00:00+00:00")
.await
.unwrap();
// The terminal task plus its attempt row are removed.
assert_eq!(removed, 2);
let claimed = store.claim_next(&Default::default()).await.unwrap().expect("queued survives");
assert_eq!(claimed.id, "queued");
}

#[tokio::test]
async fn all_stored_text_surfaces_adapter_generated_fields() {
let store = Store::open_in_memory().expect("open");
enqueue(&store, "d1", &fix_task("t1")).await;
store.claim_next(&Default::default()).await.unwrap();
finish_completed(&store, "t1", "SUMMARY_MARKER", Some("DETAIL_MARKER")).await;

let text = store.all_stored_text().await.unwrap();
assert!(text.iter().any(|t| t.contains("SUMMARY_MARKER")));
assert!(text.iter().any(|t| t.contains("DETAIL_MARKER")));
// The delivery routing is adapter-generated and included.
assert!(text.iter().any(|t| t.starts_with("task:")));
}

#[tokio::test]
async fn finish_reaches_terminal_state_and_closes_the_attempt() {
let store = Store::open_in_memory().expect("open");
Expand Down
43 changes: 37 additions & 6 deletions crates/webhook/src/routes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -511,12 +511,36 @@ pub async fn handle_webhook(
{
Ok(Recorded::New) => {
if let Some(id) = installation_id {
match state.store.delete_memory_for_installation(id).await {
Ok(purged) => {
info!(installation_id = id, purged, "purged memory on uninstall")
}
Err(e) => error!("failed to purge memory on uninstall: {e:#}"),
}
// Purge both memory (issue #6) and task artifacts (issue
// #12) for the departing tenant. Idempotent: a redelivery
// finds nothing left to remove.
let memory = state
.store
.delete_memory_for_installation(id)
.await
.unwrap_or_else(|e| {
error!("failed to purge memory on uninstall: {e:#}");
0
});
let tasks = state
.store
.delete_tasks_for_installation(id)
.await
.unwrap_or_else(|e| {
error!("failed to purge task artifacts on uninstall: {e:#}");
0
});
info!(installation_id = id, memory, tasks, "purged tenant data on uninstall");
// Audit what was deleted (issue #12).
let _ = state
.store
.record_api_read(
&format!("installation:{id}"),
&id.to_string(),
"delete_on_uninstall",
&format!("memory:{memory},tasks:{tasks}"),
)
.await;
}
(StatusCode::OK, Json(json!({"ok": true}))).into_response()
}
Expand Down Expand Up @@ -1750,6 +1774,13 @@ mod delivery_idempotency_tests {
.len(),
1
);

// The purge (memory + task artifacts) is audited (issue #12).
let audit = state.store.api_audit_entries().await.unwrap();
assert!(
audit.iter().any(|(_, _, action, _)| action == "delete_on_uninstall"),
"the uninstall purge must leave an audit record: {audit:?}"
);
}

#[tokio::test]
Expand Down
Loading
Loading