From b009f95d4e99e88d8db47869e7328d5f75513108 Mon Sep 17 00:00:00 2001 From: Merge Sim Date: Wed, 9 Sep 2026 20:38:47 -0700 Subject: [PATCH] refactor(agent-status): make the renderer a subscriber for structured rows MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Main's two half-migration filters are gone, so a locally hosted structured session's row now reaches both renderer windows over `agentStatus:set` and comes back on the `agentStatus:getSnapshot` replay pull. Removing the send guard is also what first gives the dashboard popout structured sessions at all. Removing them was not enough on its own. A structured pane key resolves to nothing in the renderer's terminal-tab routing index — an `agent-session` tab lives in `unifiedTabsByWorktree` — so the applicator held every published row as `pending` forever, with every test still green. The new routing module resolves a structured row against that tab, and requires one: deliberately narrower than `worktree ps`, because it is what the sidebar already showed, and because a terminal tab in chat view mode is backed by a structured session too and already has its own row. `structuredHost: 'owned'` maps back to `structuredHostOwned` so the freshness bypass still fires; `terminalResumeEligible: false` is kept so no sleeping record offers to relaunch a chat as a TUI; `allowOlderTimestamp` is dropped because main stamps a forward-only `receivedAt`. The row's name is read from the session tab rather than put on the wire. The bridge stops projecting for a local target only. A remote host writes into ITS store and nothing mirrors structured rows down, so it stays that session's writer. Its unmount teardown stays for both lanes: the host's close drops its row through `dropStatusEntry`, which emits no renderer clear. Command Code output seeds, parked-pane seeds and the pty-exit removal are NOT deleted, against the plan. Main emits Command Code working/done as side-effect FACTS the renderer turns into the write, gated on renderer-only pane ownership state; nothing puts them in the hook store. The done-settle window stays with them. The doc records both, with the mechanism. --- docs/reference/agent-status-store.md | 202 ++++++++++--- .../server/server-ingest-structured.ts | 6 +- src/main/ipc/agent-hooks.test.ts | 15 +- src/main/ipc/agent-hooks.ts | 10 +- src/main/startup/main-window-agent-status.ts | 14 +- ...in-window-structured-status-filter.test.ts | 90 ------ ...indow-structured-status-forwarding.test.ts | 125 ++++++++ ...tructuredAgentSessionStatusBridge.test.tsx | 40 ++- .../StructuredAgentSessionStatusBridge.tsx | 40 ++- .../ipc-events/agent-status-apply-commit.ts | 71 +++++ .../agent-status-event-applicator.ts | 76 ++--- .../structured-agent-session-row-routing.ts | 63 ++++ ...red-agent-session-row-subscription.test.ts | 271 ++++++++++++++++++ 13 files changed, 824 insertions(+), 199 deletions(-) delete mode 100644 src/main/startup/main-window-structured-status-filter.test.ts create mode 100644 src/main/startup/main-window-structured-status-forwarding.test.ts create mode 100644 src/renderer/src/hooks/ipc-events/agent-status-apply-commit.ts create mode 100644 src/renderer/src/hooks/ipc-events/structured-agent-session-row-routing.ts create mode 100644 src/renderer/src/hooks/ipc-events/structured-agent-session-row-subscription.test.ts diff --git a/docs/reference/agent-status-store.md b/docs/reference/agent-status-store.md index 22af4c4889b..c82a1ea39df 100644 --- a/docs/reference/agent-status-store.md +++ b/docs/reference/agent-status-store.md @@ -12,7 +12,7 @@ this order, each independently shippable: 3. shared: one worktree-status rollup and one freshness rule for every reader. The PR that carries this document is PR 1a. Sections below are grouped under -the step that delivers them; PR 1a, PR 1b and PR 2a have landed. +the step that delivers them; PR 1a, PR 1b, PR 2a and PR 2b have landed. ## The problem this solves @@ -92,14 +92,14 @@ The structured feed keeps its job of projecting a session's journal into a summary and streaming it to subscribers. On every publish it additionally ingests the summary into the hook server as a status row: -| Row field | From | -| ----------------- | ------------------------------------------------------------- | -| `paneKey` | `structuredAgentSessionPaneKey(sessionId)`, the key the renderer also derives (PR 2a made it take the session id alone); its leaf is UUID-shaped so pane-key validation accepts it | -| `tabId` | `structuredAgentSessionTabId(sessionId)` | -| `worktreeId` | `summary.workspaceId` (a folder workspace id is a valid value) | -| `state` | `structuredAgentSessionStatusState(summary.status)`, the mapping #19217 shared | -| `structuredHost` | `'owned'` while `summary.hostExecutionOwned` is set, otherwise `'held'`; `worktree ps` derives its row's `structuredHostOwned` from it | -| prompt, tool, last message, model, provider session | the summary's fields | +| Row field | From | +| --------------------------------------------------- | ---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| `paneKey` | `structuredAgentSessionPaneKey(sessionId)`, the key the renderer also derives (PR 2a made it take the session id alone); its leaf is UUID-shaped so pane-key validation accepts it | +| `tabId` | `structuredAgentSessionTabId(sessionId)` | +| `worktreeId` | `summary.workspaceId` (a folder workspace id is a valid value) | +| `state` | `structuredAgentSessionStatusState(summary.status)`, the mapping #19217 shared | +| `structuredHost` | `'owned'` while `summary.hostExecutionOwned` is set, otherwise `'held'`; `worktree ps` derives its row's `structuredHostOwned` from it | +| prompt, tool, last message, model, provider session | the summary's fields | Sessions with no persisted turn (`status === null`) produce no row, matching what the chat shows. When the host revokes live ownership the row is re-set @@ -179,10 +179,10 @@ path, and the hand-rolled check in `runtime-worktree-agent-rows.ts` goes. an old client ignores them. `worktree ps` rows keep their shape and vocabulary, so the mobile app sees no change. -Until PR 2 the main process does not forward structured rows to the renderer -over `agentStatus:set` or `agentStatus:getSnapshot`. The renderer's feed -bridge still writes those rows itself, and forwarding them too would give one -pane key two writers. Removing that filter is the first step of PR 2. +PR 1a did not forward structured rows to the renderer over `agentStatus:set` or +`agentStatus:getSnapshot`: the renderer's feed bridge still wrote those rows +itself, and forwarding them too would have given one pane key two writers. +PR 2b removed both filters. ## PR 1b: the runtime's retained row store is deleted @@ -426,50 +426,172 @@ moving now sorts by when it finished rather than by its latest journal write. ## PR 2b: the renderer subscribes -With structured rows arriving over `agentStatus:set`, the renderer's -`StructuredAgentSessionStatusBridge` no longer needs to write status; its -unmount cleanup becomes a tab-close signal to the host. The IPC applicator is -the single writer for observed status. The 2026-09-09 audit sorted the other -writers: +Landed. Both filters are gone, the IPC applicator applies the host's structured +rows, and `StructuredAgentSessionStatusBridge` no longer projects status for a +locally hosted session. + +### The two filters this step removed + +- `main/startup/main-window-agent-status.ts` — the `if (structuredHost) return` + guard above the `agentStatus:set` sends. It sat above BOTH the main window and + `getDashboardPopoutWindow()`, so removing it is also what first gives the + dashboard popout structured sessions. +- `main/ipc/agent-hooks.ts` — the `.filter((entry) => entry.structuredHost === undefined)` + on `agentStatus:getSnapshot`, which is the pull a renderer does after + hydration. Without it a native chat had no row until its next journal edge, and + a settled one never came back at all. + +Two things the window listener now skips for a row carrying `structuredHost`, +neither of which the plan anticipated: + +- `maybeAutoRenameBranchOnFirstWork`. First-work branch rename is a PTY-agent + feature whose structured-session gap is tracked on its own; publishing the row + must not switch it on half-covered as a side effect. +- `driveSyntheticTitleFromHook`. It injects a title into a pane's title slot, and + a structured session has no pane. + +### Removing the filters was not enough on its own + +The plan treated this step as a switch flip. It is not: a structured pane key +resolves to no entry in the renderer's terminal-tab routing index, because an +`agent-session` tab lives in `unifiedTabsByWorktree`, not `tabsByWorktree`. The +applicator's `exists` gate therefore held every published structured row as +`pending` forever — main would have published rows the renderer dropped on the +floor, with every test still green. + +`ipc-events/structured-agent-session-row-routing.ts` is what closes that. It +resolves a structured row against the `agent-session` tab, matching either the +tab's own id or the id derived from its `entityId`, so a `:history-N` mirrored +surface still matches. + +That tab is REQUIRED — deliberately narrower than the admission rule +`worktree ps` uses, which lists a session the host holds whether or not a surface +is open. Two reasons: + +- it is what the renderer already showed. The bridge this replaces only wrote a + row for a mounted `agent-session` tab, so requiring one keeps sidebar + visibility unchanged rather than adding rows as a side effect of the flip; +- a terminal tab in chat view mode is backed by a structured session too + (`TerminalPaneNativeChatPortal`), and its own pane already has a row. + Admitting the host's row for that session as well would put two rows in the + sidebar for one tab. `worktree ps` has carried that pair since PR 1a; the + sidebar must not inherit it here. + +It also resolves the row's title from that tab. The wire deliberately carries no +title — naming a structured session is a separate lane — but the sidebar +synthesizes a tab for a paneless row (`worktree-agent-row-fallback-tab.ts`) and +names it `'Agent'` when the entry has none. The plan's reading that +`worktree-agent-row-type.ts` would fall back to `tab?.title` was about +AGENT-TYPE resolution, not the row's name, and it does not apply here: a +structured session has no `TerminalTab` at all. Reading the label in the renderer +keeps today's behavior without teaching main a name or putting one on the wire. + +### The four fields the bridge wrote that main's ingest does not + +- `terminalTitle` — resolved renderer-side from the session tab, as above. +- `structuredHostOwned` — the wire spells it `structuredHost: 'owned' | 'held'`, + and the applicator maps it back. It is the freshness bypass in + `shared/agent-status-freshness.ts`; without the mapping every native chat that + works for over half an hour decays to idle in the sidebar. +- `terminalResumeEligible: false` — kept, derived from `structuredHost` being + present at all. `agent-status-sleeping-records.ts` reads it: a structured row + carries a `providerSession`, so without this Orca mints a sleeping record and + offers to relaunch a native chat as a TUI. +- `allowOlderTimestamp` — dropped. The bridge needed it because it stamped the + journal clock as `updatedAt`, which a legacy publication can move backwards. + Main stamps `receivedAt = Math.max(Date.now(), watermark + 1)` + (`server-status-update.ts`), so the applicator's `data.receivedAt < +existingStatus.updatedAt` ordering check can never see a host row go backwards. + The journal clock still reaches the row, as `evidenceObservedAt`. + +### The unmount cleanup stays, and the plan's reason it could go is wrong + +The plan said the bridge's unmount cleanup becomes a tab-close signal to the +host. That signal already exists — `tab-group/workspace-tab-close-commands.ts` +calls `agentSession.close`, which reaches `forgetStructuredAgentSession` — but +sending it is not enough. The host drops its row through `dropStatusEntry`, which +notifies `subscribeStatusDrop` subscribers inside main and emits NO renderer +clear (only `clearPaneState` does, and the renderer's clear handler skips a row +already reading `done`, which is how a settled session ends). Deleting the +unmount write would therefore have left a ghost row in the sidebar for every +closed chat until the next reload. + +So the removal stays, for the local lane as much as the remote one. It is the +"unmount" row of the disposition table, not a status write. A projection unmounts +when its tab leaves the tab map — a close or a host retraction — not on a +worktree switch: the bridge maps over every structured tab in every worktree. + +### The bridge is retired for local sessions only + +One store per execution host. A session on a REMOTE runtime is projected into +that host's store, and nothing mirrors structured rows down — the web-session +mirror carries terminal surfaces and their `agentStatus`, and an `agent-session` +surface in that snapshot has no status field. Deleting the bridge outright would +have left every remote native chat with no sidebar row. + +So the bridge's STATUS WRITES are fenced on `target.kind === 'local'`: for a +local session it opens no feed and projects nothing, and for a remote one it +stays that session's writer, exactly as the disposition table already says for +the remote-runtime OSC parse. That is rule 3 of +[`remote-wire-compatibility.md`](./remote-wire-compatibility.md) — the fence +comes off when a remote store's rows are mirrored down, which is separate work. + +### The disposition table, corrected | Writer | Disposition | | ----------------------------------------------------------------- | -------------------------------------------------------------------------------------------------------- | -| Command Code output seeds, parked-pane seeds, pty-exit removal | delete; main already emits the same facts | -| structured bridge status writes | delete; main now publishes the row | +| structured bridge status writes, local sessions | deleted; main publishes the row | +| structured bridge status writes, remote sessions | kept; nothing mirrors a remote host's structured rows down | +| structured bridge unmount removal | kept for both lanes; `dropStatusEntry` emits no renderer clear | +| Command Code output seeds, parked-pane seeds | NOT deleted — see below | +| pty-exit removal | NOT deleted — see below | | launch placeholder seeds (a user launched an agent with a prompt) | keep for now; main holds the launch config and can seed later | | dismissal, acknowledgement, unmount | keep; user facts and component lifecycle | | remote-runtime OSC parse (bytes never transit local main) | keep, fenced behind the host's published row once the host is new enough; rule 3 of the wire doc applies | | web-session mirror receipt clock | keep; the decay rule needs both clocks from one machine | -The Command Code done-settle window is renderer policy with no main -equivalent. PR 2b either moves it into main's detector or leaves it, and says -which. +The audit's "main already emits the same facts" was true of the word FACTS and +false of the store: -### The two filters this step removes +- **Command Code output seeds and parked-pane seeds.** Main's detector + (`orca-runtime-create-terminal-side-effect-command-code-detector.ts`) calls + `recordTerminalSideEffectFact(ptyId, { kind: 'command-code-working' | 'command-code-done' })`. + That fact travels to the RENDERER, where + `terminal-side-effect-facts-handler.ts` turns it into the store write. Nothing + in main puts it in the hook server, and the write is gated on + `canCommandCodeOutputOwnPane`, which reads `paneForegroundAgentByPaneKey`, + `retainedAgentsByPaneKey` and `agentLaunchConfigByPaneKey` — renderer state + main does not have. Command Code has hooks for PreToolUse/PostToolUse/Stop but + no prompt-submit hook, which is why the scrape exists at all. Deleting these + seeds without first building a main-side ingest would leave Command Code with + no working row. That ingest is a producer-routing change, PR-1 shaped, not part + of "the renderer subscribes". +- **pty-exit removal** (`pty-connection/pty-exit-hibernate.ts`). Every + attributable exit reaches `clearProviderPtyState` -> `clearPaneState`, which + does emit a renderer clear. But `server-cleanup.ts` documents the case that + resolution misses: a restored or reattached PTY may never rebuild the + spawn-time `ptyPaneKey` mapping, and those panes "keep a `working` row and its + latches for good". The renderer's own removal is the last thing retiring them. -Both were added by PR 1a and are the only thing keeping main out of the -renderer's lane. Each is pinned by a test, so removing them should turn those -tests red first, deliberately: +### The Command Code done-settle window stays in the renderer -- `main/startup/main-window-agent-status.ts` — the `if (structuredHost) return` - guard above the `agentStatus:set` sends. It sits above BOTH the main window - and `getDashboardPopoutWindow()`, so removing it is also what first gives the - dashboard popout structured sessions. -- `main/ipc/agent-hooks.ts` — the `.filter((entry) => entry.structuredHost === undefined)` - on `agentStatus:getSnapshot`. +It is left where it is. Moving it into main's detector only makes sense together +with a main-side Command Code status ingest — the window exists to decide when a +row completes, and main writes no such row today. Moving the timer without the +row would put the deadline on one side of the process boundary and the write on +the other. It is the same deferral as the item above, and belongs with it. ### What "one writer" actually means after this -Not zero renderer writers. The IPC applicator becomes the single writer for -OBSERVED status; the table above keeps four categories on purpose. Of those, -only the launch placeholder seeds are a deferral rather than a principle — -main holds the launch config and could seed them, and that is the next thing -to remove after this step, not part of it. +Not zero renderer writers. The IPC applicator is the single writer for OBSERVED +status on a locally hosted structured session; the table above keeps the rest on +purpose. Of those, only the launch placeholder seeds are a deferral rather than a +principle — main holds the launch config and could seed them. ### Ordering -Depends on PR 2a. Removing these filters before the two derivations converge -renders one session twice and shifts the chat's elapsed clock. +Depended on PR 2a: with the two derivations disagreeing, removing these filters +rendered one session twice and shifted the chat's elapsed clock. ## PR 3: one rollup, one clock diff --git a/src/main/agent-hooks/server/server-ingest-structured.ts b/src/main/agent-hooks/server/server-ingest-structured.ts index a1678fa278d..6d74913535b 100644 --- a/src/main/agent-hooks/server/server-ingest-structured.ts +++ b/src/main/agent-hooks/server/server-ingest-structured.ts @@ -54,8 +54,10 @@ export abstract class AgentHookServerIngestStructured extends AgentHookServerIng } /** The host no longer holds the session; its last projection is history the journal keeps. - * `dropStatusEntry`, not `clearPaneState`: the renderer's own bridge still owns this pane key, - * so a pane-status-clear would make main a second writer for it. */ + * `dropStatusEntry`, not `clearPaneState`: the pane caches and authority fences a pane-status + * clear tears down belong to a PTY, and a structured session never had any. The renderer's copy + * is taken out by the surface teardown in `StructuredAgentSessionStatusBridge`, since this drop + * emits no renderer clear. */ dropStructuredStatus(sessionId: string): void { this.dropStatusEntry(structuredAgentSessionPaneKey(sessionId), { preserveResumeIdentity: false diff --git a/src/main/ipc/agent-hooks.test.ts b/src/main/ipc/agent-hooks.test.ts index f8e3420720a..ba15d53d2ea 100644 --- a/src/main/ipc/agent-hooks.test.ts +++ b/src/main/ipc/agent-hooks.test.ts @@ -152,9 +152,10 @@ describe('agentStatus:getSnapshot IPC', () => { expect(handler!({})).toEqual(snapshot) }) - // The half-migration seam: until PR 2 retires the renderer's own feed bridge, main must not - // publish structured rows to the renderer at all — one pane key, one writer. - it('omits structured rows the renderer feed bridge still owns', async () => { + // PR 2b: the renderer subscribes instead of deriving, so a replay pull after hydration has to + // carry structured rows too — otherwise a native chat has no sidebar row until its next journal + // edge, and a settled one never comes back at all. + it('includes structured rows so a hydration replay does not lose native chats', async () => { getStatusSnapshot.mockReturnValue([ { paneKey: PANE_KEY, @@ -179,8 +180,12 @@ describe('agentStatus:getSnapshot IPC', () => { const { registerAgentHookHandlers } = await import('./agent-hooks') registerAgentHookHandlers() - const rows = handleHandlers.get('agentStatus:getSnapshot')!({}) as { paneKey: string }[] - expect(rows.map((row) => row.paneKey)).toEqual([PANE_KEY]) + const rows = handleHandlers.get('agentStatus:getSnapshot')!({}) as { + paneKey: string + structuredHost?: string + }[] + expect(rows.map((row) => row.paneKey)).toEqual([PANE_KEY, CHILD_PANE_KEY]) + expect(rows[1]?.structuredHost).toBe('owned') }) it('enriches the hook cache snapshot with runtime lineage metadata', async () => { diff --git a/src/main/ipc/agent-hooks.ts b/src/main/ipc/agent-hooks.ts index f460be06b5f..ee858e3cca8 100644 --- a/src/main/ipc/agent-hooks.ts +++ b/src/main/ipc/agent-hooks.ts @@ -49,13 +49,9 @@ export function registerAgentHookHandlers( // Why: the renderer pulls this after workspace hydration, so startup cannot // lose replayed statuses while its local store is still empty. Match the // live push enrichment in main/index.ts so parent/child rows survive replay. - return ( - agentHookServer - .getStatusSnapshot() - // Same rule as the live push: the renderer's feed bridge owns structured rows for now. - .filter((entry) => entry.structuredHost === undefined) - .map((entry) => enrichAgentStatusIpcPayload(entry, runtime)) - ) + return agentHookServer + .getStatusSnapshot() + .map((entry) => enrichAgentStatusIpcPayload(entry, runtime)) }) ipcMain.handle('agentStatus:inferInterrupt', (_event, request: unknown): boolean => { if (typeof request !== 'object' || request === null) { diff --git a/src/main/startup/main-window-agent-status.ts b/src/main/startup/main-window-agent-status.ts index 0f10a373bf9..50d01197802 100644 --- a/src/main/startup/main-window-agent-status.ts +++ b/src/main/startup/main-window-agent-status.ts @@ -49,11 +49,6 @@ export function installMainWindowAgentStatusListeners(options: MainWindowAgentSt if (state.mainWindow?.isDestroyed()) { return } - // Why: the renderer still derives structured rows from its own feed subscription; forwarding - // these too would give one pane key two writers until that bridge is retired. - if (structuredHost) { - return - } if (providerSessionOnly) { // Why: session_start just refreshes durable resume identity while Pi is idle; forward it without titles, telemetry, or status UI. state.mainWindow?.webContents.send('agentStatus:set', { @@ -72,7 +67,9 @@ export function installMainWindowAgentStatusListeners(options: MainWindowAgentSt }) return } - if (!restoredUnconfirmed) { + if (!restoredUnconfirmed && !structuredHost) { + // Why not structured rows: first-work branch rename is a PTY-agent feature whose + // structured-session gap is tracked on its own; publishing the row must not enable it. options.maybeAutoRenameBranchOnFirstWork({ paneKey, tabId, worktreeId, payload, isReplay }) } const runtime = state.runtime @@ -102,7 +99,8 @@ export function installMainWindowAgentStatusListeners(options: MainWindowAgentSt ...(promptInteractionKey ? { promptInteractionKey } : {}), ...(restoredUnconfirmed ? { restoredUnconfirmed: true } : {}), ...(observation ? { observation } : {}), - ...(orchestration ? { orchestration } : {}) + ...(orchestration ? { orchestration } : {}), + ...(structuredHost ? { structuredHost } : {}) } state.mainWindow?.webContents.send('agentStatus:set', statusEvent) if (!suppressSyntheticCodexAutoApprovalTitle || isAskUserQuestionTool(payload.toolName)) { @@ -113,6 +111,8 @@ export function installMainWindowAgentStatusListeners(options: MainWindowAgentSt const profile = getSyntheticAgentTitleProfile(payload.agentType) if ( profile && + // A structured session has no PTY and no terminal title slot to inject one into. + !structuredHost && shouldDriveSyntheticAgentTitleFromHook(payload.agentType, payload.state) && !suppressSyntheticCodexAutoApprovalTitle ) { diff --git a/src/main/startup/main-window-structured-status-filter.test.ts b/src/main/startup/main-window-structured-status-filter.test.ts deleted file mode 100644 index a3cd1bfdf0a..00000000000 --- a/src/main/startup/main-window-structured-status-filter.test.ts +++ /dev/null @@ -1,90 +0,0 @@ -// The renderer half of the half-migration seam. -// -// Until PR 2 retires `StructuredAgentSessionStatusBridge`, the renderer writes structured rows -// itself. Main forwarding them too would give one pane key two writers, so the window listener -// drops them — a filter nothing else asserts, which makes deleting it green everywhere. - -import { beforeEach, describe, expect, it, vi } from 'vitest' -import type { EnrichedAgentHookEventPayload } from '../agent-hooks/server' - -const hooks = vi.hoisted(() => ({ - listener: null as ((payload: EnrichedAgentHookEventPayload) => void) | null -})) - -vi.mock('electron', () => ({ - app: { getPath: () => '', on: vi.fn(), isReady: () => true } -})) -vi.mock('../agent-hooks/server', () => ({ - agentHookServer: { - setListener: (listener: ((payload: EnrichedAgentHookEventPayload) => void) | null) => { - hooks.listener = listener - }, - setPaneStatusClearListener: vi.fn() - } -})) -vi.mock('../agent-hooks/migration-unsupported-pty-state', () => ({ - setMigrationUnsupportedPtyListener: vi.fn() -})) -vi.mock('../window/dashboard-popout-window', () => ({ - getDashboardPopoutWindow: () => null -})) -vi.mock('./synthetic-title-runtime', () => ({ - driveSyntheticTitleFromHook: vi.fn(), - shouldSuppressCodexAutoApprovalSyntheticTitleFromHook: () => false, - stopAllSyntheticTitleSpinners: vi.fn() -})) - -import { installMainWindowAgentStatusListeners } from './main-window-agent-status' -import { mainProcessState } from './main-process-state' - -const sent: { channel: string; event: { paneKey: string } }[] = [] - -function statusPayload( - over: Partial -): EnrichedAgentHookEventPayload { - return { - paneKey: 'pane-1', - tabId: 'tab-1', - worktreeId: 'repo::/wt', - connectionId: null, - receivedAt: 1, - stateStartedAt: 1, - payload: { state: 'working', prompt: 'ship it', agentType: 'codex' }, - ...over - } as EnrichedAgentHookEventPayload -} - -beforeEach(() => { - sent.length = 0 - hooks.listener = null - mainProcessState.runtime = null - mainProcessState.mainWindow = { - isDestroyed: () => false, - webContents: { - send: (channel: string, event: { paneKey: string }) => sent.push({ channel, event }) - } - } as unknown as typeof mainProcessState.mainWindow - installMainWindowAgentStatusListeners({ - window: mainProcessState.mainWindow!, - maybeAutoRenameBranchOnFirstWork: vi.fn(), - onRecordAgentState: vi.fn() - }) -}) - -describe('the main-window agent-status listener', () => { - it('forwards a hook row but never a structured one', () => { - expect(hooks.listener).not.toBeNull() - - hooks.listener!(statusPayload({ paneKey: 'hook-pane' })) - hooks.listener!( - statusPayload({ - paneKey: 'structured-agent-session-s1:leaf', - structuredHost: 'owned' - }) - ) - - expect(sent.map((entry) => `${entry.channel}:${entry.event.paneKey}`)).toEqual([ - 'agentStatus:set:hook-pane' - ]) - }) -}) diff --git a/src/main/startup/main-window-structured-status-forwarding.test.ts b/src/main/startup/main-window-structured-status-forwarding.test.ts new file mode 100644 index 00000000000..df1d014c645 --- /dev/null +++ b/src/main/startup/main-window-structured-status-forwarding.test.ts @@ -0,0 +1,125 @@ +// PR 2b: main is the writer for a locally hosted structured session, so the window listener +// forwards its rows like any other. It sends to BOTH renderer windows, which is what first gives +// the dashboard popout structured sessions at all — the popout has never had them. +// +// The two things it must NOT do for a paneless row: drive a synthetic terminal title into a pane +// that does not exist, and trip first-work branch rename, whose structured-session gap is tracked +// on its own. + +import { beforeEach, describe, expect, it, vi } from 'vitest' +import type { EnrichedAgentHookEventPayload } from '../agent-hooks/server' + +const hooks = vi.hoisted(() => ({ + listener: null as ((payload: EnrichedAgentHookEventPayload) => void) | null, + popout: null as { webContents: { send: (channel: string, event: unknown) => void } } | null, + driveSyntheticTitleFromHook: vi.fn() +})) + +vi.mock('electron', () => ({ + app: { getPath: () => '', on: vi.fn(), isReady: () => true } +})) +vi.mock('../agent-hooks/server', () => ({ + agentHookServer: { + setListener: (listener: ((payload: EnrichedAgentHookEventPayload) => void) | null) => { + hooks.listener = listener + }, + setPaneStatusClearListener: vi.fn() + } +})) +vi.mock('../agent-hooks/migration-unsupported-pty-state', () => ({ + setMigrationUnsupportedPtyListener: vi.fn() +})) +vi.mock('../window/dashboard-popout-window', () => ({ + getDashboardPopoutWindow: () => hooks.popout +})) +vi.mock('./synthetic-title-runtime', () => ({ + driveSyntheticTitleFromHook: hooks.driveSyntheticTitleFromHook, + shouldSuppressCodexAutoApprovalSyntheticTitleFromHook: () => false, + stopAllSyntheticTitleSpinners: vi.fn() +})) + +import { installMainWindowAgentStatusListeners } from './main-window-agent-status' +import { mainProcessState } from './main-process-state' + +const sent: { channel: string; event: { paneKey: string } }[] = [] +const popoutSent: { channel: string; event: { paneKey: string } }[] = [] +const autoRenamed: { paneKey: string }[] = [] +const STRUCTURED_PANE_KEY = 'structured-agent-session-s1:leaf' + +function statusPayload( + over: Partial +): EnrichedAgentHookEventPayload { + return { + paneKey: 'pane-1', + tabId: 'tab-1', + worktreeId: 'repo::/wt', + connectionId: null, + receivedAt: 1, + stateStartedAt: 1, + payload: { state: 'working', prompt: 'ship it', agentType: 'codex' }, + ...over + } as EnrichedAgentHookEventPayload +} + +beforeEach(() => { + sent.length = 0 + popoutSent.length = 0 + autoRenamed.length = 0 + hooks.driveSyntheticTitleFromHook.mockClear() + hooks.listener = null + hooks.popout = { + webContents: { + send: (channel: string, event: unknown) => + popoutSent.push({ channel, event: event as { paneKey: string } }) + } + } + mainProcessState.runtime = null + mainProcessState.mainWindow = { + isDestroyed: () => false, + webContents: { + send: (channel: string, event: { paneKey: string }) => sent.push({ channel, event }) + } + } as unknown as typeof mainProcessState.mainWindow + installMainWindowAgentStatusListeners({ + window: mainProcessState.mainWindow!, + maybeAutoRenameBranchOnFirstWork: (event) => autoRenamed.push({ paneKey: event.paneKey }), + onRecordAgentState: vi.fn() + }) +}) + +describe('the main-window agent-status listener', () => { + it('forwards a structured row to both renderer windows, carrying its host ownership', () => { + expect(hooks.listener).not.toBeNull() + + hooks.listener!(statusPayload({ paneKey: 'hook-pane' })) + hooks.listener!(statusPayload({ paneKey: STRUCTURED_PANE_KEY, structuredHost: 'owned' })) + + expect(sent.map((entry) => `${entry.channel}:${entry.event.paneKey}`)).toEqual([ + 'agentStatus:set:hook-pane', + `agentStatus:set:${STRUCTURED_PANE_KEY}` + ]) + // The popout has never had structured sessions; removing the send guard is what gives it them. + expect(popoutSent.map((entry) => `${entry.channel}:${entry.event.paneKey}`)).toEqual([ + 'agentStatus:set:hook-pane', + `agentStatus:set:${STRUCTURED_PANE_KEY}` + ]) + expect(sent[1]?.event).toMatchObject({ structuredHost: 'owned' }) + }) + + it('drives no synthetic terminal title and no first-work rename for a paneless row', () => { + // `blocked` is a state the codex profile does synthesize a title for, so the PTY row proves + // the arm is live and the structured row proves it is skipped. + const blocked = { + payload: { state: 'blocked', prompt: 'ship it', agentType: 'codex' } + } as Partial + hooks.listener!(statusPayload({ paneKey: 'hook-pane', ...blocked })) + hooks.listener!( + statusPayload({ paneKey: STRUCTURED_PANE_KEY, structuredHost: 'held', ...blocked }) + ) + + expect(autoRenamed).toEqual([{ paneKey: 'hook-pane' }]) + expect(hooks.driveSyntheticTitleFromHook.mock.calls.map((call) => call[0])).toEqual([ + 'hook-pane' + ]) + }) +}) diff --git a/src/renderer/src/components/native-chat/StructuredAgentSessionStatusBridge.test.tsx b/src/renderer/src/components/native-chat/StructuredAgentSessionStatusBridge.test.tsx index 7edc93c6360..7454aa21a83 100644 --- a/src/renderer/src/components/native-chat/StructuredAgentSessionStatusBridge.test.tsx +++ b/src/renderer/src/components/native-chat/StructuredAgentSessionStatusBridge.test.tsx @@ -124,7 +124,10 @@ describe('StructuredAgentSessionStatusBridge', () => { mocks.supportsCapability.mockResolvedValue(true) mocks.store?.setState({ agentStatusByPaneKey: {}, - testRuntimeOwner: null, + // PR 2b: the LOCAL host publishes its own sessions over `agentStatus:set`, so the bridge + // no longer writes for them. What it still owns is a session on a remote runtime, whose + // store nothing mirrors down — that is the lane these cases drive. + testRuntimeOwner: 'env-1', unifiedTabsByWorktree: { 'wt-1': [structuredTab] } }) }) @@ -165,7 +168,7 @@ describe('StructuredAgentSessionStatusBridge', () => { it('projects the host status feed without opening a transcript reader', async () => { render() await waitFor(() => expect(mocks.subscribeStatus).toHaveBeenCalledOnce()) - expect(feed().target).toEqual({ kind: 'local' }) + expect(feed().target).toEqual({ kind: 'environment', environmentId: 'env-1' }) expect(mocks.subscribeTranscript).not.toHaveBeenCalled() act(() => feed().emit({ type: 'snapshot', sessions: [summary()] })) @@ -606,6 +609,39 @@ describe('StructuredAgentSessionStatusBridge', () => { expect(feed().target).toEqual({ kind: 'environment', environmentId: 'env-1' }) }) + // PR 2b: main publishes the row for a locally hosted session and the IPC applicator applies it. + // A second writer here would race the applicator on one pane key. + it('writes nothing and opens no feed for a locally hosted session', async () => { + mocks.store?.setState({ testRuntimeOwner: null }) + render() + await act(() => Promise.resolve()) + + expect(mocks.subscribeStatus).not.toHaveBeenCalled() + expect(mocks.setAgentStatus).not.toHaveBeenCalled() + expect(statuses()).toEqual([]) + }) + + // The host drops its own row on `agentSession.close`, but that goes through `dropStatusEntry`, + // which emits no renderer clear — so the last surface to leave is still what takes this copy + // out of the store, for a locally hosted session as much as a remote one. + it('still clears a locally hosted row when its last surface leaves', async () => { + mocks.store?.setState({ + testRuntimeOwner: null, + agentStatusByPaneKey: { + [structuredAgentSessionPaneKey('session-1')]: { + paneKey: structuredAgentSessionPaneKey('session-1') + } as AgentStatusEntry + } + }) + render() + await act(() => Promise.resolve()) + + act(() => mocks.store?.setState({ unifiedTabsByWorktree: { 'wt-1': [] } })) + + expect(mocks.removeAgentStatus).toHaveBeenCalledWith(structuredAgentSessionPaneKey('session-1')) + expect(statuses()).toEqual([]) + }) + it('does not project an unknown provider as Codex', async () => { mocks.store?.setState({ unifiedTabsByWorktree: { diff --git a/src/renderer/src/components/native-chat/StructuredAgentSessionStatusBridge.tsx b/src/renderer/src/components/native-chat/StructuredAgentSessionStatusBridge.tsx index 917f84035f8..adb1bc89ea7 100644 --- a/src/renderer/src/components/native-chat/StructuredAgentSessionStatusBridge.tsx +++ b/src/renderer/src/components/native-chat/StructuredAgentSessionStatusBridge.tsx @@ -54,21 +54,26 @@ export function getStructuredAgentSessionTabs( return tabs } +const NO_SUBSCRIPTION = (): (() => void) => () => {} + /** The host's projected status for one session, live while the caller is mounted. */ function useStructuredAgentSessionStatusSummary( sessionId: string, - target: RuntimeClientTarget + target: RuntimeClientTarget, + enabled: boolean ): { summary: AgentSessionStatusSummary | null; observation: 'live' | 'unverifiable' } { const feed = useMemo(() => getStructuredAgentSessionStatusFeed(target), [target]) - useEffect(() => feed.activate(), [feed]) + useEffect(() => (enabled ? feed.activate() : undefined), [feed, enabled]) const summary = useSyncExternalStore( - feed.subscribe, - () => feed.getSnapshot().get(sessionId) ?? null, + enabled ? feed.subscribe : NO_SUBSCRIPTION, + () => (enabled ? (feed.getSnapshot().get(sessionId) ?? null) : null), () => null ) + // Gated with the summary: a disabled bridge must open no feed at all, and an + // unsubscribed session is exactly what `unverifiable` means. const observation = useSyncExternalStore( - feed.subscribe, - () => feed.getSessionObservation(sessionId), + enabled ? feed.subscribe : NO_SUBSCRIPTION, + () => (enabled ? feed.getSessionObservation(sessionId) : ('unverifiable' as const)), () => 'unverifiable' as const ) return { summary, observation } @@ -211,14 +216,31 @@ function StructuredAgentSessionStatusProjection({ tab }: { tab: StructuredTab }) () => getActiveRuntimeTarget({ activeRuntimeEnvironmentId: environmentId }), [environmentId] ) - const { summary, observation } = useStructuredAgentSessionStatusSummary(tab.entityId, target) + // The local host publishes its own sessions into the agent-status store, and the IPC applicator + // is the single writer for them. A remote host writes into ITS store, which nothing mirrors down + // (the web-session mirror carries terminal surfaces only), so this surface stays that session's + // writer until a mirror exists — rule 3 of remote-wire-compatibility.md. + const hostPublishesRow = target.kind === 'local' + const { summary, observation } = useStructuredAgentSessionStatusSummary( + tab.entityId, + target, + !hostPublishesRow + ) useEffect(() => { + if (hostPublishesRow) { + return + } projectStatus(tab, summary, observation) - }, [summary, observation, tab]) + }, [hostPublishesRow, summary, observation, tab]) + // Kept for BOTH lanes — this is the "unmount" row of the writer disposition, not a status write. + // A projection unmounts when its tab leaves the tab map, which is a close or a host retraction; + // the host drops its own row on `agentSession.close` (`forgetStructuredAgentSession`), but that + // drop goes through `dropStatusEntry`, which emits no renderer clear, so nothing else would take + // this copy out of the store. useEffect( () => () => { const state = useAppStore.getState() - // Two surfaces can mirror one session (the `:history-N` id collision path), and they now share + // Two surfaces can mirror one session (the `:history-N` id collision path), and they share // one pane key. Only the last surface to leave clears the row. const stillMirrored = getStructuredAgentSessionTabs(state.unifiedTabsByWorktree).some( (candidate) => candidate.entityId === tab.entityId && candidate.id !== tab.id diff --git a/src/renderer/src/hooks/ipc-events/agent-status-apply-commit.ts b/src/renderer/src/hooks/ipc-events/agent-status-apply-commit.ts new file mode 100644 index 00000000000..d4e1297abeb --- /dev/null +++ b/src/renderer/src/hooks/ipc-events/agent-status-apply-commit.ts @@ -0,0 +1,71 @@ +import { isDecorativeAgentTitleFrameChange } from '../../../../shared/agent-decorative-title-signature' +import type { AgentStatusUpdate } from '@/store/slices/agent-status' +import { useAppStore } from '../../store' +import { + applyResolvedAgentTerminalTitleToTab, + shouldApplyResolvedAgentTerminalTitleToTab +} from './agent-status-routing' +import type { AgentStatusApplyOptions, AgentStatusApplyResult } from './agent-status-bridge-types' + +/** The title slots one accepted status may overwrite, resolved before the commit picks a lane. */ +export type AgentStatusTitleWrite = { + paneKey: string + ownerTabId: string | undefined + /** The TAB record's title, which is the slot this path writes; see agent-status-routing.ts. */ + tabTitle: string | undefined + terminalTitle: string | undefined + title: string | undefined + identityTitle: string | undefined + titleUsesTabTitle: boolean +} + +/** + * Commits one accepted status update, into the open batch or straight to the store. + * + * The two lanes have to stay paired: a batch stages the tab title (and the projected title a later + * event in the same batch reads) instead of writing it, and defers the completion notification to + * its post-commit phase, because a rolled-back transaction must not have notified. + */ +export function commitAgentStatusUpdate(args: { + store: ReturnType + update: AgentStatusUpdate + batch: AgentStatusApplyOptions['batch'] + notify: () => void + titleWrite: AgentStatusTitleWrite +}): AgentStatusApplyResult { + const { store, update, batch, notify, titleWrite } = args + const { paneKey, ownerTabId, tabTitle, terminalTitle, title, identityTitle } = titleWrite + if (!batch) { + store.setAgentStatus( + update.paneKey, + update.payload, + update.terminalTitle, + update.timing, + update.routing, + update.metadata + ) + applyResolvedAgentTerminalTitleToTab(useAppStore.getState(), paneKey, tabTitle, terminalTitle) + notify() + return 'applied' + } + if (!batch.transaction.apply(update)) { + return 'dropped' + } + batch.notificationEffects.push(notify) + if ( + !terminalTitle || + !ownerTabId || + !shouldApplyResolvedAgentTerminalTitleToTab(store, paneKey, tabTitle, terminalTitle) + ) { + return 'applied' + } + batch.tabTitlesByTabId.set(ownerTabId, terminalTitle) + if (titleWrite.titleUsesTabTitle) { + const titleChanges = !title || !isDecorativeAgentTitleFrameChange(title, terminalTitle) + batch.projectedTitlesByTabId.set(ownerTabId, { + title: titleChanges ? terminalTitle : title, + identityTitle: titleChanges ? terminalTitle : identityTitle + }) + } + return 'applied' +} diff --git a/src/renderer/src/hooks/ipc-events/agent-status-event-applicator.ts b/src/renderer/src/hooks/ipc-events/agent-status-event-applicator.ts index a4872822252..fe36b4c1eeb 100644 --- a/src/renderer/src/hooks/ipc-events/agent-status-event-applicator.ts +++ b/src/renderer/src/hooks/ipc-events/agent-status-event-applicator.ts @@ -4,7 +4,6 @@ import { resolveAgentStatusIdentity, shouldSuppressInheritedTerminalStatus } from '../../../../shared/agent-status-identity' -import { isDecorativeAgentTitleFrameChange } from '../../../../shared/agent-decorative-title-signature' import { parsePaneKey } from '../../../../shared/stable-pane-id' import { shouldSuppressCodexAutoApprovalStatus } from '@/components/terminal-pane/codex-auto-approval-notification-suppression' import { resolveAgentStatusTerminalTitle } from '@/lib/agent-status-terminal-title' @@ -14,12 +13,11 @@ import type { AgentStatusBatchUpdate, AgentStatusUpdate } from '@/store/slices/a import { observeAgentHookCompletionForNotification } from '../agent-hook-completion-notifications' import { useAppStore } from '../../store' import { - applyResolvedAgentTerminalTitleToTab, hasRuntimeBackedWorktreeAttribution, isAgentStatusForRecentlyClosedTab, - resolveHookPayloadAgentType, - shouldApplyResolvedAgentTerminalTitleToTab + resolveHookPayloadAgentType } from './agent-status-routing' +import { commitAgentStatusUpdate } from './agent-status-apply-commit' import { createAgentStatusPaneRoutingIndex, resolvePaneKeyFromRoutingIndex, @@ -31,6 +29,10 @@ import type { PendingAgentStatusEvent } from './agent-status-bridge-types' import { normalizeAgentStatusEvent } from './normalize-agent-status-event' +import { + resolveStructuredAgentSessionRowRouting, + structuredAgentSessionRowMetadata +} from './structured-agent-session-row-routing' export function createAgentStatusEventApplicator(args: { pendingAgentStatusEvents: PendingAgentStatusEvent[] @@ -81,6 +83,22 @@ export function createAgentStatusEventApplicator(args: { identityTitle = projectedTitles.identityTitle } tabTitle = options?.batch?.tabTitlesByTabId.get(ownerTabId ?? '') ?? tabTitle + const structuredRouting = data.structuredHost + ? resolveStructuredAgentSessionRowRouting(store, ownerTabId) + : undefined + if (structuredRouting) { + // The host owns this row outright: no pane and no connection to arbitrate. `tabTitle` is + // set to the same value so the tab-title write-back below is a no-op — a structured + // session has no terminal tab record to write into. + exists = true + owningWorktreeId = structuredRouting.worktreeId + repoConnectionId = null + repoConnectionResolved = true + title = structuredRouting.title + identityTitle = structuredRouting.title + titleUsesTabTitle = false + tabTitle = structuredRouting.title + } if (!exists && data.worktreeId && hasRuntimeBackedWorktreeAttribution(data)) { const fallbackOwnership = resolveWorktreeConnectionFromRoutingIndex( routingIndex, @@ -220,6 +238,7 @@ export function createAgentStatusEventApplicator(args: { ) { return 'dropped' } + const structuredMetadata = structuredAgentSessionRowMetadata(data) const terminalTitle = resolveAgentStatusTerminalTitle(statusPayload, title) const statusWorktreeId = data.worktreeId ?? owningWorktreeId const update: AgentStatusUpdate = { @@ -240,8 +259,9 @@ export function createAgentStatusEventApplicator(args: { ...(ownershipConnectionId !== undefined ? { connectionId: ownershipConnectionId } : {}) }, metadata: - data.providerSession || data.launchToken + data.providerSession || data.launchToken || structuredMetadata ? { + ...structuredMetadata, ...(data.providerSession ? { providerSession: data.providerSession } : {}), ...(data.launchToken ? { launchToken: data.launchToken } : {}) } @@ -261,39 +281,21 @@ export function createAgentStatusEventApplicator(args: { }) } } - if (options?.batch) { - if (!options.batch.transaction.apply(update)) { - return 'dropped' + return commitAgentStatusUpdate({ + store, + update, + batch: options?.batch, + notify: applyPostCommitNotification, + titleWrite: { + paneKey, + ownerTabId, + tabTitle, + terminalTitle, + title, + identityTitle, + titleUsesTabTitle } - options.batch.notificationEffects.push(applyPostCommitNotification) - if ( - terminalTitle && - shouldApplyResolvedAgentTerminalTitleToTab(store, paneKey, tabTitle, terminalTitle) - ) { - if (ownerTabId) { - options.batch.tabTitlesByTabId.set(ownerTabId, terminalTitle) - if (titleUsesTabTitle) { - const titleChanges = !title || !isDecorativeAgentTitleFrameChange(title, terminalTitle) - options.batch.projectedTitlesByTabId.set(ownerTabId, { - title: titleChanges ? terminalTitle : title, - identityTitle: titleChanges ? terminalTitle : identityTitle - }) - } - } - } - } else { - store.setAgentStatus( - update.paneKey, - update.payload, - update.terminalTitle, - update.timing, - update.routing, - update.metadata - ) - applyResolvedAgentTerminalTitleToTab(useAppStore.getState(), paneKey, tabTitle, terminalTitle) - applyPostCommitNotification() - } - return 'applied' + }) } return applyAgentStatus diff --git a/src/renderer/src/hooks/ipc-events/structured-agent-session-row-routing.ts b/src/renderer/src/hooks/ipc-events/structured-agent-session-row-routing.ts new file mode 100644 index 00000000000..4c73b2df6ec --- /dev/null +++ b/src/renderer/src/hooks/ipc-events/structured-agent-session-row-routing.ts @@ -0,0 +1,63 @@ +import { structuredAgentSessionTabId } from '../../../../shared/structured-agent-session-projection' +import type { AgentStatusIpcPayload } from '../../../../shared/agent-status-types' +import type { AgentStatusMetadata } from '@/store/slices/agent-status' +import type { AppState } from '../../store/types' + +/** + * Routing for a status row the host published for a structured (native chat) session. + * + * A structured session has no PTY and no terminal tab, so the terminal-tab routing index the PTY + * path uses resolves nothing for its pane key. Its surface is an `agent-session` tab in + * `unifiedTabsByWorktree`, and that tab is REQUIRED here — deliberately narrower than the + * admission rule `worktree ps` uses, which lists a session the host holds whether or not any + * surface is open: + * + * - it is what the renderer already showed. The bridge this replaces only ever wrote a row for a + * mounted `agent-session` tab, so requiring one keeps sidebar visibility unchanged; + * - a terminal tab in chat view mode is backed by a structured session too + * (`TerminalPaneNativeChatPortal`), and it already has its own pane's row. Admitting the host's + * row for that session as well would put two rows in the sidebar for one tab. + */ +export function resolveStructuredAgentSessionRowRouting( + state: Pick, + ownerTabId: string | undefined +): { worktreeId: string; title: string | undefined } | undefined { + if (!ownerTabId) { + return undefined + } + for (const [worktreeId, tabs] of Object.entries(state.unifiedTabsByWorktree ?? {})) { + for (const tab of tabs) { + if ( + tab.contentType === 'agent-session' && + // A mirrored session whose derived id was occupied is re-hosted at `${baseId}:history-N`, + // so match the id derived from the session as well as the surface's own. + (tab.id === ownerTabId || structuredAgentSessionTabId(tab.entityId) === ownerTabId) + ) { + // The chat's own name, read from this renderer's tab state — the row carries no title, + // and the sidebar's synthesized tab for a paneless row would otherwise read 'Agent'. + return { + worktreeId, + title: tab.customLabel?.trim() || tab.label?.trim() || undefined + } + } + } + } + return undefined +} + +/** Row facets that follow from the host owning this session, not from anything a pane reported. */ +export function structuredAgentSessionRowMetadata( + data: Pick +): AgentStatusMetadata | undefined { + if (!data.structuredHost) { + return undefined + } + return { + // No terminal to resume into: the session restores itself from its journal, and a resume + // record minted for it would offer to relaunch a native chat as a TUI. + terminalResumeEligible: false, + // The freshness bypass in agent-status-freshness.ts. Without it an owned session that works + // for over half an hour decays to stale while the host is still running it. + ...(data.structuredHost === 'owned' ? { structuredHostOwned: true as const } : {}) + } +} diff --git a/src/renderer/src/hooks/ipc-events/structured-agent-session-row-subscription.test.ts b/src/renderer/src/hooks/ipc-events/structured-agent-session-row-subscription.test.ts new file mode 100644 index 00000000000..91a3d65d04e --- /dev/null +++ b/src/renderer/src/hooks/ipc-events/structured-agent-session-row-subscription.test.ts @@ -0,0 +1,271 @@ +// PR 2b: main publishes a structured (native chat) session's row over `agentStatus:set`, and the +// renderer applies it instead of deriving its own. A structured pane key resolves to no terminal +// tab, so before this the applicator held every such row as `pending` forever — removing main's +// two filters on its own would have published rows the renderer then dropped on the floor. + +import { afterEach, describe, expect, it, vi } from 'vitest' +import { buildWorktreeAgentRows } from '@/components/sidebar/worktree-agent-rows' +import { isExplicitAgentStatusFresh } from '@/lib/pane-agent-evidence' +import { createTestStore } from '@/store/slices/store-test-helpers' +import type { AgentStatusEntry, AgentStatusIpcPayload } from '../../../../shared/agent-status-types' +import { AGENT_STATUS_STALE_AFTER_MS } from '../../../../shared/agent-status-types' +import { makePaneKey } from '../../../../shared/stable-pane-id' +import { + structuredAgentSessionPaneKey, + structuredAgentSessionTabId +} from '../../../../shared/structured-agent-session-projection' +import type { Tab } from '../../../../shared/tab-types' +import type { AgentStatusSetData } from '../ipc-events-agent-status-store-test-fixtures' +import { buildWindowApi } from '../ipc-events-agent-status-window-test-fixtures' + +vi.mock('../agent-hook-completion-notifications', () => ({ + observeAgentHookCompletionForNotification: vi.fn(), + syncAgentHookCompletionNotificationsForStoreUpdate: vi.fn() +})) + +const SESSION_ID = 'session-2b' +const WORKTREE_ID = 'repo-1::/wt-1' +const STRUCTURED_PANE_KEY = structuredAgentSessionPaneKey(SESSION_ID) +const PTY_PANE_KEY = makePaneKey('tab-pty', '11111111-1111-4111-8111-111111111111') + +const structuredTab: Tab = { + id: structuredAgentSessionTabId(SESSION_ID), + worktreeId: WORKTREE_ID, + groupId: 'group-1', + contentType: 'agent-session', + entityId: SESSION_ID, + label: 'Refactor the store', + customLabel: null, + color: null, + sortOrder: 0, + createdAt: 0, + isPinned: false, + agentSessionAgent: 'codex' +} + +/** Exactly the shape `main/startup/main-window-agent-status.ts` sends for a structured row. */ +function hostRow(over: Partial = {}): AgentStatusSetData { + const now = Date.now() + return { + paneKey: STRUCTURED_PANE_KEY, + tabId: structuredAgentSessionTabId(SESSION_ID), + worktreeId: WORKTREE_ID, + connectionId: null, + state: 'working', + agentType: 'codex', + prompt: 'hello', + receivedAt: now, + stateStartedAt: now, + evidenceObservedAt: now, + structuredHost: 'owned', + ...over + } as AgentStatusSetData +} + +function ptyRow(over: Partial = {}): AgentStatusSetData { + const now = Date.now() + return { + paneKey: PTY_PANE_KEY, + tabId: 'tab-pty', + worktreeId: WORKTREE_ID, + connectionId: null, + state: 'working', + agentType: 'claude', + prompt: 'pty work', + receivedAt: now, + stateStartedAt: now, + ...over + } as AgentStatusSetData +} + +async function withBridge( + run: (args: { + store: ReturnType + send: (row: AgentStatusSetData) => void + }) => Promise +): Promise { + vi.resetModules() + const store = createTestStore() + store.setState({ + workspaceSessionReady: true, + activeWorktreeId: null, + unifiedTabsByWorktree: { [WORKTREE_ID]: [structuredTab] }, + tabsByWorktree: { + [WORKTREE_ID]: [ + { + id: 'tab-pty', + ptyId: 'pty-1', + worktreeId: WORKTREE_ID, + title: 'Claude', + customTitle: null, + color: null, + sortOrder: 1, + createdAt: 0 + } + ] + }, + terminalLayoutsByTabId: { + 'tab-pty': { + root: { type: 'leaf', leafId: '11111111-1111-4111-8111-111111111111' }, + activeLeafId: '11111111-1111-4111-8111-111111111111', + expandedLeafId: null, + titlesByLeafId: {} + } + } + } as never) + let onSet: (payload: AgentStatusSetData) => void = () => { + throw new Error('listener missing') + } + vi.doMock('../../store', () => ({ useAppStore: store })) + vi.stubGlobal( + 'window', + buildWindowApi({ + getSnapshot: async () => [], + onSet: (callback) => { + onSet = callback + return () => {} + } + }) + ) + const { registerAgentStatusIpcBridge } = await import('./agent-status-ipc-bridge') + const unsubs: (() => void)[] = [] + const bridge = registerAgentStatusIpcBridge(unsubs) + try { + await run({ store, send: (row) => onSet(row) }) + } finally { + bridge.disposeAsyncState() + bridge.unsubscribeStore() + unsubs.forEach((unsubscribe) => unsubscribe()) + } +} + +afterEach(() => { + vi.doUnmock('../../store') + vi.unstubAllGlobals() + vi.restoreAllMocks() +}) + +describe('a host-published structured session row', () => { + it('lands as exactly one row, on the key the host and the renderer both derive', async () => { + await withBridge(async ({ store, send }) => { + send(hostRow()) + await vi.waitFor(() => { + expect(store.getState().agentStatusByPaneKey[STRUCTURED_PANE_KEY]).toBeDefined() + }) + + const rows = store.getState().agentStatusByPaneKey + expect(Object.keys(rows)).toEqual([STRUCTURED_PANE_KEY]) + expect(rows[STRUCTURED_PANE_KEY]).toMatchObject({ + state: 'working', + prompt: 'hello', + agentType: 'codex', + worktreeId: WORKTREE_ID, + // No terminal to resume into; without this Orca offers to relaunch the chat as a TUI. + terminalResumeEligible: false, + structuredHostOwned: true + }) + }) + }) + + it('settles to done on the same key instead of sticking on working', async () => { + await withBridge(async ({ store, send }) => { + send(hostRow()) + await vi.waitFor(() => { + expect(store.getState().agentStatusByPaneKey[STRUCTURED_PANE_KEY]?.state).toBe('working') + }) + send(hostRow({ state: 'done', receivedAt: Date.now() + 10 })) + await vi.waitFor(() => { + expect(store.getState().agentStatusByPaneKey[STRUCTURED_PANE_KEY]?.state).toBe('done') + }) + + expect(Object.keys(store.getState().agentStatusByPaneKey)).toEqual([STRUCTURED_PANE_KEY]) + }) + }) + + // The bypass in shared/agent-status-freshness.ts fires on `structuredHostOwned`, which the wire + // spells `structuredHost: 'owned'`. Lose the mapping and every native chat that works for over + // half an hour silently decays to idle in the sidebar. + it('stays fresh past the 30-minute window while the host owns it', async () => { + await withBridge(async ({ store, send }) => { + const observedAt = Date.now() - AGENT_STATUS_STALE_AFTER_MS - 1 + send(hostRow({ evidenceObservedAt: observedAt })) + await vi.waitFor(() => { + expect(store.getState().agentStatusByPaneKey[STRUCTURED_PANE_KEY]).toBeDefined() + }) + + const entry = store.getState().agentStatusByPaneKey[STRUCTURED_PANE_KEY] + expect(entry.structuredHostOwned).toBe(true) + expect(isExplicitAgentStatusFresh(entry, Date.now(), AGENT_STATUS_STALE_AFTER_MS)).toBe(true) + }) + }) + + it('drops the ownership flag once the host revokes live execution', async () => { + await withBridge(async ({ store, send }) => { + send(hostRow()) + await vi.waitFor(() => { + expect(store.getState().agentStatusByPaneKey[STRUCTURED_PANE_KEY]).toBeDefined() + }) + send(hostRow({ structuredHost: 'held', receivedAt: Date.now() + 10 })) + await vi.waitFor(() => { + expect( + store.getState().agentStatusByPaneKey[STRUCTURED_PANE_KEY]?.structuredHostOwned + ).toBeUndefined() + }) + }) + }) + + // The wire carries no title (deliberately: naming a structured session is its own lane), so the + // row's name has to come from this renderer's own tab state. The sidebar synthesizes a tab for a + // paneless row and would otherwise name it 'Agent'. + it('names its sidebar row from the session tab, not the wire', async () => { + await withBridge(async ({ store, send }) => { + send(hostRow()) + await vi.waitFor(() => { + expect(store.getState().agentStatusByPaneKey[STRUCTURED_PANE_KEY]).toBeDefined() + }) + + const entry = store.getState().agentStatusByPaneKey[STRUCTURED_PANE_KEY] + const rows = buildWorktreeAgentRows({ + tabs: [], + entries: [entry as AgentStatusEntry], + retained: [], + now: Date.now() + }) + + expect(rows).toHaveLength(1) + expect(rows[0].tab.title).toBe('Refactor the store') + expect(rows[0].paneKey).toBe(STRUCTURED_PANE_KEY) + }) + }) + + // A terminal tab in chat view mode is backed by a structured session too, and its own pane + // already has a row. Admitting the host's row for that session as well puts two rows in the + // sidebar for one tab — so the renderer requires an `agent-session` surface, which is narrower + // than the admission rule `worktree ps` uses and is exactly what the sidebar showed before. + it('does not admit a session with no agent-session surface in this renderer', async () => { + await withBridge(async ({ store, send }) => { + store.setState({ unifiedTabsByWorktree: { [WORKTREE_ID]: [] } } as never) + send(hostRow()) + send(ptyRow()) + await vi.waitFor(() => { + expect(store.getState().agentStatusByPaneKey[PTY_PANE_KEY]).toBeDefined() + }) + + expect(Object.keys(store.getState().agentStatusByPaneKey)).toEqual([PTY_PANE_KEY]) + }) + }) + + it('leaves a PTY row untouched', async () => { + await withBridge(async ({ store, send }) => { + send(ptyRow()) + await vi.waitFor(() => { + expect(store.getState().agentStatusByPaneKey[PTY_PANE_KEY]).toBeDefined() + }) + + const entry = store.getState().agentStatusByPaneKey[PTY_PANE_KEY] + expect(entry).toMatchObject({ state: 'working', agentType: 'claude' }) + expect(entry.structuredHostOwned).toBeUndefined() + expect(entry.terminalResumeEligible).toBeUndefined() + }) + }) +})