From ab41610ba69446044f60e362edc0eda4e9756191 Mon Sep 17 00:00:00 2001 From: Neil <4138956+nwparker@users.noreply.github.com> Date: Sun, 4 Oct 2026 14:24:50 -0700 Subject: [PATCH 1/8] Fix reordering workspaces with collapsed children (#25302) --- .../visible-worktree-options-from-state.ts | 3 +- ...ble-worktrees-manual-lineage-order.test.ts | 84 ++++++++++++ .../components/sidebar/visible-worktrees.ts | 7 +- .../listing/use-visible-worktrees.ts | 2 + tests/e2e/worktree-parent-reorder.spec.ts | 128 ++++++++++++++++++ 5 files changed, 222 insertions(+), 2 deletions(-) create mode 100644 src/renderer/src/components/sidebar/visible-worktrees-manual-lineage-order.test.ts create mode 100644 tests/e2e/worktree-parent-reorder.spec.ts diff --git a/src/renderer/src/components/sidebar/visible-worktree-options-from-state.ts b/src/renderer/src/components/sidebar/visible-worktree-options-from-state.ts index d9a09639cdf..9ab318e26a3 100644 --- a/src/renderer/src/components/sidebar/visible-worktree-options-from-state.ts +++ b/src/renderer/src/components/sidebar/visible-worktree-options-from-state.ts @@ -48,6 +48,7 @@ export function buildVisibleWorktreeOptionsFromState( workspaceHostScope: state.workspaceHostScope, visibleWorkspaceHostIds: state.visibleWorkspaceHostIds, defaultHostId: getSettingsFocusedExecutionHostId(state.settings), - worktreeLineageById: state.worktreeLineageById + worktreeLineageById: state.worktreeLineageById, + preserveLineageParentOrder: state.sortBy === 'manual' } } diff --git a/src/renderer/src/components/sidebar/visible-worktrees-manual-lineage-order.test.ts b/src/renderer/src/components/sidebar/visible-worktrees-manual-lineage-order.test.ts new file mode 100644 index 00000000000..743f4acc7be --- /dev/null +++ b/src/renderer/src/components/sidebar/visible-worktrees-manual-lineage-order.test.ts @@ -0,0 +1,84 @@ +import { describe, expect, it } from 'vitest' +import type { ExecutionHostId } from '../../../../shared/execution-host' +import type { WorktreeLineage } from '../../../../shared/worktree/lineage-types' +import { worktree, repoMap } from './worktree-list-groups-test-fixtures' +import { computeVisibleWorktrees, type VisibleWorktreeOptions } from './visible-worktrees' +import { buildRows } from './worktree-list/grouping/build-rows' + +function scenario(hostId: ExecutionHostId) { + const parent = { ...worktree, id: 'parent', instanceId: 'parent-instance', hostId } + const child = { ...worktree, id: 'child', instanceId: 'child-instance', hostId } + const sibling = { ...worktree, id: 'sibling', instanceId: 'sibling-instance', hostId } + const lineage: WorktreeLineage = { + worktreeId: child.id, + worktreeInstanceId: child.instanceId, + parentWorktreeId: parent.id, + parentWorktreeInstanceId: parent.instanceId, + origin: 'manual', + capture: { source: 'manual-action', confidence: 'explicit' }, + createdAt: 1 + } + const options: VisibleWorktreeOptions = { + filterRepoIds: [], + showSleepingWorkspaces: true, + tabsByWorktree: {}, + ptyIdsByTabId: {}, + worktreeIdsWithLiveAgent: new Set(), + hideDefaultBranchWorkspace: false, + hideAutomationGeneratedWorkspaces: false, + hideCliCreatedWorkspaces: false, + hideDetachedHeadWorkspaces: false, + hideWorkspacesFromOtherDevices: false, + pairedDeviceIdsByEnvironment: new Map(), + repoMap, + workspaceHostScope: 'all', + defaultHostId: 'local', + worktreeLineageById: { [child.id]: lineage } + } + const worktreesByRepo = { [worktree.repoId]: [parent, child, sibling] } + const sortedIds = [child.id, sibling.id, parent.id] + function renderedIds(overrides: Partial = {}): string[] { + const visible = computeVisibleWorktrees(worktreesByRepo, sortedIds, { + ...options, + ...overrides + }) + return buildRows( + 'none', + visible, + repoMap, + {}, + new Set(), + new Map(), + [], + undefined, + options.worktreeLineageById, + new Map([parent, child, sibling].map((row) => [row.id, row])), + true + ).flatMap((row) => (row.type === 'item' ? [row.worktree.id] : [])) + } + return { renderedIds } +} + +describe.each(['local', 'ssh:remote'] as const)('manual lineage order on %s', (hostId) => { + it('keeps a parent below its neighbor despite higher-ranked children', () => { + expect(scenario(hostId).renderedIds({ preserveLineageParentOrder: true })).toEqual([ + 'sibling', + 'parent', + 'child' + ]) + }) + + it('keeps a filtered structural parent in its manual position', () => { + expect( + scenario(hostId).renderedIds({ + preserveLineageParentOrder: true, + showSleepingWorkspaces: false, + worktreeIdsWithLiveAgent: new Set(['child', 'sibling']) + }) + ).toEqual(['sibling', 'parent', 'child']) + }) + + it('continues promoting a parent with its children for automatic sorts', () => { + expect(scenario(hostId).renderedIds()).toEqual(['parent', 'child', 'sibling']) + }) +}) diff --git a/src/renderer/src/components/sidebar/visible-worktrees.ts b/src/renderer/src/components/sidebar/visible-worktrees.ts index 37fb31890fb..581f2e1fef2 100644 --- a/src/renderer/src/components/sidebar/visible-worktrees.ts +++ b/src/renderer/src/components/sidebar/visible-worktrees.ts @@ -85,6 +85,7 @@ export type VisibleWorktreeOptions = { defaultHostId: ExecutionHostId worktreeLineageById: Record injectLineageAncestors?: boolean + preserveLineageParentOrder?: boolean forcedVisibleWorktreeIds?: readonly string[] } @@ -175,6 +176,10 @@ export function computeVisibleWorktrees( // Apply cached sort order. Items not yet in the cache (e.g. brand-new // worktrees before the next sortEpoch bump) are appended at the end. + // Manual placement belongs to the parent, even when a hidden child has a higher rank. + if (opts.injectLineageAncestors !== false && opts.preserveLineageParentOrder) { + all = addVisibleLineageAncestors(all, lineageAncestorById, opts.worktreeLineageById) + } const orderIndex = getSortedWorktreeRankIndex(sortedIds) all.sort((a, b) => { const ai = orderIndex.get(a.id) ?? Infinity @@ -182,7 +187,7 @@ export function computeVisibleWorktrees( return ai - bi }) - return opts.injectLineageAncestors === false + return opts.injectLineageAncestors === false || opts.preserveLineageParentOrder ? all : addVisibleLineageAncestors(all, lineageAncestorById, opts.worktreeLineageById) } diff --git a/src/renderer/src/components/sidebar/worktree-list/listing/use-visible-worktrees.ts b/src/renderer/src/components/sidebar/worktree-list/listing/use-visible-worktrees.ts index 1990fad3606..6b42ea04b22 100644 --- a/src/renderer/src/components/sidebar/worktree-list/listing/use-visible-worktrees.ts +++ b/src/renderer/src/components/sidebar/worktree-list/listing/use-visible-worktrees.ts @@ -106,6 +106,7 @@ export function useVisibleSidebarWorktrees(args: { visibleWorkspaceHostIds, defaultHostId, worktreeLineageById, + preserveLineageParentOrder: sortBy === 'manual', forcedVisibleWorktreeIds: args.agentSendTargetWorktreeId ? [args.agentSendTargetWorktreeId] : undefined @@ -130,6 +131,7 @@ export function useVisibleSidebarWorktrees(args: { ptyIdsByTabId, browserTabsByWorktree, sortedIds, + sortBy, worktreeLineageById, worktreesByRepo, pairedDeviceIdsByEnvironment, diff --git a/tests/e2e/worktree-parent-reorder.spec.ts b/tests/e2e/worktree-parent-reorder.spec.ts new file mode 100644 index 00000000000..21f96f0c0d3 --- /dev/null +++ b/tests/e2e/worktree-parent-reorder.spec.ts @@ -0,0 +1,128 @@ +import { test, expect } from './helpers/orca-app' +import { waitForSessionReady } from './helpers/store' + +for (const newCardStyle of [false, true]) { + test(`reorders a collapsed parent with 53 children (${newCardStyle ? 'new' : 'legacy'} cards)`, async ({ + orcaPage + }, testInfo) => { + await waitForSessionReady(orcaPage) + const ids = await orcaPage.evaluate(async (newCardStyle) => { + const store = window.__store + if (!store) { + throw new Error('Missing app store') + } + const state = store.getState() + const repo = state.repos[0]! + const template = state.worktreesByRepo[repo.id]![0]! + const makeRow = (name: string, rank: number) => ({ + ...template, + id: `${repo.id}::${name}`, + instanceId: name, + displayName: name, + branch: `refs/heads/${name}`, + isMainWorktree: false, + isPinned: false, + isArchived: false, + parentWorktreeId: null, + childWorktreeIds: [], + lineage: null, + manualOrder: rank, + sortOrder: rank + }) + const first = makeRow('First workspace', 100_000) + const parent = makeRow('Parent with 53 children', 90_000) + const last = makeRow('Last workspace', 10_000) + const children = Array.from({ length: 53 }, (_, index) => { + const child = makeRow(`Child ${index}`, 80_000 - index * 100) + const lineage = { + worktreeId: child.id, + worktreeInstanceId: child.instanceId, + parentWorktreeId: parent.id, + parentWorktreeInstanceId: parent.instanceId, + origin: 'manual' as const, + capture: { source: 'manual-action' as const, confidence: 'explicit' as const }, + createdAt: Date.now() + } + return { ...child, parentWorktreeId: parent.id, lineage } + }) + const collapsedGroups = [ + `lineage:${parent.hostId ? `${parent.hostId}|${parent.id}` : parent.id}` + ] + await window.api.ui.set({ groupBy: 'none', collapsedGroups }) + store.setState({ + groupBy: 'none', + sortBy: 'manual', + sidebarOpen: true, + settings: { ...state.settings, experimentalNewWorktreeCardStyle: newCardStyle }, + showActiveOnly: false, + showSleepingWorkspaces: true, + hideDefaultBranchWorkspace: false, + filterRepoIds: [], + collapsedGroups: new Set(collapsedGroups), + worktreesByRepo: { [repo.id]: [first, parent, ...children, last] }, + worktreeLineageById: Object.fromEntries(children.map((child) => [child.id, child.lineage])), + // Synthetic rows exercise the real drag path without persisting nonexistent checkouts. + updateWorktreesMeta: async (updates) => { + const byId = new Map(updates.map((update) => [update.worktreeId, update.updates])) + store.setState((current) => ({ + sortEpoch: current.sortEpoch + 1, + worktreesByRepo: Object.fromEntries( + Object.entries(current.worktreesByRepo).map(([repoId, rows]) => [ + repoId, + rows.map((row) => ({ ...row, ...byId.get(row.id) })) + ]) + ) + })) + } + }) + return { first: first.id, parent: parent.id, last: last.id } + }, newCardStyle) + const sidebar = orcaPage.locator('[data-worktree-sidebar]') + const parent = sidebar.locator(`[data-worktree-id=${JSON.stringify(ids.parent)}]`) + const last = sidebar.locator(`[data-worktree-id=${JSON.stringify(ids.last)}]`) + await expect(parent.getByRole('button', { name: 'Show 53 child workspaces' })).toBeVisible() + await sidebar.screenshot({ path: testInfo.outputPath('before.png') }) + const source = await parent.boundingBox() + const target = await last.boundingBox() + if (!source || !target) { + throw new Error('Missing card bounds') + } + await orcaPage.mouse.move(source.x + source.width / 2, source.y + 12) + await orcaPage.mouse.down() + await orcaPage.mouse.move(target.x + 2, target.y + target.height - 1, { steps: 12 }) + await expect(orcaPage.locator('[data-worktree-sidebar-drag-preview]')).toHaveCount(1) + await orcaPage.mouse.up() + try { + await expect + .poll(async () => { + const [parentBox, lastBox] = await Promise.all([parent.boundingBox(), last.boundingBox()]) + return Boolean(parentBox && lastBox && parentBox.y > lastBox.y) + }) + .toBe(true) + } finally { + await sidebar.screenshot({ path: testInfo.outputPath('after.png') }) + } + const first = sidebar.locator(`[data-worktree-id=${JSON.stringify(ids.first)}]`) + const movedSource = await parent.boundingBox() + const upwardTarget = await first.boundingBox() + if (!movedSource || !upwardTarget) { + throw new Error('Missing reordered card bounds') + } + await orcaPage.mouse.move(movedSource.x + movedSource.width / 2, movedSource.y + 12) + await orcaPage.mouse.down() + await orcaPage.mouse.move(upwardTarget.x + 2, upwardTarget.y + 1, { steps: 12 }) + await orcaPage.mouse.up() + await expect + .poll(async () => { + const [parentBox, firstBox] = await Promise.all([parent.boundingBox(), first.boundingBox()]) + return Boolean(parentBox && firstBox && parentBox.y < firstBox.y) + }) + .toBe(true) + // Wait for the drag's click suppression before exercising the children toggle. + await orcaPage.waitForTimeout(550) + await parent.getByRole('button', { name: 'Show 53 child workspaces' }).click() + await expect(parent.getByRole('button', { name: 'Hide 53 child workspaces' })).toBeVisible() + await expect(parent.locator('[role="option"]')).toHaveCount(53) + await expect(parent.locator('[role="option"]').first()).toContainText('Child 0') + }) +} From 97fa6aee746d5923962b3801b3c7274a61059e9a Mon Sep 17 00:00:00 2001 From: Brennan Benson <79079362+brennanb2025@users.noreply.github.com> Date: Sun, 4 Oct 2026 14:28:10 -0700 Subject: [PATCH 2/8] fix(native-chat): show a message Orca accepted and then failed to deliver as "Not sent" in the chat (#24710) * fix(native-chat): keep a message the host accepted then rejected in the desktop chat as not sent Draw it in place from the host's history, so a crash that loses the outbox no longer makes it vanish. A later copy of the same body supersedes it; the outbox row wins while it holds the message; the phone is unchanged. * test(native-chat): pin the same-id rule apart from the body match * test(native-chat): type the rejected-in-place fixture body as a text block * fix(native-chat): let the host's row own a message it recorded and then rejected Once the host's journal records a send as rejected, the desktop outbox lets it go, as it already does for delivered and Stop-withdrawn sends: the host's row shows it as not sent, with the host's reason and no Retry. The outbox keeps only sends the host refused before recording them, which keep their Retry. A send whose own reply says it was rejected is drawn by its outbox entry, with no Retry, until the journal carries the row; a copy left by an earlier session is dropped when the chat opens. - the transcript no longer hides a host row behind an outbox entry with the same id or the same text; those rules and their cache are gone - a rejected message the queue holds (a draft's hand-off, or a live card under its id) is drawn as its card, not as a row - a later copy of the same text hides a rejected row only when it was sent once the rejection was known, so a deliberate repeat stays - delivery notices read the same visibility rule as the transcript; a chat whose only rejection a Stop withdrew no longer rebuilds them per batch - the body fingerprint helper goes back to the host, its only user * test(native-chat): keep one row when copies of a rejected message share an instant * refactor(native-chat): let the host's notice replace the outbox's under the same id * test(native-chat): pass the queued card ids in the tool-stream cost transcript * fix(native-chat): keep the host's record as what lets a rejected message go - the outbox no longer drops a host-rejected message when a chat opens; the reconcile lets it go once the journal's submissions say it was rejected, and that drop is written to storage, so nothing reads as still owed - a message the host rejected while the chat watched waits for its journal row with no Retry; one read back from storage with no row loaded keeps its Retry under a new id, since the host may have lost it - the delivery notices keep the same map and notice objects across a batch that words every row the same, so a submission batch re-renders no row - a rejected command such as /compact stays hidden: its own reply reports it - the desktop transcript requires the queued card ids, with a controller-level test that a card holding a rejected message keeps its row hidden * fix(native-chat): draw a queued message where the host rejected it A message accepted to hand over later and rejected before any handover now sits at its rejection, as a handover places one: what the agent did while it waited happened before it, and the newest history page holds it. One handed over, or dispatched as it was recorded, keeps its place. An older host does not move it, so it stays at its submission, still drawn. A failed start now rejects the queued messages and writes its row in ONE journal append, the messages first: no reader ever meets one without the other, and the messages still draw above the row that says why. * test(native-chat): pin that rows written together roll back together * fix(native-chat): draw every rejected message where it was rejected Not only a queued message: one handed over into a turn and then rejected, or sent directly and rejected, also sits at its rejection, in no turn. A message in doubt stays where it was, a plain bubble: it may have reached the agent. * fix(native-chat): decide a rejected message's Retry from the host's stored fact - a message the host recorded and then rejected has no Retry on any mount, however that mount learned of it, and a Dismiss that clears it from storage; a send refused before the host recorded it keeps its Retry - the rule that keeps a rejected command such as /compact out of the transcript moves into the one visibility function rows and notices share - the outbox state docs say what lets a recorded message go: the client holding its rejected submission, whose row the host places at the rejection * fix(native-chat): write no start-failure row when a Stop withdrew every queued message first * test(native-chat): pass the Dismiss action in the delivery-notice hook tests * fix(native-chat): keep a rejected message's outbox copy until its row loads An older host leaves a rejected message where it was sent, which may be older than the loaded window: the chat then holds the rejected submission but not the row that draws it. The outbox copy now stays until that row loads, marked as the host recorded it (Dismiss, no Retry, in the host's words), and leaves once the page holding the row is loaded. Derived from the loaded rows each time. Tests that label their projection as the phone's now pass the phone's own setting. * test(native-chat): type the outbox hook props that carry loaded rows * fix(native-chat): write nothing when a journal batch settles nothing in the outbox The outbox re-reads the journal on every batch since it waits for a rejected message's row to load. Its reconcile now returns each unchanged entry, and the list, as themselves (a message left in doubt included), so a batch that changes nothing writes nothing to storage. The reconcile moves to its own module. A copy the host recorded and rejected owes no delivery, so it no longer keeps a hidden pane reading the journal. * test(native-chat): count storage writes on the outbox's own storage object * fix(native-chat): let a recorded rejected message's outbox copy leave on its own, with no Dismiss The outbox copy of a message the host recorded and then rejected draws it only while the host's row is not loaded, and leaves on the batch or page that loads that row. It owes no delivery and offers no control: sending it again is a new message. The Dismiss that let the user clear it is gone, from the outbox, the notices and the session controller. --- .../use-mobile-structured-agent-session.ts | 6 +- .../codex-collab-call-delegation.test.ts | 4 +- ...ab-call-rows-on-positional-clients.test.ts | 4 +- .../codex-structured-question-order.test.ts | 7 +- .../journal-dispatch-reducer.ts | 8 +- .../journal-lifecycle-batch-appender.ts | 29 ++ .../journal-pending-submission-recovery.ts | 21 + .../journal-queued-rejection-batch.test.ts | 129 +++++ .../journal-reducer.test.ts | 5 +- .../agent-session-journal/journal-reducer.ts | 6 +- ...journal-rejected-message-placement.test.ts | 158 ++++++ .../journal-row-writer.test.ts | 13 + .../journal-row-writer.ts | 34 ++ .../journal-store-collaborators.ts | 32 +- .../journal-store-contracts.ts | 4 + .../journal-submission-fold.ts | 22 + .../journal-turn-scope.test.ts | 4 +- ...d-agent-session-conversation-close.test.ts | 12 +- ...structured-agent-session-host-test-data.ts | 4 +- ...uctured-agent-session-start-failure-row.ts | 13 +- ...atMessageList.provider-retry-runs.test.tsx | 4 +- ...hatMessageList.stream-render.perf.test.tsx | 4 +- ...eChatMessageList.tool-stream-cost.test.tsx | 2 +- ...veChatMessageList.turn-membership.test.tsx | 4 +- ...iveChatMessageList.unsent-message.test.tsx | 94 +++- ...turedSession.start-failure-notice.test.tsx | 13 +- ...tiveChatStructuredSession.test-harness.tsx | 5 +- .../NativeChatStructuredSession.tsx | 51 +- ...tiveChatStructuredSessionDelivery.test.tsx | 73 ++- .../native-chat-rail-outline-parity.test.ts | 11 +- ...red-agent-session-delivery-notices.test.ts | 93 ++-- ...ructured-agent-session-delivery-notices.ts | 77 ++- ...sion-message-projection.older-host.test.ts | 161 ++++++ ...n-message-projection.recorded-copy.test.ts | 218 ++++++++ ...ssage-projection.rejected-in-place.test.ts | 465 ++++++++++++++++++ ...d-agent-session-message-projection.test.ts | 35 +- ...ctured-agent-session-message-projection.ts | 14 +- .../structured-agent-session-outbox-retry.ts | 3 +- ...structured-agent-session-outbox-storage.ts | 12 +- ...red-agent-session-transcript-order.test.ts | 4 +- ...session-withdrawn-message-restore.test.tsx | 27 +- ...ed-agent-session-delivery-notices.test.tsx | 120 +++++ ...ructured-agent-session-delivery-notices.ts | 120 +++++ ...structured-agent-session-messages.test.tsx | 18 +- .../use-structured-agent-session-messages.ts | 7 +- ...ed-agent-session-outbox-admission.test.tsx | 10 +- ...ctured-agent-session-outbox-fence.test.tsx | 5 + ...agent-session-outbox-owner-change.test.tsx | 5 + ...nt-session-outbox-rejection-cause.test.tsx | 322 +++++++++--- ...gent-session-outbox-relaunch-hold.test.tsx | 7 + ...d-agent-session-outbox-withdrawal.test.tsx | 9 +- ...-agent-session-outbox.batch-churn.test.tsx | 117 +++++ ...ent-session-outbox.draft-hand-off.test.tsx | 8 +- ...gent-session-outbox.external-send.test.tsx | 5 + ...ent-session-outbox.queue-delivery.test.tsx | 9 + ...ent-session-outbox.rejected-reply.test.tsx | 102 ++++ ...ession-outbox.stop-parks-in-doubt.test.tsx | 7 + ...e-structured-agent-session-outbox.test.tsx | 129 ++--- .../use-structured-agent-session-outbox.ts | 22 +- ...tured-agent-session.rejected-card.test.tsx | 116 +++++ .../use-structured-agent-session.ts | 6 +- .../agent-session-conversation-outline.ts | 3 +- .../native-chat-provider-retry-runs.test.ts | 8 +- src/shared/native-chat-types.ts | 4 +- ...tured-agent-session-draft-hand-off.test.ts | 4 +- ...structured-agent-session-draft-hand-off.ts | 21 +- ...ctured-agent-session-message-projection.ts | 119 ++++- ...red-agent-session-outbox-reconcile.test.ts | 100 ++++ ...ructured-agent-session-outbox-reconcile.ts | 69 +++ ...ed-agent-session-outbox-retry-hold.test.ts | 8 +- src/shared/structured-agent-session-outbox.ts | 68 +-- ...red-agent-session-send-disposition.test.ts | 10 +- ...ructured-agent-session-send-disposition.ts | 6 +- 73 files changed, 2959 insertions(+), 490 deletions(-) create mode 100644 src/main/native-chat/agent-session-journal/journal-queued-rejection-batch.test.ts create mode 100644 src/main/native-chat/agent-session-journal/journal-rejected-message-placement.test.ts create mode 100644 src/renderer/src/components/native-chat/structured-agent-session-message-projection.older-host.test.ts create mode 100644 src/renderer/src/components/native-chat/structured-agent-session-message-projection.recorded-copy.test.ts create mode 100644 src/renderer/src/components/native-chat/structured-agent-session-message-projection.rejected-in-place.test.ts create mode 100644 src/renderer/src/components/native-chat/use-structured-agent-session-delivery-notices.test.tsx create mode 100644 src/renderer/src/components/native-chat/use-structured-agent-session-delivery-notices.ts create mode 100644 src/renderer/src/components/native-chat/use-structured-agent-session-outbox.batch-churn.test.tsx create mode 100644 src/renderer/src/components/native-chat/use-structured-agent-session-outbox.rejected-reply.test.tsx create mode 100644 src/renderer/src/components/native-chat/use-structured-agent-session.rejected-card.test.tsx create mode 100644 src/shared/structured-agent-session-outbox-reconcile.test.ts create mode 100644 src/shared/structured-agent-session-outbox-reconcile.ts diff --git a/mobile/src/session/use-mobile-structured-agent-session.ts b/mobile/src/session/use-mobile-structured-agent-session.ts index 653d7fbaaa1..ae688ab9072 100644 --- a/mobile/src/session/use-mobile-structured-agent-session.ts +++ b/mobile/src/session/use-mobile-structured-agent-session.ts @@ -158,7 +158,11 @@ export function useMobileStructuredAgentSession(args: { }) const messages = useMemo( - () => projectStructuredAgentSessionMessages(state.items, [], state.submissions), + // Off: the phone hands a rejected message back to its composer, so a row would show it twice. + () => + projectStructuredAgentSessionMessages(state.items, [], state.submissions, { + rejectedInPlace: false + }), [state.items, state.submissions] ) const turnId = activeStructuredAgentSessionTurnId(state.items) diff --git a/src/main/codex/codex-collab-call-delegation.test.ts b/src/main/codex/codex-collab-call-delegation.test.ts index 96131248ddc..dbc116802e4 100644 --- a/src/main/codex/codex-collab-call-delegation.test.ts +++ b/src/main/codex/codex-collab-call-delegation.test.ts @@ -28,7 +28,9 @@ import { THREAD_ID } from './codex-structured-session-adapter-fixture' /** The delegation the parent's newest tool run reads as, the row a running chat's frontier judges. */ async function newestRunDelegation(frames: Frame[]): Promise { const { conversation } = projectNativeChatTranscript( - projectStructuredAgentSessionMessages(await publishedRows(frames), [], []) + projectStructuredAgentSessionMessages(await publishedRows(frames), [], [], { + rejectedInPlace: true + }) ) const runs = conversation.filter((message: NativeChatMessage) => message.blocks.some((block) => block.type === 'tool-call') diff --git a/src/main/codex/codex-collab-call-rows-on-positional-clients.test.ts b/src/main/codex/codex-collab-call-rows-on-positional-clients.test.ts index 795716ed553..ae7770b5beb 100644 --- a/src/main/codex/codex-collab-call-rows-on-positional-clients.test.ts +++ b/src/main/codex/codex-collab-call-rows-on-positional-clients.test.ts @@ -47,7 +47,9 @@ function positionalRuns(rows: AgentJournalRenderItem[]): { }) }) const transcript = projectNativeChatTranscript( - projectStructuredAgentSessionMessages(rows, [], []).map(withoutCallIds) + projectStructuredAgentSessionMessages(rows, [], [], { rejectedInPlace: true }).map( + withoutCallIds + ) ) // The conversation's runs, then each helper's section's. const runs = [ diff --git a/src/main/codex/codex-structured-question-order.test.ts b/src/main/codex/codex-structured-question-order.test.ts index 8ad4b75bd38..78c197644e2 100644 --- a/src/main/codex/codex-structured-question-order.test.ts +++ b/src/main/codex/codex-structured-question-order.test.ts @@ -206,6 +206,7 @@ function drawnPromptRows(): string[][] { client.items, [], client.submissions, + { rejectedInPlace: true }, projectStructuredQuestionMessages ) ) @@ -243,9 +244,9 @@ describe('a Codex ask with several questions', () => { ]) // Mobile draws the shared projection in journal order, one row per question. expect( - projectStructuredAgentSessionMessages(client.items, [], client.submissions).map( - ({ blocks }) => (blocks[0]?.type === 'text' ? blocks[0].text.split('\n')[0] : null) - ) + projectStructuredAgentSessionMessages(client.items, [], client.submissions, { + rejectedInPlace: false + }).map(({ blocks }) => (blocks[0]?.type === 'text' ? blocks[0].text.split('\n')[0] : null)) ).toEqual(ASKED.map(({ question }) => question)) }) diff --git a/src/main/native-chat/agent-session-journal/journal-dispatch-reducer.ts b/src/main/native-chat/agent-session-journal/journal-dispatch-reducer.ts index 89722959799..c7c2a671654 100644 --- a/src/main/native-chat/agent-session-journal/journal-dispatch-reducer.ts +++ b/src/main/native-chat/agent-session-journal/journal-dispatch-reducer.ts @@ -8,7 +8,11 @@ import { import { agentJournalSubmissionKey } from '../../../shared/agent-session-journal-item-key' import { journalDispatchRowApplies } from './journal-dispatch-settlement' import type { JournalReducerState } from './journal-reducer' -import { notePersonTurnAccepted, placeHandedOverMessage } from './journal-submission-fold' +import { + notePersonTurnAccepted, + placeHandedOverMessage, + placeRejectedMessage +} from './journal-submission-fold' import type { JournalRow } from './journal-row-schema' export function applyJournalDispatchRow( @@ -34,6 +38,8 @@ export function applyJournalDispatchRow( if (row.state === 'pending') { submission.handedOverAt = row.ts placeHandedOverMessage(state, submission, row) + } else if (row.state === 'rejected') { + placeRejectedMessage(state, submission, row) } if (row.recovered) { submission.recovered = row.recovered diff --git a/src/main/native-chat/agent-session-journal/journal-lifecycle-batch-appender.ts b/src/main/native-chat/agent-session-journal/journal-lifecycle-batch-appender.ts index c05d22a8745..68ba188e402 100644 --- a/src/main/native-chat/agent-session-journal/journal-lifecycle-batch-appender.ts +++ b/src/main/native-chat/agent-session-journal/journal-lifecycle-batch-appender.ts @@ -3,6 +3,7 @@ import type { JournalReducerState } from './journal-reducer' import { journalLifecycleBatchRowBuilder } from './journal-row-builders' import type { JournalLifecycleBatchInput } from './journal-store-contracts' import type { JournalRow } from './journal-row-schema' +import { journalQueuedRejectionRowBuilders } from './journal-pending-submission-recovery' const SETTLEMENT_ALREADY_APPLIED = new Error('journal_settlement_already_applied') @@ -12,10 +13,38 @@ export class JournalLifecycleBatchAppender { state: () => JournalReducerState cursor: () => AgentJournalCursor enqueue: (build: (seq: number, ts: number) => JournalRow) => Promise + enqueueRows: ( + plan: () => readonly ((seq: number, ts: number) => JournalRow)[] + ) => Promise } ) {} append(input: JournalLifecycleBatchInput): Promise { + const { rejectsQueued } = input + if (rejectsQueued) { + // Planned on the lane: the sends queued then, and this batch unless it already landed. With + // none left (a Stop withdrew them first) it failed no one, so nothing is written. + return this.deps + .enqueueRows(() => { + const rejections = journalQueuedRejectionRowBuilders( + this.deps.state, + input.fence, + rejectsQueued + ) + return rejections.length === 0 || this.wasApplied(input.settlementId) + ? rejections + : [ + ...rejections, + journalLifecycleBatchRowBuilder( + this.deps.state, + input.settlementId, + input.mutations, + input + ) + ] + }) + .then(() => this.deps.cursor()) + } if (this.wasApplied(input.settlementId)) { return Promise.resolve(this.deps.cursor()) } diff --git a/src/main/native-chat/agent-session-journal/journal-pending-submission-recovery.ts b/src/main/native-chat/agent-session-journal/journal-pending-submission-recovery.ts index 964ea42f931..b52e3648b72 100644 --- a/src/main/native-chat/agent-session-journal/journal-pending-submission-recovery.ts +++ b/src/main/native-chat/agent-session-journal/journal-pending-submission-recovery.ts @@ -2,6 +2,9 @@ import type { AgentJournalDispatchRejection } from '../../../shared/agent-sessio import type { AgentJournalSubmission } from '../../../shared/agent-session-journal-types' import { isQueuedAgentJournalSubmission } from '../../../shared/agent-session-queued-submission' import { DISPATCH_DOUBT_HOST_RESTARTED } from './journal-dispatch-doubt-reasons' +import type { JournalReducerState } from './journal-reducer' +import { journalDispatchRowBuilder } from './journal-row-builders' +import type { JournalRow } from './journal-row-schema' import type { AgentSessionJournal } from './journal-store' /** Settles every submission a process fact left unanswerable. Doubt is never @@ -88,3 +91,21 @@ export async function rejectJournalQueuedSubmissions( ) return queued.map((entry) => entry.clientMessageId) } + +/** Rows rejecting every submission still queued, read from `state` when called: for an append + * that must carry them with what follows, in one transaction. */ +export function journalQueuedRejectionRowBuilders( + state: () => JournalReducerState, + fence: number, + rejection: AgentJournalDispatchRejection +): ((seq: number, ts: number) => JournalRow)[] { + return [...state().submissions.values()].filter(isQueuedAgentJournalSubmission).map((entry) => + journalDispatchRowBuilder(state, { + clientMessageId: entry.clientMessageId, + state: 'rejected', + ...rejection, + fence, + recovered: true + }) + ) +} diff --git a/src/main/native-chat/agent-session-journal/journal-queued-rejection-batch.test.ts b/src/main/native-chat/agent-session-journal/journal-queued-rejection-batch.test.ts new file mode 100644 index 00000000000..d218a356fae --- /dev/null +++ b/src/main/native-chat/agent-session-journal/journal-queued-rejection-batch.test.ts @@ -0,0 +1,129 @@ +// A failed start's row and the queued messages it failed land in ONE append, the messages first: +// no reader meets one without the other, and the messages sit above the row that says why. + +import { mkdtemp, rm } from 'node:fs/promises' +import { tmpdir } from 'node:os' +import { join } from 'node:path' +import { afterEach, beforeEach, expect, it } from 'vitest' +import { agentSessionFailureFact } from '../../../shared/agent-session-failure' +import { agentSessionFailureWords } from '../../../shared/agent-session-failure-words' +import { agentJournalSubmissionKey } from '../../../shared/agent-session-journal-item-key' +import { + AGENT_JOURNAL_THREAD_SCOPE, + type AgentSessionJournalIdentity +} from '../../../shared/agent-session-journal-types' +import type { AgentSessionJournal } from './journal-store' +import { + closeTestJournalHostDatabases, + createTrackedJournalOpener +} from './journal-host-database-test-support' + +const IDENTITY: AgentSessionJournalIdentity = { + sessionId: 'session-start', + workspaceId: 'ws-1', + hostId: 'host-1', + agent: 'claude', + providerHandle: { kind: 'claude', sessionId: 'native-1', leafUuid: null } +} + +const START_FAILED = agentSessionFailureWords(agentSessionFailureFact('providerStartFailed'), { + surface: 'rejection' +}) +const ERROR_ROW = { provider: 'orca', clientMessageId: 'start-failure:gen-1' } as const + +let root: string +let clock = 1_000 +const journals = createTrackedJournalOpener() + +beforeEach(async () => { + root = await mkdtemp(join(tmpdir(), 'orca-queued-rejection-batch-')) +}) + +afterEach(async () => { + await closeTestJournalHostDatabases() + await rm(root, { recursive: true, force: true }) +}) + +async function openWithQueued(...ids: string[]): Promise { + const journal = await journals.open({ + identity: IDENTITY, + stateDirectory: root, + now: () => (clock += 1), + mintEpoch: () => 'epoch-1' + }) + for (const id of ids) { + await journal.appendSubmission({ + clientMessageId: id, + payloadFingerprint: `fp-${id}`, + body: { kind: 'message', role: 'user', blocks: [{ type: 'text', text: id }] }, + fence: 0, + handoverRecorded: true + }) + } + return journal +} + +function startFailureBatch(mutations = 1) { + return { + settlementId: 'start-failure:gen-1', + fence: 0, + recovered: true as const, + mutations: Array.from({ length: mutations }, () => ({ + kind: 'item' as const, + identity: ERROR_ROW, + body: { kind: 'status' as const, tone: 'error' as const, text: 'Claude did not start.' }, + turnScope: AGENT_JOURNAL_THREAD_SCOPE + })), + rejectsQueued: START_FAILED + } +} + +it('writes the rejections first and the row after them, and draws the messages above it', async () => { + const journal = await openWithQueued('first', 'second') + const before = journal.cursor().sequence + + await journal.appendLifecycleBatch(startFailureBatch()) + + expect(journal.cursor().sequence).toBe(before + 3) + expect(journal.submissions().map((entry) => entry.dispatchState)).toEqual([ + 'rejected', + 'rejected' + ]) + const order = journal.snapshot().items.map((item) => item.itemId) + expect(order).toEqual([ + agentJournalSubmissionKey('first'), + agentJournalSubmissionKey('second'), + 'orca:start-failure%3Agen-1' + ]) +}) + +it('writes neither when the row cannot be written', async () => { + const journal = await openWithQueued('first') + const before = journal.cursor().sequence + + // An empty batch breaks the row's bound, so the transaction rolls back as a whole. + await expect(journal.appendLifecycleBatch(startFailureBatch(0))).rejects.toThrow( + 'journal_lifecycle_batch_mutation_bound_exceeded' + ) + + expect(journal.cursor().sequence).toBe(before) + expect(journal.submissions().map((entry) => entry.dispatchState)).toEqual(['pending']) +}) + +// A Stop that reaches the lane first takes the message back; the failed start then failed no one. +it('writes nothing when a Stop withdrew every queued message first', async () => { + const journal = await openWithQueued('first') + const withdrawal = agentSessionFailureWords(agentSessionFailureFact('cancelled'), { + surface: 'rejection' + }) + + await Promise.all([ + journal.rejectQueuedSubmissions(0, withdrawal), + journal.appendLifecycleBatch(startFailureBatch()) + ]) + + expect(journal.submissions()[0]?.rejection).toEqual({ kind: 'cancelled' }) + expect(journal.snapshot().items.map((item) => item.itemId)).toEqual([ + agentJournalSubmissionKey('first') + ]) +}) diff --git a/src/main/native-chat/agent-session-journal/journal-reducer.test.ts b/src/main/native-chat/agent-session-journal/journal-reducer.test.ts index 45681f32871..60d068ebfd8 100644 --- a/src/main/native-chat/agent-session-journal/journal-reducer.test.ts +++ b/src/main/native-chat/agent-session-journal/journal-reducer.test.ts @@ -551,7 +551,7 @@ describe('submission and dispatch state machine', () => { expect(state.receipts.get('cm_1')).toBeTruthy() }) - it('keeps a refused write rejected and leaves its bubble where it was', () => { + it('keeps a refused write rejected, at its rejection, whatever comes after', () => { const state = fold([ submission, { @@ -579,7 +579,8 @@ describe('submission and dispatch state machine', () => { submittedAt: submission.ts, reason: 'provider_write_failed: closed before enqueue' }) - expect(renderJournalState(state).items[0]?.sequence).toBe(submission.seq) + // It sits where it was rejected; the late `pending` moves nothing. + expect(renderJournalState(state).items[0]?.sequence).toBe(2) }) it('ignores a dispatch for a submission this epoch never saw', () => { diff --git a/src/main/native-chat/agent-session-journal/journal-reducer.ts b/src/main/native-chat/agent-session-journal/journal-reducer.ts index a7d6f150e70..e2d1b1ef3ca 100644 --- a/src/main/native-chat/agent-session-journal/journal-reducer.ts +++ b/src/main/native-chat/agent-session-journal/journal-reducer.ts @@ -6,9 +6,9 @@ // dropped rather than resurrecting stale content, and ordering is by the // position (sequence, then place in the row) of the write that CREATED an item // (a later revision updates the body, it does not move the bubble) — except a -// queued message, which sits where its handover put it. Producer linkage is -// likewise the creating write's: a revision naming no producer keeps it, one -// naming any replaces it. +// queued message, which sits where its handover put it, and a rejected one, which +// sits where it was rejected. Producer linkage is likewise the creating write's: a +// revision naming no producer keeps it, one naming any replaces it. import type { AgentJournalAcceptanceReceipt, diff --git a/src/main/native-chat/agent-session-journal/journal-rejected-message-placement.test.ts b/src/main/native-chat/agent-session-journal/journal-rejected-message-placement.test.ts new file mode 100644 index 00000000000..ce5b407d3e8 --- /dev/null +++ b/src/main/native-chat/agent-session-journal/journal-rejected-message-placement.test.ts @@ -0,0 +1,158 @@ +// A rejected message sits where it was rejected, in no turn: queued, handed over or sent directly. +// One in doubt stays where it was: it may have reached the agent. + +import { describe, expect, it } from 'vitest' +import { agentJournalSubmissionKey } from '../../../shared/agent-session-journal-item-key' +import { + AGENT_JOURNAL_THREAD_SCOPE, + type AgentJournalTurnScope +} from '../../../shared/agent-session-journal-types' +import { DISPATCH_REJECTED_HOST_RESTARTED } from '../../../shared/structured-agent-session-dispatch-rejection' +import { projectStructuredAgentSessionMessages } from '../../../shared/structured-agent-session-message-projection' +import { projectJournalBatch } from '../agent-session-wire/agent-session-journal-batch' +import { DISPATCH_DOUBT_HOST_RESTARTED } from './journal-dispatch-doubt-reasons' +import { applyJournalRow, createJournalReducerState, renderJournalState } from './journal-reducer' +import { buildJournalSubmissionRow, journalRowBase } from './journal-row-builders' +import type { JournalRow } from './journal-row-schema' + +function journal() { + const state = createJournalReducerState('session-1', 'epoch-1') + let seq = 0 + const push = (next: JournalRow): JournalRow => { + applyJournalRow(state, next) + return next + } + return { + state, + submission(clientMessageId: string, handoverRecorded = true) { + seq += 1 + return push( + buildJournalSubmissionRow({ + state, + clientMessageId, + payloadFingerprint: `fp-${clientMessageId}`, + providerHandle: { kind: 'codex', threadId: 'thread-1' }, + body: { + kind: 'message', + role: 'user', + blocks: [{ type: 'text', text: clientMessageId }] + }, + seq, + fence: 1, + ts: 1_000 + seq, + ...(handoverRecorded ? { handoverRecorded: true } : {}) + }) + ) + }, + dispatch( + clientMessageId: string, + state_: 'pending' | 'rejected' | 'unknown', + turnScope: AgentJournalTurnScope = AGENT_JOURNAL_THREAD_SCOPE + ) { + seq += 1 + return push({ + kind: 'dispatch', + clientMessageId, + state: state_, + providerItemId: null, + reason: + state_ === 'rejected' + ? DISPATCH_REJECTED_HOST_RESTARTED + : state_ === 'unknown' + ? DISPATCH_DOUBT_HOST_RESTARTED + : null, + ...(state_ === 'rejected' ? { rejection: { kind: 'hostRestarted' } } : {}), + ...journalRowBase(state.epoch, seq, 1, 1_000 + seq), + turnScope + }) + }, + /** Other work between the send and its settlement, as a turn running ahead of it would write. */ + advance(rows: number) { + seq += rows + state.lastSequence = seq + } + } +} + +function placed(state: ReturnType['state'], clientMessageId: string) { + const item = state.items.get(agentJournalSubmissionKey(clientMessageId)) + return item && { sequence: item.sequence, observedAt: item.observedAt, scope: item.turnScope } +} + +const IN_TURN: AgentJournalTurnScope = { kind: 'turn', turnItemId: 'orca:turn-1' } + +describe('a rejected message', () => { + it('sits at the rejection, in no turn, when it was queued', () => { + const { state, submission, dispatch, advance } = journal() + submission('waiting') + advance(300) + dispatch('waiting', 'rejected') + + expect(placed(state, 'waiting')).toEqual({ + sequence: 302, + observedAt: 1_302, + scope: AGENT_JOURNAL_THREAD_SCOPE + }) + }) + + it('reaches a subscriber at the tail, so the newest page holds it', () => { + const { state, submission, dispatch, advance } = journal() + submission('waiting') + advance(300) + const rejection = dispatch('waiting', 'rejected') + + const projected = projectJournalBatch({ + rows: [rejection], + snapshot: renderJournalState(state), + afterSequence: 301 + }) + expect(projected.ok && projected.batch.items.map((item) => item.sequence)).toEqual([302]) + expect(renderJournalState(state).items.at(-1)?.itemId).toBe( + agentJournalSubmissionKey('waiting') + ) + }) + + it('sits at the rejection, out of the turn it was handed into, when it was a steer', () => { + const { state, submission, dispatch, advance } = journal() + submission('steer') + advance(10) + dispatch('steer', 'pending', IN_TURN) + expect(placed(state, 'steer')?.scope).toEqual(IN_TURN) + advance(10) + dispatch('steer', 'rejected') + + expect(placed(state, 'steer')).toEqual({ + sequence: 23, + observedAt: 1_023, + scope: AGENT_JOURNAL_THREAD_SCOPE + }) + }) + + it('sits at the rejection when it was sent directly', () => { + const { state, submission, dispatch, advance } = journal() + submission('direct', false) + advance(10) + dispatch('direct', 'rejected') + + expect(placed(state, 'direct')?.sequence).toBe(12) + }) +}) + +describe('a message in doubt', () => { + it('stays where it was handed over, a plain bubble in its turn: it may have reached the agent', () => { + const { state, submission, dispatch, advance } = journal() + submission('steer') + advance(10) + dispatch('steer', 'pending', IN_TURN) + advance(10) + dispatch('steer', 'unknown') + + expect(placed(state, 'steer')).toEqual({ sequence: 12, observedAt: 1_012, scope: IN_TURN }) + const { items, submissions } = renderJournalState(state) + const drawn = projectStructuredAgentSessionMessages(items, [], submissions, { + rejectedInPlace: true + }).find((message) => message.id === agentJournalSubmissionKey('steer')) + expect(drawn).toBeDefined() + expect(drawn?.unsent).toBeUndefined() + }) +}) diff --git a/src/main/native-chat/agent-session-journal/journal-row-writer.test.ts b/src/main/native-chat/agent-session-journal/journal-row-writer.test.ts index c0824a2e4ca..3671ce46cfc 100644 --- a/src/main/native-chat/agent-session-journal/journal-row-writer.test.ts +++ b/src/main/native-chat/agent-session-journal/journal-row-writer.test.ts @@ -93,4 +93,17 @@ describe('journal row writer', () => { kind: 'item' }) }) + + it('writes several rows as one: a failure on the last leaves none', async () => { + const { writer, committedRows } = writerHarness() + // Sequence 2 is taken, so the second of the two rows violates the primary key. + insertTestJournalRow(database.db, SESSION_ID, row(2, 1)) + + await expect(writer.enqueueRows(() => [row, row])).rejects.toThrow() + + expect(committedRows).toHaveLength(0) + expect(readTestJournalRows(database.db, SESSION_ID, EPOCH).map((stored) => stored.seq)).toEqual( + [2] + ) + }) }) diff --git a/src/main/native-chat/agent-session-journal/journal-row-writer.ts b/src/main/native-chat/agent-session-journal/journal-row-writer.ts index ca274120a7b..dde1a66741d 100644 --- a/src/main/native-chat/agent-session-journal/journal-row-writer.ts +++ b/src/main/native-chat/agent-session-journal/journal-row-writer.ts @@ -71,6 +71,40 @@ export class JournalRowWriter { }) } + /** Several rows in ONE transaction, in order, planned once the lane is this append's: none is + * durable unless all are, so no reader ever meets some without the rest. */ + enqueueRows( + plan: () => readonly ((seq: number, ts: number) => JournalRow)[] + ): Promise { + return this.deps.serialize(() => { + assertJournalWritable(this.deps.readOnly(), this.deps.sessionId) + const first = this.deps.nextSequence() + const ts = this.deps.now() + const rows = plan().map((build, index) => build(first + index, ts)) + if (rows.length === 0) { + return rows + } + for (const row of rows) { + assertJournalFence(row.fence, this.deps.highestFence()) + } + try { + this.deps.database().transaction((db) => { + for (const row of rows) { + insertJournalRow(db, this.deps.sessionId, row) + this.runBookkeeping(db, row) + } + }) + } catch (error) { + this.deps.rolledBack?.() + throw error + } + for (const row of rows) { + this.deps.commit(row) + } + return rows + }) + } + /** Assign the next sequence, make the row durable, and fold it through the SAME reducer * replay uses — all inside one serialized step — answering where the row landed. */ append( diff --git a/src/main/native-chat/agent-session-journal/journal-store-collaborators.ts b/src/main/native-chat/agent-session-journal/journal-store-collaborators.ts index 65ea665879b..ba879696469 100644 --- a/src/main/native-chat/agent-session-journal/journal-store-collaborators.ts +++ b/src/main/native-chat/agent-session-journal/journal-store-collaborators.ts @@ -92,6 +92,20 @@ export function createJournalStoreCollaborators(host: JournalStoreHost): Journal wroteBeforeOpen: (sequence) => host.journal().wroteBeforeOpen(sequence), committed: host.notifyCommitted }) + const rowWriter = new JournalRowWriter({ + sessionId: host.identity.sessionId, + now: host.now, + serialize: host.serialize, + database: host.database, + readOnly: host.readOnly, + highestFence: () => host.state().highestFence, + nextSequence: () => host.state().lastSequence + 1, + commit: host.commit, + // Every rejection is a dispatch row through this one writer; the draft + // returned-transition rides it so no path can bypass the hook. + inTransaction: (db, row) => queuedMessages.onRowInTransaction(db, row), + rolledBack: () => queuedMessages.invalidate() + }) return { epochController, queuedMessages, @@ -102,20 +116,7 @@ export function createJournalStoreCollaborators(host: JournalStoreHost): Journal restoreJournalStore(host, { epochController }).then(() => queuedMessages.repairAndPruneAtOpen() ), - rowWriter: new JournalRowWriter({ - sessionId: host.identity.sessionId, - now: host.now, - serialize: host.serialize, - database: host.database, - readOnly: host.readOnly, - highestFence: () => host.state().highestFence, - nextSequence: () => host.state().lastSequence + 1, - commit: host.commit, - // Every rejection is a dispatch row through this one writer; the draft - // returned-transition rides it so no path can bypass the hook. - inTransaction: (db, row) => queuedMessages.onRowInTransaction(db, row), - rolledBack: () => queuedMessages.invalidate() - }), + rowWriter, itemAppender: new JournalItemAppender({ state: host.state, enqueue: host.enqueue @@ -123,7 +124,8 @@ export function createJournalStoreCollaborators(host: JournalStoreHost): Journal lifecycleBatchAppender: new JournalLifecycleBatchAppender({ state: host.state, cursor: host.cursor, - enqueue: host.enqueue + enqueue: host.enqueue, + enqueueRows: (plan) => rowWriter.enqueueRows(plan) }) } } diff --git a/src/main/native-chat/agent-session-journal/journal-store-contracts.ts b/src/main/native-chat/agent-session-journal/journal-store-contracts.ts index 85e6b99f902..cb86a7a8e5d 100644 --- a/src/main/native-chat/agent-session-journal/journal-store-contracts.ts +++ b/src/main/native-chat/agent-session-journal/journal-store-contracts.ts @@ -70,6 +70,10 @@ export type JournalLifecycleBatchInput = { mutations: readonly JournalLifecycleMutationInput[] fence: number recovered?: true + /** Rejects the sends still queued with this first, in the same append: a failed start's row + * follows the messages it failed, and no reader meets one without the other. With none still + * queued, the batch is not written either. */ + rejectsQueued?: AgentJournalDispatchRejection } export type JournalSubmissionInput = { diff --git a/src/main/native-chat/agent-session-journal/journal-submission-fold.ts b/src/main/native-chat/agent-session-journal/journal-submission-fold.ts index 2c9c6cfcd04..80e4e2c93e9 100644 --- a/src/main/native-chat/agent-session-journal/journal-submission-fold.ts +++ b/src/main/native-chat/agent-session-journal/journal-submission-fold.ts @@ -69,6 +69,28 @@ export function placeHandedOverMessage( }) } +/** A rejected message — queued, handed over, or sent directly — joins the conversation where it was + * rejected, in no turn: what happened before the rejection happened before it, and the newest page + * holds a recent one. Only a rejection: one in doubt may have reached the agent, so it stays. */ +export function placeRejectedMessage( + state: JournalReducerState, + submission: AgentJournalSubmission, + row: Extract +): void { + const itemId = agentJournalSubmissionKey(submission.clientMessageId) + const item = state.items.get(itemId) + if (row.state !== 'rejected' || !item) { + return + } + const { sequenceIndex: _placed, ...rest } = item + state.items.set(itemId, { + ...rest, + sequence: row.seq, + observedAt: row.ts, + turnScope: AGENT_JOURNAL_THREAD_SCOPE + }) +} + export function acceptSubmissionFromProviderItem( state: JournalReducerState, providerItemId: string, diff --git a/src/main/native-chat/agent-session-journal/journal-turn-scope.test.ts b/src/main/native-chat/agent-session-journal/journal-turn-scope.test.ts index 2b8e4b80a76..4674f6de929 100644 --- a/src/main/native-chat/agent-session-journal/journal-turn-scope.test.ts +++ b/src/main/native-chat/agent-session-journal/journal-turn-scope.test.ts @@ -163,7 +163,9 @@ describe('stated turn scope', () => { const expected = [agentJournalItemKey(row('result')), agentJournalSubmissionKey('held')] const onPhone = () => { const { items, submissions } = renderJournalState(state) - return projectStructuredAgentSessionMessages(items, [], submissions) + return projectStructuredAgentSessionMessages(items, [], submissions, { + rejectedInPlace: false + }) } // Still waiting: drawn after everything the agent did, the command's result included. expect(drawn(onPhone())).toEqual(expected) diff --git a/src/main/native-chat/agent-session-wire/structured-agent-session-conversation-close.test.ts b/src/main/native-chat/agent-session-wire/structured-agent-session-conversation-close.test.ts index c084d2b2e07..2dac4cacb7d 100644 --- a/src/main/native-chat/agent-session-wire/structured-agent-session-conversation-close.test.ts +++ b/src/main/native-chat/agent-session-wire/structured-agent-session-conversation-close.test.ts @@ -241,16 +241,16 @@ describe('a start that never finishes (P2-15)', () => { providerChildPhase: 'starting' as const })) Object.assign(rig.host.deps.adapter, { awaitStarted: () => started.promise }) - const reject = AgentSessionJournal.prototype.rejectQueuedSubmissions - vi.spyOn(AgentSessionJournal.prototype, 'rejectQueuedSubmissions').mockImplementation(function ( + // The loop rejects the queued messages in the same append as its row. + const append = AgentSessionJournal.prototype.appendLifecycleBatch + vi.spyOn(AgentSessionJournal.prototype, 'appendLifecycleBatch').mockImplementation(function ( this: AgentSessionJournal, ...args ) { - // Not the open's sweep of an earlier process's leftovers. - if (args[1].rejection.kind !== 'hostRestarted') { - order.push(`rejected: ${args[1].reason}`) + if (args[0].rejectsQueued) { + order.push(`rejected: ${args[0].rejectsQueued.reason}`) } - return reject.apply(this, args) + return append.apply(this, args) }) const reader = collectSubscriber() const attached = await rig.host.attach(CALLER, hostTestAttachParams(null)) diff --git a/src/main/native-chat/agent-session-wire/structured-agent-session-host-test-data.ts b/src/main/native-chat/agent-session-wire/structured-agent-session-host-test-data.ts index 959a02919a2..a96388ac4ca 100644 --- a/src/main/native-chat/agent-session-wire/structured-agent-session-host-test-data.ts +++ b/src/main/native-chat/agent-session-wire/structured-agent-session-host-test-data.ts @@ -84,6 +84,8 @@ export function hostTestDrawnRowIds( state: 'dispatching' as const })) return projectNativeChatTranscriptMessages( - projectStructuredAgentSessionMessages(snapshot.items, outbox, snapshot.submissions) + projectStructuredAgentSessionMessages(snapshot.items, outbox, snapshot.submissions, { + rejectedInPlace: true + }) ).map(({ id }) => id) } diff --git a/src/main/native-chat/agent-session-wire/structured-agent-session-start-failure-row.ts b/src/main/native-chat/agent-session-wire/structured-agent-session-start-failure-row.ts index 12fa05710f0..9206f95fde0 100644 --- a/src/main/native-chat/agent-session-wire/structured-agent-session-start-failure-row.ts +++ b/src/main/native-chat/agent-session-wire/structured-agent-session-start-failure-row.ts @@ -42,9 +42,9 @@ export function hasStructuredAgentSessionStartFailureRow( } /** - * A start the delivery loop needed and did not get: the start's row, and every queued message - * rejected with the same words. Writes nothing when nothing is still queued: a start whose - * messages Stop withdrew did not fail anyone. + * A start the delivery loop needed and did not get: every queued message rejected with the same + * words, then the start's row. Writes nothing when nothing is still queued: a start whose messages + * Stop withdrew did not fail anyone. */ export async function recordStructuredAgentSessionStartFailure( session: Pick & { fence: number }, @@ -59,11 +59,8 @@ export async function recordStructuredAgentSessionStartFailure( settlementId: `start-failure:${startKey}`, fence: session.fence, recovered: true, - mutations: [structuredAgentSessionStartFailureRow(startKey, failure)] - }) - await session.journal.rejectQueuedSubmissions(session.fence, { - reason: failure.reason, - rejection: failure.rejection + mutations: [structuredAgentSessionStartFailureRow(startKey, failure)], + rejectsQueued: { reason: failure.reason, rejection: failure.rejection } }) } diff --git a/src/renderer/src/components/native-chat/NativeChatMessageList.provider-retry-runs.test.tsx b/src/renderer/src/components/native-chat/NativeChatMessageList.provider-retry-runs.test.tsx index eb71e3e6b54..db59afdb19d 100644 --- a/src/renderer/src/components/native-chat/NativeChatMessageList.provider-retry-runs.test.tsx +++ b/src/renderer/src/components/native-chat/NativeChatMessageList.provider-retry-runs.test.tsx @@ -10,6 +10,8 @@ import { NativeChatMessageList } from './NativeChatMessageList' import { installNativeChatMessageListTestViewport } from './native-chat-message-list-test-viewport' import { projectStructuredAgentSessionMessages } from './structured-agent-session-message-projection' +const NO_CARDS: readonly string[] = [] + let restoreViewport = (): void => {} beforeAll(() => { restoreViewport = installNativeChatMessageListTestViewport() @@ -42,7 +44,7 @@ function transcript(items: AgentJournalRenderItem[]) { return ( } const view = (items: AgentJournalRenderItem[]) => ( Promise.resolve('exhausted' as const) function Transcript({ items }: { items: AgentJournalRenderItem[] }) { - const messages = useStructuredAgentSessionMessages(items, EMPTY, EMPTY) + const messages = useStructuredAgentSessionMessages(items, EMPTY, EMPTY, EMPTY) const session: NativeChatLiveSession = { messages, status: 'working', diff --git a/src/renderer/src/components/native-chat/NativeChatMessageList.turn-membership.test.tsx b/src/renderer/src/components/native-chat/NativeChatMessageList.turn-membership.test.tsx index b01675c63a7..29f977046f6 100644 --- a/src/renderer/src/components/native-chat/NativeChatMessageList.turn-membership.test.tsx +++ b/src/renderer/src/components/native-chat/NativeChatMessageList.turn-membership.test.tsx @@ -88,7 +88,9 @@ function journalList( return ( {} beforeAll(() => { restoreViewport = installNativeChatMessageListTestViewport() @@ -135,7 +139,7 @@ function list(phase: Phase, scoped: boolean, outbox: StructuredAgentSessionOutbo return ( { + const LOST = agentJournalSubmissionKey('lost') + + function hostList(phase: Phase) { + const items = journal(phase, true) + const newIndex = items.findIndex((item) => item.itemId === NEW) + // Recorded between the seed's answer and the newer prompt; the next open rejected it. + items.splice(newIndex, 0, { + itemId: LOST, + revision: 0, + sequence: items[newIndex - 1]!.sequence, + sequenceIndex: 1, + observedAt: 1002.5, + body: said('user', 'LOST PROMPT'), + turnScope: { kind: 'thread' } + }) + const submissions: AgentJournalSubmission[] = [ + submission('seed', 'accepted'), + { + ...submission('lost', 'rejected'), + reason: DISPATCH_REJECTED_HOST_RESTARTED, + rejection: { kind: 'hostRestarted' } + }, + submission('new', phase === 'done' ? 'accepted' : 'pending') + ] + const settledTurns: NativeChatSettledTurns = new Map([ + [SEED, { startedAt: 1, workedSeconds: 3 }], + ...(phase === 'done' ? [[NEW, { startedAt: 2, workedSeconds: 5 }] as const] : []) + ]) + return ( + + ) + } + + it('draws it where it was sent, with its reason and no Retry, outside the newer turn', () => { + const { container, rerender } = render(hostList('running')) + expect(drawn(container)).toEqual([ + 'SEED PROMPT', + 'WORKED', + 'LOST PROMPT', + 'NEW PROMPT', + 'WORKING', + 'ACTIVITY' + ]) + expect(container.textContent).toContain('Orca restarted before this message was sent.') + expect(container.querySelector('button[aria-label="Retry"]')).toBeNull() + expect( + [...container.querySelectorAll('button')].map((button) => button.textContent) + ).not.toContain('Retry') + rerender(hostList('done')) + expect(drawn(container)).toEqual([ + 'SEED PROMPT', + 'WORKED', + 'LOST PROMPT', + 'NEW PROMPT', + 'WORKED' + ]) + expect(container.textContent).toContain('Worked for 5s') + }) +}) diff --git a/src/renderer/src/components/native-chat/NativeChatStructuredSession.start-failure-notice.test.tsx b/src/renderer/src/components/native-chat/NativeChatStructuredSession.start-failure-notice.test.tsx index 6f529c1d3c9..92548887f0a 100644 --- a/src/renderer/src/components/native-chat/NativeChatStructuredSession.start-failure-notice.test.tsx +++ b/src/renderer/src/components/native-chat/NativeChatStructuredSession.start-failure-notice.test.tsx @@ -82,8 +82,8 @@ function rejected(clientMessageId: string, reason: string, rejection: AgentSessi } } +// The notices are the host's rows'; a copy an earlier session left in the outbox gives way to them. function renderPane(messages: ReturnType[]): void { - mocks.mode = 'outbox' mocks.submissions = messages.map((message) => message.submission) localStorage.setItem( `orca:desktopStructuredAgentSessionOutbox:v1:${encodeURIComponent(SESSION_ID)}`, @@ -113,8 +113,9 @@ async function notice(clientMessageId: string): Promise { }) } -// The start's own row says why, so its rejected messages say only that they were not sent. -it("says only 'not sent', with its Retry, on each message the failed start's row explains", async () => { +// The start's own row says why, so its rejected messages say only that they were not sent. Sending +// one again is a new message, so none offers a Retry. +it("says only 'not sent', with no Retry, on each message the failed start's row explains", async () => { mocks.journalItems = [startFailureRow(START_FAILED)] renderPane([ @@ -125,7 +126,7 @@ it("says only 'not sent', with its Retry, on each message the failed start's row for (const id of ['first', 'second']) { const row = await notice(id) expect(within(row).getByText('Your message was not sent.')).toBeTruthy() - expect(within(row).getByRole('button', { name: 'Retry' })).toBeTruthy() + expect(within(row).queryByRole('button', { name: 'Retry' })).toBeNull() } expect(screen.queryByText(/stopped before it finished starting/)).toBeNull() }) @@ -157,7 +158,5 @@ it('keeps the full notice on a message rejected for a reason no start-failure ro it("keeps the start failure's own words when its row is not loaded", async () => { renderPane([rejected('first', START_FAILED_REASON, START_FAILED)]) - expect( - within(await notice('first')).getByText('Claude stopped before it finished starting.') - ).toBeTruthy() + expect(within(await notice('first')).getByText(START_FAILED_REASON)).toBeTruthy() }) diff --git a/src/renderer/src/components/native-chat/NativeChatStructuredSession.test-harness.tsx b/src/renderer/src/components/native-chat/NativeChatStructuredSession.test-harness.tsx index 6e39ec0129d..ee876868c35 100644 --- a/src/renderer/src/components/native-chat/NativeChatStructuredSession.test-harness.tsx +++ b/src/renderer/src/components/native-chat/NativeChatStructuredSession.test-harness.tsx @@ -140,6 +140,7 @@ export function createStructuredSessionMocks() { }) => { mocks.controllerProps = props const outbox = useStructuredAgentSessionOutbox({ + journalItems: mocks.journalItems, sessionId: props.sessionId, target: props.target, fence: props.transportEnabled === false ? null : 1, @@ -150,7 +151,9 @@ export function createStructuredSessionMocks() { messages: mocks.messages ?? (mocks.mode === 'outbox' - ? projectStructuredAgentSessionMessages([], outbox.outbox, []) + ? projectStructuredAgentSessionMessages([], outbox.outbox, [], { + rejectedInPlace: true + }) : [ { id: 'message-1', diff --git a/src/renderer/src/components/native-chat/NativeChatStructuredSession.tsx b/src/renderer/src/components/native-chat/NativeChatStructuredSession.tsx index eca921ed086..e69056836c4 100644 --- a/src/renderer/src/components/native-chat/NativeChatStructuredSession.tsx +++ b/src/renderer/src/components/native-chat/NativeChatStructuredSession.tsx @@ -1,4 +1,4 @@ -import { useCallback, useEffect, useMemo, useRef, useState } from 'react' +import { useMemo, useRef, useState } from 'react' import { agentSessionPromptQuestions } from '../../../../shared/agent-session-question-answer' import { dispatchStructuredAgentSessionComposerCommand } from '../../../../shared/structured-agent-session-composer' import { structuredAgentSessionPaneKey } from '../../../../shared/structured-agent-session-projection' @@ -29,11 +29,7 @@ import { useAppStore } from '../../store' import { structuredAgentLabel } from '@/lib/structured-agent-session-launch-label' import { NativeChatThreadGoalBanner } from './NativeChatThreadGoalBanner' import { structuredAgentSessionReadFailureNotice } from './structured-agent-session-read-failure-notice' -import { useStructuredAgentSessionStartFailureFacts } from './use-structured-agent-session-start-failure-facts' -import { structuredAgentSessionDeliveryNotices } from './structured-agent-session-delivery-notices' -import type { AgentJournalSubmission } from '../../../../shared/agent-session-journal-types' - -const NO_SUBMISSIONS: readonly AgentJournalSubmission[] = [] +import { useStructuredAgentSessionDeliveryNotices } from './use-structured-agent-session-delivery-notices' export function NativeChatStructuredSession( props: Omit @@ -115,41 +111,16 @@ export function NativeChatStructuredSession( }), [controller, historyPhase, props.agent, props.sessionId] ) - // Read at click time, so the notices stay put while the outbox's Retry is rebuilt each render. - const retryRef = useRef(controller.retry) - useEffect(() => { - retryRef.current = controller.retry - }) - const retryDelivery = useCallback((clientMessageId: string) => { - retryRef.current(clientMessageId) - }, []) const agentLabel = structuredAgentLabel(props.agent === 'codex' ? 'codex' : 'claude') - // Only a rejected message reads the journal's rows, so a new batch of them re-renders no row else. - const hasRejected = controller.outbox.some((entry) => entry.state === 'rejected') - const rejectionRows = hasRejected ? controller.submissions : NO_SUBMISSIONS - const startFailures = useStructuredAgentSessionStartFailureFacts( - controller.journalItems, - hasRejected - ) - const deliveryNotices = useMemo( - () => - structuredAgentSessionDeliveryNotices( - controller.outbox, - agentLabel, - retryDelivery, - rejectionRows, - startFailures, - controller.failedHere - ), - [ - controller.outbox, - agentLabel, - retryDelivery, - rejectionRows, - startFailures, - controller.failedHere - ] - ) + const deliveryNotices = useStructuredAgentSessionDeliveryNotices({ + outbox: controller.outbox, + submissions: controller.submissions, + journalItems: controller.journalItems, + failedHere: controller.failedHere, + queuedMessageIds: controller.queuedMessageIds, + retry: controller.retry, + agentName: agentLabel + }) const viewState = selectNativeChatViewState(session, { readRetries: true }) // Nothing reads an unread history, so its pane stays blank beside the Retry line. const loadingPane = historyPhase === 'unread' ? null : diff --git a/src/renderer/src/components/native-chat/NativeChatStructuredSessionDelivery.test.tsx b/src/renderer/src/components/native-chat/NativeChatStructuredSessionDelivery.test.tsx index c5553f00edb..64d47882740 100644 --- a/src/renderer/src/components/native-chat/NativeChatStructuredSessionDelivery.test.tsx +++ b/src/renderer/src/components/native-chat/NativeChatStructuredSessionDelivery.test.tsx @@ -26,6 +26,7 @@ const mocks = vi.hoisted(() => ({ }, questionCardProps: null as NativeChatQuestionCardProps | null, promptItems: [] as AgentJournalRenderItem[], + noJournalItems: Array.of(), respond: vi.fn(), handlePasteEvent: vi.fn(), pasteFromClipboard: vi.fn(), @@ -53,6 +54,7 @@ vi.mock('./use-structured-agent-session', async () => { target: { kind: 'local' } | { kind: 'environment'; environmentId: string } }) => { const outbox = useStructuredAgentSessionOutbox({ + journalItems: mocks.noJournalItems, sessionId: props.sessionId, target: props.target, fence: 1, @@ -62,7 +64,9 @@ vi.mock('./use-structured-agent-session', async () => { journalItems: [], messages: mocks.mode === 'outbox' - ? projectStructuredAgentSessionMessages([], outbox.outbox, []) + ? projectStructuredAgentSessionMessages([], outbox.outbox, [], { + rejectedInPlace: true + }) : [ { id: 'message-1', @@ -185,6 +189,8 @@ vi.mock('./NativeChatQuestionCard', () => ({ import { NativeChatStructuredSession } from './NativeChatStructuredSession' import { appendStructuredAgentSessionOutboxMessage } from './structured-agent-session-outbox-storage' +const REFUSED_RESTART = "The agent couldn't restart. Your message was not sent." + function useProbeClock(): void { vi.useFakeTimers({ toFake: ['setTimeout', 'clearTimeout', 'Date'] }) } @@ -319,10 +325,11 @@ describe('NativeChatStructuredSession delivery', () => { }) seedOutbox('session-held-rejected', [ seededEntry('session-held-rejected', 'op-head', 'first', 'unconfirmed'), + // Refused before the host recorded it, so its Retry is the only way it goes again. { ...seededEntry('session-held-rejected', 'op-rejected', 'second', 'queued'), state: 'rejected', - lastFailure: { kind: 'rejected', reason: 'Claude messages support at most 20 images' } + lastFailure: { kind: 'refused', code: 'agent_session_owner_restart_failed' } } ]) @@ -338,7 +345,7 @@ describe('NativeChatStructuredSession delivery', () => { ) await waitFor(() => expect(screen.getByText('Message delivery is unconfirmed.')).toBeTruthy()) - expect(screen.getByText('Claude messages support at most 20 images')).toBeTruthy() + expect(screen.getByText(REFUSED_RESTART)).toBeTruthy() // One Retry, the stopped message's: it sends only that one. fireEvent.click(screen.getByRole('button', { name: /Retry/ })) await waitFor(() => expect(mocks.call).toHaveBeenCalledOnce()) @@ -348,27 +355,38 @@ describe('NativeChatStructuredSession delivery', () => { // The queue moved, so the rejected message offers its own Retry again. await waitFor(() => expect(screen.queryByText('Message delivery is unconfirmed.')).toBeNull()) - expect(screen.getByText('Claude messages support at most 20 images')).toBeTruthy() + expect(screen.getByText(REFUSED_RESTART)).toBeTruthy() expect(screen.getAllByRole('button', { name: /Retry/ })).toHaveLength(1) expect(mocks.call).toHaveBeenCalledOnce() }) - it("words a failed start on each message by the chat's agent, leaving the resend to its Retry", async () => { + // Until the journal carries the row, the message's own copy words it; the host has it, so no Retry. + it("words a failed start its reply rejected by the chat's agent, with no Retry", async () => { mocks.mode = 'outbox' mocks.submissions = [] - const startFailed = (clientMessageId: string, text: string) => ({ - ...seededEntry('session-start-failed', clientMessageId, text, 'queued'), - state: 'rejected' as const, - lastFailure: { - kind: 'rejected' as const, - reason: 'Codex stopped before it finished starting. Send your message to try again.', - rejection: { kind: 'providerStartFailed' as const } - } - }) - seedOutbox('session-start-failed', [ - startFailed('op-first', 'first'), - startFailed('op-second', 'second') - ]) + mocks.call.mockImplementation( + async ( + _target: unknown, + _method: unknown, + params: { envelope: { clientOperationId: string } } + ) => ({ + ok: true, + value: { + clientMessageId: params.envelope.clientOperationId, + submission: { + clientMessageId: params.envelope.clientOperationId, + fence: 1, + payloadFingerprint: 'fingerprint', + dispatchState: 'rejected', + providerItemId: null, + reason: 'Codex stopped before it finished starting. Send your message to try again.', + rejection: { kind: 'providerStartFailed' }, + submittedAt: 1, + resolvedAt: 2 + } + } + }) + ) render( { /> ) + const send = mocks.composerProps?.structuredTransport?.send as + | ((text: string, attachments: readonly { id: string; path: string }[]) => boolean) + | undefined + expect(send?.('first', [])).toBe(true) await waitFor(() => - expect(screen.getAllByText('Codex stopped before it finished starting.')).toHaveLength(2) + expect( + screen.getByText( + 'Codex stopped before it finished starting. Send your message to try again.' + ) + ).toBeTruthy() ) - expect(screen.getAllByRole('button', { name: /Retry/ })).toHaveLength(2) - expect(screen.queryByText(/Send your message to try again/)).toBeNull() + expect(screen.queryByRole('button', { name: /Retry/ })).toBeNull() + expect(mocks.call).toHaveBeenCalledOnce() }) - it("words a rejected message from its loaded journal row, not the message's own copy", async () => { - mocks.mode = 'outbox' + // A copy an earlier session left in the outbox gives way to the host's row. + it("words a rejected message from its journal row, not the message's own copy, with no Retry", async () => { const reason = "Claude couldn't start. Send your message to try again." mocks.submissions = [ { @@ -430,6 +456,7 @@ describe('NativeChatStructuredSession delivery', () => { expect(screen.getByText("Codex couldn't start. Start a new chat to continue.")).toBeTruthy() ) expect(screen.queryByText(reason)).toBeNull() + expect(screen.queryByRole('button', { name: /Retry/ })).toBeNull() }) it('names the stuck message behind an admitted head, and its Retry sends that one', async () => { diff --git a/src/renderer/src/components/native-chat/native-chat-rail-outline-parity.test.ts b/src/renderer/src/components/native-chat/native-chat-rail-outline-parity.test.ts index e5254b63e94..16c9a30a397 100644 --- a/src/renderer/src/components/native-chat/native-chat-rail-outline-parity.test.ts +++ b/src/renderer/src/components/native-chat/native-chat-rail-outline-parity.test.ts @@ -19,6 +19,8 @@ import { buildNativeChatTranscriptSlots } from './native-chat-transcript-slots' import type { NativeChatTurnDiff } from './native-chat-turn-diffs' import { projectStructuredAgentSessionMessages } from './structured-agent-session-message-projection' +const NO_CARDS: readonly string[] = [] + function row(sequence: number, body: AgentJournalItemBody, itemId = `item-${sequence}`) { return { itemId, revision: 1, sequence, observedAt: 1_000 + sequence, body } } @@ -68,7 +70,7 @@ const JOURNAL: AgentJournalRenderItem[] = [ /** The renderer's own path from journal items to rail items, as the list runs it. */ function loadedRailItems(items: AgentJournalRenderItem[], submissions: AgentJournalSubmission[]) { const projected = createNativeChatMessageListProjection()( - projectStructuredAgentSessionMessages(items, [], submissions) + projectStructuredAgentSessionMessages(items, [], submissions, NO_CARDS) ).conversation const messages = omitNativeChatThreadGoalRows(projectNativeChatTaskListFrames(projected)) let turn: string | undefined @@ -93,9 +95,14 @@ function loadedRailItems(items: AgentJournalRenderItem[], submissions: AgentJour } describe('conversation outline parity with the loaded rail', () => { + // Except a rejected message: the desktop draws it in place and ticks it once loaded, while the + // host's outline, which older clients read too, leaves it out. it('lists exactly the user messages the transcript gives a rail tick, with the same ids and previews', () => { const outline = projectAgentSessionConversationOutline(JOURNAL, [REJECTED]) - const loaded = loadedRailItems(JOURNAL, [REJECTED]) + const rejectedId = agentJournalSubmissionKey(REJECTED.clientMessageId) + const loadedWithRejected = loadedRailItems(JOURNAL, [REJECTED]) + expect(loadedWithRejected.filter((item) => item.id === rejectedId)).toHaveLength(1) + const loaded = loadedWithRejected.filter((item) => item.id !== rejectedId) expect(outline.map((entry) => entry.itemId)).toEqual(loaded.map((item) => item.id)) expect( diff --git a/src/renderer/src/components/native-chat/structured-agent-session-delivery-notices.test.ts b/src/renderer/src/components/native-chat/structured-agent-session-delivery-notices.test.ts index 5dde72621ad..9e3cfedbdd1 100644 --- a/src/renderer/src/components/native-chat/structured-agent-session-delivery-notices.test.ts +++ b/src/renderer/src/components/native-chat/structured-agent-session-delivery-notices.test.ts @@ -56,7 +56,8 @@ function texts( } describe('the notice on each message that did not go through', () => { - it('gives two failed messages each their own reason and their own Retry', () => { + // Recorded by the host, so sending one again is a new message: no Retry. + it('gives two messages the host rejected each their own reason and no Retry', () => { const retry = vi.fn() const notices = structuredAgentSessionDeliveryNotices( [ @@ -77,7 +78,7 @@ describe('the notice on each message that did not go through', () => { retry, [], [], - NOT_FAILED_HERE + new Set(['first', 'second']) ) expect([...notices.keys()]).toEqual([ @@ -90,8 +91,47 @@ describe('the notice on each message that did not go through', () => { expect(notices.get(agentJournalSubmissionKey('second'))?.text).toBe( 'Claude never finished starting, so Orca stopped it.' ) - notices.get(agentJournalSubmissionKey('second'))?.onRetry?.() - expect(retry).toHaveBeenCalledExactlyOnceWith('second') + expect(notices.get(agentJournalSubmissionKey('first'))?.onRetry).toBeUndefined() + expect(notices.get(agentJournalSubmissionKey('second'))?.onRetry).toBeUndefined() + }) + + // However this chat learned of it: the host recorded it, so it has no control at all. + it('gives a message the host rejected before this chat opened no Retry and no Dismiss', () => { + const retry = vi.fn() + const notice = structuredAgentSessionDeliveryNotices( + [ + entry('earlier', { + state: 'rejected', + lastFailure: { kind: 'rejected', reason: 'Claude messages support at most 20 images' } + }) + ], + 'Claude', + retry, + [], + [], + NOT_FAILED_HERE + ).get(agentJournalSubmissionKey('earlier')) + expect(notice).toEqual({ text: 'Claude messages support at most 20 images' }) + expect(retry).not.toHaveBeenCalled() + }) + + // Refused before the host recorded it: only its Retry sends it, so it keeps one. + it('keeps the Retry on a message refused before the host recorded it', () => { + const notice = structuredAgentSessionDeliveryNotices( + [ + entry('refused', { + state: 'rejected', + lastFailure: { kind: 'refused', code: 'agent_session_owner_restart_failed' } + }) + ], + 'Claude', + () => {}, + [], + [], + NOT_FAILED_HERE + ).get(agentJournalSubmissionKey('refused')) + expect(notice?.onRetry).toBeDefined() + expect(notice?.onDismiss).toBeUndefined() }) it('chooses the words from the saved refusal on a refused message', () => { @@ -223,39 +263,15 @@ describe('the notice on each message that did not go through', () => { expect(retry.mock.calls).toEqual([['refused'], ['rejected'], ['stuck']]) }) - // Beside its own Retry the resend step is the button; without one the words keep it. - it('leaves out sending again only where the message has its own Retry', () => { - const startFailed = (clientMessageId: string): StructuredAgentSessionOutboxEntry => - entry(clientMessageId, { - state: 'rejected', - lastFailure: { - kind: 'rejected', - reason: 'Claude stopped before it finished starting. Send your message to try again.', - rejection: { kind: 'providerStartFailed' } - } - }) - expect(texts([startFailed('first'), startFailed('second')])).toEqual({ - [agentJournalSubmissionKey('first')]: 'Claude stopped before it finished starting.', - [agentJournalSubmissionKey('second')]: 'Claude stopped before it finished starting.' - }) - expect(texts([entry('held', { outlivedStop: true }), startFailed('rejected')])).toMatchObject({ - [agentJournalSubmissionKey('rejected')]: - 'Claude stopped before it finished starting. Send your message to try again.' - }) - }) - + // With no Retry beside it, the words keep the resend step. it.each([ [ - 'notDelivered', - 'This message was not delivered. Send it again to continue.', - 'This message was not delivered.' + 'providerStartFailed', + 'Claude stopped before it finished starting. Send your message to try again.' ], - [ - 'hostFault', - "Orca ran into a problem, so this didn't go through. Try again.", - "Orca ran into a problem, so this didn't go through." - ] - ] as const)('leaves the step to the Retry beside a %s message', (kind, reason, shown) => { + ['notDelivered', 'This message was not delivered. Send it again to continue.'], + ['hostFault', "Orca ran into a problem, so this didn't go through. Try again."] + ] as const)('keeps the step in the words of a %s message the host rejected', (kind, reason) => { expect( texts([ entry('rejected', { @@ -263,7 +279,7 @@ describe('the notice on each message that did not go through', () => { lastFailure: { kind: 'rejected', reason, rejection: { kind } } }) ]) - ).toEqual({ [agentJournalSubmissionKey('rejected')]: shown }) + ).toEqual({ [agentJournalSubmissionKey('rejected')]: reason }) }) // The journal holds the whole fact; the message's own copy keeps only its kind and attachment. @@ -281,7 +297,7 @@ describe('the notice on each message that did not go through', () => { const recorded = (id: string, rejection: AgentSessionFailureFact): AgentJournalSubmission => ({ clientMessageId: id, fence: 1, - payloadFingerprint: 'fingerprint', + payloadFingerprint: id, dispatchState: 'rejected', providerItemId: null, reason: "The agent couldn't be started.", @@ -312,7 +328,8 @@ describe('the notice on each message that did not go through', () => { [ 'resumable', { kind: 'startFailed', refusal: { code: 'agent_session_ownership_unknown' } }, - "Claude couldn't start." + // No Retry beside it, so the words keep the step. + "Claude couldn't start. Send your message to try again." ], [ 'provider', @@ -392,7 +409,7 @@ describe('the notice on each message that did not go through', () => { { clientMessageId: 'recorded', fence: 1, - payloadFingerprint: 'fingerprint', + payloadFingerprint: 'recorded', dispatchState: 'rejected', providerItemId: null, reason, diff --git a/src/renderer/src/components/native-chat/structured-agent-session-delivery-notices.ts b/src/renderer/src/components/native-chat/structured-agent-session-delivery-notices.ts index 6ad655e8f80..1ef4067357d 100644 --- a/src/renderer/src/components/native-chat/structured-agent-session-delivery-notices.ts +++ b/src/renderer/src/components/native-chat/structured-agent-session-delivery-notices.ts @@ -7,10 +7,11 @@ // rejected or refused message holds nothing up, so it keeps its words and gets its Retry once the // queue moves. // -// A message the host recorded and then rejected is worded from the journal's own fact, found by id; -// the message keeps only a smaller copy, read when its submission is not loaded. A rejection that -// is a failed start's, the fact its loaded row states, says only that it was not sent: the row -// already says why. +// A message the host recorded and then rejected is drawn from the host's history, worded from the +// journal's own fact, with no Retry: sending it again is a new message. A rejection that is a +// failed start's, the fact its loaded row states, says only that it was not sent: the row already +// says why. Until its row loads, the outbox draws it from its smaller copy, which leaves on the batch +// or page that loads the row. import { readAgentSessionFailureFact, @@ -26,6 +27,7 @@ import { agentSessionWriteNotDoneParts } from '../../../../shared/agent-session- import { isStructuredAgentSessionStartFailureRow } from '../../../../shared/structured-agent-session-start-failure-row-key' import { structuredAgentSessionEntryIdExpired, + structuredAgentSessionEntryRejectedByHost, type StructuredAgentSessionOutboxEntry } from '../../../../shared/structured-agent-session-outbox' import { @@ -33,11 +35,17 @@ import { structuredAgentSessionEntryHeldForRetry } from '../../../../shared/structured-agent-session-outbox-admission' import type { AgentSessionFailureWordsContext } from '../../../../shared/agent-session-failure-words' -import { structuredAgentSessionAttemptFailureParts } from '../../../../shared/structured-agent-session-send-disposition' +import { + structuredAgentSessionAttemptFailureParts, + structuredAgentSessionRejectionParts +} from '../../../../shared/structured-agent-session-send-disposition' +import { structuredAgentSessionRejectedShownInPlace } from '../../../../shared/structured-agent-session-message-projection' import { translate } from '@/i18n/i18n' import { agentSessionWriteNoticeText } from './agent-session-write-notice-text' import type { NativeChatDeliveryNotice } from './NativeChatMessageRow' +const NO_COMMANDS: ReadonlySet = new Set() + /** The facts the chat's loaded start-failure rows state. */ export function structuredAgentSessionStartFailureFacts( items: readonly AgentJournalRenderItem[] @@ -87,8 +95,6 @@ export function agentSessionFailureStatedByStartRow( function deliveryNoticeText( entry: StructuredAgentSessionOutboxEntry, context: AgentSessionFailureWordsContext, - recorded: AgentJournalSubmission | undefined, - startFailures: readonly AgentSessionFailureFact[], failedHere: ReadonlySet ): string { // A send attempted before a Stop and then interrupted may already be with the host. @@ -114,17 +120,25 @@ function deliveryNoticeText( if (structuredAgentSessionEntryHeldForRetry(entry) && !failedHere.has(entry.clientMessageId)) { return agentSessionWriteNoticeText(agentSessionWriteNotDoneParts('send')) } - if ( - entry.state === 'rejected' && - agentSessionFailureStatedByStartRow(recorded?.rejection, startFailures) - ) { + return agentSessionWriteNoticeText( + structuredAgentSessionAttemptFailureParts(entry.lastFailure, context) + ) +} + +function hostRejectionNoticeText( + submission: AgentJournalSubmission, + agentName: string, + startFailures: readonly AgentSessionFailureFact[] +): string { + if (agentSessionFailureStatedByStartRow(submission.rejection, startFailures)) { return agentSessionWriteNoticeText(agentSessionWriteNotDoneParts('send')) } return agentSessionWriteNoticeText( - structuredAgentSessionAttemptFailureParts( - entry.lastFailure, - context, - readWholeAgentSessionFailureFact(recorded?.rejection) + structuredAgentSessionRejectionParts( + submission.reason, + 'send', + readWholeAgentSessionFailureFact(submission.rejection), + { agentName } ) ) } @@ -140,7 +154,11 @@ export function structuredAgentSessionDeliveryNotices( /** What the loaded start-failure rows state, from `structuredAgentSessionStartFailureFacts`. */ startFailures: readonly AgentSessionFailureFact[], /** Ids whose send failed or was refused while this chat was open: only they word their cause. */ - failedHere: ReadonlySet + failedHere: ReadonlySet, + /** The queue's live cards, which the transcript leaves a rejected message to. */ + queuedMessageIds: readonly string[] = [], + /** The loaded commands, from `structuredAgentSessionCommandItemIds`: they report their own. */ + commandItemIds: ReadonlySet = NO_COMMANDS ): ReadonlyMap { const admission = admitStructuredAgentSessionOutboxEntry(outbox) const held = admission.state === 'blocked' ? admission.entry.clientMessageId : null @@ -157,20 +175,29 @@ export function structuredAgentSessionDeliveryNotices( structuredAgentSessionEntryHeldForRetry(entry) || entry.clientMessageId === held ) { - // Its own Retry is the step, so the words leave out sending again. - const retryControl = stalledFrom === -1 || index <= stalledFrom - const text = deliveryNoticeText( - entry, - { agentName, retryControl }, - rejected.get(entry.clientMessageId), - startFailures, - failedHere - ) + // Its own Retry is the step, so the words leave out sending again. One the host recorded is + // the host's: sending it again is a new message, so it has no Retry. + const retryControl = + (stalledFrom === -1 || index <= stalledFrom) && + !structuredAgentSessionEntryRejectedByHost(entry) + const text = deliveryNoticeText(entry, { agentName, retryControl }, failedHere) notices.set( agentJournalSubmissionKey(entry.clientMessageId), retryControl ? { text, onRetry: () => retry(entry.clientMessageId) } : { text } ) } } + // After the outbox's: in the host's words, whether its row or the outbox's copy draws it. + const shown = structuredAgentSessionRejectedShownInPlace( + submissions, + queuedMessageIds, + commandItemIds + ) + for (const submission of rejected.values()) { + const id = agentJournalSubmissionKey(submission.clientMessageId) + if (shown.has(id)) { + notices.set(id, { text: hostRejectionNoticeText(submission, agentName, startFailures) }) + } + } return notices } diff --git a/src/renderer/src/components/native-chat/structured-agent-session-message-projection.older-host.test.ts b/src/renderer/src/components/native-chat/structured-agent-session-message-projection.older-host.test.ts new file mode 100644 index 00000000000..7cc13a0a4eb --- /dev/null +++ b/src/renderer/src/components/native-chat/structured-agent-session-message-projection.older-host.test.ts @@ -0,0 +1,161 @@ +// An older host leaves a rejected message where it was sent. When that is older than the loaded +// window, the chat holds its rejected submission but not its row: the outbox copy keeps drawing it, +// with no control, until the page holding the row loads, and then the host's row draws it instead. + +import { expect, it } from 'vitest' +import { agentJournalSubmissionKey } from '../../../../shared/agent-session-journal-item-key' +import type { + AgentJournalRenderItem, + AgentJournalSubmission +} from '../../../../shared/agent-session-journal-types' +import type { AgentSessionHistoryPage } from '../../../../shared/agent-session-wire' +import { DISPATCH_REJECTED_HOST_RESTARTED } from '../../../../shared/structured-agent-session-dispatch-rejection' +import { reconcileStructuredAgentSessionOutboxWithQueue } from '../../../../shared/structured-agent-session-draft-hand-off' +import { createStructuredAgentSessionOutboxEntry } from '../../../../shared/structured-agent-session-outbox' +import { + EMPTY_STRUCTURED_AGENT_SESSION, + reduceStructuredAgentSession +} from '../../../../shared/structured-agent-session-reducer' +import { structuredAgentSessionDeliveryNotices } from './structured-agent-session-delivery-notices' +import { projectStructuredAgentSessionMessages } from './structured-agent-session-message-projection' + +const NO_CARDS: readonly string[] = [] +const MESSAGE_ID = agentJournalSubmissionKey('m') + +function answer(sequence: number): AgentJournalRenderItem { + return { + itemId: `a-${sequence}`, + revision: 1, + sequence, + observedAt: sequence, + body: { kind: 'message', role: 'assistant', blocks: [{ type: 'text', text: 'ok' }] } + } +} + +/** Where the older host left it: at its submission, far behind the loaded window. */ +const SENT: AgentJournalRenderItem = { + itemId: MESSAGE_ID, + revision: 1, + sequence: 10, + observedAt: 10, + body: { kind: 'message', role: 'user', blocks: [{ type: 'text', text: 'queued msg' }] } +} + +const REJECTED: AgentJournalSubmission = { + clientMessageId: 'm', + fence: 1, + payloadFingerprint: 'x', + dispatchState: 'rejected', + providerItemId: null, + reason: DISPATCH_REJECTED_HOST_RESTARTED, + rejection: { kind: 'hostRestarted' }, + submittedAt: 10, + resolvedAt: 2000 +} + +function page( + items: AgentJournalRenderItem[], + submissions: AgentJournalSubmission[], + hasOlder: boolean +): AgentSessionHistoryPage { + const oldest = items[0]?.sequence ?? 0 + const newest = items.at(-1)?.sequence ?? 0 + return { + sessionId: 's', + epoch: 'e', + direction: 'tail', + items, + removedItemIds: [], + submissions, + window: { + oldest: { epoch: 'e', sequence: oldest }, + newest: { epoch: 'e', sequence: newest }, + nextCursor: { epoch: 'e', sequence: oldest } + }, + liveCursor: { epoch: 'e', sequence: 1099 }, + hasOlder, + hasNewer: false + } +} + +it("keeps the outbox copy of a rejected message whose row is outside the window, then the host's row takes over", () => { + let state = reduceStructuredAgentSession(EMPTY_STRUCTURED_AGENT_SESSION, { + type: 'event', + event: { + type: 'snapshot', + sessionId: 's', + fence: 1, + page: page( + Array.from({ length: 100 }, (_, index) => answer(1000 + index)), + [], + true + ) + } + }) + // The rejection touches the row, which the older host left at its submission. + state = reduceStructuredAgentSession(state, { + type: 'event', + event: { + type: 'batch', + sessionId: 's', + batch: { + cursor: { epoch: 'e', sequence: 1100 }, + items: [SENT], + removedItemIds: [], + submissions: [REJECTED] + } + } + }) + expect(state.items.some((item) => item.itemId === MESSAGE_ID)).toBe(false) + expect(state.submissions.map((submission) => submission.clientMessageId)).toEqual(['m']) + + const sent = { + ...createStructuredAgentSessionOutboxEntry({ + clientMessageId: 'm', + sessionId: 's', + text: 'queued msg', + attachments: [], + queuedAt: 10 + }), + state: 'dispatching' as const + } + const kept = reconcileStructuredAgentSessionOutboxWithQueue( + [sent], + state.submissions, + state.items + ) + expect(kept).toMatchObject([{ clientMessageId: 'm', state: 'rejected' }]) + const drawn = (outbox: typeof kept) => + projectStructuredAgentSessionMessages(state.items, outbox, state.submissions, NO_CARDS) + .filter((message) => message.role === 'user') + .map(({ id, unsent }) => ({ id, unsent })) + expect(drawn(kept)).toEqual([{ id: MESSAGE_ID, unsent: true }]) + const notice = structuredAgentSessionDeliveryNotices( + kept, + 'Claude', + () => {}, + state.submissions, + [], + new Set() + ).get(MESSAGE_ID) + expect(notice).toEqual({ text: 'Orca restarted before this message was sent.' }) + + // Paging back loads the row: the outbox lets go, and the host's row is the one drawn. + state = reduceStructuredAgentSession(state, { + type: 'older-page', + requestedCursor: { epoch: 'e', sequence: 1000 }, + page: page( + [SENT, ...Array.from({ length: 10 }, (_, index) => answer(990 + index))], + [REJECTED], + false + ) + }) + expect(state.items.some((item) => item.itemId === MESSAGE_ID)).toBe(true) + const released = reconcileStructuredAgentSessionOutboxWithQueue( + kept, + state.submissions, + state.items + ) + expect(released).toEqual([]) + expect(drawn(released)).toEqual([{ id: MESSAGE_ID, unsent: true }]) +}) diff --git a/src/renderer/src/components/native-chat/structured-agent-session-message-projection.recorded-copy.test.ts b/src/renderer/src/components/native-chat/structured-agent-session-message-projection.recorded-copy.test.ts new file mode 100644 index 00000000000..84e10ca18b7 --- /dev/null +++ b/src/renderer/src/components/native-chat/structured-agent-session-message-projection.recorded-copy.test.ts @@ -0,0 +1,218 @@ +// An outbox copy of a message the host recorded and then rejected draws it only while the host's row +// is not loaded, and leaves, with no user action, on the batch or page that loads that row. It never +// offers a control: sending it again is a new message. + +import { expect, it } from 'vitest' +import { agentJournalSubmissionKey } from '../../../../shared/agent-session-journal-item-key' +import type { + AgentJournalRenderItem, + AgentJournalSubmission +} from '../../../../shared/agent-session-journal-types' +import type { AgentSessionHistoryPage } from '../../../../shared/agent-session-wire' +import { DISPATCH_REJECTED_HOST_RESTARTED } from '../../../../shared/structured-agent-session-dispatch-rejection' +import { reconcileStructuredAgentSessionOutboxWithQueue } from '../../../../shared/structured-agent-session-draft-hand-off' +import { + createStructuredAgentSessionOutboxEntry, + structuredAgentSessionRejectedFailure, + type StructuredAgentSessionOutboxEntry +} from '../../../../shared/structured-agent-session-outbox' +import { admitStructuredAgentSessionOutboxEntry } from '../../../../shared/structured-agent-session-outbox-admission' +import { + EMPTY_STRUCTURED_AGENT_SESSION, + reduceStructuredAgentSession, + type StructuredAgentSessionState +} from '../../../../shared/structured-agent-session-reducer' +import { structuredAgentSessionDeliveryNotices } from './structured-agent-session-delivery-notices' +import { projectStructuredAgentSessionMessages } from './structured-agent-session-message-projection' + +const NO_CARDS: readonly string[] = [] +const MESSAGE_ID = agentJournalSubmissionKey('m') +const WORDS = 'Orca restarted before this message was sent.' + +function answer(sequence: number): AgentJournalRenderItem { + return { + itemId: `a-${sequence}`, + revision: 1, + sequence, + observedAt: sequence, + body: { kind: 'message', role: 'assistant', blocks: [{ type: 'text', text: 'ok' }] } + } +} + +function hostRow(sequence: number): AgentJournalRenderItem { + return { + itemId: MESSAGE_ID, + revision: 1, + sequence, + observedAt: sequence, + body: { kind: 'message', role: 'user', blocks: [{ type: 'text', text: 'sent text' }] } + } +} + +const REJECTED: AgentJournalSubmission = { + clientMessageId: 'm', + fence: 1, + payloadFingerprint: 'm', + dispatchState: 'rejected', + providerItemId: null, + reason: DISPATCH_REJECTED_HOST_RESTARTED, + rejection: { kind: 'hostRestarted' }, + submittedAt: 10, + resolvedAt: 1100 +} + +function copy(patch: Partial = {}) { + return { + ...createStructuredAgentSessionOutboxEntry({ + clientMessageId: 'm', + sessionId: 's', + text: 'sent text', + attachments: [], + queuedAt: 10 + }), + ...patch + } +} + +/** A copy the host's own reply already marked rejected, as the outbox stores it. */ +const RECORDED_COPY = copy({ + state: 'rejected', + lastFailure: structuredAgentSessionRejectedFailure(REJECTED) +}) + +function page( + items: AgentJournalRenderItem[], + submissions: AgentJournalSubmission[] +): AgentSessionHistoryPage { + const oldest = items[0]?.sequence ?? 0 + return { + sessionId: 's', + epoch: 'e', + direction: 'tail', + items, + removedItemIds: [], + submissions, + window: { + oldest: { epoch: 'e', sequence: oldest }, + newest: { epoch: 'e', sequence: items.at(-1)?.sequence ?? 0 }, + nextCursor: { epoch: 'e', sequence: oldest } + }, + liveCursor: { epoch: 'e', sequence: items.at(-1)?.sequence ?? 0 }, + hasOlder: true, + hasNewer: false + } +} + +function opened(items: AgentJournalRenderItem[], submissions: AgentJournalSubmission[]) { + return reduceStructuredAgentSession(EMPTY_STRUCTURED_AGENT_SESSION, { + type: 'event', + event: { type: 'snapshot', sessionId: 's', fence: 1, page: page(items, submissions) } + }) +} + +const WINDOW = Array.from({ length: 100 }, (_, index) => answer(1000 + index)) + +/** What the chat draws for this message, and the notice its row carries. */ +function shown( + state: StructuredAgentSessionState, + outbox: readonly StructuredAgentSessionOutboxEntry[] +) { + const kept = reconcileStructuredAgentSessionOutboxWithQueue( + outbox, + state.submissions, + state.items + ) + const rows = projectStructuredAgentSessionMessages(state.items, kept, state.submissions, NO_CARDS) + .filter((message) => message.role === 'user') + .map(({ id, unsent }) => ({ id, unsent })) + const notice = structuredAgentSessionDeliveryNotices( + kept, + 'Claude', + () => {}, + state.submissions, + [], + new Set() + ).get(MESSAGE_ID) + return { kept, rows, notice } +} + +it('leaves on the live batch that rejects it on a host that places the row at the rejection', () => { + let state = opened(WINDOW, []) + const sent = copy({ state: 'dispatching', lastAttemptAt: 9 }) + state = reduceStructuredAgentSession(state, { + type: 'event', + event: { + type: 'batch', + sessionId: 's', + batch: { + cursor: { epoch: 'e', sequence: 1100 }, + items: [hostRow(1100)], + removedItemIds: [], + submissions: [REJECTED] + } + } + }) + + expect(shown(state, [sent])).toEqual({ + kept: [], + rows: [{ id: MESSAGE_ID, unsent: true }], + notice: { text: WORDS } + }) +}) + +it('leaves on the page that opens the chat when that page holds the row', () => { + const state = opened([...WINDOW, hostRow(1100)], [REJECTED]) + + expect(shown(state, [RECORDED_COPY])).toEqual({ + kept: [], + rows: [{ id: MESSAGE_ID, unsent: true }], + notice: { text: WORDS } + }) +}) + +it('draws once, with no control, while its row is outside the window, and leaves when that page loads', () => { + // An older host leaves the row where it was sent; the live batch brings only its record. + let state = reduceStructuredAgentSession(opened(WINDOW, []), { + type: 'event', + event: { + type: 'batch', + sessionId: 's', + batch: { + cursor: { epoch: 'e', sequence: 1100 }, + items: [hostRow(10)], + removedItemIds: [], + submissions: [REJECTED] + } + } + }) + const before = shown(state, [copy({ state: 'dispatching', lastAttemptAt: 9 })]) + expect(before.kept).toMatchObject([{ clientMessageId: 'm', state: 'rejected' }]) + expect(before.rows).toEqual([{ id: MESSAGE_ID, unsent: true }]) + expect(before.notice).toEqual({ text: WORDS }) + + // Scrolling back loads the page; no control on the copy is involved. + state = reduceStructuredAgentSession(state, { + type: 'older-page', + requestedCursor: { epoch: 'e', sequence: 1000 }, + page: page( + [hostRow(10), ...Array.from({ length: 5 }, (_, index) => answer(995 + index))], + [REJECTED] + ) + }) + expect(shown(state, before.kept)).toEqual({ + kept: [], + rows: [{ id: MESSAGE_ID, unsent: true }], + notice: { text: WORDS } + }) +}) + +it('keeps a stored copy whose record is not held drawn, with no control, and never sends it', () => { + const state = opened(WINDOW, []) + const after = shown(state, [RECORDED_COPY]) + + expect(after.kept).toEqual([RECORDED_COPY]) + expect(after.rows).toEqual([{ id: MESSAGE_ID, unsent: true }]) + expect(after.notice).toEqual({ text: WORDS }) + // The drain passes over it: nothing it holds is ever on its way. + expect(admitStructuredAgentSessionOutboxEntry(after.kept)).toEqual({ state: 'idle', entry: null }) +}) diff --git a/src/renderer/src/components/native-chat/structured-agent-session-message-projection.rejected-in-place.test.ts b/src/renderer/src/components/native-chat/structured-agent-session-message-projection.rejected-in-place.test.ts new file mode 100644 index 00000000000..4e757705225 --- /dev/null +++ b/src/renderer/src/components/native-chat/structured-agent-session-message-projection.rejected-in-place.test.ts @@ -0,0 +1,465 @@ +// A message the host accepted and then rejected stays in the desktop's chat where it was sent, +// marked not sent, from the host's own history: a crash can lose the outbox, never the host's row. + +import { describe, expect, it } from 'vitest' +import type { + AgentJournalRenderItem, + AgentJournalSubmission +} from '../../../../shared/agent-session-journal-types' +import { agentJournalSubmissionKey } from '../../../../shared/agent-session-journal-item-key' +import { + DISPATCH_REJECTED_CANCELLED, + DISPATCH_REJECTED_HOST_RESTARTED +} from '../../../../shared/structured-agent-session-dispatch-rejection' +import { + projectStructuredAgentSessionMessages as projectShared, + structuredAgentSessionCommandItemIds +} from '../../../../shared/structured-agent-session-message-projection' +import { + createStructuredAgentSessionOutboxEntry, + type StructuredAgentSessionOutboxEntry +} from '../../../../shared/structured-agent-session-outbox' +import { structuredAgentSessionDeliveryNotices } from './structured-agent-session-delivery-notices' +import { projectStructuredAgentSessionMessages } from './structured-agent-session-message-projection' + +const NO_CARDS: readonly string[] = [] + +const SESSION = 'session-1' + +function body(text: string) { + return { + kind: 'message' as const, + role: 'user' as const, + blocks: [{ type: 'text' as const, text }] + } +} + +// Same text, same fingerprint, as the host's body-only hash gives. +function fingerprint(text: string): string { + return `body:${text}` +} + +function userItem(id: string, sequence: number, text: string): AgentJournalRenderItem { + return { + itemId: agentJournalSubmissionKey(id), + revision: 1, + sequence, + observedAt: sequence, + body: body(text) + } +} + +function answer(sequence: number): AgentJournalRenderItem { + return { + itemId: `answer-${sequence}`, + revision: 1, + sequence, + observedAt: sequence, + body: { kind: 'message', role: 'assistant', blocks: [{ type: 'text', text: 'ok' }] } + } +} + +function submission( + id: string, + text: string, + submittedAt: number, + patch: Partial = {} +): AgentJournalSubmission { + return { + clientMessageId: id, + fence: 1, + payloadFingerprint: fingerprint(text), + dispatchState: 'accepted', + providerItemId: `provider-${id}`, + reason: null, + submittedAt, + resolvedAt: submittedAt, + ...patch + } +} + +/** Rejected on the next open after a crash, before the agent ever got it. */ +function restartRejected(id: string, text: string, submittedAt: number): AgentJournalSubmission { + return submission(id, text, submittedAt, { + dispatchState: 'rejected', + providerItemId: null, + reason: DISPATCH_REJECTED_HOST_RESTARTED, + rejection: { kind: 'hostRestarted' } + }) +} + +function withdrawn(id: string, text: string, submittedAt: number): AgentJournalSubmission { + return submission(id, text, submittedAt, { + dispatchState: 'rejected', + providerItemId: null, + reason: DISPATCH_REJECTED_CANCELLED, + rejection: { kind: 'cancelled' } + }) +} + +function outboxEntry( + id: string, + text: string, + patch: Partial = {} +): StructuredAgentSessionOutboxEntry { + return { + ...createStructuredAgentSessionOutboxEntry({ + clientMessageId: id, + sessionId: SESSION, + text, + attachments: [], + queuedAt: 50 + }), + ...patch + } +} + +function rows(messages: ReturnType) { + return messages + .filter((message) => message.role === 'user') + .map((message) => ({ + id: message.id, + text: message.blocks[0]?.type === 'text' ? message.blocks[0].text : null, + unsent: message.unsent ?? false + })) +} + +const SEED = submission('seed', 'seed', 1) +const SEED_ROWS = [userItem('seed', 1, 'seed'), answer(2)] + +describe('a message the host accepted and then rejected, on the desktop', () => { + it('stays where it was sent, as not sent, with no outbox entry left after a crash', () => { + const items = [...SEED_ROWS, userItem('lost', 3, 'fix the parser')] + const messages = projectStructuredAgentSessionMessages( + items, + [], + [SEED, restartRejected('lost', 'fix the parser', 3)], + NO_CARDS + ) + + expect(rows(messages)).toEqual([ + { id: agentJournalSubmissionKey('seed'), text: 'seed', unsent: false }, + { id: agentJournalSubmissionKey('lost'), text: 'fix the parser', unsent: true } + ]) + // Its place is the host's: the row keeps the journal position it was recorded at. + expect( + messages.find((message) => message.id === agentJournalSubmissionKey('lost')) + ).toMatchObject({ journalPosition: { sequence: 3, index: 0 }, source: 'transcript' }) + }) + + // An older host never moves it to its rejection: it stays at its submission, still drawn. + it('keeps the position an older host gave it, however far back', () => { + const later = Array.from({ length: 300 }, (_, index) => answer(4 + index)) + const items = [...SEED_ROWS, userItem('waited', 3, 'fix the parser'), ...later] + const messages = projectStructuredAgentSessionMessages( + items, + [], + [SEED, restartRejected('waited', 'fix the parser', 3)], + NO_CARDS + ) + + expect( + messages.find((message) => message.id === agentJournalSubmissionKey('waited')) + ).toMatchObject({ + unsent: true, + journalPosition: { sequence: 3, index: 0 } + }) + }) + + it('says why from the host fact, with no Retry', () => { + const notices = structuredAgentSessionDeliveryNotices( + [], + 'Claude', + () => {}, + [SEED, restartRejected('lost', 'fix the parser', 3)], + [], + new Set() + ) + + const notice = notices.get(agentJournalSubmissionKey('lost')) + expect(notice?.text).toBe('Orca restarted before this message was sent.') + expect(notice?.onRetry).toBeUndefined() + expect([...notices.keys()]).toEqual([agentJournalSubmissionKey('lost')]) + }) + + it("says only that it was not sent when the failed start's row already says why", () => { + const failedStart = submission('first', 'hello', 3, { + dispatchState: 'rejected', + providerItemId: null, + reason: 'Claude is not signed in.', + rejection: { kind: 'notSignedIn' } + }) + const notices = structuredAgentSessionDeliveryNotices( + [], + 'Claude', + () => {}, + [failedStart], + [{ kind: 'notSignedIn' }], + new Set() + ) + + expect(notices.get(agentJournalSubmissionKey('first'))).toEqual({ + text: 'Your message was not sent.' + }) + }) + + it('is hidden by a copy of the same body sent once its rejection was known', () => { + // An earlier build's Retry resent it under a new id, and that copy was delivered. + const items = [...SEED_ROWS, userItem('old', 3, 'retry me'), userItem('resent', 4, 'retry me')] + const submissions = [ + SEED, + restartRejected('old', 'retry me', 3), + submission('resent', 'retry me', 4) + ] + + expect(rows(projectStructuredAgentSessionMessages(items, [], submissions, NO_CARDS))).toEqual([ + { id: agentJournalSubmissionKey('seed'), text: 'seed', unsent: false }, + { id: agentJournalSubmissionKey('resent'), text: 'retry me', unsent: false } + ]) + }) + + it('stays when the same text was sent again before it was rejected', () => { + // "continue", sent twice on purpose; a restart rejected the first only after the second went. + const items = [...SEED_ROWS, userItem('first', 3, 'continue'), userItem('again', 4, 'continue')] + const submissions = [ + SEED, + { ...restartRejected('first', 'continue', 3), resolvedAt: 5 }, + submission('again', 'continue', 4) + ] + + expect(rows(projectStructuredAgentSessionMessages(items, [], submissions, NO_CARDS))).toEqual([ + { id: agentJournalSubmissionKey('seed'), text: 'seed', unsent: false }, + { id: agentJournalSubmissionKey('again'), text: 'continue', unsent: false }, + // Listed after the delivered rows; its journal position keeps its place. + { id: agentJournalSubmissionKey('first'), text: 'continue', unsent: true } + ]) + }) + + // The host re-delivers its own message under new ids; however close their times, one row stays. + it('keeps the last of several copies rejected in the same instant', () => { + const items = [...SEED_ROWS, userItem('a', 3, 'pointer'), userItem('b', 4, 'pointer')] + const submissions = [ + SEED, + restartRejected('a', 'pointer', 5), + restartRejected('b', 'pointer', 5) + ] + + expect(rows(projectStructuredAgentSessionMessages(items, [], submissions, NO_CARDS))).toEqual([ + { id: agentJournalSubmissionKey('seed'), text: 'seed', unsent: false }, + { id: agentJournalSubmissionKey('b'), text: 'pointer', unsent: true } + ]) + }) + + it('stays when the same body was only sent before it, or by a copy a Stop withdrew', () => { + const items = [ + ...SEED_ROWS, + userItem('first', 3, 'again'), + userItem('failed', 4, 'again'), + userItem('stopped', 5, 'again') + ] + const submissions = [ + SEED, + submission('first', 'again', 3), + restartRejected('failed', 'again', 4), + withdrawn('stopped', 'again', 5) + ] + + expect(rows(projectStructuredAgentSessionMessages(items, [], submissions, NO_CARDS))).toEqual([ + { id: agentJournalSubmissionKey('seed'), text: 'seed', unsent: false }, + { id: agentJournalSubmissionKey('first'), text: 'again', unsent: false }, + { id: agentJournalSubmissionKey('failed'), text: 'again', unsent: true } + ]) + }) + + // Its own reply reports the rejection, in the composer, as a command's. + it('is not drawn when it was a command such as /compact', () => { + const compact = { + ...userItem('compact', 3, '/compact'), + body: { ...body('/compact'), command: { name: 'compact' } } + } + const submissions = [SEED, restartRejected('compact', '/compact', 3)] + + expect( + rows( + projectStructuredAgentSessionMessages([...SEED_ROWS, compact], [], submissions, NO_CARDS) + ) + ).toEqual([{ id: agentJournalSubmissionKey('seed'), text: 'seed', unsent: false }]) + // One rule decides for the rows and the notices. + expect( + structuredAgentSessionDeliveryNotices( + [], + 'Claude', + () => {}, + submissions, + [], + new Set(), + NO_CARDS, + structuredAgentSessionCommandItemIds([...SEED_ROWS, compact]) + ).size + ).toBe(0) + }) + + it('keeps a message a Stop withdrew hidden: it went back to its sender', () => { + const items = [...SEED_ROWS, userItem('stopped', 3, 'never mind')] + const submissions = [SEED, withdrawn('stopped', 'never mind', 3)] + + expect(rows(projectStructuredAgentSessionMessages(items, [], submissions, NO_CARDS))).toEqual([ + { id: agentJournalSubmissionKey('seed'), text: 'seed', unsent: false } + ]) + expect( + structuredAgentSessionDeliveryNotices([], 'Claude', () => {}, submissions, [], new Set()).size + ).toBe(0) + }) +}) + +describe("one row per rejected message, the host's once it records the rejection", () => { + const items = [...SEED_ROWS, userItem('held', 3, 'host copy')] + // Fingerprinted from the host's own copy, so only the shared id ties the two rows together. + const rejected = restartRejected('held', 'host copy', 3) + const held = outboxEntry('held', 'outbox copy', { + state: 'rejected', + lastFailure: { + kind: 'rejected', + reason: DISPATCH_REJECTED_HOST_RESTARTED, + rejection: { kind: 'hostRestarted' } + } + }) + const heldRow = { id: agentJournalSubmissionKey('held'), text: 'outbox copy', unsent: true } + const hostRow = { id: agentJournalSubmissionKey('held'), text: 'host copy', unsent: true } + const seedRow = { id: agentJournalSubmissionKey('seed'), text: 'seed', unsent: false } + + it('the reply first: the outbox draws it, saying why, with no Retry', () => { + const messages = projectStructuredAgentSessionMessages(SEED_ROWS, [held], [SEED], NO_CARDS) + expect(rows(messages)).toEqual([seedRow, heldRow]) + const notice = structuredAgentSessionDeliveryNotices( + [held], + 'Claude', + () => {}, + [SEED], + [], + new Set(['held']) + ).get(heldRow.id) + expect(notice?.text).toBe('Orca restarted before this message was sent.') + expect(notice?.onRetry).toBeUndefined() + }) + + it("the journal first, or next: the host's row replaces the outbox copy at once", () => { + const dispatching = { ...held, state: 'dispatching' as const, lastFailure: undefined } + expect( + rows(projectStructuredAgentSessionMessages(SEED_ROWS, [dispatching], [SEED], NO_CARDS)) + ).toEqual([seedRow, { ...heldRow, unsent: false }]) + for (const entry of [dispatching, held]) { + // Before the reconcile drops the entry, the host's row is already the one row. + const messages = projectStructuredAgentSessionMessages( + items, + [entry], + [SEED, rejected], + NO_CARDS + ) + expect(rows(messages)).toEqual([seedRow, hostRow]) + expect(messages.at(-1)?.journalPosition).toEqual({ sequence: 3, index: 0 }) + const notices = structuredAgentSessionDeliveryNotices( + [entry], + 'Claude', + () => {}, + [SEED, rejected], + [], + new Set() + ) + // In the host's words, with no control. + expect([...notices]).toEqual([ + [hostRow.id, { text: 'Orca restarted before this message was sent.' }] + ]) + } + }) + + it("keeps the old row beside an earlier build's resend until the host records the resend", () => { + const hostItems = [...SEED_ROWS, userItem('held', 3, 'outbox copy')] + const resent = restartRejected('held', 'outbox copy', 3) + const resend = outboxEntry('resend', 'outbox copy') + expect( + rows(projectStructuredAgentSessionMessages(hostItems, [resend], [SEED, resent], NO_CARDS)) + ).toEqual([ + seedRow, + { id: agentJournalSubmissionKey('held'), text: 'outbox copy', unsent: true }, + { id: agentJournalSubmissionKey('resend'), text: 'outbox copy', unsent: false } + ]) + + const recorded = submission('resend', 'outbox copy', 60, { + dispatchState: 'pending', + providerItemId: null, + resolvedAt: null + }) + expect( + rows( + projectStructuredAgentSessionMessages( + [...hostItems, userItem('resend', 4, 'outbox copy')], + [resend], + [SEED, resent, recorded], + NO_CARDS + ) + ) + ).toEqual([ + seedRow, + { id: agentJournalSubmissionKey('resend'), text: 'outbox copy', unsent: false } + ]) + }) +}) + +describe('a rejected message the queue holds', () => { + const seedRow = { id: agentJournalSubmissionKey('seed'), text: 'seed', unsent: false } + + // A hand-off the host rejected sends its draft back to the card list, which shows the text. + it('is not drawn when it was a queued draft handed off', () => { + const items = [...SEED_ROWS, userItem('handoff', 3, 'queued text')] + const submissions = [ + SEED, + { ...restartRejected('handoff', 'queued text', 3), queuedMessageId: 'card-1' } + ] + + expect(rows(projectStructuredAgentSessionMessages(items, [], submissions, NO_CARDS))).toEqual([ + seedRow + ]) + expect( + structuredAgentSessionDeliveryNotices([], 'Claude', () => {}, submissions, [], new Set()).size + ).toBe(0) + }) + + // A send kept across a restart comes back as a paused card under its own id. + it('is not drawn while a card holds it under its id, and is drawn once that card is gone', () => { + const items = [...SEED_ROWS, userItem('kept', 3, 'kept text')] + const submissions = [SEED, restartRejected('kept', 'kept text', 3)] + + expect(rows(projectStructuredAgentSessionMessages(items, [], submissions, ['kept']))).toEqual([ + seedRow + ]) + expect( + structuredAgentSessionDeliveryNotices([], 'Claude', () => {}, submissions, [], new Set(), [ + 'kept' + ]).size + ).toBe(0) + expect(rows(projectStructuredAgentSessionMessages(items, [], submissions, []))).toEqual([ + seedRow, + { id: agentJournalSubmissionKey('kept'), text: 'kept text', unsent: true } + ]) + }) +}) + +describe('the phone', () => { + it('still hides every accepted-then-rejected message: it gives the text back to its composer', () => { + const items = [ + ...SEED_ROWS, + userItem('lost', 3, 'fix the parser'), + userItem('stopped', 4, 'never mind') + ] + const submissions = [ + SEED, + restartRejected('lost', 'fix the parser', 3), + withdrawn('stopped', 'never mind', 4) + ] + + expect(rows(projectShared(items, [], submissions, { rejectedInPlace: false }))).toEqual([ + { id: agentJournalSubmissionKey('seed'), text: 'seed', unsent: false } + ]) + }) +}) diff --git a/src/renderer/src/components/native-chat/structured-agent-session-message-projection.test.ts b/src/renderer/src/components/native-chat/structured-agent-session-message-projection.test.ts index 3a3562458cb..bd99e8f8cbb 100644 --- a/src/renderer/src/components/native-chat/structured-agent-session-message-projection.test.ts +++ b/src/renderer/src/components/native-chat/structured-agent-session-message-projection.test.ts @@ -7,6 +7,8 @@ import { agentJournalSubmissionKey } from '../../../../shared/agent-session-jour import { createStructuredAgentSessionOutboxEntry } from '../../../../shared/structured-agent-session-outbox' import { projectStructuredAgentSessionMessages } from './structured-agent-session-message-projection' +const NO_CARDS: readonly string[] = [] + function submission(index: number): AgentJournalSubmission { return { clientMessageId: `client-${index}`, @@ -31,16 +33,8 @@ function item(index: number): AgentJournalRenderItem { } describe('structured agent session message projection', () => { - it('does not render a rejected host submission as a sent user message', () => { - const rejected = { ...submission(0), dispatchState: 'rejected' as const, providerItemId: null } - const refusedItem = { ...item(0), itemId: agentJournalSubmissionKey(rejected.clientMessageId) } - const acceptedItem = item(1) - expect( - projectStructuredAgentSessionMessages([refusedItem, acceptedItem], [], [rejected]) - ).toMatchObject([{ id: acceptedItem.itemId, role: 'user' }]) - }) - - it('keeps a refused local draft available through its outbox', () => { + // The host recorded it, so its row is the message from here; the outbox copy gives way. + it("draws a send the host rejected from the host's row, not the outbox copy", () => { const rejected = { ...submission(0), dispatchState: 'rejected' as const, providerItemId: null } const refusedItem = { ...item(0), itemId: agentJournalSubmissionKey(rejected.clientMessageId) } const draft = createStructuredAgentSessionOutboxEntry({ @@ -50,9 +44,15 @@ describe('structured agent session message projection', () => { attachments: [], queuedAt: 1 }) - expect(projectStructuredAgentSessionMessages([refusedItem], [draft], [rejected])).toMatchObject( - [{ id: refusedItem.itemId, blocks: [{ text: 'An unsent draft' }] }] - ) + expect( + projectStructuredAgentSessionMessages([refusedItem], [draft], [rejected], NO_CARDS) + ).toEqual([ + expect.objectContaining({ + id: refusedItem.itemId, + blocks: [{ type: 'text', text: 'send 0' }], + unsent: true + }) + ]) }) it.each([5, 10])('renders %i rapid accepted desktop sends exactly once', (sendCount) => { @@ -68,7 +68,8 @@ describe('structured agent session message projection', () => { const messages = projectStructuredAgentSessionMessages( Array.from({ length: sendCount }, (_, index) => item(index)), outbox, - Array.from({ length: sendCount }, (_, index) => submission(sendCount - index - 1)) + Array.from({ length: sendCount }, (_, index) => submission(sendCount - index - 1)), + NO_CARDS ) expect(messages.filter((message) => message.role === 'user')).toHaveLength(sendCount) @@ -103,8 +104,8 @@ describe('structured agent session message projection', () => { resolvedAt: null } - const messages = projectStructuredAgentSessionMessages([walItem], outbox, [pending]) - const optimistic = projectStructuredAgentSessionMessages([], outbox, []) + const messages = projectStructuredAgentSessionMessages([walItem], outbox, [pending], NO_CARDS) + const optimistic = projectStructuredAgentSessionMessages([], outbox, [], NO_CARDS) expect(messages.filter((message) => message.role === 'user')).toHaveLength(1) expect(messages.map((message) => message.id)).toEqual([walItem.itemId]) @@ -122,7 +123,7 @@ describe('structured agent session message projection', () => { }) ] - expect(projectStructuredAgentSessionMessages([], outbox, [])).toMatchObject([ + expect(projectStructuredAgentSessionMessages([], outbox, [], NO_CARDS)).toMatchObject([ { id: agentJournalSubmissionKey('client-pending'), role: 'user' } ]) }) diff --git a/src/renderer/src/components/native-chat/structured-agent-session-message-projection.ts b/src/renderer/src/components/native-chat/structured-agent-session-message-projection.ts index 0ecfd7aa2f5..f527442c2b1 100644 --- a/src/renderer/src/components/native-chat/structured-agent-session-message-projection.ts +++ b/src/renderer/src/components/native-chat/structured-agent-session-message-projection.ts @@ -6,12 +6,22 @@ import type { StructuredAgentSessionOutboxEntry } from '../../../../shared/struc import { projectStructuredAgentSessionMessages as projectMessages } from '../../../../shared/structured-agent-session-message-projection' import { projectStructuredQuestionMessages } from './structured-agent-question-projection' +/** The desktop's transcript: a message the host accepted and then rejected stays where it was + * sent, as not sent, unless a queued card holds it. */ export function projectStructuredAgentSessionMessages( items: readonly AgentJournalRenderItem[], outbox: readonly StructuredAgentSessionOutboxEntry[], - submissions: readonly AgentJournalSubmission[] + submissions: readonly AgentJournalSubmission[], + /** The queue's live cards; required, since a rejected message a card holds must not draw twice. */ + queuedMessageIds: readonly string[] ) { - return projectMessages(items, outbox, submissions, projectStructuredQuestionMessages) + return projectMessages( + items, + outbox, + submissions, + { rejectedInPlace: true, queuedMessageIds }, + projectStructuredQuestionMessages + ) } export type StructuredPromptItem = AgentJournalRenderItem & { diff --git a/src/renderer/src/components/native-chat/structured-agent-session-outbox-retry.ts b/src/renderer/src/components/native-chat/structured-agent-session-outbox-retry.ts index 73619e71759..d431cccb036 100644 --- a/src/renderer/src/components/native-chat/structured-agent-session-outbox-retry.ts +++ b/src/renderer/src/components/native-chat/structured-agent-session-outbox-retry.ts @@ -4,6 +4,7 @@ import type { AgentJournalSubmission } from '../../../../shared/agent-session-journal-types' import { structuredAgentSessionEntryIdExpired, + structuredAgentSessionEntryRejectedByHost, type StructuredAgentSessionOutboxEntry } from '../../../../shared/structured-agent-session-outbox' import { @@ -30,7 +31,7 @@ export function retryStructuredAgentSessionOutboxEntry(args: { // settled the message already rotated it. An expired id is refused for good; its row told the // user to check the chat first. const recordedRejection = - current?.state === 'rejected' && current.lastFailure?.kind === 'rejected' + current !== undefined && structuredAgentSessionEntryRejectedByHost(current) if ( current && (recordedRejection || diff --git a/src/renderer/src/components/native-chat/structured-agent-session-outbox-storage.ts b/src/renderer/src/components/native-chat/structured-agent-session-outbox-storage.ts index ce270d60ddb..cbc74133d7a 100644 --- a/src/renderer/src/components/native-chat/structured-agent-session-outbox-storage.ts +++ b/src/renderer/src/components/native-chat/structured-agent-session-outbox-storage.ts @@ -1,6 +1,7 @@ import { createStructuredAgentSessionOutboxEntry, parseStructuredAgentSessionOutboxEntry, + structuredAgentSessionEntryRejectedByHost, type StructuredAgentSessionAttachment, type StructuredAgentSessionOutboxEntry } from '../../../../shared/structured-agent-session-outbox' @@ -54,9 +55,14 @@ function publishUndelivered(sessionId: string, undelivered: boolean): void { } } +/** Whether any entry still owes a delivery: a copy the host recorded and rejected owes none. */ +function owesDelivery(entries: readonly StructuredAgentSessionOutboxEntry[]): boolean { + return entries.some((entry) => !structuredAgentSessionEntryRejectedByHost(entry)) +} + /** Keep the journal subscription alive while this session still owes delivery. */ export function hasUndeliveredStructuredAgentSessionOutbox(sessionId: string): boolean { - return undeliveredSessions.get(sessionId)?.undelivered ?? readOutbox(sessionId).length > 0 + return undeliveredSessions.get(sessionId)?.undelivered ?? owesDelivery(readOutbox(sessionId)) } export function subscribeToUndeliveredStructuredAgentSessionOutbox( @@ -65,7 +71,7 @@ export function subscribeToUndeliveredStructuredAgentSessionOutbox( ): () => void { let subscription = undeliveredSessions.get(sessionId) if (!subscription) { - subscription = { undelivered: readOutbox(sessionId).length > 0, listeners: new Set() } + subscription = { undelivered: owesDelivery(readOutbox(sessionId)), listeners: new Set() } undeliveredSessions.set(sessionId, subscription) } const owned = subscription @@ -92,7 +98,7 @@ export function writeOutbox( } else { localStorage.setItem(storageKey(sessionId), JSON.stringify(entries)) } - publishUndelivered(sessionId, entries.length > 0) + publishUndelivered(sessionId, owesDelivery(entries)) return true } catch { return false diff --git a/src/renderer/src/components/native-chat/structured-agent-session-transcript-order.test.ts b/src/renderer/src/components/native-chat/structured-agent-session-transcript-order.test.ts index aa774f7fbf3..3b12ed7b3dc 100644 --- a/src/renderer/src/components/native-chat/structured-agent-session-transcript-order.test.ts +++ b/src/renderer/src/components/native-chat/structured-agent-session-transcript-order.test.ts @@ -13,6 +13,8 @@ import { import { createNativeChatMessageListProjection } from './native-chat-message-list-projection' import { projectStructuredAgentSessionMessages } from './structured-agent-session-message-projection' +const NO_CARDS: readonly string[] = [] + function journalItem( itemId: string, sequence: number, @@ -43,7 +45,7 @@ function drawn( submissions: AgentJournalSubmission[] = [] ): string[] { return createNativeChatMessageListProjection()( - projectStructuredAgentSessionMessages(items, outbox, submissions) + projectStructuredAgentSessionMessages(items, outbox, submissions, NO_CARDS) ).conversation.map(({ id }) => id) } diff --git a/src/renderer/src/components/native-chat/structured-agent-session-withdrawn-message-restore.test.tsx b/src/renderer/src/components/native-chat/structured-agent-session-withdrawn-message-restore.test.tsx index 150b520bcd3..dde230110dd 100644 --- a/src/renderer/src/components/native-chat/structured-agent-session-withdrawn-message-restore.test.tsx +++ b/src/renderer/src/components/native-chat/structured-agent-session-withdrawn-message-restore.test.tsx @@ -75,6 +75,15 @@ import { import { useStructuredAgentSessionOutbox } from './use-structured-agent-session-outbox' import { useStructuredAgentSession } from './use-structured-agent-session' +/** What these hooks render with: the journal's submissions, and the rows loaded so far. */ +type OutboxProps = { submissions: AgentJournalSubmission[]; rows?: AgentJournalRenderItem[] } + +function outboxProps(submissions: AgentJournalSubmission[]): OutboxProps { + return { submissions } +} + +const NO_JOURNAL_ITEMS: readonly AgentJournalRenderItem[] = [] + const SESSION = 'session-1' const PANE = 'tab-1::session-1' const OTHER_PANE = 'tab-2::session-1' @@ -129,15 +138,16 @@ function answerSendsPending(): void { function renderOutbox(composerScopeKey: string | null = PANE) { return renderHook( - (props: { submissions: AgentJournalSubmission[] }) => + (props: OutboxProps) => useStructuredAgentSessionOutbox({ + journalItems: props.rows ?? NO_JOURNAL_ITEMS, sessionId: SESSION, target, fence: 1, submissions: props.submissions, ...(composerScopeKey ? { composerScopeKey } : {}) }), - { initialProps: { submissions: NONE } } + { initialProps: outboxProps(NONE) } ) } @@ -259,7 +269,7 @@ describe('a message the host withdrew at a Stop', () => { expect(readNativeChatDraftCache(PANE)).toBe('hello\n\nhello') }) - it('keeps a message refused for any other reason on its Retry, and gives nothing back', async () => { + it("leaves a message rejected for any other reason to the host's row, and gives nothing back", async () => { answerSendsPending() const { result, rerender } = renderOutbox() const id = await sendToHost(result, 'hello') @@ -267,10 +277,19 @@ describe('a message the host withdrew at a Stop', () => { rerender({ submissions: [ submission(id, { dispatchState: 'rejected', reason: DISPATCH_REJECTED_WRITE_FAILED }) + ], + rows: [ + { + itemId: `orca:${id}`, + revision: 1, + sequence: 1, + observedAt: 1, + body: { kind: 'message', role: 'user', blocks: [{ type: 'text', text: 'hello' }] } + } ] }) - await waitFor(() => expect(result.current.outbox[0]?.state).toBe('rejected')) + await waitFor(() => expect(result.current.outbox).toEqual([])) expect(readNativeChatDraftCache(PANE)).toBe('') }) diff --git a/src/renderer/src/components/native-chat/use-structured-agent-session-delivery-notices.test.tsx b/src/renderer/src/components/native-chat/use-structured-agent-session-delivery-notices.test.tsx new file mode 100644 index 00000000000..28ef88564b9 --- /dev/null +++ b/src/renderer/src/components/native-chat/use-structured-agent-session-delivery-notices.test.tsx @@ -0,0 +1,120 @@ +// @vitest-environment happy-dom + +import { cleanup, renderHook } from '@testing-library/react' +import { afterEach, expect, it } from 'vitest' +import type { AgentJournalSubmission } from '../../../../shared/agent-session-journal-types' +import { DISPATCH_REJECTED_CANCELLED } from '../../../../shared/structured-agent-session-dispatch-rejection' +import { useStructuredAgentSessionDeliveryNotices } from './use-structured-agent-session-delivery-notices' + +afterEach(cleanup) + +const NONE = new Set() +const NO_CARDS: readonly string[] = [] +const EMPTY: never[] = [] + +function rejected( + clientMessageId: string, + kind: 'hostRestarted' | 'notDelivered' +): AgentJournalSubmission { + return { + clientMessageId, + fence: 1, + payloadFingerprint: clientMessageId, + dispatchState: 'rejected', + providerItemId: null, + reason: null, + rejection: { kind }, + submittedAt: 1, + resolvedAt: 2 + } +} + +function accepted(clientMessageId: string): AgentJournalSubmission { + return { + clientMessageId, + fence: 1, + payloadFingerprint: clientMessageId, + dispatchState: 'accepted', + providerItemId: `provider-${clientMessageId}`, + reason: null, + submittedAt: 3, + resolvedAt: 4 + } +} + +function withdrawn(clientMessageId: string): AgentJournalSubmission { + return { + clientMessageId, + fence: 1, + payloadFingerprint: clientMessageId, + dispatchState: 'rejected', + providerItemId: null, + reason: DISPATCH_REJECTED_CANCELLED, + rejection: { kind: 'cancelled' }, + submittedAt: 1, + resolvedAt: 2 + } +} + +// A Stop's withdrawn message draws no row, so a chat that has one rebuilds no notice per batch. +it('keeps the same notices across batches in a chat whose only rejection a Stop withdrew', () => { + const { result, rerender } = renderHook( + ({ submissions }: { submissions: readonly AgentJournalSubmission[] }) => + useStructuredAgentSessionDeliveryNotices({ + outbox: EMPTY, + submissions, + journalItems: EMPTY, + failedHere: NONE, + queuedMessageIds: NO_CARDS, + retry: () => {}, + agentName: 'Claude' + }), + { initialProps: { submissions: [withdrawn('stopped')] } } + ) + const first = result.current + + rerender({ submissions: [withdrawn('stopped')] }) + + expect(result.current).toBe(first) + expect(result.current.size).toBe(0) +}) + +function renderNotices(submissions: readonly AgentJournalSubmission[]) { + return renderHook( + ({ submissions: current }: { submissions: readonly AgentJournalSubmission[] }) => + useStructuredAgentSessionDeliveryNotices({ + outbox: EMPTY, + submissions: current, + journalItems: EMPTY, + failedHere: NONE, + queuedMessageIds: NO_CARDS, + retry: () => {}, + agentName: 'Claude' + }), + { initialProps: { submissions } } + ) +} + +// A batch for another message rebuilds the journal's rows; the rows already marked not sent keep +// the same notices, so no row wrapper re-renders. +it('keeps the same notices when a batch leaves every not-sent message as it was', () => { + const { result, rerender } = renderNotices([rejected('lost', 'hostRestarted')]) + const first = result.current + expect(first.size).toBe(1) + + rerender({ submissions: [rejected('lost', 'hostRestarted'), accepted('next')] }) + + expect(result.current).toBe(first) +}) + +it('keeps the notice of a row that did not change when another one does', () => { + const { result, rerender } = renderNotices([rejected('lost', 'hostRestarted')]) + const lost = result.current.get('orca:lost') + + rerender({ + submissions: [rejected('lost', 'hostRestarted'), rejected('other', 'notDelivered')] + }) + + expect(result.current.size).toBe(2) + expect(result.current.get('orca:lost')).toBe(lost) +}) diff --git a/src/renderer/src/components/native-chat/use-structured-agent-session-delivery-notices.ts b/src/renderer/src/components/native-chat/use-structured-agent-session-delivery-notices.ts new file mode 100644 index 00000000000..26cdd00fb81 --- /dev/null +++ b/src/renderer/src/components/native-chat/use-structured-agent-session-delivery-notices.ts @@ -0,0 +1,120 @@ +import { useCallback, useEffect, useMemo, useRef } from 'react' +import type { + AgentJournalRenderItem, + AgentJournalSubmission +} from '../../../../shared/agent-session-journal-types' +import { dispatchWasWithdrawn } from '../../../../shared/structured-agent-session-dispatch-rejection' +import { structuredAgentSessionCommandItemIds } from '../../../../shared/structured-agent-session-message-projection' +import type { StructuredAgentSessionOutboxEntry } from '../../../../shared/structured-agent-session-outbox' +import { structuredAgentSessionDeliveryNotices } from './structured-agent-session-delivery-notices' +import { useStructuredAgentSessionStartFailureFacts } from './use-structured-agent-session-start-failure-facts' +import type { NativeChatDeliveryNotice } from './NativeChatMessageRow' + +const NO_SUBMISSIONS: readonly AgentJournalSubmission[] = [] + +/** The structured chat's delivery notices, by the message id each row renders under. */ +export function useStructuredAgentSessionDeliveryNotices(args: { + outbox: readonly StructuredAgentSessionOutboxEntry[] + submissions: readonly AgentJournalSubmission[] + journalItems: readonly AgentJournalRenderItem[] + failedHere: ReadonlySet + queuedMessageIds: readonly string[] + retry: (clientMessageId: string) => void + agentName: string +}): ReadonlyMap { + const { agentName, failedHere, outbox, queuedMessageIds, submissions } = args + // Read at click time, so the notices stay put while the outbox's Retry is rebuilt each render. + const retryRef = useRef(args.retry) + useEffect(() => { + retryRef.current = args.retry + }) + const retry = useCallback((clientMessageId: string) => { + retryRef.current(clientMessageId) + }, []) + // Only a message shown as not sent reads the journal's rows (a withdrawn one draws nothing), so + // in a chat without one a new batch of them re-renders no row. + const hasRejected = + outbox.some((entry) => entry.state === 'rejected') || + submissions.some( + (submission) => submission.dispatchState === 'rejected' && !dispatchWasWithdrawn(submission) + ) + const rejectionRows = hasRejected ? submissions : NO_SUBMISSIONS + const startFailures = useStructuredAgentSessionStartFailureFacts(args.journalItems, hasRejected) + const commandItemIds = useCommandItemIds(args.journalItems, hasRejected) + const notices = useMemo( + () => + structuredAgentSessionDeliveryNotices( + outbox, + agentName, + retry, + rejectionRows, + startFailures, + failedHere, + queuedMessageIds, + commandItemIds + ), + [ + outbox, + agentName, + retry, + rejectionRows, + startFailures, + failedHere, + queuedMessageIds, + commandItemIds + ] + ) + // A submission batch rebuilds the map; one that says the same keeps the old, so no row re-renders. + const previousRef = useRef(notices) + const stable = sameNoticesKept(previousRef.current, notices) + useEffect(() => { + previousRef.current = stable + }, [stable]) + return stable +} + +const NO_COMMANDS: ReadonlySet = new Set() + +/** The loaded commands, read only while `enabled`, and held while unchanged so a streaming turn + * rebuilds no notice. */ +function useCommandItemIds( + items: readonly AgentJournalRenderItem[], + enabled: boolean +): ReadonlySet { + const ids = useMemo( + () => (enabled ? structuredAgentSessionCommandItemIds(items) : NO_COMMANDS), + [enabled, items] + ) + const previousRef = useRef(ids) + const previous = previousRef.current + const stable = + previous.size === ids.size && [...ids].every((id) => previous.has(id)) ? previous : ids + useEffect(() => { + previousRef.current = stable + }, [stable]) + return stable +} + +/** `next`, reusing each notice `previous` words the same way, and `previous` itself when all are. */ +function sameNoticesKept( + previous: ReadonlyMap, + next: ReadonlyMap +): ReadonlyMap { + if (previous === next) { + return next + } + let allKept = previous.size === next.size + const kept = new Map() + for (const [id, notice] of next) { + const before = previous.get(id) + // Each Retry calls the stable `retry` with its own id, so one under the same key is the same. + const same = + before !== undefined && + before.text === notice.text && + (before.onRetry === undefined) === (notice.onRetry === undefined) && + (before.onDismiss === undefined) === (notice.onDismiss === undefined) + allKept &&= same + kept.set(id, same ? before : notice) + } + return allKept ? previous : kept +} diff --git a/src/renderer/src/components/native-chat/use-structured-agent-session-messages.test.tsx b/src/renderer/src/components/native-chat/use-structured-agent-session-messages.test.tsx index 68e388de58c..419e6a6cae9 100644 --- a/src/renderer/src/components/native-chat/use-structured-agent-session-messages.test.tsx +++ b/src/renderer/src/components/native-chat/use-structured-agent-session-messages.test.tsx @@ -11,6 +11,7 @@ import { useStructuredAgentSessionMessages } from './use-structured-agent-sessio afterEach(cleanup) const EMPTY: never[] = [] +const NO_CARDS: readonly string[] = [] function tool(id: string, sequence: number): AgentJournalRenderItem { return { itemId: id, @@ -25,7 +26,8 @@ it('retains only unchanged item projections across updates, reorder, deletion, a const first = tool('first', 1) const second = tool('second', 2) const { result, rerender } = renderHook( - (items: AgentJournalRenderItem[]) => useStructuredAgentSessionMessages(items, EMPTY, EMPTY), + (items: AgentJournalRenderItem[]) => + useStructuredAgentSessionMessages(items, EMPTY, EMPTY, NO_CARDS), { initialProps: [first, second] } ) const initial = result.current @@ -50,7 +52,9 @@ it('retains only unchanged item projections across updates, reorder, deletion, a [structuredClone(completed)] ]) { rerender(items) - expect(result.current).toEqual(projectStructuredAgentSessionMessages(items, EMPTY, EMPTY)) + expect(result.current).toEqual( + projectStructuredAgentSessionMessages(items, EMPTY, EMPTY, NO_CARDS) + ) expect(result.current.find((message) => message.id === 'second')).not.toBe(initial[1]) } const replacement = { @@ -62,7 +66,9 @@ it('retains only unchanged item projections across updates, reorder, deletion, a } } rerender([replacement]) - expect(result.current).toEqual(projectStructuredAgentSessionMessages([replacement], EMPTY, EMPTY)) + expect(result.current).toEqual( + projectStructuredAgentSessionMessages([replacement], EMPTY, EMPTY, NO_CARDS) + ) expect(result.current[0]).not.toBe(initial[0]) }) @@ -91,14 +97,14 @@ it('keeps optimistic sends and their settlement identical to uncached projection }: { items: AgentJournalRenderItem[] submissions: AgentJournalSubmission[] - }) => useStructuredAgentSessionMessages(items, [entry], submissions), + }) => useStructuredAgentSessionMessages(items, [entry], submissions, NO_CARDS), { initialProps: { items: [tool('tool', 1)], submissions: [submission] } } ) for (const dispatchState of ['pending', 'unknown', 'accepted'] as const) { const props = { items: [tool('tool', 1)], submissions: [{ ...submission, dispatchState }] } rerender(props) expect(result.current).toEqual( - projectStructuredAgentSessionMessages(props.items, [entry], props.submissions) + projectStructuredAgentSessionMessages(props.items, [entry], props.submissions, NO_CARDS) ) } }) @@ -106,7 +112,7 @@ it('keeps optimistic sends and their settlement identical to uncached projection it('does no transcript projection work on a status-only render', () => { const items = [tool('tool', 1)] const { result, rerender } = renderHook(() => - useStructuredAgentSessionMessages(items, EMPTY, EMPTY) + useStructuredAgentSessionMessages(items, EMPTY, EMPTY, NO_CARDS) ) const initial = result.current rerender() diff --git a/src/renderer/src/components/native-chat/use-structured-agent-session-messages.ts b/src/renderer/src/components/native-chat/use-structured-agent-session-messages.ts index f965c2d3eaa..af38cda41df 100644 --- a/src/renderer/src/components/native-chat/use-structured-agent-session-messages.ts +++ b/src/renderer/src/components/native-chat/use-structured-agent-session-messages.ts @@ -9,10 +9,11 @@ import { projectStructuredAgentSessionMessages } from './structured-agent-sessio export function useStructuredAgentSessionMessages( items: readonly AgentJournalRenderItem[], outbox: readonly StructuredAgentSessionOutboxEntry[], - submissions: readonly AgentJournalSubmission[] + submissions: readonly AgentJournalSubmission[], + queuedMessageIds: readonly string[] ) { return useMemo( - () => projectStructuredAgentSessionMessages(items, outbox, submissions), - [items, outbox, submissions] + () => projectStructuredAgentSessionMessages(items, outbox, submissions, queuedMessageIds), + [items, outbox, submissions, queuedMessageIds] ) } diff --git a/src/renderer/src/components/native-chat/use-structured-agent-session-outbox-admission.test.tsx b/src/renderer/src/components/native-chat/use-structured-agent-session-outbox-admission.test.tsx index 083a128ca0e..a810a331a2c 100644 --- a/src/renderer/src/components/native-chat/use-structured-agent-session-outbox-admission.test.tsx +++ b/src/renderer/src/components/native-chat/use-structured-agent-session-outbox-admission.test.tsx @@ -7,7 +7,10 @@ import { act, cleanup, renderHook, waitFor } from '@testing-library/react' import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' -import type { AgentJournalSubmission } from '../../../../shared/agent-session-journal-types' +import type { + AgentJournalRenderItem, + AgentJournalSubmission +} from '../../../../shared/agent-session-journal-types' import type { AgentSessionWireRefusalCode } from '../../../../shared/agent-session-wire' const mocks = vi.hoisted(() => ({ @@ -26,6 +29,8 @@ import { agentJournalSubmissionKey } from '../../../../shared/agent-session-jour import type { StructuredAgentSessionOutboxEntry } from '../../../../shared/structured-agent-session-outbox' import { structuredAgentSessionEntryHeldForRetry } from '../../../../shared/structured-agent-session-outbox-admission' +const NO_JOURNAL_ITEMS: readonly AgentJournalRenderItem[] = [] + const LOCAL_TARGET = { kind: 'local' } as const function deferred() { @@ -93,6 +98,7 @@ function refusedResult(code: AgentSessionWireRefusalCode) { function renderOutbox() { return renderHook(() => useStructuredAgentSessionOutbox({ + journalItems: NO_JOURNAL_ITEMS, sessionId: 'session-1', target: LOCAL_TARGET, fence: 1, @@ -230,6 +236,7 @@ describe('structured agent session outbox admission', () => { const { result, rerender } = renderHook( ({ submissions }: { submissions: readonly AgentJournalSubmission[] }) => useStructuredAgentSessionOutbox({ + journalItems: NO_JOURNAL_ITEMS, sessionId: 'session-1', target: LOCAL_TARGET, fence: 1, @@ -311,6 +318,7 @@ describe('structured agent session outbox admission', () => { const { result, rerender } = renderHook( ({ submissions }: { submissions: readonly AgentJournalSubmission[] }) => useStructuredAgentSessionOutbox({ + journalItems: NO_JOURNAL_ITEMS, sessionId: 'session-1', target: LOCAL_TARGET, fence: 1, diff --git a/src/renderer/src/components/native-chat/use-structured-agent-session-outbox-fence.test.tsx b/src/renderer/src/components/native-chat/use-structured-agent-session-outbox-fence.test.tsx index 9545c083fa7..86d1ddf8322 100644 --- a/src/renderer/src/components/native-chat/use-structured-agent-session-outbox-fence.test.tsx +++ b/src/renderer/src/components/native-chat/use-structured-agent-session-outbox-fence.test.tsx @@ -20,6 +20,10 @@ import { settleStructuredAgentLaunchPrompt } from '@/lib/structured-agent-sessio import { enqueueStructuredAgentSessionLaunchPrompt } from './structured-agent-session-outbox-storage' import { useStructuredAgentSessionOutbox } from './use-structured-agent-session-outbox' +import type { AgentJournalRenderItem } from '../../../../shared/agent-session-journal-types' + +const NO_JOURNAL_ITEMS: readonly AgentJournalRenderItem[] = [] + // Why: every hook here shares the session outbox store; one left mounted would drain the next test's. afterEach(cleanup) @@ -65,6 +69,7 @@ function render(fence: number | null = 1) { return renderHook( (props) => useStructuredAgentSessionOutbox({ + journalItems: NO_JOURNAL_ITEMS, sessionId: 'session-1', target: LOCAL_TARGET, fence: props.fence, diff --git a/src/renderer/src/components/native-chat/use-structured-agent-session-outbox-owner-change.test.tsx b/src/renderer/src/components/native-chat/use-structured-agent-session-outbox-owner-change.test.tsx index 926d77d4794..fcc0d2a6c62 100644 --- a/src/renderer/src/components/native-chat/use-structured-agent-session-outbox-owner-change.test.tsx +++ b/src/renderer/src/components/native-chat/use-structured-agent-session-outbox-owner-change.test.tsx @@ -34,6 +34,10 @@ import { import { writeOutbox } from './structured-agent-session-outbox-storage' import { useStructuredAgentSessionOutbox } from './use-structured-agent-session-outbox' +import type { AgentJournalRenderItem } from '../../../../shared/agent-session-journal-types' + +const NO_JOURNAL_ITEMS: readonly AgentJournalRenderItem[] = [] + type SendRequest = { envelope?: { clientOperationId?: string; expectedRuntimeFence?: number } } // Stable, as the view passes it: a new object each render would re-run the owner-change requeue. @@ -82,6 +86,7 @@ function mount(fence: number) { return renderHook( ({ fence: current }: { fence: number }) => useStructuredAgentSessionOutbox({ + journalItems: NO_JOURNAL_ITEMS, sessionId: 'session-1', target: TARGET, fence: current, diff --git a/src/renderer/src/components/native-chat/use-structured-agent-session-outbox-rejection-cause.test.tsx b/src/renderer/src/components/native-chat/use-structured-agent-session-outbox-rejection-cause.test.tsx index 8bf95a8a511..e32006c50d0 100644 --- a/src/renderer/src/components/native-chat/use-structured-agent-session-outbox-rejection-cause.test.tsx +++ b/src/renderer/src/components/native-chat/use-structured-agent-session-outbox-rejection-cause.test.tsx @@ -2,7 +2,10 @@ import { act, cleanup, renderHook, waitFor } from '@testing-library/react' import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' -import type { AgentJournalSubmission } from '../../../../shared/agent-session-journal-types' +import type { + AgentJournalRenderItem, + AgentJournalSubmission +} from '../../../../shared/agent-session-journal-types' import { DISPATCH_REJECTED_CANCELLED } from '../../../../shared/structured-agent-session-dispatch-rejection' const mocks = vi.hoisted(() => ({ @@ -16,13 +19,28 @@ vi.mock('@/runtime/structured-agent-session-client', () => ({ import { setLocalRuntimeCapabilitiesForTests } from '@/runtime/local-runtime-capabilities' import { RuntimeRpcCallError } from '@/runtime/runtime-rpc-result' import { useStructuredAgentSessionOutbox } from './use-structured-agent-session-outbox' +import { structuredAgentSessionDeliveryNotices } from './structured-agent-session-delivery-notices' +import { agentJournalSubmissionKey } from '../../../../shared/agent-session-journal-item-key' import { agentSessionWriteNoticeEnglish } from '../../../../shared/agent-session-refusal-notice' import { structuredAgentSessionAttemptFailureParts } from '../../../../shared/structured-agent-session-send-disposition' import { createStructuredAgentSessionOutboxEntry, type StructuredAgentSessionOutboxEntry } from '../../../../shared/structured-agent-session-outbox' -import { writeOutbox } from './structured-agent-session-outbox-storage' +import { + hasUndeliveredStructuredAgentSessionOutbox, + readOutbox, + writeOutbox +} from './structured-agent-session-outbox-storage' + +/** What these hooks render with: the journal's submissions, and the rows loaded so far. */ +type OutboxProps = { submissions: AgentJournalSubmission[]; rows?: AgentJournalRenderItem[] } + +function outboxProps(submissions: AgentJournalSubmission[]): OutboxProps { + return { submissions } +} + +const NO_JOURNAL_ITEMS: readonly AgentJournalRenderItem[] = [] // Why: every hook here shares the session outbox store; one left mounted would drain the next test's. afterEach(cleanup) @@ -88,7 +106,7 @@ describe('a send the host rejected because the agent never started', () => { localStorage.clear() }) - it('names the cause on the message and keeps it for Retry', async () => { + it('names the cause on the message until the journal row takes it over', async () => { mocks.call.mockImplementationOnce( async ( _target: unknown, @@ -98,6 +116,7 @@ describe('a send the host rejected because the agent never started', () => { ) const { result } = renderHook(() => useStructuredAgentSessionOutbox({ + journalItems: NO_JOURNAL_ITEMS, sessionId: 'session-1', target: { kind: 'local' }, fence: 1, @@ -108,7 +127,7 @@ describe('a send the host rejected because the agent never started', () => { act(() => expect(result.current.send('hello')).toBe(true)) await waitFor(() => expect(shownFailure(result.current.outbox[0])).toBe(REASON)) - // Settled as not delivered: it waits for Retry and holds no later message up. + // Settled as not delivered: it holds no later message up. expect(result.current.error).toBeNull() expect(result.current.outbox[0]?.state).toBe('rejected') }) @@ -126,6 +145,7 @@ describe('a send the host rejected because the agent never started', () => { }) const { result } = renderHook(() => useStructuredAgentSessionOutbox({ + journalItems: NO_JOURNAL_ITEMS, sessionId: 'session-1', target: { kind: 'local' }, fence: 1, @@ -151,7 +171,7 @@ describe('a send the host rejected because the agent never started', () => { ]) }) - it('keeps a message the host accepted and then could not deliver, with its reason and Retry', async () => { + it('leaves a message the host accepted and then could not deliver to its journal row', async () => { const reason = "Codex couldn't restart: spawn codex ENOENT." mocks.call.mockImplementation( async ( @@ -162,14 +182,15 @@ describe('a send the host rejected because the agent never started', () => { ) const target = { kind: 'local' } as const const { result, rerender } = renderHook( - (props: { submissions: AgentJournalSubmission[] }) => + (props: OutboxProps) => useStructuredAgentSessionOutbox({ + journalItems: props.rows ?? NO_JOURNAL_ITEMS, sessionId: 'session-1', target, fence: 1, submissions: props.submissions }), - { initialProps: { submissions: NO_SUBMISSIONS } } + { initialProps: outboxProps(NO_SUBMISSIONS) } ) act(() => expect(result.current.send('hello')).toBe(true)) @@ -177,22 +198,17 @@ describe('a send the host rejected because the agent never started', () => { const id = result.current.outbox[0]!.clientMessageId rerender({ - submissions: [{ ...pendingResultFor(id).value.submission, dispatchState: 'rejected', reason }] + submissions: [ + { ...pendingResultFor(id).value.submission, dispatchState: 'rejected', reason } + ], + rows: rowsFor(id) }) - await waitFor(() => expect(result.current.outbox[0]?.state).toBe('rejected')) - expect(shownFailure(result.current.outbox[0])).toBe(reason) + // The host's row shows it as not sent, with no Retry: nothing resends it. + await waitFor(() => expect(result.current.outbox).toEqual([])) expect(result.current.error).toBeNull() - - // Retry is a new message with the same text: a fresh id, sent once. - act(() => result.current.retry(id)) - await waitFor(() => expect(mocks.call).toHaveBeenCalledTimes(2)) - const retried: { - envelope: { clientOperationId: string } - body: { blocks: { text?: string }[] } - } = mocks.call.mock.calls[1]![2] - expect(retried.envelope.clientOperationId).not.toBe(id) - expect(retried.body.blocks[0]?.text).toBe('hello') + await act(() => new Promise((resolve) => setTimeout(resolve, 50))) + expect(mocks.call).toHaveBeenCalledOnce() }) it('says nothing when a Stop withdrew the message', async () => { @@ -205,14 +221,15 @@ describe('a send the host rejected because the agent never started', () => { ) const target = { kind: 'local' } as const const { result, rerender } = renderHook( - (props: { submissions: AgentJournalSubmission[] }) => + (props: OutboxProps) => useStructuredAgentSessionOutbox({ + journalItems: props.rows ?? NO_JOURNAL_ITEMS, sessionId: 'session-1', target, fence: 1, submissions: props.submissions }), - { initialProps: { submissions: NO_SUBMISSIONS } } + { initialProps: outboxProps(NO_SUBMISSIONS) } ) act(() => expect(result.current.send('hello')).toBe(true)) @@ -233,7 +250,7 @@ describe('a send the host rejected because the agent never started', () => { expect(result.current.error).toBeNull() }) - it('reads a message rejected while the chat was closed as not sent, and sends past it', async () => { + it('leaves a message rejected while the chat was closed to its journal row, and sends past it', async () => { const reason = "Codex couldn't restart: spawn codex ENOENT." mocks.call.mockImplementation( async ( @@ -245,6 +262,7 @@ describe('a send the host rejected because the agent never started', () => { const target = { kind: 'local' } as const const first = renderHook(() => useStructuredAgentSessionOutbox({ + journalItems: NO_JOURNAL_ITEMS, sessionId: 'session-1', target, fence: 1, @@ -260,8 +278,10 @@ describe('a send the host rejected because the agent never started', () => { const rejected = [ { ...pendingResultFor(id).value.submission, dispatchState: 'rejected' as const, reason } ] + const reopenedRows = rowsFor(id) const { result } = renderHook(() => useStructuredAgentSessionOutbox({ + journalItems: reopenedRows, sessionId: 'session-1', target, fence: 1, @@ -269,54 +289,192 @@ describe('a send the host rejected because the agent never started', () => { }) ) - await waitFor(() => expect(result.current.outbox[0]?.state).toBe('rejected')) - expect(shownFailure(result.current.outbox[0])).toBe(reason) + await waitFor(() => expect(result.current.outbox).toEqual([])) act(() => expect(result.current.send('second')).toBe(true)) await waitFor(() => expect(mocks.call).toHaveBeenCalledTimes(2)) }) - // After a restart nothing in memory remembers the rejection, and its journal row may be older - // than the loaded page: the message's own state is what says a resend needs a new id. - it('retries a message rejected before a restart under a new id', async () => { - const rejected = createStructuredAgentSessionOutboxEntry({ - clientMessageId: 'rejected-before-restart', - sessionId: 'session-1', - text: 'first', - attachments: [], - queuedAt: 1 - }) + // One stored host fact, one rendering: a message the host recorded and rejected, whose record this + // chat does not hold, keeps showing why on every mount, with no control, and is never sent again. + it('keeps a message the host rejected, with no control, on every mount, and never resends it', async () => { writeOutbox('session-1', [ { - ...rejected, - state: 'rejected', - lastFailure: { - kind: 'rejected', - reason: 'The provider did not accept this message.', - rejection: { kind: 'providerRejected' } - } + ...rejectedBeforeRestart(), + state: 'dispatching', + lastFailure: undefined, + lastAttemptAt: 5 } ]) + // The host replays its rejection when the reopened chat asks about the send it left in doubt. mocks.call.mockImplementation(async (_target, _method, params) => - acceptedResultFor(String(params.envelope.clientOperationId)) + rejectedResultFor(String(params.envelope.clientOperationId)) ) - const { result } = renderHook(() => - useStructuredAgentSessionOutbox({ - sessionId: 'session-1', - target: { kind: 'local' }, - fence: 1, - submissions: [] - }) - ) - expect(result.current.outbox[0]?.state).toBe('rejected') + const notice = ( + outbox: readonly StructuredAgentSessionOutboxEntry[], + failedHere: ReadonlySet + ) => + structuredAgentSessionDeliveryNotices( + outbox, + 'Claude', + () => {}, + NO_SUBMISSIONS, + [], + failedHere + ).get(agentJournalSubmissionKey('rejected-before-restart')) + const mount = () => + renderHook(() => + useStructuredAgentSessionOutbox({ + journalItems: NO_JOURNAL_ITEMS, + sessionId: 'session-1', + target: { kind: 'local' }, + fence: 1, + submissions: NO_SUBMISSIONS + }) + ) - act(() => result.current.retry('rejected-before-restart')) - await waitFor(() => expect(mocks.call).toHaveBeenCalledOnce()) - const sentId: unknown = mocks.call.mock.calls[0]![2].envelope.clientOperationId - expect(sentId).not.toBe('rejected-before-restart') - await waitFor(() => expect(result.current.outbox).toHaveLength(0)) + const first = mount() + await waitFor(() => expect(first.result.current.outbox[0]?.state).toBe('rejected'), { + timeout: 4000 + }) + const onFirst = notice(first.result.current.outbox, first.result.current.failedHere) + first.unmount() + const second = mount() + const onSecond = notice(second.result.current.outbox, second.result.current.failedHere) + + for (const shown of [onFirst, onSecond]) { + expect(shown).toEqual({ text: REASON }) + } + await act(() => new Promise((resolve) => setTimeout(resolve, 1500))) + // Only the first mount's question about the send it left in doubt; nothing resends it. + expect(mocks.call).toHaveBeenCalledOnce() + expect(second.result.current.outbox.map((entry) => entry.state)).toEqual(['rejected']) + expect(readOutbox('session-1')).toHaveLength(1) + expect(hasUndeliveredStructuredAgentSessionOutbox('session-1')).toBe(false) + }, 10000) + + // The host's own record is what lets it go, and storage forgets it too: nothing is owed. + it('drops a message rejected before a restart once the journal says so, from storage too', async () => { + writeOutbox('session-1', [rejectedBeforeRestart()]) + const { result, rerender } = renderHook( + (props: OutboxProps) => + useStructuredAgentSessionOutbox({ + journalItems: props.rows ?? NO_JOURNAL_ITEMS, + sessionId: 'session-1', + target: { kind: 'local' }, + fence: 1, + submissions: props.submissions + }), + { initialProps: outboxProps(NO_SUBMISSIONS) } + ) + expect(result.current.outbox).toHaveLength(1) + + rerender({ + submissions: [ + { + ...pendingResultFor('rejected-before-restart').value.submission, + dispatchState: 'rejected', + reason: 'The provider did not accept this message.', + resolvedAt: 2 + } + ], + rows: rowsFor('rejected-before-restart') + }) + + await waitFor(() => expect(result.current.outbox).toEqual([])) + expect(readOutbox('session-1')).toEqual([]) + expect(hasUndeliveredStructuredAgentSessionOutbox('session-1')).toBe(false) + expect(mocks.call).not.toHaveBeenCalled() }) - it('keeps the rejection when the journal settles the message before the send answers', async () => { + // An older host leaves a rejected message where it was sent, which may be outside the loaded + // window: the entry draws it, with no control, until the row that draws it loads. + it('keeps a rejected message whose row is not loaded, then lets it go once the row loads', async () => { + writeOutbox('session-1', [ + { ...rejectedBeforeRestart(), state: 'dispatching', lastFailure: undefined, lastAttemptAt: 5 } + ]) + const rejected = [ + { + ...pendingResultFor('rejected-before-restart').value.submission, + dispatchState: 'rejected' as const, + reason: REASON, + resolvedAt: 2 + } + ] + const { result, rerender } = renderHook( + (props: OutboxProps) => + useStructuredAgentSessionOutbox({ + journalItems: props.rows ?? NO_JOURNAL_ITEMS, + sessionId: 'session-1', + target: { kind: 'local' }, + fence: 1, + submissions: props.submissions + }), + { initialProps: outboxProps(rejected) } + ) + + await waitFor(() => expect(result.current.outbox[0]?.state).toBe('rejected')) + expect(shownFailure(result.current.outbox[0])).toBe(REASON) + expect(readOutbox('session-1')).toHaveLength(1) + + rerender({ submissions: rejected, rows: rowsFor('rejected-before-restart') }) + await waitFor(() => expect(result.current.outbox).toEqual([])) + expect(readOutbox('session-1')).toEqual([]) + expect(mocks.call).not.toHaveBeenCalled() + }) + + it('keeps a send its reply rejected without a Retry until the journal row takes it over', async () => { + const writeFailed = (clientMessageId: string): AgentJournalSubmission => ({ + clientMessageId, + fence: 1, + payloadFingerprint: 'fingerprint', + dispatchState: 'rejected', + providerItemId: null, + reason: 'provider_write_failed: broken pipe', + submittedAt: 10, + resolvedAt: 10 + }) + mocks.call.mockImplementationOnce( + async ( + _target: unknown, + _method: unknown, + params: { envelope: { clientOperationId: string } } + ) => ({ + ok: true, + replayed: false, + fence: 1, + cursor: { epoch: 'epoch-1', sequence: 10 }, + value: { + clientMessageId: params.envelope.clientOperationId, + submission: writeFailed(params.envelope.clientOperationId) + } + }) + ) + const { result, rerender } = renderHook( + (props: OutboxProps) => + useStructuredAgentSessionOutbox({ + journalItems: props.rows ?? NO_JOURNAL_ITEMS, + sessionId: 'session-1', + target: LOCAL_TARGET, + fence: 1, + submissions: props.submissions + }), + { initialProps: outboxProps(NO_SUBMISSIONS) } + ) + + act(() => expect(result.current.send('first')).toBe(true)) + await waitFor(() => expect(mocks.call).toHaveBeenCalledOnce()) + const firstId = String(mocks.call.mock.calls[0]![2].envelope.clientOperationId) + + // Answered, not doubted: the entry draws the message, saying why, until the journal has it. + await waitFor(() => expect(result.current.outbox[0]?.lastFailure?.kind).toBe('rejected')) + expect(result.current.outbox[0]?.state).toBe('rejected') + + rerender({ submissions: [writeFailed(firstId)], rows: rowsFor(firstId) }) + await waitFor(() => expect(result.current.outbox).toHaveLength(0)) + expect(mocks.call).toHaveBeenCalledOnce() + }) + + it('lets the journal settle the message when its rejection lands before the send answers', async () => { const reason = "Codex couldn't restart: spawn codex ENOENT." let answer: (value: unknown) => void = () => undefined mocks.call.mockImplementationOnce( @@ -327,14 +485,15 @@ describe('a send the host rejected because the agent never started', () => { ) const target = { kind: 'local' } as const const { result, rerender } = renderHook( - (props: { submissions: AgentJournalSubmission[] }) => + (props: OutboxProps) => useStructuredAgentSessionOutbox({ + journalItems: props.rows ?? NO_JOURNAL_ITEMS, sessionId: 'session-1', target, fence: 1, submissions: props.submissions }), - { initialProps: { submissions: NO_SUBMISSIONS } } + { initialProps: outboxProps(NO_SUBMISSIONS) } ) act(() => expect(result.current.send('hello')).toBe(true)) @@ -343,19 +502,51 @@ describe('a send the host rejected because the agent never started', () => { // A start refused at once: the rejection frame lands before the send's own `pending` answer. rerender({ - submissions: [{ ...pendingResultFor(id).value.submission, dispatchState: 'rejected', reason }] + submissions: [ + { ...pendingResultFor(id).value.submission, dispatchState: 'rejected', reason } + ], + rows: rowsFor(id) }) - await waitFor(() => expect(result.current.outbox[0]?.state).toBe('rejected')) + await waitFor(() => expect(result.current.outbox).toEqual([])) + // The late `pending` answer must not bring it back. await act(async () => answer(pendingResultFor(id))) - expect(result.current.outbox[0]?.state).toBe('rejected') - expect(shownFailure(result.current.outbox[0])).toBe(reason) + expect(result.current.outbox).toEqual([]) expect(result.current.error).toBeNull() }) }) const NO_SUBMISSIONS: AgentJournalSubmission[] = [] +/** The host's rows for these messages, loaded. */ +function rowsFor(...ids: string[]): AgentJournalRenderItem[] { + return ids.map((id, index) => ({ + itemId: agentJournalSubmissionKey(id), + revision: 1, + sequence: index + 1, + observedAt: index + 1, + body: { kind: 'message', role: 'user', blocks: [{ type: 'text', text: id }] } + })) +} + +function rejectedBeforeRestart(): StructuredAgentSessionOutboxEntry { + return { + ...createStructuredAgentSessionOutboxEntry({ + clientMessageId: 'rejected-before-restart', + sessionId: 'session-1', + text: 'first', + attachments: [], + queuedAt: 1 + }), + state: 'rejected', + lastFailure: { + kind: 'rejected', + reason: 'The provider did not accept this message.', + rejection: { kind: 'providerRejected' } + } + } +} + function pendingResultFor(clientMessageId: string) { const submission: AgentJournalSubmission = { clientMessageId, @@ -426,6 +617,7 @@ describe('a send refused while its agent restarted', () => { const { result, rerender } = renderHook( ({ fence, submissions }: { fence: number; submissions: AgentJournalSubmission[] }) => useStructuredAgentSessionOutbox({ + journalItems: NO_JOURNAL_ITEMS, sessionId: 'session-1', target: LOCAL_TARGET, fence, @@ -485,6 +677,7 @@ describe('a send the host refused by throwing', () => { ) const { result } = renderHook(() => useStructuredAgentSessionOutbox({ + journalItems: NO_JOURNAL_ITEMS, sessionId: 'session-1', target: { kind: 'local' }, fence: 1, @@ -525,6 +718,7 @@ describe('a send refused on a journal a newer Orca wrote', () => { }) const { result } = renderHook(() => useStructuredAgentSessionOutbox({ + journalItems: NO_JOURNAL_ITEMS, sessionId: 'session-1', target: { kind: 'local' }, fence: 1, diff --git a/src/renderer/src/components/native-chat/use-structured-agent-session-outbox-relaunch-hold.test.tsx b/src/renderer/src/components/native-chat/use-structured-agent-session-outbox-relaunch-hold.test.tsx index 5ef47af4f69..c40123377ea 100644 --- a/src/renderer/src/components/native-chat/use-structured-agent-session-outbox-relaunch-hold.test.tsx +++ b/src/renderer/src/components/native-chat/use-structured-agent-session-outbox-relaunch-hold.test.tsx @@ -28,6 +28,10 @@ import { createStructuredAgentSessionOutboxEntry } from '../../../../shared/stru import { writeOutbox } from './structured-agent-session-outbox-storage' import { structuredAgentSessionDeliveryNotices } from './structured-agent-session-delivery-notices' +import type { AgentJournalRenderItem } from '../../../../shared/agent-session-journal-types' + +const NO_JOURNAL_ITEMS: readonly AgentJournalRenderItem[] = [] + const SESSION = 'session-1' // Stable, as the view passes it: a new object each render would re-run the owner-change requeue. const LOCAL_TARGET = { kind: 'local' } as const @@ -96,6 +100,7 @@ function mount(fence = 1) { return renderHook( ({ fence: current }: { fence: number }) => useStructuredAgentSessionOutbox({ + journalItems: NO_JOURNAL_ITEMS, sessionId: SESSION, target: LOCAL_TARGET, fence: current, @@ -423,6 +428,7 @@ describe('a message whose send could not be saved before it went out', () => { const { result, rerender } = renderHook( ({ fence }: { fence: number | null }) => useStructuredAgentSessionOutbox({ + journalItems: NO_JOURNAL_ITEMS, sessionId: SESSION, target: LOCAL_TARGET, fence, @@ -541,6 +547,7 @@ describe('a held message whose id expired', () => { mocks.call.mockResolvedValue(expired()) const { result } = renderHook(() => useStructuredAgentSessionOutbox({ + journalItems: NO_JOURNAL_ITEMS, sessionId: SESSION, target: LOCAL_TARGET, fence: 1, diff --git a/src/renderer/src/components/native-chat/use-structured-agent-session-outbox-withdrawal.test.tsx b/src/renderer/src/components/native-chat/use-structured-agent-session-outbox-withdrawal.test.tsx index 1c7d69f1aa7..4e815c9252f 100644 --- a/src/renderer/src/components/native-chat/use-structured-agent-session-outbox-withdrawal.test.tsx +++ b/src/renderer/src/components/native-chat/use-structured-agent-session-outbox-withdrawal.test.tsx @@ -5,7 +5,10 @@ import { act, cleanup, renderHook, waitFor } from '@testing-library/react' import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' -import type { AgentJournalSubmission } from '../../../../shared/agent-session-journal-types' +import type { + AgentJournalRenderItem, + AgentJournalSubmission +} from '../../../../shared/agent-session-journal-types' import type { StructuredAgentSessionOutboxEntry } from '../../../../shared/structured-agent-session-outbox' import { hasUnsentStructuredAgentSessionOutboxEntry, @@ -25,6 +28,8 @@ vi.mock('@/runtime/structured-agent-session-client', () => ({ import { useStructuredAgentSessionOutbox } from './use-structured-agent-session-outbox' import { readOutbox } from './structured-agent-session-outbox-storage' +const NO_JOURNAL_ITEMS: readonly AgentJournalRenderItem[] = [] + // One object: a target rebuilt each render reads as a new owner, which re-sends what is on its way. const TARGET = { kind: 'local' } as const @@ -82,6 +87,7 @@ describe('a Stop withdrawing what the host does not hold', () => { ) const { result } = renderHook(() => useStructuredAgentSessionOutbox({ + journalItems: NO_JOURNAL_ITEMS, sessionId: 'session-1', target: TARGET, fence: 1, @@ -147,6 +153,7 @@ describe('a Stop withdrawing what the host does not hold', () => { mocks.call.mockRejectedValue(new Error('the host refused the frame')) const { result } = renderHook(() => useStructuredAgentSessionOutbox({ + journalItems: NO_JOURNAL_ITEMS, sessionId: 'session-1', target: TARGET, fence: 1, diff --git a/src/renderer/src/components/native-chat/use-structured-agent-session-outbox.batch-churn.test.tsx b/src/renderer/src/components/native-chat/use-structured-agent-session-outbox.batch-churn.test.tsx new file mode 100644 index 00000000000..26e46bd2bbf --- /dev/null +++ b/src/renderer/src/components/native-chat/use-structured-agent-session-outbox.batch-churn.test.tsx @@ -0,0 +1,117 @@ +// @vitest-environment happy-dom + +// The outbox re-reads the journal on every batch; a batch that settles nothing writes nothing, so a +// message left in doubt costs no storage write per streamed delta. And a copy the host recorded and +// rejected owes no delivery, so it keeps no hidden pane reading the journal. + +import { act, cleanup, renderHook } from '@testing-library/react' +import { afterEach, beforeEach, expect, it, vi } from 'vitest' +import type { + AgentJournalRenderItem, + AgentJournalSubmission +} from '../../../../shared/agent-session-journal-types' +import { createStructuredAgentSessionOutboxEntry } from '../../../../shared/structured-agent-session-outbox' + +vi.mock('@/runtime/structured-agent-session-client', () => ({ + callStructuredAgentSession: vi.fn(() => new Promise(() => {})) +})) + +import { + hasUndeliveredStructuredAgentSessionOutbox, + writeOutbox +} from './structured-agent-session-outbox-storage' +import { useStructuredAgentSessionOutbox } from './use-structured-agent-session-outbox' + +afterEach(cleanup) + +beforeEach(() => { + localStorage.clear() +}) + +const SESSION = 'session-churn' + +function stored( + id: string, + patch: Partial> +) { + return { + ...createStructuredAgentSessionOutboxEntry({ + clientMessageId: id, + sessionId: SESSION, + text: id, + attachments: [], + queuedAt: 1 + }), + ...patch + } +} + +function inDoubt(clientMessageId: string): AgentJournalSubmission { + return { + clientMessageId, + fence: 1, + payloadFingerprint: clientMessageId, + dispatchState: 'unknown', + providerItemId: null, + reason: 'in doubt', + submittedAt: 5, + resolvedAt: 6 + } +} + +function streamed(sequence: number): AgentJournalRenderItem[] { + return [ + { + itemId: `answer-${sequence}`, + revision: sequence, + sequence, + observedAt: sequence, + body: { kind: 'message', role: 'assistant', blocks: [{ type: 'text', text: 'streaming' }] } + } + ] +} + +it('writes nothing to storage across 50 batches while a message waits in doubt', async () => { + writeOutbox(SESSION, [stored('doubt', { state: 'dispatching', lastAttemptAt: 2 })]) + const { rerender } = renderHook( + (props: { items: AgentJournalRenderItem[]; submissions: AgentJournalSubmission[] }) => + useStructuredAgentSessionOutbox({ + sessionId: SESSION, + target: { kind: 'local' }, + fence: 1, + submissions: props.submissions, + journalItems: props.items + }), + { initialProps: { items: streamed(1), submissions: [inDoubt('doubt')] } } + ) + await act(async () => {}) + const writes = vi.spyOn(localStorage, 'setItem') + + for (let sequence = 2; sequence <= 51; sequence += 1) { + // Each batch is a new journal array and a new submissions array with the same content. + rerender({ items: streamed(sequence), submissions: [inDoubt('doubt')] }) + } + await act(async () => {}) + + expect(writes).not.toHaveBeenCalled() + writes.mockRestore() +}) + +it('counts a copy the host recorded and rejected as owing no delivery', () => { + writeOutbox(SESSION, [ + stored('recorded', { + state: 'rejected', + lastFailure: { kind: 'rejected', reason: 'Orca restarted before this message was sent.' } + }) + ]) + expect(hasUndeliveredStructuredAgentSessionOutbox(SESSION)).toBe(false) + + writeOutbox(SESSION, [ + stored('recorded', { + state: 'rejected', + lastFailure: { kind: 'rejected', reason: 'Orca restarted before this message was sent.' } + }), + stored('waiting', {}) + ]) + expect(hasUndeliveredStructuredAgentSessionOutbox(SESSION)).toBe(true) +}) diff --git a/src/renderer/src/components/native-chat/use-structured-agent-session-outbox.draft-hand-off.test.tsx b/src/renderer/src/components/native-chat/use-structured-agent-session-outbox.draft-hand-off.test.tsx index ff13958f160..61fc0685816 100644 --- a/src/renderer/src/components/native-chat/use-structured-agent-session-outbox.draft-hand-off.test.tsx +++ b/src/renderer/src/components/native-chat/use-structured-agent-session-outbox.draft-hand-off.test.tsx @@ -7,7 +7,10 @@ import { act, cleanup, renderHook, waitFor } from '@testing-library/react' import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' -import type { AgentJournalSubmission } from '../../../../shared/agent-session-journal-types' +import type { + AgentJournalRenderItem, + AgentJournalSubmission +} from '../../../../shared/agent-session-journal-types' import { createStructuredAgentSessionOutboxEntry } from '../../../../shared/structured-agent-session-outbox' import { DISPATCH_REJECTED_CANCELLED } from '../../../../shared/structured-agent-session-dispatch-rejection' import { @@ -28,6 +31,8 @@ vi.mock('@/runtime/structured-agent-session-client', () => ({ import { useStructuredAgentSessionOutbox } from './use-structured-agent-session-outbox' import { writeOutbox } from './structured-agent-session-outbox-storage' +const NO_JOURNAL_ITEMS: readonly AgentJournalRenderItem[] = [] + // Why: every hook here shares the session outbox store; one left mounted would drain the next test's. afterEach(cleanup) @@ -77,6 +82,7 @@ function renderOutbox() { return renderHook( (props: Props) => useStructuredAgentSessionOutbox({ + journalItems: NO_JOURNAL_ITEMS, sessionId: 'session-1', target: TARGET, fence: 1, diff --git a/src/renderer/src/components/native-chat/use-structured-agent-session-outbox.external-send.test.tsx b/src/renderer/src/components/native-chat/use-structured-agent-session-outbox.external-send.test.tsx index 95d9021413a..0cb70aab14b 100644 --- a/src/renderer/src/components/native-chat/use-structured-agent-session-outbox.external-send.test.tsx +++ b/src/renderer/src/components/native-chat/use-structured-agent-session-outbox.external-send.test.tsx @@ -15,6 +15,10 @@ vi.mock('@/runtime/structured-agent-session-client', () => ({ import { appendStructuredAgentSessionOutboxMessage } from './structured-agent-session-outbox-storage' import { useStructuredAgentSessionOutbox } from './use-structured-agent-session-outbox' +import type { AgentJournalRenderItem } from '../../../../shared/agent-session-journal-types' + +const NO_JOURNAL_ITEMS: readonly AgentJournalRenderItem[] = [] + afterEach(cleanup) beforeEach(() => { @@ -39,6 +43,7 @@ it('delivers a message queued from outside the chat through the open outbox', as }) const { result } = renderHook(() => useStructuredAgentSessionOutbox({ + journalItems: NO_JOURNAL_ITEMS, sessionId: 'session-1', target: { kind: 'local' }, fence: 1, diff --git a/src/renderer/src/components/native-chat/use-structured-agent-session-outbox.queue-delivery.test.tsx b/src/renderer/src/components/native-chat/use-structured-agent-session-outbox.queue-delivery.test.tsx index 26e0fd71112..fadb4440534 100644 --- a/src/renderer/src/components/native-chat/use-structured-agent-session-outbox.queue-delivery.test.tsx +++ b/src/renderer/src/components/native-chat/use-structured-agent-session-outbox.queue-delivery.test.tsx @@ -31,6 +31,10 @@ vi.mock('@/runtime/structured-agent-session-client', () => ({ import { useStructuredAgentSessionOutbox } from './use-structured-agent-session-outbox' import { readOutbox } from './structured-agent-session-outbox-storage' +import type { AgentJournalRenderItem } from '../../../../shared/agent-session-journal-types' + +const NO_JOURNAL_ITEMS: readonly AgentJournalRenderItem[] = [] + // Why: every hook here shares the session outbox store; one left mounted would drain the next test's. afterEach(cleanup) @@ -53,6 +57,7 @@ function queuedReceipt(clientMessageId: string) { function renderOutbox(queue: boolean) { return renderHook(() => useStructuredAgentSessionOutbox({ + journalItems: NO_JOURNAL_ITEMS, sessionId: 'session-1', target: LOCAL_TARGET, fence: 1, @@ -154,6 +159,7 @@ describe('outbox queue delivery selection', () => { const first = renderHook( (props: { queuedMessageIds: string[] }) => useStructuredAgentSessionOutbox({ + journalItems: NO_JOURNAL_ITEMS, sessionId: 'session-1', target: LOCAL_TARGET, fence: 1, @@ -184,6 +190,7 @@ describe('outbox queue delivery selection', () => { const view = renderHook( (props: { queuedMessageIds: string[] }) => useStructuredAgentSessionOutbox({ + journalItems: NO_JOURNAL_ITEMS, sessionId: 'session-1', target: LOCAL_TARGET, fence: 1, @@ -252,6 +259,7 @@ describe('outbox queue delivery selection', () => { })) const { result } = renderHook(() => useStructuredAgentSessionOutbox({ + journalItems: NO_JOURNAL_ITEMS, sessionId: 'session-1', target: LOCAL_TARGET, fence: 1, @@ -306,6 +314,7 @@ async function attemptedQueueSend() { const view = renderHook( (props: { capability: StructuredAgentSessionQueueCapability }) => useStructuredAgentSessionOutbox({ + journalItems: NO_JOURNAL_ITEMS, sessionId: 'session-1', target: LOCAL_TARGET, fence: 1, diff --git a/src/renderer/src/components/native-chat/use-structured-agent-session-outbox.rejected-reply.test.tsx b/src/renderer/src/components/native-chat/use-structured-agent-session-outbox.rejected-reply.test.tsx new file mode 100644 index 00000000000..bfebc877b2c --- /dev/null +++ b/src/renderer/src/components/native-chat/use-structured-agent-session-outbox.rejected-reply.test.tsx @@ -0,0 +1,102 @@ +// @vitest-environment happy-dom + +// A send whose own reply says the host recorded it and then rejected it is shown once, in the chat +// where it was sent; its text never also goes back to the composer. + +import { act, cleanup, renderHook, waitFor } from '@testing-library/react' +import { afterEach, beforeEach, expect, it, vi } from 'vitest' +import type { + AgentJournalRenderItem, + AgentJournalSubmission +} from '../../../../shared/agent-session-journal-types' +import { agentJournalSubmissionKey } from '../../../../shared/agent-session-journal-item-key' +import { DISPATCH_REJECTED_NOT_DELIVERED } from '../../../../shared/structured-agent-session-dispatch-rejection' + +type SendParams = { envelope: { clientOperationId: string; payloadFingerprint: string } } + +const mocks = vi.hoisted(() => ({ + call: vi.fn<(target: unknown, method: string, params: SendParams) => Promise>() +})) + +vi.mock('@/runtime/structured-agent-session-client', () => ({ + callStructuredAgentSession: mocks.call +})) + +import { + clearNativeChatDraftCacheForTests, + readNativeChatDraftCache +} from './native-chat-draft-cache' +import { projectStructuredAgentSessionMessages } from './structured-agent-session-message-projection' +import { useStructuredAgentSessionOutbox } from './use-structured-agent-session-outbox' + +const NO_JOURNAL_ITEMS: readonly AgentJournalRenderItem[] = [] + +const NO_CARDS: readonly string[] = [] + +afterEach(cleanup) + +beforeEach(() => { + vi.clearAllMocks() + localStorage.clear() + clearNativeChatDraftCacheForTests() +}) + +it('draws a send its reply rejected in place once, and leaves the composer empty', async () => { + const reply: { submission: AgentJournalSubmission | null } = { submission: null } + mocks.call.mockImplementationOnce(async (_target, _method, { envelope }) => { + const rejected: AgentJournalSubmission = { + clientMessageId: envelope.clientOperationId, + fence: 1, + payloadFingerprint: envelope.payloadFingerprint, + dispatchState: 'rejected', + providerItemId: null, + reason: DISPATCH_REJECTED_NOT_DELIVERED, + rejection: { kind: 'notDelivered' }, + submittedAt: 10, + resolvedAt: 11 + } + reply.submission = rejected + return { + ok: true, + replayed: false, + fence: 1, + cursor: { epoch: 'epoch-1', sequence: 10 }, + value: { clientMessageId: envelope.clientOperationId, submission: rejected } + } + }) + const { result } = renderHook(() => + useStructuredAgentSessionOutbox({ + journalItems: NO_JOURNAL_ITEMS, + sessionId: 'session-1', + target: { kind: 'local' }, + fence: 1, + submissions: [], + composerScopeKey: 'pane-1' + }) + ) + + act(() => expect(result.current.send('steer this way')).toBe(true)) + await waitFor(() => expect(result.current.outbox[0]?.state).toBe('rejected')) + expect(readNativeChatDraftCache('pane-1')).toBe('') + + const submission = reply.submission + if (!submission) { + throw new Error('the send was never answered') + } + const hostItem: AgentJournalRenderItem = { + itemId: agentJournalSubmissionKey(submission.clientMessageId), + revision: 1, + sequence: 10, + observedAt: 10, + body: { kind: 'message', role: 'user', blocks: [{ type: 'text', text: 'steer this way' }] } + } + const users = (outbox: typeof result.current.outbox) => + projectStructuredAgentSessionMessages([hostItem], outbox, [submission], NO_CARDS) + .filter((message) => message.role === 'user') + .map((message) => ({ id: message.id, unsent: message.unsent })) + + expect(users(result.current.outbox)).toEqual([{ id: hostItem.itemId, unsent: true }]) + // After a crash takes the outbox, the host's row is still the one row. + expect(users([])).toEqual([{ id: hostItem.itemId, unsent: true }]) + expect(readNativeChatDraftCache('pane-1')).toBe('') +}) diff --git a/src/renderer/src/components/native-chat/use-structured-agent-session-outbox.stop-parks-in-doubt.test.tsx b/src/renderer/src/components/native-chat/use-structured-agent-session-outbox.stop-parks-in-doubt.test.tsx index 293d9719bc0..45d87f26f66 100644 --- a/src/renderer/src/components/native-chat/use-structured-agent-session-outbox.stop-parks-in-doubt.test.tsx +++ b/src/renderer/src/components/native-chat/use-structured-agent-session-outbox.stop-parks-in-doubt.test.tsx @@ -25,6 +25,10 @@ import { readNativeChatDraftCache } from './native-chat-draft-cache' +import type { AgentJournalRenderItem } from '../../../../shared/agent-session-journal-types' + +const NO_JOURNAL_ITEMS: readonly AgentJournalRenderItem[] = [] + // Why: every hook here shares the session outbox store; one left mounted would drain the next test's. afterEach(cleanup) @@ -66,6 +70,7 @@ describe('a Stop with a queued send in doubt', () => { const view = renderHook( (props: { fence: number | null }) => useStructuredAgentSessionOutbox({ + journalItems: NO_JOURNAL_ITEMS, sessionId: 'session-1', target: REMOTE, fence: props.fence, @@ -110,6 +115,7 @@ describe('a Stop with a queued send in doubt', () => { mocks.call.mockImplementationOnce(() => answer.promise) const { result } = renderHook(() => useStructuredAgentSessionOutbox({ + journalItems: NO_JOURNAL_ITEMS, sessionId: 'session-1', target: TARGET, fence: 1, @@ -170,6 +176,7 @@ describe('a Stop with a queued send in doubt', () => { ]) const { result } = renderHook(() => useStructuredAgentSessionOutbox({ + journalItems: NO_JOURNAL_ITEMS, sessionId: 'session-1', target: TARGET, fence: 1, diff --git a/src/renderer/src/components/native-chat/use-structured-agent-session-outbox.test.tsx b/src/renderer/src/components/native-chat/use-structured-agent-session-outbox.test.tsx index 422d94b618a..f9957ed41cc 100644 --- a/src/renderer/src/components/native-chat/use-structured-agent-session-outbox.test.tsx +++ b/src/renderer/src/components/native-chat/use-structured-agent-session-outbox.test.tsx @@ -4,7 +4,10 @@ import { act, cleanup, renderHook, waitFor } from '@testing-library/react' import { useLayoutEffect } from 'react' import { createRoot } from 'react-dom/client' import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' -import type { AgentJournalSubmission } from '../../../../shared/agent-session-journal-types' +import type { + AgentJournalRenderItem, + AgentJournalSubmission +} from '../../../../shared/agent-session-journal-types' import type { AgentSessionWireRefusalCode } from '../../../../shared/agent-session-wire' import { enqueueStructuredAgentSessionLaunchPrompt } from './structured-agent-session-outbox-storage' @@ -19,6 +22,16 @@ vi.mock('@/runtime/structured-agent-session-client', () => ({ import { useStructuredAgentSessionOutbox } from './use-structured-agent-session-outbox' import { settleStructuredAgentLaunchPrompt } from '@/lib/structured-agent-session-launch-prompt' +const hostRow = (id: string): AgentJournalRenderItem => ({ + itemId: `orca:${id}`, + revision: 1, + sequence: 1, + observedAt: 1, + body: { kind: 'message', role: 'user', blocks: [{ type: 'text', text: id }] } +}) + +const NO_JOURNAL_ITEMS: readonly AgentJournalRenderItem[] = [] + // Why: every hook here shares the session outbox store; one left mounted would drain the next test's. afterEach(cleanup) @@ -149,6 +162,7 @@ describe('useStructuredAgentSessionOutbox', () => { const { result, rerender } = renderHook( ({ fence }) => useStructuredAgentSessionOutbox({ + journalItems: NO_JOURNAL_ITEMS, sessionId: 'session-1', target: LOCAL_TARGET, fence, @@ -190,6 +204,7 @@ describe('useStructuredAgentSessionOutbox', () => { const { result } = renderHook(() => useStructuredAgentSessionOutbox({ + journalItems: NO_JOURNAL_ITEMS, sessionId: 'session-1', target: LOCAL_TARGET, fence: 1, @@ -211,6 +226,7 @@ describe('useStructuredAgentSessionOutbox', () => { const { result, rerender } = renderHook( ({ fence }) => useStructuredAgentSessionOutbox({ + journalItems: NO_JOURNAL_ITEMS, sessionId: 'session-1', target: LOCAL_TARGET, fence, @@ -245,6 +261,7 @@ describe('useStructuredAgentSessionOutbox', () => { mocks.call.mockResolvedValueOnce(refusedResult(code)).mockResolvedValueOnce(acceptedResult(1)) const { result } = renderHook(() => useStructuredAgentSessionOutbox({ + journalItems: NO_JOURNAL_ITEMS, sessionId: 'session-1', target: LOCAL_TARGET, fence: 1, @@ -277,6 +294,7 @@ describe('useStructuredAgentSessionOutbox', () => { const { result, rerender } = renderHook( ({ submissions }: { submissions: readonly AgentJournalSubmission[] }) => useStructuredAgentSessionOutbox({ + journalItems: NO_JOURNAL_ITEMS, sessionId: 'session-1', target: LOCAL_TARGET, fence: 1, @@ -316,6 +334,7 @@ describe('useStructuredAgentSessionOutbox', () => { const { result, rerender } = renderHook( ({ submissions }: { submissions: readonly AgentJournalSubmission[] }) => useStructuredAgentSessionOutbox({ + journalItems: NO_JOURNAL_ITEMS, sessionId: 'session-1', target: LOCAL_TARGET, fence: 1, @@ -346,6 +365,7 @@ describe('useStructuredAgentSessionOutbox', () => { const { result, rerender } = renderHook( ({ submissions }: { submissions: readonly AgentJournalSubmission[] }) => useStructuredAgentSessionOutbox({ + journalItems: NO_JOURNAL_ITEMS, sessionId: 'session-1', target: LOCAL_TARGET, fence: 1, @@ -375,6 +395,7 @@ describe('useStructuredAgentSessionOutbox', () => { const { result, rerender } = renderHook( ({ submissions }: { submissions: readonly AgentJournalSubmission[] }) => useStructuredAgentSessionOutbox({ + journalItems: NO_JOURNAL_ITEMS, sessionId: 'session-1', target: LOCAL_TARGET, fence: 1, @@ -400,6 +421,7 @@ describe('useStructuredAgentSessionOutbox', () => { const { result, rerender } = renderHook( ({ submissions }: { submissions: readonly AgentJournalSubmission[] }) => useStructuredAgentSessionOutbox({ + journalItems: NO_JOURNAL_ITEMS, sessionId: 'session-1', target: LOCAL_TARGET, fence: 1, @@ -431,6 +453,7 @@ describe('useStructuredAgentSessionOutbox', () => { }) const first = renderHook(() => useStructuredAgentSessionOutbox({ + journalItems: NO_JOURNAL_ITEMS, sessionId: 'session-1', target: LOCAL_TARGET, fence: 1, @@ -445,6 +468,7 @@ describe('useStructuredAgentSessionOutbox', () => { const restored = renderHook(() => useStructuredAgentSessionOutbox({ + journalItems: NO_JOURNAL_ITEMS, sessionId: 'session-1', target: LOCAL_TARGET, fence: 1, @@ -476,6 +500,7 @@ describe('useStructuredAgentSessionOutbox', () => { const { result, rerender } = renderHook( ({ submissions }: { submissions: readonly AgentJournalSubmission[] }) => useStructuredAgentSessionOutbox({ + journalItems: NO_JOURNAL_ITEMS, sessionId: 'session-1', target: LOCAL_TARGET, fence: 1, @@ -515,6 +540,7 @@ describe('useStructuredAgentSessionOutbox', () => { .mockResolvedValueOnce(acceptedResult(1)) const { result } = renderHook(() => useStructuredAgentSessionOutbox({ + journalItems: NO_JOURNAL_ITEMS, sessionId: 'session-1', target: LOCAL_TARGET, fence: 1, @@ -544,6 +570,7 @@ describe('useStructuredAgentSessionOutbox', () => { }) const { result } = renderHook(() => useStructuredAgentSessionOutbox({ + journalItems: NO_JOURNAL_ITEMS, sessionId: 'session-1', target: LOCAL_TARGET, fence: 1, @@ -581,6 +608,7 @@ describe('useStructuredAgentSessionOutbox', () => { mocks.call.mockResolvedValue(acceptedResult(1)) const { result } = renderHook(() => useStructuredAgentSessionOutbox({ + journalItems: NO_JOURNAL_ITEMS, sessionId: 'session-1', target: LOCAL_TARGET, fence: 1, @@ -628,6 +656,7 @@ describe('useStructuredAgentSessionOutbox', () => { const { result, rerender } = renderHook( ({ submissions }: { submissions: readonly AgentJournalSubmission[] }) => useStructuredAgentSessionOutbox({ + journalItems: NO_JOURNAL_ITEMS, sessionId: 'session-1', target: LOCAL_TARGET, fence: 1, @@ -667,12 +696,7 @@ describe('useStructuredAgentSessionOutbox', () => { expect(retryParams?.retryUnknown).toBeUndefined() }) - it('rotates a history-rejected unknown head so the queued tail can advance', async () => { - // oxlint-disable-next-line no-restricted-properties -- stubbing the global the generator reads, to pin ids in this test - vi.mocked(globalThis.crypto.randomUUID) - .mockReturnValueOnce('aaaaaaaa-aaaa-4aaa-8aaa-aaaaaaaaaaaa') - .mockReturnValueOnce('bbbbbbbb-bbbb-4bbb-8bbb-bbbbbbbbbbbb') - .mockReturnValueOnce('cccccccc-cccc-4ccc-8ccc-cccccccccccc') + it('lets a history-rejected unknown head go to the journal so the queued tail advances', async () => { mocks.call .mockImplementationOnce(async (_target, _method, params) => { const clientMessageId = (params as { envelope: { clientOperationId: string } }).envelope @@ -684,14 +708,11 @@ describe('useStructuredAgentSessionOutbox', () => { .clientOperationId return acceptedResultFor(clientMessageId, 11) }) - .mockImplementationOnce(async (_target, _method, params) => { - const clientMessageId = (params as { envelope: { clientOperationId: string } }).envelope - .clientOperationId - return acceptedResultFor(clientMessageId, 12) - }) + let rows = NO_JOURNAL_ITEMS const { result, rerender } = renderHook( ({ submissions }: { submissions: readonly AgentJournalSubmission[] }) => useStructuredAgentSessionOutbox({ + journalItems: rows, sessionId: 'session-1', target: LOCAL_TARGET, fence: 1, @@ -704,6 +725,7 @@ describe('useStructuredAgentSessionOutbox', () => { await waitFor(() => expect(result.current.outbox[0]?.state).toBe('unconfirmed')) const firstId = result.current.outbox[0]!.clientMessageId act(() => expect(result.current.send('second')).toBe(true)) + rows = [hostRow(firstId)] rerender({ submissions: [ { @@ -719,83 +741,13 @@ describe('useStructuredAgentSessionOutbox', () => { ] }) - act(() => result.current.retry(firstId)) - await waitFor(() => expect(mocks.call).toHaveBeenCalledTimes(3)) + // The host's row shows the first as not sent; nothing resends it. + await waitFor(() => expect(mocks.call).toHaveBeenCalledTimes(2)) await waitFor(() => expect(result.current.outbox).toHaveLength(0)) - const retryParams = mocks.call.mock.calls[1]?.[2] as - | { envelope: { clientOperationId: string } } - | undefined - expect(retryParams?.envelope.clientOperationId).not.toBe(firstId) - }) - - it('rotates the id after a refused write and delivers the message exactly once', async () => { - // oxlint-disable-next-line no-restricted-properties -- stubbing the global the generator reads, to pin ids in this test - vi.mocked(globalThis.crypto.randomUUID) - .mockReturnValueOnce('aaaaaaaa-aaaa-4aaa-8aaa-aaaaaaaaaaaa') - .mockReturnValueOnce('bbbbbbbb-bbbb-4bbb-8bbb-bbbbbbbbbbbb') - const writeFailed = (clientMessageId: string) => ({ - clientMessageId, - fence: 1, - payloadFingerprint: 'fingerprint', - dispatchState: 'rejected' as const, - providerItemId: null, - reason: 'provider_write_failed: broken pipe', - submittedAt: 10, - resolvedAt: 10 + expect(mocks.call.mock.calls[1]?.[2]).toMatchObject({ body: { blocks: [{ text: 'second' }] } }) + expect(mocks.call.mock.calls[1]?.[2]).not.toMatchObject({ + envelope: { clientOperationId: firstId } }) - mocks.call - .mockImplementationOnce(async (_target, _method, params) => ({ - ok: true, - replayed: false, - fence: 1, - cursor: { epoch: 'epoch-1', sequence: 10 }, - value: { - clientMessageId: (params as { envelope: { clientOperationId: string } }).envelope - .clientOperationId, - submission: writeFailed( - (params as { envelope: { clientOperationId: string } }).envelope.clientOperationId - ) - } - })) - .mockImplementationOnce(async (_target, _method, params) => - acceptedResultFor( - (params as { envelope: { clientOperationId: string } }).envelope.clientOperationId, - 11 - ) - ) - const { result } = renderHook( - ({ submissions }: { submissions: readonly AgentJournalSubmission[] }) => - useStructuredAgentSessionOutbox({ - sessionId: 'session-1', - target: LOCAL_TARGET, - fence: 1, - submissions - }), - { initialProps: { submissions: [] as readonly AgentJournalSubmission[] } } - ) - - act(() => expect(result.current.send('first')).toBe(true)) - await waitFor(() => expect(mocks.call).toHaveBeenCalledOnce()) - const firstId = mocks.call.mock.calls[0]![2].envelope.clientOperationId as string - - // A refused write is answered, not doubted: the entry parks with its rejection rather than - // under the "delivery is unconfirmed" banner. The disposition tests pin its words. - await waitFor(() => expect(result.current.outbox[0]?.lastFailure?.kind).toBe('rejected')) - expect(result.current.outbox[0]?.state).toBe('rejected') - - // Retry immediately, before the journal subscription can publish the rejected row. - act(() => result.current.retry(firstId)) - await waitFor(() => expect(result.current.outbox).toHaveLength(0)) - - // Exactly one further delivery, under a new id, and with no `retryUnknown`: - // this is a first delivery of a new message, so it cannot duplicate. - expect(mocks.call).toHaveBeenCalledTimes(2) - const retryParams = mocks.call.mock.calls[1]?.[2] as { - envelope: { clientOperationId: string } - retryUnknown?: true - } - expect(retryParams.envelope.clientOperationId).not.toBe(firstId) - expect(retryParams.retryUnknown).toBeUndefined() }) it('loads the new session outbox when a pane switches sessions', async () => { @@ -808,6 +760,7 @@ describe('useStructuredAgentSessionOutbox', () => { const { result, rerender } = renderHook( ({ sessionId }: { sessionId: string }) => useStructuredAgentSessionOutbox({ + journalItems: NO_JOURNAL_ITEMS, sessionId, target: LOCAL_TARGET, fence: 1, @@ -840,6 +793,7 @@ describe('useStructuredAgentSessionOutbox', () => { const { result, rerender } = renderHook( ({ sessionId }: { sessionId: string }) => useStructuredAgentSessionOutbox({ + journalItems: NO_JOURNAL_ITEMS, sessionId, target: LOCAL_TARGET, fence: 1, @@ -869,6 +823,7 @@ describe('useStructuredAgentSessionOutbox', () => { } = { current: null } function Probe({ sessionId }: { sessionId: string }): null { controllerRef.current = useStructuredAgentSessionOutbox({ + journalItems: NO_JOURNAL_ITEMS, sessionId, target: LOCAL_TARGET, fence: 1, diff --git a/src/renderer/src/components/native-chat/use-structured-agent-session-outbox.ts b/src/renderer/src/components/native-chat/use-structured-agent-session-outbox.ts index 28bf17b8aa7..46f2f482e52 100644 --- a/src/renderer/src/components/native-chat/use-structured-agent-session-outbox.ts +++ b/src/renderer/src/components/native-chat/use-structured-agent-session-outbox.ts @@ -6,7 +6,10 @@ import { useState, useSyncExternalStore } from 'react' -import type { AgentJournalSubmission } from '../../../../shared/agent-session-journal-types' +import type { + AgentJournalRenderItem, + AgentJournalSubmission +} from '../../../../shared/agent-session-journal-types' import { createStructuredAgentSessionOperationId } from '../../../../shared/structured-agent-session-mutation' import { admitStructuredAgentSessionOutboxEntry, @@ -63,6 +66,8 @@ export function useStructuredAgentSessionOutbox(args: { target: RuntimeClientTarget fence: number | null submissions: readonly AgentJournalSubmission[] + /** The loaded journal rows: a rejected message stays here until the row that draws it loads. */ + journalItems: readonly AgentJournalRenderItem[] /** The composer that gets back what a Stop withdrew from this client's outbox. */ composerScopeKey?: string /** The host's queued-messages capability and the user's setting; a send stamped @@ -76,6 +81,7 @@ export function useStructuredAgentSessionOutbox(args: { const { composerScopeKey, fence, + journalItems, queueDelivery = NO_QUEUE_DELIVERY, queuedMessageIds, sessionId, @@ -147,15 +153,13 @@ export function useStructuredAgentSessionOutbox(args: { .map((submission) => submission.clientMessageId), ...handedOffQueuedMessageIds(submissions) ]) - const next = reconcileStructuredAgentSessionOutboxWithQueue(current, submissions) + const next = reconcileStructuredAgentSessionOutboxWithQueue(current, submissions, journalItems) const admittedInFlight = journalAnswersInFlightSend(submissions, inFlightIdRef.current) - if ( - admittedInFlight || - next.some((entry, index) => entry !== current[index]) || - next.length !== current.length - ) { + // The reconcile returns `current` itself when no entry changed, so a batch that changes + // nothing writes nothing. + if (admittedInFlight || next !== current) { restoreWithdrawn.byHost(current, submissions) - commitStructuredAgentSessionOutbox(sessionId, next) + commitStructuredAgentSessionOutbox(sessionId, [...next]) } // Keyed on the entry actually in flight, which is no longer always the head: the journal // owning it outranks a send promise that has not settled, so release single-flight and make @@ -174,7 +178,7 @@ export function useStructuredAgentSessionOutbox(args: { ) { setError(null) } - }, [restoreWithdrawn, sessionId, submissions]) + }, [journalItems, restoreWithdrawn, sessionId, submissions]) // The one place that owns the refs, the React state and the storage write. const applyDisposition = useCallback( diff --git a/src/renderer/src/components/native-chat/use-structured-agent-session.rejected-card.test.tsx b/src/renderer/src/components/native-chat/use-structured-agent-session.rejected-card.test.tsx new file mode 100644 index 00000000000..b37350cb39d --- /dev/null +++ b/src/renderer/src/components/native-chat/use-structured-agent-session.rejected-card.test.tsx @@ -0,0 +1,116 @@ +// @vitest-environment happy-dom + +// The session controller hands the queue's live cards to the transcript: a rejected message one of +// them holds under its own id is drawn as that card, never also as a not-sent row. + +import { renderHook } from '@testing-library/react' +import { beforeEach, expect, it, vi } from 'vitest' +import { agentJournalSubmissionKey } from '../../../../shared/agent-session-journal-item-key' +import type { + AgentJournalMessageItem, + AgentJournalRenderItem, + AgentJournalSubmission +} from '../../../../shared/agent-session-journal-types' +import type { AgentSessionQueuedMessage } from '../../../../shared/agent-session-wire' +import { DISPATCH_REJECTED_HOST_RESTARTED } from '../../../../shared/structured-agent-session-dispatch-rejection' + +let queuedMessages: AgentSessionQueuedMessage[] = [] + +const KEPT_BODY: AgentJournalMessageItem = { + kind: 'message', + role: 'user', + blocks: [{ type: 'text', text: 'kept text' }] +} + +const KEPT_ITEM: AgentJournalRenderItem = { + itemId: agentJournalSubmissionKey('kept'), + revision: 1, + sequence: 1, + observedAt: 1, + body: KEPT_BODY +} + +const KEPT_SUBMISSION: AgentJournalSubmission = { + clientMessageId: 'kept', + fence: 3, + payloadFingerprint: 'fingerprint', + dispatchState: 'rejected', + providerItemId: null, + reason: DISPATCH_REJECTED_HOST_RESTARTED, + rejection: { kind: 'hostRestarted' }, + submittedAt: 1, + resolvedAt: 2 +} + +vi.mock('@/runtime/structured-agent-session-client', () => ({ + callStructuredAgentSession: vi.fn(async () => null), + supportsStructuredAgentSessionPromptCancel: vi.fn(async () => false) +})) + +vi.mock('./use-structured-agent-session-read', () => ({ + useStructuredAgentSessionRead: () => ({ + state: { + fence: 3, + items: [KEPT_ITEM], + submissions: [KEPT_SUBMISSION], + status: 'ready', + error: null, + hasOlder: false, + queuedMessages + }, + loadingOlder: false, + loadOlder: vi.fn() + }) +})) + +vi.mock('./use-structured-agent-session-outbox', () => ({ + structuredSessionOperationId: () => 'operation-1', + useStructuredAgentSessionOutbox: () => ({ + outbox: [], + error: null, + send: vi.fn(), + retry: vi.fn(), + withdrawUnsent: vi.fn() + }) +})) + +import { useStructuredAgentSession } from './use-structured-agent-session' + +function card(messageId: string): AgentSessionQueuedMessage { + return { + messageId, + position: 1, + body: KEPT_BODY, + state: 'waiting' + } +} + +function keptRows(): { id: string; unsent?: true }[] { + const { result, unmount } = renderHook(() => + useStructuredAgentSession({ + sessionId: 'session-1', + agent: 'claude', + target: { kind: 'local' }, + isVisible: true, + composerScopeKey: 'scope-1' + }) + ) + const rows = result.current.messages + .filter((message) => message.id === KEPT_ITEM.itemId) + .map(({ id, unsent }) => ({ id, ...(unsent ? { unsent } : {}) })) + unmount() + return rows +} + +beforeEach(() => { + queuedMessages = [] +}) + +it('draws no row for a rejected message while a card holds it under its id', () => { + queuedMessages = [card('kept')] + expect(keptRows()).toEqual([]) +}) + +it('draws it as not sent once no card holds it', () => { + expect(keptRows()).toEqual([{ id: KEPT_ITEM.itemId, unsent: true }]) +}) diff --git a/src/renderer/src/components/native-chat/use-structured-agent-session.ts b/src/renderer/src/components/native-chat/use-structured-agent-session.ts index 96ad805560d..46571fcf54f 100644 --- a/src/renderer/src/components/native-chat/use-structured-agent-session.ts +++ b/src/renderer/src/components/native-chat/use-structured-agent-session.ts @@ -120,6 +120,7 @@ export function useStructuredAgentSession(args: { target, fence: transportState.fence, submissions: transportState.submissions, + journalItems: transportState.journalItems, composerScopeKey, queueDelivery: { capability: queueCapability, enabled: queueFollowUps }, queuedMessageIds @@ -166,7 +167,8 @@ export function useStructuredAgentSession(args: { const messages = useStructuredAgentSessionMessages( transportState.journalItems, transcriptOutbox, - transportState.submissions + transportState.submissions, + queuedMessageIds ) const queuedController = useStructuredAgentSessionQueuedMessages({ enabled: queueCapable && transportState.fence !== null, @@ -216,6 +218,8 @@ export function useStructuredAgentSession(args: { failedHere: outboxController.failedHere, /** The journal's rows for sent messages, which carry a rejected message's whole fact. */ submissions: transportState.submissions, + /** The host's queued cards, which hold their own rejected hand-offs. */ + queuedMessageIds, // A message typed during a command queues behind it on the host. send: (...input: Parameters) => // Legacy: an older host refuses sends while a command runs; removable once those hosts age out. diff --git a/src/shared/agent-session-conversation-outline.ts b/src/shared/agent-session-conversation-outline.ts index 2b6bb846adb..4d6b814d64d 100644 --- a/src/shared/agent-session-conversation-outline.ts +++ b/src/shared/agent-session-conversation-outline.ts @@ -89,7 +89,8 @@ export function projectAgentSessionConversationOutline( } const entries: AgentSessionConversationOutlineEntry[] = [] const transcript = projectNativeChatTranscriptMessages( - projectStructuredAgentSessionMessages(items, [], submissions) + // Unchanged on the wire: a desktop's rejected rows tick once their page is loaded. + projectStructuredAgentSessionMessages(items, [], submissions, { rejectedInPlace: false }) ) for (const message of transcript) { const sequence = sequences.get(message.id) diff --git a/src/shared/native-chat-provider-retry-runs.test.ts b/src/shared/native-chat-provider-retry-runs.test.ts index 0dff50c5f1d..3bb0c2d0227 100644 --- a/src/shared/native-chat-provider-retry-runs.test.ts +++ b/src/shared/native-chat-provider-retry-runs.test.ts @@ -49,7 +49,7 @@ function texts(messages: readonly NativeChatMessage[]): string[] { } function drawn(items: AgentJournalRenderItem[]): string[] { - return texts(projectStructuredAgentSessionMessages(items, [], [])) + return texts(projectStructuredAgentSessionMessages(items, [], [], { rejectedInPlace: true })) } describe('a run of provider retry rows', () => { @@ -101,7 +101,8 @@ describe('a run of provider retry rows', () => { retry(2, 'subagent-1') ], [], - [] + [], + { rejectedInPlace: true } ) ) expect(texts(conversation)).toEqual([ @@ -136,7 +137,8 @@ describe('a run of provider retry rows', () => { resolvedAt: null, handoverRecorded: true } - ] + ], + { rejectedInPlace: true } ) expect(messages.map((message) => [message.id, message.queued ?? false])).toEqual([ [expect.stringMatching(/^item-/), false], diff --git a/src/shared/native-chat-types.ts b/src/shared/native-chat-types.ts index 127a9837910..7fd5fe66192 100644 --- a/src/shared/native-chat-types.ts +++ b/src/shared/native-chat-types.ts @@ -214,8 +214,8 @@ export type NativeChatMessage = AgentJournalProducerLinkage & { sentAs?: AgentJournalMessageSendMode /** Accepted but not yet handed to the agent: drawn after everything the agent has done. */ queued?: true - /** Shown as not sent, waiting for the user's Retry: in no turn, so a newer turn's bar and clock - * never land on it, and drawn after the conversation. */ + /** Shown as not sent: in no turn, so a newer turn's bar and clock never land on it. Drawn where + * the journal recorded it, or after the conversation when it holds no place there. */ unsent?: true /** Set only by the structured projection, on rows the journal holds, and ranks * them ahead of time. Terminal-backed messages never carry it, and worker reads strip it. */ diff --git a/src/shared/structured-agent-session-draft-hand-off.test.ts b/src/shared/structured-agent-session-draft-hand-off.test.ts index 6e0cbc7e644..1b42e1a93a1 100644 --- a/src/shared/structured-agent-session-draft-hand-off.test.ts +++ b/src/shared/structured-agent-session-draft-hand-off.test.ts @@ -44,7 +44,9 @@ function handOff(dispatchState: AgentJournalSubmission['dispatchState']): AgentJ describe('a queued draft handed off under a fresh submission id', () => { it('takes the outbox entry off the client, in every dispatch state', () => { for (const state of ['pending', 'accepted', 'rejected', 'unknown'] as const) { - expect(reconcileStructuredAgentSessionOutboxWithQueue([entry], [handOff(state)])).toEqual([]) + expect(reconcileStructuredAgentSessionOutboxWithQueue([entry], [handOff(state)], [])).toEqual( + [] + ) } }) diff --git a/src/shared/structured-agent-session-draft-hand-off.ts b/src/shared/structured-agent-session-draft-hand-off.ts index 29a14159038..4e756d331f7 100644 --- a/src/shared/structured-agent-session-draft-hand-off.ts +++ b/src/shared/structured-agent-session-draft-hand-off.ts @@ -2,11 +2,9 @@ // under a fresh submission id, so this link, never a draft id compared with a `clientMessageId`, // is how a client knows the host has taken a message over. -import type { AgentJournalSubmission } from './agent-session-journal-types' -import { - reconcileStructuredAgentSessionOutbox, - type StructuredAgentSessionOutboxEntry -} from './structured-agent-session-outbox' +import type { AgentJournalRenderItem, AgentJournalSubmission } from './agent-session-journal-types' +import type { StructuredAgentSessionOutboxEntry } from './structured-agent-session-outbox' +import { reconcileStructuredAgentSessionOutbox } from './structured-agent-session-outbox-reconcile' /** Ids of the queued drafts the journal shows handed off, in any dispatch state. An outbox entry * under one of these ids belongs to the host: its card or bubble carries the text from here. */ @@ -29,13 +27,14 @@ export function handedOffQueuedMessageIds( */ export function reconcileStructuredAgentSessionOutboxWithQueue( entries: readonly StructuredAgentSessionOutboxEntry[], - submissions: readonly AgentJournalSubmission[] -): StructuredAgentSessionOutboxEntry[] { + submissions: readonly AgentJournalSubmission[], + items: readonly AgentJournalRenderItem[] +): readonly StructuredAgentSessionOutboxEntry[] { const handedOff = handedOffQueuedMessageIds(submissions) + const ours = entries.filter((entry) => !handedOff.has(entry.clientMessageId)) return reconcileStructuredAgentSessionOutbox( - handedOff.size === 0 - ? entries - : entries.filter((entry) => !handedOff.has(entry.clientMessageId)), - submissions + ours.length === entries.length ? entries : ours, + submissions, + items ) } diff --git a/src/shared/structured-agent-session-message-projection.ts b/src/shared/structured-agent-session-message-projection.ts index 5515b6478a2..57a2f6fe15d 100644 --- a/src/shared/structured-agent-session-message-projection.ts +++ b/src/shared/structured-agent-session-message-projection.ts @@ -1,33 +1,110 @@ import type { AgentJournalRenderItem, AgentJournalSubmission } from './agent-session-journal-types' import { agentJournalSubmissionKey } from './agent-session-journal-item-key' -import { agentJournalItemPosition } from './agent-session-journal-position' import { isQueuedAgentJournalSubmission } from './agent-session-queued-submission' import { collapseProviderRetryRuns } from './native-chat-provider-retry-runs' import type { NativeChatMessage } from './native-chat-types' +import { dispatchWasWithdrawn } from './structured-agent-session-dispatch-rejection' import type { StructuredAgentSessionOutboxEntry } from './structured-agent-session-outbox' import { structuredAgentSessionEntryHeldForRetry } from './structured-agent-session-outbox-admission' import { reconcileStructuredAgentSessionOutboxWithQueue } from './structured-agent-session-draft-hand-off' import { projectStructuredItemsToNativeChat } from './structured-agent-session-projection' +export type StructuredAgentSessionMessageProjectionOptions = { + /** Draw a message the host accepted and then rejected where the host recorded it, as not sent. + * Off for a client that hands such a message back to its composer instead. */ + rejectedInPlace: boolean + /** The queue's live cards: a rejected message one of them holds is drawn there, not here. */ + queuedMessageIds?: readonly string[] +} + +/** The loaded items that are a conversation command such as `/compact`, by item id. */ +export function structuredAgentSessionCommandItemIds( + items: readonly AgentJournalRenderItem[] +): Set { + return new Set( + items + .filter((item) => item.body.kind === 'message' && item.body.command) + .map((item) => item.itemId) + ) +} + +/** + * The rejected submissions the host's history shows in place as not sent, by item id; `submissions` + * in submission order, as the client keeps them. A withdrawn one went back to its sender, one the + * queue holds (a draft's hand-off, or a card under its id) is drawn as its card, and a command such + * as `/compact` has its rejection reported as its own reply. + */ +export function structuredAgentSessionRejectedShownInPlace( + submissions: readonly AgentJournalSubmission[], + queuedMessageIds: readonly string[], + commandItemIds: ReadonlySet +): Set { + const cards = new Set(queuedMessageIds) + // Each body's copies, as positions in submission order. A withdrawn one is hidden too, so it + // supersedes nothing. + const copies = new Map() + for (const [index, submission] of submissions.entries()) { + if (!dispatchWasWithdrawn(submission)) { + const copy = { index, submittedAt: submission.submittedAt } + const same = copies.get(submission.payloadFingerprint) + if (same) { + same.push(copy) + } else { + copies.set(submission.payloadFingerprint, [copy]) + } + } + } + const shown = new Set() + for (const [index, submission] of submissions.entries()) { + const { resolvedAt } = submission + if ( + submission.dispatchState !== 'rejected' || + dispatchWasWithdrawn(submission) || + submission.queuedMessageId !== undefined || + cards.has(submission.clientMessageId) || + commandItemIds.has(agentJournalSubmissionKey(submission.clientMessageId)) || + // Collapses resends of a rejected message: past Retries resent it under a new id, and the + // host re-delivers its own messages under new ids. Only a later copy sent once the rejection + // was known counts, so a repeat sent before it is kept. + (resolvedAt !== null && + (copies.get(submission.payloadFingerprint) ?? []).some( + (copy) => copy.index > index && copy.submittedAt >= resolvedAt + )) + ) { + continue + } + shown.add(agentJournalSubmissionKey(submission.clientMessageId)) + } + return shown +} + export function projectStructuredAgentSessionMessages( items: readonly AgentJournalRenderItem[], outbox: readonly StructuredAgentSessionOutboxEntry[], submissions: readonly AgentJournalSubmission[], + options: StructuredAgentSessionMessageProjectionOptions, projectItems = projectStructuredItemsToNativeChat ): NativeChatMessage[] { - const optimistic = reconcileStructuredAgentSessionOutboxWithQueue(outbox, submissions) - // Refused sends are ledger evidence, not conversation history; local drafts remain in the outbox. + const optimistic = reconcileStructuredAgentSessionOutboxWithQueue(outbox, submissions, items) + // Refused sends are ledger evidence, not conversation history, unless drawn in place as not sent. const rejected = new Set( submissions .filter((submission) => submission.dispatchState === 'rejected') .map((submission) => agentJournalSubmissionKey(submission.clientMessageId)) ) + const inPlace = options.rejectedInPlace + ? structuredAgentSessionRejectedShownInPlace( + submissions, + options.queuedMessageIds ?? [], + structuredAgentSessionCommandItemIds(items) + ) + : new Set() const visibleItems: AgentJournalRenderItem[] = [] - const refused = new Map() + const unsentItems: AgentJournalRenderItem[] = [] for (const item of items) { - if (rejected.has(item.itemId)) { - refused.set(item.itemId, item) - } else { + if (inPlace.has(item.itemId)) { + unsentItems.push(item) + } else if (!rejected.has(item.itemId)) { visibleItems.push(item) } } @@ -52,23 +129,19 @@ export function projectStructuredAgentSessionMessages( // After the held sends leave: they are drawn after the conversation, never inside a run. ...collapseProviderRetryRuns(delivered), ...held, + // In no turn, like the outbox's not-sent rows; the journal position keeps their place. + ...projectItems(unsentItems).map((message) => ({ ...message, unsent: true as const })), ...optimistic .filter((entry) => !journalled.has(agentJournalSubmissionKey(entry.clientMessageId))) - .map((entry): NativeChatMessage => { - const id = agentJournalSubmissionKey(entry.clientMessageId) - const recorded = refused.get(id) - return { - id, - role: 'user', - source: 'transcript', - timestamp: entry.queuedAt, - blocks: entry.body.blocks, - ...(entry.state === 'rejected' || structuredAgentSessionEntryHeldForRetry(entry) - ? { unsent: true as const } - : {}), - // A send the journal recorded before refusing it keeps its place there. - ...(recorded ? { journalPosition: agentJournalItemPosition(recorded) } : {}) - } - }) + .map((entry): NativeChatMessage => ({ + id: agentJournalSubmissionKey(entry.clientMessageId), + role: 'user', + source: 'transcript', + timestamp: entry.queuedAt, + blocks: entry.body.blocks, + ...(entry.state === 'rejected' || structuredAgentSessionEntryHeldForRetry(entry) + ? { unsent: true as const } + : {}) + })) ] } diff --git a/src/shared/structured-agent-session-outbox-reconcile.test.ts b/src/shared/structured-agent-session-outbox-reconcile.test.ts new file mode 100644 index 00000000000..0336bd649e4 --- /dev/null +++ b/src/shared/structured-agent-session-outbox-reconcile.test.ts @@ -0,0 +1,100 @@ +// The reconcile runs on every journal batch, so reading an unchanged journal again must change +// nothing: the same entries, and the same list. + +import { expect, it } from 'vitest' +import { agentJournalSubmissionKey } from './agent-session-journal-item-key' +import type { + AgentJournalDispatchState, + AgentJournalRenderItem, + AgentJournalSubmission +} from './agent-session-journal-types' +import { + createStructuredAgentSessionOutboxEntry, + type StructuredAgentSessionOutboxEntry +} from './structured-agent-session-outbox' +import { reconcileStructuredAgentSessionOutbox } from './structured-agent-session-outbox-reconcile' + +function entry( + id: string, + patch: Partial = {} +): StructuredAgentSessionOutboxEntry { + return { + ...createStructuredAgentSessionOutboxEntry({ + clientMessageId: id, + sessionId: 'session-1', + text: id, + attachments: [], + queuedAt: 1 + }), + ...patch + } +} + +function submission(id: string, dispatchState: AgentJournalDispatchState): AgentJournalSubmission { + return { + clientMessageId: id, + fence: 1, + payloadFingerprint: id, + dispatchState, + providerItemId: null, + reason: dispatchState === 'unknown' ? 'in doubt' : null, + ...(dispatchState === 'rejected' ? { rejection: { kind: 'hostRestarted' } } : {}), + submittedAt: 5, + resolvedAt: dispatchState === 'pending' ? null : 6 + } +} + +function row(id: string): AgentJournalRenderItem { + return { + itemId: agentJournalSubmissionKey(id), + revision: 1, + sequence: 1, + observedAt: 1, + body: { kind: 'message', role: 'user', blocks: [{ type: 'text', text: id }] } + } +} + +it('returns every kept entry, and the list, as themselves when read again', () => { + const entries = [ + entry('queued'), + entry('landing', { state: 'unconfirmed', lastFailure: { kind: 'failed' } }), + entry('sending', { state: 'dispatching' }), + entry('doubt', { state: 'dispatching', lastFailure: { kind: 'failed' } }), + entry('rejected-unloaded', { state: 'dispatching' }), + entry('rejected-loaded', { state: 'dispatching' }), + entry('delivered', { state: 'dispatching' }), + entry('refused', { + state: 'rejected', + lastFailure: { kind: 'refused', code: 'agent_session_operation_invalid' } + }) + ] + const submissions = [ + submission('landing', 'pending'), + submission('sending', 'pending'), + submission('doubt', 'unknown'), + submission('rejected-unloaded', 'rejected'), + submission('rejected-loaded', 'rejected'), + submission('delivered', 'accepted') + ] + const items = [row('rejected-loaded')] + + const first = reconcileStructuredAgentSessionOutbox(entries, submissions, items) + expect(first.map((kept) => [kept.clientMessageId, kept.state])).toEqual([ + ['queued', 'queued'], + ['landing', 'dispatching'], + ['sending', 'dispatching'], + ['doubt', 'unconfirmed'], + ['rejected-unloaded', 'rejected'], + ['refused', 'rejected'] + ]) + const second = reconcileStructuredAgentSessionOutbox(first, submissions, items) + expect(second).toBe(first) + second.forEach((kept, index) => expect(kept).toBe(first[index])) +}) + +it('returns the list it was given when nothing changes', () => { + const entries = [entry('queued'), entry('doubt', { state: 'unconfirmed' })] + expect(reconcileStructuredAgentSessionOutbox(entries, [submission('doubt', 'unknown')], [])).toBe( + entries + ) +}) diff --git a/src/shared/structured-agent-session-outbox-reconcile.ts b/src/shared/structured-agent-session-outbox-reconcile.ts new file mode 100644 index 00000000000..9fa699eece1 --- /dev/null +++ b/src/shared/structured-agent-session-outbox-reconcile.ts @@ -0,0 +1,69 @@ +// How the journal's view of each submission settles the outbox: the sibling of +// `disposeStructuredAgentSessionSendResult`, which folds a single send's answer. + +import { agentJournalSubmissionKey } from './agent-session-journal-item-key' +import type { AgentJournalRenderItem, AgentJournalSubmission } from './agent-session-journal-types' +import { dispatchWasWithdrawn } from './structured-agent-session-dispatch-rejection' +import { + structuredAgentSessionEntryRejectedByHost, + structuredAgentSessionRejectedFailure, + type StructuredAgentSessionOutboxEntry +} from './structured-agent-session-outbox' + +export function reconcileStructuredAgentSessionOutbox( + entries: readonly StructuredAgentSessionOutboxEntry[], + submissions: readonly AgentJournalSubmission[], + /** The loaded journal rows: a rejected message leaves only once the row that draws it is here. */ + items: readonly AgentJournalRenderItem[] +): readonly StructuredAgentSessionOutboxEntry[] { + const settled = new Map(submissions.map((entry) => [entry.clientMessageId, entry])) + let loaded: Set | undefined + // An entry whose reconciled value is unchanged is returned as itself, and so is the list when + // none changed: a caller re-reading every journal batch writes nothing then. + const next = entries.flatMap((entry) => { + const submission = settled.get(entry.clientMessageId) + // Settled by the host: its history shows one delivered; a withdrawn one goes back to the + // composer. + if (submission?.dispatchState === 'accepted' || dispatchWasWithdrawn(submission)) { + return [] + } + if (submission?.dispatchState === 'rejected') { + // Its row draws it once loaded; until then the entry does, as the host recorded it. An older + // host leaves that row where it was sent, which may be outside the loaded window. + loaded ??= new Set(items.map((item) => item.itemId)) + if (loaded.has(agentJournalSubmissionKey(entry.clientMessageId))) { + return [] + } + const lastFailure = structuredAgentSessionRejectedFailure(submission) + return [ + structuredAgentSessionEntryRejectedByHost(entry) + ? entry + : { ...entry, state: 'rejected' as const, lastFailure } + ] + } + if (submission?.dispatchState === 'pending') { + if (entry.state === 'dispatching') { + return [entry] + } + // The host has it, so no failure of an earlier attempt describes it now. + const { lastFailure: _landed, ...landed } = entry + return [{ ...landed, state: 'dispatching' as const }] + } + if ( + submission?.dispatchState === 'unknown' && + entry.retryAfterUnknownSubmittedAt !== -1 && + entry.retryAfterUnknownSubmittedAt !== submission.submittedAt + ) { + // In doubt now, not failed: the probe's resend decides it, as for any unconfirmed send. + if (entry.state === 'unconfirmed' && entry.lastFailure === undefined) { + return [entry] + } + const { lastFailure: _superseded, ...inDoubt } = entry + return [{ ...inDoubt, state: 'unconfirmed' as const }] + } + return [entry] + }) + return next.length === entries.length && next.every((entry, index) => entry === entries[index]) + ? entries + : next +} diff --git a/src/shared/structured-agent-session-outbox-retry-hold.test.ts b/src/shared/structured-agent-session-outbox-retry-hold.test.ts index 3f0c439fc9a..cd2bb570b57 100644 --- a/src/shared/structured-agent-session-outbox-retry-hold.test.ts +++ b/src/shared/structured-agent-session-outbox-retry-hold.test.ts @@ -2,9 +2,9 @@ import { describe, expect, it } from 'vitest' import type { AgentJournalSubmission } from './agent-session-journal-types' import { createStructuredAgentSessionOutboxEntry, - reconcileStructuredAgentSessionOutbox, type StructuredAgentSessionOutboxEntry } from './structured-agent-session-outbox' +import { reconcileStructuredAgentSessionOutbox } from './structured-agent-session-outbox-reconcile' import { admitStructuredAgentSessionOutboxEntry, structuredAgentSessionEntryHeldForRetry @@ -71,7 +71,8 @@ describe('a message held for its Retry', () => { } const [landed] = reconcileStructuredAgentSessionOutbox( [entry('held', { lastFailure: REFUSED })], - [submission] + [submission], + [] ) expect(landed?.state).toBe('dispatching') expect(landed?.lastFailure).toBeUndefined() @@ -91,7 +92,8 @@ describe('a message held for its Retry', () => { } const reconciled = reconcileStructuredAgentSessionOutbox( [entry('held', { lastAttemptAt: 2, lastFailure: REFUSED }), entry('next')], - [submission] + [submission], + [] ) expect(reconciled[0]).toMatchObject({ state: 'unconfirmed' }) expect(reconciled[0]?.lastFailure).toBeUndefined() diff --git a/src/shared/structured-agent-session-outbox.ts b/src/shared/structured-agent-session-outbox.ts index 7b3606f3dc8..32dcab0e66b 100644 --- a/src/shared/structured-agent-session-outbox.ts +++ b/src/shared/structured-agent-session-outbox.ts @@ -1,6 +1,6 @@ import type { AgentSessionFailureFact } from './agent-session-failure' import { readWholeAgentSessionFailureFact } from './agent-session-failure' -import type { AgentJournalMessageItem, AgentJournalSubmission } from './agent-session-journal-types' +import type { AgentJournalMessageItem } from './agent-session-journal-types' import { parseAgentSessionWriteFailure, type AgentSessionWriteFailure, @@ -14,11 +14,11 @@ import { structuredAgentSessionMessageSendMutation, type StructuredAgentSessionSendMutation } from './structured-agent-session-send-mutation' -import { classifyDispatchRejection } from './structured-agent-session-dispatch-rejection' import { parseStructuredAgentSessionOutboxQueueFields } from './structured-agent-session-outbox-delivery' -/** `rejected`: the host settled the send as not delivered. The drain never sends it again on its - * own and nothing queues behind it; only the user's Retry does. */ +/** `rejected`: settled as not delivered. The drain never sends it again and nothing queues behind + * it. One the host refused unrecorded waits for the user's Retry. One it recorded owes no delivery + * and leaves on the batch or page that loads its row (`structured-agent-session-outbox-reconcile`). */ export type StructuredAgentSessionOutboxState = | 'queued' | 'dispatching' @@ -176,6 +176,14 @@ export function structuredAgentSessionEntryIdExpired( ) } +/** The host recorded this send and then rejected it: no Retry, since sending it again is a new + * message. The reconcile drops it once the client holds the rejected submission. */ +export function structuredAgentSessionEntryRejectedByHost( + entry: StructuredAgentSessionOutboxEntry +): boolean { + return entry.state === 'rejected' && entry.lastFailure?.kind === 'rejected' +} + export function requeueStructuredAgentSessionSendRefusal( entry: StructuredAgentSessionOutboxEntry, refusal: AgentSessionWriteRefusal, @@ -208,58 +216,6 @@ export function requeueStructuredAgentSessionSendRefusal( } } -export function reconcileStructuredAgentSessionOutbox( - entries: readonly StructuredAgentSessionOutboxEntry[], - submissions: readonly AgentJournalSubmission[] -): StructuredAgentSessionOutboxEntry[] { - const settled = new Map(submissions.map((entry) => [entry.clientMessageId, entry])) - return entries.flatMap((entry) => { - const submission = settled.get(entry.clientMessageId) - if (submission?.dispatchState === 'accepted') { - return [] - } - if ( - submission?.dispatchState === 'rejected' && - classifyDispatchRejection(submission).category === 'withdrawn' - ) { - return [] - } - if (submission?.dispatchState === 'pending') { - if (entry.state === 'dispatching') { - return [entry] - } - // The host has it, so no failure of an earlier attempt describes it now. - const { lastFailure: _landed, ...landed } = entry - return [{ ...landed, state: 'dispatching' as const }] - } - // Accepted, then not delivered — the agent never started, or its start was refused. The text - // and why stay here for the user's Retry, and nothing queues behind it. `unconfirmed` is how a - // remount reads an entry it left dispatching; the journal has since answered it. - if ( - submission?.dispatchState === 'rejected' && - (entry.state === 'dispatching' || entry.state === 'unconfirmed') - ) { - return [ - { - ...entry, - state: 'rejected' as const, - lastFailure: structuredAgentSessionRejectedFailure(submission) - } - ] - } - if ( - submission?.dispatchState === 'unknown' && - entry.retryAfterUnknownSubmittedAt !== -1 && - entry.retryAfterUnknownSubmittedAt !== submission.submittedAt - ) { - // In doubt now, not failed: the probe's resend decides it, as for any unconfirmed send. - const { lastFailure: _superseded, ...inDoubt } = entry - return [{ ...inDoubt, state: 'unconfirmed' as const }] - } - return [entry] - }) -} - export function parseStructuredAgentSessionOutboxEntry( value: unknown, sessionId: string diff --git a/src/shared/structured-agent-session-send-disposition.test.ts b/src/shared/structured-agent-session-send-disposition.test.ts index 98ea2ff8b67..9605764b017 100644 --- a/src/shared/structured-agent-session-send-disposition.test.ts +++ b/src/shared/structured-agent-session-send-disposition.test.ts @@ -19,9 +19,9 @@ import { } from './structured-agent-session-send-disposition' import { createStructuredAgentSessionOutboxEntry, - reconcileStructuredAgentSessionOutbox, type StructuredAgentSessionOutboxEntry } from './structured-agent-session-outbox' +import { reconcileStructuredAgentSessionOutbox } from './structured-agent-session-outbox-reconcile' import { structuredAgentSessionEntryHeldForRetry } from './structured-agent-session-outbox-admission' const entry: StructuredAgentSessionOutboxEntry = createStructuredAgentSessionOutboxEntry({ @@ -120,7 +120,9 @@ describe('what a rejection shows the user', () => { throw new Error('expected rejected submission fixture') } - expect(reconcileStructuredAgentSessionOutbox([entry], [result.value.submission])).toEqual([]) + expect(reconcileStructuredAgentSessionOutbox([entry], [result.value.submission], [])).toEqual( + [] + ) }) it('never puts the transport marker on screen', () => { @@ -168,7 +170,9 @@ describe('what a rejection shows the user', () => { if (!result.ok || !('submission' in result.value)) { throw new Error('expected rejected submission fixture') } - expect(reconcileStructuredAgentSessionOutbox([entry], [result.value.submission])).toEqual([]) + expect(reconcileStructuredAgentSessionOutbox([entry], [result.value.submission], [])).toEqual( + [] + ) expect( disposeStructuredAgentSessionSendResult({ entries: [entry], diff --git a/src/shared/structured-agent-session-send-disposition.ts b/src/shared/structured-agent-session-send-disposition.ts index d503b453b8e..77e1030d4b2 100644 --- a/src/shared/structured-agent-session-send-disposition.ts +++ b/src/shared/structured-agent-session-send-disposition.ts @@ -83,8 +83,8 @@ function dropEntry(input: SendDispositionInput): StructuredAgentSessionOutboxEnt * the outbox. Nothing is lost from the conversation: the durable submission row * already renders the message. * - * A `rejected` submission takes the other path — the message provably did not - * happen, so Retry rotates the id and sends it as a genuinely new message. + * A `rejected` submission is the host's to show, with no Retry: the reconcile + * drops its entry once the journal carries it. */ function refusedRedelivery( entry: StructuredAgentSessionOutboxEntry, @@ -274,6 +274,8 @@ export function disposeStructuredAgentSessionSendResult( error: null } } + // Recorded, so the journal's row shows it once it arrives; until then the entry draws it, saying + // why and offering no Retry. if (submission.dispatchState === 'rejected') { return { entries: replaceEntryState( From f0b5b8566ce27a8bc1c9035194a074f4cf92c71c Mon Sep 17 00:00:00 2001 From: Neil <4138956+nwparker@users.noreply.github.com> Date: Sun, 4 Oct 2026 14:31:22 -0700 Subject: [PATCH 3/8] Bound OpenCode history reads and repeated worker failures (#25292) * fix(opencode): keep scan budgets across queue waits and batches Reuse the scan-owned lifetime proposed in #10708 by @AmethystLiang with the existing shared worker queue. * test(opencode): check nonempty session fixtures and lint scoped controls * Derive OpenCode scan deadline message from its budget --------- Co-authored-by: Neil Parker Co-authored-by: OpenCode issue campaign --- ...sion-scanner-opencode-cancellation.test.ts | 88 +++++++++- ...scanner-opencode-sqlite-scan-scope.test.ts | 138 +++++++++++++++ ...sion-scanner-opencode-sqlite-scan-scope.ts | 75 ++++++++ ...nner-opencode-sqlite-worker-client.test.ts | 62 ++++++- ...n-scanner-opencode-sqlite-worker-client.ts | 34 +++- ...on-scanner-opencode-sqlite-worker-spawn.ts | 31 +++- ...ssion-scanner-opencode-wsl-routing.test.ts | 47 ++++- src/main/ai-vault/session-scanner.ts | 5 + src/main/worker-thread-request-queue.test.ts | 160 +++++++++++++++++- src/main/worker-thread-request-queue.ts | 59 +++++-- 10 files changed, 666 insertions(+), 33 deletions(-) create mode 100644 src/main/ai-vault/session-scanner-opencode-sqlite-scan-scope.test.ts create mode 100644 src/main/ai-vault/session-scanner-opencode-sqlite-scan-scope.ts diff --git a/src/main/ai-vault/session-scanner-opencode-cancellation.test.ts b/src/main/ai-vault/session-scanner-opencode-cancellation.test.ts index 4f47af380cd..084a73cd661 100644 --- a/src/main/ai-vault/session-scanner-opencode-cancellation.test.ts +++ b/src/main/ai-vault/session-scanner-opencode-cancellation.test.ts @@ -1,4 +1,7 @@ import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' +import { mkdtempSync, rmSync, writeFileSync } from 'node:fs' +import { tmpdir } from 'node:os' +import { join } from 'node:path' import type * as workerSpawn from './session-scanner-opencode-sqlite-worker-spawn' import type { SessionFileDiscovery } from './session-scanner-types' import type { TranscriptReadOutcome } from './session-transcript-consumers' @@ -24,6 +27,7 @@ import { registerTranscriptConsumer, resetTranscriptConsumersForTests } from './session-transcript-consumers' +import { runOpenCodeSqliteScanRequest } from './session-scanner-opencode-sqlite-scan-scope' const file = { path: '/fixture/opencode.db#session', @@ -40,6 +44,7 @@ beforeEach(() => { resetSessionParseCacheForTests() }) afterEach(() => { + vi.useRealTimers() resetTranscriptConsumersForTests() resetSessionParseCacheForTests() }) @@ -48,7 +53,11 @@ function configure(agent: 'opencode' | 'opencode2') { readers.discover.mockResolvedValue([{ agent, rootDir: '/fixture', files: [file] }]) const accumulator = createAccumulator({ agent, file, sessionId: 'session' }) accumulator.title = 'SQLite session' - return finalizeSession(accumulator, 'linux') + const session = finalizeSession(accumulator, 'linux') + if (!session) { + throw new Error('Configured SQLite session was empty') + } + return session } function untilAborted(signal: AbortSignal | undefined): Promise { @@ -61,6 +70,83 @@ function untilAborted(signal: AbortSignal | undefined): Promise { } describe.each(['opencode', 'opencode2'] as const)('%s scan cancellation', (agent) => { + it('reports a deadline while retaining completed sessions and other agents, then retries', async () => { + vi.useFakeTimers() + const root = mkdtempSync(join(tmpdir(), 'orca-scan-deadline-')) + try { + const session = configure(agent) + const blocked = { ...file, path: '/fixture/opencode.db#blocked' } + const claudePath = join(root, 'claude.jsonl') + writeFileSync( + claudePath, + `${JSON.stringify({ + type: 'user', + sessionId: 'retained-claude', + timestamp: '2026-05-01T10:00:00.000Z', + cwd: root, + message: { role: 'user', content: 'Retain this other-agent session' } + })}\n` + ) + readers.discover.mockResolvedValue([ + { agent, rootDir: '/fixture', files: [file, blocked] }, + { agent: 'claude', rootDir: root, files: [{ ...file, path: claudePath }] } + ]) + readers.parse.mockImplementation(({ sessionId, signal }) => + sessionId === 'blocked' + ? runOpenCodeSqliteScanRequest(signal, untilAborted) + : Promise.resolve(session) + ) + const pending = scanAiVaultSessions({ platform: 'linux' }) + await vi.waitFor(() => expect(readers.parse).toHaveBeenCalledTimes(2)) + await vi.advanceTimersByTimeAsync(45_000) + const result = await pending + expect(result.sessions.map((row) => row.sessionId)).toEqual( + expect.arrayContaining(['session', 'retained-claude']) + ) + expect(result.sessions).toHaveLength(2) + expect(result.issues).toEqual([ + expect.objectContaining({ + agent, + path: blocked.path, + message: expect.stringContaining('45s work budget') + }) + ]) + readers.parse.mockResolvedValue({ ...session, sessionId: 'blocked', filePath: blocked.path }) + const recovered = await scanAiVaultSessions({ platform: 'linux' }) + expect(recovered.sessions).toHaveLength(3) + expect( + readers.parse.mock.calls.filter(([args]) => args.sessionId === 'blocked') + ).toHaveLength(2) + } finally { + rmSync(root, { recursive: true, force: true }) + } + }) + + it('marks a deadline capture incomplete instead of caching a failed history', async () => { + vi.useFakeTimers() + const session = configure(agent) + const outcomes: TranscriptReadOutcome[] = [] + registerTranscriptConsumer({ + beginRead: () => ({ message() {}, finish: (outcome) => outcomes.push(outcome) }) + }) + readers.capture.mockImplementationOnce(({ signal }) => + runOpenCodeSqliteScanRequest(signal, untilAborted) + ) + const pending = scanAiVaultSessions({ platform: 'linux' }) + await vi.waitFor(() => expect(readers.capture).toHaveBeenCalledOnce()) + await vi.advanceTimersByTimeAsync(45_000) + const result = await pending + expect(result.sessions).toEqual([]) + expect(result.issues).toEqual([ + expect.objectContaining({ message: expect.stringContaining('45s work budget') }) + ]) + expect(outcomes).toEqual([{ session: null, byteOffset: 0, incomplete: true }]) + readers.capture.mockResolvedValue({ session, messages }) + expect((await scanAiVaultSessions({ platform: 'linux' })).sessions).toHaveLength(1) + expect(readers.capture).toHaveBeenCalledTimes(2) + expect(outcomes.at(-1)?.incomplete).toBe(false) + }) + it.each(['parse', 'capture'] as const)( 'cancels an active %s and retries the uncached read', async (mode) => { diff --git a/src/main/ai-vault/session-scanner-opencode-sqlite-scan-scope.test.ts b/src/main/ai-vault/session-scanner-opencode-sqlite-scan-scope.test.ts new file mode 100644 index 00000000000..bc97f1aa388 --- /dev/null +++ b/src/main/ai-vault/session-scanner-opencode-sqlite-scan-scope.test.ts @@ -0,0 +1,138 @@ +import { afterEach, expect, it, vi } from 'vitest' +import { + OPENCODE_SQLITE_SCAN_BUDGET_MS, + runOpenCodeSqliteScanRequest, + withOpenCodeSqliteScanScope +} from './session-scanner-opencode-sqlite-scan-scope' + +afterEach(() => vi.useRealTimers()) + +function waitForAbort(signal: AbortSignal | undefined): Promise { + if (!signal) { + throw new Error('Missing scoped request signal') + } + return new Promise((_resolve, reject) => { + signal.addEventListener('abort', () => reject(signal.reason), { once: true }) + }) +} + +function wait(ms: number): Promise { + return new Promise((resolve) => setTimeout(resolve, ms)) +} + +it('spends the budget while admission is pending and refuses later work in that scan', async () => { + vi.useFakeTimers() + const admitted = vi.fn() + const outcome = withOpenCodeSqliteScanScope(async () => { + const error = await runOpenCodeSqliteScanRequest(undefined, waitForAbort).catch((err) => err) + expect(error).toMatchObject({ name: 'OpenCodeSqliteScanDeadlineError' }) + await expect(runOpenCodeSqliteScanRequest(undefined, admitted)).rejects.toBe(error) + }) + await vi.advanceTimersByTimeAsync(OPENCODE_SQLITE_SCAN_BUDGET_MS) + await outcome + expect(admitted).not.toHaveBeenCalled() + expect(vi.getTimerCount()).toBe(0) +}) + +it('banks only outstanding work across legs and does not spend other-agent time', async () => { + vi.useFakeTimers() + const outcome = withOpenCodeSqliteScanScope(async () => { + await runOpenCodeSqliteScanRequest(undefined, () => wait(20_000)) + await wait(70_000) + return runOpenCodeSqliteScanRequest(undefined, waitForAbort) + }).catch((error) => error) + await vi.advanceTimersByTimeAsync(90_000) + let completed = false + void outcome.then(() => { + completed = true + }) + await vi.advanceTimersByTimeAsync(24_999) + expect(completed).toBe(false) + await vi.advanceTimersByTimeAsync(1) + expect(await outcome).toMatchObject({ name: 'OpenCodeSqliteScanDeadlineError' }) + expect(vi.getTimerCount()).toBe(0) +}) + +it('counts overlapping preparation and worker waits once', async () => { + vi.useFakeTimers() + const outcome = withOpenCodeSqliteScanScope(() => + runOpenCodeSqliteScanRequest(undefined, () => + Promise.all([ + runOpenCodeSqliteScanRequest(undefined, waitForAbort), + runOpenCodeSqliteScanRequest(undefined, waitForAbort) + ]) + ) + ).catch((error) => error) + await vi.advanceTimersByTimeAsync(OPENCODE_SQLITE_SCAN_BUDGET_MS - 1) + expect(vi.getTimerCount()).toBe(1) + await vi.advanceTimersByTimeAsync(1) + expect(await outcome).toMatchObject({ name: 'OpenCodeSqliteScanDeadlineError' }) + expect(vi.getTimerCount()).toBe(0) +}) + +it('keeps concurrent scans independent and gives the next scan a fresh owner and budget', async () => { + vi.useFakeTimers() + const owners: unknown[] = [] + const first = withOpenCodeSqliteScanScope(() => + runOpenCodeSqliteScanRequest(undefined, (signal, owner) => { + owners.push(owner) + return waitForAbort(signal) + }) + ).catch((error) => error) + await vi.advanceTimersByTimeAsync(30_000) + const second = withOpenCodeSqliteScanScope(() => + runOpenCodeSqliteScanRequest(undefined, (signal, owner) => { + owners.push(owner) + return waitForAbort(signal) + }) + ).catch((error) => error) + await vi.advanceTimersByTimeAsync(15_000) + expect(await first).toMatchObject({ name: 'OpenCodeSqliteScanDeadlineError' }) + expect(vi.getTimerCount()).toBe(1) + await vi.advanceTimersByTimeAsync(30_000) + expect(await second).toMatchObject({ name: 'OpenCodeSqliteScanDeadlineError' }) + await withOpenCodeSqliteScanScope(() => + runOpenCodeSqliteScanRequest(undefined, async (_signal, owner) => { + owners.push(owner) + }) + ) + expect(new Set(owners).size).toBe(3) + expect(vi.getTimerCount()).toBe(0) +}) + +it('preserves the caller cancellation reason and disposes its timer', async () => { + vi.useFakeTimers() + const controller = new AbortController() + const reason = new Error('caller cancelled') + const outcome = withOpenCodeSqliteScanScope(() => + runOpenCodeSqliteScanRequest(controller.signal, waitForAbort) + ).catch((error) => error) + controller.abort(reason) + expect(await outcome).toBe(reason) + expect(vi.getTimerCount()).toBe(0) +}) + +it('leaves unrelated native-chat and Zcode calls unscoped and retires the scan signal', async () => { + vi.useFakeTimers() + let scopedSignal: AbortSignal | undefined + const caller = new AbortController() + await withOpenCodeSqliteScanScope(async () => { + for (const agent of ['zcode', 'native-chat'] as const) { + await runOpenCodeSqliteScanRequest( + caller.signal, + async (signal, owner) => { + expect(signal).toBe(caller.signal) + expect(owner).toBeUndefined() + expect(vi.getTimerCount()).toBe(0) + }, + agent + ) + } + await runOpenCodeSqliteScanRequest(undefined, async (signal) => { + scopedSignal = signal + }) + }) + expect(scopedSignal?.aborted).toBe(true) + expect(caller.signal.aborted).toBe(false) + expect(vi.getTimerCount()).toBe(0) +}) diff --git a/src/main/ai-vault/session-scanner-opencode-sqlite-scan-scope.ts b/src/main/ai-vault/session-scanner-opencode-sqlite-scan-scope.ts new file mode 100644 index 00000000000..e3a53a535b2 --- /dev/null +++ b/src/main/ai-vault/session-scanner-opencode-sqlite-scan-scope.ts @@ -0,0 +1,75 @@ +import { AsyncLocalStorage } from 'node:async_hooks' +import { throwIfSignalAborted } from '../../shared/abort-signal-reason' +import type { WorkerThreadRequestOwner } from '../worker-thread-request-queue' + +export const OPENCODE_SQLITE_SCAN_BUDGET_MS = 45_000 + +class OpenCodeSqliteScanScope implements WorkerThreadRequestOwner { + private readonly controller = new AbortController() + readonly signal = this.controller.signal + private remainingMs = OPENCODE_SQLITE_SCAN_BUDGET_MS + private outstanding = 0 + private armedAt = 0 + private timer: NodeJS.Timeout | undefined + + async run( + callerSignal: AbortSignal | undefined, + fn: (signal: AbortSignal, owner: WorkerThreadRequestOwner) => Promise + ): Promise { + throwIfSignalAborted(callerSignal) + throwIfSignalAborted(this.signal) + if (this.outstanding++ === 0) { + this.armedAt = Date.now() + this.timer = setTimeout(() => { + const error = new Error( + `OpenCode SQLite scan exceeded its ${OPENCODE_SQLITE_SCAN_BUDGET_MS / 1000}s work budget` + ) + error.name = 'OpenCodeSqliteScanDeadlineError' + this.controller.abort(error) + }, this.remainingMs) + this.timer.unref?.() + } + const signal = callerSignal ? AbortSignal.any([callerSignal, this.signal]) : this.signal + try { + return await fn(signal, this) + } finally { + if (--this.outstanding === 0) { + this.pause() + } + } + } + + dispose(): void { + this.pause() + this.controller.abort(new Error('OpenCode SQLite scan ended')) + } + + private pause(): void { + if (this.timer) { + clearTimeout(this.timer) + this.timer = undefined + this.remainingMs = Math.max(0, this.remainingMs - (Date.now() - this.armedAt)) + } + } +} + +const scanScope = new AsyncLocalStorage() + +export async function withOpenCodeSqliteScanScope(fn: () => Promise): Promise { + const scope = new OpenCodeSqliteScanScope() + try { + return await scanScope.run(scope, fn) + } finally { + scope.dispose() + } +} + +// The clock covers outstanding SQLite work, including admission and WSL preparation. +export function runOpenCodeSqliteScanRequest( + signal: AbortSignal | undefined, + fn: (signal?: AbortSignal, owner?: WorkerThreadRequestOwner) => Promise, + agent?: 'opencode2' | 'zcode' | 'native-chat' +): Promise { + const scope = agent === 'zcode' || agent === 'native-chat' ? undefined : scanScope.getStore() + return scope ? scope.run(signal, fn) : fn(signal) +} diff --git a/src/main/ai-vault/session-scanner-opencode-sqlite-worker-client.test.ts b/src/main/ai-vault/session-scanner-opencode-sqlite-worker-client.test.ts index 93df4ed987c..68fb8d09d90 100644 --- a/src/main/ai-vault/session-scanner-opencode-sqlite-worker-client.test.ts +++ b/src/main/ai-vault/session-scanner-opencode-sqlite-worker-client.test.ts @@ -12,6 +12,7 @@ import type { OpenCodeSqliteWorkerResponse } from './session-scanner-opencode-sqlite-worker-protocol' import type { AiVaultScanIssue } from '../../shared/ai-vault-types' +import { withOpenCodeSqliteScanScope } from './session-scanner-opencode-sqlite-scan-scope' // A worker_threads stand-in the tests drive directly: it records posted requests // and lets a test emit message/error/exit without a built worker bundle. @@ -75,6 +76,63 @@ function makeFactory(workers: FakeWorker[]): () => Worker { } describe('OpenCodeSqliteWorkerClient', () => { + it('expires a scan in the FIFO without cancelling ordinary reads or a later scan', async () => { + vi.useFakeTimers() + const workers: FakeWorker[] = [] + const client = new OpenCodeSqliteWorkerClient({ workerFactory: makeFactory(workers), log() {} }) + const issues: AiVaultScanIssue[] = [] + try { + const ordinary = [1, 2].map((id) => + client.list({ + dbPaths: [`/ordinary-${id}.db`], + limit: 1, + issues: [] + }) + ) + const scan = withOpenCodeSqliteScanScope(() => + client.list({ + dbPaths: ['/scan.db'], + limit: 1, + issues + }) + ) + await vi.advanceTimersByTimeAsync(25_000) + workers[0].emit('message', { + id: workers[0].lastId(), + ok: true, + value: { candidates: [], issues: [] } + }) + await vi.advanceTimersByTimeAsync(20_000) + await expect(scan).resolves.toEqual([]) + expect(issues).toEqual([ + expect.objectContaining({ + kind: 'scope', + message: expect.stringContaining('45s work budget') + }) + ]) + expect(workers[0].postedRequests).toHaveLength(2) + expect(workers[0].terminated).toBe(false) + workers[0].emit('message', { + id: workers[0].lastId(), + ok: true, + value: { candidates: [], issues: [] } + }) + await expect(Promise.all(ordinary)).resolves.toEqual([[], []]) + const next = withOpenCodeSqliteScanScope(() => + client.list({ dbPaths: ['/next.db'], limit: 1, issues: [] }) + ) + workers[0].emit('message', { + id: workers[0].lastId(), + ok: true, + value: { candidates: [], issues: [] } + }) + await expect(next).resolves.toEqual([]) + } finally { + client.dispose() + vi.useRealTimers() + } + }) + it('correlates responses by id and ignores stale ids', async () => { const workers: FakeWorker[] = [] const client = new OpenCodeSqliteWorkerClient({ workerFactory: makeFactory(workers), log() {} }) @@ -188,7 +246,7 @@ describe('OpenCodeSqliteWorkerClient', () => { client.list({ dbPaths: ['/tmp/opencode.db'], limit: 10, issues: listIssues }) ).resolves.toEqual([]) expect( - listIssues.some((issue) => /background scanner could not start/.test(issue.message)) + listIssues.some((issue) => issue.message.includes('background scanner could not start')) ).toBe(true) await expect( client.parse({ dbPath: '/tmp/opencode.db', sessionId: 'ses_skipped', platform: 'darwin' }) @@ -264,7 +322,7 @@ describe('OpenCodeSqliteWorkerClient', () => { const first = await client.list({ dbPaths: ['/db'], limit: 10, issues: firstIssues }) expect(first).toEqual([]) expect( - firstIssues.some((issue) => /background scanner could not start/.test(issue.message)) + firstIssues.some((issue) => issue.message.includes('background scanner could not start')) ).toBe(true) await expect( client.parse({ dbPath: '/db', sessionId: 'ses_heal', platform: 'darwin' }) diff --git a/src/main/ai-vault/session-scanner-opencode-sqlite-worker-client.ts b/src/main/ai-vault/session-scanner-opencode-sqlite-worker-client.ts index f303de52443..c6a85cb58b0 100644 --- a/src/main/ai-vault/session-scanner-opencode-sqlite-worker-client.ts +++ b/src/main/ai-vault/session-scanner-opencode-sqlite-worker-client.ts @@ -12,6 +12,7 @@ import type { import { parseOpenCodeSqliteCaptureValue } from './session-scanner-opencode-sqlite-worker-response' import type { SessionFileCandidate } from './session-scanner-types' import { errorMessage } from './session-scanner-values' +import { runOpenCodeSqliteScanRequest } from './session-scanner-opencode-sqlite-scan-scope' // Why (#8864): a lazily-spawned, unref'd worker runs OpenCode SQLite reads off // the main-process event loop. This module owns only the OpenCode legs; the @@ -117,7 +118,8 @@ export class OpenCodeSqliteWorkerClient { ...(args.agent ? { agent: args.agent } : {}) }), LIST_TIMEOUT_MS, - args.signal + args.signal, + args.agent )) as OpenCodeSqliteListValue args.issues.push(...value.issues) return value.candidates @@ -178,7 +180,8 @@ export class OpenCodeSqliteWorkerClient { ...(args.agent ? { agent: args.agent } : {}) }), PARSE_TIMEOUT_MS, - args.signal + args.signal, + args.agent ) // oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: the worker's parse leg returns exactly this, built by the repo's own reader on the other side of a structured clone. return value as AiVaultSession | null @@ -217,7 +220,8 @@ export class OpenCodeSqliteWorkerClient { ...(args.agent ? { agent: args.agent } : {}) }), CAPTURE_TIMEOUT_MS, - args.signal + args.signal, + args.agent ) return parseOpenCodeSqliteCaptureValue(value) } catch (err) { @@ -229,7 +233,12 @@ export class OpenCodeSqliteWorkerClient { args: Omit, signal?: AbortSignal ): Promise { - const value = await this.dispatch((id) => ({ ...args, id }), PARSE_TIMEOUT_MS, signal) + const value = await this.dispatch( + (id) => ({ ...args, id }), + PARSE_TIMEOUT_MS, + signal, + 'native-chat' + ) // oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: Only this build's internal worker dispatch constructs page/signal results; they are not client-supplied paths or frames. return value as OpenCodeNativeChatReadValue } @@ -241,13 +250,20 @@ export class OpenCodeSqliteWorkerClient { private async dispatch( buildRequest: (id: number) => OpenCodeSqliteWorkerRequest, timeoutMs: number, - signal?: AbortSignal + signal?: AbortSignal, + agent?: 'opencode2' | 'zcode' | 'native-chat' ): Promise { const deadline = this.requestTimeoutMs ?? timeoutMs - const response = await this.requests.dispatch( - (id) => ({ ...buildRequest(id), timeoutMs: deadline }), - deadline, - signal + const response = await runOpenCodeSqliteScanRequest( + signal, + (requestSignal, owner) => + this.requests.dispatch( + (id) => ({ ...buildRequest(id), timeoutMs: deadline }), + deadline, + requestSignal, + owner + ), + agent ) if (!response.ok) { throw new Error(response.error) diff --git a/src/main/ai-vault/session-scanner-opencode-sqlite-worker-spawn.ts b/src/main/ai-vault/session-scanner-opencode-sqlite-worker-spawn.ts index 13b9cd3cd5e..e89a6361b1e 100644 --- a/src/main/ai-vault/session-scanner-opencode-sqlite-worker-spawn.ts +++ b/src/main/ai-vault/session-scanner-opencode-sqlite-worker-spawn.ts @@ -16,6 +16,7 @@ import { openCodeWslPath } from './session-scanner-opencode-wsl-client' import { findForeignSqliteReaderEntry } from '../foreign-sqlite-readers/foreign-sqlite-reader-entry-path' +import { runOpenCodeSqliteScanRequest } from './session-scanner-opencode-sqlite-scan-scope' // Why: resolve the built worker entry + own the process-wide shared client so // the client class stays free of Electron (require'd lazily here) and the @@ -179,8 +180,14 @@ async function listForHost( const first = paths.values().next().value! const issues: AiVaultScanIssue[] = [] try { - const client = await openCodeWslClient(distro, first, args.signal) - const result = await client.list({ ...args, dbPaths: [...paths.keys()], issues }) + const result = await runOpenCodeSqliteScanRequest( + args.signal, + async (signal) => { + const client = await openCodeWslClient(distro, first, signal) + return client.list({ ...args, signal, dbPaths: [...paths.keys()], issues }) + }, + args.agent + ) return result.flatMap((candidate) => { const parsed = splitOpenCodeSqliteCandidate(candidate.file.path, args.agent) const original = parsed && paths.get(parsed.dbPath) @@ -223,8 +230,14 @@ async function parseForHost( if (!wsl) { return getSharedClient().parse(args) } - const client = await openCodeWslClient(wsl.distro, args.dbPath, args.signal) - const session = await client.parse({ ...args, dbPath: wsl.linuxPath, platform: 'linux' }) + const session = await runOpenCodeSqliteScanRequest( + args.signal, + async (signal) => { + const client = await openCodeWslClient(wsl.distro, args.dbPath, signal) + return client.parse({ ...args, signal, dbPath: wsl.linuxPath, platform: 'linux' }) + }, + args.agent + ) return mapOpenCodeWslSession(session, args.dbPath) } @@ -235,8 +248,14 @@ async function captureForHost( if (!wsl) { return getSharedClient().capture(args) } - const client = await openCodeWslClient(wsl.distro, args.dbPath, args.signal) - const capture = await client.capture({ ...args, dbPath: wsl.linuxPath, platform: 'linux' }) + const capture = await runOpenCodeSqliteScanRequest( + args.signal, + async (signal) => { + const client = await openCodeWslClient(wsl.distro, args.dbPath, signal) + return client.capture({ ...args, signal, dbPath: wsl.linuxPath, platform: 'linux' }) + }, + args.agent + ) return { ...capture, session: mapOpenCodeWslSession(capture.session, args.dbPath) } } diff --git a/src/main/ai-vault/session-scanner-opencode-wsl-routing.test.ts b/src/main/ai-vault/session-scanner-opencode-wsl-routing.test.ts index 754211c323c..9db4ba85e83 100644 --- a/src/main/ai-vault/session-scanner-opencode-wsl-routing.test.ts +++ b/src/main/ai-vault/session-scanner-opencode-wsl-routing.test.ts @@ -3,6 +3,7 @@ import type { AiVaultScanIssue } from '../../shared/ai-vault-types' import { createAccumulator, finalizeSession } from './session-scanner-accumulator' import type { SessionFileCandidate } from './session-scanner-types' import type * as wslClientModule from './session-scanner-opencode-wsl-client' +import { withOpenCodeSqliteScanScope } from './session-scanner-opencode-sqlite-scan-scope' const mocks = vi.hoisted(() => ({ native: { @@ -59,9 +60,53 @@ beforeEach(() => { vi.clearAllMocks() vi.spyOn(process, 'platform', 'get').mockReturnValue('win32') }) -afterEach(() => vi.restoreAllMocks()) +afterEach(() => { + vi.useRealTimers() + vi.restoreAllMocks() +}) describe('OpenCode SQLite execution-host routes', () => { + it('budgets WSL preparation while retaining a native source that already answered', async () => { + vi.useFakeTimers() + mocks.native.list.mockResolvedValueOnce([row(native)]) + mocks.guest.mockImplementationOnce((_distro, _path, signal: AbortSignal | undefined) => { + if (!signal) { + throw new Error('Missing scoped preparation signal') + } + return new Promise((_resolve, reject) => + signal.addEventListener('abort', () => reject(signal.reason), { once: true }) + ) + }) + const issues: AiVaultScanIssue[] = [] + const result = withOpenCodeSqliteScanScope(() => + listOpenCodeSqliteSessionsViaWorker({ + dbPaths: [native, ubuntu], + limit: 2, + issues + }) + ) + await vi.advanceTimersByTimeAsync(45_000) + expect((await result).map((entry) => entry.file.path)).toEqual([`${native}#same-session`]) + expect(issues).toEqual([ + expect.objectContaining({ + path: ubuntu, + kind: 'scope', + message: expect.stringContaining('45s work budget') + }) + ]) + mocks.guest.mockResolvedValueOnce({ list: vi.fn(async () => [row(guest)]) }) + const recoveredIssues: AiVaultScanIssue[] = [] + const recovered = await withOpenCodeSqliteScanScope(() => + listOpenCodeSqliteSessionsViaWorker({ + dbPaths: [ubuntu], + limit: 2, + issues: recoveredIssues + }) + ) + expect(recovered).toHaveLength(1) + expect(recoveredIssues).toEqual([]) + }) + it('separates native and distro databases and preserves equal IDs in different distros', async () => { const list = vi.fn(async (args) => [row(args.dbPaths[0])]) mocks.guest.mockResolvedValue({ list }) diff --git a/src/main/ai-vault/session-scanner.ts b/src/main/ai-vault/session-scanner.ts index 8716440a565..f0525c918c8 100644 --- a/src/main/ai-vault/session-scanner.ts +++ b/src/main/ai-vault/session-scanner.ts @@ -45,6 +45,7 @@ import { clampPositiveInteger, errorMessage } from './session-scanner-values' import { throwIfAiVaultScanCancelled } from './ai-vault-scan-cancellation' import { DEFAULT_AI_VAULT_SCAN_LIMIT } from '../../shared/ai-vault-session-depth' import { withDevinSessionsDbScan } from './session-scanner-devin-db' +import { withOpenCodeSqliteScanScope } from './session-scanner-opencode-sqlite-scan-scope' const SESSION_PARSE_CONCURRENCY = 8 const SESSION_PARSE_CANDIDATE_MULTIPLIER = 2 @@ -61,6 +62,10 @@ const SESSION_PARSE_CANDIDATE_MULTIPLIER = 2 export async function scanAiVaultSessions( options: AiVaultScanOptions = {} ): Promise { + return withOpenCodeSqliteScanScope(() => scanAiVaultSessionStores(options)) +} + +async function scanAiVaultSessionStores(options: AiVaultScanOptions): Promise { // The span makes scan cost visible in the local trace file: STA-1278-style // "one core pegged" reports need to show whether transcript scanning is the // subsystem burning CPU, and how much of each scan the cache absorbed. diff --git a/src/main/worker-thread-request-queue.test.ts b/src/main/worker-thread-request-queue.test.ts index 4bf2188dc54..2046267396c 100644 --- a/src/main/worker-thread-request-queue.test.ts +++ b/src/main/worker-thread-request-queue.test.ts @@ -92,9 +92,10 @@ function makeQueue( function send( queue: WorkerThreadRequestQueue, - label: string + label: string, + owner?: { readonly signal: AbortSignal } ): Promise { - return queue.dispatch((id) => ({ id, label }), TIMEOUT_MS) + return queue.dispatch((id) => ({ id, label }), TIMEOUT_MS, undefined, owner) } /** Resolve to the response or to the rejection, so a test can assert on either. */ @@ -328,6 +329,161 @@ describe('WorkerThreadRequestQueue', () => { expect(await behind).toMatchObject({ label: 'd' }) }) + it('keeps one owner fault budget across idle batches and refuses a fourth worker', async () => { + const workers: FakeWorker[] = [] + const queue = makeQueue(workers) + const owner = { signal: new AbortController().signal } + try { + for (let fault = 0; fault < 3; fault++) { + const call = settle(send(queue, `owned-${fault}`, owner)) + workers.at(-1)?.emit('error', new Error(`fault-${fault}`)) + expect(await call).toMatchObject({ message: `fault-${fault}` }) + } + const refused = settle(send(queue, 'fourth', owner)) + expect(workers).toHaveLength(3) + await expect(refused).resolves.toMatchObject({ message: 'crashed repeatedly (fault-2)' }) + const nextScan = send(queue, 'new-scan', { signal: new AbortController().signal }) + expect(workers).toHaveLength(4) + workers[3].respond() + await expect(nextScan).resolves.toMatchObject({ label: 'new-scan' }) + } finally { + queue.dispose() + } + }) + + it('resets only the successful owner and does not count idle worker exits', async () => { + const workers: FakeWorker[] = [] + const queue = makeQueue(workers) + const a = { signal: new AbortController().signal } + const b = { signal: new AbortController().signal } + try { + const initial = send(queue, 'initial', a) + workers[0].respond() + await initial + workers[0].emit('exit', 17) + for (let fault = 0; fault < 2; fault++) { + const call = settle(send(queue, `a-${fault}`, a)) + workers.at(-1)?.emit('error', new Error(`a-fault-${fault}`)) + await call + } + const peer = send(queue, 'b-success', b) + workers.at(-1)?.respond() + await peer + const third = settle(send(queue, 'a-third', a)) + workers.at(-1)?.emit('error', new Error('a-third-fault')) + await third + const refused = settle(send(queue, 'a-fourth', a)) + expect(workers).toHaveLength(4) + await expect(refused).resolves.toMatchObject({ + message: 'crashed repeatedly (a-third-fault)' + }) + const healthy = send(queue, 'b-still-healthy', b) + workers.at(-1)?.respond() + await expect(healthy).resolves.toMatchObject({ label: 'b-still-healthy' }) + } finally { + queue.dispose() + } + }) + + it('clears a successful owner budget while preserving another owner failure count', async () => { + const workers: FakeWorker[] = [] + const queue = makeQueue(workers) + const a = { signal: new AbortController().signal } + const b = { signal: new AbortController().signal } + try { + for (const owner of [a, b]) { + for (let fault = 0; fault < 2; fault++) { + const call = settle(send(queue, 'failure', owner)) + workers.at(-1)?.emit('error', new Error('failure')) + await call + } + } + const successful = send(queue, 'a-success', a) + workers.at(-1)?.respond() + await successful + const thirdB = settle(send(queue, 'b-third', b)) + workers.at(-1)?.emit('error', new Error('b-third-fault')) + await thirdB + for (let fault = 0; fault < 2; fault++) { + const call = settle(send(queue, 'a-new-failure', a)) + workers.at(-1)?.emit('error', new Error('a-new-failure')) + await call + } + const stillAllowed = send(queue, 'a-allowed', a) + expect(workers).toHaveLength(8) + workers.at(-1)?.respond() + await expect(stillAllowed).resolves.toMatchObject({ label: 'a-allowed' }) + await expect(settle(send(queue, 'b-refused', b))).resolves.toMatchObject({ + message: 'crashed repeatedly (b-third-fault)' + }) + expect(workers).toHaveLength(8) + } finally { + queue.dispose() + } + }) + + it('drains only the failing owner while queued peers keep their FIFO order', async () => { + const workers: FakeWorker[] = [] + const queue = makeQueue(workers) + const a = { signal: new AbortController().signal } + const b = { signal: new AbortController().signal } + try { + const failed = ['a1', 'a2', 'a3', 'a4'].map((label) => settle(send(queue, label, a))) + const peer = settle(send(queue, 'peer', b)) + const ordinary = settle(send(queue, 'ordinary')) + for (let fault = 0; fault < 3; fault++) { + workers.at(-1)?.emit('error', new Error(`fault-${fault}`)) + } + expect(workers).toHaveLength(4) + expect(labels(workers[3])).toEqual(['peer']) + expect((await Promise.all(failed)).at(-1)).toMatchObject({ + message: 'crashed repeatedly (fault-2)' + }) + workers[3].respond() + await expect(peer).resolves.toMatchObject({ label: 'peer' }) + expect(labels(workers[3])).toEqual(['peer', 'ordinary']) + workers[3].respond() + await expect(ordinary).resolves.toMatchObject({ label: 'ordinary' }) + } finally { + queue.dispose() + } + }) + + it('keeps retirement refusals out of the owner failure budget', async () => { + vi.useFakeTimers() + const workers: FakeWorker[] = [] + let finish: (code: number) => void = () => {} + const first = new FakeWorker() + first.exit = new Promise((resolve) => { + finish = resolve + }) + const queue = makeQueue(workers, { + awaitRetirement: true, + makeWorker: () => (workers.length === 0 ? first : new FakeWorker()) + }) + const owner = { signal: new AbortController().signal } + try { + const failed = settle(send(queue, 'first', owner)) + first.emit('error', new Error('first-fault')) + await failed + for (let attempt = 0; attempt < 4; attempt++) { + await expect(settle(send(queue, 'not-executed', owner))).resolves.toMatchObject({ + message: 'unavailable: previous worker still exiting' + }) + } + expect(workers).toHaveLength(1) + finish(1) + await vi.advanceTimersByTimeAsync(0) + const recovered = send(queue, 'recovered', owner) + expect(workers).toHaveLength(2) + workers[1].respond() + await expect(recovered).resolves.toMatchObject({ label: 'recovered' }) + } finally { + finish(1) + queue.dispose() + } + }) + describe('awaitRetirement', () => { function stalledExit(): { worker: FakeWorker; finish: (code: number) => void } { const worker = new FakeWorker() diff --git a/src/main/worker-thread-request-queue.ts b/src/main/worker-thread-request-queue.ts index 39561fa80a4..6793d597e27 100644 --- a/src/main/worker-thread-request-queue.ts +++ b/src/main/worker-thread-request-queue.ts @@ -42,6 +42,10 @@ export type WorkerThreadRequestQueueOptions = { awaitRetirement?: boolean } +export type WorkerThreadRequestOwner = { readonly signal: AbortSignal } + +type OwnerFailures = { consecutiveDeaths: number; refused: Error | null } + type PendingCall = { request: TRequest timeoutMs: number @@ -49,6 +53,7 @@ type PendingCall = { reject: (error: Error) => void timer: NodeJS.Timeout | null signal?: AbortSignal + owner?: WorkerThreadRequestOwner cleanupAbort: () => void } @@ -59,6 +64,7 @@ export class WorkerThreadRequestQueue< private active: PendingCall | null = null private queue: PendingCall[] = [] private consecutiveDeaths = 0 + private readonly ownerFailures = new WeakMap() private nextId = 1 private disposed = false private readonly host: LazyWorkerThreadHost @@ -85,7 +91,8 @@ export class WorkerThreadRequestQueue< dispatch( buildRequest: (id: number) => TRequest, timeoutMs: number, - signal?: AbortSignal + signal?: AbortSignal, + owner?: WorkerThreadRequestOwner ): Promise { return new Promise((resolve, reject) => { if (this.disposed) { @@ -96,6 +103,11 @@ export class WorkerThreadRequestQueue< reject(signal.reason ?? new Error('Worker request aborted')) return } + const refused = owner && this.ownerFailures.get(owner)?.refused + if (refused) { + reject(refused) + return + } // Built before the cap check so a rejection can name the dropped work; // the id it burns is only a correlation token, so a gap costs nothing. const request = buildRequest(this.nextId++) @@ -106,7 +118,7 @@ export class WorkerThreadRequestQueue< } // A fresh burst from full idle starts new work: clear any death count // carried from a prior burst so the respawn cap can't drain it early. - if (!this.active && this.queue.length === 0) { + if (!owner && !this.active && this.queue.length === 0) { this.consecutiveDeaths = 0 } const call: PendingCall = { @@ -116,6 +128,7 @@ export class WorkerThreadRequestQueue< reject, timer: null, signal, + owner, cleanupAbort: () => signal?.removeEventListener('abort', abort) } const abort = (): void => { @@ -201,7 +214,11 @@ export class WorkerThreadRequestQueue< this.armDeadline(call) return } - this.consecutiveDeaths = 0 + if (call.owner) { + this.failuresFor(call.owner).consecutiveDeaths = 0 + } else { + this.consecutiveDeaths = 0 + } this.settle(call, () => call.resolve(response)) this.afterSettle() } @@ -226,12 +243,16 @@ export class WorkerThreadRequestQueue< private onWorkerFault(error: Error): void { const failed = this.active this.host.destroy() - this.consecutiveDeaths++ - if (failed) { - this.settle(failed, () => failed.reject(error)) + if (!failed) { + this.pump() + return } - if (this.consecutiveDeaths >= this.options.maxConsecutiveDeaths) { - this.drainQueueAfterCrashLoop(error) + const deaths = failed.owner + ? ++this.failuresFor(failed.owner).consecutiveDeaths + : ++this.consecutiveDeaths + this.settle(failed, () => failed.reject(error)) + if (deaths >= this.options.maxConsecutiveDeaths) { + this.drainQueueAfterCrashLoop(error, failed.owner) return } if (this.queue.length > 0) { @@ -239,14 +260,28 @@ export class WorkerThreadRequestQueue< } } - private drainQueueAfterCrashLoop(error: Error): void { - const pending = this.queue - this.queue = [] - this.consecutiveDeaths = 0 + private drainQueueAfterCrashLoop(error: Error, owner?: WorkerThreadRequestOwner): void { + const pending = this.queue.filter((call) => call.owner === owner) + this.queue = this.queue.filter((call) => call.owner !== owner) const drainError = new Error(this.options.describeCrashLoop(error.message)) + if (owner) { + this.failuresFor(owner).refused = drainError + } else { + this.consecutiveDeaths = 0 + } for (const call of pending) { this.settle(call, () => call.reject(drainError)) } + this.afterSettle() + } + + private failuresFor(owner: WorkerThreadRequestOwner): OwnerFailures { + let failures = this.ownerFailures.get(owner) + if (!failures) { + failures = { consecutiveDeaths: 0, refused: null } + this.ownerFailures.set(owner, failures) + } + return failures } private failQueuedAsUnavailable(): void { From e42768fb2340d71f0d6d7e99964f13934a5252f1 Mon Sep 17 00:00:00 2001 From: Brennan Benson <79079362+brennanb2025@users.noreply.github.com> Date: Sun, 4 Oct 2026 14:32:12 -0700 Subject: [PATCH 4/8] Update in-app Android APK links to mobile 0.0.52 (#25168) --- src/renderer/src/components/mobile/mobile-platform-copy.ts | 2 +- src/renderer/src/components/settings/MobileSettingsPane.tsx | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/renderer/src/components/mobile/mobile-platform-copy.ts b/src/renderer/src/components/mobile/mobile-platform-copy.ts index f33f9a42196..45149f90858 100644 --- a/src/renderer/src/components/mobile/mobile-platform-copy.ts +++ b/src/renderer/src/components/mobile/mobile-platform-copy.ts @@ -22,7 +22,7 @@ const IOS_CHANNEL_COPY: Record = { const ANDROID_COPY: InstallCopy = { ctaLabel: 'Download APK', - url: 'https://github.com/stablyai/orca/releases/download/mobile-android-v0.0.48/app-release.apk' + url: 'https://github.com/stablyai/orca/releases/download/mobile-android-v0.0.52/app-release.apk' } export function getInstallCopy(platform: Platform, iosChannel: IosChannel): InstallCopy { diff --git a/src/renderer/src/components/settings/MobileSettingsPane.tsx b/src/renderer/src/components/settings/MobileSettingsPane.tsx index 2dd10a006f4..203e95abbae 100644 --- a/src/renderer/src/components/settings/MobileSettingsPane.tsx +++ b/src/renderer/src/components/settings/MobileSettingsPane.tsx @@ -13,7 +13,7 @@ export { getMobileSettingsPaneSearchEntries } const ORCA_IOS_APP_STORE_URL = 'https://apps.apple.com/app/orca-ide/id6766130217' const ORCA_ANDROID_APK_URL = - 'https://github.com/stablyai/orca/releases/download/mobile-android-v0.0.48/app-release.apk' + 'https://github.com/stablyai/orca/releases/download/mobile-android-v0.0.52/app-release.apk' export function MobileSettingsPane(): React.JSX.Element { const showMobileButton = useAppStore((s) => s.settings?.showMobileButton !== false) From 0971479866f2dc625b5c32dbc260ca568d1460dc Mon Sep 17 00:00:00 2001 From: Jinjing <6427696+AmethystLiang@users.noreply.github.com> Date: Sun, 4 Oct 2026 14:35:16 -0700 Subject: [PATCH 5/8] Add shared workspace settings note and prevent filter modal expansion (#25300) * fix(mobile): say that workspace sort, grouping, and filters are shared The Manual sort option was subtitled 'Server order', but it orders by the desktop's drag ranks. Sort, grouping, and filters on the phone all write the host's shared view settings, so changing them also changes every other device on that host, which the screen never said. Relabel Manual as 'Desktop drag order' and add 'Shared with other devices on this host' under the Sort By, Group By, and Filter titles. The note avoids naming a desktop sidebar because headless hosts have none. * fix(mobile): prevent filter modal heading expansion Add flexShrink: 1 to allow the heading container to shrink when space is constrained. Update comment to clarify why workspace view is shared across devices. * update wording --- mobile/src/components/PickerModal.tsx | 8 ++++++++ mobile/src/host-screen/host-screen-overlays.tsx | 10 ++++++++-- mobile/src/host-screen/host-screen-secondary-styles.ts | 8 ++++++++ mobile/src/worktree/workspace-list-picker-options.ts | 5 ++++- mobile/src/worktree/workspace-view-settings.ts | 2 +- 5 files changed, 29 insertions(+), 4 deletions(-) diff --git a/mobile/src/components/PickerModal.tsx b/mobile/src/components/PickerModal.tsx index 9456307091f..45b6593c09f 100644 --- a/mobile/src/components/PickerModal.tsx +++ b/mobile/src/components/PickerModal.tsx @@ -15,6 +15,7 @@ export type PickerOption = { type Props = { visible: boolean title: string + subtitle?: string options: PickerOption[] selected: T onSelect: (value: T) => void @@ -32,6 +33,7 @@ type PickerModalContentProps = Pick< export function PickerModal({ visible, title, + subtitle, options, selected, onSelect, @@ -44,6 +46,7 @@ export function PickerModal({ {title} + {subtitle ? {subtitle} : null} state.setShowFilterModal(false)}> - Filter + + Filter + {WORKSPACE_VIEW_SHARED_NOTE} + {settings.activeFilterCount > 0 && ( Clear filters diff --git a/mobile/src/host-screen/host-screen-secondary-styles.ts b/mobile/src/host-screen/host-screen-secondary-styles.ts index af14aa2b97a..7f6c4f3f65c 100644 --- a/mobile/src/host-screen/host-screen-secondary-styles.ts +++ b/mobile/src/host-screen/host-screen-secondary-styles.ts @@ -47,11 +47,19 @@ export const hostScreenSecondaryStyles = StyleSheet.create({ paddingHorizontal: spacing.xs, marginBottom: spacing.md }, + filterModalHeading: { + flexShrink: 1 + }, filterModalTitle: { fontSize: 15, fontWeight: '600', color: colors.textPrimary }, + filterModalSubtitle: { + fontSize: 11, + color: colors.textMuted, + marginTop: 2 + }, clearFiltersText: { fontSize: 13, color: colors.textSecondary diff --git a/mobile/src/worktree/workspace-list-picker-options.ts b/mobile/src/worktree/workspace-list-picker-options.ts index 180d4038320..ca597ed38cc 100644 --- a/mobile/src/worktree/workspace-list-picker-options.ts +++ b/mobile/src/worktree/workspace-list-picker-options.ts @@ -1,6 +1,9 @@ import type { PickerOption } from '../components/PickerModal' import type { MobileGroupMode, MobileSortMode } from './workspace-view-settings' +// Why: the host may be headless, so the note can't promise a desktop sidebar. +export const WORKSPACE_VIEW_SHARED_NOTE = 'Synced across your devices' + export const WORKSPACE_SORT_OPTIONS: PickerOption[] = [ // Why: desktop and persisted state keep the `smart` key, while mobile shows the product label. { @@ -11,7 +14,7 @@ export const WORKSPACE_SORT_OPTIONS: PickerOption[] = [ { value: 'name', label: 'Name', subtitle: 'Alphabetical by name' }, { value: 'recent', label: 'Recent', subtitle: 'Most recent output first' }, { value: 'repo', label: 'Repo', subtitle: 'Repository, then workspace name' }, - { value: 'manual', label: 'Manual', subtitle: 'Server order' } + { value: 'manual', label: 'Manual', subtitle: 'Desktop drag order' } ] export const WORKSPACE_GROUP_OPTIONS: PickerOption[] = [ diff --git a/mobile/src/worktree/workspace-view-settings.ts b/mobile/src/worktree/workspace-view-settings.ts index f1d17189adc..221fa7211be 100644 --- a/mobile/src/worktree/workspace-view-settings.ts +++ b/mobile/src/worktree/workspace-view-settings.ts @@ -7,7 +7,7 @@ import type { WorkspaceStatusDefinition } from '../../../src/shared/worktree/typ import { coerceMobileWorkspaceStatuses } from './mobile-workspace-statuses' export type MobileGroupMode = 'none' | 'workspaceStatus' | 'repo' | 'prStatus' -// Desktop sort adds 'manual'; mobile renders it but sorts by server order. +// Desktop sort adds 'manual'; mobile orders it by the desktop's drag ranks. export type MobileSortMode = 'smart' | 'name' | 'recent' | 'repo' | 'manual' // Desktop PersistedUIState fields this screen syncs (a structural subset). From b1e12d7bb41db6c8de03951d8323cfff4a3e1329 Mon Sep 17 00:00:00 2001 From: Brennan Benson <79079362+brennanb2025@users.noreply.github.com> Date: Sun, 4 Oct 2026 14:37:40 -0700 Subject: [PATCH 6/8] fix(native-chat): a new structured chat's model list comes from the machine that runs it (#25143) * fix(native-chat): a new structured chat's model list comes from the host that runs it agentSession.modelCatalog builds the structured-session host like agentSession.options, so a host with no saved chats since it started answers instead of refusing. When the host answers unknown because its first listing runs in the background, the picker re-reads on a bounded schedule until that listing lands. Local terminal-backed chat skips the structured catalog when a custom launch command is configured. * fix(native-chat): wait on the host's first model listing instead of re-reading on a timer A new structured chat's picker read the host catalog once; on an account the host had never listed, the answer was "unknown" while a background listing ran, and the client re-read on a 1-30 s schedule. Replace the schedule with the host's own completion signal: - The host answers a cold read with `listingInProgress: true` (new optional field) once it has started or joined that listing. A read that passes the new optional `waitForListing` param awaits the same joined listing and answers with it, or a plain "unknown" if it failed. The at-rest options read never waits. A host that predates the field never sends it, so the client never sends the param to a host that would refuse it. - The picker reads once per open and attach; after the host's report it sends one waiting read, with a 90 s client timeout above the slowest listing. While that read is out, the model pill keeps its label but cannot open or be set (typed /model included). An answer, failure, timeout, hide, attach or the provider's own list releases it. - `agentSession.modelCatalog` builds the structured-session host only for a read that names a session (a structured chat). Terminal-backed chat's session-less read keeps the non-building gate, so a desktop that never runs structured chat never opens the session journal. * fix(native-chat): one waiting model-list read per chat, and no late menu open The waiting catalog read was owned by one run of the picker's effect. Attach (a new fence), hide/show or a send re-ran the effect: the cleanup released the model picker onto the built-in list for a round trip, and the new run sent a second waiting read while the first, which cannot be withdrawn, kept a remote call slot until the listing ended. The waiting read now belongs to the chat (runtime target + agent + session): a small registry keeps one in flight per chat, every re-run or remount joins it, and the entry is deleted when the read settles. The picker hold is derived from that entry being in flight, so it lasts across attach and hide/show and ends when the read settles, the provider reports its own list, or the pane switches to another session. Answers still pass the stale and record checks. A bare /model typed while the list loads no longer opens the model menu by itself when the list lands: the menu stays keyed on the request, and only its initial open is suppressed while pending, so the request is spent shut and the end of the pending period never remounts it. * fix(native-chat): release the model picker in the same commit as the host list When the waiting catalog read settled in the chat that started it, the registry dropped its entry and told subscribers first, and the host list was applied a few microtasks later. React committed once with the picker enabled on the built-in list, then again with the host's list. Joiners now hand the registry their apply callback, and the registry runs every joiner (with the answer, or nothing when the read failed or timed out) before it deletes the entry and notifies. The release and the list land in one commit. An effect cleanup leaves the wait instead of flagging itself stale. * fix(runtime): queue model catalog reads in the long-wait lane A model catalog read that waits on a host's first listing replies only when that listing ends, yet it took one of the 8 foreground call slots for its server. Enough chats opened during one cold listing would stall that server's sends and interrupts until a wait settled. agentSession.modelCatalog now joins worktree.rm in the long-wait lane: same concurrency, counted apart from the foreground calls. The queue classifies by method only, and a warm catalog read answers at once, so the whole method moves. --- .../agent-model-catalog-service.test.ts | 105 +++++- .../agent-model-catalog-service.ts | 32 +- ...uctured-agent-session-options-read.test.ts | 48 +++ ...uctured-agent-session-options-read.test.ts | 62 +++ .../structured-agent-session-options-read.ts | 10 +- ...SessionOptionPickers.menu-request.test.tsx | 109 ++++++ .../NativeChatSessionOptionPickers.test.tsx | 38 ++ .../NativeChatSessionOptionPickers.tsx | 7 +- .../native-chat/host-model-listing-waits.ts | 62 +++ .../native-chat-session-option-discovery.ts | 8 +- ...ive-chat-session-option-enrichment.test.ts | 18 + .../use-host-model-catalog-upgrade.test.tsx | 357 ++++++++++++++++++ .../use-host-model-catalog-upgrade.ts | 74 +++- .../use-structured-agent-session-options.ts | 13 +- .../structured-agent-session-client.test.ts | 24 ++ .../structured-agent-session-client.ts | 14 +- src/shared/agent-session-wire.ts | 7 +- src/shared/native-chat-session-options.ts | 2 + .../structured-agent-session-params.ts | 7 +- src/shared/runtime-rpc-call-queue.test.ts | 20 + src/shared/runtime-rpc-call-queue.ts | 7 +- .../structured-agent-session-options.ts | 11 + 22 files changed, 989 insertions(+), 46 deletions(-) create mode 100644 src/main/native-chat/agent-session-wire/structured-agent-session-options-read.test.ts create mode 100644 src/renderer/src/components/native-chat/NativeChatSessionOptionPickers.menu-request.test.tsx create mode 100644 src/renderer/src/components/native-chat/host-model-listing-waits.ts create mode 100644 src/renderer/src/components/native-chat/use-host-model-catalog-upgrade.test.tsx diff --git a/src/main/native-chat/agent-model-catalog/agent-model-catalog-service.test.ts b/src/main/native-chat/agent-model-catalog/agent-model-catalog-service.test.ts index d51a98b3d8b..02025485151 100644 --- a/src/main/native-chat/agent-model-catalog/agent-model-catalog-service.test.ts +++ b/src/main/native-chat/agent-model-catalog/agent-model-catalog-service.test.ts @@ -5,7 +5,11 @@ import { agentModelCatalogFingerprintForRecord } from './agent-model-catalog-fingerprint' import { createAgentModelCatalogService } from './agent-model-catalog-service' -import { AgentModelCatalogStore, type AgentModelCatalogSuccess } from './agent-model-catalog-store' +import { + AGENT_MODEL_CATALOG_FRESH_MS, + AgentModelCatalogStore, + type AgentModelCatalogSuccess +} from './agent-model-catalog-store' function record(accountHomePath: string): AgentSessionRecord { // oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: the service reads only provider, accountHome and location; the rest of the record is irrelevant here. @@ -55,7 +59,8 @@ describe('agent model catalog service', () => { probes: { codex: probe } }) expect(await service.read({ agent: 'codex', sessionId: 'session-1' })).toEqual({ - origin: 'unknown' + origin: 'unknown', + listingInProgress: true }) // A second read while the probe is in flight must not start another, and a // record-scoped read probes the RECORD's pinned home, not the selection. @@ -81,7 +86,10 @@ describe('agent model catalog service', () => { probes: { codex: probe } }) // The record-less read follows the CURRENT selection: unknown, never gpt-old. - expect(await service.read({ agent: 'codex' })).toEqual({ origin: 'unknown' }) + expect(await service.read({ agent: 'codex' })).toEqual({ + origin: 'unknown', + listingInProgress: true + }) expect(probe).toHaveBeenCalledWith('/homes/new') await vi.waitFor(async () => { const result = await service.read({ agent: 'codex' }) @@ -139,7 +147,8 @@ describe('agent model catalog service', () => { probes: { codex: probe } }) expect(await service.read({ agent: 'codex', sessionId: 'session-1' })).toEqual({ - origin: 'unknown' + origin: 'unknown', + listingInProgress: true }) await vi.waitFor(() => expect(probe).toHaveBeenCalledTimes(1)) // Still a clean unknown — and the failure TTL suppresses a probe storm. @@ -164,6 +173,94 @@ describe('agent model catalog service', () => { expect(probe).not.toHaveBeenCalled() }) + describe('a read that waits for the first listing', () => { + function deferredListing() { + let resolve!: (success: AgentModelCatalogSuccess) => void + let reject!: (error: Error) => void + const promise = new Promise((res, rej) => { + resolve = res + reject = rej + }) + return { promise, resolve, reject } + } + + function coldService(probe: (home: string) => Promise) { + const store = new AgentModelCatalogStore() + const service = createAgentModelCatalogService({ + store, + getRecord: () => undefined, + resolveAccountHome: async () => CODEX_HOME('/homes/selected'), + probes: { codex: probe } + }) + return { store, service } + } + + it('joins the listing the first read started and answers with it', async () => { + const pending = deferredListing() + const probe = vi.fn(() => pending.promise) + const { service } = coldService(probe) + expect(await service.read({ agent: 'codex' })).toEqual({ + origin: 'unknown', + listingInProgress: true + }) + const waited = service.read({ agent: 'codex', waitForListing: true }) + pending.resolve(listing('gpt-listed')) + const result = await waited + expect(result.origin === 'unknown' ? null : result.models[0]!.id).toBe('gpt-listed') + expect(probe).toHaveBeenCalledTimes(1) + }) + + it('answers a plain unknown when the listing fails', async () => { + const pending = deferredListing() + const { service } = coldService(() => pending.promise) + const waited = service.read({ agent: 'codex', waitForListing: true }) + pending.reject(new Error('spawn failed')) + expect(await waited).toEqual({ origin: 'unknown' }) + }) + + it('does not wait or report a listing while a failure is inside its TTL', async () => { + const probe = vi.fn(async (): Promise => { + throw new Error('spawn failed') + }) + const { store, service } = coldService(probe) + store.recordFailure(selectedHomeFingerprint('/homes/selected'), 'spawn failed') + expect(await service.read({ agent: 'codex', waitForListing: true })).toEqual({ + origin: 'unknown' + }) + expect(await service.read({ agent: 'codex' })).toEqual({ origin: 'unknown' }) + expect(probe).not.toHaveBeenCalled() + }) + + it('reports no listing where the host has no lister for the account', async () => { + const store = new AgentModelCatalogStore() + const service = createAgentModelCatalogService({ + store, + getRecord: () => undefined, + resolveAccountHome: async () => CODEX_HOME('/homes/selected') + }) + expect(await service.read({ agent: 'codex', waitForListing: true })).toEqual({ + origin: 'unknown' + }) + }) + + it('serves an aged entry at once and refreshes it behind the answer', async () => { + let now = 0 + const store = new AgentModelCatalogStore({ now: () => now }) + store.recordSuccess(selectedHomeFingerprint('/homes/selected'), 'codex', listing('gpt-old')) + now = AGENT_MODEL_CATALOG_FRESH_MS + const probe = vi.fn(() => new Promise(() => {})) + const service = createAgentModelCatalogService({ + store, + getRecord: () => undefined, + resolveAccountHome: async () => CODEX_HOME('/homes/selected'), + probes: { codex: probe } + }) + const result = await service.read({ agent: 'codex', waitForListing: true }) + expect(result.origin === 'unknown' ? null : result.models[0]!.id).toBe('gpt-old') + expect(probe).toHaveBeenCalledTimes(1) + }) + }) + describe('a read for the workspace a new chat runs in', () => { function serviceWith(mayOverride: boolean) { const store = new AgentModelCatalogStore() diff --git a/src/main/native-chat/agent-model-catalog/agent-model-catalog-service.ts b/src/main/native-chat/agent-model-catalog/agent-model-catalog-service.ts index 8c794331a2c..638b0f54272 100644 --- a/src/main/native-chat/agent-model-catalog/agent-model-catalog-service.ts +++ b/src/main/native-chat/agent-model-catalog/agent-model-catalog-service.ts @@ -35,6 +35,8 @@ export type AgentModelCatalogService = { sessionId?: string /** Where a new chat would run; null when one was named but is not a local directory. */ workspacePath?: string | null + /** With no entry yet, answer from the listing this read starts or joins instead of `unknown`. */ + waitForListing?: boolean }) => Promise } @@ -79,8 +81,9 @@ async function workspaceKeepsListedDefault( * launch); without one, the key is the account a launch would pin right now — * never "whichever account listed last". `unknown` tells the client to keep * its static seed, and a missing or aged entry kicks one joined background - * probe so the next read is warm. Failures are the store's 30s TTL, never an - * answer — a picker is a user surface and must not block. + * probe so the next read is warm. With no entry, the answer says that listing + * is running, and only a read that asks waits for it. Failures are the store's + * 30s TTL, never an answer: inside it a read answers `unknown` at once. */ export function createAgentModelCatalogService( deps: AgentModelCatalogServiceDeps @@ -110,14 +113,27 @@ export function createAgentModelCatalogService( }) accountHomePath = resolved.path } - const entry = deps.store.get(fingerprint) + let entry = deps.store.get(fingerprint) const probe = deps.probes?.[params.agent] - if (probe && accountHomePath && deps.store.shouldRefresh(fingerprint)) { - const home = accountHomePath - void deps.store.refresh(fingerprint, params.agent, () => probe(home)) - } + const home = accountHomePath + // Without an entry, join a running listing too: that is the one a waiting read answers from. + const listing = + probe && + home && + (entry ? deps.store.shouldRefresh(fingerprint) : !deps.store.hasActiveFailure(fingerprint)) + ? deps.store.refresh(fingerprint, params.agent, () => probe(home)) + : null if (!entry) { - return { origin: 'unknown' } + if (!listing) { + return { origin: 'unknown' } + } + if (!params.waitForListing) { + return { origin: 'unknown', listingInProgress: true } + } + entry = await listing + if (!entry) { + return { origin: 'unknown' } + } } return resultFromEntry( entry, diff --git a/src/main/native-chat/agent-session-wire/structured-agent-session-options-read.test.ts b/src/main/native-chat/agent-session-wire/structured-agent-session-options-read.test.ts new file mode 100644 index 00000000000..5657fcfb4d8 --- /dev/null +++ b/src/main/native-chat/agent-session-wire/structured-agent-session-options-read.test.ts @@ -0,0 +1,48 @@ +// A chat's options at rest come from the host catalog without waiting on a listing. + +import { describe, expect, it, vi } from 'vitest' +import type { AgentSessionRecord } from '../../../shared/agent-session-record' +import { createAgentModelCatalogService } from '../agent-model-catalog/agent-model-catalog-service' +import { + AgentModelCatalogStore, + type AgentModelCatalogSuccess +} from '../agent-model-catalog/agent-model-catalog-store' +import type { StructuredAgentSessionMutationContext } from './structured-agent-session-host-mutations' +import { readStructuredAgentSessionOptions } from './structured-agent-session-options-read' + +const SESSION = 'session-1' + +function restingRecord(): AgentSessionRecord { + // oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: the resting read and the catalog key touch only these fields. + return { + provider: 'codex', + accountHome: { variable: 'CODEX_HOME', path: '/homes/a' }, + location: { wslDistro: null }, + options: {} + } as unknown as AgentSessionRecord +} + +describe('options at rest', () => { + it('answers while the first catalog listing is still running', async () => { + const record = restingRecord() + const probe = vi.fn(() => new Promise(() => {})) + const modelCatalog = createAgentModelCatalogService({ + store: new AgentModelCatalogStore(), + getRecord: () => record, + resolveAccountHome: async () => ({ variable: 'CODEX_HOME', path: '/homes/a' }), + probes: { codex: probe } + }) + const resting = { child: null, params: { provider: 'codex' } } + // oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: the resting read touches only these members. + const context = { + deps: { adapter: {}, store: { getRecord: () => record }, modelCatalog }, + serialize: (_sessionId: string, task: () => Promise) => task(), + openConversation: async () => resting, + conversation: async () => resting + } as unknown as StructuredAgentSessionMutationContext + + const result = await readStructuredAgentSessionOptions(context, SESSION) + expect(probe).toHaveBeenCalledTimes(1) + expect(result.models).toEqual([]) + }) +}) diff --git a/src/main/runtime/rpc/methods/structured-agent-session-options-read.test.ts b/src/main/runtime/rpc/methods/structured-agent-session-options-read.test.ts index 77d44ece745..459ae5c75a4 100644 --- a/src/main/runtime/rpc/methods/structured-agent-session-options-read.test.ts +++ b/src/main/runtime/rpc/methods/structured-agent-session-options-read.test.ts @@ -56,4 +56,66 @@ describe('agentSession.modelCatalog', () => { await call('agentSession.modelCatalog', { agent: 'claude' }, STRUCTURED_CLIENT) expect(read).toHaveBeenCalledWith({ agent: 'claude' }) }) + + it('passes a wait for the listing through to the catalog', async () => { + await call( + 'agentSession.modelCatalog', + { agent: 'codex', sessionId: SESSION, waitForListing: true }, + STRUCTURED_CLIENT + ) + expect(read).toHaveBeenCalledWith({ agent: 'codex', sessionId: SESSION, waitForListing: true }) + }) +}) + +describe('agentSession.modelCatalog before anything built the host', () => { + const read = vi.fn(async () => ({ + origin: 'probe' as const, + models: [{ id: 'gpt-host', label: 'GPT Host', isDefault: true, efforts: [] }], + fetchedAt: 1 + })) + const installHost = vi.fn(async () => { + setStructuredAgentSessionHost(Object.assign(hostStub(), { deps: { modelCatalog: { read } } })) + }) + + beforeEach(() => { + read.mockClear() + installHost.mockClear() + clearStructuredHostStub() + }) + + // A new chat's picker reads before its create lands; on a host with no saved chats nothing else + // has built the host yet, and a refusal here left the picker on the client's built-in list. + it('builds the host for a structured chat and answers from its catalog', async () => { + const reply = await call( + 'agentSession.modelCatalog', + { agent: 'codex', sessionId: SESSION }, + STRUCTURED_CLIENT, + { ensureStructuredAgentSessionHost: installHost } + ) + expect(installHost).toHaveBeenCalledTimes(1) + expect(reply).toMatchObject({ ok: true, result: { origin: 'probe' } }) + expect(read).toHaveBeenCalledWith({ agent: 'codex', sessionId: SESSION }) + }) + + // Terminal-backed chat reads with no session: a host that runs no structured chat keeps its + // journal closed, and the read falls back to the CLI listing. + it('does not build the host for a read that names no session', async () => { + const reply = await call('agentSession.modelCatalog', { agent: 'codex' }, STRUCTURED_CLIENT, { + ensureStructuredAgentSessionHost: installHost + }) + expect(installHost).not.toHaveBeenCalled() + expect(reply).toMatchObject({ ok: false }) + expect(read).not.toHaveBeenCalled() + }) + + it('does not build the host for a client that cannot read structured sessions', async () => { + const reply = await call( + 'agentSession.modelCatalog', + { agent: 'codex', sessionId: SESSION }, + { clientKind: 'runtime', clientCapabilities: [] }, + { ensureStructuredAgentSessionHost: installHost } + ) + expect(installHost).not.toHaveBeenCalled() + expect(reply).toMatchObject({ ok: false }) + }) }) diff --git a/src/main/runtime/rpc/methods/structured-agent-session-options-read.ts b/src/main/runtime/rpc/methods/structured-agent-session-options-read.ts index 19e93689577..3cd86c876ad 100644 --- a/src/main/runtime/rpc/methods/structured-agent-session-options-read.ts +++ b/src/main/runtime/rpc/methods/structured-agent-session-options-read.ts @@ -11,7 +11,7 @@ import { defineMethod } from '../core' import { requireInstalledStructuredHost, - requireStructuredHost as requireHost + requireStructuredHost } from './structured-agent-session-gate' import { ModelCatalogParams, OptionsParams } from './structured-agent-session-schemas' @@ -25,8 +25,14 @@ export const STRUCTURED_AGENT_SESSION_OPTIONS_READ_METHODS = [ defineMethod({ name: 'agentSession.modelCatalog', params: ModelCatalogParams, + // A structured chat's read names its session and builds the host, since it may come first; + // terminal-backed chat's session-less read must not open the journal where none runs. handler: async ({ worktree, ...params }, ctx) => { - const catalog = requireHost(ctx).deps.modelCatalog + const host = + params.sessionId === undefined + ? requireStructuredHost(ctx) + : await requireInstalledStructuredHost(ctx) + const catalog = host.deps.modelCatalog if (!catalog) { return { origin: 'unknown' as const } } diff --git a/src/renderer/src/components/native-chat/NativeChatSessionOptionPickers.menu-request.test.tsx b/src/renderer/src/components/native-chat/NativeChatSessionOptionPickers.menu-request.test.tsx new file mode 100644 index 00000000000..90abdc3ef25 --- /dev/null +++ b/src/renderer/src/components/native-chat/NativeChatSessionOptionPickers.menu-request.test.tsx @@ -0,0 +1,109 @@ +// @vitest-environment happy-dom + +// Against the real menu: a `/model` request opens the menu once, and a period while the host still +// lists models neither opens it later nor reopens one the user closed. + +import { act, cleanup, fireEvent, render, screen } from '@testing-library/react' +import { afterEach, describe, expect, it, vi } from 'vitest' +import type { + SessionOptionDescriptor, + SessionOptionsSurface +} from '../../../../shared/native-chat-session-options' + +vi.mock('sonner', () => ({ toast: { error: vi.fn() } })) + +vi.mock('@/i18n/i18n', () => ({ + translate: (_key: string, fallback: string, values?: Record) => + values + ? Object.entries(values).reduce( + (text, [name, value]) => text.replaceAll(`{{${name}}}`, String(value)), + fallback + ) + : fallback +})) + +import { TooltipProvider } from '@/components/ui/tooltip' +import { NativeChatSessionOptionPickers } from './NativeChatSessionOptionPickers' +import type { NativeChatOptionPickerRequest } from './native-chat-composer-types' + +function model(pending: boolean): SessionOptionDescriptor { + return { + id: 'model', + label: 'Model', + category: 'model', + kind: { + type: 'select', + currentValue: 'opus', + choices: [ + { value: 'opus', label: 'Opus 4.8' }, + { value: 'sonnet', label: 'Sonnet 5' } + ] + }, + valueSource: 'applied', + transport: 'agent-session', + settable: !pending, + ...(pending ? { choicesPending: true as const } : {}) + } +} + +const surface: SessionOptionsSurface = { + getSnapshot: () => [], + setOption: vi.fn(async () => ({ snapshot: [] })), + invokeAction: vi.fn(async () => ({ snapshot: [] })), + subscribe: () => () => {} +} + +function view(pending: boolean, request: NativeChatOptionPickerRequest | null): React.JSX.Element { + return ( + +