Expandable approval detail on notification cards

Status summary#

Implemented, twice over: first the expandable approval detail on notification rows (shared components, spec, video), then — per Misha's review verdict — the standalone Approvals screen was RETIRED: decide actions and the approver-key banner moved to the Notifications surface, approvals-destination pushes route there with the matching row pre-expanded, and both mobile specs run through the new surface (3x consecutive passes each). A bugbot round then closed the device-stream hole: open batches with no row on this device now ride in from the project root stream as synthetic "Needs approval" rows (see the root-stream union section below). Remaining: final gates + CI.

Ask (Misha, 2026-08-01)#

The Notifications view's approval rows are thin — a status line and a deep link into the chat, which no longer shows the approval once it's decided (the in-thread dialog renders open batches only). The rich rendering of a historical approval — the requests, the rule, the verdict/reason, the thread's status at the time, "the script + status at the time + whatever else" — should be reachable from the notification: make the approval notification card expandable to show that detail inline, rather than being only a deep link.

Design#

• Approval-born notification rows already carry approvalRequestEventOffset (top-level on the intent since #2371). Tapping an approval row toggles an expanded state (collapsed by default) instead of navigating; the deep link moves to an explicit affordance inside the expanded area ("Open thread"). Non-approval notifications keep their current tap-to-navigate behavior.
• The expanded body reuses the approvals screen's history rendering — the batch card content from approvals.tsx (requests with method+host, rule key/description, per-index verdicts, rejection reason, decidedBy, and the #2372 thread-context line with its status fold). Extract the shared pieces into reusable components rather than duplicating markup — the approvals screen and the notification expansion must not drift.
• Data: the expansion fetches the batch's events from the project root stream by offset (requested + decided + settled), one-shot react-query keyed on the offset, staleTime: Infinity (decided history is immutable; an undecided batch's row can refetch like the thread-context line does — reuse that settledness-gating pattern if it applies).
• Expansion state: no useState — same toggle pattern the activity card uses (check how ActivityCard handles expanded/collapsed and follow it; if it uses useState internally, match the existing precedent rather than inventing a new one).
• The script itself: the thread-context line covers "status at the time"; if the batch's streamContext carries the executionId, showing the originating script's code (fetched from the agent stream's script-run-requested event, rendered with the existing CodeBlock) is in scope if it stays simple — it is the "script" part of the ask.

Checklist#

• Shared approval-detail component extracted from the approvals screen (no drift between the two surfaces) components/approval-batch.tsx: ApprovalBatchBody (requests/script/ policy + sub-toggles + one-shot script fetch) and ThreadContextLine, moved verbatim from approvals.tsx; BatchCard now composes them and keeps only headline/actions/targeting. The in-body link reads "Open thread" (was "Show thread") on both surfaces
• Approval notification rows expand inline: requests, rule, verdicts, reason, thread-context line, originating script code; "Open thread" affordance inside NotificationRow (ActivityCard's useState toggle precedent, chevron) + ApprovalNotificationDetail in notifications.tsx: one-shot query by batch offset against the root stream, cached forever once decided AND settled (deriveBatchDetail's complete flag), 5s provisional refetch otherwise — the thread-context line's settledness gating. Non-thread batches get an "Open in Approvals" link instead of the body's Open thread
• Non-approval rows unchanged rows without a batch identity keep tap-to-navigate; the identity itself is new on DeviceNotificationRow (top-level intent field with a legacy approvals-destination fallback)
• Unit tests for any new pure derivation; existing suites green deriveBatchDetail cases in approvals.test.ts (undecided/all-reject/ partial-settle/unknown offset); row batch-identity cases in notifications.test.ts
• specs/mobile/notifications.spec.ts extended: expand a row, assert the detail renders (requests + verdict + status line) the spec now rejects the elsewhere batch admin-side (with a reason) and seeds an agent status before the run, then expands the "Send failed" row and asserts URL + "Rejected because: …" + the status line, and drives "Open thread" from inside the expansion; 3 consecutive local passes
• pnpm typecheck && pnpm lint && pnpm knip && pnpm test; PR hygiene all four green from the worktree root after the spinner-disable removal round (2026-08-02); PR body update stays with the parent

Out of scope#

• Web dashboard notifications surface
• Re-showing decided approvals in the chat thread (separate design question)

Implementation log#

• ActivityCard's expansion precedent is a plain useState toggle (the task file anticipated this) — NotificationRow follows it exactly; the members/script sub-toggles keep BatchCard's query-cache pattern, moved with them into ApprovalBatchBody.
• CodeBlock untouched (mid-rework in #2379); the shared body imports whatever main has.
• Renamed the shared in-body link "Show thread" → "Open thread" so the notifications requirement and the approvals screen share one affordance; nothing asserted on the old label.
• Spec gotcha: after navigating via "Open thread", expo-router keeps the Notifications screen mounted-but-hidden and its thread-context line contains the thread name — the chat-arrival assertion targets the header HEADING role, not bare text.
• The local dev server needed two pnpm dev restart --detach cycles during spec iteration (known miniflare degradation; one mid-series failure was exactly that, not the spec).

Removal scope (Misha: "we should just get rid of the approvals view")#

• Decide actions for open batches inside the notification expansion shared ApprovalBatchActions in components/approval-batch.tsx — the retired screen's respond mutation verbatim (approve-all signs via signWithApproverKey, reject-all prompts a reason, never signs); rendered when resolved === null and unexpired; onDecided invalidates the batch query so the record replaces the buttons
• Approver key UI on the Notifications screen components/approver-key-banner.tsx (enroll / re-enroll / signing-as, moved verbatim), mounted above the list; shares the approver-key-status query cache with the actions
• Routing: approvals-destination pushes → Notifications view lib/notification-routing.ts routes to /project/[projectId]/notifications with approvalRequestEventOffset; the screen pre-expands the matching row (ActivityCard's toggled-or-default pattern). CHOICE: no scroll-to-row — newest-first list puts a fresh push's row at/near the top; noted as acceptable. The expansion's "Open in Approvals" link removed (it would self-navigate) — scope batches show their source line, nothing more
• Removal: approvals.tsx + drawer entry deleted; focusOpenBatch (+ its test) pruned as screen-only; knip clean
• approvals.spec.ts final act reworked through the Notifications view spec now enrolls the browser's device identity (rows need a device stream; both batches are claimed in-thread so the fake token is never dialed), expands both rows, keeps the #2372 full-text thread-context assertions and the tap-through verbatim
• notifications.spec.ts decides from the expansion replaces the admin-side reject: "Awaiting decision" + Reject via the window.prompt stand-in (retried press, button-departure success signal), then asserts the historical record

Removal-round log:

• decide() signature compatibility confirmed: it takes the requested event's payload, which deriveBatchDetail already returns verbatim (both deriveOpenBatches and the by-offset fetch carry requested.payload untransformed) — no reconciliation needed beyond an expired guard on showing the actions (deriveOpenBatches used to filter expired batches; the expansion checks payload.expiresAt instead).
• What resisted: the LOCAL dev server (miniflare host) dies with a V8 heap OOM after ~2-3 consecutive spec runs — every mid-series spec failure in this round was that (health 000, OOM in dev-server.log), not spec logic. Cure: pnpm dev restart --detach before each run; the 3x series for both specs was recorded with fresh servers. Worth a separate look at the dev server's memory ceiling.

Tabbed activity cards (Misha, comment 5158038879)#

• "Ran code" cards show the run's approvals in context via the SAME shared component each code step is now Script | Approvals | Result tabs (activity-card.tsx CodeStepTabs); the Approvals tab renders decision badge + reason + the shared ApprovalBatchBody, matched by streamContext.executionId against the step's executionId (deriveBatchesForExecution in lib/approvals.ts, unit-tested)
• Approvals tab absent with no batches; Result absent while pending tabs array is conditional; Result appears once the run settled with a value or an error; single-tab steps render no tab row at all
• Multi-turn runs organize vertically as Round 1 / Round 2 pure groupActivityRounds in lib/feed.ts (llm-writes-script + code-runs-it pairs; stray steps get their own round), labels only when >1 round. WORD CHOICE: kept "Round" — "Turn" is overloaded with chat turns and "Pass"/"Attempt" mislead (not retries; the script passes itself a value)
• Collapsed status glyphs spinner while running (unchanged) + approval marks in the summary row: ◷ any batch awaiting decision, ✓ fully approved batch(es), ✗ rejected or mixed (summarizeBatchOutcomes, unit-tested)
• Batch plumbing CHOICE: prop-drilled from chat.tsx (FeedList → FeedItem → ActivityCard as ActivityApprovalContext) — the chat screen already holds the live root-stream approval subscription for its in-thread dialogs, so the tabs and glyphs update live with zero extra fetches; a per-card query would duplicate already-subscribed data and need the same context props anyway. ActivityCard's only usage is chat.tsx
• approvals.spec.ts extended: finds both settled cards by glyph (✓ / ✗), opens the approved card's Approvals tab, asserts badge + policy through the shared body; 3x consecutive passes, and notifications.spec.ts re-verified 3x (fresh dev server per run — the miniflare heap OOM again)
• VIDEO_MODE re-record of approvals spec → activity-tabs.webm (32s: live card with Script|Approvals tabs + ◷, settled cards with ✓/✗, Approvals tab opened)
• Expired-undecided batches stop reading as awaiting (bugbot medium, comment 3702066150) deriveBatchesForExecution takes nowMs (threaded, stays pure) and flags expired = undecided AND past expiresAt — the same wall-clock gate the decide actions and deriveOpenBatches use. GLYPH CHOICE: expired-undecided joins the ✗/rejected bucket in summarizeBatchOutcomes, because that is exactly where the door's imminent expiry decision (all-reject, decidedBy "expiry") will land it — the glyph never flips when the event arrives, and ◷ never advertises a hold nobody can answer. COPY CHOICE: the Approvals tab shows a plain muted "Expired" note in place of "Awaiting decision" — NOT the resolved badge, which stays reserved for the real expiry decision that upgrades it shortly after. Unit-tested (before/after horizon, and count stability across the expiry event landing); approvals spec re-run green

Spinner-waiter disables: measured, then removed#

(Misha: "See if the flickery spinner issue is actually an issue before making the spinner stitch together gaps.")

Verdict: the "frame gap" the scoped disables guarded against was NOT real. Every disable is gone from both mobile specs. What the measurements showed:

• Baseline: all ~17 spinnerWaiter.settings.run({ disabled: true }) blocks removed, 5+ runs per spec, fresh dev server per run.
• approvals.spec.ts: 6 pass / 2 fail in 8 runs. BOTH failures were the same site — decideBatch's waitFor({ state: "detached" }) — and the trace proved the product was fine: the press landed, the confirm dialog fired and was accepted, the decision appended, the burst ran (200s), the card left the thread. The spec still failed because spinner-waiter's readiness heuristic (visible && enabled) is inverted for DISAPPEARANCE waits: a gone-or-renamed button reads as "target missing, no spinner" and the wait gets the 1ms fast-fail rewrite — and timeout: 1 aborts before the first evaluation can report "already detached", so an ALREADY-SATISFIED wait hard-fails. Middlewright bug, not a product gap.
• notifications.spec.ts: 0 pass / 4 real runs (plus one dev-server OOM), all at the same line — lane two's getByText("Send failed").waitFor. Genuinely no spinner AND no target: the wait starts the moment the elsewhere-thread's script fires, and for several seconds the pipeline (script start → egress hold → 2s debounce → grace window → Expo round trip) has NO on-screen counterpart on this device — like a phone idling before a push arrives. Not a frame gap; a structural external-event wait that belongs on the protocol.
• Every other formerly-disabled site (route transitions, batch-card mounts, row expansions, glyph/tab assertions, deep links) passed repeatedly with the middleware fully on — covered by real product spinners (working row, activity card, screen/detail ActivityIndicators) plus the 100ms handoff bridge.

Fixes (no disables anywhere):

• patches/middlewright.patch extended: spinner-waiter is now goal-aware — waitFor({ state: "detached" | "hidden" }) inverts the readiness check (target no longer visible = success-adjacent), so satisfied disappearance waits proceed with their original timeout instead of the 1ms fast-fail. Candidate upstream middlewright change.
• Product (apps/mobile/src/lib/notifications.ts): non-terminal row statuses are now honest in-flight indicators — "Waiting to send…" / "Sending…" (the chat working-row's anythinging… convention). The push obligation IS server work in flight; the row now shows it as such (and spinner-waiter recognizes it, keeping the UI wait covered from row appearance to settlement).
• Spec (notifications.spec.ts lane two): the external-event window is waited out on the PROTOCOL (expect.poll for the device stream's second notification-settled event), then the UI assertion rides the row's own in-flight status flip.
• Timeouts tightened now that waits are honest: UI-local assertions on already-present data 30s/15s → 5s; cross-server waits (screen-mount fetches, expansion one-shot queries, chat mounts) → 15s; cold multi-hop waits (first project open, batch coalescing) keep 30s.

Root-stream union: orphan batches (bugbot HIGH, 2026-08-03)#

Bugbot's gap was real: the retired Approvals screen derived open batches from the project ROOT stream (device-independent), while the new list derives from the DEVICE stream — so an open batch that never journaled a notification on this device (parked before enrollment, notifications denied, any delivery gap) had no decision surface anywhere and would sit until expiry.

• Notifications screen additionally subscribes to the root stream's approval events the chat screen's exact subscription: shared APPROVAL_STREAM_EVENT_TYPES constant now exported from lib/approvals.ts (module-level for identity-stable connection deps — chat.tsx's lesson; chat.tsx now imports it too), same ["approval-events", …] query key so the two screens share one cache
• Open batches with no matching device row render as synthetic "Needs approval" rows pure deriveNotificationListRows(deviceRows, rootApprovalEvents) in lib/notifications.ts: deriveOpenBatches output deduped by batch offset against device rows' approvalRequestEventOffset (device rows stay authoritative), synthetic rows sorted ABOVE the device rows newest-batch-first (they want action; root vs device offsets aren't comparable anyway). Title/body mirror the server's push copy (approvalPushBody) so the row reads like the row the push WOULD have made; status line says "Needs approval — no notification reached this device" (honest about why there's no delivery status)
• Same expansion, detail, decide actions as a real row NotificationRow generalized to the NotificationListRow union — the SAME component, chevron toggle, ApprovalNotificationDetail, ApprovalBatchBody, ApprovalBatchActions; synthetic rows also honor the push-targeting pre-expansion param. No fork
• Decided-batch fate noted CHOICE: a decided orphan mirrors the retired screen's open list verbatim (it IS deriveOpenBatches): all-reject vanishes immediately (with its expansion), approved lingers as "Decided — awaiting release" until every approved index settles, then vanishes. Decided history lives only on real device rows where they exist — same as the retired screen, whose Recent list was equally ephemeral per-load
• Unit tests for the union/dedupe derivation notifications.test.ts: orphan-above-device ordering + copy, dedupe by offset, rejected-vanishes / approved-lingers-until-settled, expired never surfaces, newest-first sort + burst copy
• Spec: the orphan lane notifications.spec.ts stages a batch BEFORE device enrollment (the device's intent subscription starts at enrollment — start: "now" in device-processor-implementation.ts — so the poll waits for the push intent on the root stream, then enrolls: the batch is permanently unrepresented on the device). Asserts the synthetic row on arrival, expands it at the end, rejects from the expansion, and asserts the row detaches entirely. approvalTimeoutMs bumped 120s → 300s so the orphan (decided last) can't expire mid-spec. 3x consecutive passes; approvals.spec.ts (chat.tsx shares the new constant) also 3x

Timeout experiment: one 1s action timeout everywhere (Misha, 2026-08-03)#

Directive: "make the global action timeout 1s and stick with that always", plus option 3 (a Suspense spinner for Metro's lazy compiles) if there's a clear seam, else option 2 (a dev-bundler network waiter).

• SPEC_ACTION_TIMEOUT_MS 750 → 1_000; the videoMode ? 10_000 global conditional and the mobile project's 5_000 override both deleted one number, every project, video mode included; the reassurance comment for future agents lives at the constant in packages/shared/src/test-support/e2e-policy/budgets.ts
• Metro compile coverage — option 2, the dev-bundler network waiter CHOICE and why: option 3 has NO clean seam in this app. expo-router wraps every screen in Suspense but hard-codes its fallback (build/views/SuspenseFallback.js, "TODO: Support user's customizing") — no user hook to render our labeled ActivityIndicator; AND async routes are disabled here (app.json extra.router has no asyncRoutes), so route chunks don't exist — the real mid-spec compiles are component-level dynamic imports (e.g. react-native-enriched-markdown's wasm), which a route Suspense boundary would never cover. So: specs/test-support/metro-bundle-spinner.ts holds a [data-spinner="true"] marker while any Metro .bundle request is in flight (wired for every page in test.ts). Dev-spec-only by construction: .bundle is Metro's dev-server compile endpoint; production builds and the OS app never match
• The one thing that genuinely needed >1s, with trace evidence both mobile specs' page.waitForEvent("popup") inherited the global default and failed at 1s. Trace: click "Sign in" completed in 26ms, popup still absent 966ms later — signIn makes THREE sequential auth round trips (issuer discovery → OIDC config → dynamic client registration) before window.open, >1s against a cold local dev worker. Not a UI action (spinner-waiter never wraps waitForEvent) — an event wait; both call sites now pass an explicit { timeout: 15_000 } (the repo's cross-server tier) with a comment. The global 1s stands
• Verification at 1s (dev restart before every run) notifications 5/5 after the popup fix (before it: 2 failures, both that popup wait); approvals 5/5 spec passes with one mid-series "webServer exited early" dev-server OOM excluded (the known miniflare heap death, not a spec result); signup (web project) 1/1 at 9.6s; VIDEO_MODE mobile approvals 1/1 at 40.4s — 1s survives the click-moment screenshot overhead

Review round (tabbed-cards screenshot feedback):

• Self-referential provenance block hidden inside activity cards ApprovalBatchBody gains a REQUIRED showThreadInfo prop (explicit at both callsites per repo preference): the Notifications expansion passes true, the activity card false — the card lives inside the thread the block would link to. The non-script "Triggered from …" / "Source metadata unavailable" fallbacks are gated too (same provenance concern)
• Tab selection restyled to read as a TAB, not a pill flat tab bar: full-width hairline baseline in colors.border, active tab marked by a 2px underline + label in neutral colors.text (never the accent — that is the status badges' color), inactive labels textFaint. No borders/pills/backgrounds, so nothing resembles the Approved/Rejected badges. Spec tab presses target role+name, so the restyle needed no selector changes