From 8c2c4401945a2d7ae5f4aff464d3b208d5142ba1 Mon Sep 17 00:00:00 2001
From: Alexxigang <37231458+Alexxigang@users.noreply.github.com>
Date: Wed, 20 May 2026 23:13:43 +0800
Subject: [PATCH 1/2] fix(approval): expire stale pending requests
---
src/openhuman/approval/store.rs | 213 +++++++++++++++++++++++++++-----
1 file changed, 179 insertions(+), 34 deletions(-)
diff --git a/src/openhuman/approval/store.rs b/src/openhuman/approval/store.rs
index b0dd5188c1..b19d67943c 100644
--- a/src/openhuman/approval/store.rs
+++ b/src/openhuman/approval/store.rs
@@ -2,18 +2,22 @@
//!
//! Pending rows survive core restart so a queued approval is not lost
//! when the user quits before deciding. Each row carries the
-//! `session_id` of the launch that queued it (informational —
-//! `list_pending` returns every undecided row regardless of session
-//! so the UI can audit / dismiss orphans after restart, per the
-//! issue #1339 acceptance criterion).
+//! `session_id` of the launch that queued it (informational only).
+//! `list_pending` returns every undecided row regardless of session so
+//! the UI can audit or dismiss orphans after restart, per the issue
+//! #1339 acceptance criterion.
//!
//! Replay safety: a `decide` on an orphan row (process that queued it
-//! is gone) updates the DB but cannot resume the parked future — no
-//! side effect can fire across processes. `purge_session` is a
-//! best-effort cleanup helper kept for an explicit RPC in a follow-up.
+//! is gone) updates the DB but cannot resume the parked future, so no
+//! side effect can fire across processes.
+//!
+//! Durability safety: `expires_at` is enforced in the store. When a
+//! pending row has already expired by the time the store is read again
+//! after a restart, it is lazily transitioned into a terminal state so
+//! stale rows stop showing up as actionable approvals forever.
//!
//! Follows the same `with_connection` shape as `notifications/store.rs`
-//! and `cron/store.rs` — synchronous `rusqlite::Connection` opened per
+//! and `cron/store.rs`: synchronous `rusqlite::Connection` opened per
//! call, schema applied idempotently.
use anyhow::{Context, Result};
@@ -24,7 +28,6 @@ use crate::openhuman::config::Config;
use super::types::{ApprovalDecision, PendingApproval};
-/// SQL schema applied on every `with_connection` call.
const SCHEMA: &str = "
PRAGMA foreign_keys = ON;
@@ -45,8 +48,6 @@ CREATE INDEX IF NOT EXISTS idx_pending_approvals_session
ON pending_approvals(session_id);
";
-/// Open (and migrate) the approval DB, then call `f` with a live
-/// connection. Mirrors `notifications/store.rs::with_connection`.
fn with_connection(config: &Config, f: impl FnOnce(&Connection) -> Result) -> Result {
let db_path = config.workspace_dir.join("approval").join("approval.db");
@@ -77,8 +78,6 @@ fn with_connection(config: &Config, f: impl FnOnce(&Connection) -> Result)
f(&conn)
}
-/// Insert a pending row. Caller supplies the `request_id` and
-/// `session_id` so the gate can correlate the parked future.
pub fn insert_pending(config: &Config, pending: &PendingApproval) -> Result<()> {
with_connection(config, |conn| {
let args = serde_json::to_string(&pending.args_redacted)
@@ -105,18 +104,25 @@ pub fn insert_pending(config: &Config, pending: &PendingApproval) -> Result<()>
})
}
-/// List all rows with no `decided_at` (still awaiting user input)
-/// regardless of which launch queued them. Orphan rows (the gate's
-/// in-memory waiter has been dropped — process died between
-/// `intercept` and the user's decision) stay visible so the UI can
-/// audit / dismiss them after restart, satisfying the issue #1339
-/// acceptance criterion "pending rows survive app restart".
+/// Transition any stale rows into a terminal state so they no longer
+/// appear as actionable pending approvals after restart.
///
-/// `decide` on an orphan row updates the DB and returns the row but
-/// the parked tool call is gone — no side effect ever fires, which
-/// matches the security invariant.
+/// We currently reuse `deny` as the persisted terminal value to avoid
+/// widening the externally visible approval decision enum before the
+/// broader durable-audit work lands. This preserves the audit trail
+/// (`decided_at` + `decision`) without leaving expired rows pending
+/// forever.
+pub fn expire_stale(config: &Config) -> Result {
+ with_connection(config, |conn| expire_stale_with_now(conn, Utc::now()))
+}
+
+/// List all rows that are still awaiting user input, regardless of
+/// which launch queued them. Orphan rows from prior sessions remain
+/// visible until they are explicitly decided or expire.
pub fn list_pending(config: &Config) -> Result> {
with_connection(config, |conn| {
+ expire_stale_with_now(conn, Utc::now())?;
+
let mut stmt = conn
.prepare(
"SELECT request_id, tool_name, action_summary, args_redacted,
@@ -138,14 +144,16 @@ pub fn list_pending(config: &Config) -> Result> {
}
/// Mark a pending row as decided and return the now-decided row.
-/// Returns `Ok(None)` if no row matched (already decided, expired,
-/// or unknown id).
+/// Returns `Ok(None)` if no row matched (already decided, expired, or
+/// unknown id).
pub fn decide(
config: &Config,
request_id: &str,
decision: ApprovalDecision,
) -> Result