From 2f828e44621071c1d2d12a28631bf5a4d97a9b7b Mon Sep 17 00:00:00 2001 From: Brennan Benson <79079362+brennanb2025@users.noreply.github.com> Date: Wed, 9 Sep 2026 21:38:35 -0700 Subject: [PATCH 1/8] fix(native-chat): show Claude working from the send, not the provider echo (#19822) * fix(native-chat): show Claude working from the send, not the provider echo A structured session read as working only once a turnLifecycle row existed. Codex writes that row ~150ms after the send; Claude cannot write it until the SDK echoes the user message back, measured at a 3.4s median and 18s at p90, so the chat and every session list read idle for the whole wait. The journalled submission is the host's own evidence a turn is owed, so the shared projection reads it too. `unknown` still counts -- the ack budget elapsing answers delivery, not whether work is owed -- while a recovered `unknown` does not, which needed the existing row flag carried onto the projected submission. Claude's activity line now stays the generic fallback. Its only turn-wide frame carries a bare token, and its task_* prose describes a spawned task rather than this turn; compaction is kept because it explains an otherwise silent wait. * Fix structured chat pending-work lifecycle and mobile cancellation --------- Co-authored-by: Merge Sim --- .../src/session/MobileNativeChatOverlay.tsx | 1 + .../src/session/MobileNativeChatView.test.ts | 13 +++ mobile/src/session/MobileNativeChatView.tsx | 8 +- .../mobile-native-chat-controller-contract.ts | 1 + .../use-mobile-native-chat-controller.test.ts | 35 +++++++- .../use-mobile-native-chat-controller.ts | 3 + .../use-mobile-structured-agent-session.ts | 10 ++- .../journal-crash-boundary.test.ts | 27 ++++++ .../journal-pending-submission-recovery.ts | 14 ++- .../agent-session-journal/journal-reducer.ts | 6 ++ .../agent-session-journal/journal-store.ts | 7 +- .../provider-frame-activity.test.ts | 42 ++++----- .../provider-frame-activity.ts | 49 ++++------- .../provider-turn-activity-routing.test.ts | 5 +- .../structured-agent-session-host-handoff.ts | 7 ++ ...red-agent-session-send-idempotency.test.ts | 34 +++++++ ...ructured-agent-session-status-feed.test.ts | 57 +++++++++++- .../structured-agent-session-status-feed.ts | 7 +- ...red-agent-session-surface-lifetime.test.ts | 6 ++ .../structured-agent-session-turns.ts | 12 ++- ...ured-agent-session-unexpected-exit.test.ts | 11 ++- ...tructured-agent-session-unexpected-exit.ts | 13 +++ .../NativeChatStructuredSession.tsx | 4 +- .../use-structured-agent-session.test.tsx | 66 +++++++++++++- .../use-structured-agent-session.ts | 11 ++- src/shared/agent-session-journal-schemas.ts | 3 +- src/shared/agent-session-journal-types.ts | 4 + ...tructured-agent-session-projection.test.ts | 88 ++++++++++++++++++- .../structured-agent-session-projection.ts | 51 +++++++++-- 29 files changed, 504 insertions(+), 91 deletions(-) diff --git a/mobile/src/session/MobileNativeChatOverlay.tsx b/mobile/src/session/MobileNativeChatOverlay.tsx index 357a089466e..dcb353e54f8 100644 --- a/mobile/src/session/MobileNativeChatOverlay.tsx +++ b/mobile/src/session/MobileNativeChatOverlay.tsx @@ -71,6 +71,7 @@ export function MobileNativeChatOverlay({ error={session.error} agent={controller.nativeChatAgent} agentWorking={controller.nativeChatAgentWorking} + canStop={controller.nativeChatCanStop} structuredActivityUi={controller.nativeChatStructured} streaming={streaming} onStop={controller.handleNativeChatStop} diff --git a/mobile/src/session/MobileNativeChatView.test.ts b/mobile/src/session/MobileNativeChatView.test.ts index d101c3f0ef6..63bfb715445 100644 --- a/mobile/src/session/MobileNativeChatView.test.ts +++ b/mobile/src/session/MobileNativeChatView.test.ts @@ -74,6 +74,7 @@ type Overrides = { pending?: Parameters[0]['pending'] structuredActivityUi?: boolean agentWorking?: boolean + canStop?: boolean sendSurfaceId?: string } @@ -118,6 +119,18 @@ describe('MobileNativeChatView', () => { } /** Ids of the rows the list is currently rendering. */ + it('keeps Stop hidden during a structured dispatch until a provider turn can be cancelled', async () => { + const props = { structuredActivityUi: true, agentWorking: true, canStop: false } + await render(props) + const stops = () => + renderer!.root.findAll((node) => node.props.accessibilityLabel === 'Stop the agent') + expect(stops()).toHaveLength(0) + await update({ ...props, canStop: true }) + expect(stops()).toHaveLength(1) + await update({ agentWorking: true }) + expect(stops()).toHaveLength(1) + }) + function listIds(): string[] { const list = renderer!.root.find((node) => node.type === 'FlatList') return (list.props.data as { id: string }[]).map((row) => row.id) diff --git a/mobile/src/session/MobileNativeChatView.tsx b/mobile/src/session/MobileNativeChatView.tsx index 70a67787de0..f58e2ad05b0 100644 --- a/mobile/src/session/MobileNativeChatView.tsx +++ b/mobile/src/session/MobileNativeChatView.tsx @@ -49,10 +49,11 @@ type Props = { /** Resolved agent for this chat; names the empty-state copy (desktop parity). */ agent?: string | null agentWorking?: boolean + canStop?: boolean /** Structured lane: per-turn "Working for N" status plus live tool progress, * replacing the bridge lane's static three-dot working row (desktop parity). */ structuredActivityUi?: boolean - /** Interrupt the agent mid-turn (shown as a Stop button on the working bar). */ + /** Interrupt a provider turn. */ onStop?: () => void /** Live partial assistant text to show as an in-progress bubble, already gated * by the overlay against the transcript catching up. */ @@ -129,6 +130,7 @@ export function MobileNativeChatView({ error, agent, agentWorking, + canStop = agentWorking, structuredActivityUi = false, onStop, streaming, @@ -396,8 +398,6 @@ export function MobileNativeChatView({ question={question} onAnswerQuestion={onAnswerQuestion} /> - {/* Chrome row above the composer: the working indicator and the global - tool-calls expand/collapse toggle on the left, Stop in the far corner. */} {agentWorking && !structuredActivityUi ? : null} @@ -414,7 +414,7 @@ export function MobileNativeChatView({ {toolsExpanded ? 'Collapse' : 'Tools'} - {agentWorking ? ( + {canStop ? ( [styles.stopButton, pressed && styles.pressed]} onPress={onStop} diff --git a/mobile/src/session/mobile-native-chat-controller-contract.ts b/mobile/src/session/mobile-native-chat-controller-contract.ts index 53187e0d6db..0c6ffd6a764 100644 --- a/mobile/src/session/mobile-native-chat-controller-contract.ts +++ b/mobile/src/session/mobile-native-chat-controller-contract.ts @@ -28,6 +28,7 @@ export type MobileNativeChatController = { /** Structured lane: drives the per-turn status row and live tool progress. */ nativeChatStructured: boolean nativeChatAgentWorking: boolean + nativeChatCanStop: boolean nativeChatStreamingText?: string /** Agent mid-turn, regardless of whether chat is the visible view. */ nativeChatStreamLive: boolean diff --git a/mobile/src/session/use-mobile-native-chat-controller.test.ts b/mobile/src/session/use-mobile-native-chat-controller.test.ts index 83f8075d914..c90033e2404 100644 --- a/mobile/src/session/use-mobile-native-chat-controller.test.ts +++ b/mobile/src/session/use-mobile-native-chat-controller.test.ts @@ -55,6 +55,7 @@ const structuredQuestion = { allowOther: true, optionTokens: ['choice-a', 'choice-b'] } +const structuredActivity = { isWorking: false, turnId: null as string | null } const structuredSessionState = { messages: [] as unknown[], status: 'ready', @@ -86,8 +87,7 @@ vi.mock('./use-mobile-native-chat-session', () => ({ vi.mock('./use-mobile-structured-agent-session', () => ({ useMobileStructuredAgentSession: () => ({ session: structuredSessionState, - isWorking: false, - turnId: null, + ...structuredActivity, sendWithOutcome: structuredSendWithOutcome, cancel: structuredCancel, permission: structuredPermission, @@ -341,6 +341,37 @@ describe('useMobileNativeChatController handleNativeChatSend', () => { expect(clientStub.sendRequest).not.toHaveBeenCalled() }) + it('separates structured working status from provider cancellation availability', async () => { + const props = { + tab: { + type: 'agent-session', + id: 'agent-tab-1', + title: 'Chat', + sessionId: 'session-structured', + agent: 'codex', + isActive: true + }, + activeHandle: null, + inputLeaseReady: false + } + structuredActivity.isWorking = true + try { + await act(async () => { + renderer?.update(createElement(Harness, props)) + }) + expect(controller?.nativeChatAgentWorking).toBe(true) + expect(controller?.nativeChatCanStop).toBe(false) + structuredActivity.turnId = 'provider-turn' + await act(async () => { + renderer?.update(createElement(Harness, props)) + }) + expect(controller?.nativeChatCanStop).toBe(true) + } finally { + structuredActivity.isWorking = false + structuredActivity.turnId = null + } + }) + it('exposes structured prompt cards and session options on structured tabs', async () => { await act(async () => { renderer?.update( diff --git a/mobile/src/session/use-mobile-native-chat-controller.ts b/mobile/src/session/use-mobile-native-chat-controller.ts index 729cec302c1..5195edf2124 100644 --- a/mobile/src/session/use-mobile-native-chat-controller.ts +++ b/mobile/src/session/use-mobile-native-chat-controller.ts @@ -299,6 +299,9 @@ export function useMobileNativeChatController(args: { /** Structured lane: drives the per-turn status row and live tool progress. */ nativeChatStructured: activeChatStructured, nativeChatAgentWorking, + nativeChatCanStop: activeChatStructured + ? structuredNativeChat.turnId !== null + : nativeChatAgentWorking, nativeChatStreamingText, nativeChatStreamLive, nativeChatStreamScopeKey: streamScopeKey, diff --git a/mobile/src/session/use-mobile-structured-agent-session.ts b/mobile/src/session/use-mobile-structured-agent-session.ts index 271f9143671..86d3f2c8e44 100644 --- a/mobile/src/session/use-mobile-structured-agent-session.ts +++ b/mobile/src/session/use-mobile-structured-agent-session.ts @@ -11,7 +11,10 @@ import { import { encodeNativeChatTranscriptIdentity } from '../../../src/shared/native-chat-transcript-retention' import type { MobileNativeChatSendOutcome } from './mobile-native-chat-send' import { projectStructuredAgentSessionMessages } from '../../../src/shared/structured-agent-session-message-projection' -import { activeStructuredAgentSessionTurnId } from '../../../src/shared/structured-agent-session-projection' +import { + activeStructuredAgentSessionTurnId, + hasUnansweredStructuredAgentSessionDispatch +} from '../../../src/shared/structured-agent-session-projection' import { pendingStructuredApproval, pendingStructuredQuestion, @@ -291,7 +294,10 @@ export function useMobileStructuredAgentSession(args: { loadingEarlier: loadingOlder, loadEarlier }, - isWorking: activeStructuredAgentSessionTurnId(state.items) !== null, + // A dispatch the provider has not answered yet is already work — see the desktop hook. + isWorking: + activeStructuredAgentSessionTurnId(state.items) !== null || + hasUnansweredStructuredAgentSessionDispatch(state.submissions, state.fence), turnId: activeStructuredAgentSessionTurnId(state.items), sendWithOutcome, cancel, diff --git a/src/main/native-chat/agent-session-journal/journal-crash-boundary.test.ts b/src/main/native-chat/agent-session-journal/journal-crash-boundary.test.ts index a7f54a14a4c..85cda398c1d 100644 --- a/src/main/native-chat/agent-session-journal/journal-crash-boundary.test.ts +++ b/src/main/native-chat/agent-session-journal/journal-crash-boundary.test.ts @@ -15,6 +15,7 @@ import type { AgentJournalMessageItem, AgentSessionJournalIdentity } from '../../../shared/agent-session-journal-types' +import { hasUnansweredStructuredAgentSessionDispatch } from '../../../shared/structured-agent-session-projection' import { digestPayload } from './journal-payload-bounds' import { reconcileSubmissions, @@ -110,6 +111,8 @@ describe('crash between provider accept and journal commit', () => { expect(restarted.pendingSubmissions().map((entry) => entry.clientMessageId)).toEqual(['cm_1']) await restarted.markPendingSubmissionsUnknown(2) expect(restarted.submissions()[0]?.dispatchState).toBe('unknown') + // Marks the send as outlived by its writer, so no reader reports it as still working. + expect(restarted.submissions()[0]?.recovered).toBe(true) const [outcome] = reconcileSubmissions({ submissions: restarted.submissions(), @@ -139,6 +142,30 @@ describe('crash between provider accept and journal commit', () => { expect(restarted.receiptFor('cm_1')?.providerItemId).toBe(agentJournalItemKey(outcome.identity)) }) + it('retires an ack timeout on restart without changing its delivery verdict', async () => { + const journal = await open() + await journal.appendSubmission({ + clientMessageId: 'cm_timeout', + payloadFingerprint: digestPayload('slow'), + body: userMessage('slow'), + fence: 1 + }) + await journal.resolveDispatch({ + clientMessageId: 'cm_timeout', + state: 'unknown', + reason: 'ack timeout', + fence: 1 + }) + expect(hasUnansweredStructuredAgentSessionDispatch(journal.submissions())).toBe(true) + const restarted = await open() + await restarted.markPendingSubmissionsUnknown(2) + expect(restarted.submissions()[0]?.dispatchState).toBe('unknown') + expect(hasUnansweredStructuredAgentSessionDispatch(restarted.submissions())).toBe(false) + const cursor = restarted.cursor() + await restarted.markPendingSubmissionsUnknown(2) + expect(restarted.cursor()).toEqual(cursor) + }) + it('reports a rejected submission as never delivered, and never re-sends it', async () => { const journal = await open() await journal.appendSubmission({ 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 76bc00394f2..e09dd109d50 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,14 +2,22 @@ import type { AgentSessionJournal } from './journal-store' export async function markJournalPendingSubmissionsUnknown( journal: AgentSessionJournal, - fence: number + fence: number, + reason = 'host_restarted_before_acknowledgement' ): Promise { - const pending = journal.pendingSubmissions().map((entry) => entry.clientMessageId) + const pending = journal + .submissions() + .filter( + (entry) => + entry.dispatchState === 'pending' || + (entry.dispatchState === 'unknown' && entry.recovered !== true) + ) + .map((entry) => entry.clientMessageId) for (const clientMessageId of pending) { await journal.resolveDispatch({ clientMessageId, state: 'unknown', - reason: 'host_restarted_before_acknowledgement', + reason, fence, recovered: true }) 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 41625792aa0..e01e7d6158f 100644 --- a/src/main/native-chat/agent-session-journal/journal-reducer.ts +++ b/src/main/native-chat/agent-session-journal/journal-reducer.ts @@ -256,10 +256,16 @@ function applyDispatch( if (submission.dispatchState === 'rejected' || submission.dispatchState === 'accepted') { return } + submission.fence = row.fence submission.dispatchState = row.state submission.providerItemId = row.providerItemId submission.reason = row.reason submission.resolvedAt = row.ts + if (row.recovered) { + submission.recovered = row.recovered + } else { + delete submission.recovered + } if (row.state !== 'accepted' || !row.providerItemId) { return } diff --git a/src/main/native-chat/agent-session-journal/journal-store.ts b/src/main/native-chat/agent-session-journal/journal-store.ts index 3c16800099b..812e2bca834 100644 --- a/src/main/native-chat/agent-session-journal/journal-store.ts +++ b/src/main/native-chat/agent-session-journal/journal-store.ts @@ -245,10 +245,9 @@ export class AgentSessionJournal { })) } - /** On restart every `pending` submission becomes `unknown` before the session - * accepts a writer. Orca never re-sends on the user's behalf. */ - async markPendingSubmissionsUnknown(fence: number): Promise { - return markJournalPendingSubmissionsUnknown(this, fence) + /** Retire unanswered sends after their execution owner ended, without assuming delivery. */ + async markPendingSubmissionsUnknown(fence: number, reason?: string): Promise { + return markJournalPendingSubmissionsUnknown(this, fence, reason) } /** The escape hatch for corruption, an unreconcilable prefix, a forked handle, diff --git a/src/main/native-chat/agent-session-wire/provider-frame-activity.test.ts b/src/main/native-chat/agent-session-wire/provider-frame-activity.test.ts index 40504873282..ba2eba58062 100644 --- a/src/main/native-chat/agent-session-wire/provider-frame-activity.test.ts +++ b/src/main/native-chat/agent-session-wire/provider-frame-activity.test.ts @@ -38,36 +38,24 @@ describe('provider frame activity', () => { } }) - it('uses Claude descriptions and safe semantic status without exposing tool labels', () => { - expect( - claudeProviderFrameActivity('message:system:task_started', { - description: 'Trace the activity channel' - }) - ).toBe('Working on: Trace the activity channel') - expect( - claudeProviderFrameActivity('message:system:task_progress', { - description: 'Reading tests', - summary: 'Checking remote compatibility' - }) - ).toBe('Checking remote compatibility') - expect( - claudeProviderFrameActivity('message:system:task_updated', { - patch: { description: 'Validating the renderer' } - }) - ).toBe('Validating the renderer') + it('leaves the Claude line on the generic fallback, since Claude never narrates its turn', () => { + // Prose on these frames belongs to a spawned task, not to this turn. + for (const [kind, payload] of [ + ['message:system:task_started', { description: 'Trace the activity channel' }], + ['message:system:task_progress', { summary: 'Checking remote compatibility' }], + ['message:system:task_updated', { patch: { description: 'Validating the renderer' } }], + ['message:system:control_request_progress', { status: 'api_retry' }], + ['message:tool_progress', { tool_name: 'ReadSecretFile' }] + ] as const) { + expect(claudeProviderFrameActivity(kind, payload)).toBeNull() + } + // `requesting` holds for nearly the whole turn and says no more than the fallback. + expect(claudeProviderFrameActivity('message:system:status', { status: 'requesting' })).toBeNull() expect(claudeProviderFrameActivity('message:system:status', { status: 'compacting' })).toBe( 'Compacting the conversation' ) - expect( - claudeProviderFrameActivity('message:system:control_request_progress', { - status: 'api_retry' - }) - ).toBe('Retrying a side question') - expect( - claudeProviderFrameActivity('message:tool_progress', { - tool_name: 'ReadSecretFile' - }) - ).toBeNull() + // An unmodeled frame still declines to answer, so it cannot clear live copy. + expect(claudeProviderFrameActivity('message:system:unknown_frame', {})).toBeUndefined() }) it('falls through on protocol noise and bounds long copy', () => { diff --git a/src/main/native-chat/agent-session-wire/provider-frame-activity.ts b/src/main/native-chat/agent-session-wire/provider-frame-activity.ts index 336170a4cfe..3cd6c4abaf0 100644 --- a/src/main/native-chat/agent-session-wire/provider-frame-activity.ts +++ b/src/main/native-chat/agent-session-wire/provider-frame-activity.ts @@ -100,40 +100,29 @@ export function codexProviderFrameActivity( return itemType ? (CODEX_ITEM_ACTIVITY[itemType] ?? null) : null } +/** + * Claude does not narrate its own turn, so the activity line stays the generic fallback. + * + * Codex names each item it starts, which is what makes its line worth reading. Claude's only + * turn-wide frame is `system/status`, whose payload is a bare token — every sentence Orca ever + * put on this line for it was Orca's own wording for `requesting`, which is true for nearly the + * whole turn and says no more than the fallback does. Its `task_*` frames do carry prose, but + * they are keyed by task id and subagent type: they describe a spawned task, not this turn, and + * the background-tasks strip already owns that. Compaction is the one exception kept — a real, + * rare state that explains an otherwise unexplained wait, and the Codex map reports it too. + */ export function claudeProviderFrameActivity(kind: string, payload: unknown): ActivityText { const source = record(payload) - if (kind === 'message:system:task_started') { - if (source?.ambient === true || source?.skip_transcript === true) { - return null - } - const description = providerActivityText(stringField(source, 'description')) - return description ? providerActivityText(`Working on: ${description}`) : null - } - if (kind === 'message:system:task_progress') { - return providerActivityText( - stringField(source, 'summary') ?? stringField(source, 'description') - ) - } - if (kind === 'message:system:task_updated') { - return providerActivityText(stringField(record(source?.patch), 'description')) - } if (kind === 'message:system:status') { - const status = stringField(source, 'status') - return status === 'compacting' - ? 'Compacting the conversation' - : status === 'requesting' - ? 'Requesting a response' - : null + return stringField(source, 'status') === 'compacting' ? 'Compacting the conversation' : null } - if (kind === 'message:system:control_request_progress') { - const status = stringField(source, 'status') - return status === 'started' - ? 'Exploring a side question' - : status === 'api_retry' - ? 'Retrying a side question' - : null - } - if (kind === 'message:tool_progress') { + if ( + kind === 'message:system:task_started' || + kind === 'message:system:task_progress' || + kind === 'message:system:task_updated' || + kind === 'message:system:control_request_progress' || + kind === 'message:tool_progress' + ) { return null } return undefined diff --git a/src/main/native-chat/agent-session-wire/provider-turn-activity-routing.test.ts b/src/main/native-chat/agent-session-wire/provider-turn-activity-routing.test.ts index a66ac567a4d..eea8398f7eb 100644 --- a/src/main/native-chat/agent-session-wire/provider-turn-activity-routing.test.ts +++ b/src/main/native-chat/agent-session-wire/provider-turn-activity-routing.test.ts @@ -248,10 +248,11 @@ describe('provider turn activity routing', () => { }) ) expect(state.rows).toHaveLength(turnRows) + // Only compaction reaches the line; task and side-question prose is not this turn's work. expect(state.activities.slice(-3)).toEqual([ - { turnId: TURN_ID, text: 'Checking the renderer state' }, + null, { turnId: TURN_ID, text: 'Compacting the conversation' }, - { turnId: TURN_ID, text: 'Exploring a side question' } + null ]) translator.handle(claudeMessage({ type: 'tool_progress', tool_name: 'SecretReader' })) diff --git a/src/main/native-chat/agent-session-wire/structured-agent-session-host-handoff.ts b/src/main/native-chat/agent-session-wire/structured-agent-session-host-handoff.ts index 3495409f127..113940ff0f4 100644 --- a/src/main/native-chat/agent-session-wire/structured-agent-session-host-handoff.ts +++ b/src/main/native-chat/agent-session-wire/structured-agent-session-host-handoff.ts @@ -86,6 +86,13 @@ export function createStructuredAgentSessionHostHandoff( host.publishStatus?.(sessionId) try { await host.flush(sessionId) + const session = host.session(sessionId) + await session.journal.markPendingSubmissionsUnknown( + session.fence, + 'provider_exited_before_acknowledgement' + ) + host.subscribers.publish(sessionId, session.journal) + host.publishStatus?.(sessionId) host.eventSink(sessionId).unbind() return { state: 'stopped' } } catch (error) { diff --git a/src/main/native-chat/agent-session-wire/structured-agent-session-send-idempotency.test.ts b/src/main/native-chat/agent-session-wire/structured-agent-session-send-idempotency.test.ts index 581743633c0..be9386815dd 100644 --- a/src/main/native-chat/agent-session-wire/structured-agent-session-send-idempotency.test.ts +++ b/src/main/native-chat/agent-session-wire/structured-agent-session-send-idempotency.test.ts @@ -3,6 +3,7 @@ import { tmpdir } from 'node:os' import { join } from 'node:path' import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' import type { AgentJournalMessageItem } from '../../../shared/agent-session-journal-types' +import { hasUnansweredStructuredAgentSessionDispatch } from '../../../shared/structured-agent-session-projection' import { structuredAgentSessionPayloadFingerprint } from '../../../shared/structured-agent-session-mutation' import { createTrackedJournalOpener } from '../agent-session-journal/journal-store-test-open' import type { AgentSessionJournal } from '../agent-session-journal/journal-store' @@ -34,6 +35,39 @@ afterEach(async () => { }) describe('structured send idempotency', () => { + it('publishes a recovered retry as working before waiting for its provider', async () => { + const body: AgentJournalMessageItem = { + kind: 'message', + role: 'user', + blocks: [{ type: 'text', text: 'retry' }] + } + const input = { clientMessageId: 'retry-id', payloadFingerprint: 'fingerprint', body } + await journal.appendSubmission({ ...input, fence: 1 }) + await journal.markPendingSubmissionsUnknown(2) + const originalItem = journal.snapshot().items[0] + const publish = vi.fn() + const dispatch = vi.fn(async () => { + expect(publish).toHaveBeenCalledOnce() + expect(hasUnansweredStructuredAgentSessionDispatch(journal.submissions(), 2)).toBe(true) + return { state: 'unknown' as const, reason: 'ack timeout' } + }) + await performSend( + { + sessionId: 'session-1', + journal, + fence: 2, + adapter: { dispatch } as unknown as StructuredAgentSessionAdapter, + persistOptions: async () => undefined, + resolvedBy: 'caller', + publish, + now: () => 1 + }, + { ...input, retryUnknown: true } + ) + expect(hasUnansweredStructuredAgentSessionDispatch(journal.submissions(), 2)).toBe(true) + expect(journal.snapshot().items).toEqual([originalItem]) + }) + it('does not redispatch one send id reused across caller ledgers', async () => { const body: AgentJournalMessageItem = { kind: 'message', diff --git a/src/main/native-chat/agent-session-wire/structured-agent-session-status-feed.test.ts b/src/main/native-chat/agent-session-wire/structured-agent-session-status-feed.test.ts index 1e3b9bb25a3..7bd53742430 100644 --- a/src/main/native-chat/agent-session-wire/structured-agent-session-status-feed.test.ts +++ b/src/main/native-chat/agent-session-wire/structured-agent-session-status-feed.test.ts @@ -56,9 +56,11 @@ async function openJournal(sessionId = SESSION, now?: () => number) { function indexed(session: { journal: Awaited> hasProviderChild?: boolean + fence?: number }) { return { journal: session.journal, + fence: session.fence ?? 1, ...(session.hasProviderChild !== undefined ? { hasProviderChild: session.hasProviderChild } : {}), @@ -69,7 +71,7 @@ function indexed(session: { function feedFor( sessions: Map< string, - { journal: Awaited>; hasProviderChild?: boolean } + { journal: Awaited>; hasProviderChild?: boolean; fence?: number } >, record: Partial | null = null, onStatusChanged?: StructuredAgentSessionStatusFeedDeps['onStatusChanged'] @@ -153,6 +155,59 @@ describe('StructuredAgentSessionStatusFeed', () => { ]) }) + it('stops projecting an old-host unknown submission after the owner fence advances', async () => { + const journal = await openJournal() + const session = { journal, fence: 1 } + const { feed, events } = feedFor(new Map([[SESSION, session]])) + await journal.appendSubmission({ + clientMessageId: 'old-host', + payloadFingerprint: 'fp', + body: { kind: 'message', role: 'user', blocks: [{ type: 'text', text: 'slow' }] }, + fence: 1 + }) + await journal.resolveDispatch({ + clientMessageId: 'old-host', + state: 'unknown', + reason: 'ack timeout', + fence: 1 + }) + feed.publish(SESSION) + expect(events.at(-1)).toMatchObject({ session: { status: 'working' } }) + session.fence = 2 + feed.publish(SESSION) + expect(events.at(-1)).toMatchObject({ session: { status: 'idle' } }) + }) + + it('publishes working from the pending submission, before the provider replays the turn', async () => { + const journal = await openJournal() + const { feed, events } = feedFor(new Map([[SESSION, { journal }]])) + events.length = 0 + await journal.appendSubmission({ + clientMessageId: 'client-1', + payloadFingerprint: 'fingerprint-1', + body: { kind: 'message', role: 'user', blocks: [{ type: 'text', text: 'write a poem' }] }, + fence: 1 + }) + + feed.publish(SESSION) + expect(events.at(-1)).toEqual({ + type: 'status', + session: expect.objectContaining({ status: 'working' }) + }) + + await journal.resolveDispatch({ + clientMessageId: 'client-1', + state: 'accepted', + providerIdentity: USER_IDENTITY, + fence: 1 + }) + feed.publish(SESSION) + expect(events.at(-1)).toEqual({ + type: 'status', + session: expect.objectContaining({ status: 'idle' }) + }) + }) + it('publishes working, then idle once the running marker is tombstoned, and never a repeat', async () => { const journal = await openJournal() const { feed, events } = feedFor(new Map([[SESSION, { journal }]])) diff --git a/src/main/native-chat/agent-session-wire/structured-agent-session-status-feed.ts b/src/main/native-chat/agent-session-wire/structured-agent-session-status-feed.ts index 61d9649587e..3c4ce4b97a6 100644 --- a/src/main/native-chat/agent-session-wire/structured-agent-session-status-feed.ts +++ b/src/main/native-chat/agent-session-wire/structured-agent-session-status-feed.ts @@ -31,6 +31,7 @@ type StatusFeedSession = { journal: AgentSessionJournal params: { location: { workspaceId: string }; provider: AgentSessionRecord['provider'] } hasProviderChild?: boolean + fence?: number } export type StructuredAgentSessionStatusFeedDeps = { @@ -153,7 +154,9 @@ export class StructuredAgentSessionStatusFeed { journal: AgentSessionJournal ): AgentSessionStatusSummary { // An unreadable journal projects as "no turn": the chat itself shows the reset. - const items = journal.isReadOnly ? [] : journal.snapshot().items + const snapshot = journal.isReadOnly ? null : journal.snapshot() + const items = snapshot?.items ?? [] + const submissions = snapshot?.submissions ?? [] const record = this.deps.getRecord(sessionId) const providerSession = structuredAgentSessionProviderSessionMetadata(record) // The journal has no model: the record's acknowledged options are where an owner @@ -164,7 +167,7 @@ export class StructuredAgentSessionStatusFeed { workspaceId: session.params.location.workspaceId, agent: session.params.provider, ...(session.hasProviderChild ? { hostExecutionOwned: true as const } : {}), - ...projectStructuredAgentSessionStatusSummary(items), + ...projectStructuredAgentSessionStatusSummary(items, submissions, session.fence), ...(record?.rewind?.phase === 'prepared' || record?.rewind?.phase === 'provider-succeeded' ? { rewindBlockedReason: 'outcome-unknown' as const } : {}), diff --git a/src/main/native-chat/agent-session-wire/structured-agent-session-surface-lifetime.test.ts b/src/main/native-chat/agent-session-wire/structured-agent-session-surface-lifetime.test.ts index 230e39cdaef..f4e02f4491b 100644 --- a/src/main/native-chat/agent-session-wire/structured-agent-session-surface-lifetime.test.ts +++ b/src/main/native-chat/agent-session-wire/structured-agent-session-surface-lifetime.test.ts @@ -8,6 +8,7 @@ import { tmpdir } from 'node:os' import { join } from 'node:path' import { afterEach, beforeEach, describe, expect, it, vi, type Mock } from 'vitest' import type { AgentSessionOwnerProbe } from '../../../shared/agent-session-lease-adjudication' +import { hasUnansweredStructuredAgentSessionDispatch } from '../../../shared/structured-agent-session-projection' import { computeAgentSessionPayloadFingerprint } from '../../../shared/agent-session-mutation-envelope' import type { AgentSessionMutationEnvelope, @@ -334,6 +335,11 @@ describe('an unexpected provider exit', () => { acquisitionGeneration: 'generation-1' }) + const recoveredHistory = host.history({ sessionId: SESSION, direction: 'tail' }) + expect( + recoveredHistory.ok && + hasUnansweredStructuredAgentSessionDispatch(recoveredHistory.page.submissions) + ).toBe(false) expect(acquire).toHaveBeenCalledTimes(2) expect(dispatch).toHaveBeenCalledOnce() expect(store.getRecord(SESSION)?.lease).toMatchObject({ diff --git a/src/main/native-chat/agent-session-wire/structured-agent-session-turns.ts b/src/main/native-chat/agent-session-wire/structured-agent-session-turns.ts index 3bd98a61735..ba83b7e54ac 100644 --- a/src/main/native-chat/agent-session-wire/structured-agent-session-turns.ts +++ b/src/main/native-chat/agent-session-wire/structured-agent-session-turns.ts @@ -97,6 +97,15 @@ export async function performSend( if (!(input.retryUnknown && existing?.dispatchState === 'unknown')) { await ctx.journal.appendSubmission({ ...input, fence: ctx.fence }) ctx.publish() + } else { + // Retry resumes work without moving or duplicating the original message. + await ctx.journal.resolveDispatch({ + clientMessageId: input.clientMessageId, + state: 'unknown', + reason: 'dispatch_retry_in_progress', + fence: ctx.fence + }) + ctx.publish() } const outcome = await dispatchSafely(ctx, input.clientMessageId, input.body) @@ -124,8 +133,7 @@ export async function performSend( clientMessageId: input.clientMessageId, state: 'unknown', reason: 'dispatch_result_persistence_failed', - fence: ctx.fence, - recovered: true + fence: ctx.fence }) } catch { // Nothing further to record; the pending row is settled on the next attach. diff --git a/src/main/native-chat/agent-session-wire/structured-agent-session-unexpected-exit.test.ts b/src/main/native-chat/agent-session-wire/structured-agent-session-unexpected-exit.test.ts index 084ffc7547d..2c2b9428a43 100644 --- a/src/main/native-chat/agent-session-wire/structured-agent-session-unexpected-exit.test.ts +++ b/src/main/native-chat/agent-session-wire/structured-agent-session-unexpected-exit.test.ts @@ -49,7 +49,11 @@ describe('provider-exit recovery tickets', () => { hasProviderChild: true, fence: 7, acquisitionGeneration: GENERATION, - journal: { snapshot: () => ({ items: [] }), appendLifecycleBatch } + journal: { + snapshot: () => ({ items: [] }), + appendLifecycleBatch, + markPendingSubmissionsUnknown: vi.fn(async () => []) + } } as unknown as StructuredAgentSessionHostSession const store = { getRecord: () => ({ @@ -89,6 +93,10 @@ describe('provider-exit recovery tickets', () => { expect(result).toMatchObject({ settlementRetryRequired: false, releasedFence: 8 }) expect(appendLifecycleBatch).toHaveBeenCalledOnce() + expect(session.journal.markPendingSubmissionsUnknown).toHaveBeenCalledWith( + 7, + 'provider_exited_before_acknowledgement' + ) expect(session.hasProviderChild).toBe(false) }) @@ -98,6 +106,7 @@ describe('provider-exit recovery tickets', () => { fence: 7, acquisitionGeneration: GENERATION, journal: { + markPendingSubmissionsUnknown: vi.fn(async () => []), snapshot: () => ({ items: [] }), appendLifecycleBatch: vi.fn(async () => { throw new Error('journal still unavailable') diff --git a/src/main/native-chat/agent-session-wire/structured-agent-session-unexpected-exit.ts b/src/main/native-chat/agent-session-wire/structured-agent-session-unexpected-exit.ts index ddc4af9f2d3..43ba7dbbb2d 100644 --- a/src/main/native-chat/agent-session-wire/structured-agent-session-unexpected-exit.ts +++ b/src/main/native-chat/agent-session-wire/structured-agent-session-unexpected-exit.ts @@ -81,6 +81,15 @@ export async function settleUnexpectedStructuredAgentSessionExit( settlementRetryRequired = true context.onBarrierError?.(unexpectedEvent.sessionId, error) } + try { + await session.journal.markPendingSubmissionsUnknown( + session.fence, + 'provider_exited_before_acknowledgement' + ) + } catch (error) { + settlementRetryRequired = true + context.onBarrierError?.(unexpectedEvent.sessionId, error) + } if (unexpectedEvent.settlementRetryRequired || settlementRetryRequired) { const retried = await retryUnexpectedExitSettlement({ context, @@ -170,6 +179,10 @@ export async function retryUnexpectedExitSettlement(input: { stableSettlementId: string }): Promise { try { + await input.session.journal.markPendingSubmissionsUnknown( + input.session.fence, + 'provider_exited_before_acknowledgement' + ) const mutations = unexpectedExitFallbackMutations( input.event, input.session, diff --git a/src/renderer/src/components/native-chat/NativeChatStructuredSession.tsx b/src/renderer/src/components/native-chat/NativeChatStructuredSession.tsx index da70c4b3446..a2b70db55b0 100644 --- a/src/renderer/src/components/native-chat/NativeChatStructuredSession.tsx +++ b/src/renderer/src/components/native-chat/NativeChatStructuredSession.tsx @@ -352,7 +352,9 @@ export function NativeChatStructuredSession( targetPtyId={null} agent={props.agent} canSend={!prompt} - isWorking={controller.isWorking} + // Stop, not status: only a provider-minted turn can be interrupted, so the button + // must not flip while a dispatch is still unanswered. + isWorking={controller.turnId !== null} onStop={() => { if (controller.turnId) { void controller.cancel(controller.turnId) diff --git a/src/renderer/src/components/native-chat/use-structured-agent-session.test.tsx b/src/renderer/src/components/native-chat/use-structured-agent-session.test.tsx index abf185cb913..5d8b291a957 100644 --- a/src/renderer/src/components/native-chat/use-structured-agent-session.test.tsx +++ b/src/renderer/src/components/native-chat/use-structured-agent-session.test.tsx @@ -10,6 +10,7 @@ const mocks = vi.hoisted(() => ({ })) let fence = 3 let sessionCommands: { name: string; kind: 'command' | 'skill' }[] | undefined +let submissions: AgentJournalSubmission[] = [] vi.mock('@/runtime/structured-agent-session-client', () => ({ callStructuredAgentSession: mocks.call @@ -25,7 +26,7 @@ vi.mock('./use-structured-agent-session-read', () => ({ fence, commands: sessionCommands, items: [], - submissions: [], + submissions, status: 'ready', error: null, hasOlder: false, @@ -47,6 +48,7 @@ vi.mock('./use-structured-agent-session-outbox', () => ({ }) })) +import type { AgentJournalSubmission } from '../../../../shared/agent-session-journal-types' import { applyNativeChatSessionOptionSettingsMutation, resolveStructuredLaunchSeedOptions @@ -95,10 +97,72 @@ const OPTIONS = { current: { model: 'gpt-live', effort: 'medium' } } +describe('useStructuredAgentSession working state', () => { + beforeEach(() => { + vi.clearAllMocks() + fence = 3 + submissions = [] + mocks.call.mockResolvedValue(null) + }) + + it('reports work from an unanswered dispatch, and keeps the turn id provider-minted', () => { + submissions = [ + { + clientMessageId: 'client-1', + fence: 3, + payloadFingerprint: 'fingerprint-1', + dispatchState: 'pending', + providerItemId: null, + reason: null, + submittedAt: 1, + resolvedAt: null + } + ] + const { result } = renderHook(() => + useStructuredAgentSession({ + sessionId: 'session-1', + agent: 'codex', + target: LOCAL_TARGET, + isVisible: true + }) + ) + + expect(result.current.isWorking).toBe(true) + // Only the provider can mint a cancellable turn, so Stop stays unavailable here. + expect(result.current.turnId).toBeNull() + }) + + it('reports no work once the dispatch resolves and no turn is running', () => { + submissions = [ + { + clientMessageId: 'client-1', + fence: 3, + payloadFingerprint: 'fingerprint-1', + dispatchState: 'accepted', + providerItemId: 'codex:thread-1:turn-1', + reason: null, + submittedAt: 1, + resolvedAt: 2 + } + ] + const { result } = renderHook(() => + useStructuredAgentSession({ + sessionId: 'session-1', + agent: 'codex', + target: LOCAL_TARGET, + isVisible: true + }) + ) + + expect(result.current.isWorking).toBe(false) + }) +}) + describe('useStructuredAgentSession options', () => { beforeEach(() => { vi.clearAllMocks() fence = 3 + submissions = [] mocks.operationId .mockReset() .mockReturnValueOnce('operation-1') 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 3befefa5edd..864b7e1f608 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 @@ -22,7 +22,10 @@ import { structuredAgentSessionOptionPicks, structuredAgentSessionOptionSnapshot } from '../../../../shared/structured-agent-session-options' -import { activeStructuredAgentSessionTurnId } from '../../../../shared/structured-agent-session-projection' +import { + activeStructuredAgentSessionTurnId, + hasUnansweredStructuredAgentSessionDispatch +} from '../../../../shared/structured-agent-session-projection' import type { RuntimeClientTarget } from '@/runtime/runtime-rpc-client' import { callStructuredAgentSession } from '@/runtime/structured-agent-session-client' import { useStructuredAgentSessionHold } from './use-structured-agent-session-hold' @@ -80,6 +83,10 @@ export function useStructuredAgentSession(args: { // Refresh options each turn to confirm which model the provider actually selected. const turnId = activeStructuredAgentSessionTurnId(state.items) + // A dispatch the provider has not answered is already work; Claude's running row trails the + // send by seconds, and only a provider-minted turn is cancellable, so the two stay separate. + const isWorking = + turnId !== null || hasUnansweredStructuredAgentSessionDispatch(state.submissions, state.fence) const turnActivity = useMemo( () => selectStructuredAgentTurnActivity(state.items, turnId, state.activity), [state.activity, state.items, turnId] @@ -210,7 +217,7 @@ export function useStructuredAgentSession(args: { send: (...input: Parameters) => !commandPending.current && outboxController.send(...input), retry: outboxController.retry, - isWorking: turnId !== null, + isWorking, turnActivity, ...backgroundTasksView, turnId, diff --git a/src/shared/agent-session-journal-schemas.ts b/src/shared/agent-session-journal-schemas.ts index 89c2d9f7293..bab70dd2044 100644 --- a/src/shared/agent-session-journal-schemas.ts +++ b/src/shared/agent-session-journal-schemas.ts @@ -186,7 +186,8 @@ export const AgentJournalSubmissionSchema = z.object({ providerItemId: z.string().nullable(), reason: z.string().nullable(), submittedAt: z.number(), - resolvedAt: z.number().nullable() + resolvedAt: z.number().nullable(), + recovered: z.literal(true).optional() }) export function isAdmissibleAgentJournalItemBody(value: unknown): value is AgentJournalItemBody { diff --git a/src/shared/agent-session-journal-types.ts b/src/shared/agent-session-journal-types.ts index c0074e015f5..c017a94b36e 100644 --- a/src/shared/agent-session-journal-types.ts +++ b/src/shared/agent-session-journal-types.ts @@ -193,6 +193,7 @@ export type AgentJournalDispatchState = (typeof AGENT_JOURNAL_DISPATCH_STATES)[n * the turn reads as delivery unconfirmed, never as sent and never as failed. */ export type AgentJournalSubmission = { clientMessageId: string + /** Execution fence of the latest dispatch attempt or recovery. */ fence: number payloadFingerprint: string dispatchState: AgentJournalDispatchState @@ -202,6 +203,9 @@ export type AgentJournalSubmission = { reason: string | null submittedAt: number resolvedAt: number | null + /** Set when crash reconciliation resolved the dispatch, not the provider. A live + * `unknown` is a send still outstanding; a recovered one outlived its writer. */ + recovered?: true } /** Durable answer to "did my send land?", keyed by client message id. Only an diff --git a/src/shared/structured-agent-session-projection.test.ts b/src/shared/structured-agent-session-projection.test.ts index 77980fa86cc..76cf6088cba 100644 --- a/src/shared/structured-agent-session-projection.test.ts +++ b/src/shared/structured-agent-session-projection.test.ts @@ -1,10 +1,11 @@ import { describe, expect, it } from 'vitest' import { AGENT_STATUS_MAX_FIELD_LENGTH } from './agent-status-field-normalization' -import type { AgentJournalRenderItem } from './agent-session-journal-types' +import type { AgentJournalRenderItem, AgentJournalSubmission } from './agent-session-journal-types' import { parsePaneKey } from './stable-pane-id' import { activeStructuredAgentSessionTurnId, hasPersistedStructuredAgentSessionTurn, + hasUnansweredStructuredAgentSessionDispatch, projectStructuredItemToNativeChat, projectStructuredAgentSessionStatus, projectStructuredAgentSessionStatusSummary, @@ -19,6 +20,22 @@ function item( return { itemId, sequence, revision: 1, observedAt: sequence, body } } +function submission( + clientMessageId: string, + dispatchState: AgentJournalSubmission['dispatchState'] +): AgentJournalSubmission { + return { + clientMessageId, + fence: 1, + payloadFingerprint: clientMessageId, + dispatchState, + providerItemId: null, + reason: null, + submittedAt: 1, + resolvedAt: dispatchState === 'pending' ? null : 2 + } +} + describe('structured agent session status projection', () => { it('reuses immutable item projections and refreshes revisions and resolved prompts', () => { const original = item('diff', 1, { @@ -139,6 +156,75 @@ describe('structured agent session status projection', () => { }) }) + it('reads a session as working while a dispatch is unanswered, before any lifecycle row', () => { + const asked = item('asked', 1, { + kind: 'message', + role: 'user', + blocks: [{ type: 'text', text: 'go' }] + }) + const pending = [submission('m1', 'pending')] + + expect(hasUnansweredStructuredAgentSessionDispatch(pending)).toBe(true) + expect(projectStructuredAgentSessionStatus([asked], pending)).toBe('working') + // The first send has no journalled message until the provider replays it. + expect(projectStructuredAgentSessionStatusSummary([], pending)).toEqual({ + status: 'working', + latestPrompt: '' + }) + expect(projectStructuredAgentSessionStatusSummary([asked], pending)).toEqual({ + status: 'working', + latestPrompt: 'go' + }) + }) + + it('does not resurrect old-host unknown work after its execution fence advances', () => { + const oldHostSubmission = { ...submission('m1', 'unknown'), fence: 2 } + expect(hasUnansweredStructuredAgentSessionDispatch([oldHostSubmission], 2)).toBe(true) + expect(hasUnansweredStructuredAgentSessionDispatch([oldHostSubmission], 3)).toBe(false) + }) + + it('recognizes recovery from an older host without the optional marker', () => { + expect( + hasUnansweredStructuredAgentSessionDispatch([ + { ...submission('m1', 'unknown'), reason: 'host_restarted_before_acknowledgement' } + ]) + ).toBe(false) + }) + + it('stops reading a resolved dispatch as work, and lets a pending prompt outrank it', () => { + const asked = item('asked', 1, { + kind: 'message', + role: 'user', + blocks: [{ type: 'text', text: 'go' }] + }) + const prompt = item('prompt', 2, { + kind: 'approval', + title: 'Run command?', + detail: null, + options: [{ id: 'yes', label: 'Allow' }], + resolution: { state: 'pending', selectedOptionId: null, resolvedBy: null, resolvedAt: null } + }) + + for (const state of ['accepted', 'rejected'] as const) { + expect(hasUnansweredStructuredAgentSessionDispatch([submission('m1', state)])).toBe(false) + expect(projectStructuredAgentSessionStatus([asked], [submission('m1', state)])).toBe('idle') + } + // The ack budget elapsing is a delivery answer, not an answer about the turn. + expect(hasUnansweredStructuredAgentSessionDispatch([submission('m1', 'unknown')])).toBe(true) + expect( + hasUnansweredStructuredAgentSessionDispatch([ + { ...submission('m1', 'unknown'), recovered: true } + ]) + ).toBe(false) + expect( + projectStructuredAgentSessionStatus([asked, prompt], [submission('m1', 'pending')]) + ).toBe('attention') + expect(projectStructuredAgentSessionStatusSummary([], [])).toEqual({ + status: null, + latestPrompt: '' + }) + }) + it('carries the running tool and the newest assistant prose the sidebar row shows', () => { const ask = item('ask', 1, { kind: 'message', diff --git a/src/shared/structured-agent-session-projection.ts b/src/shared/structured-agent-session-projection.ts index 1301f766e2b..a48537d05b7 100644 --- a/src/shared/structured-agent-session-projection.ts +++ b/src/shared/structured-agent-session-projection.ts @@ -5,6 +5,7 @@ import { } from './agent-status-field-normalization' import type { AgentJournalRenderItem, + AgentJournalSubmission, AgentJournalToolCallItem } from './agent-session-journal-types' import { @@ -175,6 +176,34 @@ export function hasPersistedStructuredAgentSessionTurn( ) } +/** + * A send the host has journaled that the provider has neither opened a turn for nor refused. + * + * Codex declares `turn/started` within ~150ms, but Claude's running row can only be written once + * the SDK echoes the user message back — a 3.4s median and 18s at p90 on real journals. Waiting + * on that echo to call a session working leaves the whole gap reading idle in the chat and in + * every session list, so the send itself is the evidence. + * + * `unknown` still counts: it only means the ack budget elapsed, which happens on 30% of Claude + * sends whose turn then arrives anyway, and delivery confidence is a separate question from + * whether work is owed. A recovered `unknown` does not — that one outlived the host generation + * that sent it, so there is nothing still running to report. + */ +export function hasUnansweredStructuredAgentSessionDispatch( + submissions: readonly AgentJournalSubmission[], + currentFence?: number | null +): boolean { + return submissions.some( + (submission) => + (currentFence == null || submission.fence >= currentFence) && + (submission.dispatchState === 'pending' || + (submission.dispatchState === 'unknown' && + submission.recovered !== true && + // Older hosts publish the recovery reason but omit the optional marker. + submission.reason !== 'host_restarted_before_acknowledgement')) + ) +} + export type StructuredAgentSessionProjectedStatus = 'working' | 'attention' | 'idle' export function structuredAgentSessionTabId(sessionId: string): string { @@ -182,7 +211,9 @@ export function structuredAgentSessionTabId(sessionId: string): string { } export function projectStructuredAgentSessionStatus( - items: readonly AgentJournalRenderItem[] + items: readonly AgentJournalRenderItem[], + submissions: readonly AgentJournalSubmission[] = [], + currentFence?: number | null ): StructuredAgentSessionProjectedStatus { if ( items.some( @@ -193,7 +224,10 @@ export function projectStructuredAgentSessionStatus( ) { return 'attention' } - return activeStructuredAgentSessionTurnId(items) ? 'working' : 'idle' + return activeStructuredAgentSessionTurnId(items) || + hasUnansweredStructuredAgentSessionDispatch(submissions, currentFence) + ? 'working' + : 'idle' } function messageProse(blocks: readonly NativeChatBlock[]): string { @@ -270,12 +304,19 @@ export type StructuredAgentSessionStatusProjection = { * the 8 KB body): a streamed reply re-projects on every journal checkpoint, so the frame * has to stay small even though the row only ever renders one line of it. */ export function projectStructuredAgentSessionStatusSummary( - items: readonly AgentJournalRenderItem[] + items: readonly AgentJournalRenderItem[], + submissions: readonly AgentJournalSubmission[] = [], + currentFence?: number | null ): StructuredAgentSessionStatusProjection { - if (!hasPersistedStructuredAgentSessionTurn(items)) { + // A first send has no journalled message until the provider replays it, so the pending + // dispatch is also what makes a brand-new session listable at all. + if ( + !hasPersistedStructuredAgentSessionTurn(items) && + !hasUnansweredStructuredAgentSessionDispatch(submissions, currentFence) + ) { return { status: null, latestPrompt: '' } } - const status = projectStructuredAgentSessionStatus(items) + const status = projectStructuredAgentSessionStatus(items, submissions, currentFence) const activeToolCall = status === 'working' ? activeStructuredAgentSessionToolCall(items) : null const toolName = activeToolCall ? normalizeOptionalField(activeToolCall.name, AGENT_STATUS_TOOL_NAME_MAX_LENGTH) From 4b4acf26a4775cfdde4258566683e41a6c276f5b Mon Sep 17 00:00:00 2001 From: Brennan Benson <79079362+brennanb2025@users.noreply.github.com> Date: Wed, 9 Sep 2026 21:50:31 -0700 Subject: [PATCH 2/8] fix(mobile): enable patch-free iOS text selection in native chat (#19769) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix(mobile): make every native-chat text node selectable Long-press selection worked on some chat text and not others. Markdown paragraphs — the default block for agent prose — were the one block type left out when headings, quotes, code, lists and table cells gained `selectable`, and tool result output, diff rows, the unloadable-image placeholder, permission/question bodies and the send-error banner never had it at all. Selection is now set on every content Text in the chat surface, on the outermost block Text so nested inline spans inherit it. Labels inside a Pressable (option rows, tool-line headers, buttons) are deliberately left alone: selection there would swallow the tap they exist for. Extracting MobileNativeChatEmptyState keeps the view under its max-lines cap and matches desktop, where NativeChatEmptyState is already its own component. Tests render each surface and assert selection on the block that carries the prose; both files were ablated against the unfixed source (4/10 and 3/5 red) so they pin the defect rather than the current behavior. * fix(mobile): support native text range selection on iOS * fix(mobile): remove persistent assistant message controls * fix(mobile): scope patch-free text selection to chat Use the stock react-native-uitextview dependency behind an iOS adapter and opt assistant Markdown into range selection only in native chat. Preserve the existing React Native Text behavior elsewhere and remove the persistent assistant controls. --------- Co-authored-by: Merge Sim --- mobile/package.json | 1 + mobile/pnpm-lock.yaml | 14 ++ mobile/src/components/MobileMarkdown.tsx | 155 ++++++++++---- .../components/MobileSelectableText.ios.tsx | 38 ++++ .../src/components/MobileSelectableText.tsx | 1 + .../mobile-markdown-selectable.test.tsx | 110 ++++++++++ .../mobile-selectable-text-ios.test.tsx | 198 ++++++++++++++++++ .../session/MobileNativeChatMessage.test.ts | 15 ++ .../src/session/MobileNativeChatMessage.tsx | 107 ++-------- .../src/session/MobileNativeChatToolRun.tsx | 3 - mobile/src/session/MobileNativeChatView.tsx | 27 +-- .../mobile-native-chat-message-styles.ts | 21 -- .../mobile-native-chat-message-text.test.ts | 28 +-- .../mobile-native-chat-message-text.ts | 12 -- 14 files changed, 503 insertions(+), 227 deletions(-) create mode 100644 mobile/src/components/MobileSelectableText.ios.tsx create mode 100644 mobile/src/components/MobileSelectableText.tsx create mode 100644 mobile/src/components/mobile-markdown-selectable.test.tsx create mode 100644 mobile/src/components/mobile-selectable-text-ios.test.tsx diff --git a/mobile/package.json b/mobile/package.json index 159028c978d..13f2f98acf5 100644 --- a/mobile/package.json +++ b/mobile/package.json @@ -57,6 +57,7 @@ "react-native-safe-area-context": "^5.7.0", "react-native-screens": "^4.24.0", "react-native-svg": "^15.15.4", + "react-native-uitextview": "2.2.0", "react-native-web": "^0.21.2", "react-native-webview": "13.16.2", "react-native-worklets": "^0.8.3", diff --git a/mobile/pnpm-lock.yaml b/mobile/pnpm-lock.yaml index d44e612bb73..7cebee89eec 100644 --- a/mobile/pnpm-lock.yaml +++ b/mobile/pnpm-lock.yaml @@ -136,6 +136,9 @@ importers: react-native-svg: specifier: ^15.15.4 version: 15.15.4(react-native@0.83.10(patch_hash=44876634a8efbb0f2c3f66cd4332be170ec821d1cbfc0264ac80680983e8513d)(@babel/core@7.29.7)(@react-native/metro-config@0.85.2(@babel/core@7.29.7))(@types/react@19.2.14)(react@19.2.8))(react@19.2.8) + react-native-uitextview: + specifier: 2.2.0 + version: 2.2.0(react-native@0.83.10(patch_hash=44876634a8efbb0f2c3f66cd4332be170ec821d1cbfc0264ac80680983e8513d)(@babel/core@7.29.7)(@react-native/metro-config@0.85.2(@babel/core@7.29.7))(@types/react@19.2.14)(react@19.2.8))(react@19.2.8) react-native-web: specifier: ^0.21.2 version: 0.21.2(react-dom@19.2.8(react@19.2.8))(react@19.2.8) @@ -6158,6 +6161,12 @@ packages: react: '*' react-native: '*' + react-native-uitextview@2.2.0: + resolution: {integrity: sha512-Vbv3cTAuyfkYrfsR2YKFsOd9OYgfys+IX5yvvYY7Wd6TrOd6FSGrX93xtQHjeLXV1ds6fDJcFJsuLi3S4Ypr8A==} + peerDependencies: + react: '*' + react-native: '*' + react-native-web@0.21.2: resolution: {integrity: sha512-SO2t9/17zM4iEnFvlu2DA9jqNbzNhoUP+AItkoCOyFmDMOhUnBBznBDCYN92fGdfAkfQlWzPoez6+zLxFNsZEg==} peerDependencies: @@ -14609,6 +14618,11 @@ snapshots: react-native: 0.83.10(patch_hash=44876634a8efbb0f2c3f66cd4332be170ec821d1cbfc0264ac80680983e8513d)(@babel/core@7.29.7)(@react-native/metro-config@0.85.2(@babel/core@7.29.7))(@types/react@19.2.14)(react@19.2.8) warn-once: 0.1.1 + react-native-uitextview@2.2.0(react-native@0.83.10(patch_hash=44876634a8efbb0f2c3f66cd4332be170ec821d1cbfc0264ac80680983e8513d)(@babel/core@7.29.7)(@react-native/metro-config@0.85.2(@babel/core@7.29.7))(@types/react@19.2.14)(react@19.2.8))(react@19.2.8): + dependencies: + react: 19.2.8 + react-native: 0.83.10(patch_hash=44876634a8efbb0f2c3f66cd4332be170ec821d1cbfc0264ac80680983e8513d)(@babel/core@7.29.7)(@react-native/metro-config@0.85.2(@babel/core@7.29.7))(@types/react@19.2.14)(react@19.2.8) + react-native-web@0.21.2(react-dom@19.2.8(react@19.2.8))(react@19.2.8): dependencies: '@babel/runtime': 7.29.2 diff --git a/mobile/src/components/MobileMarkdown.tsx b/mobile/src/components/MobileMarkdown.tsx index 2f5b52cfe18..7ecfab8c393 100644 --- a/mobile/src/components/MobileMarkdown.tsx +++ b/mobile/src/components/MobileMarkdown.tsx @@ -1,5 +1,22 @@ -import { Fragment, memo, useMemo, type ReactNode } from 'react' -import { Linking, Pressable, ScrollView, Text, View } from 'react-native' +import { MobileSelectableText } from './MobileSelectableText' +import { + Fragment, + createElement, + createContext, + memo, + useContext, + useMemo, + type ComponentType, + type ReactNode +} from 'react' +import { + Linking, + Pressable, + ScrollView, + Text as NativeText, + View, + type TextProps +} from 'react-native' import { normalizeMobileMarkdownPreviewHtml } from './mobile-markdown-preview-html' import { styles } from './mobile-markdown-styles' import { @@ -19,6 +36,8 @@ import { MermaidDiagram } from './pr-sidebar/MermaidDiagram' type Props = { content?: string fallback?: string + /** Enables iOS range selection for native-chat transcript prose. */ + rangeSelectable?: boolean /** Multiplier for prose font size (paragraphs, lists, quotes). Defaults to 1; * the chat view passes >1 so agent prose reads larger than the compact base. */ textScale?: number @@ -33,6 +52,12 @@ const MAX_TABLE_ROWS = 40 const MAX_TABLE_COLUMNS = 8 /** Prose base size — passed to MermaidDiagram fallback mono text. */ const MERMAID_BASE = 13 +const MarkdownTextContext = createContext>(NativeText) + +function MarkdownText(props: TextProps): React.JSX.Element { + const TextComponent = useContext(MarkdownTextContext) + return createElement(TextComponent, props) +} // Web/mail hrefs open the system handler; file-target hrefs (file: URIs and // scheme-less paths — the entire desktop file-link contract) go to onOpenFile. @@ -64,13 +89,13 @@ function renderTextRun( return segments.map((segment, segmentIndex) => { if (segment.type === 'file') { return ( - onOpenFile(segment.path)} > {segment.value} - + ) } return {segment.value} @@ -105,22 +130,34 @@ function renderInline(text: string, onOpenFile?: (pathText: string) => void): Re const link = token.match(/^\[([^\]]+)\]\(([^)]+)\)$/) if (image) { parts.push( - openMarkdownHref(image[2]!, onOpenFile)}> + openMarkdownHref(image[2]!, onOpenFile)} + > {image[1] || 'image'} - + ) } else if (link) { parts.push( - openMarkdownHref(link[2]!, onOpenFile)}> + openMarkdownHref(link[2]!, onOpenFile)} + > {link[1]} - + ) } else if (/^https?:\/\//i.test(token)) { const { url, trailing } = trimAutolinkTrailingPunctuation(token) parts.push( - openMarkdownHref(url, onOpenFile)}> + openMarkdownHref(url, onOpenFile)} + > {url} - + ) if (trailing) { parts.push({trailing}) @@ -129,38 +166,38 @@ function renderInline(text: string, onOpenFile?: (pathText: string) => void): Re const code = token.slice(1, -1) if (onOpenFile && isFilePathCodeSpan(code)) { parts.push( - onOpenFile(normalizeFilePath(code.trim()))} > {code} - + ) } else { parts.push( - + {code} - + ) } } else if (token.startsWith('~~')) { parts.push( - + {renderTextRun(token.slice(2, -2), `${key}i`, onOpenFile)} - + ) } else if (token.startsWith('**') || token.startsWith('__')) { parts.push( - + {renderTextRun(token.slice(2, -2), `${key}i`, onOpenFile)} - + ) } else { parts.push( - + {renderTextRun(token.slice(1, -1), `${key}i`, onOpenFile)} - + ) } } @@ -171,7 +208,13 @@ function renderInline(text: string, onOpenFile?: (pathText: string) => void): Re return parts } -function MobileMarkdownInner({ content, fallback = '', textScale = 1, onOpenFile }: Props) { +function MobileMarkdownContent({ + content, + fallback = '', + rangeSelectable = false, + textScale = 1, + onOpenFile +}: Props) { const text = content?.trim() ?? '' const previewText = useMemo(() => normalizeMobileMarkdownPreviewHtml(text), [text]) const blocks = useMemo(() => parseMobileMarkdown(previewText), [previewText]) @@ -181,30 +224,35 @@ function MobileMarkdownInner({ content, fallback = '', textScale = 1, onOpenFile const proseScale = scaled(13) const listScale = scaled(14) if (!text) { - return fallback ? {fallback} : null + return fallback ? ( + + {fallback} + + ) : null } const mermaidSourceOccurrences = new Map() + // Native-chat range selection is set on each block; nested inline spans inherit it. return ( {blocks.map((block, index) => { if (block.type === 'heading') { return ( - {renderInline(block.text, onOpenFile)} - + ) } if (block.type === 'quote') { return ( - + {renderInline(block.text, onOpenFile)} - + ) } @@ -225,10 +273,12 @@ function MobileMarkdownInner({ content, fallback = '', textScale = 1, onOpenFile } return ( - {block.language ? {block.language} : null} - + {block.language ? ( + {block.language} + ) : null} + {block.text} - + ) } @@ -239,10 +289,10 @@ function MobileMarkdownInner({ content, fallback = '', textScale = 1, onOpenFile style={styles.imageFrame} onPress={() => openMarkdownHref(block.url, onOpenFile)} > - {block.alt || 'Open image'} - + {block.alt || 'Open image'} + {block.url} - + ) } @@ -256,26 +306,30 @@ function MobileMarkdownInner({ content, fallback = '', textScale = 1, onOpenFile {visibleHeaders.map((header, cellIndex) => ( - + {renderInline(header, onOpenFile)} - + ))} {visibleRows.map((row, rowIndex) => ( {visibleHeaders.map((_, cellIndex) => ( - + {renderInline(row[cellIndex] ?? '', onOpenFile)} - + ))} ))} {hiddenRows > 0 || hiddenColumns > 0 ? ( - + {hiddenRows > 0 ? `${hiddenRows} more rows` : ''} {hiddenRows > 0 && hiddenColumns > 0 ? ' · ' : ''} {hiddenColumns > 0 ? `${hiddenColumns} more columns` : ''} - + ) : null} @@ -286,7 +340,7 @@ function MobileMarkdownInner({ content, fallback = '', textScale = 1, onOpenFile {block.items.map((item, itemIndex) => ( - + {item.checked == null ? block.ordered ? `${itemIndex + 1}.` @@ -294,10 +348,10 @@ function MobileMarkdownInner({ content, fallback = '', textScale = 1, onOpenFile : item.checked ? '[x]' : '[ ]'} - - + + {renderInline(item.text, onOpenFile)} - + ))} @@ -307,18 +361,31 @@ function MobileMarkdownInner({ content, fallback = '', textScale = 1, onOpenFile return } return ( - + {block.text.split('\n').map((line, lineIndex) => ( {lineIndex > 0 ? '\n' : null} {renderInline(line, onOpenFile)} ))} - + ) })} ) } +function MobileMarkdownInner(props: Props): React.JSX.Element | null { + const TextComponent = props.rangeSelectable ? MobileSelectableText : NativeText + return ( + + + + ) +} + export const MobileMarkdown = memo(MobileMarkdownInner) diff --git a/mobile/src/components/MobileSelectableText.ios.tsx b/mobile/src/components/MobileSelectableText.ios.tsx new file mode 100644 index 00000000000..74cb686e26e --- /dev/null +++ b/mobile/src/components/MobileSelectableText.ios.tsx @@ -0,0 +1,38 @@ +import { Children, Fragment, isValidElement, type ReactNode } from 'react' +import { StyleSheet, Text, UIManager, type TextProps } from 'react-native' +import { UITextView } from 'react-native-uitextview' + +// Older development clients can load this bundle before rebuilding their native views. +const hasRangeSelection = UIManager.hasViewManagerConfig('RNUITextView') + +function flattenFragments(children: ReactNode): ReactNode[] { + return ( + Children.map(children, (child) => + isValidElement<{ children?: ReactNode }>(child) && child.type === Fragment + ? flattenFragments(child.props.children) + : child + ) ?? [] + ) +} + +export function MobileSelectableText({ children, style, ...props }: TextProps): React.JSX.Element { + if (!hasRangeSelection) { + return ( + + {children} + + ) + } + + // The native span adapter otherwise maps numeric bold to semibold. + const textStyle = StyleSheet.flatten(style) + const nativeStyle = + textStyle?.fontWeight === '700' || textStyle?.fontWeight === 700 + ? { ...textStyle, fontWeight: 'bold' as const } + : style + return ( + + {flattenFragments(children)} + + ) +} diff --git a/mobile/src/components/MobileSelectableText.tsx b/mobile/src/components/MobileSelectableText.tsx new file mode 100644 index 00000000000..ddfffc37ee9 --- /dev/null +++ b/mobile/src/components/MobileSelectableText.tsx @@ -0,0 +1 @@ +export { Text as MobileSelectableText } from 'react-native' diff --git a/mobile/src/components/mobile-markdown-selectable.test.tsx b/mobile/src/components/mobile-markdown-selectable.test.tsx new file mode 100644 index 00000000000..2b42fc70715 --- /dev/null +++ b/mobile/src/components/mobile-markdown-selectable.test.tsx @@ -0,0 +1,110 @@ +import { createElement } from 'react' +import { act, create, type ReactTestRenderer } from 'react-test-renderer' +import { afterEach, describe, expect, it, vi } from 'vitest' +import { MobileMarkdown } from './MobileMarkdown' + +vi.mock('react-native', () => ({ + Linking: { openURL: () => Promise.resolve() }, + Pressable: 'Pressable', + ScrollView: 'ScrollView', + StyleSheet: { create: (styles: unknown) => styles, hairlineWidth: 1 }, + Text: 'Text', + View: 'View' +})) + +vi.mock('./pr-sidebar/MermaidDiagram', () => ({ MermaidDiagram: 'MermaidDiagram' })) + +type TestNode = { + type: string + props: Record + children: (TestNode | string)[] | null +} + +/** Every Text that is not nested inside another Text, paired with the full prose + * it renders. Nested inline spans inherit selection, so only these carry it. */ +function outermostTextNodes( + node: TestNode | string, + insideText = false +): { text: string; selectable: boolean }[] { + if (typeof node === 'string') { + return [] + } + const children = node.children ?? [] + if (node.type === 'Text' && !insideText) { + return [{ text: flattenText(node), selectable: node.props.selectable === true }] + } + return children.flatMap((child) => outermostTextNodes(child, insideText || node.type === 'Text')) +} + +function flattenText(node: TestNode | string): string { + if (typeof node === 'string') { + return node + } + return (node.children ?? []).map(flattenText).join('') +} + +function renderMarkdown(props: Parameters[0]): TestNode { + let renderer: ReactTestRenderer | null = null + act(() => { + renderer = create(createElement(MobileMarkdown, { rangeSelectable: true, ...props })) + }) + const tree = renderer!.toJSON() as unknown as TestNode + act(() => renderer!.unmount()) + return tree +} + +function selectableFor(tree: TestNode, needle: string): boolean { + const match = outermostTextNodes(tree).find((entry) => entry.text.includes(needle)) + if (!match) { + throw new Error(`no Text rendered "${needle}"`) + } + return match.selectable +} + +describe('MobileMarkdown selection', () => { + afterEach(() => vi.clearAllMocks()) + + // Paragraphs are the default block for agent prose, and were the one block + // type left non-selectable when the others gained it. + it.each([ + ['paragraph', 'Paragraph prose here.'], + ['heading', 'Heading prose'], + ['quote', 'Quote prose'], + ['code', 'const code = 1'], + ['list item', 'List item prose'], + ['table header', 'Head A'], + ['table cell', 'Cell A'] + ])('makes %s prose selectable', (_label, needle) => { + const content = [ + '# Heading prose', + '', + 'Paragraph prose here.', + '', + '> Quote prose', + '', + '```ts', + 'const code = 1', + '```', + '', + '- List item prose', + '', + '| Head A | Head B |', + '| --- | --- |', + '| Cell A | Cell B |' + ].join('\n') + expect(selectableFor(renderMarkdown({ content }), needle)).toBe(true) + }) + + it('makes the empty-content fallback selectable', () => { + const tree = renderMarkdown({ content: '', fallback: 'Fallback prose' }) + expect(selectableFor(tree, 'Fallback prose')).toBe(true) + }) + + it('keeps inline spans inside their selectable block rather than splitting it', () => { + const tree = renderMarkdown({ content: 'Prose with `code` and **bold** inline.' }) + const blocks = outermostTextNodes(tree) + expect(blocks).toHaveLength(1) + expect(blocks[0]!.selectable).toBe(true) + expect(blocks[0]!.text).toBe('Prose with code and bold inline.') + }) +}) diff --git a/mobile/src/components/mobile-selectable-text-ios.test.tsx b/mobile/src/components/mobile-selectable-text-ios.test.tsx new file mode 100644 index 00000000000..3e9ee710be4 --- /dev/null +++ b/mobile/src/components/mobile-selectable-text-ios.test.tsx @@ -0,0 +1,198 @@ +import { createElement, Fragment } from 'react' +import { act, create, type ReactTestRenderer } from 'react-test-renderer' +import { afterEach, describe, expect, it, vi } from 'vitest' + +const native = vi.hoisted(() => ({ available: true })) +vi.mock('react-native', () => ({ + Platform: { OS: 'ios' }, + UIManager: { hasViewManagerConfig: () => native.available }, + Linking: { openURL: vi.fn() }, + Text: 'Text', + View: 'View', + ScrollView: 'ScrollView', + Pressable: 'Pressable', + StyleSheet: { + create: (styles: unknown) => styles, + flatten: (style: unknown): object => + Array.isArray(style) + ? Object.assign({}, ...style.flat(Infinity).filter(Boolean)) + : (style ?? {}), + hairlineWidth: 1 + } +})) +vi.mock('react-native/Libraries/Utilities/codegenNativeComponent', () => ({ + default: (name: string) => name +})) +// Exercise the dependency's real span conversion without a native runtime. +vi.mock('react-native-uitextview', () => import('react-native-uitextview/src/Text')) +vi.mock('./MobileSelectableText', () => import('./MobileSelectableText.ios')) +vi.mock('./pr-sidebar/MermaidDiagram', () => ({ MermaidDiagram: 'MermaidDiagram' })) + +let renderer: ReactTestRenderer | undefined +afterEach(() => { + act(() => renderer?.unmount()) + renderer = undefined + native.available = true + vi.resetModules() + vi.restoreAllMocks() +}) + +function render(element: React.ReactElement): ReactTestRenderer { + act(() => { + renderer = create(element) + }) + return renderer! +} + +function nodes(tree: ReactTestRenderer, name: string) { + return tree.root.findAll((node) => node.type === name) +} + +describe('iOS selectable text boundary', () => { + it('preserves line-scoped keys when repeated inline spans update or disappear', async () => { + const errors = vi.spyOn(console, 'error').mockImplementation(() => {}) + const { MobileMarkdown } = await import('./MobileMarkdown') + const onOpenFile = vi.fn() + const line = '**same** [file](src/main.ts)' + const tree = render( + createElement(MobileMarkdown, { + content: `${line}\n${line}`, + rangeSelectable: true, + onOpenFile + }) + ) + expect( + nodes(tree, 'RNUITextViewChild') + .map((node) => node.props.text) + .join('') + ).toBe('same file\nsame file') + act(() => + tree.update( + createElement(MobileMarkdown, { + content: `${line}\n**changed** [file](src/main.ts)`, + rangeSelectable: true, + onOpenFile + }) + ) + ) + expect( + nodes(tree, 'RNUITextViewChild') + .map((node) => node.props.text) + .join('') + ).toBe('same file\nchanged file') + act(() => + tree.update( + createElement(MobileMarkdown, { content: line, rangeSelectable: true, onOpenFile }) + ) + ) + const spans = nodes(tree, 'RNUITextViewChild') + expect(spans.map((node) => node.props.text).join('')).toBe('same file') + act(() => spans.find((node) => node.props.text === 'file')!.props.onPress()) + expect(onOpenFile).toHaveBeenCalledExactlyOnceWith('src/main.ts') + expect(errors.mock.calls.filter((args) => String(args[0]).includes('same key'))).toEqual([]) + }) + + it.each([ + ['500', 'medium'], + ['600', 'semibold'], + ['700', 'bold'] + ] as const)('preserves font weight %s', async (fontWeight, expected) => { + const { MobileSelectableText: Text } = await import('./MobileSelectableText.ios') + const tree = render(createElement(Text, { selectable: true, style: { fontWeight } }, 'Weight')) + expect(nodes(tree, 'RNUITextViewChild')[0]!.props.style.fontWeight).toBe(expected) + }) + + it('keeps fragments, arrays, newlines and nested styles in one native root', async () => { + const { MobileSelectableText: Text } = await import('./MobileSelectableText.ios') + const tree = render( + createElement( + Text, + { selectable: true, style: { fontSize: 18 } }, + createElement(Fragment, null, 'Before ', ['one', '\n']), + createElement(Text, { style: { fontWeight: '700' } }, 'bold'), + createElement(Text, { style: { color: 'blue' } }, 'nested'), + ' after' + ) + ) + expect(nodes(tree, 'RNUITextView')).toHaveLength(1) + expect(nodes(tree, 'Text')).toHaveLength(0) + const spans = nodes(tree, 'RNUITextViewChild') + expect(spans.map((node) => node.props.text).join('')).toBe('Before one\nboldnested after') + expect(spans.find((node) => node.props.text === 'bold')?.props.style).toMatchObject({ + fontSize: 18, + fontWeight: 'bold' + }) + expect(spans.find((node) => node.props.text === 'nested')?.props.style).toMatchObject({ + fontSize: 18, + color: 'blue' + }) + }) + + it('preserves Markdown text, inline styles and file-link callbacks', async () => { + const { MobileMarkdown } = await import('./MobileMarkdown') + const onOpenFile = vi.fn() + const tree = render( + createElement(MobileMarkdown, { + content: 'Hello 😀 [src/main.ts](src/main.ts) and `code`.\nNext line.', + rangeSelectable: true, + onOpenFile + }) + ) + const spans = nodes(tree, 'RNUITextViewChild') + expect(spans.map((node) => node.props.text).join('')).toBe( + 'Hello 😀 src/main.ts and code.\nNext line.' + ) + const link = spans.find((node) => node.props.text === 'src/main.ts')! + expect(link.props.style.color).toBeDefined() + act(() => link.props.onPress()) + expect(onOpenFile).toHaveBeenCalledExactlyOnceWith('src/main.ts') + expect(nodes(tree, 'RNUITextView')).toHaveLength(1) + }) + + it('keeps ordinary button labels on React Native Text', async () => { + const { MobileSelectableText: Text } = await import('./MobileSelectableText.ios') + const tree = render(createElement(Text, null, 'Submit')) + expect(nodes(tree, 'RNUITextView')).toHaveLength(0) + expect(nodes(tree, 'Text')).toHaveLength(1) + }) + + it('uses native range selection only when Markdown opts in', async () => { + const { MobileMarkdown } = await import('./MobileMarkdown') + const tree = render(createElement(MobileMarkdown, { content: 'Transcript prose' })) + expect(nodes(tree, 'RNUITextView')).toHaveLength(0) + expect( + nodes(tree, 'Text').find((node) => node.children.includes('Transcript prose'))?.props + .selectable + ).toBe(false) + act(() => + tree.update( + createElement(MobileMarkdown, { content: 'Transcript prose', rangeSelectable: true }) + ) + ) + expect(nodes(tree, 'RNUITextView')).toHaveLength(1) + }) + + it('keeps code-language labels on styled React Native Text', async () => { + const { MobileMarkdown } = await import('./MobileMarkdown') + const tree = render( + createElement(MobileMarkdown, { + content: '```ts\nconst value = 1\n```', + rangeSelectable: true + }) + ) + const label = nodes(tree, 'Text').find((node) => node.children.join('') === 'ts')! + expect(label.props.style.textTransform).toBe('uppercase') + expect(nodes(tree, 'RNUITextView')).toHaveLength(1) + }) + + it('falls back for older clients without the native view', async () => { + native.available = false + const { MobileSelectableText: Text } = await import('./MobileSelectableText.ios') + const tree = render( + createElement(Text, { selectable: true }, 'Old client ', createElement(Text, null, 'inline')) + ) + expect(nodes(tree, 'RNUITextView')).toHaveLength(0) + expect(nodes(tree, 'Text')).toHaveLength(2) + expect(nodes(tree, 'Text')[0]!.props.selectable).toBe(true) + }) +}) diff --git a/mobile/src/session/MobileNativeChatMessage.test.ts b/mobile/src/session/MobileNativeChatMessage.test.ts index 677b76d240c..e09d1631a8d 100644 --- a/mobile/src/session/MobileNativeChatMessage.test.ts +++ b/mobile/src/session/MobileNativeChatMessage.test.ts @@ -106,6 +106,21 @@ describe('MobileNativeChatMessage', () => { expect(texts.some((text) => text.includes('/tmp/host.png'))).toBe(true) }) + it('makes user message text selectable', () => { + const tree = render(userMessage([{ type: 'text', text: 'Prompt I typed' }])) + const text = tree.root + .findAllByType('Text' as never) + .find((node) => String(node.children.join('')) === 'Prompt I typed') + expect(text?.props.selectable).toBe(true) + }) + + it('routes assistant prose through selectable Markdown', () => { + const tree = render(toolMessage([{ type: 'text', text: 'Agent reply prose' }])) + const markdown = tree.root.findByType('MobileMarkdown' as never) + expect(markdown.props.content).toBe('Agent reply prose') + expect(markdown.props.rangeSelectable).toBe(true) + }) + it('labels a tool row with the target path instead of raw input JSON', () => { const tree = render( toolMessage([{ type: 'tool-call', name: 'Read', input: { file_path: 'src/index.ts' } }]), diff --git a/mobile/src/session/MobileNativeChatMessage.tsx b/mobile/src/session/MobileNativeChatMessage.tsx index cc6c086b1f3..5d013688249 100644 --- a/mobile/src/session/MobileNativeChatMessage.tsx +++ b/mobile/src/session/MobileNativeChatMessage.tsx @@ -1,7 +1,6 @@ -import { memo, useEffect, useRef, useState } from 'react' -import { Image, Pressable, Text, View } from 'react-native' -import * as Clipboard from 'expo-clipboard' -import { ArrowUp, Copy } from 'lucide-react-native' +import { MobileSelectableText as Text } from '../components/MobileSelectableText' +import { memo } from 'react' +import { Image, Text as NativeText, View } from 'react-native' import { splitNativeChatBlocks } from '../../../src/shared/native-chat-tool-fold' import { selectActiveToolCall } from '../../../src/shared/native-chat-tool-activity' import { isImageRefBlock, isTextBlock } from '../../../src/shared/native-chat-types' @@ -10,10 +9,8 @@ import { MobileMarkdown } from '../components/MobileMarkdown' import { MobileNativeChatTurnStatus } from './MobileNativeChatTurnStatus' import { ToolRun } from './MobileNativeChatToolRun' import type { NativeChatTurnStatus } from './use-mobile-native-chat-turn-status' -import { colors } from '../theme/mobile-theme' import { isRenderableImageUri } from './mobile-native-chat-image-preview' import { styles, TEXT_SIZE } from './mobile-native-chat-message-styles' -import { nativeChatMessageText } from './mobile-native-chat-message-text' function Prose({ block, @@ -37,7 +34,12 @@ function Prose({ ) } return ( - + ) } if (isImageRefBlock(block)) { @@ -55,53 +57,18 @@ function Prose({ ) } return ( - + 🖼 {block.alt ?? block.path ?? block.url ?? 'image'} - + ) } return null } -/** Subtle top-right controls for an agent message: copy its prose, or scroll so - * this message's top aligns to the top of the viewport. */ -function AgentControls({ - onCopy, - onScrollToTop -}: { - onCopy: () => void - onScrollToTop?: () => void -}): React.JSX.Element { - return ( - - [styles.controlButton, pressed && styles.controlPressed]} - onPress={onCopy} - hitSlop={8} - accessibilityLabel="Copy message" - > - - - {onScrollToTop ? ( - [styles.controlButton, pressed && styles.controlPressed]} - onPress={onScrollToTop} - hitSlop={8} - accessibilityLabel="Scroll this message to top" - > - - - ) : null} - - ) -} - function MobileNativeChatMessageImpl({ message, toolsExpanded = false, fontScale = 1, - messageIndex, - onScrollToMessage, onOpenFile, turnStatus, turnExpanded, @@ -114,10 +81,6 @@ function MobileNativeChatMessageImpl({ toolsExpanded?: boolean /** Multiplies all chat text sizes for pinch-to-zoom (1 = no change). */ fontScale?: number - /** This message's index in the list, paired with onScrollToMessage. */ - messageIndex?: number - /** Ask the list to align this message's top to the top of the viewport. */ - onScrollToMessage?: (index: number) => void onOpenFile?: (relativePath: string) => void /** This turn's status row, rendered under a user message (desktop parity). */ turnStatus?: NativeChatTurnStatus | null @@ -134,18 +97,6 @@ function MobileNativeChatMessageImpl({ }): React.JSX.Element { const isUser = message.role === 'user' const isReasoning = message.role === 'reasoning' - const isAgent = !isUser - // Briefly tint the bubble to confirm a copy landed. - const [copied, setCopied] = useState(false) - const copyTimer = useRef | null>(null) - useEffect( - () => () => { - if (copyTimer.current) { - clearTimeout(copyTimer.current) - } - }, - [] - ) // Separate the agent's words from its tool activity: prose renders first, the // tool calls fold into a collapsible run beneath. The user's own messages get // an inverted (filled accent) bubble so they stand apart from agent prose. @@ -165,42 +116,11 @@ function MobileNativeChatMessageImpl({ !toolsExpanded const showToolRun = tools.length > 0 && !settledToolsHidden - const handleCopy = (): void => { - const text = nativeChatMessageText(message.blocks) - if (!text) { - return - } - void Clipboard.setStringAsync(text) - setCopied(true) - if (copyTimer.current) { - clearTimeout(copyTimer.current) - } - copyTimer.current = setTimeout(() => setCopied(false), 700) - } - - // Copy + scroll-to-top, shown inline with the first tool call (or after the - // prose when there are no tools). - const controls = isAgent ? ( - onScrollToMessage(messageIndex) - : undefined - } - /> - ) : null - return ( <> {prose.map((block, index) => ( - ) : controls ? ( - {controls} ) : null} diff --git a/mobile/src/session/MobileNativeChatToolRun.tsx b/mobile/src/session/MobileNativeChatToolRun.tsx index ccc732dab88..4dad714957c 100644 --- a/mobile/src/session/MobileNativeChatToolRun.tsx +++ b/mobile/src/session/MobileNativeChatToolRun.tsx @@ -172,7 +172,6 @@ export function ToolRun({ defaultExpanded, expandChildren, activeCall, - trailing, onOpenFile }: { blocks: NativeChatBlock[] @@ -181,7 +180,6 @@ export function ToolRun({ expandChildren: boolean /** The still-running call, when the turn is live (desktop parity). */ activeCall: ReturnType - trailing?: React.ReactNode onOpenFile?: (relativePath: string) => void }): React.JSX.Element { const [open, setOpen] = useState(defaultExpanded) @@ -230,7 +228,6 @@ export function ToolRun({ )} - {trailing} {open ? ( diff --git a/mobile/src/session/MobileNativeChatView.tsx b/mobile/src/session/MobileNativeChatView.tsx index f58e2ad05b0..b197faa9791 100644 --- a/mobile/src/session/MobileNativeChatView.tsx +++ b/mobile/src/session/MobileNativeChatView.tsx @@ -253,11 +253,6 @@ export function MobileNativeChatView({ [hasMore, loadingEarlier, onLoadEarlier] ) - // Align a single message's top to the top of the viewport. - const onScrollToMessage = useCallback((index: number) => { - listRef.current?.scrollToIndex({ index, viewPosition: 0, animated: true }) - }, []) - // Per-turn "Thinking / Working for N / Worked for N" rows. The structured lane // owns them; the bridge lane keeps its three-dot indicator. const turns = useMobileNativeChatTurnDisclosure({ @@ -273,15 +268,13 @@ export function MobileNativeChatView({ message={item} toolsExpanded={toolsExpanded} fontScale={fontScale} - messageIndex={index} - onScrollToMessage={onScrollToMessage} onOpenFile={onOpenFile} structuredActivityUi={structuredActivityUi} onToggleTurn={turns.onToggleTurn} {...turns.resolveRow(index, item)} /> ), - [toolsExpanded, fontScale, onScrollToMessage, onOpenFile, structuredActivityUi, turns] + [toolsExpanded, fontScale, onOpenFile, structuredActivityUi, turns] ) const emptyState = mobileNativeChatEmptyState(status, agent ?? null, error) @@ -325,21 +318,6 @@ export function MobileNativeChatView({ listRef.current?.scrollToEnd({ animated: false }) } }} - // scrollToIndex can fail before an off-screen row is measured — - // fall back to an estimated offset, then retry once it's laid out. - onScrollToIndexFailed={(info) => { - listRef.current?.scrollToOffset({ - offset: info.averageItemLength * info.index, - animated: true - }) - setTimeout(() => { - listRef.current?.scrollToIndex({ - index: info.index, - viewPosition: 0, - animated: true - }) - }, 120) - }} ListHeaderComponent={ hasMore ? ( - {/* Jump-to-latest control. The scroll-to-top affordance now lives - per-message (the up-arrow in each agent message's controls). */} + {/* Jump-to-latest control. */} {!atBottom ? ( { - it('joins text blocks and skips non-text blocks', () => { - const blocks: NativeChatBlock[] = [ - { type: 'text', text: 'Hello' }, - { type: 'tool-call', name: 'Read', input: {} }, - { type: 'text', text: 'World' } - ] - expect(nativeChatMessageText(blocks)).toBe('Hello\n\nWorld') - }) - - it('returns an empty string when there is no prose', () => { - const blocks: NativeChatBlock[] = [{ type: 'tool-call', name: 'Read', input: {} }] - expect(nativeChatMessageText(blocks)).toBe('') - }) - - it('trims surrounding whitespace', () => { - expect(nativeChatMessageText([{ type: 'text', text: ' hi ' }])).toBe('hi') - }) -}) +import { clampFontScale, FONT_SCALE_MAX, FONT_SCALE_MIN } from './mobile-native-chat-message-text' describe('clampFontScale', () => { it('clamps below the minimum', () => { diff --git a/mobile/src/session/mobile-native-chat-message-text.ts b/mobile/src/session/mobile-native-chat-message-text.ts index a94104835a9..84d2f8f1cc9 100644 --- a/mobile/src/session/mobile-native-chat-message-text.ts +++ b/mobile/src/session/mobile-native-chat-message-text.ts @@ -1,15 +1,3 @@ -import { isTextBlock, type NativeChatBlock } from '../../../src/shared/native-chat-types' - -/** Concatenate a message's text blocks into a single copyable string. Tool - * calls/results and image refs are skipped — Copy is for the agent's prose. */ -export function nativeChatMessageText(blocks: readonly NativeChatBlock[]): string { - return blocks - .filter(isTextBlock) - .map((b) => b.text) - .join('\n\n') - .trim() -} - /** Pinch-to-zoom font bounds. Default 1 means no visible change until pinched. */ export const FONT_SCALE_MIN = 0.8 export const FONT_SCALE_MAX = 1.8 From a067cccd38e38487772741ebf9d0ea4a8fed8644 Mon Sep 17 00:00:00 2001 From: Neil <4138956+nwparker@users.noreply.github.com> Date: Wed, 9 Sep 2026 22:06:31 -0700 Subject: [PATCH 3/8] perf(terminal): resume OSC terminator search past the carried frame (#19839) A single unterminated OSC 9999 marker split across many PTY chunks re-scanned the whole accumulation for a terminator on every chunk, so work grew with the square of the frame length. Carry how much of `pending` already failed the search and resume one character before it, which is enough for an `ESC \\` straddling the chunk boundary. --- ...status-osc-split-frame-scan-budget.test.ts | 75 +++++++++++++++++++ src/shared/agent-status-osc.ts | 11 ++- 2 files changed, 85 insertions(+), 1 deletion(-) create mode 100644 src/shared/agent-status-osc-split-frame-scan-budget.test.ts diff --git a/src/shared/agent-status-osc-split-frame-scan-budget.test.ts b/src/shared/agent-status-osc-split-frame-scan-budget.test.ts new file mode 100644 index 00000000000..fda2e9b8f3e --- /dev/null +++ b/src/shared/agent-status-osc-split-frame-scan-budget.test.ts @@ -0,0 +1,75 @@ +import { describe, expect, it, vi } from 'vitest' +import { createAgentStatusOscProcessor } from './agent-status-osc' + +/** Total characters swept by terminator/prefix searches across every chunk of a feed. */ +function feedWithScanBudget(chunks: string[]) { + let searchedChars = 0 + const indexOf = String.prototype.indexOf + const spy = vi.spyOn(String.prototype, 'indexOf').mockImplementation(function ( + this: string, + search, + from = 0 + ) { + const found = indexOf.call(this, search, from) + searchedChars += (found === -1 ? this.length : found + String(search).length) - Number(from) + return found + }) + try { + const process = createAgentStatusOscProcessor() + const results = chunks.map((chunk) => process(chunk)) + return { results, searchedChars } + } finally { + spy.mockRestore() + } +} + +describe('OSC 9999 split-frame scan budget', () => { + it('keeps per-chunk work flat as the split frame accumulates', () => { + // One unterminated marker whose payload arrives one character at a time. + const feedOf = (chunkCount: number): string[] => [ + '\x1b]9999;{"state":"working","prompt":"', + ...Array(chunkCount).fill('x') + ] + + const small = feedWithScanBudget(feedOf(2000)) + const large = feedWithScanBudget(feedOf(4000)) + + expect(small.results.every((result) => result.payloads.length === 0)).toBe(true) + // Re-scanning the accumulation would quadruple the budget when the feed doubles. + expect(large.searchedChars).toBeLessThan(small.searchedChars * 3) + }) + + it.each(['\x07', '\x1b\\'])( + 'matches whole-string parsing when split at every offset with terminator %j', + (terminator) => { + const stream = `head\x1b]9999;{"state":"working","prompt":"p"}${terminator}tail` + const whole = createAgentStatusOscProcessor()(stream) + + for (let split = 1; split < stream.length; split += 1) { + const process = createAgentStatusOscProcessor() + const first = process(stream.slice(0, split)) + const second = process(stream.slice(split)) + expect({ + cleanData: first.cleanData + second.cleanData, + payloads: [...first.payloads, ...second.payloads] + }).toEqual({ cleanData: whole.cleanData, payloads: whole.payloads }) + } + } + ) + + it('finds a string terminator straddling the resume boundary', () => { + const process = createAgentStatusOscProcessor() + // The ESC lands as the last character of the carried frame; the backslash arrives next. + expect(process('\x1b]9999;{"state":"working"}\x1b').payloads).toEqual([]) + expect(process('\\rest').payloads).toMatchObject([{ state: 'working' }]) + }) + + it('still parses a payload that completes many chunks later', () => { + const process = createAgentStatusOscProcessor() + process('\x1b]9999;{"state":"wor') + for (const chunk of ['k', 'i', 'n', 'g']) { + expect(process(chunk).payloads).toEqual([]) + } + expect(process('"}\x07done').payloads).toMatchObject([{ state: 'working' }]) + }) +}) diff --git a/src/shared/agent-status-osc.ts b/src/shared/agent-status-osc.ts index adb0c0eee47..3b13d2de5bc 100644 --- a/src/shared/agent-status-osc.ts +++ b/src/shared/agent-status-osc.ts @@ -57,6 +57,9 @@ function findAgentStatusTerminator( export function createAgentStatusOscProcessor(): (data: string) => ProcessedAgentStatusChunk { const MAX_PENDING = 64 * 1024 let pending = '' + // How much of `pending` already failed a terminator search, so a frame split across + // many chunks re-scans only the new bytes instead of the whole accumulation. + let pendingSearched = 0 return (data: string): ProcessedAgentStatusChunk => { // Ordinary terminal output is by far the common case. Keep it on the @@ -76,7 +79,9 @@ export function createAgentStatusOscProcessor(): (data: string) => ProcessedAgen } const combined = pending + data + const resumeFrom = pendingSearched pending = '' + pendingSearched = 0 const payloads: ParsedAgentStatusPayload[] = [] let lastPayloadCleanOffset: number | null = null @@ -100,12 +105,16 @@ export function createAgentStatusOscProcessor(): (data: string) => ProcessedAgen cleanData += combined.slice(cursor, start) const payloadStart = start + OSC_AGENT_STATUS_PREFIX.length - const terminator = findAgentStatusTerminator(combined, payloadStart, nextTerminator) + // Minus one so a `\x1b\\` straddling the previous chunk boundary is still found. + const searchFrom = + start === 0 && resumeFrom > 0 ? Math.max(payloadStart, resumeFrom - 1) : payloadStart + const terminator = findAgentStatusTerminator(combined, searchFrom, nextTerminator) if (terminator === null) { const candidate = combined.slice(start) // Own the frame so it stops pinning the consumed chunk it was sliced from. pending = candidate.length > MAX_PENDING ? '' : ownRetainedString(candidate) + pendingSearched = pending.length break } From 0b60b0dcb1215b7005a7320e6fbe45304f3abbfd Mon Sep 17 00:00:00 2001 From: Neil <4138956+nwparker@users.noreply.github.com> Date: Wed, 9 Sep 2026 22:12:52 -0700 Subject: [PATCH 4/8] perf(native-chat): bound retained items on the structured session path (#19841) `mergeSubmissions` caps submissions at 256, but `mergeItems` had no equivalent bound, so `state.items` grew for the whole life of a long structured session while the live path caps itself to its read window. Head-trim `items` to a retained-item limit when a live batch merges, and set `hasOlder` so anything trimmed is still reachable by paging. Paging older raises the limit to what the page produced, so a live batch slides the widened window instead of collapsing it back to the cap -- the same shape as the live path's growing `limitRef`. --- ...tured-agent-session-item-retention.test.ts | 152 ++++++++++++++++++ .../structured-agent-session-reducer.ts | 33 +++- 2 files changed, 179 insertions(+), 6 deletions(-) create mode 100644 src/shared/structured-agent-session-item-retention.test.ts diff --git a/src/shared/structured-agent-session-item-retention.test.ts b/src/shared/structured-agent-session-item-retention.test.ts new file mode 100644 index 00000000000..fe65b882596 --- /dev/null +++ b/src/shared/structured-agent-session-item-retention.test.ts @@ -0,0 +1,152 @@ +import { describe, expect, it } from 'vitest' +import type { AgentJournalRenderItem } from './agent-session-journal-types' +import type { AgentSessionHistoryPage } from './agent-session-wire' +import { + EMPTY_STRUCTURED_AGENT_SESSION, + oldestStructuredAgentSessionCursor, + reduceStructuredAgentSession, + type StructuredAgentSessionState +} from './structured-agent-session-reducer' + +const CAP = 1024 + +function item(sequence: number): AgentJournalRenderItem { + return { + itemId: `item-${sequence}`, + revision: 1, + sequence, + observedAt: sequence, + body: { kind: 'message', role: 'assistant', blocks: [{ type: 'text', text: `t-${sequence}` }] } + } +} + +function page(items: AgentJournalRenderItem[], hasOlder: boolean): AgentSessionHistoryPage { + const oldest = items[0]?.sequence ?? 0 + const newest = items.at(-1)?.sequence ?? 0 + return { + sessionId: 'session-a', + epoch: 'epoch-a', + direction: 'tail', + items, + removedItemIds: [], + submissions: [], + window: { + oldest: { epoch: 'epoch-a', sequence: oldest }, + newest: { epoch: 'epoch-a', sequence: newest }, + nextCursor: { epoch: 'epoch-a', sequence: oldest } + }, + liveCursor: { epoch: 'epoch-a', sequence: newest }, + hasOlder, + hasNewer: false + } +} + +function hydrate(items: AgentJournalRenderItem[], hasOlder = false): StructuredAgentSessionState { + return reduceStructuredAgentSession(EMPTY_STRUCTURED_AGENT_SESSION, { + type: 'event', + event: { type: 'snapshot', sessionId: 'session-a', fence: 1, page: page(items, hasOlder) } + }) +} + +function streamItems( + state: StructuredAgentSessionState, + sequences: number[] +): StructuredAgentSessionState { + return sequences.reduce( + (current, sequence) => + reduceStructuredAgentSession(current, { + type: 'event', + event: { + type: 'batch', + sessionId: 'session-a', + batch: { + cursor: { epoch: 'epoch-a', sequence }, + items: [item(sequence)], + removedItemIds: [], + submissions: [] + } + } + }), + state + ) +} + +describe('structured agent session item retention', () => { + it('bounds retained items on a long live session', () => { + const streamed = streamItems( + hydrate([item(0)]), + Array.from({ length: CAP + 500 }, (_, index) => index + 1) + ) + + expect(streamed.items).toHaveLength(CAP) + expect(streamed.items.at(-1)?.sequence).toBe(CAP + 500) + expect(streamed.items[0]?.sequence).toBe(501) + }) + + it('offers paging for items the cap dropped', () => { + const streamed = streamItems( + hydrate([item(0)]), + Array.from({ length: CAP + 10 }, (_, index) => index + 1) + ) + + expect(streamed.hasOlder).toBe(true) + expect(oldestStructuredAgentSessionCursor(streamed)).toEqual({ + epoch: 'epoch-a', + sequence: streamed.items[0]?.sequence + }) + }) + + it('leaves a session under the cap untouched', () => { + const hydrated = hydrate([item(0)]) + const streamed = streamItems( + hydrated, + Array.from({ length: 200 }, (_, index) => index + 1) + ) + + expect(streamed.items).toHaveLength(201) + expect(streamed.hasOlder).toBe(false) + }) + + it('widens the retained window when older items are paged in', () => { + const streamed = streamItems( + hydrate([item(1_000)], true), + Array.from({ length: CAP + 10 }, (_, index) => index + 1_001) + ) + const older = reduceStructuredAgentSession(streamed, { + type: 'older-page', + requestedEpoch: 'epoch-a', + page: page( + Array.from({ length: 300 }, (_, index) => item(index + 700)), + true + ) + }) + expect(older.items).toHaveLength(CAP + 300) + + // A live batch slides the widened window by one instead of collapsing it back to the cap. + const afterLive = streamItems(older, [3_000]) + + expect(afterLive.items).toHaveLength(CAP + 300) + expect(afterLive.items[0]?.sequence).toBe(701) + expect(afterLive.items.some((entry) => entry.sequence === 800)).toBe(true) + }) + + it('keeps item identity stable when a batch carries no journal change', () => { + const hydrated = hydrate([item(0)]) + const unchanged = reduceStructuredAgentSession(hydrated, { + type: 'event', + event: { + type: 'batch', + sessionId: 'session-a', + fence: 2, + batch: { + cursor: { epoch: 'epoch-a', sequence: 0 }, + items: [], + removedItemIds: [], + submissions: [] + } + } + }) + + expect(unchanged.items).toBe(hydrated.items) + }) +}) diff --git a/src/shared/structured-agent-session-reducer.ts b/src/shared/structured-agent-session-reducer.ts index 10897c3f61f..6397ca0c8c4 100644 --- a/src/shared/structured-agent-session-reducer.ts +++ b/src/shared/structured-agent-session-reducer.ts @@ -18,6 +18,8 @@ export type StructuredAgentSessionState = { fence: number | null items: AgentJournalRenderItem[] submissions: AgentJournalSubmission[] + /** Head-trim floor for `items`; paging back raises it so a live batch cannot undo the page. */ + retainedItemLimit: number hasOlder: boolean status: 'idle' | 'loading' | 'ready' | 'error' error?: string @@ -35,19 +37,23 @@ export type StructuredAgentSessionAction = | { type: 'tail-page'; page: AgentSessionHistoryPage } | { type: 'older-page'; requestedEpoch: string; page: AgentSessionHistoryPage } +const MAX_RETAINED_SUBMISSIONS = 256 +// Well above the renderer's initial read window (300) plus a page, so only genuinely +// long live sessions trim; anything trimmed is still reachable by paging older. +const MAX_RETAINED_ITEMS = 1024 + export const EMPTY_STRUCTURED_AGENT_SESSION: StructuredAgentSessionState = { epoch: null, cursor: null, fence: null, items: [], submissions: [], + retainedItemLimit: MAX_RETAINED_ITEMS, hasOlder: false, status: 'idle', handoff: null } -const MAX_RETAINED_SUBMISSIONS = 256 - function backgroundTaskStatesEqual( left: AgentSessionBackgroundTaskState | null | undefined, right: AgentSessionBackgroundTaskState | null | undefined @@ -91,6 +97,7 @@ function replacePage( fence, items: [...page.items].sort((left, right) => left.sequence - right.sequence), submissions: page.submissions, + retainedItemLimit: Math.max(MAX_RETAINED_ITEMS, page.items.length), hasOlder: page.hasOlder, status: 'ready', handoff: handoff ?? null, @@ -121,6 +128,13 @@ function mergeItems( return [...byId.values()].sort((left, right) => left.sequence - right.sequence) } +function trimRetainedItems( + items: AgentJournalRenderItem[], + limit: number +): AgentJournalRenderItem[] { + return items.length <= limit ? items : items.slice(items.length - limit) +} + function mergeSubmissions( current: readonly AgentJournalSubmission[], incoming: readonly AgentJournalSubmission[] @@ -186,6 +200,7 @@ export function reduceStructuredAgentSession( submissions: sameEpoch ? mergeSubmissions(state.submissions, action.page.submissions) : action.page.submissions, + retainedItemLimit: Math.max(MAX_RETAINED_ITEMS, action.page.items.length), hasOlder: action.page.hasOlder, status: 'ready', handoff: state.handoff, @@ -202,9 +217,11 @@ export function reduceStructuredAgentSession( if (state.epoch !== action.requestedEpoch || action.page.epoch !== action.requestedEpoch) { return state } + const paged = mergeItems(state.items, action.page.items, action.page.removedItemIds) return { ...state, - items: mergeItems(state.items, action.page.items, action.page.removedItemIds), + items: paged, + retainedItemLimit: Math.max(state.retainedItemLimit, paged.length), submissions: mergeSubmissions(state.submissions, action.page.submissions), hasOlder: action.page.hasOlder } @@ -246,13 +263,17 @@ export function reduceStructuredAgentSession( ) { return state } + const merged = journalUnchanged + ? state.items + : mergeItems(state.items, event.batch.items, event.batch.removedItemIds) + const items = trimRetainedItems(merged, state.retainedItemLimit) return { ...state, cursor: event.batch.cursor, fence: event.fence ?? state.fence, - items: journalUnchanged - ? state.items - : mergeItems(state.items, event.batch.items, event.batch.removedItemIds), + items, + // A trim leaves older items behind the cursor, so paging must stay offered. + hasOlder: items.length < merged.length ? true : state.hasOlder, submissions: journalUnchanged ? state.submissions : mergeSubmissions(state.submissions, event.batch.submissions), From 34790ce0843affb73bb6340c9189b5b77e04a06b Mon Sep 17 00:00:00 2001 From: Neil <4138956+nwparker@users.noreply.github.com> Date: Wed, 9 Sep 2026 22:14:56 -0700 Subject: [PATCH 5/8] perf(terminal): share one ESC dispatch table between the two scanners (#19842) The partial-tail state machine and the preview normalizer's per-sequence parser each carried their own copy of the byte-after-ESC table, so the DCS/SOS/PM/APC set was stated twice and could drift. Move it to `terminal-escape-introducer.ts` and have both read it. The normalizer now classifies from `charCodeAt` instead of `value[i]`, which drops the one-char string it minted on every escape it parsed -- the same allocation the ground scan already avoids. No behaviour change: `parseAnsiControlSequence` matches its previous implementation on every 2- and 3-byte sequence after ESC and on 20k random control-dense streams (differential check, not committed). --- .../runtime/terminal-ansi-normalization.ts | 14 +++--- src/shared/terminal-escape-introducer.test.ts | 46 +++++++++++++++++++ src/shared/terminal-escape-introducer.ts | 32 +++++++++++++ src/shared/terminal-partial-escape-tail.ts | 31 ++++++------- 4 files changed, 99 insertions(+), 24 deletions(-) create mode 100644 src/shared/terminal-escape-introducer.test.ts create mode 100644 src/shared/terminal-escape-introducer.ts diff --git a/src/main/runtime/terminal-ansi-normalization.ts b/src/main/runtime/terminal-ansi-normalization.ts index df76da3c35e..e1bbe33a79d 100644 --- a/src/main/runtime/terminal-ansi-normalization.ts +++ b/src/main/runtime/terminal-ansi-normalization.ts @@ -1,4 +1,5 @@ import { MAX_TAIL_PENDING_ANSI_CHARS } from './terminal-tail-limits' +import { classifyTerminalEscapeIntroducer } from '../../shared/terminal-escape-introducer' import { ownRetainedString } from '../../shared/own-retained-string' export function parseAnsiControlSequence( @@ -11,8 +12,9 @@ export function parseAnsiControlSequence( endIndex: number } | null { - const introducer = value[escapeIndex + 1] - if (introducer === '[') { + // charCodeAt, not value[i]: indexing mints a one-char string on every escape. + const introducer = classifyTerminalEscapeIntroducer(value.charCodeAt(escapeIndex + 1)) + if (introducer === 'csi') { for (let index = escapeIndex + 2; index < value.length; index += 1) { const code = value.charCodeAt(index) if (code < 0x40 || code > 0x7e) { @@ -30,7 +32,7 @@ export function parseAnsiControlSequence( } return null } - if (introducer === ']') { + if (introducer === 'osc') { for (let index = escapeIndex + 2; index < value.length; index += 1) { if (value[index] === '\u0007') { return { kind: 'other', endIndex: index } @@ -41,7 +43,7 @@ export function parseAnsiControlSequence( } return null } - if (isStTerminatedStringControlIntroducer(introducer)) { + if (introducer === 'string') { for (let index = escapeIndex + 2; index < value.length; index += 1) { if (value[index] === '\u001b' && value[index + 1] === '\\') { return { kind: 'other', endIndex: index + 1 } @@ -52,10 +54,6 @@ export function parseAnsiControlSequence( return { kind: 'other', endIndex: escapeIndex + 1 } } -function isStTerminatedStringControlIntroducer(introducer: string | undefined): boolean { - return introducer === 'P' || introducer === 'X' || introducer === '^' || introducer === '_' -} - export function hasCanonicalNumericCsiParams(params: string): boolean { return /^[0-9;]*$/.test(params) } diff --git a/src/shared/terminal-escape-introducer.test.ts b/src/shared/terminal-escape-introducer.test.ts new file mode 100644 index 00000000000..2fa3bfc662e --- /dev/null +++ b/src/shared/terminal-escape-introducer.test.ts @@ -0,0 +1,46 @@ +import { describe, expect, it } from 'vitest' +import { + classifyTerminalEscapeIntroducer, + type TerminalEscapeIntroducer +} from './terminal-escape-introducer' + +/** Independent restatement of the VT500 dispatch table, written from the ranges rather + * than the implementation, so a reordered branch in the real one shows up here. */ +function expected(code: number): TerminalEscapeIntroducer { + if ('[' === String.fromCharCode(code)) { + return 'csi' + } + if (']' === String.fromCharCode(code)) { + return 'osc' + } + if ('PX^_'.includes(String.fromCharCode(code))) { + return 'string' + } + if (String.fromCharCode(code) >= ' ' && String.fromCharCode(code) <= '/') { + return 'intermediate' + } + if (code < 0x20 || code === 0x7f) { + return 'execute' + } + return 'final' +} + +describe('terminal escape introducer', () => { + it('classifies every single-byte introducer the way the VT500 table does', () => { + for (let code = 0; code <= 0xff; code += 1) { + expect([code, classifyTerminalEscapeIntroducer(code)]).toEqual([code, expected(code)]) + } + }) + + it('opens ST-terminated strings for DCS, SOS, PM and APC only', () => { + const strings = Array.from({ length: 0x100 }, (_, code) => code).filter( + (code) => classifyTerminalEscapeIntroducer(code) === 'string' + ) + expect(strings.map((code) => String.fromCharCode(code))).toEqual(['P', 'X', '^', '_']) + }) + + it('reads a missing byte (ESC at end of input) as a completed two-byte sequence', () => { + // `charCodeAt` past the end is NaN; callers rely on that not becoming csi/osc/string. + expect(classifyTerminalEscapeIntroducer(''.charCodeAt(0))).toBe('final') + }) +}) diff --git a/src/shared/terminal-escape-introducer.ts b/src/shared/terminal-escape-introducer.ts new file mode 100644 index 00000000000..850300c91a1 --- /dev/null +++ b/src/shared/terminal-escape-introducer.ts @@ -0,0 +1,32 @@ +// The byte after ESC decides which VT500 sequence just opened. Two scanners need that +// decision -- the partial-tail state machine (`terminal-partial-escape-tail.ts`) and the +// preview normalizer's per-sequence parser (`terminal-ansi-normalization.ts`) -- and they +// must agree on the DCS/SOS/PM/APC set, so the table lives here rather than in each. + +export type TerminalEscapeIntroducer = + | 'csi' // [ + | 'osc' // ] + | 'string' // P / X / ^ / _ open DCS / SOS / PM / APC -- ST-terminated + | 'intermediate' // 0x20-0x2f, more bytes to come before the final + | 'execute' // C0 executes / DEL is ignored; the ESC is still pending + | 'final' // completes a two-byte sequence (ESC 7, ESC 8, ESC c, ...) + +/** Classifies the code unit after ESC. `NaN` (ESC at end of input) reads as `final`. */ +export function classifyTerminalEscapeIntroducer(code: number): TerminalEscapeIntroducer { + if (code === 0x5b) { + return 'csi' + } + if (code === 0x5d) { + return 'osc' + } + if (code === 0x50 || code === 0x58 || code === 0x5e || code === 0x5f) { + return 'string' + } + if (code >= 0x20 && code <= 0x2f) { + return 'intermediate' + } + if (code < 0x20 || code === 0x7f) { + return 'execute' + } + return 'final' +} diff --git a/src/shared/terminal-partial-escape-tail.ts b/src/shared/terminal-partial-escape-tail.ts index 1a5e2dfc613..53cfd3c17b4 100644 --- a/src/shared/terminal-partial-escape-tail.ts +++ b/src/shared/terminal-partial-escape-tail.ts @@ -6,6 +6,8 @@ // partial sequence at the ingest boundary lets snapshot producers append it // after the serialized screen so the continuation completes exactly as live. +import { classifyTerminalEscapeIntroducer } from './terminal-escape-introducer' + // Mirrors the VT500 parser states that can span a chunk boundary. C0 controls // (except ESC/CAN/SUB) execute mid-sequence without aborting it, matching // xterm's state machine. @@ -32,23 +34,20 @@ export const MAX_PARTIAL_ESCAPE_TAIL_LENGTH = 4096 /** ESC-state transition shared by the fresh-ESC and abort-reprocess paths. */ function stateAfterEscByte(code: number): ScanState { - if (code === 0x5b) { - return 'csi' // [ + switch (classifyTerminalEscapeIntroducer(code)) { + case 'csi': + return 'csi' + case 'osc': + return 'osc' + case 'string': + return 'string' + case 'intermediate': + return 'escIntermediate' + case 'execute': + return 'esc' // the ESC is still pending; a further ESC arrives via the callers + case 'final': + return 'ground' } - if (code === 0x5d) { - return 'osc' // ] - } - // P / X / ^ / _ open DCS / SOS / PM / APC — ST-terminated strings. - if (code === 0x50 || code === 0x58 || code === 0x5e || code === 0x5f) { - return 'string' - } - if (code >= 0x20 && code <= 0x2f) { - return 'escIntermediate' - } - if (code < 0x20 || code === 0x7f) { - return 'esc' // C0 executes / DEL is ignored mid-sequence; ESC via callers - } - return 'ground' // final byte — two-byte sequence (ESC 7, ESC 8, ESC c, …) } /** Returns the trailing incomplete escape sequence of `stream` ('' when the From dda103d2cf69dbdd730d0a7179bb3021b6467501 Mon Sep 17 00:00:00 2001 From: Brennan Benson <79079362+brennanb2025@users.noreply.github.com> Date: Wed, 9 Sep 2026 23:21:57 -0700 Subject: [PATCH 6/8] fix(native-chat): one `/` picker for every agent, anywhere in the prompt (#19832) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix(native-chat): one `/` picker for every agent, anywhere in the prompt The composer only opened its picker when `/` was the first character of the draft, so a skill named mid-sentence ("validate it with /electron") offered nothing. Codex was worse: its `/` menu listed commands only, and skills lived on a separate `$` trigger, so the prompt box behaved differently per agent. `/` is now the whole composer grammar. It opens one grouped commands+skills menu for every agent with a known grammar, both at the start of the draft and mid-prompt after whitespace. The `$` trigger is gone. Per-agent invocation is preserved where it belongs — in what a pick writes. Each row carries its own token, so choosing a skill in Codex inserts `$electron` while Claude inserts `/electron`, and the text that reaches the agent stays the text that agent actually invokes. Only a draft-leading command is dispatchable; picking one mid-sentence completes the token instead of sending the command on its own and discarding the draft. Name collisions now key on whether both kinds share a sigil, so a Codex `/review` command and a `$review` skill stay separate rows. * test(native-chat): model dismissal inputs on the live `/` grammar The trigger-key swap cases still used `$:4` keys. editReplacesTriggerToken is sigil-agnostic so they passed, but they modelled an input the composer can no longer produce. * test(native-chat): pin inline picker dispatch and discovery reuse --------- Co-authored-by: Merge Sim --- .../NativeChatAutocompleteMenus.test.tsx | 3 + .../NativeChatAutocompleteMenus.tsx | 12 +- .../native-chat/NativeChatComposerField.tsx | 13 +- .../native-chat-composer-state.test.ts | 201 ++++++++++++++---- .../native-chat/native-chat-composer-state.ts | 86 ++++---- .../native-chat/native-chat-picker-items.ts | 54 +++-- .../use-native-chat-composer-catalog.test.tsx | 1 + .../use-native-chat-composer-keydown.test.tsx | 44 +++- .../use-native-chat-composer-keydown.ts | 6 +- ...tive-chat-picker-command-dispatch.test.tsx | 1 + .../use-native-chat-picker-state.ts | 20 +- .../use-native-chat-skills.react.test.tsx | 24 +++ src/shared/native-chat-agent-profiles.test.ts | 9 +- src/shared/native-chat-agent-profiles.ts | 5 - 14 files changed, 342 insertions(+), 137 deletions(-) diff --git a/src/renderer/src/components/native-chat/NativeChatAutocompleteMenus.test.tsx b/src/renderer/src/components/native-chat/NativeChatAutocompleteMenus.test.tsx index d5dac1efab5..88506c51838 100644 --- a/src/renderer/src/components/native-chat/NativeChatAutocompleteMenus.test.tsx +++ b/src/renderer/src/components/native-chat/NativeChatAutocompleteMenus.test.tsx @@ -13,6 +13,7 @@ function autocomplete( query: '', triggerKey: '/:0', prefix: '/', + dispatchable: true, grouped: true, commandsEnabled: true, skillsEnabled: true, @@ -21,6 +22,7 @@ function autocomplete( kind: 'command', id: 'command:clear', name: 'clear', + token: '/clear', description: 'Clear history', skillCollision: false }, @@ -28,6 +30,7 @@ function autocomplete( kind: 'skill', id: 'skill:browser', name: 'browser', + token: '/browser', description: 'Use a browser', sources: [{ sourceKind: 'repo', skillFilePath: '/repo/browser/SKILL.md' }] } diff --git a/src/renderer/src/components/native-chat/NativeChatAutocompleteMenus.tsx b/src/renderer/src/components/native-chat/NativeChatAutocompleteMenus.tsx index 8357931fb49..7f1cd943c0f 100644 --- a/src/renderer/src/components/native-chat/NativeChatAutocompleteMenus.tsx +++ b/src/renderer/src/components/native-chat/NativeChatAutocompleteMenus.tsx @@ -12,7 +12,7 @@ export const NativeChatPickerMenu = memo(function NativeChatPickerMenu({ onChoose, onRetry }: { - autocomplete: Extract + autocomplete: Extract activeIndex: number listboxId: string onChoose: (item: NativeChatPickerItem) => void @@ -54,7 +54,6 @@ export const NativeChatPickerMenu = memo(function NativeChatPickerMenu({ + autocomplete: Extract ): string { - if (autocomplete.mode === 'skill' || !autocomplete.commandsEnabled) { + if (!autocomplete.commandsEnabled) { return translate('components.native-chat.composer.noSkills', 'No matching skills') } if (autocomplete.skillsEnabled) { @@ -174,7 +172,6 @@ function PickerStatus({ children }: { children: React.ReactNode }): React.JSX.El function PickerOption({ item, - prefix, index, activeIndex, listboxId, @@ -182,7 +179,6 @@ function PickerOption({ onChoose }: { item: NativeChatPickerItem - prefix: '/' | '$' index: number activeIndex: number listboxId: string @@ -213,7 +209,7 @@ function PickerOption({ ) : null} - {prefix + item.name} + {item.token} {item.description ? ( {item.description} ) : null} diff --git a/src/renderer/src/components/native-chat/NativeChatComposerField.tsx b/src/renderer/src/components/native-chat/NativeChatComposerField.tsx index 6b474a2f267..f51bf4c5400 100644 --- a/src/renderer/src/components/native-chat/NativeChatComposerField.tsx +++ b/src/renderer/src/components/native-chat/NativeChatComposerField.tsx @@ -166,7 +166,7 @@ export function NativeChatComposerField({ {/* Extra bottom padding keeps the input box off the window rim. */}
- {autocomplete.mode === 'slash' || autocomplete.mode === 'skill' ? ( + {autocomplete.mode === 'slash' ? ( 0 + autocomplete.mode === 'slash' && autocomplete.items.length > 0 ? `${pickerListboxId}-option-${Math.min(activeSuggestion, autocomplete.items.length - 1)}` : undefined } diff --git a/src/renderer/src/components/native-chat/native-chat-composer-state.test.ts b/src/renderer/src/components/native-chat/native-chat-composer-state.test.ts index 483d29f1ff2..9d116d373fb 100644 --- a/src/renderer/src/components/native-chat/native-chat-composer-state.test.ts +++ b/src/renderer/src/components/native-chat/native-chat-composer-state.test.ts @@ -9,6 +9,7 @@ import { editReplacesTriggerToken, EMPTY_HISTORY, filterSlashCommands, + isSkillPickerTriggered, isSlashCommandDraft, pushHistory, recallNext, @@ -96,28 +97,30 @@ describe('deriveComposerAutocomplete — mention', () => { }) }) -describe('deriveComposerAutocomplete — skill', () => { +describe('deriveComposerAutocomplete — one grammar for every agent', () => { const skills = [ skill({ name: 'typescript' }), skill({ name: 'react-useeffect', directoryPath: '/repo/.agents/skills/react-useeffect' }) ] + const codex = getNativeChatAgentProfile('codex') - it('enters skill mode with the query after `$`', () => { - const result = deriveComposerAutocomplete('use $type', 9, COMMANDS, skills) - expect(result.mode).toBe('skill') - if (result.mode !== 'skill') { + it('offers Codex skills under `/`, tokenised as the form Codex invokes', () => { + const result = deriveComposerAutocomplete('use /type', 9, COMMANDS, skills, codex) + expect(result.mode).toBe('slash') + if (result.mode !== 'slash') { return } expect(result.query).toBe('type') - expect(result.items.map((entry) => entry.name)).toEqual(['typescript']) + expect(result.items.map((entry) => entry.token)).toEqual(['$typescript']) }) - it('fires at the start of input too', () => { - expect(deriveComposerAutocomplete('$react', 6, COMMANDS, skills).mode).toBe('skill') + it('no longer treats `$` as a composer trigger', () => { + expect(deriveComposerAutocomplete('use $type', 9, COMMANDS, skills, codex).mode).toBe('none') + expect(deriveComposerAutocomplete('$react', 6, COMMANDS, skills, codex).mode).toBe('none') }) it('does not fire inside shell-style text', () => { - expect(deriveComposerAutocomplete('price$tag', 9, COMMANDS, skills).mode).toBe('none') + expect(deriveComposerAutocomplete('price$tag', 9, COMMANDS, skills, codex).mode).toBe('none') }) }) @@ -194,31 +197,151 @@ describe('apply suggestions', () => { expect(result.caret).toBe('open @src/app.ts '.length) }) - it('applyPickerSuggestion replaces the active $token at the caret', () => { - const result = applyPickerSuggestion( - 'use $typ now', - 8, - { kind: 'skill', id: 'skill:typescript', name: 'typescript', description: null, sources: [] }, - '$' - ) + it('applyPickerSuggestion swaps the typed /token for the agent-native token', () => { + const result = applyPickerSuggestion('use /typ now', 8, { + kind: 'skill', + id: 'skill:typescript', + name: 'typescript', + token: '$typescript', + description: null, + sources: [] + }) expect(result.draft).toBe('use $typescript now') expect(result.caret).toBe('use $typescript '.length) + expect(result.insertedToken).toBe('$typescript') }) }) describe('native skill and command picker', () => { - it('keeps Codex commands under slash and skills under dollar', () => { - const profile = getNativeChatAgentProfile('codex') - const slash = deriveComposerAutocomplete('/', 1, COMMANDS, [skill({})], profile) + it('puts Codex commands and skills in one `/` menu, each with its own token', () => { + const slash = deriveComposerAutocomplete( + '/', + 1, + COMMANDS, + [skill({ name: 'browser' })], + getNativeChatAgentProfile('codex') + ) expect(slash.mode).toBe('slash') - if (slash.mode === 'slash') { - expect(slash.items.every((item) => item.kind === 'command')).toBe(true) + if (slash.mode !== 'slash') { + return } - const dollar = deriveComposerAutocomplete('$', 1, COMMANDS, [skill({})], profile) - expect(dollar.mode).toBe('skill') - if (dollar.mode === 'skill') { - expect(dollar.items.every((item) => item.kind === 'skill')).toBe(true) + expect(slash.grouped).toBe(true) + expect(slash.items.filter((item) => item.kind === 'command').map((item) => item.token)).toEqual( + ['/clear', '/compact', '/help'] + ) + expect(slash.items.filter((item) => item.kind === 'skill').map((item) => item.token)).toEqual([ + '$browser' + ]) + }) + + it('keeps a Codex command and a same-named skill as separate rows', () => { + const result = deriveComposerAutocomplete( + '/clear', + 6, + COMMANDS, + [skill({ name: 'clear' })], + getNativeChatAgentProfile('codex') + ) + expect(result.mode).toBe('slash') + if (result.mode !== 'slash') { + return } + expect(result.items.map((item) => item.token)).toEqual(['/clear', '$clear']) + expect(result.items.find((item) => item.kind === 'command')?.skillCollision).toBe(false) + }) + + it('offers the same commands and skills for a `/` typed mid-prompt as for a leading one', () => { + const args = [ + COMMANDS, + [skill({ name: 'electron' })], + getNativeChatAgentProfile('claude') + ] as const + const leading = deriveComposerAutocomplete('/', 1, ...args) + const midPrompt = deriveComposerAutocomplete('validate it with /', 18, ...args) + expect(midPrompt.mode).toBe('slash') + if (midPrompt.mode !== 'slash' || leading.mode !== 'slash') { + return + } + expect(midPrompt.items).toEqual(leading.items) + expect(midPrompt.items.map((item) => item.kind)).toContain('command') + expect(midPrompt.items.map((item) => item.kind)).toContain('skill') + expect(midPrompt.grouped).toBe(leading.grouped) + }) + + it('filters the mid-prompt `/` menu by the typed token', () => { + const result = deriveComposerAutocomplete( + 'validate it with /elec', + 22, + COMMANDS, + [skill({ name: 'electron' })], + getNativeChatAgentProfile('claude') + ) + expect(result.mode).toBe('slash') + if (result.mode === 'slash') { + expect(result.prefix).toBe('/') + expect(result.items.map((item) => item.name)).toEqual(['electron']) + } + }) + + it('marks only a draft-leading `/command` dispatchable', () => { + const profile = getNativeChatAgentProfile('claude') + const leading = deriveComposerAutocomplete('/comp', 5, COMMANDS, [], profile) + const midPrompt = deriveComposerAutocomplete('then /comp', 10, COMMANDS, [], profile) + expect(leading.mode === 'slash' && leading.dispatchable).toBe(true) + expect(midPrompt.mode === 'slash' && midPrompt.dispatchable).toBe(false) + }) + + it('leaves a mid-prompt path alone', () => { + expect( + deriveComposerAutocomplete( + 'open /Users/me/notes', + 20, + COMMANDS, + [skill({ name: 'electron' })], + getNativeChatAgentProfile('claude') + ).mode + ).toBe('none') + }) + + it('opens the mid-prompt `/` menu for Codex too, tokenised for Codex', () => { + const result = deriveComposerAutocomplete( + 'validate it with /elec', + 22, + COMMANDS, + [skill({ name: 'electron' })], + getNativeChatAgentProfile('codex') + ) + expect(result.mode).toBe('slash') + if (result.mode !== 'slash') { + return + } + expect(result.dispatchable).toBe(false) + expect(result.items.map((item) => item.token)).toEqual(['$electron']) + }) + + it.each(['claude', 'codex'] as const)( + 'loads the skill catalog for both `/` trigger positions on %s', + (agent) => { + const profile = getNativeChatAgentProfile(agent) + expect(isSkillPickerTriggered('/elec', profile)).toBe(true) + expect(isSkillPickerTriggered('validate it with /elec', profile)).toBe(true) + expect(isSkillPickerTriggered('open /Users/me', profile)).toBe(false) + // Without a catalog fetch the menu would sit on a permanent loading row. + expect(isSkillPickerTriggered('use $elec', profile)).toBe(false) + } + ) + + it('applyPickerSuggestion replaces a mid-prompt /token at the caret', () => { + const result = applyPickerSuggestion('validate it with /elec now', 22, { + kind: 'skill', + id: 'skill:electron', + name: 'electron', + token: '/electron', + description: null, + sources: [] + }) + expect(result.draft).toBe('validate it with /electron now') + expect(result.caret).toBe('validate it with /electron '.length) }) it('groups Claude commands and skills under slash', () => { @@ -327,7 +450,7 @@ describe('native skill and command picker', () => { '$' ) expect(items.map((item) => item.name)).toEqual([longName]) - const applied = applyPickerSuggestion('$sk', 3, items[0], '$') + const applied = applyPickerSuggestion('/sk', 3, items[0]) expect(applied.draft).toBe(`$${longName} `) }) @@ -364,12 +487,14 @@ describe('native skill and command picker', () => { }) it('replaces only the active slash token and preserves text after the caret', () => { - const result = applyPickerSuggestion( - '/bro trailing', - 4, - { kind: 'skill', id: 'skill:browser', name: 'browser', description: null, sources: [] }, - '/' - ) + const result = applyPickerSuggestion('/bro trailing', 4, { + kind: 'skill', + id: 'skill:browser', + name: 'browser', + token: '/browser', + description: null, + sources: [] + }) expect(result.draft).toBe('/browser trailing') expect(result.caret).toBe('/browser '.length) }) @@ -396,29 +521,29 @@ describe('native skill and command picker', () => { it('treats a one-edit token swap as a new trigger occurrence', () => { expect(editReplacesTriggerToken('/foo', '/bar', '/:0')).toBe(true) - expect(editReplacesTriggerToken('use $foo', 'use $bar', '$:4')).toBe(true) + expect(editReplacesTriggerToken('use /foo', 'use /bar', '/:4')).toBe(true) }) it('keeps suppression while typing or deleting inside the dismissed token', () => { expect(editReplacesTriggerToken('/foo', '/food', '/:0')).toBe(false) expect(editReplacesTriggerToken('/food', '/foo', '/:0')).toBe(false) - expect(editReplacesTriggerToken('use $foo now', 'ran $foo now', '$:4')).toBe(false) + expect(editReplacesTriggerToken('use /foo now', 'ran /foo now', '/:4')).toBe(false) }) it('suppresses only the dismissed trigger occurrence', () => { const profile = getNativeChatAgentProfile('codex') - expect(deriveComposerAutocomplete('use $bro', 8, COMMANDS, [skill({})], profile).mode).toBe( - 'skill' + expect(deriveComposerAutocomplete('use /bro', 8, COMMANDS, [skill({})], profile).mode).toBe( + 'slash' ) expect( deriveComposerAutocomplete( - 'use $bro', + 'use /bro', 8, COMMANDS, [skill({})], profile, { status: 'ready', skills: [skill({})] }, - '$:4' + '/:4' ).mode ).toBe('none') }) diff --git a/src/renderer/src/components/native-chat/native-chat-composer-state.ts b/src/renderer/src/components/native-chat/native-chat-composer-state.ts index 3535a90c04b..b76622ed255 100644 --- a/src/renderer/src/components/native-chat/native-chat-composer-state.ts +++ b/src/renderer/src/components/native-chat/native-chat-composer-state.ts @@ -9,6 +9,8 @@ import { } from '../../../../shared/native-chat-slash-commands' import { buildNativeChatPickerItems, + LEADING_SLASH_TRIGGER, + MID_PROMPT_SLASH_TRIGGER, type NativeChatPickerItem, type NativeChatSkillDiscoverySnapshot } from './native-chat-picker-items' @@ -28,7 +30,9 @@ type PickerAutocomplete = { query: string items: NativeChatPickerItem[] triggerKey: string - prefix: '/' | '$' + prefix: '/' + /** Only a draft-leading `/command` reaches the agent as a command. */ + dispatchable: boolean grouped: boolean commandsEnabled: boolean skillsEnabled: boolean @@ -40,10 +44,20 @@ export type ComposerAutocomplete = | { mode: 'none' } | ({ mode: 'slash' } & PickerAutocomplete) | { mode: 'mention'; query: string } - | ({ mode: 'skill' } & PickerAutocomplete) const EMPTY_DISCOVERY: NativeChatSkillDiscoverySnapshot = { status: 'ready', skills: [] } +/** Whether the caret sits in a token that needs the skill catalog loaded. */ +export function isSkillPickerTriggered( + before: string, + profile: NativeChatAgentProfile | null +): boolean { + if (!profile) { + return false + } + return LEADING_SLASH_TRIGGER.test(before) || MID_PROMPT_SLASH_TRIGGER.test(before) +} + export function deriveComposerAutocomplete( draft: string, caret: number, @@ -55,9 +69,11 @@ export function deriveComposerAutocomplete( sessionSkillNames?: readonly string[] ): ComposerAutocomplete { const before = draft.slice(0, caret) - if (before.startsWith('/') && !/\s/.test(before)) { + const leadingMatch = before.match(LEADING_SLASH_TRIGGER) + if (leadingMatch) { return deriveSlashAutocomplete( - before, + leadingMatch[1], + 0, agentCommands, profile, discovery, @@ -69,70 +85,64 @@ export function deriveComposerAutocomplete( if (mentionMatch) { return { mode: 'mention', query: mentionMatch[1] } } - const skillMatch = - profile?.skillPrefix === '$' || (!profile && skills.length > 0) - ? before.match(/(?:^|\s)\$(\S*)$/) - : null - if (!skillMatch) { + // Why: `/` is the whole composer grammar, so a mid-prompt token opens the same + // menu a leading one does — it just cannot dispatch. + const midPromptMatch = profile ? before.match(MID_PROMPT_SLASH_TRIGGER) : null + if (!midPromptMatch) { return { mode: 'none' } } - const triggerKey = `$:${before.length - skillMatch[1].length - 1}` - if (dismissedTriggerKey === triggerKey) { - return { mode: 'none' } - } - const query = skillMatch[1] - return { - mode: 'skill', + const query = midPromptMatch[1] + return deriveSlashAutocomplete( query, - triggerKey, - prefix: '$', - grouped: false, - commandsEnabled: false, - skillsEnabled: true, - items: buildNativeChatPickerItems([], discovery.skills, query, '$', sessionSkillNames), - skillStatus: discovery.status === 'idle' ? 'loading' : discovery.status, - ...(discovery.errorKind ? { skillErrorKind: discovery.errorKind } : {}) - } + before.length - query.length - 1, + agentCommands, + profile, + discovery, + dismissedTriggerKey, + sessionSkillNames + ) } function deriveSlashAutocomplete( - before: string, + query: string, + triggerPosition: number, agentCommands: readonly SlashCommandSuggestion[], profile: NativeChatAgentProfile | null, discovery: NativeChatSkillDiscoverySnapshot, dismissedTriggerKey: string | null, sessionSkillNames: readonly string[] | undefined ): ComposerAutocomplete { - const triggerKey = '/:0' + const triggerKey = `/:${triggerPosition}` if (dismissedTriggerKey === triggerKey) { return { mode: 'none' } } - const query = before.slice(1) - const hasSlashSkills = profile?.skillPrefix === '/' - // Why: the caller owns catalog policy (e.g. Grok ships skills-only until a - // verified catalog lands); this derivation must not re-gate per agent. + // Every agent with a known grammar offers skills here; only the token a pick + // inserts differs. The caller owns catalog policy (e.g. Grok ships skills-only + // until a verified catalog lands), so this derivation must not re-gate per agent. + const skillsEnabled = profile !== null const items = buildNativeChatPickerItems( agentCommands, - hasSlashSkills ? discovery.skills : [], + skillsEnabled ? discovery.skills : [], query, - '/', - hasSlashSkills ? sessionSkillNames : [] + profile?.skillPrefix ?? '/', + skillsEnabled ? sessionSkillNames : [] ) return { mode: 'slash', query, triggerKey, prefix: '/', - grouped: profile?.groupedSlash === true, + dispatchable: triggerPosition === 0, + grouped: skillsEnabled, commandsEnabled: agentCommands.length > 0, - skillsEnabled: hasSlashSkills, + skillsEnabled, items, - skillStatus: hasSlashSkills + skillStatus: skillsEnabled ? discovery.status === 'idle' ? 'loading' : discovery.status : 'ready', - ...(hasSlashSkills && discovery.errorKind ? { skillErrorKind: discovery.errorKind } : {}) + ...(skillsEnabled && discovery.errorKind ? { skillErrorKind: discovery.errorKind } : {}) } } diff --git a/src/renderer/src/components/native-chat/native-chat-picker-items.ts b/src/renderer/src/components/native-chat/native-chat-picker-items.ts index 427e3b74497..1a21a2461c6 100644 --- a/src/renderer/src/components/native-chat/native-chat-picker-items.ts +++ b/src/renderer/src/components/native-chat/native-chat-picker-items.ts @@ -18,6 +18,8 @@ export type NativeChatPickerItem = kind: 'command' id: string name: string + /** Exactly what a pick inserts — the form the agent invokes. */ + token: string description?: string skillCollision: boolean } @@ -25,6 +27,7 @@ export type NativeChatPickerItem = kind: 'skill' id: string name: string + token: string description: string | null sources: { sourceKind: SkillSourceKind; skillFilePath: string }[] } @@ -47,16 +50,24 @@ export function buildNativeChatPickerItems( commands: readonly SlashCommandSuggestion[], skills: readonly DiscoveredSkill[], query: string, - prefix: '/' | '$', + skillSigil: '/' | '$', sessionSkillNames?: readonly string[] ): NativeChatPickerItem[] { + // A name can only collide when both kinds invoke through the same sigil; + // where skills carry their own, `/review` and `$review` are distinct entries. + const sharedSigil = skillSigil === '/' const unclassifiedNames = new Set( commands.filter((command) => command.kindUnspecified).map((command) => command.name) ) - const mergedSkills = mergeNativeChatSkills(skills, sessionSkillNames, unclassifiedNames) + const mergedSkills = mergeNativeChatSkills( + skills, + sessionSkillNames, + unclassifiedNames, + skillSigil + ) const skillNames = new Set(mergedSkills.map((skill) => skill.name)) const resolvedCommands = commands.filter( - (command) => !(command.kindUnspecified && skillNames.has(command.name)) + (command) => !(sharedSigil && command.kindUnspecified && skillNames.has(command.name)) ) const commandNames = new Set(resolvedCommands.map((command) => command.name)) const commandItems = rankItems( @@ -67,8 +78,9 @@ export function buildNativeChatPickerItems( // it is inserted verbatim; only untrusted skill text gets sanitized. id: `command:${command.name}`, name: command.name, + token: `/${command.name}`, description: command.description ? sanitizePickerText(command.description, 240) : undefined, - skillCollision: prefix === '/' && skillNames.has(command.name) + skillCollision: sharedSigil && skillNames.has(command.name) }, stableOrder: index })), @@ -76,7 +88,7 @@ export function buildNativeChatPickerItems( ) const skillItems = rankItems( mergedSkills - .filter((skill) => !(prefix === '/' && commandNames.has(skill.name))) + .filter((skill) => !(sharedSigil && commandNames.has(skill.name))) .map((item, index) => ({ item, stableOrder: index })), query ) @@ -89,7 +101,8 @@ export function buildNativeChatPickerItems( function mergeNativeChatSkills( skills: readonly DiscoveredSkill[], sessionSkillNames: readonly string[] | undefined, - unclassifiedNames: ReadonlySet + unclassifiedNames: ReadonlySet, + skillSigil: '/' | '$' ): Extract[] { const exactPaths = new Map() for (const skill of skills) { @@ -106,7 +119,10 @@ function mergeNativeChatSkills( byName.set(safeName, [...(byName.get(safeName) ?? []), { ...skill, name: safeName }]) } const discovered = new Map( - [...byName.entries()].map(([name, namedSkills]) => [name, pickerSkill(name, namedSkills)]) + [...byName.entries()].map(([name, namedSkills]) => [ + name, + pickerSkill(name, namedSkills, skillSigil) + ]) ) // Why: when the running session reports its own skills, that report is the // authority on which ones exist — a disk scan cannot see what the session @@ -121,19 +137,21 @@ function mergeNativeChatSkills( ] : [...discovered.keys()] return [...new Set(names)] - .map((name) => discovered.get(name) ?? pickerSkill(name, [])) + .map((name) => discovered.get(name) ?? pickerSkill(name, [], skillSigil)) .sort(comparePickerSkills) } function pickerSkill( name: string, - namedSkills: readonly DiscoveredSkill[] + namedSkills: readonly DiscoveredSkill[], + skillSigil: '/' | '$' ): Extract { const sorted = [...namedSkills].sort(compareDiscoveredSkills) return { kind: 'skill' as const, id: `skill:${name}`, name, + token: `${skillSigil}${name}`, description: sorted[0]?.description ? sanitizePickerText(sorted[0].description, 240) : null, sources: sorted.map((skill) => ({ sourceKind: skill.sourceKind, @@ -245,21 +263,27 @@ function comparePickerSkills( ) } +// `/` is the composer's only trigger, for every agent. A draft-leading slash is +// the one that can dispatch; elsewhere the token starts after whitespace and its +// query stops at the next `/` so file paths stay prose. +export const LEADING_SLASH_TRIGGER = /^\/(\S*)$/ +export const MID_PROMPT_SLASH_TRIGGER = /\s\/([^\s/]*)$/ + +/** Replaces the typed `/token` with the item's own token, which for a skill is + * the agent-native form even though every agent is typed the same way. */ export function applyPickerSuggestion( draft: string, caret: number, - item: NativeChatPickerItem, - prefix: '/' | '$' + item: NativeChatPickerItem ): { draft: string; caret: number; insertedToken: string } { const before = draft.slice(0, caret) const after = draft.slice(caret) - const match = prefix === '/' ? before.match(/^\/(\S*)$/) : before.match(/(^|\s)\$(\S*)$/) + const match = before.match(LEADING_SLASH_TRIGGER) ?? before.match(MID_PROMPT_SLASH_TRIGGER) if (!match) { return { draft, caret, insertedToken: '' } } const query = match.at(-1) ?? '' const tokenStart = before.length - query.length - 1 - const insertedToken = `${prefix}${item.name}` - const nextBefore = `${before.slice(0, tokenStart)}${insertedToken} ` - return { draft: nextBefore + after, caret: nextBefore.length, insertedToken } + const nextBefore = `${before.slice(0, tokenStart)}${item.token} ` + return { draft: nextBefore + after, caret: nextBefore.length, insertedToken: item.token } } diff --git a/src/renderer/src/components/native-chat/use-native-chat-composer-catalog.test.tsx b/src/renderer/src/components/native-chat/use-native-chat-composer-catalog.test.tsx index 1c68ac91ffa..9e7b3abc5b2 100644 --- a/src/renderer/src/components/native-chat/use-native-chat-composer-catalog.test.tsx +++ b/src/renderer/src/components/native-chat/use-native-chat-composer-catalog.test.tsx @@ -103,6 +103,7 @@ it('Enter completes a known pre-init skill while still dispatching a built-in co items, triggerKey: '/', prefix: '/', + dispatchable: true, grouped: true, commandsEnabled: true, skillsEnabled: true, diff --git a/src/renderer/src/components/native-chat/use-native-chat-composer-keydown.test.tsx b/src/renderer/src/components/native-chat/use-native-chat-composer-keydown.test.tsx index ed3814e8476..bf39562169e 100644 --- a/src/renderer/src/components/native-chat/use-native-chat-composer-keydown.test.tsx +++ b/src/renderer/src/components/native-chat/use-native-chat-composer-keydown.test.tsx @@ -2,13 +2,20 @@ import { renderHook } from '@testing-library/react' import { describe, expect, it, vi } from 'vitest' -import { EMPTY_HISTORY, type ComposerAutocomplete } from './native-chat-composer-state' +import { + applyPickerSuggestion, + deriveComposerAutocomplete, + EMPTY_HISTORY, + type ComposerAutocomplete +} from './native-chat-composer-state' +import { getNativeChatAgentProfile } from '../../../../shared/native-chat-agent-profiles' import { useNativeChatComposerKeyDown } from './use-native-chat-composer-keydown' const COMMAND = { kind: 'command' as const, id: 'command:clear', name: 'clear', + token: '/clear', description: 'Clear history', skillCollision: false } @@ -20,6 +27,7 @@ function picker(items = [COMMAND]): Extract composing, ...callbacks @@ -81,6 +89,36 @@ describe('useNativeChatComposerKeyDown', () => { expect(callbacks.send).toHaveBeenCalledOnce() }) + it.each(['claude', 'openclaude', 'codex', 'grok'] as const)( + 'completes mid-prompt command Enter without dispatching or losing prose for %s', + (agent) => { + const draft = 'Explain /cle before continuing' + const caret = 'Explain /cle'.length + const autocomplete = deriveComposerAutocomplete( + draft, + caret, + [COMMAND], + [], + getNativeChatAgentProfile(agent) + ) + expect(autocomplete.mode).toBe('slash') + const { handler, callbacks } = setup(autocomplete, false, draft) + const event = keyEvent('Enter') + handler(event as never) + + expect(event.preventDefault).toHaveBeenCalledOnce() + expect(callbacks.dispatchPickerCommand).not.toHaveBeenCalled() + expect(callbacks.send).not.toHaveBeenCalled() + expect(callbacks.completePickerItem).toHaveBeenCalledOnce() + const [item] = callbacks.completePickerItem.mock.calls[0] + expect(applyPickerSuggestion(draft, caret, item)).toEqual({ + draft: 'Explain /clear before continuing', + caret: 'Explain /clear '.length, + insertedToken: '/clear' + }) + } + ) + it('dismisses Escape without interrupting the agent', () => { const { handler, callbacks } = setup() handler(keyEvent('Escape') as never) diff --git a/src/renderer/src/components/native-chat/use-native-chat-composer-keydown.ts b/src/renderer/src/components/native-chat/use-native-chat-composer-keydown.ts index 2fb712f9825..c85ad949aad 100644 --- a/src/renderer/src/components/native-chat/use-native-chat-composer-keydown.ts +++ b/src/renderer/src/components/native-chat/use-native-chat-composer-keydown.ts @@ -51,7 +51,7 @@ export function useNativeChatComposerKeyDown({ return } - if (autocomplete.mode === 'slash' || autocomplete.mode === 'skill') { + if (autocomplete.mode === 'slash') { const items = autocomplete.items if (event.key === 'ArrowDown' && items.length > 0) { event.preventDefault() @@ -66,7 +66,9 @@ export function useNativeChatComposerKeyDown({ if ((event.key === 'Enter' || event.key === 'Tab') && items.length > 0) { event.preventDefault() const item = items[activeSuggestion] ?? items[0] - if (event.key === 'Enter' && item.kind === 'command') { + // A mid-prompt command is part of the sentence being written, so Enter + // completes the token instead of sending the command on its own. + if (event.key === 'Enter' && item.kind === 'command' && autocomplete.dispatchable) { dispatchPickerCommand(item) } else { completePickerItem(item) diff --git a/src/renderer/src/components/native-chat/use-native-chat-picker-command-dispatch.test.tsx b/src/renderer/src/components/native-chat/use-native-chat-picker-command-dispatch.test.tsx index 90c495a2bc7..7584dcc15a4 100644 --- a/src/renderer/src/components/native-chat/use-native-chat-picker-command-dispatch.test.tsx +++ b/src/renderer/src/components/native-chat/use-native-chat-picker-command-dispatch.test.tsx @@ -23,6 +23,7 @@ const COMMAND = { kind: 'command' as const, id: 'command:status', name: 'status', + token: '/status', description: 'Show status', skillCollision: false } diff --git a/src/renderer/src/components/native-chat/use-native-chat-picker-state.ts b/src/renderer/src/components/native-chat/use-native-chat-picker-state.ts index d96060342e5..9d4054b433e 100644 --- a/src/renderer/src/components/native-chat/use-native-chat-picker-state.ts +++ b/src/renderer/src/components/native-chat/use-native-chat-picker-state.ts @@ -18,6 +18,7 @@ import { classifyNativeChatSend, deriveComposerAutocomplete, editReplacesTriggerToken, + isSkillPickerTriggered, type ComposerAutocomplete, type NativeChatPickerItem, type NativeChatSendClassification @@ -68,13 +69,7 @@ export function useNativeChatPickerState(args: { setActiveSuggestion } = args const profile = useMemo(() => getNativeChatAgentProfile(agent), [agent]) - const beforeCaret = draft.slice(0, caret) - const skillPickerTriggered = - profile?.skillPrefix === '$' - ? /(?:^|\s)\$\S*$/.test(beforeCaret) - : profile?.skillPrefix === '/' - ? beforeCaret.startsWith('/') && !/\s/.test(beforeCaret) - : false + const skillPickerTriggered = isSkillPickerTriggered(draft.slice(0, caret), profile) const discovery = useNativeChatSkills(agent, terminalTabId, skillPickerTriggered) const listboxId = `native-chat-picker-${useId().replaceAll(':', '')}` const dismissalContext = `${draftScopeKey}:${agent}` @@ -114,7 +109,7 @@ export function useNativeChatPickerState(args: { }, [dismissalContext]) useEffect(() => { - if (autocomplete.mode !== 'slash' && autocomplete.mode !== 'skill') { + if (autocomplete.mode !== 'slash') { lastOpenKeyRef.current = null return } @@ -127,10 +122,10 @@ export function useNativeChatPickerState(args: { const completeItem = useCallback( (item: NativeChatPickerItem) => { - if (autocomplete.mode !== 'slash' && autocomplete.mode !== 'skill') { + if (autocomplete.mode !== 'slash') { return } - const result = applyPickerSuggestion(draft, caret, item, autocomplete.prefix) + const result = applyPickerSuggestion(draft, caret, item) if (item.kind === 'skill' && textareaRef.current?.insertSkill) { const from = result.caret - result.insertedToken.length - 1 textareaRef.current.insertSkill(from, caret, result.insertedToken) @@ -174,10 +169,7 @@ export function useNativeChatPickerState(args: { null, sessionSkillNames ) - if ( - (next.mode !== 'slash' && next.mode !== 'skill') || - next.triggerKey !== dismissed.triggerKey - ) { + if (next.mode !== 'slash' || next.triggerKey !== dismissed.triggerKey) { setDismissed(null) } }, diff --git a/src/renderer/src/components/native-chat/use-native-chat-skills.react.test.tsx b/src/renderer/src/components/native-chat/use-native-chat-skills.react.test.tsx index f360394cf37..e05cbb7a870 100644 --- a/src/renderer/src/components/native-chat/use-native-chat-skills.react.test.tsx +++ b/src/renderer/src/components/native-chat/use-native-chat-skills.react.test.tsx @@ -3,6 +3,8 @@ import { act, cleanup, render, waitFor } from '@testing-library/react' import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' import type { NativeChatSkillDiscovery } from './use-native-chat-skills' +import { getNativeChatAgentProfile } from '../../../../shared/native-chat-agent-profiles' +import { isSkillPickerTriggered } from './native-chat-composer-state' const mocks = vi.hoisted(() => ({ callRuntimeRpc: vi.fn(), @@ -56,6 +58,10 @@ function Probe({ enabled }: { enabled: boolean }): null { return null } +function DraftProbe({ draft }: { draft: string }): React.JSX.Element { + return +} + describe('useNativeChatSkills', () => { beforeEach(() => { mocks.state = stateForHost('local') @@ -132,6 +138,24 @@ describe('useNativeChatSkills', () => { ) }) + it('reuses one discovery while typing and reopening leading and mid-prompt slash tokens', async () => { + const view = render() + expect(mocks.callRuntimeRpc).not.toHaveBeenCalled() + + view.rerender() + await waitFor(() => expect(mocks.snapshots.at(-1)?.status).toBe('ready')) + for (const draft of ['Explain /b', 'Explain /br', 'Explain /bro']) { + view.rerender() + expect(mocks.snapshots.at(-1)?.skills.map((skill) => skill.name)).toEqual(['browser']) + } + view.rerender() + expect(mocks.snapshots.at(-1)?.status).toBe('idle') + view.rerender() + await waitFor(() => expect(mocks.snapshots.at(-1)?.status).toBe('ready')) + view.rerender() + expect(mocks.callRuntimeRpc).toHaveBeenCalledTimes(1) + }) + it('surfaces discovery failure instead of remaining loading', async () => { mocks.callRuntimeRpc.mockRejectedValueOnce(new Error('scan failed')) render() diff --git a/src/shared/native-chat-agent-profiles.test.ts b/src/shared/native-chat-agent-profiles.test.ts index 27b7e07d33b..4d64e283c01 100644 --- a/src/shared/native-chat-agent-profiles.test.ts +++ b/src/shared/native-chat-agent-profiles.test.ts @@ -2,24 +2,23 @@ import { describe, expect, it } from 'vitest' import { getNativeChatAgentProfile } from './native-chat-agent-profiles' describe('native chat agent picker profiles', () => { - it('keeps Codex dollar skills separate from slash commands', () => { + // The composer types the same `/` for every agent; skillPrefix is only the + // form a picked skill is written as. + it('keeps Codex skills invocable as dollar tokens', () => { expect(getNativeChatAgentProfile('codex')).toMatchObject({ skillPrefix: '$', - groupedSlash: false, skillSourceOwner: 'codex' }) }) - it('groups Claude-family and Grok skills under slash', () => { + it('writes Claude-family and Grok skills as slash tokens', () => { expect(getNativeChatAgentProfile('claude')).toMatchObject({ skillPrefix: '/', - groupedSlash: true, skillSourceOwner: 'claude' }) expect(getNativeChatAgentProfile('openclaude')).toMatchObject({ skillSourceOwner: 'claude' }) expect(getNativeChatAgentProfile('grok')).toMatchObject({ skillPrefix: '/', - groupedSlash: true, skillSourceOwner: 'grok' }) }) diff --git a/src/shared/native-chat-agent-profiles.ts b/src/shared/native-chat-agent-profiles.ts index ae52b48efef..6f86313d17d 100644 --- a/src/shared/native-chat-agent-profiles.ts +++ b/src/shared/native-chat-agent-profiles.ts @@ -3,7 +3,6 @@ import { getAgentSlashCommands, type SlashCommandSuggestion } from './native-cha export type NativeChatAgentProfile = { skillPrefix: '$' | '/' - groupedSlash: boolean /** OpenClaude reads Claude-owned roots, so this can differ from the agent. */ skillSourceOwner: AgentType } @@ -11,22 +10,18 @@ export type NativeChatAgentProfile = { const NATIVE_CHAT_AGENT_PROFILES: Partial> = { codex: { skillPrefix: '$', - groupedSlash: false, skillSourceOwner: 'codex' }, claude: { skillPrefix: '/', - groupedSlash: true, skillSourceOwner: 'claude' }, openclaude: { skillPrefix: '/', - groupedSlash: true, skillSourceOwner: 'claude' }, grok: { skillPrefix: '/', - groupedSlash: true, skillSourceOwner: 'grok' } } From 4e0aa7a4735621f72f7bbb5b0ba46585eb3e4066 Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" <41898282+github-actions[bot]@users.noreply.github.com> Date: Thu, 10 Sep 2026 06:40:42 +0000 Subject: [PATCH 7/8] Update README downloads badge --- docs/assets/readme-downloads.svg | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/docs/assets/readme-downloads.svg b/docs/assets/readme-downloads.svg index 39008eaa963..3ea69247a26 100644 --- a/docs/assets/readme-downloads.svg +++ b/docs/assets/readme-downloads.svg @@ -1,5 +1,5 @@ - - downloads: 45m + + downloads: 46m @@ -15,7 +15,7 @@ downloads downloads - 45m - 45m + 46m + 46m From 2bf298d1dc1a41c03a270e89eca08834c36f13e0 Mon Sep 17 00:00:00 2001 From: Brennan Benson <79079362+brennanb2025@users.noreply.github.com> Date: Wed, 9 Sep 2026 23:47:47 -0700 Subject: [PATCH 8/8] feat(native-chat): the background-tasks strip says what is running (#19311) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * feat(native-chat): name, group, and state the background-tasks strip The strip above the composer described five different kinds of background work as "Monitoring background tasks", with identical flat-dot rows. Now: - Wire: additive optional `name`, `state`, `startedAt` on AgentSessionBackgroundTask, plus `settledTasks` on the state object so terminal siblings of a live fan-out stay visible without changing what old clients render (they keep exactly the live `tasks` list). - Reducer equality learns the new fields, so a publish whose only change is a task's state is no longer judged equal and dropped. - Header counts by kind and lists states within a kind; past three kind segments (or on a narrow strip, measured by its own border-box against the live root font size) it falls back to an honest total, never a partial enumeration, and the strip stays expandable whenever the header is lossy. - Rows group by kind (Agents / Shell / Monitors / Workflows / Tasks), stable-sorted first-seen-then-id, each with a kind icon, its own state dot, a resolved name (description -> name -> kind label), and elapsed. - Claude producer: task frames now carry name (agent_type/subagent_type), a run state mapped from patch status, and first-seen startedAt. Terminal statuses settle a task (completed->done, failed->blocked, killed/stopped->idle) instead of deleting it; settled tasks render only beside still-live work and flush when the last live task ends, so the strip exits exactly when it does today. An unreadable patch leaves a task open, never settled. - Turn gating moves off the strip: the tracker no longer zeroes its roster during a foreground turn, and the client renders the strip whenever it has contents while the idle-only flag now gates just the animated monitoring indicator and conversation commands. * feat(sidebar): indent native-chat subagents under their session row buildSubagentChildRows() has always rendered indented children from parentEntry.subagents, and the structured-session status bridge has always published an AgentStatusEntry for native chat — it just never populated subagents. Connect them: - Wire: additive optional `backgroundTasks` on AgentSessionStatusSummary (live tasks only), projected by the host status feed from the provider's backgroundTaskState hook and republished on task edges via the background-task channel, with the shared task equality suppressing no-op re-projections. - Bridge: maps agent-kind tasks onto the sidebar's own AgentSubagentState (working/waiting/blocked, terminal -> idle) — kinds stay distinct, so a backgrounded shell never lands in a subagent count — and extends its pre-write equality with the existing agentSubagentsEqual. - parentIsFresh for a bridge entry means "the host feed reported a change inside the sidebar's ordinary evidence window": every publish restamps evidenceObservedAt, and a dead feed stops restamping, so children decay to idle on lost contact instead of pinning 'working'. * fix(native-chat): settle tasks the aggregate roster evicted first; carry usage Real-agent QA showed settledTasks never rendered. A frame capture from the SDK (probe against claude 2.1.261) explains it: when a backgrounded child finishes, the producer emits `background_tasks_changed` FIRST — with the task already absent — and only then `task_updated`/`task_notification` with the outcome, in the same tick. The tracker's settle path looked the task up in the live roster the aggregate had just evicted, so retention lost the race 100% of the time. Fix: aggregate eviction of a live backgrounded task now parks its details in a bounded recently-removed map (new claude-settled-background-tasks.ts, which also owns the settled roster), and the trailing terminal edge consumes it. A removal whose outcome frame never arrives still vanishes — nothing is guessed into a finished state. A second terminal edge for the same task re-derives the settled state and can add final usage. The captured sequence is replayed verbatim as a tracker test, including the kill-at-exit tail proving the strip still exits with the last live task. The same capture disproved the PR's earlier claim that Claude task frames carry no usage: task_progress and task_notification both carry usage.total_tokens. Additive optional `totalTokens` on the wire task, covered by the shared equality; the tracker takes usage (never the transient "Running " description) from task_progress, and rows render the mock's "18.1k · 2m" meta — settled rows keep final usage with no still-growing clock. * chore(i18n): sync runtime-required catalog for backgroundTasks.runningList * fix(native-chat): preserve background task lifecycle and bound update work * fix(native-chat): transfer resumed background tasks to one live owner * fix(native-chat): bring structured session host under the line cap and restore subscribe fixture * fix(native-chat): complete journal stubs and stop notifying on feed teardown The status feed's projection cache calls journal.cursor(); the rename test's stubs are cast through unknown, so the missing method only surfaced at runtime. Teardown runs only once nothing is activated, so there is no mounted reader to notify - clearing confirmed sessions is what prevents a stale live on reactivation. * feat(native-chat): lead each strip header count with its kind icon The header carried one aggregate state dot, so a fan-out of agents and a monitor looked alike. Each count segment now leads with its own kind glyph; a collapsed total spans kinds and takes none. Monitor is the heartbeat AgentStateDot already draws for monitoring, so the strip and the agent sidebar speak one vocabulary. * feat(native-chat): give the strip's monitor heartbeat the sidebar amber The glyph matched AgentStateDot but the colour did not, so a monitor in the strip did not read as the monitor in the agent sidebar. One shared tone helper now serves the header segment and the expanded row, so they cannot diverge. Monitoring is a state the app already colours; the other four kinds are plain markers and stay neutral. A running turn still dims the whole set. * fix(native-chat): draw the strip header separator in a visible tone The separator used `text-border`, a divider-line token that is 7% white in dark mode - an order of magnitude fainter than the counts on either side, so the dot between them read as absent. main.css already records that token as too faint for a visible mark. * fix(native-chat): give the worktree-ps journal stub a cursor The status feed's projection cache calls journal.cursor(); this stub is cast through unknown, so the missing method only surfaced at runtime. Its journal never changes, so a real one would hold the cursor steady. * refactor(native-chat): split the sidebar subagent rows out of this PR The strip stands alone: the sidebar mapping, its observation plumbing and the AgentStatusEntry.subagents wiring move to a stacked follow-up. No wire field here is sidebar-only - the strip's rows read name, state, elapsed and tokens. * perf(native-chat): keep task usage out of the session status summary A `task_progress` frame ticks a background task's `totalTokens`, which failed the status feed's equality check and re-broadcast a full summary to every `agentSession.subscribeStatus` subscriber — paired-web and SSH/relay clients included — for a number no session list renders. The projection now drops usage; tokens keep flowing on the background-task channel the strip reads. * fix(native-chat): correct token unit rounding and drop the unused dot state `formatBackgroundTaskTokens` rounded before choosing the unit, so 999_950 rendered as "1000k" instead of "1m"; pick the unit from the rounded value. `backgroundTasksDotState` has no caller on this branch or the stacked sidebar PR, and its multi-kind branch would report 'monitoring' over an attention state. Delete it rather than leave it to be wired up. * fix(i18n): drop the orphaned backgroundTasks.runningList key The strip rewrite removed its only call site, and an unreferenced key gets promoted into the eagerly parsed boot catalog. Delete it from en.json and regenerate en-runtime-required.json. * fix(native-chat): show the reason on every attention row The row guarded the reason line on 'waiting', so an 'unverifiable' child ("no contact") and a 'blocked' one ("failed") rendered bare while the collapsed header named exactly those reasons. `backgroundTaskStateReason` already returns null for the non-attention states, so the guard was only lossy — the SSH boundary requires the unverifiable verdict stay legible. Also keys the header segments off their kind discriminant instead of the translated display text. * fix(native-chat): make the strip header agree with its own count The headline counts live AND settled rows, but the state breakdown omitted 'done', so one working agent beside four settled ones read "5 agents — 1 working": the count said five, the breakdown accounted for one. Done now appears in the muted detail (never as an emphasised segment) so the two agree. The single-command header also drew an elapsed clock on a settled task, which the row already refuses as a lie about finished work. * perf(native-chat): memoize the background-task roster grouping The 1 Hz elapsed tick re-rendered the strip, and the render body regrouped, re-sorted and re-translated every task each time only `now` had changed. The header still derives from `now` on purpose. * test(native-chat): cover settled rows and the mid-turn mounted strip Neither headline behaviour had component coverage: every strip render passed `settledTasks={[]}`, and the `showBackgroundTasks` seam was never set true, so the strip staying mounted through a running turn was exercised nowhere. Adds a settled-beside-live row test (final usage kept, no clock, no stop) and a mid-turn mount test (strip present, turn owns the voice). The background-task tests share one session-element helper so the file stays under its line cap. * refactor(claude): keep MAX_TASK_ID_LENGTH module-private Nothing outside claude-background-task-frames.ts references it; the export was residue from this PR's split. * test(native-chat): give the mid-turn strip test a real turn main now gates the composer's stop button on a provider-minted turnId rather than the send-time working signal, so a test claiming a running turn has to supply one. The controller mock hardcoded turnId null. --------- Co-authored-by: Merge Sim --- .../first-work-branch-rename.test.ts | 7 +- .../claude/claude-background-task-frames.ts | 106 ++++++ .../claude-background-task-resume.test.ts | 92 +++++ .../claude-background-task-tracker.test.ts | 336 +++++++++++++++--- .../claude/claude-background-task-tracker.ts | 253 +++++++------ .../claude/claude-settled-background-tasks.ts | 127 +++++++ .../claude-structured-session-close.test.ts | 8 +- .../provider-frame-activity.test.ts | 4 +- ...d-agent-session-background-task-channel.ts | 6 +- .../structured-agent-session-host.ts | 13 +- ...ructured-agent-session-status-feed.test.ts | 152 +++++++- .../structured-agent-session-status-feed.ts | 86 ++++- .../local-file-sink-memory.test.ts | 6 +- .../worktree-ps-structured-host.spec.ts | 2 + ...ructured-agent-session-rpc.test-fixture.ts | 1 + .../NativeChatBackgroundTasksStatus.test.tsx | 239 ++++++++++++- .../NativeChatBackgroundTasksStatus.tsx | 265 ++++++++++---- .../NativeChatStructuredSession.test.tsx | 120 ++++--- .../NativeChatStructuredSession.tsx | 11 +- .../background-task-header-content.ts | 192 ++++++++++ .../background-task-roster.test.ts | 245 +++++++++++++ .../native-chat/background-task-roster.ts | 181 ++++++++++ ...tructured-session-background-tasks-view.ts | 32 +- .../use-structured-agent-session.ts | 11 +- src/renderer/src/i18n/locales/en.json | 30 +- src/shared/agent-session-wire.ts | 58 +++ .../structured-agent-session-reducer.test.ts | 71 ++++ .../structured-agent-session-reducer.ts | 29 +- 28 files changed, 2332 insertions(+), 351 deletions(-) create mode 100644 src/main/claude/claude-background-task-frames.ts create mode 100644 src/main/claude/claude-background-task-resume.test.ts create mode 100644 src/main/claude/claude-settled-background-tasks.ts create mode 100644 src/renderer/src/components/native-chat/background-task-header-content.ts create mode 100644 src/renderer/src/components/native-chat/background-task-roster.test.ts create mode 100644 src/renderer/src/components/native-chat/background-task-roster.ts diff --git a/src/main/agent-hooks/first-work-branch-rename.test.ts b/src/main/agent-hooks/first-work-branch-rename.test.ts index 9cd4d544041..8ecd4997aa0 100644 --- a/src/main/agent-hooks/first-work-branch-rename.test.ts +++ b/src/main/agent-hooks/first-work-branch-rename.test.ts @@ -100,10 +100,14 @@ describe('maybeAutoRenameBranchOnFirstWork', () => { isPendingFirstAgentMessageRename: () => true }) const items: AgentJournalRenderItem[] = [] + // A real journal's sequence only ever advances, so the feed's projection + // cache must miss on every publish here: this test is about the rename. + let sequence = 0 const journal = { snapshot: () => ({ items }), lastActivityAt: () => 1, - isReadOnly: false + isReadOnly: false, + cursor: () => ({ epoch: 1, sequence: (sequence += 1) }) } as unknown as AgentSessionJournal const pending: Promise[] = [] const observe = vi.fn((summary, options) => { @@ -175,6 +179,7 @@ describe('maybeAutoRenameBranchOnFirstWork', () => { const journal = { isReadOnly: false, lastActivityAt: () => 1, + cursor: () => ({ epoch: 1, sequence: 1 }), snapshot: () => ({ items: [ { body: { kind: 'message', role: 'user', blocks: [{ type: 'text', text: 'Fix auth' }] } }, diff --git a/src/main/claude/claude-background-task-frames.ts b/src/main/claude/claude-background-task-frames.ts new file mode 100644 index 00000000000..d954128d91b --- /dev/null +++ b/src/main/claude/claude-background-task-frames.ts @@ -0,0 +1,106 @@ +// Field readers for the Claude SDK's background-task lifecycle frames +// (task_started / task_updated / task_notification / background_tasks_changed). +// Pure and bounded: every reader rejects absent, non-string, or oversized +// values so a malformed frame degrades to "field unknown", never to a throw. + +import type { + AgentSessionBackgroundTask, + AgentSessionBackgroundTaskRunState +} from '../../shared/agent-session-wire' + +const MAX_TASK_ID_LENGTH = 512 +const MAX_TASK_TEXT_LENGTH = 512 + +export type ClaudeBackgroundTaskKind = AgentSessionBackgroundTask['kind'] + +export function record(value: unknown): Record | null { + return typeof value === 'object' && value !== null ? (value as Record) : null +} + +/** The bound every task id shares, wherever it enters. An id the roster stores + * becomes a durable entry key, so a provisional one takes the same bound the + * announced path applies — an over-long id is rejected, never truncated. */ +export function isBoundedClaudeTaskId(value: string): boolean { + return value.length > 0 && value.length <= MAX_TASK_ID_LENGTH +} + +export function taskId(message: Record): string | null { + const value = message.task_id + return typeof value === 'string' && isBoundedClaudeTaskId(value) ? value : null +} + +function boundedTaskText(value: unknown): string | undefined { + if (typeof value !== 'string') { + return undefined + } + const trimmed = value.trim().replace(/\s+/g, ' ') + return trimmed.length > 0 ? trimmed.slice(0, MAX_TASK_TEXT_LENGTH) : undefined +} + +export function taskDescription(value: unknown): string | undefined { + return boundedTaskText(value) +} + +/** The provider-reported identity for a task. Subagent frames have carried the + * type under both `agent_type` and `subagent_type` across SDK versions. */ +export function taskName(frame: Record): string | undefined { + return ( + boundedTaskText(frame.name) ?? + boundedTaskText(frame.agent_type) ?? + boundedTaskText(frame.subagent_type) + ) +} + +export function classifyClaudeBackgroundTaskKind(taskType: unknown): ClaudeBackgroundTaskKind { + switch (taskType) { + case 'local_agent': + return 'agent' + case 'local_workflow': + return 'workflow' + case 'local_bash': + return 'command' + case 'monitor': + return 'monitor' + default: + return 'unknown' + } +} + +/** Cumulative token usage from a task_progress / task_notification frame. */ +export function taskUsageTotalTokens(frame: Record): number | undefined { + const usage = record(frame.usage) + const total = usage?.total_tokens + return typeof total === 'number' && Number.isFinite(total) && total >= 0 + ? Math.floor(total) + : undefined +} + +/** Settled state for a terminal status. Null for anything else — an unreadable + * status never settles a task by itself. */ +export function terminalClaudeTaskRunState( + status: unknown +): AgentSessionBackgroundTaskRunState | null { + switch (status) { + case 'completed': + return 'done' + case 'failed': + return 'blocked' + case 'killed': + case 'stopped': + return 'idle' + default: + return null + } +} + +/** Live state for a non-terminal status. Null leaves the tracked state alone. */ +export function liveClaudeTaskRunState(status: unknown): AgentSessionBackgroundTaskRunState | null { + switch (status) { + case 'pending': + case 'running': + case 'paused': + return 'working' + default: + return null + } +} diff --git a/src/main/claude/claude-background-task-resume.test.ts b/src/main/claude/claude-background-task-resume.test.ts new file mode 100644 index 00000000000..c043d485304 --- /dev/null +++ b/src/main/claude/claude-background-task-resume.test.ts @@ -0,0 +1,92 @@ +import { describe, expect, it } from 'vitest' +import { ClaudeBackgroundTaskTracker } from './claude-background-task-tracker' + +const agent = { + task_id: 'a962f88aa82feb1c1', + task_type: 'local_agent', + description: 'Long proof writer' +} +const shell = { task_id: 'bcl6x3ixf', task_type: 'local_bash', description: 'sleep 150' } +const sibling = { task_id: 'sibling', task_type: 'local_agent' } + +function system(subtype: string, fields: Record) { + return { type: 'system', subtype, ...fields } +} + +describe('Claude background task pause/resume ownership', () => { + it('moves a retained child back to live ownership across eviction, outcome, and auto-resume', () => { + let now = 100 + const tracker = new ClaudeBackgroundTaskTracker(() => now) + const roster = (tasks: unknown[]) => + tracker.observe(system('background_tasks_changed', { tasks })) + roster([agent, sibling, shell]) + tracker.observe( + system('task_progress', { task_id: agent.task_id, usage: { total_tokens: 18000 } }) + ) + tracker.observe({ type: 'result' }) + expect(tracker.state?.tasks).toHaveLength(3) + roster([sibling, shell]) + expect(tracker.state?.settledTasks).toBeUndefined() + tracker.observe( + system('task_updated', { task_id: agent.task_id, patch: { status: 'completed' } }) + ) + tracker.observe( + system('task_notification', { + task_id: agent.task_id, + status: 'completed', + usage: { total_tokens: 19003 } + }) + ) + expect(tracker.state?.settledTasks).toEqual([ + expect.objectContaining({ + id: agent.task_id, + state: 'done', + startedAt: 100, + totalTokens: 19003 + }) + ]) + now = 150000 + roster([agent, sibling, shell]) + expect(tracker.state?.settledTasks).toBeUndefined() + expect(tracker.state?.tasks).toEqual([ + expect.objectContaining({ + id: agent.task_id, + state: 'working', + startedAt: 100, + totalTokens: 19003 + }), + expect.objectContaining({ id: sibling.task_id }), + expect.objectContaining({ id: shell.task_id }) + ]) + expect(tracker.stoppableTaskIds).toEqual([agent.task_id, sibling.task_id, shell.task_id]) + roster([sibling, shell]) + tracker.observe( + system('task_notification', { + task_id: agent.task_id, + status: 'completed', + usage: { total_tokens: 21000 } + }) + ) + expect(tracker.state?.settledTasks).toEqual([ + expect.objectContaining({ id: agent.task_id, startedAt: 100, totalTokens: 21000 }) + ]) + roster([]) + expect(tracker.state).toBeNull() + }) + + it('reconciles an edge-only resume without keeping its earlier settled copy', () => { + const tracker = new ClaudeBackgroundTaskTracker(() => 100) + for (const task of [agent, sibling]) { + tracker.observe(system('task_started', { ...task, is_backgrounded: true })) + } + tracker.observe(system('task_notification', { task_id: agent.task_id, status: 'completed' })) + tracker.observe( + system('task_updated', { + task_id: agent.task_id, + patch: { status: 'running', is_backgrounded: true } + }) + ) + expect(tracker.state?.tasks).toHaveLength(2) + expect(tracker.state?.settledTasks).toBeUndefined() + }) +}) diff --git a/src/main/claude/claude-background-task-tracker.test.ts b/src/main/claude/claude-background-task-tracker.test.ts index d8f316d7dcd..df89a64a968 100644 --- a/src/main/claude/claude-background-task-tracker.test.ts +++ b/src/main/claude/claude-background-task-tracker.test.ts @@ -16,6 +16,11 @@ function aggregate(tasks: unknown[]): Record { return system('background_tasks_changed', { tasks }) } +function trackerAt(times: number[]): ClaudeBackgroundTaskTracker { + let index = 0 + return new ClaudeBackgroundTaskTracker(() => times[Math.min(index++, times.length - 1)]) +} + describe('ClaudeBackgroundTaskTracker', () => { it('classifies SDK task types without inferring them from descriptions', () => { expect(classifyClaudeBackgroundTaskKind('local_agent')).toBe('agent') @@ -25,27 +30,30 @@ describe('ClaudeBackgroundTaskTracker', () => { expect(classifyClaudeBackgroundTaskKind('future_task')).toBe('unknown') }) - it('waits for the foreground turn to settle before monitoring a background task', () => { - const tracker = new ClaudeBackgroundTaskTracker() + it('publishes a backgrounded task while the foreground turn is still running', () => { + const tracker = trackerAt([100]) tracker.observe({ type: 'user' }, true) - tracker.observe( - system('task_started', { - task_id: 'task-1', - task_type: 'local_agent', - is_backgrounded: true - }) - ) - expect(tracker.state).toBeNull() - - expect(tracker.observe(result())).toBe(true) + expect( + tracker.observe( + system('task_started', { + task_id: 'task-1', + task_type: 'local_agent', + is_backgrounded: true + }) + ) + ).toBe(true) expect(tracker.state).toEqual({ state: 'monitoring', - tasks: [{ id: 'task-1', kind: 'agent' }] + tasks: [{ id: 'task-1', kind: 'agent', state: 'working', startedAt: 100 }] }) + + // The turn settling changes nothing the strip renders. + expect(tracker.observe(result())).toBe(false) + expect(tracker.state?.tasks).toHaveLength(1) }) it('uses an explicit background update for a foreground task and ignores progress alone', () => { - const tracker = new ClaudeBackgroundTaskTracker() + const tracker = trackerAt([100]) tracker.observe({ type: 'user' }, true) tracker.observe( system('task_started', { @@ -63,12 +71,12 @@ describe('ClaudeBackgroundTaskTracker', () => { tracker.observe(system('task_updated', { task_id: 'task-1', patch: { is_backgrounded: true } })) expect(tracker.state).toEqual({ state: 'monitoring', - tasks: [{ id: 'task-1', kind: 'command' }] + tasks: [{ id: 'task-1', kind: 'command', state: 'working', startedAt: 100 }] }) }) it('publishes bounded display details when a running task description changes', () => { - const tracker = new ClaudeBackgroundTaskTracker() + const tracker = trackerAt([100]) expect( tracker.observe( system('task_started', { @@ -81,7 +89,15 @@ describe('ClaudeBackgroundTaskTracker', () => { ).toBe(true) expect(tracker.state).toEqual({ state: 'monitoring', - tasks: [{ id: 'task-1', kind: 'command', description: 'run the build' }] + tasks: [ + { + id: 'task-1', + kind: 'command', + description: 'run the build', + state: 'working', + startedAt: 100 + } + ] }) expect( @@ -103,8 +119,193 @@ describe('ClaudeBackgroundTaskTracker', () => { ).toBe(false) }) + it('carries provider-reported names and re-derives classification per transition', () => { + const tracker = trackerAt([100]) + tracker.observe( + system('task_started', { + task_id: 'task-1', + task_type: 'future_task', + is_backgrounded: true + }) + ) + expect(tracker.state?.tasks?.[0]).toMatchObject({ kind: 'unknown' }) + + expect( + tracker.observe( + system('task_updated', { + task_id: 'task-1', + patch: { task_type: 'local_agent', agent_type: 'deep_review' } + }) + ) + ).toBe(true) + expect(tracker.state?.tasks?.[0]).toMatchObject({ + kind: 'agent', + name: 'deep_review', + state: 'working' + }) + }) + + it('retains settled siblings beside live work and exits with the last live task', () => { + const tracker = trackerAt([100, 200]) + tracker.observe( + system('task_started', { task_id: 'task-a', task_type: 'local_agent', is_backgrounded: true }) + ) + tracker.observe( + system('task_started', { task_id: 'task-b', task_type: 'local_agent', is_backgrounded: true }) + ) + + expect( + tracker.observe(system('task_updated', { task_id: 'task-a', patch: { status: 'completed' } })) + ).toBe(true) + expect(tracker.state).toEqual({ + state: 'monitoring', + tasks: [{ id: 'task-b', kind: 'agent', state: 'working', startedAt: 200 }], + settledTasks: [{ id: 'task-a', kind: 'agent', state: 'done', startedAt: 100 }] + }) + expect(tracker.stoppableTaskIds).toEqual(['task-b']) + + expect( + tracker.observe(system('task_updated', { task_id: 'task-b', patch: { status: 'killed' } })) + ).toBe(true) + expect(tracker.state).toBeNull() + }) + + it('settles a sibling from the captured producer order: aggregate eviction, then the outcome', () => { + // Verbatim sequence from a real SDK capture (2026-09-07): the aggregate + // roster arrives FIRST, already missing the finished task, and the + // terminal edges trail in the same tick. + const tracker = trackerAt([100, 200]) + tracker.observe( + system('task_started', { + task_id: 'bh4zn8der', + tool_use_id: 'toolu_01M', + description: 'Sleep for 5 seconds', + is_backgrounded: true, + task_type: 'local_bash' + }) + ) + tracker.observe( + aggregate([ + { task_id: 'bh4zn8der', task_type: 'local_bash', description: 'Sleep for 5 seconds' }, + { task_id: 'bprosaiim', task_type: 'local_bash', description: 'Sleep for 25 seconds' } + ]) + ) + + // The settling child is evicted by the aggregate before any outcome frame. + tracker.observe( + aggregate([ + { task_id: 'bprosaiim', task_type: 'local_bash', description: 'Sleep for 25 seconds' } + ]) + ) + tracker.observe( + system('task_updated', { + task_id: 'bh4zn8der', + patch: { status: 'completed', end_time: 1788804376515 } + }) + ) + expect( + tracker.observe( + system('task_notification', { + task_id: 'bh4zn8der', + tool_use_id: 'toolu_01M', + status: 'completed', + summary: 'Background command "Sleep for 5 seconds" completed (exit code 0)', + usage: { total_tokens: 18130, tool_uses: 1, duration_ms: 10772 } + }) + ) + ).toBe(true) + expect(tracker.state).toEqual({ + state: 'monitoring', + tasks: [ + { + id: 'bprosaiim', + kind: 'command', + description: 'Sleep for 25 seconds', + state: 'working', + startedAt: 200 + } + ], + settledTasks: [ + { + id: 'bh4zn8der', + kind: 'command', + description: 'Sleep for 5 seconds', + state: 'done', + startedAt: 100, + totalTokens: 18130 + } + ] + }) + + // Last task killed, same captured order: the strip exits. + tracker.observe(aggregate([])) + tracker.observe(system('task_updated', { task_id: 'bprosaiim', patch: { status: 'killed' } })) + tracker.observe(system('task_notification', { task_id: 'bprosaiim', status: 'stopped' })) + expect(tracker.state).toBeNull() + }) + + it('carries task_progress usage into a live row without clobbering its name', () => { + const tracker = trackerAt([100]) + tracker.observe( + system('task_started', { + task_id: 'agent-1', + task_type: 'local_agent', + subagent_type: 'general-purpose', + description: 'Sleep 6 seconds test', + is_backgrounded: true + }) + ) + expect( + tracker.observe( + system('task_progress', { + task_id: 'agent-1', + description: 'Running Sleep for 6 seconds', + subagent_type: 'general-purpose', + usage: { total_tokens: 14866, tool_uses: 1, duration_ms: 2818 }, + last_tool_name: 'Bash' + }) + ) + ).toBe(true) + expect(tracker.state?.tasks?.[0]).toEqual({ + id: 'agent-1', + kind: 'agent', + // Progress descriptions are transient activity, never the task's name. + description: 'Sleep 6 seconds test', + name: 'general-purpose', + state: 'working', + startedAt: 100, + totalTokens: 14866 + }) + }) + + it('maps terminal statuses onto settled states', () => { + const tracker = trackerAt([100, 200]) + tracker.observe( + system('task_started', { task_id: 'live', task_type: 'local_agent', is_backgrounded: true }) + ) + tracker.observe( + system('task_started', { task_id: 'failed', task_type: 'local_agent', is_backgrounded: true }) + ) + tracker.observe(system('task_notification', { task_id: 'failed', status: 'failed' })) + expect(tracker.state?.settledTasks).toEqual([ + { id: 'failed', kind: 'agent', state: 'blocked', startedAt: 200 } + ]) + }) + + it('leaves a task open when a patch cannot be read', () => { + const tracker = trackerAt([100]) + tracker.observe( + system('task_started', { task_id: 'task-1', task_type: 'local_agent', is_backgrounded: true }) + ) + expect(tracker.observe(system('task_updated', { task_id: 'task-1', patch: 'garbage' }))).toBe( + false + ) + expect(tracker.state?.tasks).toHaveLength(1) + expect(tracker.state?.settledTasks).toBeUndefined() + }) + it('replaces its roster from aggregate lifecycle frames and preserves stoppable provider ids', () => { - const tracker = new ClaudeBackgroundTaskTracker() + const tracker = trackerAt([100]) expect( tracker.observe( aggregate([ @@ -117,8 +318,8 @@ describe('ClaudeBackgroundTaskTracker', () => { expect(tracker.state).toEqual({ state: 'monitoring', tasks: [ - { id: 'task-agent', kind: 'agent', description: 'agent' }, - { id: 'task-bash', kind: 'command', description: 'bash' } + { id: 'task-agent', kind: 'agent', description: 'agent', state: 'working', startedAt: 100 }, + { id: 'task-bash', kind: 'command', description: 'bash', state: 'working', startedAt: 100 } ] }) @@ -128,18 +329,31 @@ describe('ClaudeBackgroundTaskTracker', () => { ) ).toBe(true) expect(tracker.stoppableTaskIds).toEqual(['task-next']) - expect(tracker.state).toEqual({ - state: 'monitoring', - tasks: [{ id: 'task-next', kind: 'workflow', description: 'workflow' }] - }) expect(tracker.observe(aggregate([]))).toBe(true) expect(tracker.stoppableTaskIds).toEqual([]) expect(tracker.state).toBeNull() }) + it('preserves first-seen timestamps across aggregate roster replacement', () => { + const tracker = trackerAt([100, 200]) + tracker.observe( + system('task_started', { task_id: 'task-1', task_type: 'local_agent', is_backgrounded: true }) + ) + tracker.observe( + aggregate([ + { task_id: 'task-1', task_type: 'local_agent' }, + { task_id: 'task-2', task_type: 'local_bash' } + ]) + ) + expect(tracker.state?.tasks).toEqual([ + { id: 'task-1', kind: 'agent', state: 'working', startedAt: 100 }, + { id: 'task-2', kind: 'command', state: 'working', startedAt: 200 } + ]) + }) + it('excludes ambient aggregate tasks', () => { - const tracker = new ClaudeBackgroundTaskTracker() + const tracker = trackerAt([100]) tracker.observe( aggregate([ { task_id: 'ambient', task_type: 'monitor', description: 'watcher', ambient: true }, @@ -151,7 +365,7 @@ describe('ClaudeBackgroundTaskTracker', () => { }) it('does not let late edge frames revive tasks cleared by an aggregate roster', () => { - const tracker = new ClaudeBackgroundTaskTracker() + const tracker = trackerAt([100]) tracker.observe( aggregate([{ task_id: 'task-late', task_type: 'local_agent', description: 'agent' }]) ) @@ -173,7 +387,7 @@ describe('ClaudeBackgroundTaskTracker', () => { }) it('lets an authoritative aggregate roster replace earlier terminal-edge evidence', () => { - const tracker = new ClaudeBackgroundTaskTracker() + const tracker = trackerAt([100]) tracker.observe(system('task_notification', { task_id: 'task-live', status: 'completed' })) tracker.observe( @@ -183,12 +397,28 @@ describe('ClaudeBackgroundTaskTracker', () => { expect(tracker.stoppableTaskIds).toEqual(['task-live']) expect(tracker.state).toEqual({ state: 'monitoring', - tasks: [{ id: 'task-live', kind: 'agent', description: 'agent' }] + tasks: [ + { id: 'task-live', kind: 'agent', description: 'agent', state: 'working', startedAt: 100 } + ] }) }) + it('retracts a settled copy when an authoritative roster reports the task live again', () => { + const tracker = trackerAt([100, 200, 300]) + const tasks = [ + { task_id: 'agent', task_type: 'local_agent', description: 'Review sample' }, + { task_id: 'shell', task_type: 'local_bash' } + ] + tracker.observe(aggregate(tasks)) + tracker.observe(system('task_notification', { task_id: 'agent', status: 'completed' })) + expect(tracker.state?.settledTasks).toHaveLength(1) + tracker.observe(aggregate(tasks)) + expect(tracker.state?.tasks?.map((task) => task.id)).toEqual(['agent', 'shell']) + expect(tracker.state?.settledTasks).toBeUndefined() + }) + it('keeps terminal edges authoritative on either side of aggregate replacement', () => { - const terminalFirst = new ClaudeBackgroundTaskTracker() + const terminalFirst = trackerAt([100]) terminalFirst.observe( system('task_notification', { task_id: 'task-first', status: 'completed' }) ) @@ -202,7 +432,7 @@ describe('ClaudeBackgroundTaskTracker', () => { ) expect(terminalFirst.state).toBeNull() - const terminalLast = new ClaudeBackgroundTaskTracker() + const terminalLast = trackerAt([100]) terminalLast.observe( aggregate([{ task_id: 'task-last', task_type: 'local_agent', description: 'agent' }]) ) @@ -218,7 +448,7 @@ describe('ClaudeBackgroundTaskTracker', () => { }) it('keeps terminal evidence authoritative across duplicates and out-of-order starts', () => { - const tracker = new ClaudeBackgroundTaskTracker() + const tracker = trackerAt([100]) const terminal = system('task_notification', { task_id: 'task-late', status: 'completed' }) tracker.observe(terminal) tracker.observe(terminal) @@ -239,7 +469,7 @@ describe('ClaudeBackgroundTaskTracker', () => { ) expect(tracker.state).toEqual({ state: 'monitoring', - tasks: [{ id: 'task-live', kind: 'monitor' }] + tasks: [{ id: 'task-live', kind: 'monitor', state: 'monitoring', startedAt: 100 }] }) expect( tracker.observe(system('task_updated', { task_id: 'task-live', patch: { status: 'killed' } })) @@ -249,17 +479,24 @@ describe('ClaudeBackgroundTaskTracker', () => { it('recognizes task types that are registered only as background work', () => { for (const taskType of ['local_workflow', 'monitor']) { - const tracker = new ClaudeBackgroundTaskTracker() + const tracker = trackerAt([100]) tracker.observe(system('task_started', { task_id: taskType, task_type: taskType })) expect(tracker.state).toEqual({ state: 'monitoring', - tasks: [{ id: taskType, kind: taskType === 'local_workflow' ? 'workflow' : 'monitor' }] + tasks: [ + { + id: taskType, + kind: taskType === 'local_workflow' ? 'workflow' : 'monitor', + state: taskType === 'local_workflow' ? 'working' : 'monitoring', + startedAt: 100 + } + ] }) } }) it('admits unknown background updates conservatively and bounds edge-only fallback ids', () => { - const tracker = new ClaudeBackgroundTaskTracker() + const tracker = trackerAt([100]) tracker.observe( system('task_updated', { task_id: 'unknown', patch: { is_backgrounded: true } }) ) @@ -278,7 +515,7 @@ describe('ClaudeBackgroundTaskTracker', () => { }) it('bounds aggregate rosters and resets to the edge-only fallback on clear', () => { - const tracker = new ClaudeBackgroundTaskTracker() + const tracker = trackerAt([100]) tracker.observe( aggregate( Array.from({ length: 400 }, (_, index) => ({ @@ -301,23 +538,30 @@ describe('ClaudeBackgroundTaskTracker', () => { expect(tracker.stoppableTaskIds).toEqual(['edge-after-reset']) }) - it('gates aggregate monitoring behind foreground turn completion', () => { - const tracker = new ClaudeBackgroundTaskTracker() + it('publishes an aggregate roster observed mid-turn', () => { + const tracker = trackerAt([100]) tracker.observe({ type: 'user' }, true) - tracker.observe( - aggregate([{ task_id: 'task-live', task_type: 'local_bash', description: 'command' }]) - ) - expect(tracker.state).toBeNull() - - expect(tracker.observe(result())).toBe(true) + expect( + tracker.observe( + aggregate([{ task_id: 'task-live', task_type: 'local_bash', description: 'command' }]) + ) + ).toBe(true) expect(tracker.state).toEqual({ state: 'monitoring', - tasks: [{ id: 'task-live', kind: 'command', description: 'command' }] + tasks: [ + { + id: 'task-live', + kind: 'command', + description: 'command', + state: 'working', + startedAt: 100 + } + ] }) }) it('ignores ambient SDK tasks and clears all liveness when the session ends', () => { - const tracker = new ClaudeBackgroundTaskTracker() + const tracker = trackerAt([100]) tracker.observe( system('task_started', { task_id: 'ambient', diff --git a/src/main/claude/claude-background-task-tracker.ts b/src/main/claude/claude-background-task-tracker.ts index de24b9a4fba..68c7f4f1185 100644 --- a/src/main/claude/claude-background-task-tracker.ts +++ b/src/main/claude/claude-background-task-tracker.ts @@ -1,78 +1,54 @@ import type { AgentSessionBackgroundTask, + AgentSessionBackgroundTaskRunState, AgentSessionBackgroundTaskState } from '../../shared/agent-session-wire' +import { + classifyClaudeBackgroundTaskKind, + liveClaudeTaskRunState, + record, + taskDescription, + taskId, + taskName, + taskUsageTotalTokens, + terminalClaudeTaskRunState +} from './claude-background-task-frames' +import { + ClaudeSettledBackgroundTasks, + claudeBackgroundTaskDetail, + type TrackedClaudeBackgroundTask +} from './claude-settled-background-tasks' + +// `claude-subagent-*` reads this channel through these names; the readers themselves +// live in the frames module so both consumers share one definition. +export { + classifyClaudeBackgroundTaskKind, + isBoundedClaudeTaskId, + taskDescription as claudeTaskDescription, + taskId as claudeTaskId +} from './claude-background-task-frames' +export type { ClaudeBackgroundTaskKind } from './claude-background-task-frames' const MAX_TRACKED_TASKS = 256 -const MAX_TASK_ID_LENGTH = 512 -const MAX_TASK_DESCRIPTION_LENGTH = 512 -const TERMINAL_TASK_STATES = new Set(['completed', 'failed', 'killed', 'stopped']) - -export type ClaudeBackgroundTaskKind = AgentSessionBackgroundTask['kind'] - -type TrackedTask = { - backgrounded: boolean - kind: ClaudeBackgroundTaskKind - description?: string -} - -function record(value: unknown): Record | null { - return typeof value === 'object' && value !== null ? (value as Record) : null -} - -/** The bound every task id shares, wherever it enters. An id the roster stores - * becomes a durable entry key, so a provisional one takes the same bound the - * announced path applies — an over-long id is rejected, never truncated. */ -export function isBoundedClaudeTaskId(value: string): boolean { - return value.length > 0 && value.length <= MAX_TASK_ID_LENGTH -} - -/** The task's canonical, resume-stable id. Shared with the subagent roster so - * both readers of this channel agree on what identifies a task. */ -export function claudeTaskId(message: Record): string | null { - const value = message.task_id - return typeof value === 'string' && isBoundedClaudeTaskId(value) ? value : null -} - -/** A task's human label, collapsed and bounded. */ -export function claudeTaskDescription(value: unknown): string | undefined { - if (typeof value !== 'string') { - return undefined - } - const trimmed = value.trim().replace(/\s+/g, ' ') - return trimmed.length > 0 ? trimmed.slice(0, MAX_TASK_DESCRIPTION_LENGTH) : undefined -} - -export function classifyClaudeBackgroundTaskKind(taskType: unknown): ClaudeBackgroundTaskKind { - switch (taskType) { - case 'local_agent': - return 'agent' - case 'local_workflow': - return 'workflow' - case 'local_bash': - return 'command' - case 'monitor': - return 'monitor' - default: - return 'unknown' - } -} export class ClaudeBackgroundTaskTracker { - private readonly tasks = new Map() + private readonly tasks = new Map() + private readonly retention = new ClaudeSettledBackgroundTasks() private readonly terminalTaskIds = new Set() private aggregateRosterObserved = false - private foregroundTurnActive = false private monitoring = false private publishedTasksFingerprint = '' + constructor(private readonly now: () => number = () => Date.now()) {} + get state(): AgentSessionBackgroundTaskState | null { if (!this.monitoring) { return null } return { state: 'monitoring', - tasks: this.backgroundTaskDetails() + tasks: this.backgroundTaskDetails(), + ...(this.retention.hasSettled ? { settledTasks: this.retention.settledDetails() } : {}) } } @@ -87,16 +63,14 @@ export class ClaudeBackgroundTaskTracker { } observe(message: Record, startsTurn = false): boolean { - if (startsTurn) { - this.foregroundTurnActive = true - } - if (message.type === 'result') { - this.foregroundTurnActive = false - } else if (message.type === 'system') { + // Background work publishes through a foreground turn: the strip stays + // honest mid-fan-out and the client alone decides when the idle-only + // monitoring label may speak. + if (message.type === 'system') { if (!this.observeSystemFrame(message) && !startsTurn) { return false } - } else if (!startsTurn) { + } else if (!startsTurn && message.type !== 'result') { return false } return this.refreshMonitoring() @@ -104,47 +78,51 @@ export class ClaudeBackgroundTaskTracker { clear(): boolean { this.tasks.clear() + this.retention.clear() this.terminalTaskIds.clear() this.aggregateRosterObserved = false - this.foregroundTurnActive = false return this.refreshMonitoring() } + private settle( + id: string, + state: AgentSessionBackgroundTaskRunState, + outcome: { totalTokens?: number } = {} + ): void { + this.retention.settle(id, state, outcome, this.tasks.get(id)) + this.finish(id) + } + private observeSystemFrame(message: Record): boolean { if (message.subtype === 'background_tasks_changed') { this.replaceAggregateRoster(message.tasks) return true } - const id = claudeTaskId(message) + const id = taskId(message) if (!id) { return false } if (message.subtype === 'task_notification') { - this.finish(id) + // The notification is affirmative terminal evidence even when its status + // field is unreadable — matching the liveness semantics this edge always had. + this.settle(id, terminalClaudeTaskRunState(message.status) ?? 'done', { + totalTokens: taskUsageTotalTokens(message) + }) + return true + } + if (message.subtype === 'task_progress') { + // Progress `description` is the current activity ("Running "), not + // the task's name — only usage (and a missing identity) may update. + const existing = this.tasks.get(id) + const totalTokens = taskUsageTotalTokens(message) + if (!existing?.backgrounded || totalTokens === undefined) { + return false + } + this.tasks.set(id, { ...existing, totalTokens, name: existing.name ?? taskName(message) }) return true } if (message.subtype === 'task_updated') { - const patch = record(message.patch) - if (!patch) { - return false - } - if (TERMINAL_TASK_STATES.has(String(patch.status))) { - this.finish(id) - return true - } - const existing = this.tasks.get(id) - if ( - (patch.is_backgrounded === true || claudeTaskDescription(patch.description)) && - (!this.aggregateRosterObserved || existing) - ) { - this.upsert(id, { - backgrounded: patch.is_backgrounded === true || existing?.backgrounded === true, - kind: existing?.kind ?? 'unknown', - description: claudeTaskDescription(patch.description) ?? existing?.description - }) - return true - } - return false + return this.observeTaskUpdated(id, message) } if (message.subtype !== 'task_started' || this.terminalTaskIds.has(id)) { return false @@ -160,15 +138,55 @@ export class ClaudeBackgroundTaskTracker { this.upsert(id, { backgrounded: message.is_backgrounded === true || kind === 'workflow' || kind === 'monitor', kind, - description: claudeTaskDescription(message.description) + description: taskDescription(message.description), + name: taskName(message), + state: liveClaudeTaskRunState(message.status) ?? undefined, + startedAt: this.now() }) return true } + private observeTaskUpdated(id: string, message: Record): boolean { + const patch = record(message.patch) + if (!patch) { + return false + } + const settledState = terminalClaudeTaskRunState(patch.status) + if (settledState) { + this.settle(id, settledState) + return true + } + const existing = this.tasks.get(id) + // Classification is re-derived per transition: a later frame that reveals a + // real type moves the task between buckets instead of pinning first-seen. + const patchKind = + 'task_type' in patch ? classifyClaudeBackgroundTaskKind(patch.task_type) : undefined + const liveState = liveClaudeTaskRunState(patch.status) + const hasContent = + patch.is_backgrounded === true || + taskDescription(patch.description) !== undefined || + taskName(patch) !== undefined || + liveState !== null || + (patchKind !== undefined && patchKind !== 'unknown') + if (hasContent && (!this.aggregateRosterObserved || existing)) { + this.upsert(id, { + backgrounded: patch.is_backgrounded === true || existing?.backgrounded === true, + kind: patchKind ?? existing?.kind ?? 'unknown', + description: taskDescription(patch.description), + name: taskName(patch), + state: liveState ?? undefined, + startedAt: this.now() + }) + return true + } + return false + } + private replaceAggregateRoster(value: unknown): void { if (!Array.isArray(value)) { return } + const prior = new Map(this.tasks) this.aggregateRosterObserved = true this.tasks.clear() this.terminalTaskIds.clear() @@ -180,29 +198,33 @@ export class ClaudeBackgroundTaskTracker { if (!task || task.ambient === true) { continue } - const id = claudeTaskId(task) + const id = taskId(task) if (!id) { continue } + // An authoritative live roster supersedes an earlier terminal edge. + const retained = this.retention.resume(id) + const existing = prior.get(id) ?? retained + const kind = classifyClaudeBackgroundTaskKind(task.task_type) this.tasks.set(id, { backgrounded: true, - kind: classifyClaudeBackgroundTaskKind(task.task_type), - description: claudeTaskDescription(task.description) + kind: kind !== 'unknown' ? kind : (existing?.kind ?? 'unknown'), + description: taskDescription(task.description) ?? existing?.description, + name: taskName(task) ?? existing?.name, + state: liveClaudeTaskRunState(task.status) ?? existing?.state, + startedAt: existing?.startedAt ?? this.now(), + totalTokens: existing?.totalTokens }) } + for (const [id, task] of prior) { + if (task.backgrounded && !this.tasks.has(id)) { + this.retention.rememberRemoved(id, task) + } + } } - private upsert(id: string, task: TrackedTask): void { - const existing = this.tasks.get(id) - if (existing) { - this.tasks.set(id, { - backgrounded: existing.backgrounded || task.backgrounded, - kind: existing.kind === 'unknown' ? task.kind : existing.kind, - description: task.description ?? existing.description - }) - return - } - if (this.tasks.size >= MAX_TRACKED_TASKS) { + private upsert(id: string, task: TrackedClaudeBackgroundTask): void { + if (!this.tasks.has(id) && this.tasks.size >= MAX_TRACKED_TASKS) { let foregroundId: string | undefined for (const [candidateId, candidate] of this.tasks) { if (!candidate.backgrounded) { @@ -215,6 +237,20 @@ export class ClaudeBackgroundTaskTracker { } this.tasks.delete(foregroundId) } + const existing = this.tasks.get(id) ?? this.retention.resume(id) + this.terminalTaskIds.delete(id) + if (existing) { + this.tasks.set(id, { + backgrounded: existing.backgrounded || task.backgrounded, + kind: task.kind !== 'unknown' ? task.kind : existing.kind, + description: task.description ?? existing.description, + name: task.name ?? existing.name, + state: task.state ?? existing.state, + startedAt: existing.startedAt, + totalTokens: existing.totalTokens + }) + return + } this.tasks.set(id, task) } @@ -231,9 +267,12 @@ export class ClaudeBackgroundTaskTracker { } private refreshMonitoring(): boolean { - const details = this.foregroundTurnActive ? [] : this.backgroundTaskDetails() + const details = this.backgroundTaskDetails() + if (details.length === 0 && this.retention.hasSettled) { + this.retention.flushSettled() + } const next = details.length > 0 - const fingerprint = next ? JSON.stringify(details) : '' + const fingerprint = next ? JSON.stringify([details, this.retention.settledDetails()]) : '' if (next === this.monitoring && fingerprint === this.publishedTasksFingerprint) { return false } @@ -248,11 +287,7 @@ export class ClaudeBackgroundTaskTracker { if (!task.backgrounded) { continue } - details.push({ - id, - kind: task.kind, - ...(task.description ? { description: task.description } : {}) - }) + details.push(claudeBackgroundTaskDetail(id, task)) } return details } diff --git a/src/main/claude/claude-settled-background-tasks.ts b/src/main/claude/claude-settled-background-tasks.ts new file mode 100644 index 00000000000..91e975d07bb --- /dev/null +++ b/src/main/claude/claude-settled-background-tasks.ts @@ -0,0 +1,127 @@ +// Retention state for background tasks that have reached a terminal edge. +// +// The real producer settles a task in two steps inside one tick: +// `background_tasks_changed` arrives FIRST with the task already absent, then +// `task_updated` / `task_notification` carry the outcome. So the terminal edge +// must be able to settle a task the live roster no longer holds — that is what +// `rememberRemoved` preserves. A removal whose outcome frame never arrives +// simply vanishes: removed tasks are never rendered and never guessed into a +// finished state. + +import type { + AgentSessionBackgroundTask, + AgentSessionBackgroundTaskRunState +} from '../../shared/agent-session-wire' + +const MAX_RETAINED_TASKS = 256 + +export type TrackedClaudeBackgroundTask = { + backgrounded: boolean + kind: AgentSessionBackgroundTask['kind'] + description?: string + name?: string + state?: AgentSessionBackgroundTaskRunState + /** First-observed epoch ms; preserved across updates and roster replacement + * so clients can render elapsed and keep a stable first-seen sort. */ + startedAt: number + totalTokens?: number +} + +export function claudeBackgroundTaskDetail( + id: string, + task: TrackedClaudeBackgroundTask +): AgentSessionBackgroundTask { + return { + id, + kind: task.kind, + ...(task.description ? { description: task.description } : {}), + ...(task.name ? { name: task.name } : {}), + state: task.state ?? (task.kind === 'monitor' ? 'monitoring' : 'working'), + startedAt: task.startedAt, + ...(task.totalTokens !== undefined ? { totalTokens: task.totalTokens } : {}) + } +} + +function setBounded(map: Map, key: K, value: V): void { + map.delete(key) + map.set(key, value) + if (map.size > MAX_RETAINED_TASKS) { + const oldest = map.keys().next() + if (!oldest.done) { + map.delete(oldest.value) + } + } +} + +export class ClaudeSettledBackgroundTasks { + private readonly settled = new Map() + private readonly recentlyRemoved = new Map() + + /** An aggregate roster evicted a still-live backgrounded task; hold its + * details so the outcome frame trailing in the same tick can settle it. */ + rememberRemoved(id: string, task: TrackedClaudeBackgroundTask): void { + setBounded(this.recentlyRemoved, id, task) + } + + /** Terminal edge for `id`. `liveSource` is the live roster's entry when it + * still has one; otherwise the recently-removed copy is consumed. A second + * edge (updated, then notification) re-derives the settled state and can + * add the final usage the first edge lacked. */ + settle( + id: string, + state: AgentSessionBackgroundTaskRunState, + outcome: { totalTokens?: number }, + liveSource: TrackedClaudeBackgroundTask | undefined + ): void { + const source = liveSource ?? this.recentlyRemoved.get(id) + const already = this.settled.get(id) + if (source?.backgrounded) { + setBounded(this.settled, id, { + ...claudeBackgroundTaskDetail(id, { + ...source, + totalTokens: outcome.totalTokens ?? source.totalTokens + }), + state + }) + } else if (already) { + this.settled.set(id, { + ...already, + state, + ...(outcome.totalTokens !== undefined ? { totalTokens: outcome.totalTokens } : {}) + }) + } + this.recentlyRemoved.delete(id) + } + + /** Positive live evidence transfers identity back to the tracker, never the old outcome. */ + resume(id: string): TrackedClaudeBackgroundTask | undefined { + const settled = this.settled.get(id) + const removed = this.recentlyRemoved.get(id) + this.settled.delete(id) + this.recentlyRemoved.delete(id) + const source = settled ?? removed + if (!source || source.startedAt === undefined) { + return undefined + } + return { ...source, backgrounded: true, state: undefined, startedAt: source.startedAt } + } + + get hasSettled(): boolean { + return this.settled.size > 0 + } + + settledDetails(): AgentSessionBackgroundTask[] { + return [...this.settled.values()] + } + + /** Settled context only makes sense beside live work; the strip exits at the + * same instant it always has — when the last live task ends. */ + flushSettled(): void { + this.settled.clear() + } + + clear(): void { + this.settled.clear() + this.recentlyRemoved.clear() + } +} diff --git a/src/main/claude/claude-structured-session-close.test.ts b/src/main/claude/claude-structured-session-close.test.ts index 670c0daaf8c..0de5049a71c 100644 --- a/src/main/claude/claude-structured-session-close.test.ts +++ b/src/main/claude/claude-structured-session-close.test.ts @@ -55,7 +55,9 @@ describe('Claude published session close lifecycle', () => { expect(backgroundStates).toEqual([ { state: 'monitoring', - tasks: [{ id: 'background-1', kind: 'agent' }], + tasks: [ + { id: 'background-1', kind: 'agent', state: 'working', startedAt: expect.any(Number) } + ], supportsTaskStop: true } ]) @@ -74,7 +76,9 @@ describe('Claude published session close lifecycle', () => { expect(backgroundStates).toEqual([ { state: 'monitoring', - tasks: [{ id: 'background-1', kind: 'agent' }], + tasks: [ + { id: 'background-1', kind: 'agent', state: 'working', startedAt: expect.any(Number) } + ], supportsTaskStop: true }, null diff --git a/src/main/native-chat/agent-session-wire/provider-frame-activity.test.ts b/src/main/native-chat/agent-session-wire/provider-frame-activity.test.ts index ba2eba58062..c8860f227cc 100644 --- a/src/main/native-chat/agent-session-wire/provider-frame-activity.test.ts +++ b/src/main/native-chat/agent-session-wire/provider-frame-activity.test.ts @@ -50,7 +50,9 @@ describe('provider frame activity', () => { expect(claudeProviderFrameActivity(kind, payload)).toBeNull() } // `requesting` holds for nearly the whole turn and says no more than the fallback. - expect(claudeProviderFrameActivity('message:system:status', { status: 'requesting' })).toBeNull() + expect( + claudeProviderFrameActivity('message:system:status', { status: 'requesting' }) + ).toBeNull() expect(claudeProviderFrameActivity('message:system:status', { status: 'compacting' })).toBe( 'Compacting the conversation' ) diff --git a/src/main/native-chat/agent-session-wire/structured-agent-session-background-task-channel.ts b/src/main/native-chat/agent-session-wire/structured-agent-session-background-task-channel.ts index 4592dea26e9..6dd9db80e1d 100644 --- a/src/main/native-chat/agent-session-wire/structured-agent-session-background-task-channel.ts +++ b/src/main/native-chat/agent-session-wire/structured-agent-session-background-task-channel.ts @@ -21,7 +21,10 @@ export class StructuredAgentSessionBackgroundTaskChannel { private readonly requireSession: (sessionId: string) => StructuredAgentSessionHostSession, private readonly handoffStatus: ( sessionId: string - ) => Parameters[0]['handoff'] + ) => Parameters[0]['handoff'], + /** Task edges change the status summary too; the feed's equality check + * keeps a no-op re-projection from reaching subscribers. */ + private readonly onPublished: (sessionId: string) => void ) {} history(request: AgentSessionHistoryRequest): AgentSessionHistoryResult { @@ -53,6 +56,7 @@ export class StructuredAgentSessionBackgroundTaskChannel { const state = publishedState !== undefined ? publishedState : this.state(sessionId) if (session && state !== undefined) { this.subscribers.backgroundTasks(sessionId, state, session.fence) + this.onPublished(sessionId) } } diff --git a/src/main/native-chat/agent-session-wire/structured-agent-session-host.ts b/src/main/native-chat/agent-session-wire/structured-agent-session-host.ts index 2df2fdf5412..358ca952b57 100644 --- a/src/main/native-chat/agent-session-wire/structured-agent-session-host.ts +++ b/src/main/native-chat/agent-session-wire/structured-agent-session-host.ts @@ -53,7 +53,8 @@ import type { StructuredAgentSessionHostSession, StructuredAgentSessionReveal } from './structured-agent-session-host-types' -import { StructuredAgentSessionStatusFeed } from './structured-agent-session-status-feed' +import { createStructuredAgentSessionHostStatusFeed } from './structured-agent-session-status-feed' +import type { StructuredAgentSessionStatusSubscriber } from './structured-agent-session-status-feed' import { StructuredAgentSessionEventRecovery } from './structured-agent-session-event-recovery' import { StructuredAgentSessionBackgroundTaskChannel } from './structured-agent-session-background-task-channel' export type { StructuredAgentSessionHostDeps } from './structured-agent-session-host-types' @@ -64,11 +65,10 @@ export class StructuredAgentSessionHost { this ) private readonly sessions = new Map() - private readonly statusFeed = new StructuredAgentSessionStatusFeed({ + private readonly statusFeed = createStructuredAgentSessionHostStatusFeed({ sessions: this.sessions, - getRecord: (sessionId) => this.deps.store.getRecord(sessionId), now: () => this.now(), - onStatusChanged: (summary, options) => this.deps.onSessionStatusChanged?.(summary, options) + deps: () => this.deps }) private readonly subscribers = new AgentSessionSubscribers({ readCommands: (sessionId) => this.deps.adapter.readCommands?.(sessionId), @@ -91,7 +91,8 @@ export class StructuredAgentSessionHost { this.sessions, this.subscribers, (sessionId) => this.requireSession(sessionId), - (sessionId) => this.handoffs.status(sessionId) + (sessionId) => this.handoffs.status(sessionId), + (sessionId) => this.statusFeed.publish(sessionId) ) this.runtimeState = new StructuredAgentSessionHostRuntimeState( deps, @@ -345,7 +346,7 @@ export class StructuredAgentSessionHost { unsubscribe = (sessionId: string, id: string): void => this.subscribers.close(sessionId, id) /** Every session's projected status for session lists; unlike `subscribe`, retains nothing. */ - subscribeStatus: StructuredAgentSessionStatusFeed['subscribe'] = (subscriber) => + subscribeStatus = (subscriber: StructuredAgentSessionStatusSubscriber): (() => void) => this.statusFeed.subscribe(subscriber) private requireSession(sessionId: string): StructuredAgentSessionHostSession { diff --git a/src/main/native-chat/agent-session-wire/structured-agent-session-status-feed.test.ts b/src/main/native-chat/agent-session-wire/structured-agent-session-status-feed.test.ts index 7bd53742430..5fbd58016d0 100644 --- a/src/main/native-chat/agent-session-wire/structured-agent-session-status-feed.test.ts +++ b/src/main/native-chat/agent-session-wire/structured-agent-session-status-feed.test.ts @@ -1,9 +1,12 @@ import { mkdtemp, rm } from 'node:fs/promises' import { tmpdir } from 'node:os' import { join } from 'node:path' -import { afterEach, beforeEach, describe, expect, it } from 'vitest' +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' import type { AgentSessionRecord } from '../../../shared/agent-session-record' -import type { AgentSessionStatusEvent } from '../../../shared/agent-session-wire' +import type { + AgentSessionBackgroundTask, + AgentSessionStatusEvent +} from '../../../shared/agent-session-wire' import { createClaudeJournalTranslator } from '../../claude/claude-structured-journal-translation' import { publishCodexTurnLifecycle } from '../../codex/codex-structured-journal-translation-turns' import { createDeferredStructuredAgentSessionEventSink } from './structured-agent-session-event-sink' @@ -74,11 +77,13 @@ function feedFor( { journal: Awaited>; hasProviderChild?: boolean; fence?: number } >, record: Partial | null = null, - onStatusChanged?: StructuredAgentSessionStatusFeedDeps['onStatusChanged'] + onStatusChanged?: StructuredAgentSessionStatusFeedDeps['onStatusChanged'], + readBackgroundTasks?: StructuredAgentSessionStatusFeedDeps['readBackgroundTasks'] ) { let now = 1_000 const feed = new StructuredAgentSessionStatusFeed({ ...(onStatusChanged ? { onStatusChanged } : {}), + ...(readBackgroundTasks ? { readBackgroundTasks } : {}), sessions: { get: (sessionId: string) => { const session = sessions.get(sessionId) @@ -621,6 +626,147 @@ describe('StructuredAgentSessionStatusFeed', () => { session: expect.objectContaining({ status: 'idle', latestPrompt: 'hello' }) }) }) + + it('reuses the journal projection across task progress and invalidates on journal changes', async () => { + const journal = await openJournal() + await journal.appendItem( + USER_IDENTITY, + { kind: 'message', role: 'user', blocks: [{ type: 'text', text: 'fan out' }] }, + { fence: 1 } + ) + await journal.appendItem( + TURN_IDENTITY, + { kind: 'status', text: 'Working', turnLifecycle: { turnId: 'turn-1', state: 'running' } }, + { fence: 1 } + ) + const snapshot = vi.spyOn(journal, 'snapshot') + let taskState: 'working' | 'waiting' = 'working' + const { feed, events } = feedFor(new Map([[SESSION, { journal }]]), null, undefined, () => ({ + state: 'monitoring', + tasks: [{ id: 'child', kind: 'agent', state: taskState }] + })) + for (let tick = 1; tick <= 100; tick++) { + taskState = tick % 2 === 1 ? 'waiting' : 'working' + feed.publish(SESSION) + } + expect(events).toHaveLength(101) + expect(snapshot).toHaveBeenCalledTimes(1) + expect(events.at(-1)).toMatchObject({ + type: 'status', + session: { status: 'working', backgroundTasks: [{ state: 'working' }] } + }) + await journal.appendTombstone(TURN_IDENTITY, { fence: 1 }) + feed.publish(SESSION) + expect(snapshot).toHaveBeenCalledTimes(2) + expect(events.at(-1)).toMatchObject({ type: 'status', session: { status: 'idle' } }) + }) + + it('invalidates cached status on unreadability and keeps record metadata live', async () => { + const journal = await openJournal() + await journal.appendItem( + USER_IDENTITY, + { kind: 'message', role: 'user', blocks: [{ type: 'text', text: 'hello' }] }, + { fence: 1 } + ) + const record = { options: { model: 'first-model' }, providerHandleChain: [] } + const { feed, events } = feedFor(new Map([[SESSION, { journal }]]), record) + record.options.model = 'second-model' + feed.publish(SESSION) + expect(events.at(-1)).toMatchObject({ + type: 'status', + session: { status: 'idle', model: 'second-model' } + }) + const readOnly = vi.spyOn(journal, 'isReadOnly', 'get').mockReturnValue(true) + feed.publish(SESSION) + expect(events.at(-1)).toMatchObject({ type: 'status', session: { status: null } }) + readOnly.mockRestore() + feed.publish(SESSION) + expect(events.at(-1)).toMatchObject({ type: 'status', session: { status: 'idle' } }) + }) + + it('projects live background tasks and republishes a task-only state change', async () => { + const journal = await openJournal() + let tasks = [ + { id: 'task-1', kind: 'agent' as const, name: 'deep_review', state: 'working' as const } + ] + const { feed, events } = feedFor(new Map([[SESSION, { journal }]]), null, undefined, () => ({ + state: 'monitoring', + tasks + })) + await journal.appendItem( + USER_IDENTITY, + { kind: 'message', role: 'user', blocks: [{ type: 'text', text: 'fan out' }] }, + { fence: 1 } + ) + feed.publish(SESSION, journal) + expect(events.at(-1)).toEqual({ + type: 'status', + session: expect.objectContaining({ + backgroundTasks: [{ id: 'task-1', kind: 'agent', name: 'deep_review', state: 'working' }] + }) + }) + + // No journal change: only the task state moved. + tasks = [{ id: 'task-1', kind: 'agent', name: 'deep_review', state: 'waiting' as never }] + const before = events.length + feed.publish(SESSION, journal) + expect(events).toHaveLength(before + 1) + expect(events.at(-1)).toEqual({ + type: 'status', + session: expect.objectContaining({ + backgroundTasks: [expect.objectContaining({ state: 'waiting' })] + }) + }) + + // An identical projection is suppressed. + feed.publish(SESSION, journal) + expect(events).toHaveLength(before + 1) + }) + + it('omits task usage so a progress tick never re-broadcasts the summary', async () => { + const journal = await openJournal() + let tasks: AgentSessionBackgroundTask[] = [ + { id: 'task-1', kind: 'agent', name: 'deep_review', state: 'working', totalTokens: 10 } + ] + const { feed, events } = feedFor(new Map([[SESSION, { journal }]]), null, undefined, () => ({ + state: 'monitoring', + tasks + })) + await journal.appendItem( + USER_IDENTITY, + { kind: 'message', role: 'user', blocks: [{ type: 'text', text: 'fan out' }] }, + { fence: 1 } + ) + feed.publish(SESSION, journal) + const before = events.length + + // A `task_progress` frame moves only usage, which no status-summary reader renders; + // re-broadcasting the whole summary per frame would cost every remote subscriber. + tasks = [ + { id: 'task-1', kind: 'agent', name: 'deep_review', state: 'working', totalTokens: 4_200 } + ] + feed.publish(SESSION, journal) + expect(events).toHaveLength(before) + expect(events.at(-1)).toEqual({ + type: 'status', + session: expect.objectContaining({ + backgroundTasks: [{ id: 'task-1', kind: 'agent', name: 'deep_review', state: 'working' }] + }) + }) + + // A state change on the same task still reaches subscribers. + tasks = [ + { id: 'task-1', kind: 'agent', name: 'deep_review', state: 'waiting', totalTokens: 4_200 } + ] + feed.publish(SESSION, journal) + expect(events).toHaveLength(before + 1) + expect(events.at(-1)).toEqual({ + type: 'status', + session: expect.objectContaining({ + backgroundTasks: [expect.objectContaining({ state: 'waiting' })] + }) + }) + }) }) /** diff --git a/src/main/native-chat/agent-session-wire/structured-agent-session-status-feed.ts b/src/main/native-chat/agent-session-wire/structured-agent-session-status-feed.ts index 3c4ce4b97a6..bddb9ae93ce 100644 --- a/src/main/native-chat/agent-session-wire/structured-agent-session-status-feed.ts +++ b/src/main/native-chat/agent-session-wire/structured-agent-session-status-feed.ts @@ -14,9 +14,11 @@ import { agentProviderSessionsEqual } from '../../../shared/agent-session-resume import type { AgentSessionRecord } from '../../../shared/agent-session-record' import { normalizeOptionalField } from '../../../shared/agent-status-field-normalization' import { AGENT_MODEL_MAX_LENGTH } from '../../../shared/agent-status-types' -import type { - AgentSessionStatusEvent, - AgentSessionStatusSummary +import { + agentSessionBackgroundTasksEqual, + type AgentSessionBackgroundTaskState, + type AgentSessionStatusEvent, + type AgentSessionStatusSummary } from '../../../shared/agent-session-wire' import { projectStructuredAgentSessionStatusSummary } from '../../../shared/structured-agent-session-projection' import type { AgentSessionJournal } from '../agent-session-journal/journal-store' @@ -41,6 +43,9 @@ export type StructuredAgentSessionStatusFeedDeps = { /** Every projection change, whether or not anyone is subscribed. `replay` marks a re-projection * of state the host already knew (restore, an arriving subscriber) rather than a journal edge. */ onStatusChanged?: (summary: AgentSessionStatusSummary, options: { replay: boolean }) => void + /** Live provider-owned background tasks for the summary, so session lists can + * render subagent children. Optional: a provider without the hook projects none. */ + readBackgroundTasks?: (sessionId: string) => AgentSessionBackgroundTaskState | null | undefined } function summariesEqual(a: AgentSessionStatusSummary, b: AgentSessionStatusSummary): boolean { @@ -57,13 +62,50 @@ function summariesEqual(a: AgentSessionStatusSummary, b: AgentSessionStatusSumma a.toolName === b.toolName && a.toolInput === b.toolInput && a.lastAssistantMessage === b.lastAssistantMessage && + agentSessionBackgroundTasksEqual(a.backgroundTasks, b.backgroundTasks) && agentProviderSessionsEqual(undefined, a.providerSession, b.providerSession) ) } +/** Wire the host's own deps into a feed; keeps the host at one call site. + * `deps` is a thunk because the host builds the feed in a field initializer, + * before its constructor parameters are assigned. */ +export function createStructuredAgentSessionHostStatusFeed(args: { + sessions: StructuredAgentSessionStatusFeedDeps['sessions'] + now: () => number + deps: () => { + store: { getRecord: (sessionId: string) => AgentSessionRecord | null } + adapter: { + backgroundTaskState?: ( + sessionId: string + ) => AgentSessionBackgroundTaskState | null | undefined + } + onSessionStatusChanged?: StructuredAgentSessionStatusFeedDeps['onStatusChanged'] + } +}): StructuredAgentSessionStatusFeed { + return new StructuredAgentSessionStatusFeed({ + sessions: args.sessions, + getRecord: (sessionId) => args.deps().store.getRecord(sessionId), + now: args.now, + onStatusChanged: (summary, options) => args.deps().onSessionStatusChanged?.(summary, options), + readBackgroundTasks: (sessionId) => args.deps().adapter.backgroundTaskState?.(sessionId) + }) +} + export class StructuredAgentSessionStatusFeed { private readonly subscribers = new Map() private readonly published = new Map() + // Task progress must not sort and scan an unchanged conversation. Journal identity owns cleanup. + private readonly journalProjections = new WeakMap< + AgentSessionJournal, + { + epoch: string + sequence: number + readOnly: boolean + fence: number | undefined + summary: ReturnType + } + >() constructor(private readonly deps: StructuredAgentSessionStatusFeedDeps) {} @@ -154,24 +196,54 @@ export class StructuredAgentSessionStatusFeed { journal: AgentSessionJournal ): AgentSessionStatusSummary { // An unreadable journal projects as "no turn": the chat itself shows the reset. - const snapshot = journal.isReadOnly ? null : journal.snapshot() - const items = snapshot?.items ?? [] - const submissions = snapshot?.submissions ?? [] + const cursor = journal.cursor() + const readOnly = journal.isReadOnly + const fence = session.fence + let projection = this.journalProjections.get(journal) + if ( + !projection || + projection.epoch !== cursor.epoch || + projection.sequence !== cursor.sequence || + projection.readOnly !== readOnly || + projection.fence !== fence + ) { + // A journalled submission bumps `lastSequence`, so the send-time working + // signal reaches the cache; the lease fence does not, hence the extra key. + const snapshot = readOnly ? null : journal.snapshot() + projection = { + ...cursor, + readOnly, + fence, + summary: projectStructuredAgentSessionStatusSummary( + snapshot?.items ?? [], + snapshot?.submissions ?? [], + fence + ) + } + this.journalProjections.set(journal, projection) + } const record = this.deps.getRecord(sessionId) const providerSession = structuredAgentSessionProviderSessionMetadata(record) // The journal has no model: the record's acknowledged options are where an owner // handoff or a mid-session switch lands, so the row follows whichever is in force. const model = normalizeOptionalField(record?.options?.model, AGENT_MODEL_MAX_LENGTH) + // Usage is dropped here on purpose: a `task_progress` tick would otherwise fail the + // equality check and re-broadcast a full summary to every remote subscriber for a + // number no session list renders. Tokens stay live on the background-task channel. + const backgroundTasks = this.deps + .readBackgroundTasks?.(sessionId) + ?.tasks?.map(({ totalTokens: _totalTokens, ...task }) => task) return { sessionId, workspaceId: session.params.location.workspaceId, agent: session.params.provider, ...(session.hasProviderChild ? { hostExecutionOwned: true as const } : {}), - ...projectStructuredAgentSessionStatusSummary(items, submissions, session.fence), + ...projection.summary, ...(record?.rewind?.phase === 'prepared' || record?.rewind?.phase === 'provider-succeeded' ? { rewindBlockedReason: 'outcome-unknown' as const } : {}), ...(model ? { model } : {}), + ...(backgroundTasks && backgroundTasks.length > 0 ? { backgroundTasks } : {}), ...(providerSession ? { providerSession } : {}), updatedAt: journal.lastActivityAt() || this.deps.now() } diff --git a/src/main/observability/local-file-sink-memory.test.ts b/src/main/observability/local-file-sink-memory.test.ts index fef5b8d00dd..2c6644694b9 100644 --- a/src/main/observability/local-file-sink-memory.test.ts +++ b/src/main/observability/local-file-sink-memory.test.ts @@ -2,11 +2,7 @@ import { mkdtempSync, readFileSync, rmSync, statSync } from 'node:fs' import { tmpdir } from 'node:os' import { join } from 'node:path' import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' -import { - createLocalFileSink, - DROPPED_RECORD_TYPE, - type LocalFileSink -} from './local-file-sink' +import { createLocalFileSink, DROPPED_RECORD_TYPE, type LocalFileSink } from './local-file-sink' function parseLine(raw: string): Record { return JSON.parse(raw) as Record diff --git a/src/main/runtime/orca-runtime-tests/worktree-ps-structured-host.spec.ts b/src/main/runtime/orca-runtime-tests/worktree-ps-structured-host.spec.ts index 58e7de1b674..5779c65edda 100644 --- a/src/main/runtime/orca-runtime-tests/worktree-ps-structured-host.spec.ts +++ b/src/main/runtime/orca-runtime-tests/worktree-ps-structured-host.spec.ts @@ -53,6 +53,8 @@ function journalWith(prompt: string): AgentSessionJournal { return { isReadOnly: false, lastActivityAt: () => OBSERVED_AT, + // This journal never changes, so a real one would hold its cursor steady. + cursor: () => ({ epoch: 1, sequence: 1 }), snapshot: () => ({ items: runningTurn(prompt) }) } as unknown as AgentSessionJournal } diff --git a/src/main/runtime/rpc/methods/structured-agent-session-rpc.test-fixture.ts b/src/main/runtime/rpc/methods/structured-agent-session-rpc.test-fixture.ts index a702bda5afc..f9f60ded887 100644 --- a/src/main/runtime/rpc/methods/structured-agent-session-rpc.test-fixture.ts +++ b/src/main/runtime/rpc/methods/structured-agent-session-rpc.test-fixture.ts @@ -97,6 +97,7 @@ function statusFeed(): StructuredAgentSessionStatusFeed { { journal: { isReadOnly: false, + cursor: () => ({ epoch: 'epoch-status', sequence: 2 }), lastActivityAt: () => 2, snapshot: () => ({ items: STATUS_ITEMS }) } as unknown as AgentSessionJournal, diff --git a/src/renderer/src/components/native-chat/NativeChatBackgroundTasksStatus.test.tsx b/src/renderer/src/components/native-chat/NativeChatBackgroundTasksStatus.test.tsx index e137220dd66..96be91820a3 100644 --- a/src/renderer/src/components/native-chat/NativeChatBackgroundTasksStatus.test.tsx +++ b/src/renderer/src/components/native-chat/NativeChatBackgroundTasksStatus.test.tsx @@ -2,12 +2,16 @@ import '@testing-library/jest-dom/vitest' -import { cleanup, fireEvent, render, screen } from '@testing-library/react' +import { act, cleanup, fireEvent, render, screen, within } from '@testing-library/react' +import { Profiler } from 'react' import { afterEach, describe, expect, it, vi } from 'vitest' import type { AgentSessionBackgroundTask } from '../../../../shared/agent-session-wire' import { NativeChatBackgroundTasksStatus } from './NativeChatBackgroundTasksStatus' -afterEach(cleanup) +afterEach(() => { + cleanup() + vi.useRealTimers() +}) const TASKS: AgentSessionBackgroundTask[] = [ { id: 'codex-agent:child-1', kind: 'agent', description: 'count_a' }, @@ -20,7 +24,10 @@ function renderStrip(props: { supportsTaskStop: boolean; supportsStopAll: boolea const onStop = vi.fn() render( { expect(screen.getByText('sleep 90')).toBeInTheDocument() }) }) + +describe('background-tasks strip header', () => { + function renderHeader(tasks: AgentSessionBackgroundTask[]): HTMLElement { + render( + {}} + /> + ) + return screen.getByRole('button', { expanded: false }) + } + + it('leads each kind segment with that kind icon and keeps the counts in the accessible name', () => { + const header = renderHeader([ + { id: 'a1', kind: 'agent' }, + { id: 'a2', kind: 'agent' }, + { id: 'a3', kind: 'agent' }, + { id: 'm1', kind: 'monitor' } + ]) + expect(header).toHaveAttribute('aria-label', '3 agents · 1 monitor') + expect(header.querySelector('.lucide-bot')).toBeInTheDocument() + // Heartbeat, the same glyph the agent sidebar shows for monitoring. + expect(header.querySelector('.lucide-activity')).toBeInTheDocument() + // Two kind icons and the chevron: the aggregate state dot is gone. + expect(header.querySelectorAll('svg')).toHaveLength(3) + for (const icon of header.querySelectorAll('svg')) { + expect(icon).toHaveAttribute('aria-hidden', 'true') + } + }) + + it('gives the monitor heartbeat the sidebar amber and leaves other kinds neutral', () => { + const header = renderHeader([ + { id: 'a1', kind: 'agent' }, + { id: 'm1', kind: 'monitor' } + ]) + // Same glyph AND same colour as AgentStateDot/StatusIndicator, or a monitor + // here does not read as the monitor there. + expect(header.querySelector('.lucide-activity')?.classList).toContain('text-yellow-500') + expect(header.querySelector('.lucide-bot')?.classList).toContain('text-muted-foreground') + expect(header.querySelector('.lucide-bot')?.classList).not.toContain('text-yellow-500') + }) + + it('dims the monitor amber while a turn owns the voice', () => { + render( + {}} + /> + ) + const header = screen.getByRole('button', { expanded: false }) + expect(header.querySelector('.lucide-activity')?.classList).toContain('text-yellow-500/40') + }) + + it('carries the monitor amber on the expanded row too', () => { + const header = renderHeader([ + { id: 'm1', kind: 'monitor', description: 'watcher' }, + { id: 'c1', kind: 'command', description: 'sleep 90' } + ]) + fireEvent.click(header) + // Each kind group is its own labelled list, so scope to the monitor one. + const monitors = screen.getByRole('list', { name: 'Monitors' }) + expect(monitors.querySelector('.lucide-activity')?.classList).toContain('text-yellow-500') + const shell = screen.getByRole('list', { name: 'Shell' }) + expect(shell.querySelector('.lucide-square-terminal')?.classList).toContain( + 'text-muted-foreground' + ) + }) + + it('draws the segment separator in a visible text tone, not the divider token', () => { + const header = renderHeader([ + { id: 'a1', kind: 'agent' }, + { id: 'c1', kind: 'command' } + ]) + const separators = [...header.querySelectorAll('span')].filter( + (element) => element.textContent === ' · ' + ) + expect(separators).toHaveLength(1) + // `--border` is a divider line (7% white in dark), an order of magnitude + // fainter than the counts it sits between. + expect(separators[0].classList).not.toContain('text-border') + expect(separators[0].classList).toContain('text-muted-foreground') + // One space either side; the icon's own margin is the icon-to-label gap. + expect(header.textContent).toBe('1 agent · 1 shell') + }) + + it('carries no icon on a collapsed total, which spans kinds', () => { + const header = renderHeader([ + { id: 'a1', kind: 'agent' }, + { id: 'c1', kind: 'command' }, + { id: 'm1', kind: 'monitor' }, + { id: 'w1', kind: 'workflow' } + ]) + expect(header).toHaveAttribute('aria-label', '4 background tasks') + expect(header.querySelectorAll('svg')).toHaveLength(1) + }) +}) + +describe('settled rows beside their live siblings', () => { + // Retention is the PR's headline: a finished child stays visible, keeps the + // usage it ended on, and stops claiming a clock or a stop control. + it('keeps a settled row with its final usage, no clock and no stop', () => { + render( + {}} + /> + ) + fireEvent.click(screen.getByRole('button', { expanded: false })) + const agents = screen.getByRole('list', { name: 'Agents' }) + const rows = within(agents).getAllByRole('listitem') + expect(rows).toHaveLength(2) + // First seen first: the settled sibling started earlier. + expect(rows[0].textContent).toBe('settled child18.1k') + expect(rows[1].textContent).toMatch(/^live child4\.1k · .+Stop$/) + expect(within(rows[1]).getByRole('button', { name: 'Stop live child' })).toBeInTheDocument() + expect(within(rows[0]).queryByRole('button')).toBeNull() + }) +}) + +describe('background-task row reasons', () => { + function expandedRows(tasks: AgentSessionBackgroundTask[]): HTMLElement[] { + render( + {}} + /> + ) + fireEvent.click(screen.getByRole('button', { expanded: false })) + return screen.getAllByRole('listitem') + } + + // `unverifiable` is the SSH verdict for "no contact"; a row that hides it reads + // like a working child. `blocked` is the same class of loss. + it('names the reason on every attention state, not only on waiting', () => { + const rows = expandedRows([ + { id: 'a1', kind: 'agent', description: 'ssh child', state: 'unverifiable' }, + { id: 'a2', kind: 'agent', description: 'flaky child', state: 'blocked' }, + { id: 'a3', kind: 'agent', description: 'approval child', state: 'waiting' }, + { id: 'a4', kind: 'agent', description: 'busy child', state: 'working' } + ]) + expect(rows).toHaveLength(4) + expect(rows[0].textContent).toContain('ssh child · no contact') + expect(rows[1].textContent).toContain('flaky child · failed') + expect(rows[2].textContent).toContain('approval child · needs approval') + // A running row has nothing to explain. + expect(rows[3].textContent).not.toContain('·') + }) +}) + +it('stops elapsed renders in a hidden pane and catches up on reveal', () => { + vi.useFakeTimers() + vi.setSystemTime(100_000) + const committed = vi.fn() + const view = (isVisible: boolean) => ( + + {}} + /> + + ) + const { rerender, unmount } = render(view(true)) + committed.mockClear() + act(() => vi.advanceTimersByTime(1_000)) + expect(committed).toHaveBeenCalled() + rerender(view(false)) + committed.mockClear() + act(() => vi.advanceTimersByTime(10_000)) + expect(committed).not.toHaveBeenCalled() + rerender(view(true)) + committed.mockClear() + act(() => vi.advanceTimersByTime(1_000)) + expect(committed).toHaveBeenCalled() + unmount() + expect(vi.getTimerCount()).toBe(0) +}) diff --git a/src/renderer/src/components/native-chat/NativeChatBackgroundTasksStatus.tsx b/src/renderer/src/components/native-chat/NativeChatBackgroundTasksStatus.tsx index c52b73842a9..21b8d73f258 100644 --- a/src/renderer/src/components/native-chat/NativeChatBackgroundTasksStatus.tsx +++ b/src/renderer/src/components/native-chat/NativeChatBackgroundTasksStatus.tsx @@ -1,62 +1,207 @@ -import { useId, useState } from 'react' -import { ChevronDown } from 'lucide-react' +import { useEffect, useId, useMemo, useRef, useState } from 'react' +import { Activity, Bot, ChevronDown, CircleHelp, SquareTerminal, Workflow } from 'lucide-react' import type { AgentSessionBackgroundTask } from '../../../../shared/agent-session-wire' import { AgentStateDot } from '@/components/AgentStateDot' import { Button } from '@/components/ui/button' +import { useNow } from '@/hooks/use-now' import { translate } from '@/i18n/i18n' +import { backgroundTasksHeaderContent } from './background-task-header-content' +import { + backgroundTaskElapsedLabel, + backgroundTaskGroupLabel, + backgroundTaskStateReason, + buildBackgroundTaskGroups, + formatBackgroundTaskTokens, + type BackgroundRosterTask +} from './background-task-roster' -function backgroundTaskLabel(task: AgentSessionBackgroundTask): string { - if (task.description) { - return task.description - } - switch (task.kind) { - case 'agent': - return translate('components.native-chat.backgroundTasks.agent', 'Background agent') - case 'workflow': - return translate('components.native-chat.backgroundTasks.workflow', 'Background workflow') - case 'command': - return translate('components.native-chat.backgroundTasks.command', 'Background command') - case 'monitor': - return translate('components.native-chat.backgroundTasks.monitor', 'Background monitor') - case 'unknown': - return translate('components.native-chat.backgroundTasks.task', 'Background task') - } +/** Below this strip width (border-box, live root font size) the header drops + * its per-kind breakdown for an honest total. A narrow split pane on a wide + * monitor must behave like a narrow window, so no viewport media query. */ +const NARROW_STRIP_REM = 24 + +function rootFontSizePx(): number { + const parsed = Number.parseFloat(getComputedStyle(document.documentElement).fontSize) + return Number.isFinite(parsed) && parsed > 0 ? parsed : 16 +} + +/** Observe the strip's own border-box width; the viewport is only the + * pre-measurement stand-in before the first observer callback. */ +function useNarrowStrip(ref: React.RefObject): boolean { + const [narrow, setNarrow] = useState(() => window.innerWidth < NARROW_STRIP_REM * 16) + useEffect(() => { + const element = ref.current + if (!element || typeof ResizeObserver === 'undefined') { + return + } + const observer = new ResizeObserver((observerEntries) => { + const width = + observerEntries[0]?.borderBoxSize?.[0]?.inlineSize ?? element.getBoundingClientRect().width + setNarrow(width < NARROW_STRIP_REM * rootFontSizePx()) + }) + observer.observe(element, { box: 'border-box' }) + return () => observer.disconnect() + }, [ref]) + return narrow +} + +const KIND_ICONS = { + agent: Bot, + command: SquareTerminal, + monitor: Activity, + workflow: Workflow, + unknown: CircleHelp +} as const + +/** Monitoring is a STATE the app colours the same on every surface — the agent + * sidebar and `AgentStateDot` both draw an amber heartbeat — so the strip must + * match it or the two stop reading as the same thing. The other four are plain + * kind markers and stay neutral. `dimmed` is the running-turn treatment. */ +function kindIconTone(kind: AgentSessionBackgroundTask['kind'], dimmed: boolean): string { + const tone = kind === 'monitor' ? 'text-yellow-500' : 'text-muted-foreground' + return dimmed ? `${tone}/40` : tone +} + +function BackgroundTaskRow(props: { + entry: BackgroundRosterTask + now: number + supportsTaskStop: boolean + stopping: boolean + onStop: (taskId: string) => void +}): React.JSX.Element { + const { entry, now } = props + const Icon = KIND_ICONS[entry.task.kind] + // Every attention state states its reason on the row, the same ones the collapsed + // header names; `unverifiable` ("no contact") must never be silently dropped. + const reason = backgroundTaskStateReason(entry.state) + // Settled rows keep their final usage but no elapsed — a still-growing clock + // on finished work would lie. + const meta = [ + entry.task.totalTokens !== undefined + ? formatBackgroundTaskTokens(entry.task.totalTokens) + : null, + entry.settled ? null : backgroundTaskElapsedLabel(entry.task, now) + ] + .filter((part): part is string => part !== null) + .join(' · ') + return ( +
  • +
    • + ) } export function NativeChatBackgroundTasksStatus(props: { tasks: readonly AgentSessionBackgroundTask[] + settledTasks: readonly AgentSessionBackgroundTask[] supportsTaskStop: boolean /** False when the provider exposes no honest stop at all; the fallback * control is hidden rather than offering a button that cannot act. */ supportsStopAll: boolean stoppingTaskIds: ReadonlySet stoppingAll: boolean + /** True while the session is idle: only then may the strip speak as the + * animated monitoring indicator. A running turn owns the voice. */ + indicatorActive: boolean + isVisible: boolean onStop: (taskId?: string) => void }): React.JSX.Element { const [expanded, setExpanded] = useState(false) const taskListId = useId() + const stripRef = useRef(null) + const narrow = useNarrowStrip(stripRef) + // The 1 Hz elapsed tick must not re-group, re-sort and re-translate the whole roster. + const groups = useMemo( + () => buildBackgroundTaskGroups(props.tasks, props.settledTasks), + [props.tasks, props.settledTasks] + ) + const singleLiveCommand = + groups.length === 1 && groups[0].kind === 'command' && groups[0].tasks.length === 1 + const hasElapsed = groups.some((group) => + group.tasks.some((entry) => !entry.settled && (entry.task.startedAt ?? 0) > 0) + ) + const now = useNow(1_000, props.isVisible && hasElapsed && (expanded || singleLiveCommand)) + const header = backgroundTasksHeaderContent(groups, { narrow, now }) + const headerText = `${header.segments.map((segment) => segment.text).join(' · ')}${header.detail ? `${header.segments.length > 0 ? ' — ' : ''}${header.detail}` : ''}` return (
    -
    +
    - ) : null} - - ) - })} - + ))} + +
    + )) ) : (

    {translate( @@ -119,7 +250,7 @@ export function NativeChatBackgroundTasksStatus(props: {

    )} {!props.supportsTaskStop && props.supportsStopAll ? ( -
    0 ? 'mt-2 border-t border-border pt-2' : 'mt-2'}> +
    0 ? 'mt-2 border-t border-border pt-2' : 'mt-2'}>