fix(agent-dashboard): persist acknowledgedAgentsByPaneKey across restart

Without this, agent rows the user already visited come back bold every relaunch now that the rows themselves survive restart (per docs/agent-dashboard-retention-restart.md). Hydrate sanitizes input field-by-field (rejects null/non-object/array, prototype-pollution keys, non-finite/non-positive values) and applies a 7-day TTL paralleling HYDRATE_MAX_AGE_MS in agent-hooks/server.ts so hard-quit/crash paths can't grow the persisted map forever.

Co-authored-by: Orca <help@stably.ai>
This commit is contained in:
brennanb2025
2026-05-10 22:58:53 -07:00
co-authored by Orca
parent bff9989709
commit d6827bd344
6 changed files with 251 additions and 8 deletions
+4 -2
View File
@@ -18,7 +18,7 @@ In scope:
Out of scope:
- Renderer-side persistence (`retainedAgentsByPaneKey`, `acknowledgedAgentsByPaneKey`, `retentionSuppressedPaneKeys`). The renderer's retention slice and `useRetainedAgentsSync` continue to operate purely in memory; their "stay until dismissed" contract is upheld by the live entry being pulled from the main-process status snapshot after renderer hydration.
- Renderer-side persistence of `retainedAgentsByPaneKey` and `retentionSuppressedPaneKeys`. The retention slice and `useRetainedAgentsSync` continue to operate purely in memory; their "stay until dismissed" contract is upheld by the live entry being pulled from the main-process status snapshot after renderer hydration. `acknowledgedAgentsByPaneKey` is the deliberate exception — see "Why this beats the renderer-side alternative" below for the rationale.
- Daemon-side persistence. The daemon owns PTY lifetime and scrollback, not hook events. The hook server (in the Electron main process) is the right home.
- Cross-device sync. The on-disk file lives in `userData`; running Orca on a second machine does not carry retained activity.
- Reintroducing the old experimental dashboard gate. Agent activity is now default-on; row visibility is controlled by worktree-card properties.
@@ -253,7 +253,7 @@ Follow-ups (explicitly out of this design):
## Non-Goals
- Persisting `retainedAgentsByPaneKey`, `acknowledgedAgentsByPaneKey`, or `retentionSuppressedPaneKeys`. The renderer's retention slice is computed downstream from the live cache; the live cache is now persistent, so retention falls out for free.
- Persisting `retainedAgentsByPaneKey` or `retentionSuppressedPaneKeys`. The renderer's retention slice is computed downstream from the live cache; the live cache is now persistent, so retention falls out for free. `acknowledgedAgentsByPaneKey` is intentionally persisted — see "Why this beats the renderer-side alternative" for why that one map is the exception.
- Daemon-side persistence. The daemon owns PTY lifetime and does not see hook events. The hook server (in the Electron main process) is the right home.
- Cross-device sync. The persisted file lives in local `userData/agent-hooks/`; running Orca on a second machine does not carry status over.
- A dedicated settings UI. Agent activity is default-on; no new toggle is added.
@@ -267,6 +267,8 @@ An earlier draft of this design proposed mirroring `retainedAgentsByPaneKey` to
2. **Wrong scope.** A renderer-side retention persister, by definition, cannot resurrect a `blocked` Claude or a quiet `working` agent — those entries never made it into `retainedAgentsByPaneKey` in the first place (retention is gated on `state === 'done'`). Persisting the upstream cache covers all three states with one mechanism.
3. **Wrong file.** `PersistedUIState` holds scalar prefs and small bounded metadata; structured event payloads with multi-KB `lastAssistantMessage` strings do not belong there. `WorkspaceSessionState` is closer in spirit but inherits problem (1) and (2) above. `userData/agent-hooks/` already exists for hook-server-owned cross-restart artifacts (the endpoint file); the cache file is the natural neighbor.
The deliberately narrow exception: `acknowledgedAgentsByPaneKey` IS persisted in `PersistedUIState`, despite the general rule above. Why it fits: it's a tiny `Record<string, number>` of paneKey → ack timestamp, not an event payload — it inherits exactly the "scalar prefs and small bounded metadata" character `PersistedUIState` was designed for. Hydrate-time TTL plus shape sanitization (in `src/renderer/src/store/slices/ui.ts`) bound size across many sessions. And without it, agent rows the user already visited come back bold every relaunch now that the rows themselves survive restart, defeating the whole point of the restart-preservation work.
The renderer-side approach was strictly smaller code; the upstream approach is strictly more correct. The diff size for both is roughly the same once tests are factored in (one main-process file with bounded changes vs. one renderer slice + one shared type + one App.tsx init effect + sanitization), so we picked correctness.
## Acceptance Criteria
+11 -2
View File
@@ -189,6 +189,7 @@ function App(): React.JSX.Element {
const showActiveOnly = useAppStore((s) => s.showActiveOnly)
const hideDefaultBranchWorkspace = useAppStore((s) => s.hideDefaultBranchWorkspace)
const filterRepoIds = useAppStore((s) => s.filterRepoIds)
const acknowledgedAgentsByPaneKey = useAppStore((s) => s.acknowledgedAgentsByPaneKey)
const persistedUIReady = useAppStore((s) => s.persistedUIReady)
const rightSidebarWidth = useAppStore((s) => s.rightSidebarWidth)
const rightSidebarOpen = useAppStore((s) => s.rightSidebarOpen)
@@ -490,7 +491,14 @@ function App(): React.JSX.Element {
sortBy,
showActiveOnly,
hideDefaultBranchWorkspace,
filterRepoIds
filterRepoIds,
// Why: rides the same debounced save so dashboard auto-acks (which fire
// on focus/visibility) and the in-memory ack cleanup paths in
// agent-status.ts (close/dismiss) both flow to disk through map
// identity changes. Without persisting, agent rows that survive
// restart (per docs/agent-dashboard-retention-restart.md) come back
// bold even when the user had already visited them.
acknowledgedAgentsByPaneKey
})
}, 150)
@@ -503,7 +511,8 @@ function App(): React.JSX.Element {
sortBy,
showActiveOnly,
hideDefaultBranchWorkspace,
filterRepoIds
filterRepoIds,
acknowledgedAgentsByPaneKey
])
// Apply theme to document
+177
View File
@@ -1,3 +1,4 @@
/* eslint-disable max-lines */
import { createStore, type StoreApi } from 'zustand/vanilla'
import { afterEach, describe, expect, it, vi } from 'vitest'
import { getDefaultUIState } from '../../../../shared/constants'
@@ -199,6 +200,182 @@ describe('createUISlice hydratePersistedUI', () => {
})
})
it('restores acknowledgedAgentsByPaneKey from persisted UI state', () => {
const now = 1_700_000_000_000
vi.useFakeTimers()
vi.setSystemTime(now)
try {
const store = createUIStore()
store.getState().hydratePersistedUI(
makePersistedUI({
acknowledgedAgentsByPaneKey: { 'tab-a:0': now, 'tab-b:1': now - 5_000 }
})
)
expect(store.getState().acknowledgedAgentsByPaneKey).toEqual({
'tab-a:0': now,
'tab-b:1': now - 5_000
})
} finally {
vi.useRealTimers()
}
})
it('falls back to an empty ack map when persisted UI omits acknowledgedAgentsByPaneKey', () => {
const store = createUIStore()
store.getState().hydratePersistedUI(makePersistedUI())
expect(store.getState().acknowledgedAgentsByPaneKey).toEqual({})
})
it('falls back to an empty ack map when persisted acknowledgedAgentsByPaneKey is null', () => {
const store = createUIStore()
store.getState().hydratePersistedUI(
makePersistedUI({
acknowledgedAgentsByPaneKey:
null as unknown as PersistedUIState['acknowledgedAgentsByPaneKey']
})
)
expect(store.getState().acknowledgedAgentsByPaneKey).toEqual({})
})
it('falls back to an empty ack map when persisted acknowledgedAgentsByPaneKey is a string', () => {
const store = createUIStore()
store.getState().hydratePersistedUI(
makePersistedUI({
acknowledgedAgentsByPaneKey:
'oops' as unknown as PersistedUIState['acknowledgedAgentsByPaneKey']
})
)
expect(store.getState().acknowledgedAgentsByPaneKey).toEqual({})
})
it('falls back to an empty ack map when persisted acknowledgedAgentsByPaneKey is an array', () => {
const store = createUIStore()
store.getState().hydratePersistedUI(
makePersistedUI({
acknowledgedAgentsByPaneKey: [
'a',
'b'
] as unknown as PersistedUIState['acknowledgedAgentsByPaneKey']
})
)
expect(store.getState().acknowledgedAgentsByPaneKey).toEqual({})
})
it('drops non-number / non-finite / non-positive entries from acknowledgedAgentsByPaneKey', () => {
const now = 1_700_000_000_000
vi.useFakeTimers()
vi.setSystemTime(now)
try {
const store = createUIStore()
store.getState().hydratePersistedUI(
makePersistedUI({
acknowledgedAgentsByPaneKey: {
'tab-a:0': now,
'tab-b:1': now - 1000,
'tab-c:2': 'not-a-number',
'tab-d:3': Number.NaN,
'tab-e:4': Number.POSITIVE_INFINITY,
'tab-f:5': -1
} as unknown as Record<string, number>
})
)
expect(store.getState().acknowledgedAgentsByPaneKey).toEqual({
'tab-a:0': now,
'tab-b:1': now - 1000
})
} finally {
vi.useRealTimers()
}
})
it('prunes acknowledgedAgentsByPaneKey entries older than the 7-day TTL during hydration', () => {
// HYDRATE_MAX_AGE_MS lives in src/renderer/src/store/slices/ui.ts and matches
// the constant in src/main/agent-hooks/server.ts.
const SEVEN_DAYS_MS = 7 * 24 * 60 * 60 * 1000
const now = 1_700_000_000_000
vi.useFakeTimers()
vi.setSystemTime(now)
try {
const store = createUIStore()
store.getState().hydratePersistedUI(
makePersistedUI({
acknowledgedAgentsByPaneKey: {
'tab-recent:0': now,
'tab-old:1': now - SEVEN_DAYS_MS - 1
}
})
)
expect(store.getState().acknowledgedAgentsByPaneKey).toEqual({
'tab-recent:0': now
})
} finally {
// The shared afterEach restores mocks/globals but not timers, so clean up
// here to avoid leaking fake timers into subsequent tests.
vi.useRealTimers()
}
})
it('drops prototype-pollution keys from acknowledgedAgentsByPaneKey during hydration', () => {
const now = 1_700_000_000_000
vi.useFakeTimers()
vi.setSystemTime(now)
try {
const store = createUIStore()
const malicious: Record<string, number> = {}
// Object.defineProperty so these land as own enumerable properties rather
// than getting silently re-routed to Object.prototype by the JS engine.
Object.defineProperty(malicious, '__proto__', {
value: now,
enumerable: true,
configurable: true,
writable: true
})
Object.defineProperty(malicious, 'constructor', {
value: now,
enumerable: true,
configurable: true,
writable: true
})
Object.defineProperty(malicious, 'prototype', {
value: now,
enumerable: true,
configurable: true,
writable: true
})
malicious['tab-safe:0'] = now
store.getState().hydratePersistedUI(
makePersistedUI({
acknowledgedAgentsByPaneKey: malicious
})
)
expect(store.getState().acknowledgedAgentsByPaneKey).toEqual({
'tab-safe:0': now
})
} finally {
vi.useRealTimers()
}
})
it('merges and persists partial task resume updates', () => {
const setUI = vi.fn().mockResolvedValue(undefined)
vi.stubGlobal('window', { api: { ui: { set: setUI } } })
+48 -3
View File
@@ -76,6 +76,12 @@ const MAX_LEFT_SIDEBAR_WIDTH = 500
// cap on wide displays. Use a large hard ceiling purely as a safety net for
// corrupted/manually-edited values rather than as a product limit.
const MAX_RIGHT_SIDEBAR_WIDTH = 4000
// Why: bound disk growth for acknowledgedAgentsByPaneKey across hard quits —
// in-session cleanup (agent-status.ts) prunes on pane lifecycle, but crash/
// forced-kill paths leave entries pinned. Mirrors HYDRATE_MAX_AGE_MS in
// src/main/agent-hooks/server.ts for parallel reasoning with the sibling
// hook-status entries these acks pair with.
const HYDRATE_MAX_AGE_MS = 7 * 24 * 60 * 60 * 1000
const VALID_TASK_PRESETS = new Set<TaskViewPresetId>([
'all',
'issues',
@@ -111,6 +117,34 @@ function sanitizePersistedSidebarWidth(width: unknown, fallback: number, maxWidt
return Math.min(maxWidth, Math.max(MIN_SIDEBAR_WIDTH, width))
}
// Why: persisted JSON can be tampered with or carry legacy/corrupt shapes.
// Reject arrays (typeof [] === 'object'), prototype-pollution keys, and
// non-positive-finite values; drop entries past the TTL so hard-quit leaks
// don't accumulate forever.
function sanitizeAcknowledgedAgentsByPaneKey(value: unknown): Record<string, number> {
if (value === null || typeof value !== 'object' || Array.isArray(value)) {
return {}
}
const cutoff = Date.now() - HYDRATE_MAX_AGE_MS
const out: Record<string, number> = {}
for (const [key, ackAt] of Object.entries(value as Record<string, unknown>)) {
if (typeof key !== 'string') {
continue
}
if (key === '__proto__' || key === 'constructor' || key === 'prototype') {
continue
}
if (typeof ackAt !== 'number' || !Number.isFinite(ackAt) || ackAt <= 0) {
continue
}
if (ackAt < cutoff) {
continue
}
out[key] = ackAt
}
return out
}
function sanitizeTaskResumeState(value: unknown): TaskResumeState | undefined {
if (!value || typeof value !== 'object') {
return undefined
@@ -154,9 +188,10 @@ export type UISlice = {
* the user clicks an agent row or its parent workspace card from the
* dashboard. A row is considered unvisited when no ack exists OR the
* agent's current stateStartedAt is newer than the last ack (i.e. the
* agent has transitioned state since the user last saw it). Session-only
* — restart resets everyone to unvisited, which is harmless since the
* first visit after launch is a legitimate "need to see" moment. */
* agent has transitioned state since the user last saw it). Persisted
* via PersistedUIState because agent rows themselves now survive restart
* (see docs/agent-dashboard-retention-restart.md) — without this, rows
* you'd already visited come back bold on relaunch. */
acknowledgedAgentsByPaneKey: Record<string, number>
acknowledgeAgents: (paneKeys: string[]) => void
/** Per-worktree collapsed state for the inline agents section shown inside
@@ -746,6 +781,16 @@ export const createUISlice: StateCreator<AppState, [], [], UISlice> = (set, get)
ui.trustedOrcaHooks ?? {},
validRepoIds
),
// Why: restore visited-row acks alongside the persisted hook entries
// they pair with. Stale acks for paneKeys whose tab/PTY no longer
// exists are inert (no row references them); a paneKey reuse stamps a
// fresh stateStartedAt that beats the old ack via the ackAt <
// stateStartedAt comparison in WorktreeCardAgents. Sanitizer drops
// entries past HYDRATE_MAX_AGE_MS so hard-quit/crash paths that miss
// the in-session cleanup in agent-status.ts can't accumulate forever.
acknowledgedAgentsByPaneKey: sanitizeAcknowledgedAgentsByPaneKey(
ui.acknowledgedAgentsByPaneKey
),
persistedUIReady: true
}
}),
+2 -1
View File
@@ -292,7 +292,8 @@ export function getDefaultUIState(): PersistedUIState {
statusBarVisible: true,
dismissedUpdateVersion: null,
lastUpdateCheckAt: null,
trustedOrcaHooks: {}
trustedOrcaHooks: {},
acknowledgedAgentsByPaneKey: {}
}
}
+9
View File
@@ -1461,6 +1461,15 @@ export type PersistedUIState = {
/** Once the user has seen the "your sessions won't be interrupted"
* reassurance card, we never show it again. */
updateReassuranceSeen?: boolean
/** Per-paneKey "user has visited this row" timestamps, used by the inline
* agents list to mute rows the user has already seen. Persisted because
* agent rows themselves now survive restart (see
* docs/agent-dashboard-retention-restart.md); without persisting acks too,
* rows you'd already clicked come back bold on relaunch. Stale entries
* keyed on dead panes are inert: a future paneKey reuse stamps a fresh
* stateStartedAt that beats the old ack via the existing comparison in
* WorktreeCardAgents. Renderer-owned, written through ui:set. */
acknowledgedAgentsByPaneKey?: Record<string, number>
/** URL to navigate to when a new browser tab is opened. Null means blank tab.
* Phase 3 will expand this to a full BrowserSessionProfile per workspace. */
browserDefaultUrl?: string | null