Repository navigation
Description
How this surfaced
A codex (GPT-5) cross-model review of #200 / PR #213 (the "surface pending approvals from any surface" work) caught this. #213 fixed the off-Activity cues by re-sourcing them from the uncapped PendingList, but the review exposed a pre-existing daemon-level hazard underneath it that #213 deliberately left in scope for a follow-up. This issue is that follow-up. The brief below comes from a fresh read-only exploration of the daemon + consumers + wire + tests — but the root cause is worth re-examining with fresh eyes, not just patched at the first plausible seam (see Open questions).
Root cause (verify before fixing)
activity_feed() collects all of self.requests, sorts newest-first purely by insertion seq, then .take(ACTIVITY_FEED_CAP) with zero regard for lifecycle:
crates/deckard-signerd/src/daemon.rs:1547-1551—rows.sort_by_key(|(_, req)| Reverse(req.seq)); rows.into_iter().take(ACTIVITY_FEED_CAP)- cap constant
= 200atdaemon.rs:151
A still-Proposed request is by nature old (proposed earlier, not yet acted on → lower seq), so it is exactly the row shed once 200 higher-seq records exist. The cap is a response-size bound, not a correctness bound (daemon.rs:148-150): expire_stale() only flips status in place (daemon.rs:1679-1692), terminal records are never pruned (no remove/retain; only a wholesale clear() on Unlock), so self.requests grows unbounded per session and >200 is genuinely reachable in a long, high-volume agent run.
The authoritative uncapped set already exists — pending_list() maps the full request map with no .take() (daemon.rs:1499-1526). The deeper asymmetry: a value-bearing "waiting on you" count is derived from a lossy, size-bounded LEDGER (ActivityFeed) instead of the uncapped INBOX (PendingList).
Blast radius
On the Activity surface only, two consumers undercount together (both read the capped self.activity):
- the sidebar Activity badge —
Shell::waiting_count(shell.rs:3056, on-Activity branch) →activity_pendingover the capped feed; - the NEEDS YOU triage band (
crates/deckard-app/src/activity_view.rs:343, filter at:259-261).
They hide up to (N − 200) waiting approvals on the very surface built to triage them — the request can lapse unseen at the approval TTL (daemon.rs:44).
Already safe (post-#200): the home waiting strip + meta-rail read the uncapped PendingList background poll (see the documented NOTE at shell.rs:3050-3055). Not affected: the MCP sidecar — activity_lifecycle_label (crates/deckard-mcp/src/sidecar.rs:404-412) maps a single-request StatusView from a per-request Status poll, not a feed-derived count. No agent-facing tool counts feed rows today.
Severity: ~P2. A real wrong-direction trust cue, but the trigger is narrow (>200 records in one session before a Pending expires) and #200 already closed the everywhere-cue.
Solution space (a menu, not a decision — trade-offs to weigh)
A. Lifecycle-aware feed selection (fix at the daemon source). Always include all Proposed rows; apply the 200 cap only to the settled tail. Pros: fixes every feed consumer at the source; keeps band == feed-count coupling; keeps the rich display fields (reason, timestamp_ms). Cons: feed can exceed 200 when many proposals are open (naturally bounded — a human has few open cards); doesn't bound the settled LOG. Wire: additive (daemon-internal).
B. App sources band + badge from the uncapped PendingList on Activity too. Use ActivityFeed only for the settled LOG. Pros: fully closes the undercount from the existing authoritative source; one source of truth for "waiting" matching #200. Cons: two wire calls per Activity refresh; a PendingRecord lacks the feed's display fields (reason: BreachedLimit, timestamp_ms — rpc.rs:246-255 vs 363/369), so the band loses the over-cap reason chip + relative time unless joined by request_id; must rewrite the test that pins the current coupling (badge_matches_feed_on_activity_surface, shell.rs:3656-3661); doesn't protect a future feed-count consumer. Wire: additive (app-only, both requests already exist).
C. Add a feed-level truncation signal (truncated: bool / total_pending: u32). Pros: makes the loss explicit + auditable for any consumer. Cons: the only option that touches the wire, and it's breaking if it wraps the existing reply — Activity(Vec<..>) is a CBOR array; turning it into Activity(struct{rows,total}) is a shape change that breaks old decoders and bumps spec_version (evolution rules #2/#3, docs/build/40-wire-evolution.md). Additive-safe only as a new variant + new capability name. Likely unnecessary if B/A already yield an uncapped count.
D. Prune terminal records and/or raise/remove the cap. Pros: pruning settled rows attacks the root unbounded-growth the cap only masks; raising is a one-line stopgap. Cons: raising only lowers probability (still a cliff); removing serializes the unbounded map on every ~2s poll → frame/memory/render growth over a long session; pruning changes the session-ledger semantics the feed exists to preserve and needs a what/when-to-prune policy.
(A + a settled-only cap, or A + B, may compose better than any single option — worth discussing.)
Test strategy (write the guard at the level the bug lives)
- Primary — daemon-level regression test in
crates/deckard-signerd/src/daemon.rs(closest template: the existing in-process daemon unit tests there). Insert oneProposed/Pendingrecord at a lowseq, then 200+ newer settled records, and assert the old row is still discoverable — onactivity_feed()(fails today) and onpending_list()(passes today, pins the uncapped guarantee). This is the tightest guard: the defect is daemon-level and there is currently no test exercising the cap boundary. Needs only in-process record insertion — no GPUI, no socket. - App-level caveat:
shell.rsmod pending_poll_testscurrently pins the buggy coupling —badge_matches_feed_on_activity_surfaceasserts the on-Activity badge equals the capped feed count. Option B must rewrite it; option A keeps it valid. - Wire (only if C): the E1–E4 round-trip / unknown-variant / golden-byte freeze tests (
docs/build/40-wire-evolution.md) already gate any new field/variant.
Open questions (please reconsider, don't just pick A)
- Should the daemon prune terminal records by age/count so the settled LOG is naturally bounded while
Proposedrows are always kept (A + settled-only cap)? What prune policy? - Is the 200 cap worth keeping at all given
self.requestsalready grows unbounded per session? Is a much higher cap + lifecycle priority the simpler honest fix, or does removing it reintroduce unbounded ~2s-poll render growth? - Confirm no future agent-facing tool will derive a pending/proposed count from
ActivityFeed— if one is planned, the daemon-source fix (A) protects it; the app-only fix (B) does not.
Found via cross-model review of #200 (PR #213). Root-caused by a read-only multi-agent exploration; claims cite file:line but a fresh look at the root cause is explicitly invited before committing to a fix.
Activity
hellno commentedon Jul 8, 2026
Fixed in #217. A few corrections to the framing from implementing it, for the record:
1. It's a reachability failure, not just an undercount. The blast radius is wider than the
badge + band: every
activity_pending(&self.activity)consumer read the capped feed — selection,arrow-nav, inline approve/deny, the clear-signing review card, and the meta-rail. A shed
Pendingrow was absent from the Vec entirely, so it was unselectable and un-approvable, not merely
miscounted. Sharpest symptom: the waiting count drops when you open Activity (Home reads the
uncapped
PendingList→ "3 waiting"; Activity read the capped feed → shows 2; the 3rd wasunreachable).
2. The keep-set is "live
Pending" only.Allowed(approved, awaiting the agent to execute)maps to
Decided{approved:true}— it's not "waiting on you" and is safe to cap.3. We shipped the clean inbox/log split, not the daemon Option-A partition. A single call
returning "complete inbox + capped log" has no honest name. The daemon already had two functions
(
pending_list= inbox,activity_feed= log); the bug was the app reading the log for theinbox. Fix: INBOX ← uncapped
pending_list(); LOG ← cappedactivity_feed()(cap unchanged —capping a log is correct).
PendingRecordgainedreason/timestamp_ms(additive) for displayparity.
Invariant to preserve going forward: never cap the pending subset — any cap on it re-opens this
bug. Verified via codex adversarial review (GOAL MET) + full DoD green. Two follow-ups filed for the
remaining edges (unbounded
self.requestsgrowth; the LOG band's own 200-cap).refactor(activity): rename waiting_count param feed_pending -> on_scr…
fix(activity): #216 · source approval inbox from the uncapped Pending…