diff --git a/config/scripts/locale-collator-sort-benchmark.mjs b/config/scripts/locale-collator-sort-benchmark.mjs index 3d34466532a..1a0b08e1643 100644 --- a/config/scripts/locale-collator-sort-benchmark.mjs +++ b/config/scripts/locale-collator-sort-benchmark.mjs @@ -129,6 +129,7 @@ for (const count of [36, 50, 250]) { const issues = makeJiraIssues(count) const before = () => [...issues] + // oxlint-disable-next-line sort-comparator-performance/no-repeated-collator -- Baseline measures per-comparison setup against a reused collator. .sort((a, b) => a.key.localeCompare(b.key, undefined, { numeric: true })) .map((issue) => issue.key) const after = () => sortJiraIssues(issues, 'key', 'asc').map((issue) => issue.key) @@ -142,6 +143,7 @@ for (const count of [36, 50, 250]) { for (const count of [10, 50, 250]) { const values = makeBaseSensitivityValues(count) const before = () => + // oxlint-disable-next-line sort-comparator-performance/no-repeated-collator -- Baseline measures per-comparison setup against a reused collator. [...values].sort((a, b) => a.localeCompare(b, undefined, { sensitivity: 'base' })) const after = () => [...values].sort(compareBaseSensitivityLocaleText) assertSameOrder(before, after, `base ${count}`) diff --git a/src/cli/runtime/websocket-transport.test.ts b/src/cli/runtime/websocket-transport.test.ts index 162b431f1bf..4c178b71fd8 100644 --- a/src/cli/runtime/websocket-transport.test.ts +++ b/src/cli/runtime/websocket-transport.test.ts @@ -18,6 +18,7 @@ import { launchOrcaApp } from './launch' import { addEnvironmentFromPairingCode } from './environments' import { RuntimeClientError } from './types' import { + AGENT_SESSION_BACKGROUND_TASK_STOP_CAPABILITY, AGENT_SESSION_BOUNDARY_RUNTIME_CAPABILITY, AUTOMATION_OWNER_FENCING_RUNTIME_CAPABILITY, MIN_COMPATIBLE_RUNTIME_CLIENT_VERSION, @@ -70,6 +71,7 @@ describe('CLI remote WebSocket transport', () => { expect(runtime.authFrames).toContainEqual( expect.objectContaining({ clientCapabilities: [ + AGENT_SESSION_BACKGROUND_TASK_STOP_CAPABILITY, SESSION_TAB_CLOSE_INTENT_RUNTIME_CAPABILITY, SESSION_TABS_AUTHORITATIVE_INVENTORY_RUNTIME_CAPABILITY, AGENT_SESSION_BOUNDARY_RUNTIME_CAPABILITY, diff --git a/src/main/codex/codex-background-command-tracker.test.ts b/src/main/codex/codex-background-command-tracker.test.ts new file mode 100644 index 00000000000..abccb4e4a35 --- /dev/null +++ b/src/main/codex/codex-background-command-tracker.test.ts @@ -0,0 +1,128 @@ +import { describe, expect, it } from 'vitest' +import type { AgentJournalItemBody } from '../../shared/agent-session-journal-types' +import { agentJournalItemKey } from '../../shared/agent-session-journal-item-key' +import { CodexBackgroundCommandTracker } from './codex-background-command-tracker' +import { createCodexJournalTranslator } from './codex-structured-journal-translation' +import type { CodexStructuredSessionEvent } from './codex-structured-session-state' + +function notification( + method: string, + params: Record +): Extract { + return { + type: 'notification', + sessionId: 'session', + threadId: 'root', + method, + params: { threadId: 'root', turnId: 'turn', ...params } + } +} + +function command(method: string, id = 'exec', threadId = 'root') { + return { + ...notification(method, { + item: { + type: 'commandExecution', + id, + command: 'sleep 30', + source: 'unifiedExecStartup', + status: method === 'item/completed' ? 'completed' : 'inProgress', + exitCode: method === 'item/completed' ? 0 : null + } + }), + threadId + } +} + +describe('persistent command ownership', () => { + it('preflights finite metadata capacity and admits work again after process completion', () => { + const tracker = new CodexBackgroundCommandTracker('root', 700) + const first = command('item/started', 'first') + const second = command('item/started', 'second') + expect(tracker.canObserve(first)).toBe(true) + tracker.observe(first) + expect(tracker.canObserve(second)).toBe(false) + expect(() => tracker.observe(second)).toThrow('not admitted') + expect(tracker.tasks()).toHaveLength(1) + expect(tracker.retainedMetadataBytes).toBeLessThanOrEqual(700) + tracker.observe(command('item/completed', 'first')) + expect(tracker.canObserve(second)).toBe(true) + tracker.observe(second) + expect(tracker.tasks()).toHaveLength(1) + expect(tracker.retainedMetadataBytes).toBeLessThanOrEqual(700) + tracker.clear() + expect(tracker.retainedMetadataBytes).toBe(0) + }) + + it('keeps the journal running across turn completion and accepts late output and exit', () => { + const rows: { key: string; body: AgentJournalItemBody }[] = [] + const translator = createCodexJournalTranslator({ + primaryThreadId: () => 'root', + sink: { + appendItem: (identity, body) => rows.push({ key: agentJournalItemKey(identity), body }), + appendTombstone: () => {}, + publish: () => {} + } + }) + const tracker = new CodexBackgroundCommandTracker('root') + const deliver = (event: Extract) => { + expect(translator.handle(event)).toEqual({ accepted: true }) + tracker.observe(event) + } + deliver(notification('turn/started', { turn: { id: 'turn' } })) + deliver(command('item/started')) + const originalKey = rows.find(({ body }) => body.kind === 'tool-call')?.key + deliver(notification('turn/completed', { turn: { id: 'turn' } })) + expect(rows.filter(({ body }) => body.kind === 'tool-call').map(({ body }) => body)).toEqual([ + expect.objectContaining({ state: 'running' }) + ]) + expect(tracker.tasks()).toHaveLength(1) + deliver( + notification('item/commandExecution/outputDelta', { itemId: 'exec', delta: 'late output' }) + ) + translator.flush() + expect(rows.at(-1)).toMatchObject({ key: originalKey, body: { state: 'running' } }) + deliver(command('item/completed')) + expect(rows.at(-1)).toMatchObject({ key: originalKey, body: { state: 'completed' } }) + expect(tracker.tasks()).toEqual([]) + translator.dispose() + }) + + it('counts child shells only after the child stops covering them, without resurrecting exits', () => { + const tracker = new CodexBackgroundCommandTracker('root') + tracker.observe(command('item/started', 'child-exec', 'child')) + tracker.observe( + notification('item/started', { + item: { + type: 'commandExecution', + id: 'poll', + source: 'unifiedExecInteraction', + status: 'inProgress' + } + }) + ) + expect(tracker.tasks(new Set(['child']))).toEqual([]) + expect(tracker.tasks()).toEqual([ + { id: 'codex-command:thread:child:child-exec', kind: 'command', description: 'sleep 30' } + ]) + tracker.observe(command('item/completed', 'child-exec', 'child')) + tracker.observe(command('item/started')) + tracker.observe(command('item/completed')) + tracker.observe(command('item/started')) + expect(tracker.tasks()).toEqual([]) + }) + + it('retains live commands while recycling bounded settled history', () => { + const tracker = new CodexBackgroundCommandTracker('root') + tracker.observe(command('item/started', 'long-lived')) + for (let index = 0; index < 300; index += 1) { + tracker.observe(command('item/started', `short-${index}`)) + tracker.observe(command('item/completed', `short-${index}`)) + } + expect(tracker.tasks()).toEqual([ + { id: 'codex-command:primary:long-lived', kind: 'command', description: 'sleep 30' } + ]) + tracker.clear() + expect(tracker.tasks()).toEqual([]) + }) +}) diff --git a/src/main/codex/codex-background-command-tracker.ts b/src/main/codex/codex-background-command-tracker.ts new file mode 100644 index 00000000000..becbb18a67c --- /dev/null +++ b/src/main/codex/codex-background-command-tracker.ts @@ -0,0 +1,150 @@ +import type { AgentSessionBackgroundTask } from '../../shared/agent-session-wire' +import type { CodexBackgroundTaskEvent } from './codex-background-task-frames' +import { codexCommandOutlivesTurn } from './codex-command-lifecycle' +import { readRecord, readString } from './codex-item-field-readers' +import { readCodexThreadItem } from './codex-structured-item-translation' +import { MAX_CODEX_ITEM_STREAM_METADATA_BYTES } from './codex-item-stream-retention' + +const MAX_SETTLED_COMMANDS = 128 +const MAX_DESCRIPTION_CHARS = 512 + +type Command = { threadId: string; task: AgentSessionBackgroundTask; bytes: number } + +/** Stays within the retained bound, so read-time qualification cannot outgrow admission. */ +function qualifiedDescription(label: string, description: string | undefined): string { + return (description ? `${label} — ${description}` : label).slice(0, MAX_DESCRIPTION_CHARS) +} + +export class CodexBackgroundCommandTracker { + private readonly commands = new Map() + private readonly settled = new Map() + private liveBytes = 0 + private settledBytes = 0 + + constructor( + private readonly primaryThreadId: string, + private readonly maxMetadataBytes = MAX_CODEX_ITEM_STREAM_METADATA_BYTES + ) {} + + get retainedMetadataBytes(): number { + return this.liveBytes + this.settledBytes + } + + canObserve(event: CodexBackgroundTaskEvent): boolean { + const parsed = this.parse(event) + return ( + !parsed || + parsed.completed || + this.commands.has(parsed.key) || + this.settled.has(parsed.key) || + this.liveBytes + parsed.command.bytes <= this.maxMetadataBytes + ) + } + + observe(event: CodexBackgroundTaskEvent): void { + const parsed = this.parse(event) + if (!parsed || this.settled.has(parsed.key)) { + return + } + const { key, command, completed } = parsed + const existing = this.commands.get(key) + if (completed) { + if (existing) { + this.liveBytes -= existing.bytes + this.commands.delete(key) + } + const bytes = Buffer.byteLength(key, 'utf8') + 256 + if (this.liveBytes + bytes <= this.maxMetadataBytes) { + this.settled.set(key, bytes) + this.settledBytes += bytes + } + this.trimSettled() + return + } + if (existing) { + return + } + if (this.liveBytes + command.bytes > this.maxMetadataBytes) { + throw new Error('Codex command metadata was not admitted before observation') + } + this.commands.set(key, command) + this.liveBytes += command.bytes + this.trimSettled() + } + + tasks( + coveredThreads?: ReadonlySet, + childLabel?: (threadId: string) => string | null + ): AgentSessionBackgroundTask[] { + return [...this.commands.values()] + .filter((command) => !coveredThreads?.has(command.threadId)) + .map(({ threadId, task }) => { + // The agent row carrying the child's name is gone by the time this row shows; + // unqualified it reads as a bare shell string with no owner. Resolved on read so + // a label registered after the command still lands. + const label = threadId === this.primaryThreadId ? null : childLabel?.(threadId) + return label + ? { ...task, description: qualifiedDescription(label, task.description) } + : task + }) + } + + clear(): void { + this.commands.clear() + this.settled.clear() + this.liveBytes = 0 + this.settledBytes = 0 + } + + private trimSettled(): void { + while ( + this.settled.size > MAX_SETTLED_COMMANDS || + this.retainedMetadataBytes > this.maxMetadataBytes + ) { + const oldest = this.settled.entries().next().value + if (!oldest) { + break + } + this.settled.delete(oldest[0]) + this.settledBytes -= oldest[1] + } + } + + private parse( + event: CodexBackgroundTaskEvent + ): { key: string; command: Command; completed: boolean } | null { + if (event.method !== 'item/started' && event.method !== 'item/completed') { + return null + } + const item = readCodexThreadItem(readRecord(event.params).item) + if (!item || !codexCommandOutlivesTurn(item)) { + return null + } + const key = JSON.stringify([event.threadId, item.id]) + const completed = event.method === 'item/completed' || item.status !== 'inProgress' + const description = readString(item, 'command') + ?.slice(0, MAX_DESCRIPTION_CHARS) + .replace(/\s+/g, ' ') + .trim() + const value = { + threadId: event.threadId, + task: { + id: + event.threadId === this.primaryThreadId + ? `codex-command:primary:${encodeURIComponent(item.id)}` + : `codex-command:thread:${encodeURIComponent(event.threadId)}:${encodeURIComponent(item.id)}`, + kind: 'command' as const, + ...(description ? { description } : {}) + } + } + return { + key, + completed, + command: { + ...value, + bytes: + Buffer.byteLength(key, 'utf8') + Buffer.byteLength(JSON.stringify(value), 'utf8') + 256 + } + } + } +} diff --git a/src/main/codex/codex-background-task-frames.ts b/src/main/codex/codex-background-task-frames.ts new file mode 100644 index 00000000000..a2e7a7e2151 --- /dev/null +++ b/src/main/codex/codex-background-task-frames.ts @@ -0,0 +1,72 @@ +import type { NativeChatSubagentState } from '../../shared/native-chat-types' +import { + codexSubagentLabel, + isCodexRootAgentActivity, + readCodexSubagentActivity +} from './codex-subagent-activity' +import { codexChildTurnState } from './codex-subagent-executions' +import { readRecord } from './codex-item-field-readers' +import { readCodexThreadItem } from './codex-structured-item-translation' +import { readCodexTurnId } from './codex-structured-thread-facts' + +export type CodexBackgroundTaskFrame = + | { + kind: 'subagent' + agentThreadId: string + label: string | null + parentTurnId: string | null | undefined + } + | { + kind: 'turn' + threadId: string + turnId: string + state: NativeChatSubagentState + } + +export type CodexBackgroundTaskEvent = { + method: string + threadId: string + params: unknown +} + +export function readCodexBackgroundTaskFrame( + event: CodexBackgroundTaskEvent, + primaryThreadId: string +): CodexBackgroundTaskFrame | null { + if (event.method === 'turn/started' || event.method === 'turn/completed') { + const turnId = readCodexTurnId(event.params) + if (turnId === null) { + return null + } + return { + kind: 'turn', + threadId: event.threadId, + turnId, + state: + event.method === 'turn/started' + ? 'working' + : codexChildTurnState(readRecord(readRecord(event.params).turn).status) + } + } + if (event.method !== 'item/started' && event.method !== 'item/completed') { + return null + } + const item = readCodexThreadItem(readRecord(event.params).item) + const activity = item && readCodexSubagentActivity(item) + if ( + !activity || + activity.agentThreadId === primaryThreadId || + isCodexRootAgentActivity(activity) + ) { + return null + } + return { + kind: 'subagent', + agentThreadId: activity.agentThreadId, + label: codexSubagentLabel(activity), + parentTurnId: + activity.kind === 'started' || activity.kind === 'interacted' + ? readCodexTurnId(event.params) + : undefined + } +} diff --git a/src/main/codex/codex-background-task-tracker.test.ts b/src/main/codex/codex-background-task-tracker.test.ts new file mode 100644 index 00000000000..47987fe3fc0 --- /dev/null +++ b/src/main/codex/codex-background-task-tracker.test.ts @@ -0,0 +1,280 @@ +import { describe, expect, it } from 'vitest' +import { CodexBackgroundTaskTracker } from './codex-background-task-tracker' +import { + readCodexBackgroundTaskFrame, + type CodexBackgroundTaskEvent +} from './codex-background-task-frames' + +const PRIMARY = 'parent-thread' +const PARENT_TURN = 'parent-turn' +const CHILD = 'child-thread' +const CHILD_TURN = 'child-turn' + +function turn( + method: 'turn/started' | 'turn/completed', + threadId: string, + turnId: string, + status = 'completed' +): CodexBackgroundTaskEvent { + return { method, threadId, params: { threadId, turn: { id: turnId, status } } } +} + +function activity( + kind = 'started', + parentTurn = PARENT_TURN, + child = CHILD +): CodexBackgroundTaskEvent { + return { + method: 'item/started', + threadId: PRIMARY, + params: { + threadId: PRIMARY, + turnId: parentTurn, + item: { + type: 'subAgentActivity', + id: `activity-${kind}`, + kind, + agentThreadId: child, + agentPath: '/root/count_a' + } + } + } +} + +function runningChild(): CodexBackgroundTaskTracker { + const tracker = new CodexBackgroundTaskTracker(PRIMARY) + tracker.observe(turn('turn/started', PRIMARY, PARENT_TURN)) + tracker.observe(turn('turn/started', CHILD, CHILD_TURN)) + tracker.observe(activity()) + return tracker +} + +function command(threadId = PRIMARY, method = 'item/started'): CodexBackgroundTaskEvent { + return { + method, + threadId, + params: { + threadId, + turnId: PARENT_TURN, + item: { + type: 'commandExecution', + id: 'exec-1', + processId: '71831', + source: 'unifiedExecStartup', + command: 'sleep 90', + status: method === 'item/started' ? 'inProgress' : 'completed' + } + } + } +} + +describe('readCodexBackgroundTaskFrame', () => { + it('reads activity as child metadata without inferring execution state', () => { + expect(readCodexBackgroundTaskFrame(activity('interacted'), PRIMARY)).toEqual({ + kind: 'subagent', + agentThreadId: CHILD, + label: 'count_a', + parentTurnId: PARENT_TURN + }) + }) + + it('reads a child turn with its own execution identity', () => { + expect(readCodexBackgroundTaskFrame(turn('turn/started', CHILD, CHILD_TURN), PRIMARY)).toEqual({ + kind: 'turn', + threadId: CHILD, + turnId: CHILD_TURN, + state: 'working' + }) + }) + + it('does not register the primary thread even when its activity path is missing', () => { + const event = activity('interacted', PARENT_TURN, PRIMARY) + ;(event.params as { item: { agentPath?: string } }).item.agentPath = undefined + expect(readCodexBackgroundTaskFrame(event, PRIMARY)).toBeNull() + }) +}) + +describe('CodexBackgroundTaskTracker child execution ownership', () => { + it('does not claim work from an activity item without a child turn', () => { + const tracker = new CodexBackgroundTaskTracker(PRIMARY) + tracker.observe(activity()) + tracker.observe(activity('interacted')) + tracker.observe(turn('turn/completed', PRIMARY, PARENT_TURN)) + expect(tracker.state).toBeNull() + }) + + it('reports an executing child only after the foreground turn ends', () => { + const tracker = runningChild() + expect(tracker.state).toBeNull() + expect(tracker.observe(turn('turn/completed', PRIMARY, PARENT_TURN))).toBe(true) + expect(tracker.state).toEqual({ + state: 'monitoring', + supportsStopAll: false, + tasks: [{ id: `codex-agent:${CHILD}`, kind: 'agent', description: 'count_a' }] + }) + }) + + it('never settles a child when a primary turn ends', () => { + const tracker = runningChild() + tracker.observe(turn('turn/completed', PRIMARY, PARENT_TURN)) + for (let index = 0; index < 300; index++) { + expect(tracker.observe(turn('turn/completed', PRIMARY, `later-${index}`))).toBe(false) + } + expect(tracker.state?.tasks).toHaveLength(1) + }) + + it.each(['completed', 'interrupted', 'failed'])( + 'settles on the matching child turn %s', + (status) => { + const tracker = runningChild() + tracker.observe(turn('turn/completed', PRIMARY, PARENT_TURN)) + expect(tracker.observe(turn('turn/completed', CHILD, CHILD_TURN, status))).toBe(true) + expect(tracker.state).toBeNull() + } + ) + + it('does not mistake late activity completion for the current child execution', () => { + const tracker = runningChild() + tracker.observe(turn('turn/completed', PRIMARY, PARENT_TURN)) + tracker.observe(activity('completed')) + expect(tracker.state?.tasks).toHaveLength(1) + }) + + it.each([PARENT_TURN, 'followup-parent'])( + 'reports follow-up work in %s using the new child turn', + (parentTurn) => { + const tracker = runningChild() + tracker.observe(turn('turn/completed', CHILD, CHILD_TURN)) + tracker.observe(turn('turn/completed', PRIMARY, PARENT_TURN)) + tracker.observe(turn('turn/started', PRIMARY, parentTurn)) + tracker.observe(activity('interacted', parentTurn)) + expect(tracker.state).toBeNull() + tracker.observe(turn('turn/started', CHILD, 'followup-child-turn')) + tracker.observe(turn('turn/completed', PRIMARY, parentTurn)) + expect(tracker.state?.tasks).toHaveLength(1) + tracker.observe(turn('turn/completed', CHILD, CHILD_TURN)) + tracker.observe(turn('turn/started', CHILD, CHILD_TURN)) + tracker.observe(activity('completed')) + expect(tracker.state?.tasks).toHaveLength(1) + tracker.observe(turn('turn/completed', CHILD, 'followup-child-turn')) + expect(tracker.state).toBeNull() + } + ) + + it('keeps idle send_message activity out of the strip', () => { + const tracker = runningChild() + tracker.observe(turn('turn/completed', CHILD, CHILD_TURN)) + tracker.observe(turn('turn/completed', PRIMARY, PARENT_TURN)) + tracker.observe(activity('interacted', 'message-parent')) + tracker.observe(turn('turn/completed', PRIMARY, 'message-parent')) + expect(tracker.state).toBeNull() + }) + + it('does not invent another execution for a message to a working child', () => { + const tracker = runningChild() + tracker.observe(turn('turn/completed', PRIMARY, PARENT_TURN)) + tracker.observe(activity('interacted', 'message-parent')) + tracker.observe(turn('turn/completed', PRIMARY, 'message-parent')) + expect(tracker.state?.tasks).toHaveLength(1) + tracker.observe(turn('turn/completed', CHILD, CHILD_TURN)) + expect(tracker.state).toBeNull() + }) + + it('retains completion delivered before child registration', () => { + const tracker = new CodexBackgroundTaskTracker(PRIMARY) + tracker.observe(turn('turn/started', CHILD, CHILD_TURN)) + tracker.observe(turn('turn/completed', CHILD, CHILD_TURN)) + tracker.observe(activity()) + tracker.observe(turn('turn/completed', PRIMARY, PARENT_TURN)) + expect(tracker.state).toBeNull() + }) + + it('publishes no extra state for duplicate owner or metadata events', () => { + const tracker = runningChild() + tracker.observe(turn('turn/completed', PRIMARY, PARENT_TURN)) + expect(tracker.observe(turn('turn/started', CHILD, CHILD_TURN))).toBe(false) + expect(tracker.observe({ ...activity(), method: 'item/completed' })).toBe(false) + expect(tracker.observe(turn('turn/completed', CHILD, CHILD_TURN))).toBe(true) + expect(tracker.observe(turn('turn/completed', CHILD, CHILD_TURN))).toBe(false) + }) + + it('bounds retained child history while allowing repeated completed runs', () => { + const tracker = new CodexBackgroundTaskTracker(PRIMARY) + tracker.observe(activity()) + for (let index = 0; index < 300; index++) { + const id = `child-turn-${index}` + tracker.observe(turn('turn/started', CHILD, id)) + expect(tracker.state?.tasks).toHaveLength(1) + tracker.observe(turn('turn/completed', CHILD, id)) + expect(tracker.state).toBeNull() + } + }) + + it('clears the roster at session teardown', () => { + const tracker = runningChild() + tracker.observe(turn('turn/completed', PRIMARY, PARENT_TURN)) + expect(tracker.clear()).toBe(true) + expect(tracker.state).toBeNull() + expect(tracker.clear()).toBe(false) + }) +}) + +describe('CodexBackgroundTaskTracker command integration', () => { + it('keeps a primary shell visible after the turn until its own completion', () => { + const tracker = new CodexBackgroundTaskTracker(PRIMARY) + tracker.observe(turn('turn/started', PRIMARY, PARENT_TURN)) + tracker.observe(command()) + expect(tracker.state).toBeNull() + tracker.observe(turn('turn/completed', PRIMARY, PARENT_TURN)) + expect(tracker.state?.tasks).toEqual([ + { id: 'codex-command:primary:exec-1', kind: 'command', description: 'sleep 90' } + ]) + tracker.observe(command(PRIMARY, 'item/completed')) + expect(tracker.state).toBeNull() + }) + + it('reveals a child shell only after the child execution finishes', () => { + const tracker = runningChild() + tracker.observe(turn('turn/completed', PRIMARY, PARENT_TURN)) + tracker.observe(command(CHILD)) + expect(tracker.state?.tasks).toHaveLength(1) + tracker.observe(turn('turn/completed', CHILD, CHILD_TURN, 'interrupted')) + expect(tracker.state?.tasks).toEqual([ + { + id: `codex-command:thread:${CHILD}:exec-1`, + kind: 'command', + description: 'count_a — sleep 90' + } + ]) + tracker.observe(command(CHILD, 'item/completed')) + expect(tracker.state).toBeNull() + }) + + it('leaves a primary shell unqualified', () => { + const tracker = runningChild() + tracker.observe(command(PRIMARY)) + tracker.observe(turn('turn/completed', PRIMARY, PARENT_TURN)) + expect(tracker.state?.tasks).toContainEqual({ + id: 'codex-command:primary:exec-1', + kind: 'command', + description: 'sleep 90' + }) + }) + + it('names a child shell whose label only arrives after the command', () => { + const tracker = new CodexBackgroundTaskTracker(PRIMARY) + tracker.observe(turn('turn/started', PRIMARY, PARENT_TURN)) + tracker.observe(turn('turn/started', CHILD, CHILD_TURN)) + tracker.observe(command(CHILD)) + tracker.observe(turn('turn/completed', PRIMARY, PARENT_TURN)) + tracker.observe(activity()) + tracker.observe(turn('turn/completed', CHILD, CHILD_TURN)) + expect(tracker.state?.tasks).toEqual([ + { + id: `codex-command:thread:${CHILD}:exec-1`, + kind: 'command', + description: 'count_a — sleep 90' + } + ]) + }) +}) diff --git a/src/main/codex/codex-background-task-tracker.ts b/src/main/codex/codex-background-task-tracker.ts new file mode 100644 index 00000000000..2918f087809 --- /dev/null +++ b/src/main/codex/codex-background-task-tracker.ts @@ -0,0 +1,96 @@ +import type { + AgentSessionBackgroundTask, + AgentSessionBackgroundTaskState +} from '../../shared/agent-session-wire' +import { + readCodexBackgroundTaskFrame, + type CodexBackgroundTaskEvent +} from './codex-background-task-frames' +import { CodexSubagentExecutions } from './codex-subagent-executions' +import { CodexBackgroundCommandTracker } from './codex-background-command-tracker' +import { boundSubagentField } from './codex-subagent-group-body' + +/** Projects the same child execution facts the durable roster consumes. */ +export class CodexBackgroundTaskTracker { + private primaryTurnId: string | null = null + private publishedFingerprint = '[]' + private publishedState: AgentSessionBackgroundTaskState | null = null + private readonly commands: CodexBackgroundCommandTracker + + constructor( + private readonly primaryThreadId: string, + private readonly executions = new CodexSubagentExecutions() + ) { + this.commands = new CodexBackgroundCommandTracker(primaryThreadId) + } + + get state(): AgentSessionBackgroundTaskState | null { + // Journal admission precedes observe; readers must not see its pending facts. + return this.publishedState + } + + canObserve(event: CodexBackgroundTaskEvent): boolean { + return this.commands.canObserve(event) + } + + observe(event: CodexBackgroundTaskEvent): boolean { + const itemEvent = event.method === 'item/started' || event.method === 'item/completed' + if (itemEvent) { + this.commands.observe(event) + } + const frame = readCodexBackgroundTaskFrame(event, this.primaryThreadId) + if (!frame) { + return itemEvent ? this.refresh() : false + } + if (frame.kind === 'subagent') { + this.executions.register(frame.agentThreadId, frame.label, frame.parentTurnId) + } else if (frame.threadId === this.primaryThreadId) { + if (frame.state === 'working') { + this.primaryTurnId = frame.turnId + } else if (frame.turnId === this.primaryTurnId) { + this.primaryTurnId = null + } + } else { + this.executions.observeTurn(frame.threadId, frame.turnId, frame.state) + } + return this.refresh() + } + + clear(): boolean { + this.executions.clear() + this.commands.clear() + this.primaryTurnId = null + return this.refresh() + } + + private tasks(): AgentSessionBackgroundTask[] { + if (this.primaryTurnId !== null) { + return [] + } + const children = this.executions.workingChildren() + const agents: AgentSessionBackgroundTask[] = children.map((child, index) => ({ + id: `codex-agent:${child.agentThreadId}`, + kind: 'agent', + ...(child.label ? { description: boundSubagentField(child.label, index) } : {}) + })) + return [ + ...agents, + ...this.commands.tasks(new Set(children.map((child) => child.agentThreadId)), (threadId) => + this.executions.label(threadId) + ) + ] + } + + private refresh(): boolean { + const tasks = this.tasks() + const fingerprint = JSON.stringify(tasks) + if (fingerprint === this.publishedFingerprint) { + return false + } + this.publishedFingerprint = fingerprint + this.publishedState = tasks.length + ? { state: 'monitoring', tasks, supportsStopAll: false } + : null + return true + } +} diff --git a/src/main/codex/codex-command-lifecycle.ts b/src/main/codex/codex-command-lifecycle.ts new file mode 100644 index 00000000000..dac8d7ddf97 --- /dev/null +++ b/src/main/codex/codex-command-lifecycle.ts @@ -0,0 +1,6 @@ +import type { CodexThreadItem } from './codex-structured-item-translation' + +/** Persistent exec has its own process-exit notification, independent of a turn. */ +export function codexCommandOutlivesTurn(item: CodexThreadItem): boolean { + return item.type === 'commandExecution' && item.source === 'unifiedExecStartup' +} diff --git a/src/main/codex/codex-item-stream-retention.ts b/src/main/codex/codex-item-stream-retention.ts new file mode 100644 index 00000000000..ace12f34667 --- /dev/null +++ b/src/main/codex/codex-item-stream-retention.ts @@ -0,0 +1,107 @@ +import { codexCommandOutlivesTurn } from './codex-command-lifecycle' +import { + MAX_CODEX_ITEM_STREAM_ITEM_BYTES, + MAX_CODEX_ITEM_STREAM_STATES +} from './codex-structured-item-stream-bounds' +import type { CodexItemStreamState } from './codex-structured-item-stream-contracts' + +// Preserve the previous metadata ceiling while letting small live commands share it. +export const MAX_CODEX_ITEM_STREAM_METADATA_BYTES = + MAX_CODEX_ITEM_STREAM_STATES * MAX_CODEX_ITEM_STREAM_ITEM_BYTES + +type RetainedState = { state: CodexItemStreamState; bytes: number; persistent: boolean } + +export class CodexItemStreamRetention { + private readonly states = new Map() + private bytes = 0 + private persistentBytes = 0 + private persistentCount = 0 + + constructor(private readonly maxBytes = MAX_CODEX_ITEM_STREAM_METADATA_BYTES) {} + + get retainedBytes(): number { + return this.bytes + } + + get size(): number { + return this.states.size + } + + get persistentSize(): number { + return this.persistentCount + } + + get overCapacity(): boolean { + return ( + this.bytes > this.maxBytes || + this.states.size - this.persistentCount > MAX_CODEX_ITEM_STREAM_STATES + ) + } + + get(key: string): CodexItemStreamState | undefined { + return this.states.get(key)?.state + } + + isPersistent(key: string): boolean { + return this.states.get(key)?.persistent === true + } + + canRetain(key: string, state: CodexItemStreamState): boolean { + const previous = this.states.get(key) + return ( + this.persistentBytes - + (previous?.persistent ? previous.bytes : 0) + + this.stateBytes(key, state) <= + this.maxBytes + ) + } + + retain(key: string, state: CodexItemStreamState): boolean { + if (!this.canRetain(key, state)) { + return false + } + this.forget(key) + const bytes = this.stateBytes(key, state) + const persistent = codexCommandOutlivesTurn(state.item) + this.states.set(key, { state, bytes, persistent }) + this.bytes += bytes + if (persistent) { + this.persistentBytes += bytes + this.persistentCount += 1 + } + return true + } + + oldestEvictable(): string | undefined { + for (const [key, entry] of this.states) { + if (!entry.persistent) { + return key + } + } + return undefined + } + + forget(key: string): void { + const entry = this.states.get(key) + if (!entry) { + return + } + this.bytes -= entry.bytes + if (entry.persistent) { + this.persistentBytes -= entry.bytes + this.persistentCount -= 1 + } + this.states.delete(key) + } + + clear(): void { + this.states.clear() + this.bytes = 0 + this.persistentBytes = 0 + this.persistentCount = 0 + } + + private stateBytes(key: string, state: CodexItemStreamState): number { + return Buffer.byteLength(key, 'utf8') + Buffer.byteLength(JSON.stringify(state), 'utf8') + 256 + } +} diff --git a/src/main/codex/codex-persistent-command-retention.test.ts b/src/main/codex/codex-persistent-command-retention.test.ts new file mode 100644 index 00000000000..2acb357cd71 --- /dev/null +++ b/src/main/codex/codex-persistent-command-retention.test.ts @@ -0,0 +1,219 @@ +import { describe, expect, it, vi } from 'vitest' +import type { AgentJournalItemBody } from '../../shared/agent-session-journal-types' +import { agentJournalItemKey } from '../../shared/agent-session-journal-item-key' +import { CodexBackgroundCommandTracker } from './codex-background-command-tracker' +import { + CodexItemStreamRetention, + MAX_CODEX_ITEM_STREAM_METADATA_BYTES +} from './codex-item-stream-retention' +import { CodexJournalItems } from './codex-structured-journal-items' +import { settleCodexJournalTurn } from './codex-structured-journal-settlement' + +function command(threadId: string, id: string, method = 'item/started') { + return { + threadId, + method, + params: { + turnId: 'turn', + item: { + type: 'commandExecution', + id, + source: 'unifiedExecStartup', + command: `sleep 30 # ${threadId}/${id}`, + cwd: '/workspace', + status: method === 'item/completed' ? 'completed' : 'inProgress', + ...(method === 'item/completed' ? { exitCode: 0, aggregatedOutput: 'BEFORE\nAFTER\n' } : {}) + } + } + } +} + +function fixture(maxMetadataBytes?: number) { + const rows = new Map() + const scheduled = new Set<() => void>() + const sink = { + appendItem: ( + identity: Parameters[0], + body: AgentJournalItemBody + ) => { + rows.set(agentJournalItemKey(identity), body) + }, + appendTombstone() {}, + publish() {} + } + const items = new CodexJournalItems( + { + sink, + maxMetadataBytes, + schedule: (run) => { + scheduled.add(run) + return () => { + scheduled.delete(run) + } + } + }, + () => 'turn', + () => {} + ) + return { items, sink, rows, scheduled } +} + +describe('persistent command retention', () => { + it('does not rebuild unchanged persistent output on every later lifecycle flush', () => { + const { items } = fixture() + const event = command('root', 'quiet') + items.handle(event) + items.streams.handle('root', 'item/commandExecution/outputDelta', { + itemId: 'quiet', + delta: 'retained-prefix' + }) + items.streams.flush() + const originalJoin = Array.prototype.join + let retainedJoins = 0 + const spy = vi + .spyOn(Array.prototype, 'join') + .mockImplementation(function (this: unknown[], separator) { + if (this[0] === 'retained-prefix') { + retainedJoins += 1 + } + return originalJoin.call(this, separator) + }) + try { + for (let index = 0; index < 100; index += 1) { + items.streams.flush() + } + } finally { + spy.mockRestore() + items.dispose() + } + expect(retainedJoins).toBe(0) + }) + + it('retains 448 live commands through completed turns, late output, and process completion', () => { + const { items, sink, rows, scheduled } = fixture() + const tracker = new CodexBackgroundCommandTracker('thread-0') + const events = Array.from({ length: 7 }, (_, thread) => + Array.from({ length: 64 }, (_, index) => command(`thread-${thread}`, `exec-${index}`)) + ).flat() + for (const event of events) { + expect(tracker.canObserve(event)).toBe(true) + expect(items.handle(event)).toMatchObject({ admission: { accepted: true } }) + tracker.observe(event) + expect( + items.streams.handle(event.threadId, 'item/commandExecution/outputDelta', { + turnId: 'turn', + itemId: event.params.item.id, + delta: 'BEFORE\n' + }).admission + ).toEqual({ accepted: true }) + } + for (let thread = 0; thread < 7; thread += 1) { + expect( + settleCodexJournalTurn({ + sessionId: 'session', + threadId: `thread-${thread}`, + turnId: 'turn', + sink, + streams: items.streams, + activeItems: items.activeItems + }) + ).toEqual({ accepted: true }) + } + expect(items.activeItems.size).toBe(448) + expect(items.streams.persistentCount).toBe(448) + expect(tracker.tasks()).toHaveLength(448) + expect(tracker.retainedMetadataBytes).toBeLessThan(256 * 1024) + for (const event of events) { + items.streams.handle(event.threadId, 'item/commandExecution/outputDelta', { + turnId: 'turn', + itemId: event.params.item.id, + delta: 'AFTER\n' + }) + } + expect(items.streams.flush()).toBe(true) + for (const event of events) { + const key = agentJournalItemKey({ + provider: 'orca', + clientMessageId: `codex-item:${event.threadId}:${event.params.item.id}` + }) + expect(rows.get(key)).toMatchObject({ + state: 'running', + input: { command: event.params.item.command, cwd: '/workspace' }, + output: { head: 'BEFORE\nAFTER\n' } + }) + const completed = command(event.threadId, event.params.item.id, 'item/completed') + expect(items.handle(completed)).toMatchObject({ admission: { accepted: true } }) + tracker.observe(completed) + expect(rows.get(key)).toMatchObject({ + state: 'completed', + output: { head: 'BEFORE\nAFTER\n' } + }) + expect(items.streams.snapshot(event.threadId, event.params.item.id)).toBeNull() + } + expect(items.activeItems.size).toBe(0) + expect(items.streams.persistentCount).toBe(0) + expect(tracker.tasks()).toEqual([]) + expect(tracker.retainedMetadataBytes).toBeLessThan(64 * 1024) + items.dispose() + tracker.clear() + expect(tracker.retainedMetadataBytes).toBe(0) + expect(scheduled.size).toBe(0) + }) + + it('rejects command metadata exhaustion before appending or evicting live state and frees it on completion', () => { + const { items, rows } = fixture(800) + const first = command('root', 'first') + const second = command('root', 'second') + expect(items.handle(first)).toMatchObject({ admission: { accepted: true } }) + const prior = [...rows] + expect(items.handle(second)).toMatchObject({ admission: { accepted: false, reason: 'failed' } }) + expect([...rows]).toEqual(prior) + expect(items.activeItems.size).toBe(1) + expect(items.handle(command('root', 'first', 'item/completed'))).toMatchObject({ + admission: { accepted: true } + }) + expect(items.handle(second)).toMatchObject({ admission: { accepted: true } }) + items.dispose() + }) + + it('accounts metadata bytes instead of interpreting the item count as liveness', () => { + const retention = new CodexItemStreamRetention() + for (let index = 0; index < 448; index += 1) { + const item = command('root', `exec-${index}`).params.item + expect( + retention.retain(item.id, { + item, + identity: { provider: 'orca', clientMessageId: item.id } + }) + ).toBe(true) + } + expect(retention.size).toBe(448) + expect(retention.retainedBytes).toBeLessThan(256 * 1024) + expect(retention.retainedBytes).toBeLessThan(MAX_CODEX_ITEM_STREAM_METADATA_BYTES) + expect(retention.overCapacity).toBe(false) + expect(retention.oldestEvictable()).toBeUndefined() + retention.clear() + expect(retention.retainedBytes).toBe(0) + expect(retention.persistentSize).toBe(0) + }) + + it('retains startup provenance when large command metadata is bounded', () => { + const { items, sink } = fixture() + const event = command('root', 'large') + event.params.item.command = 'x'.repeat(128 * 1024) + expect(items.handle(event)).toMatchObject({ admission: { accepted: true } }) + expect( + settleCodexJournalTurn({ + sessionId: 'session', + threadId: 'root', + turnId: 'turn', + sink, + streams: items.streams, + activeItems: items.activeItems + }) + ).toEqual({ accepted: true }) + expect(items.activeItems.size).toBe(1) + expect(items.streams.persistentCount).toBe(1) + items.dispose() + }) +}) diff --git a/src/main/codex/codex-structured-item-stream-bounds.ts b/src/main/codex/codex-structured-item-stream-bounds.ts index 117b9b3a911..46104c54360 100644 --- a/src/main/codex/codex-structured-item-stream-bounds.ts +++ b/src/main/codex/codex-structured-item-stream-bounds.ts @@ -30,6 +30,7 @@ export function boundStreamItem(item: Record): Record void + readonly persistentCount: number + canTrack: (threadId: string, item: CodexThreadItem, identity: AgentJournalItemIdentity) => boolean + track: (threadId: string, item: CodexThreadItem, identity: AgentJournalItemIdentity) => boolean handle: ( threadId: string, method: string, diff --git a/src/main/codex/codex-structured-item-streams.ts b/src/main/codex/codex-structured-item-streams.ts index a765f8339da..e67263f133d 100644 --- a/src/main/codex/codex-structured-item-streams.ts +++ b/src/main/codex/codex-structured-item-streams.ts @@ -1,5 +1,6 @@ import { agentJournalItemKey } from '../../shared/agent-session-journal-item-key' import { createAgentSessionDeltaCoalescer } from '../native-chat/agent-session-wire/agent-session-delta-coalescer' +import { CodexItemStreamRetention } from './codex-item-stream-retention' import { codexJournalItem, codexStreamingJournalItem, @@ -10,7 +11,6 @@ import { MAX_CODEX_ITEM_STREAM_PENDING_PATCHES, MAX_CODEX_ITEM_STREAM_PENDING_PATCH_BYTES, MAX_CODEX_ITEM_STREAM_RETAINED_BYTES, - MAX_CODEX_ITEM_STREAM_STATES, boundStreamItem, pendingPatchBytes } from './codex-structured-item-stream-bounds' @@ -47,8 +47,9 @@ export { export function createCodexStructuredItemStreams( deps: CodexItemStreamDeps ): CodexStructuredItemStreams { - const states = new Map() + const states = new CodexItemStreamRetention(deps.maxMetadataBytes) const checkpointLengths = new Map() + const pendingCheckpoints = new Set() // Patch updates are authoritative item snapshots. Keep the latest rejected // snapshot until the journal admits it; unlike streamed deltas, there is no // coalescer timer to retry these events for us. @@ -57,8 +58,9 @@ export function createCodexStructuredItemStreams( const forgetState = (key: string): void => { coalescer.forget(key) - states.delete(key) + states.forget(key) checkpointLengths.delete(key) + pendingCheckpoints.delete(key) const pending = pendingPatches.get(key) if (pending) { retainedPatchBytes = Math.max(0, retainedPatchBytes - pendingPatchBytes(pending)) @@ -67,8 +69,8 @@ export function createCodexStructuredItemStreams( } const trimStates = (): void => { - while (states.size > MAX_CODEX_ITEM_STREAM_STATES) { - const oldest = states.keys().next().value + while (states.overCapacity) { + const oldest = states.oldestEvictable() if (typeof oldest !== 'string') { break } @@ -124,6 +126,7 @@ export function createCodexStructuredItemStreams( const state = states.get(key) if (state && append(state, text)) { checkpointLengths.set(key, text.length) + pendingCheckpoints.delete(key) return true } return false @@ -133,6 +136,7 @@ export function createCodexStructuredItemStreams( windowMs: deps.coalesceMs, maxRetainedBytes: deps.maxRetainedBytes, maxTotalRetainedBytes: deps.maxTotalRetainedBytes, + isProtected: (key) => states.isPersistent(key), schedule: deps.schedule, emit: (key, text) => { return persist(key, text, false) @@ -144,7 +148,7 @@ export function createCodexStructuredItemStreams( itemId: string, type: string, params: unknown - ): CodexItemStreamState => { + ): CodexItemStreamState | null => { const key = codexStructuredItemKey(threadId, itemId) const existing = states.get(key) if (existing) { @@ -152,36 +156,25 @@ export function createCodexStructuredItemStreams( } const item = { type, id: itemId } const state = { item, identity: deps.identityFor(threadId, params, item) } - states.set(key, state) + if (!states.retain(key, state)) { + return null + } trimStates() return state } const flush = (): boolean => { let flushed = coalescer.flushAll() - for (const key of states.keys()) { + for (const key of pendingCheckpoints) { const snapshot = coalescer.snapshot(key) if (snapshot && checkpointLengths.get(key) !== snapshot.text.length) { flushed = persist(key, snapshot.text, true) && flushed + } else { + pendingCheckpoints.delete(key) } } - for (const [key, pending] of pendingPatches) { - const admission = deps.sink.tryAppendItem - ? deps.sink.tryAppendItem(pending.identity, pending.body) - : (deps.sink.appendItem(pending.identity, pending.body), { accepted: true as const }) - if (!admission.accepted) { - flushed = false - continue - } - const published = deps.sink.tryPublish - ? deps.sink.tryPublish() - : (deps.sink.publish(), { accepted: true as const }) - if (!published.accepted) { - flushed = false - continue - } - retainedPatchBytes = Math.max(0, retainedPatchBytes - pendingPatchBytes(pending)) - pendingPatches.delete(key) + for (const key of pendingPatches.keys()) { + flushed = flushPatch(key).accepted && flushed } return flushed } @@ -209,11 +202,21 @@ export function createCodexStructuredItemStreams( } return { + get persistentCount() { + return states.persistentSize + }, + canTrack: (threadId, item, identity) => + states.canRetain(codexStructuredItemKey(threadId, item.id), { + item: boundStreamItem(item) as CodexThreadItem, + identity + }), track: (threadId, item, identity) => { const key = codexStructuredItemKey(threadId, item.id) - states.delete(key) - states.set(key, { item: boundStreamItem(item) as CodexThreadItem, identity }) + if (!states.retain(key, { item: boundStreamItem(item) as CodexThreadItem, identity })) { + return false + } trimStates() + return true }, handle: (threadId, method, params) => { const paramsRecord = readCodexItemStreamRecord(params) @@ -230,6 +233,9 @@ export function createCodexStructuredItemStreams( const key = codexStructuredItemKey(threadId, itemId) const streamFlushed = coalescer.flush(key) const state = ensureState(threadId, itemId, 'fileChange', params) + if (!state) { + return { handled: true, admission: { accepted: false, reason: 'failed' } } + } state.item = { ...state.item, changes: paramsRecord.changes } const translated = codexJournalItem(state.item) if (translated.body) { @@ -269,9 +275,14 @@ export function createCodexStructuredItemStreams( return { handled: true, admission: { accepted: true } } } const state = ensureState(threadId, itemId, type ?? 'reasoning', params) + if (!state) { + return { handled: true, admission: { accepted: false, reason: 'failed' } } + } const delta = method === REASONING_PART_METHOD ? '\n' : paramsRecord.delta if (typeof delta === 'string') { - const accepted = coalescer.append(codexStructuredItemKey(threadId, state.item.id), delta) + const key = codexStructuredItemKey(threadId, state.item.id) + pendingCheckpoints.add(key) + const accepted = coalescer.append(key, delta) if (!accepted) { return { handled: true, admission: { accepted: false, reason: 'backpressure' } } } @@ -287,6 +298,7 @@ export function createCodexStructuredItemStreams( coalescer.dispose() states.clear() checkpointLengths.clear() + pendingCheckpoints.clear() pendingPatches.clear() retainedPatchBytes = 0 }, diff --git a/src/main/codex/codex-structured-item-translation.test.ts b/src/main/codex/codex-structured-item-translation.test.ts index 45afb0c9fa5..201df58bc90 100644 --- a/src/main/codex/codex-structured-item-translation.test.ts +++ b/src/main/codex/codex-structured-item-translation.test.ts @@ -584,6 +584,16 @@ describe('codex item bodies', () => { }) }) + it('preserves plan prose documents byte-for-byte as status text', () => { + const text = + ' # Implementation plan\r\n\r\n- [ ] Preserve prose\r\n- [x] Keep café → 日本語\r\n\r\n```ts\r\nconst task = "pending"\r\n```\r\n ' + + expect(codexJournalItem({ type: 'plan', id: 'plan-document', text })).toEqual({ + body: { kind: 'status', text, presentation: 'plan-document' }, + handled: true + }) + }) + it('renders reasoning as status and exposes an unknown item as a provider frame', () => { expect(codexItemBody({ type: 'reasoning', id: 'r', text: 'thinking' })).toEqual({ kind: 'status', diff --git a/src/main/codex/codex-structured-journal-contracts.ts b/src/main/codex/codex-structured-journal-contracts.ts index acd04a4cf30..56a543fe001 100644 --- a/src/main/codex/codex-structured-journal-contracts.ts +++ b/src/main/codex/codex-structured-journal-contracts.ts @@ -1,11 +1,13 @@ import type { AgentSessionDeltaCoalescerDeps } from '../native-chat/agent-session-wire/agent-session-delta-coalescer' import type { StructuredAgentSessionEventSink } from '../native-chat/agent-session-wire/structured-agent-session-event-sink' import type { CodexStructuredSessionEvent } from './codex-structured-session-adapter' +import type { CodexSubagentExecutions } from './codex-subagent-executions' export type CodexJournalTranslatorDeps = { sink: StructuredAgentSessionEventSink bindPromptItemId?: (journalItemId: string, threadId: string, promptKey: string) => void primaryThreadId?: () => string | null + subagentExecutions?: CodexSubagentExecutions coalesceMs?: number maxRetainedBytes?: number schedule?: AgentSessionDeltaCoalescerDeps['schedule'] diff --git a/src/main/codex/codex-structured-journal-items.ts b/src/main/codex/codex-structured-journal-items.ts index f984bc1d9bf..62091580da3 100644 --- a/src/main/codex/codex-structured-journal-items.ts +++ b/src/main/codex/codex-structured-journal-items.ts @@ -11,7 +11,8 @@ import { type CodexThreadItem } from './codex-structured-item-translation' import { createCodexStructuredItemStreams } from './codex-structured-item-streams' -import { codexStructuredItemKey } from './codex-structured-item-stream-bounds' +import { boundStreamItem, codexStructuredItemKey } from './codex-structured-item-stream-bounds' +import { codexCommandOutlivesTurn } from './codex-command-lifecycle' import type { CodexItemTranslation, CodexJournalTranslationAdmission, @@ -40,7 +41,7 @@ export class CodexJournalItems { private readonly deps: Pick< CodexJournalTranslatorDeps, 'sink' | 'coalesceMs' | 'maxRetainedBytes' | 'schedule' - >, + > & { maxMetadataBytes?: number }, private readonly activeTurn: (threadId: string) => string | null, private readonly suppress: (threadId: string, turnId: string) => void ) { @@ -49,6 +50,7 @@ export class CodexJournalItems { coalesceMs: deps.coalesceMs, maxRetainedBytes: deps.maxRetainedBytes, schedule: deps.schedule, + maxMetadataBytes: deps.maxMetadataBytes, identityFor: (threadId, params, item) => { const turnId = readCodexTurnId(params) ?? this.activeTurn(threadId) return this.identityFor(threadId, turnId, item) @@ -81,6 +83,12 @@ export class CodexJournalItems { if (item.type === 'contextCompaction' && event.method === 'item/started') { return { handled: true, admission: CODEX_JOURNAL_ADMITTED } } + if ( + event.method !== 'item/completed' && + !this.streams.canTrack(event.threadId, item, identity) + ) { + return { handled: true, admission: { accepted: false, reason: 'failed' } } + } const translated = codexJournalItem(item) const command = readCodexJournalString(item, 'command') if (command) { @@ -157,12 +165,15 @@ export class CodexJournalItems { item: CodexThreadItem, identity: AgentJournalItemIdentity ): void { - this.streams.track(threadId, item, identity) + const retainedItem = codexCommandOutlivesTurn(item) + ? (boundStreamItem(item) as CodexThreadItem) + : item + this.streams.track(threadId, retainedItem, identity) this.activeItems.set(codexStructuredItemKey(threadId, item.id), { threadId, turnId, identity, - item + item: retainedItem }) } @@ -194,8 +205,10 @@ export class CodexJournalItems { } private trimActiveState(): CodexJournalTranslationAdmission { - while (this.activeItems.size > MAX_CODEX_ACTIVE_ITEMS) { - const oldest = this.activeItems.keys().next().value + while (this.activeItems.size - this.streams.persistentCount > MAX_CODEX_ACTIVE_ITEMS) { + const oldest = [...this.activeItems].find( + ([, active]) => !codexCommandOutlivesTurn(active.item) + )?.[0] if (typeof oldest !== 'string') { break } diff --git a/src/main/codex/codex-structured-journal-settlement.ts b/src/main/codex/codex-structured-journal-settlement.ts index 2b8eb627d4f..8c8bf091839 100644 --- a/src/main/codex/codex-structured-journal-settlement.ts +++ b/src/main/codex/codex-structured-journal-settlement.ts @@ -20,6 +20,7 @@ import { } from './codex-structured-item-translation' import type { CodexStructuredItemStreams } from './codex-structured-item-streams' import type { CodexStructuredSessionEvent } from './codex-structured-session-adapter' +import { codexCommandOutlivesTurn } from './codex-command-lifecycle' export type CodexActiveJournalItem = { threadId: string @@ -118,6 +119,9 @@ export function settleCodexJournalTurn(input: { if (active.threadId !== input.threadId || active.turnId !== input.turnId) { continue } + if (codexCommandOutlivesTurn(active.item)) { + continue + } const streamed = input.streams.snapshot(active.threadId, active.item.id) const translated = streamed ? codexStreamingJournalItem(active.item, streamed.text) diff --git a/src/main/codex/codex-structured-journal-translation-frames.ts b/src/main/codex/codex-structured-journal-translation-frames.ts index 22dc516b210..d4cb2c825d9 100644 --- a/src/main/codex/codex-structured-journal-translation-frames.ts +++ b/src/main/codex/codex-structured-journal-translation-frames.ts @@ -6,6 +6,8 @@ * than as the shape checks each arm performs. */ +import type { CodexStructuredSessionEvent } from './codex-structured-session-adapter' +import type { CodexJournalItems } from './codex-structured-journal-items' import type { CodexJournalTranslationAdmission } from './codex-structured-journal-contracts' import { settleCodexOversizedNotification } from './codex-structured-journal-settlement' import { @@ -41,3 +43,23 @@ export function settleCodexOversizedNotificationFrame(input: { }) : null } + +export function createCodexOversizedNotificationSettler( + deps: { sink: OversizedInput['sink'] }, + items: Pick +) { + return settleOversizedNotification + + /** Settles the item a notification the transport refused to carry left + * mid-flight; null when the frame is not one. */ + function settleOversizedNotification( + event: Extract + ): CodexJournalTranslationAdmission | null { + return settleCodexOversizedNotificationFrame({ + ...event, + sink: deps.sink, + streams: items.streams, + activeItems: items.activeItems + }) + } +} diff --git a/src/main/codex/codex-structured-journal-translation-subagents.test.ts b/src/main/codex/codex-structured-journal-translation-subagents.test.ts index bf5cdffa5a9..8db30d31b21 100644 --- a/src/main/codex/codex-structured-journal-translation-subagents.test.ts +++ b/src/main/codex/codex-structured-journal-translation-subagents.test.ts @@ -59,6 +59,22 @@ function deliverActivity( translator: ReturnType, params: unknown ): void { + const item = (params as { item: { kind: string; agentThreadId: string } }).item + if (item.kind === 'started' || item.kind === 'completed') { + translator.handle({ + type: 'notification', + sessionId: SESSION_ID, + threadId: item.agentThreadId, + method: item.kind === 'started' ? 'turn/started' : 'turn/completed', + params: { + threadId: item.agentThreadId, + turn: { + id: `execution:${item.agentThreadId}`, + status: item.kind === 'started' ? 'inProgress' : 'completed' + } + } + }) + } translator.handle(notification('item/started', params)) translator.handle(notification('item/completed', params)) } diff --git a/src/main/codex/codex-structured-journal-translation.ts b/src/main/codex/codex-structured-journal-translation.ts index 00b90d7ffc8..cea129e5cdf 100644 --- a/src/main/codex/codex-structured-journal-translation.ts +++ b/src/main/codex/codex-structured-journal-translation.ts @@ -19,7 +19,7 @@ import { settleCodexJournalSession, settleCodexJournalTurn } from './codex-structured-journal-settlement' -import { settleCodexOversizedNotificationFrame } from './codex-structured-journal-translation-frames' +import { createCodexOversizedNotificationSettler } from './codex-structured-journal-translation-frames' import { restoreCodexJournalThread } from './codex-structured-journal-translation-restore' import { CodexJournalActiveTurns } from './codex-structured-journal-translation-turn-state' import { publishCodexTurnLifecycle } from './codex-structured-journal-translation-turns' @@ -58,13 +58,15 @@ export function createCodexJournalTranslator( (threadId) => activeTurns.current(threadId), (threadId, turnId) => genericFrames.suppress(threadId, turnId) ) + const settleOversizedNotification = createCodexOversizedNotificationSettler(deps, items) const prompts = new CodexJournalPrompts(deps, (threadId, itemId) => items.detailFor(threadId, itemId) ) const subagents = new CodexSubagentRoster({ sink: deps.sink, primaryThreadId: () => deps.primaryThreadId?.() ?? null, - activeTurn: (threadId) => activeTurns.current(threadId) + activeTurn: (threadId) => activeTurns.current(threadId), + ...(deps.subagentExecutions ? { executions: deps.subagentExecutions } : {}) }) const flushStreams = (): CodexJournalTranslationAdmission => items.streams.flush() ? CODEX_JOURNAL_ADMITTED : { accepted: false, reason: 'backpressure' } @@ -174,16 +176,17 @@ export function createCodexJournalTranslator( } return genericFrames.appendUnhandled(event.kind, event.payload, event.threadId) } - if (event.method === 'turn/started') { - return startTurn(event) + if (event.method === 'turn/started' || event.method === 'turn/completed') { + const childAdmission = subagents.handleTurnEvent(event) + if (!childAdmission.accepted) { + return childAdmission + } + return event.method === 'turn/started' ? startTurn(event) : completeTurn(event) } const compaction = compactions.handle(event) if (compaction) { return publishActivity(event, compaction) } - if (event.method === 'turn/completed') { - return completeTurn(event) - } if (event.method === CODEX_TOKEN_USAGE_METHOD) { // Classified `status-chrome`, so the generic-frame path swallows it // before the journal. The roster consumes it as a typed notification. @@ -240,19 +243,6 @@ export function createCodexJournalTranslator( } } - /** Settles the item a notification the transport refused to carry left - * mid-flight; null when the frame is not one. */ - function settleOversizedNotification( - event: Extract - ): CodexJournalTranslationAdmission | null { - return settleCodexOversizedNotificationFrame({ - ...event, - sink: deps.sink, - streams: items.streams, - activeItems: items.activeItems - }) - } - function startTurn( event: Extract ): CodexJournalTranslationAdmission { diff --git a/src/main/codex/codex-structured-session-acquire.ts b/src/main/codex/codex-structured-session-acquire.ts index 8c8b39ca48b..3f7f130e173 100644 --- a/src/main/codex/codex-structured-session-acquire.ts +++ b/src/main/codex/codex-structured-session-acquire.ts @@ -8,6 +8,8 @@ import { closeFailedCodexAcquisition, stopSupersededCodexAcquisition } from './codex-structured-acquisition-lifecycle' +import { CodexBackgroundTaskTracker } from './codex-background-task-tracker' +import { CodexSubagentExecutions } from './codex-subagent-executions' import { createCodexJournalTranslator } from './codex-structured-journal-translation' import { openCodexAppServerConnection } from './codex-app-server-connection' import { codexProcessIdentity, codexProviderHandleLink } from './codex-structured-owner-identity' @@ -74,10 +76,12 @@ export async function acquireCodexStructuredSession(input: { acquireInput.identity.providerHandle.kind === 'codex' ? acquireInput.identity.providerHandle.threadId : null + const subagentExecutions = new CodexSubagentExecutions() const translator = acquireInput.events ? createCodexJournalTranslator({ sink: acquireInput.events, primaryThreadId: () => primaryThreadId, + subagentExecutions, bindPromptItemId: (journalItemId, threadId, promptKey) => acquisition.prompts.bindJournalItemId(journalItemId, threadId, promptKey) }) @@ -138,6 +142,7 @@ export async function acquireCodexStructuredSession(input: { connection: acquisition.connection, error, prompts: acquisition.prompts, + onBackgroundTasksChanged: deps.onBackgroundTasksChanged, ...(deps.onEvent ? { onEvent: deps.onEvent } : {}) }) } finally { @@ -199,6 +204,7 @@ export async function acquireCodexStructuredSession(input: { reportedOptions: reportedCodexThreadOptions(opened), turnIdWaiters: [], translator, + backgroundTasks: new CodexBackgroundTaskTracker(opened.threadId, subagentExecutions), forceCloseUnexpected: (reason) => input.forceCloseUnexpected( sessionId, diff --git a/src/main/codex/codex-structured-session-adapter.ts b/src/main/codex/codex-structured-session-adapter.ts index ebd7c3331bf..8b0e649b177 100644 --- a/src/main/codex/codex-structured-session-adapter.ts +++ b/src/main/codex/codex-structured-session-adapter.ts @@ -16,11 +16,7 @@ import type { CodexJournalTranslationAdmission } from './codex-structured-journa import { answerCodexPrompt } from './codex-structured-prompt-replies' import { dispatchCodexTurn, isCodexTurnOptionKey } from './codex-structured-turn-start' import { supportsCodexStructuredLocation } from './codex-structured-location-support' -import { - closeAllCodexSessions, - closeCodexPublishedSession, - closeCodexSession -} from './codex-structured-session-close' +import { CodexStructuredSessionTeardown } from './codex-structured-session-teardown' import { applyCodexStructuredSessionOption, readLiveCodexSessionOptions @@ -54,6 +50,7 @@ export class CodexStructuredSessionAdapter implements StructuredAgentSessionAdap private readonly acquisitions = new CodexAcquisitionRegistry() private readonly turnCancellation: CodexStructuredTurnCancellation private readonly notificationRetries: ReturnType + private readonly teardown: CodexStructuredSessionTeardown constructor(private readonly deps: CodexStructuredSessionAdapterDeps) { this.notificationRetries = createCodexStructuredNotificationRetry({ @@ -61,6 +58,15 @@ export class CodexStructuredSessionAdapter implements StructuredAgentSessionAdap translate: (sessionId, session, method, params) => this.translateNotification(sessionId, session, method, params) }) + this.teardown = new CodexStructuredSessionTeardown({ + sessions: this.sessions, + acquisitions: this.acquisitions, + ...(deps.onEvent ? { onEvent: deps.onEvent } : {}), + ...(deps.onBackgroundTasksChanged + ? { onBackgroundTasksChanged: deps.onBackgroundTasksChanged } + : {}), + forgetNotificationRetries: (sessionId) => this.notificationRetries.clear(sessionId, null) + }) this.turnCancellation = new CodexStructuredTurnCancellation({ captureTurnProcesses: deps.captureTurnProcesses, terminateTurnProcesses: deps.terminateTurnProcesses, @@ -92,7 +98,7 @@ export class CodexStructuredSessionAdapter implements StructuredAgentSessionAdap handleUnhandledFrame: (sessionId, kind, payload) => this.handleUnhandledFrame(sessionId, kind, payload), forceCloseUnexpected: (sessionId, fence, acquisitionGeneration, reason) => - this.forceCloseUnexpected(sessionId, fence, acquisitionGeneration, reason) + this.teardown.forceCloseUnexpected(sessionId, fence, acquisitionGeneration, reason) }) /** Buffers pre-publication events and drops events from superseded children. */ @@ -134,12 +140,20 @@ export class CodexStructuredSessionAdapter implements StructuredAgentSessionAdap session: CodexSession, event: CodexStructuredSessionEvent ): CodexJournalTranslationAdmission { + if (event.type === 'notification' && !session.backgroundTasks.canObserve(event)) { + return { accepted: false, reason: 'failed' } + } const admission = session.translator?.handle(event) ?? { accepted: true } if (!admission.accepted) { return admission } if (event.type === 'notification') { this.compactions.codex(event.sessionId, event.method, event.params) + // After the admission check, so a refused frame is observed by the strip + // only on the retry that also reaches the journal. + if (session.backgroundTasks.observe(event)) { + this.deps.onBackgroundTasksChanged?.(event.sessionId, session.backgroundTasks.state) + } } if (event.type === 'ended') { this.compactions.ended(event.sessionId) @@ -167,6 +181,10 @@ export class CodexStructuredSessionAdapter implements StructuredAgentSessionAdap ) } + backgroundTaskState: NonNullable = ( + sessionId + ) => this.sessions.get(sessionId)?.backgroundTasks.state + bindPromptItemId = (sessionId: string, journalItemId: string, promptKey: string): void => this.sessions .get(sessionId) @@ -267,59 +285,12 @@ export class CodexStructuredSessionAdapter implements StructuredAgentSessionAdap identity: AgentSessionJournalIdentity }): Promise => this.sessions.get(input.identity.sessionId)?.historyPath ?? null - closeSession = async (sessionId: string): Promise => { - const closed = await closeCodexSession( - sessionId, - this.sessions, - this.acquisitions, - this.deps.onEvent - ) - if (closed) { - this.notificationRetries.clear(sessionId, null) - } - return closed - } - forceCloseSession = async (sessionId: string): Promise => { - const closed = await closeCodexPublishedSession(this.sessions, sessionId, this.deps.onEvent, { - allowFailedSettlement: true, - requestedClose: false - }) - if (closed) { - this.notificationRetries.clear(sessionId, null) - } - return closed - } - - private forceCloseUnexpected( - sessionId: string, - fence: number, - acquisitionGeneration: string, - reason: Error - ): Promise { - const session = this.sessions.get(sessionId) - if ( - !session || - session.ended || - session.fence !== fence || - session.acquisitionGeneration !== acquisitionGeneration - ) { - return Promise.resolve(false) - } - return closeCodexPublishedSession(this.sessions, sessionId, this.deps.onEvent, { - allowFailedSettlement: true, - requestedClose: false, - expectedFence: fence, - expectedAcquisitionGeneration: acquisitionGeneration, - unexpectedReason: reason - }) - } - disposeSession = (sessionId: string): Promise => this.closeSession(sessionId) - closeAll = (): Promise => - closeAllCodexSessions(this.sessions, this.acquisitions, (sessionId) => - this.disposeSession(sessionId) - ) + closeSession = (sessionId: string): Promise => this.teardown.close(sessionId) + forceCloseSession = (sessionId: string): Promise => this.teardown.forceClose(sessionId) + disposeSession = (sessionId: string): Promise => this.teardown.close(sessionId) + closeAll = (): Promise => this.teardown.closeAll() releaseAcquisition = (input: { sessionId: string }): Promise => - this.closeSession(input.sessionId) + this.teardown.close(input.sessionId) private session(sessionId: string): CodexSession { return requireLiveCodexSession(this.sessions, sessionId) diff --git a/src/main/codex/codex-structured-session-background-tasks.test.ts b/src/main/codex/codex-structured-session-background-tasks.test.ts new file mode 100644 index 00000000000..aa6156027e8 --- /dev/null +++ b/src/main/codex/codex-structured-session-background-tasks.test.ts @@ -0,0 +1,258 @@ +import { describe, expect, it, vi } from 'vitest' +import type { AgentSessionJournalIdentity } from '../../shared/agent-session-journal-types' +import type { AgentSessionBackgroundTaskState } from '../../shared/agent-session-wire' +import type { + CodexAppServerConnection, + CodexAppServerConnectionHandlers, + openCodexAppServerConnection +} from './codex-app-server-connection' +import { CodexStructuredSessionAdapter } from './codex-structured-session-adapter' +import { CodexBackgroundTaskTracker } from './codex-background-task-tracker' +import type { CodexStructuredSessionEvent } from './codex-structured-session-state' +import type { StructuredAgentSessionEventSink } from '../native-chat/agent-session-wire/structured-agent-session-event-sink' + +// Proves the strip is actually REACHED from provider traffic: the tracker is +// unit-tested separately, and a producer that is correct but unwired publishes +// nothing while every one of its own tests stays green. + +const THREAD_ID = '01a07d54-3785-71d0-b065-82c8ebbc572a' +const PARENT_TURN = '01a07d54-37be-72e1-8206-8f0c23dd2cef' +const CHILD_ID = '01a07d54-5523-78a3-91f5-e0acb1dab065' + +/** A three-route stand-in, deliberately smaller than the full adapter harness: + * this suite only needs a thread and a notification pipe. */ +function fakeCodex(close: () => Promise = async () => true): { + handlers: () => CodexAppServerConnectionHandlers + openConnection: typeof openCodexAppServerConnection +} { + let live: CodexAppServerConnectionHandlers = {} + const openConnection = (async (_launch, handlers = {}) => { + live = handlers + const connection: CodexAppServerConnection = { + pid: 4321, + closed: false, + request: async (method) => + method === 'thread/start' ? { thread: { id: THREAD_ID, path: null } } : {}, + notify: () => {}, + respond: () => {}, + respondWithError: () => {}, + close + } as unknown as CodexAppServerConnection + return connection + }) as typeof openCodexAppServerConnection + return { handlers: () => live, openConnection } +} + +function identity(sessionId: string): AgentSessionJournalIdentity { + return { + sessionId, + workspaceId: 'ws-1', + hostId: 'host-1', + agent: 'codex', + providerHandle: { kind: 'codex', threadId: THREAD_ID } + } +} + +function subagentNotification(kind: string): { method: string; params: unknown } { + return { + method: 'item/started', + params: { + item: { + type: 'subAgentActivity', + id: 'call_1', + kind, + agentThreadId: CHILD_ID, + agentPath: '/root/count_a' + }, + threadId: THREAD_ID, + turnId: PARENT_TURN + } + } +} + +const TURN_COMPLETED = { + method: 'turn/completed', + params: { threadId: THREAD_ID, turn: { id: PARENT_TURN, status: 'completed' } } +} + +async function adapterWithSession( + published: { sessionId: string; state: AgentSessionBackgroundTaskState | null }[], + events?: StructuredAgentSessionEventSink, + onEvent?: (event: CodexStructuredSessionEvent) => void, + close?: () => Promise +): Promise<{ adapter: CodexStructuredSessionAdapter; codex: ReturnType }> { + const codex = fakeCodex(close) + const adapter = new CodexStructuredSessionAdapter({ + resolveLaunch: async () => ({ + command: 'codex', + args: ['app-server'], + cwd: '/work/repo', + codexHome: null, + resumeThreadId: null + }), + openConnection: codex.openConnection, + readProcessStartTime: async () => 1_700_000_000_000, + onEvent, + onBackgroundTasksChanged: (sessionId, state) => published.push({ sessionId, state }) + }) + await adapter.acquire({ + identity: identity('session-1'), + fence: 7, + spawnToken: 'spawn-9', + events + }) + codex.handlers().onNotification?.('turn/started', { + threadId: THREAD_ID, + turn: { id: PARENT_TURN, status: 'inProgress' } + }) + codex.handlers().onNotification?.('turn/started', { + threadId: CHILD_ID, + turn: { id: 'child-turn', status: 'inProgress' } + }) + return { adapter, codex } +} + +describe('codex background tasks reach the strip', () => { + it('clears natural-exit state before lifecycle observers can read it', async () => { + const published: { sessionId: string; state: AgentSessionBackgroundTaskState | null }[] = [] + const onEvent = vi.fn() + const { adapter, codex } = await adapterWithSession(published, undefined, onEvent) + const spawn = subagentNotification('started') + codex.handlers().onNotification?.(spawn.method, spawn.params) + codex.handlers().onNotification?.(TURN_COMPLETED.method, TURN_COMPLETED.params) + expect(adapter.backgroundTaskState('session-1')?.tasks).toHaveLength(1) + published.length = 0 + onEvent.mockImplementation((event: CodexStructuredSessionEvent) => { + if (event.type === 'ended') { + expect(adapter.backgroundTaskState('session-1')).toBeNull() + } + }) + codex.handlers().onExit?.(new Error('provider exited')) + expect(adapter.backgroundTaskState('session-1')).toBeNull() + expect(published).toEqual([{ sessionId: 'session-1', state: null }]) + await adapter.closeSession('session-1') + }) + + it('keeps live tasks when close is refused', async () => { + const published: { sessionId: string; state: AgentSessionBackgroundTaskState | null }[] = [] + const close = vi.fn(async () => false) + const { adapter, codex } = await adapterWithSession(published, undefined, undefined, close) + const spawn = subagentNotification('started') + codex.handlers().onNotification?.(spawn.method, spawn.params) + codex.handlers().onNotification?.(TURN_COMPLETED.method, TURN_COMPLETED.params) + const before = adapter.backgroundTaskState('session-1') + published.length = 0 + expect(await adapter.closeSession('session-1')).toBe(false) + expect(adapter.backgroundTaskState('session-1')).toEqual(before) + expect(published).toEqual([]) + close.mockResolvedValue(true) + await adapter.closeSession('session-1') + }) + + it('does not let an old exit callback clear a replacement roster', async () => { + const published: { sessionId: string; state: AgentSessionBackgroundTaskState | null }[] = [] + const { adapter, codex } = await adapterWithSession(published) + const oldExit = codex.handlers().onExit + await adapter.acquire({ identity: identity('session-1'), fence: 8, spawnToken: 'spawn-10' }) + codex.handlers().onNotification?.('turn/started', { + threadId: CHILD_ID, + turn: { id: 'replacement-child-turn' } + }) + const spawn = subagentNotification('started') + codex.handlers().onNotification?.(spawn.method, spawn.params) + const before = adapter.backgroundTaskState('session-1') + expect(before?.tasks).toHaveLength(1) + published.length = 0 + oldExit?.(new Error('old provider exited late')) + expect(adapter.backgroundTaskState('session-1')).toEqual(before) + expect(published).toEqual([]) + await adapter.closeSession('session-1') + }) + + it('recovers the exact provider generation when command metadata cannot be admitted', async () => { + const published: { sessionId: string; state: AgentSessionBackgroundTaskState | null }[] = [] + const observed: CodexStructuredSessionEvent[] = [] + const appendItem = vi.fn() + const { adapter, codex } = await adapterWithSession( + published, + { appendItem, appendTombstone: () => {}, publish: () => {} }, + (event) => observed.push(event) + ) + appendItem.mockClear() + observed.length = 0 + const admission = vi + .spyOn(CodexBackgroundTaskTracker.prototype, 'canObserve') + .mockReturnValue(false) + try { + codex.handlers().onNotification?.('item/started', { + threadId: THREAD_ID, + turnId: PARENT_TURN, + item: { + type: 'commandExecution', + id: 'over-budget', + command: 'sleep 1', + source: 'unifiedExecStartup', + status: 'inProgress' + } + }) + await vi.waitFor(() => expect(adapter.backgroundTaskState('session-1')).toBeUndefined()) + expect(appendItem.mock.calls.map((call) => call[1])).toEqual([ + { kind: 'status', text: 'Provider exited: notification admission failed (failed)' } + ]) + expect(observed).toEqual([ + expect.objectContaining({ + type: 'ended', + cause: 'unexpected-exit', + fence: 7, + acquisitionGeneration: expect.any(String), + reason: 'notification admission failed (failed)' + }) + ]) + expect(published).toEqual([{ sessionId: 'session-1', state: null }]) + } finally { + admission.mockRestore() + await adapter.closeSession('session-1') + } + }) + + it('publishes the orphaned fan-out once the spawning turn completes', async () => { + const published: { sessionId: string; state: AgentSessionBackgroundTaskState | null }[] = [] + const { adapter, codex } = await adapterWithSession(published) + + const spawn = subagentNotification('started') + codex.handlers().onNotification?.(spawn.method, spawn.params) + // The child is still inside the turn, so the strip stays silent. + expect(published).toEqual([]) + expect(adapter.backgroundTaskState('session-1')).toBeNull() + + codex.handlers().onNotification?.(TURN_COMPLETED.method, TURN_COMPLETED.params) + + expect(published).toEqual([ + { + sessionId: 'session-1', + state: { + state: 'monitoring', + supportsStopAll: false, + tasks: [{ id: `codex-agent:${CHILD_ID}`, kind: 'agent', description: 'count_a' }] + } + } + ]) + expect(adapter.backgroundTaskState('session-1')).toEqual(published[0].state) + }) + + it('clears the strip when the session closes', async () => { + const published: { sessionId: string; state: AgentSessionBackgroundTaskState | null }[] = [] + const { adapter, codex } = await adapterWithSession(published) + const spawn = subagentNotification('started') + codex.handlers().onNotification?.(spawn.method, spawn.params) + codex.handlers().onNotification?.(TURN_COMPLETED.method, TURN_COMPLETED.params) + published.length = 0 + + expect(await adapter.closeSession('session-1')).toBe(true) + + // Explicit null, not silence: the reader answers `undefined` once the + // session is gone, which every channel treats as "unchanged". + expect(published).toEqual([{ sessionId: 'session-1', state: null }]) + expect(adapter.backgroundTaskState('session-1')).toBeUndefined() + }) +}) diff --git a/src/main/codex/codex-structured-session-close.test.ts b/src/main/codex/codex-structured-session-close.test.ts index b04e7bc2540..45bfbbf45a1 100644 --- a/src/main/codex/codex-structured-session-close.test.ts +++ b/src/main/codex/codex-structured-session-close.test.ts @@ -10,6 +10,7 @@ import { type CodexStructuredSessionEvent } from './codex-structured-session-adapter' import { handleCodexSessionExit } from './codex-structured-session-close' +import { CodexBackgroundTaskTracker } from './codex-background-task-tracker' import type { CodexSession } from './codex-structured-session-state' import type { StructuredAgentSessionAdapter } from '../native-chat/agent-session-wire/structured-agent-session-adapter' import { StructuredAgentSessionAdapterRouter } from '../native-chat/agent-session-wire/structured-agent-session-adapter-router' @@ -90,6 +91,7 @@ describe('Codex structured session close lifecycle', () => { } as unknown as NonNullable const session = { connection, + backgroundTasks: new CodexBackgroundTaskTracker('thread-1'), ended: false, requestedClose: false, fence: 7, diff --git a/src/main/codex/codex-structured-session-close.ts b/src/main/codex/codex-structured-session-close.ts index db723096177..5b29dc6c056 100644 --- a/src/main/codex/codex-structured-session-close.ts +++ b/src/main/codex/codex-structured-session-close.ts @@ -4,6 +4,7 @@ import { cancelCodexAcquisitionAttempt, type CodexAcquisitionRegistry, type CodexSession, + type CodexStructuredSessionAdapterDeps, type CodexStructuredSessionEvent } from './codex-structured-session-state' import type { StructuredAgentSessionLifecycleEvent } from '../native-chat/agent-session-wire/structured-agent-session-adapter' @@ -16,6 +17,7 @@ export function handleCodexSessionExit(input: { prompts?: CodexSession['prompts'] allowFailedSettlement?: boolean onEvent?: (event: CodexStructuredSessionEvent) => void + onBackgroundTasksChanged?: CodexStructuredSessionAdapterDeps['onBackgroundTasksChanged'] }): boolean { const session = input.sessions.get(input.sessionId) if (!session || session.connection !== input.connection || session.ended) { @@ -43,6 +45,8 @@ export function handleCodexSessionExit(input: { event.settlementRetryRequired = true } session.ended = true + session.backgroundTasks.clear() + input.onBackgroundTasksChanged?.(input.sessionId, null) session.unbindReadingControl?.() input.onEvent?.(event) session.prompts.clear() diff --git a/src/main/codex/codex-structured-session-options.test.ts b/src/main/codex/codex-structured-session-options.test.ts index 1f28ff5e197..b081e52dd6a 100644 --- a/src/main/codex/codex-structured-session-options.test.ts +++ b/src/main/codex/codex-structured-session-options.test.ts @@ -7,6 +7,7 @@ import { reportedCodexThreadOptions, restoredCodexSessionOptions } from './codex-structured-session-options' +import { CodexBackgroundTaskTracker } from './codex-background-task-tracker' import type { CodexSession } from './codex-structured-session-state' function optionSession(request: CodexAppServerConnection['request']): CodexSession { @@ -20,6 +21,7 @@ function optionSession(request: CodexAppServerConnection['request']): CodexSessi respondWithError: () => {}, close: async () => true }, + backgroundTasks: new CodexBackgroundTaskTracker('thread-1'), ended: false, requestedClose: false, fence: 1, diff --git a/src/main/codex/codex-structured-session-state.ts b/src/main/codex/codex-structured-session-state.ts index 12ba28d712f..d004b0006e6 100644 --- a/src/main/codex/codex-structured-session-state.ts +++ b/src/main/codex/codex-structured-session-state.ts @@ -6,6 +6,8 @@ import type { openCodexAppServerConnection } from './codex-app-server-connection' import { CodexAcquisitionWindow } from './codex-structured-acquisition-window' +import type { AgentSessionBackgroundTaskState } from '../../shared/agent-session-wire' +import type { CodexBackgroundTaskTracker } from './codex-background-task-tracker' import type { CodexJournalTranslator } from './codex-structured-journal-translation' import type { CodexTurnProcessSnapshot } from './codex-structured-turn-processes' import type { StructuredAgentSessionLifecycleEvent } from '../native-chat/agent-session-wire/structured-agent-session-adapter' @@ -44,6 +46,10 @@ export type CodexStructuredSessionAdapterDeps = { /** Host capability seam; production uses the native Windows process table. */ isWindowsProcessStartTimeAvailable?: () => boolean onEvent?: (event: CodexStructuredSessionEvent) => void + onBackgroundTasksChanged?: ( + sessionId: string, + state: AgentSessionBackgroundTaskState | null + ) => void openConnection?: typeof openCodexAppServerConnection readProcessStartTime?: (pid: number) => Promise mintLinkId?: () => string @@ -73,6 +79,8 @@ export type CodexSession = { reportedOptions: { model?: string; effort?: string } turnIdWaiters: ((turnId: string) => void)[] translator: CodexJournalTranslator | null + /** Ephemeral roster behind the background-tasks strip; never durable state. */ + backgroundTasks: CodexBackgroundTaskTracker unbindReadingControl?: () => void /** Terminates this exact child as an unexpected death and enters host recovery. */ forceCloseUnexpected?: (reason: Error) => Promise diff --git a/src/main/codex/codex-structured-session-teardown.ts b/src/main/codex/codex-structured-session-teardown.ts new file mode 100644 index 00000000000..dcd38eb82fe --- /dev/null +++ b/src/main/codex/codex-structured-session-teardown.ts @@ -0,0 +1,94 @@ +// Stopping one Codex app-server child, in the four ways the host asks for it. +// +// Every path funnels through `settled` so the ephemeral surfaces a closed +// session owns are cleared exactly once, and only when the child was actually +// proven stopped — a refused close leaves the session indexed for a retry. + +import type { AgentSessionBackgroundTaskState } from '../../shared/agent-session-wire' +import { + closeAllCodexSessions, + closeCodexPublishedSession, + closeCodexSession +} from './codex-structured-session-close' +import type { + CodexAcquisitionRegistry, + CodexSession, + CodexStructuredSessionEvent +} from './codex-structured-session-state' + +export type CodexStructuredSessionTeardownDeps = { + sessions: Map + acquisitions: CodexAcquisitionRegistry + onEvent?: (event: CodexStructuredSessionEvent) => void + onBackgroundTasksChanged?: ( + sessionId: string, + state: AgentSessionBackgroundTaskState | null + ) => void + forgetNotificationRetries: (sessionId: string) => void +} + +export class CodexStructuredSessionTeardown { + constructor(private readonly deps: CodexStructuredSessionTeardownDeps) {} + + close = async (sessionId: string): Promise => { + const closed = await closeCodexSession( + sessionId, + this.deps.sessions, + this.deps.acquisitions, + this.deps.onEvent + ) + return this.settled(sessionId, closed) + } + + forceClose = async (sessionId: string): Promise => { + const closed = await closeCodexPublishedSession( + this.deps.sessions, + sessionId, + this.deps.onEvent, + { allowFailedSettlement: true, requestedClose: false } + ) + return this.settled(sessionId, closed) + } + + /** Terminates this exact child as an unexpected death. Every ownership check + * stays here so a stale caller cannot close a replacement child. */ + forceCloseUnexpected = ( + sessionId: string, + fence: number, + acquisitionGeneration: string, + reason: Error + ): Promise => { + const session = this.deps.sessions.get(sessionId) + if ( + !session || + session.ended || + session.fence !== fence || + session.acquisitionGeneration !== acquisitionGeneration + ) { + return Promise.resolve(false) + } + return closeCodexPublishedSession(this.deps.sessions, sessionId, this.deps.onEvent, { + allowFailedSettlement: true, + requestedClose: false, + expectedFence: fence, + expectedAcquisitionGeneration: acquisitionGeneration, + unexpectedReason: reason + }).then((closed) => this.settled(sessionId, closed)) + } + + closeAll = (): Promise => + closeAllCodexSessions(this.deps.sessions, this.deps.acquisitions, (sessionId) => + this.close(sessionId) + ) + + private settled(sessionId: string, closed: boolean): boolean { + if (closed) { + this.deps.forgetNotificationRetries(sessionId) + // Explicit null, not silence: the state reader answers `undefined` once + // the session leaves the map, which every channel reads as "unchanged" + // and would leave the last roster on screen. + this.deps.onBackgroundTasksChanged?.(sessionId, null) + } + return closed + } +} diff --git a/src/main/codex/codex-subagent-activity.ts b/src/main/codex/codex-subagent-activity.ts index f12e9dfb1b3..9761f8634f8 100644 --- a/src/main/codex/codex-subagent-activity.ts +++ b/src/main/codex/codex-subagent-activity.ts @@ -7,11 +7,10 @@ // segment is a semantic task name and the only label available. There is no // `thread/started` for a child, so nickname/role/depth do not exist. // * `agentsStates` on `collabAgentToolCall` arrived empty (`{}`) throughout the -// probe, so nothing here reads it — state comes from `kind` alone. +// probe, so nothing here reads it; child turn events own execution state. // * `thread/tokenUsage/updated` reports a per-thread RUNNING TOTAL, so the // latest frame replaces the previous one — it is never accumulated. -import type { NativeChatSubagentState } from '../../shared/native-chat-types' import type { CodexThreadItem } from './codex-structured-item-translation' export const CODEX_SUBAGENT_ITEM_TYPE = 'subAgentActivity' @@ -48,24 +47,6 @@ export function readCodexSubagentActivity(item: CodexThreadItem): CodexSubagentA } } -/** - * The state a `kind` implies for the child it names. - * - * An unrecognized kind means "this child exists and reported something we - * cannot classify" — `working`, which the session sweep will later settle to - * `unverifiable` if nothing better ever arrives. Claiming a terminal state from - * an unknown kind would assert an outcome the wire never gave us. - */ -export function codexSubagentStateForKind(kind: string): NativeChatSubagentState { - if (kind === 'completed') { - return 'completed' - } - if (kind === 'interrupted') { - return 'stopped' - } - return 'working' -} - /** Path segments, empty ones dropped: `/root/list_directory` → 2 segments. */ export function codexSubagentPathSegments(agentPath: string | null): string[] { return agentPath === null ? [] : agentPath.split('/').filter((part) => part.length > 0) diff --git a/src/main/codex/codex-subagent-execution-projection.test.ts b/src/main/codex/codex-subagent-execution-projection.test.ts new file mode 100644 index 00000000000..21d39f4f195 --- /dev/null +++ b/src/main/codex/codex-subagent-execution-projection.test.ts @@ -0,0 +1,211 @@ +import { describe, expect, it } from 'vitest' +import type { AgentJournalItemBody } from '../../shared/agent-session-journal-types' +import type { CodexStructuredSessionEvent } from './codex-structured-session-adapter' +import { createCodexJournalTranslator } from './codex-structured-journal-translation' +import { CodexBackgroundTaskTracker } from './codex-background-task-tracker' +import { CodexSubagentExecutions } from './codex-subagent-executions' + +const PRIMARY = 'primary' +const CHILD = 'child' + +function turn( + method: 'turn/started' | 'turn/completed', + threadId: string, + turnId: string, + status = 'completed' +): CodexStructuredSessionEvent { + return { + type: 'notification', + sessionId: 'session', + method, + threadId, + params: { threadId, turn: { id: turnId, status } } + } +} + +function activity(parentTurn: string, kind = 'started'): CodexStructuredSessionEvent { + return { + type: 'notification', + sessionId: 'session', + threadId: PRIMARY, + method: 'item/started', + params: { + threadId: PRIMARY, + turnId: parentTurn, + item: { + type: 'subAgentActivity', + id: `activity-${parentTurn}-${kind}`, + kind, + agentThreadId: CHILD, + agentPath: '/root/task' + } + } + } +} + +function harness() { + const executions = new CodexSubagentExecutions() + const tracker = new CodexBackgroundTaskTracker(PRIMARY, executions) + const rows = new Map() + let refused = false + const translator = createCodexJournalTranslator({ + primaryThreadId: () => PRIMARY, + subagentExecutions: executions, + sink: { + appendItem: (identity, body) => rows.set(JSON.stringify(identity), body), + appendTombstone: () => {}, + publish: () => {}, + tryAppendItem: (identity, body) => { + if (refused) { + return { accepted: false, reason: 'backpressure' } + } + rows.set(JSON.stringify(identity), body) + return { accepted: true } + } + }, + schedule: (run) => { + run() + return () => {} + } + }) + function send(event: CodexStructuredSessionEvent) { + const admission = translator.handle(event) + if (admission.accepted && event.type === 'notification') { + tracker.observe(event) + } + return admission + } + function state(parentTurn: string): string | undefined { + for (const body of rows.values()) { + if (body.kind !== 'message') { + continue + } + for (const block of body.blocks) { + if (block.type === 'subagent-group' && block.groupId === `${PRIMARY}:${parentTurn}`) { + return block.agents[0]?.state + } + } + } + return undefined + } + return { + send, + tracker, + state, + refuse: (value: boolean) => { + refused = value + }, + dispose: () => translator.dispose() + } +} + +function firstRun(h: ReturnType) { + h.send(turn('turn/started', PRIMARY, 'parent-1')) + h.send(turn('turn/started', CHILD, 'child-1')) + h.send(activity('parent-1')) + h.send(turn('turn/completed', CHILD, 'child-1')) +} + +describe('shared child execution projection', () => { + it('creates no execution record from activity without an owner turn', () => { + const h = harness() + h.send(activity('parent-1')) + h.send(activity('parent-2', 'interacted')) + expect(h.state('parent-1')).toBeUndefined() + expect(h.state('parent-2')).toBeUndefined() + expect(h.tracker.state).toBeNull() + h.dispose() + }) + + it.each(['parent-1', 'parent-2'])( + 'reopens a child for real follow-up in %s and fences old execution events', + (parent) => { + const h = harness() + firstRun(h) + if (parent === 'parent-2') { + h.send(turn('turn/completed', PRIMARY, 'parent-1')) + h.send(turn('turn/started', PRIMARY, parent)) + } + h.send(activity(parent, 'interacted')) + expect(h.tracker.state).toBeNull() + h.send(turn('turn/started', CHILD, 'child-2')) + h.send(turn('turn/completed', PRIMARY, parent)) + expect(h.state(parent)).toBe('working') + expect(h.tracker.state?.tasks).toHaveLength(1) + h.send(turn('turn/completed', CHILD, 'child-1')) + h.send(turn('turn/started', CHILD, 'child-1')) + h.send(activity('parent-1', 'completed')) + expect(h.state(parent)).toBe('working') + expect(h.tracker.state?.tasks).toHaveLength(1) + if (parent === 'parent-2') { + expect(h.state('parent-1')).toBe('completed') + } + h.send(turn('turn/completed', CHILD, 'child-2')) + expect(h.state(parent)).toBe('completed') + expect(h.tracker.state).toBeNull() + h.dispose() + } + ) + + it('keeps an idle message from creating execution on either surface', () => { + const h = harness() + firstRun(h) + h.send(turn('turn/completed', PRIMARY, 'parent-1')) + h.send(activity('parent-2', 'interacted')) + h.send(turn('turn/completed', PRIMARY, 'parent-2')) + expect(h.state('parent-1')).toBe('completed') + expect(h.state('parent-2')).toBeUndefined() + expect(h.tracker.state).toBeNull() + h.dispose() + }) + + it('keeps a late completion activity from retargeting a queued follow-up', () => { + const h = harness() + firstRun(h) + h.send(turn('turn/completed', PRIMARY, 'parent-1')) + h.send(activity('parent-2', 'interacted')) + h.send(activity('parent-1', 'completed')) + h.send(turn('turn/started', CHILD, 'child-2')) + expect(h.state('parent-1')).toBe('completed') + expect(h.state('parent-2')).toBe('working') + expect(h.tracker.state?.tasks).toHaveLength(1) + h.dispose() + }) + + it('keeps a message to a running child in the original execution group', () => { + const h = harness() + h.send(turn('turn/started', PRIMARY, 'parent-1')) + h.send(turn('turn/started', CHILD, 'child-1')) + h.send(activity('parent-1')) + h.send(turn('turn/completed', PRIMARY, 'parent-1')) + h.send(turn('turn/started', PRIMARY, 'parent-2')) + h.send(activity('parent-2', 'interacted')) + h.send(turn('turn/started', CHILD, 'child-1')) + h.send(turn('turn/completed', PRIMARY, 'parent-2')) + expect(h.tracker.state?.tasks).toHaveLength(1) + expect(h.state('parent-2')).toBeUndefined() + h.send(turn('turn/completed', CHILD, 'child-1')) + expect(h.state('parent-1')).toBe('completed') + expect(h.state('parent-2')).toBeUndefined() + expect(h.tracker.state).toBeNull() + h.dispose() + }) + + it('does not expose pending child settlement before journal admission and retries the same fact', () => { + const h = harness() + h.send(turn('turn/started', PRIMARY, 'parent-1')) + h.send(activity('parent-1')) + h.send(turn('turn/started', CHILD, 'child-1')) + h.send(turn('turn/completed', PRIMARY, 'parent-1')) + const complete = turn('turn/completed', CHILD, 'child-1') + h.refuse(true) + expect(h.send(complete)).toEqual({ accepted: false, reason: 'backpressure' }) + expect(h.tracker.state?.tasks).toHaveLength(1) + expect(h.state('parent-1')).toBe('working') + h.refuse(false) + expect(h.send(complete)).toEqual({ accepted: true }) + expect(h.state('parent-1')).toBe('completed') + expect(h.tracker.state).toBeNull() + h.dispose() + }) +}) diff --git a/src/main/codex/codex-subagent-executions.test.ts b/src/main/codex/codex-subagent-executions.test.ts new file mode 100644 index 00000000000..aceff7ab465 --- /dev/null +++ b/src/main/codex/codex-subagent-executions.test.ts @@ -0,0 +1,51 @@ +import { describe, expect, it } from 'vitest' +import { CodexSubagentExecutions } from './codex-subagent-executions' + +describe('CodexSubagentExecutions retention and identity', () => { + it('bounds settled history through repeated execution without evicting live children', () => { + const executions = new CodexSubagentExecutions() + executions.register('long-lived', 'long-lived', 'parent') + executions.observeTurn('long-lived', 'long-lived-turn', 'working') + for (let index = 0; index < 1_000; index++) { + const id = `child-${index}` + executions.observeTurn(id, id, 'working') + executions.register(id, id, 'parent') + executions.observeTurn(id, id, 'completed') + } + expect(executions.workingChildren().map((child) => child.agentThreadId)).toEqual(['long-lived']) + expect(Reflect.get(executions, 'children').size).toBeLessThanOrEqual(128) + expect(Reflect.get(executions, 'settledTurns').size).toBeLessThanOrEqual(256) + }) + + it('retains early live owner events at capacity and makes room only after settlement', () => { + const executions = new CodexSubagentExecutions() + for (let index = 0; index < 128; index++) { + executions.observeTurn(`child-${index}`, `turn-${index}`, 'working') + } + expect(executions.observeTurn('overflow', 'overflow', 'working')).toBeNull() + for (let index = 0; index < 128; index++) { + executions.register(`child-${index}`, `child-${index}`, 'parent') + } + expect(executions.workingChildren()).toHaveLength(128) + executions.observeTurn('child-0', 'turn-0', 'completed') + expect(executions.observeTurn('overflow', 'overflow', 'working')).not.toBeNull() + executions.register('overflow', 'overflow', 'parent') + expect(executions.workingChildren()).toHaveLength(128) + }) + + it('corrects an unverifiable execution with its own terminal event and ignores stale starts', () => { + const executions = new CodexSubagentExecutions() + executions.register('child', 'child', 'parent') + executions.observeTurn('child', 'turn', 'working') + executions.settleSession() + expect(executions.workingChildren()).toEqual([]) + expect(executions.observeTurn('child', 'turn', 'working')).toBeNull() + expect(executions.observeTurn('child', 'turn', 'completed')?.execution.state).toBe('completed') + executions.observeTurn('child', 'new-turn', 'working') + executions.observeTurn('child', 'turn', 'failed') + expect(executions.workingChildren()[0]?.execution?.turnId).toBe('new-turn') + executions.clear() + expect(Reflect.get(executions, 'children').size).toBe(0) + expect(Reflect.get(executions, 'settledTurns').size).toBe(0) + }) +}) diff --git a/src/main/codex/codex-subagent-executions.ts b/src/main/codex/codex-subagent-executions.ts new file mode 100644 index 00000000000..cd33b4eb1d5 --- /dev/null +++ b/src/main/codex/codex-subagent-executions.ts @@ -0,0 +1,144 @@ +import type { NativeChatSubagentState } from '../../shared/native-chat-types' +import { MAX_SUBAGENT_FIELD_CHARS } from '../../shared/native-chat-subagent-summary' + +const MAX_CHILDREN = 128 +const MAX_SETTLED_TURNS = 256 + +export type CodexChildExecution = { + turnId: string + state: NativeChatSubagentState +} + +export type CodexExecutionChild = { + agentThreadId: string + registered: boolean + label: string | null + parentTurnId: string | null + execution: CodexChildExecution | null +} + +/** Child turn events own execution; activity items only identify the child. */ +export class CodexSubagentExecutions { + private readonly children = new Map() + private readonly settledTurns = new Map() + + register( + agentThreadId: string, + label: string | null, + parentTurnId: string | null | undefined + ): CodexExecutionChild | undefined { + const child = this.child(agentThreadId) + if (!child) { + return undefined + } + if (!child.registered || parentTurnId !== undefined) { + child.parentTurnId = parentTurnId ?? null + } + child.registered = true + // Retain one overflow unit so the journal can append its per-row truncation marker. + child.label ??= + label + ?.trim() + .replace(/\s+/g, ' ') + .slice(0, MAX_SUBAGENT_FIELD_CHARS + 1) || null + return child + } + + observeTurn( + agentThreadId: string, + turnId: string, + state: NativeChatSubagentState + ): { child: CodexExecutionChild; execution: CodexChildExecution } | null { + const key = JSON.stringify([agentThreadId, turnId]) + const settled = this.settledTurns.get(key) + if (state === 'working' && settled !== undefined) { + return null + } + const child = this.child(agentThreadId) + if (!child) { + return null + } + if ( + state === 'working' && + child.execution?.turnId === turnId && + child.execution.state !== 'working' + ) { + return null + } + const execution = { turnId, state: settled ?? state } + if (state !== 'working') { + this.settledTurns.set(key, execution.state) + while (this.settledTurns.size > MAX_SETTLED_TURNS) { + const oldest = this.settledTurns.keys().next().value + if (oldest === undefined) { + break + } + this.settledTurns.delete(oldest) + } + } + if (state === 'working' || !child.execution || child.execution.turnId === turnId) { + child.execution = execution + } + return { child, execution } + } + + /** Survives the child's turn, so a row outliving that turn can still name it. */ + label(agentThreadId: string): string | null { + return this.children.get(agentThreadId)?.label ?? null + } + + workingChildren(): CodexExecutionChild[] { + return [...this.children.values()].filter( + (child) => child.registered && child.execution?.state === 'working' + ) + } + + settleSession(): void { + for (const child of this.children.values()) { + if (child.execution?.state === 'working') { + child.execution = { ...child.execution, state: 'unverifiable' } + } + } + } + + clear(): void { + this.children.clear() + this.settledTurns.clear() + } + + private child(agentThreadId: string): CodexExecutionChild | undefined { + const existing = this.children.get(agentThreadId) + if (existing) { + return existing + } + if (this.children.size >= MAX_CHILDREN) { + const settled = [...this.children].find(([, child]) => child.execution?.state !== 'working') + if (!settled) { + return undefined + } + this.children.delete(settled[0]) + } + const child: CodexExecutionChild = { + agentThreadId, + registered: false, + label: null, + parentTurnId: null, + execution: null + } + this.children.set(agentThreadId, child) + return child + } +} + +export function codexChildTurnState(status: unknown): NativeChatSubagentState { + if (status === 'completed') { + return 'completed' + } + if (status === 'interrupted') { + return 'stopped' + } + if (status === 'failed') { + return 'failed' + } + return 'unverifiable' +} diff --git a/src/main/codex/codex-subagent-group-body.ts b/src/main/codex/codex-subagent-group-body.ts new file mode 100644 index 00000000000..c2157b0e7eb --- /dev/null +++ b/src/main/codex/codex-subagent-group-body.ts @@ -0,0 +1,51 @@ +import type { AgentJournalItemBody } from '../../shared/agent-session-journal-types' +import type { NativeChatSubagentEntry } from '../../shared/native-chat-types' +import { + MAX_SUBAGENT_FIELD_CHARS, + subagentGroupFallbackText +} from '../../shared/native-chat-subagent-summary' + +/** The roster row: the structured block plus the plain sentence an older client + * renders in its place. A message whose only block is the new variant would + * reach such a client with nothing it can draw. */ +export function codexSubagentGroupBody( + groupId: string, + agents: readonly NativeChatSubagentEntry[] +): AgentJournalItemBody { + const bounded = agents.map((agent, index) => ({ + ...agent, + id: boundSubagentField(agent.id, index), + label: boundSubagentField(agent.label, index) + })) + return { + kind: 'message', + role: 'system', + blocks: [ + { type: 'text', text: subagentGroupFallbackText(bounded) }, + { type: 'subagent-group', groupId, agents: bounded } + ] + } +} + +/** `id` and `label` are provider strings, so they take the bound both readers of + * this row already clip them to. A plain length check, not the tool-output + * bound: that one digests the whole value before it checks the length, and this + * runs twice per child on every streamed token-usage frame. + * + * A clip is not identity-preserving, so a clipped value carries the child's + * index: two ids sharing a long prefix collapse to one React key, and + * `claimLabel` writes its ordinal at the very tail the clip removes. The index + * is reserved out of the bound, not appended to it, because both readers + * re-clip to the same cap and would cut a suffix that overflowed it. */ +export function boundSubagentField(value: string, index: number): string { + if (value.length <= MAX_SUBAGENT_FIELD_CHARS) { + return value + } + const suffix = `…~${index}` + const keep = MAX_SUBAGENT_FIELD_CHARS - suffix.length + // Slicing UTF-16 units can split a surrogate pair; a lone surrogate is + // malformed in a durable row and lossy through any non-JSON UTF-8 hop. + const last = value.charCodeAt(keep - 1) + const end = last >= 0xd800 && last <= 0xdbff ? keep - 1 : keep + return `${value.slice(0, end)}${suffix}` +} diff --git a/src/main/codex/codex-subagent-roster.test.ts b/src/main/codex/codex-subagent-roster.test.ts index 2f20c9df9bf..9d3e2c690c4 100644 --- a/src/main/codex/codex-subagent-roster.test.ts +++ b/src/main/codex/codex-subagent-roster.test.ts @@ -83,6 +83,22 @@ function deliver( item: CodexThreadItem, turnId: string | null = TURN ): void { + // The fixture includes the child's owner event separately from its activity metadata. + const state = + item.kind === 'started' + ? 'working' + : item.kind === 'completed' + ? 'completed' + : item.kind === 'interrupted' + ? 'stopped' + : null + if (state && typeof item.agentThreadId === 'string') { + roster.handleTurn({ + threadId: item.agentThreadId, + turnId: `execution:${item.agentThreadId}`, + state + }) + } // Every activity item reaches the wire twice: item/started, then item/completed. roster.handleItem({ threadId: THREAD, turnId, item }) roster.handleItem({ threadId: THREAD, turnId, item }) @@ -270,7 +286,7 @@ describe('CodexSubagentRoster', () => { expect(appended).toHaveLength(1) }) - it('rule 2 — a first event of any kind creates the entry in the state it implies', () => { + it('registers a child after its owner already reported completion', () => { const { roster, agents } = createHarness() deliver( @@ -374,7 +390,7 @@ describe('CodexSubagentRoster', () => { deliver( roster, - activity({ kind: 'interacted', agentThreadId: 'child-1', agentPath: '/root/read' }) + activity({ kind: 'started', agentThreadId: 'child-1', agentPath: '/root/read' }) ) roster.settleSession() const afterFirstSweep = appended.length @@ -676,6 +692,7 @@ describe('CodexSubagentRoster', () => { now: () => 1_000 }) const item = activity({ kind: 'started', agentThreadId: 'child-1', agentPath: '/root/read' }) + roster.handleTurn({ threadId: 'child-1', turnId: 'child-turn', state: 'working' }) expect(roster.handleItem({ threadId: THREAD, turnId: TURN, item })).toEqual(refusal) @@ -724,6 +741,7 @@ describe('CodexSubagentRoster', () => { activeTurn: () => TURN, now: () => 1_000 }) + roster.handleTurn({ threadId: 'child-1', turnId: 'child-turn', state: 'working' }) roster.handleItem({ threadId: THREAD, turnId: TURN, @@ -758,6 +776,7 @@ describe('CodexSubagentRoster', () => { activeTurn: () => TURN }) + roster.handleTurn({ threadId: 'child-1', turnId: 'child-turn', state: 'working' }) expect( roster.handleItem({ threadId: THREAD, diff --git a/src/main/codex/codex-subagent-roster.ts b/src/main/codex/codex-subagent-roster.ts index 257fe705764..a2b58e1a6ce 100644 --- a/src/main/codex/codex-subagent-roster.ts +++ b/src/main/codex/codex-subagent-roster.ts @@ -1,11 +1,6 @@ // The Codex subagent roster: one journal row per spawn group, revised in place. // -// There is no snapshot to read. `agentsStates` arrived empty in the live probe -// and children get no `thread/started`, so the roster is -// accumulated purely from `subAgentActivity` items — each of which arrives TWICE -// (`item/started` and `item/completed`). Every transition here is therefore -// idempotent, and a terminal state latches: duplicate and out-of-order delivery -// must not resurrect a settled child. +// Activity supplies membership; child turn events supply execution state. // // KNOWN LIMITATION: `groups` is process-local and is never seeded from the // journal, while the row's identity is keyed on the group id alone. So once a @@ -18,28 +13,32 @@ // thread, and a real turn id is assumed freshly minted per turn. Seeding from // the journal is the fix. +import type { AgentJournalItemIdentity } from '../../shared/agent-session-journal-types' +import { isTerminalSubagentState } from '../../shared/native-chat-subagent-summary' import type { - AgentJournalItemBody, - AgentJournalItemIdentity -} from '../../shared/agent-session-journal-types' -import { - canReplaceSubagentState, - isTerminalSubagentState, - MAX_SUBAGENT_FIELD_CHARS, - subagentGroupFallbackText -} from '../../shared/native-chat-subagent-summary' -import type { NativeChatSubagentEntry } from '../../shared/native-chat-types' + NativeChatSubagentEntry, + NativeChatSubagentState +} from '../../shared/native-chat-types' import type { StructuredAgentSessionEventSink, StructuredAgentSessionSinkAdmission } from '../native-chat/agent-session-wire/structured-agent-session-event-sink' import { codexSubagentLabel, - codexSubagentStateForKind, isCodexRootAgentActivity, readCodexSubagentActivity, readCodexThreadTokenTotal } from './codex-subagent-activity' +import { + CodexSubagentExecutions, + codexChildTurnState, + type CodexChildExecution, + type CodexExecutionChild +} from './codex-subagent-executions' +import { readRecord } from './codex-item-field-readers' +import { readCodexTurnId } from './codex-structured-thread-facts' +import { codexSubagentGroupBody } from './codex-subagent-group-body' +export { codexSubagentGroupBody } from './codex-subagent-group-body' import type { CodexThreadItem } from './codex-structured-item-translation' import { MAX_CODEX_SUBAGENT_GROUPS, @@ -51,15 +50,12 @@ const ADMITTED: StructuredAgentSessionSinkAdmission = { accepted: true } /** The turn a group belongs to when Codex reports activity outside any turn. * Mirrors the generic-frame bucket name so the two read alike in the journal. */ -const OUTSIDE_TURN = 'outside-turn' - -const UNLABELLED_AGENT = 'subagent' - type RosterGroup = { groupId: string identity: AgentJournalItemIdentity /** Insertion order is the display order; the map holds the state. */ entries: Map + executionTurns: Map /** Times each label has been claimed, so a repeat gets an ordinal suffix. */ labelCounts: Map /** Last body written, so an idempotent replay writes no new revision. */ @@ -70,7 +66,7 @@ type RosterGroup = { * tree rooted at the parent thread, so every child of one turn shares a row * no matter which thread's stream carried its activity item. */ export function codexSubagentGroupId(threadId: string, turnId: string | null): string { - return `${threadId}:${turnId ?? OUTSIDE_TURN}` + return `${threadId}:${turnId ?? 'outside-turn'}` } /** Durable journal identity for the group's row — stable across revisions and @@ -85,6 +81,7 @@ export type CodexSubagentRosterDeps = { primaryThreadId: () => string | null activeTurn: (threadId: string) => string | null now?: () => number + executions?: CodexSubagentExecutions } export class CodexSubagentRoster { @@ -95,9 +92,11 @@ export class CodexSubagentRoster { * the map itself is LRU-capped in `handleTokenUsage`. */ private readonly tokensByThread = new Map() private readonly now: () => number + private readonly executions: CodexSubagentExecutions constructor(private readonly deps: CodexSubagentRosterDeps) { this.now = deps.now ?? (() => Date.now()) + this.executions = deps.executions ?? new CodexSubagentExecutions() } /** Consume a `subAgentActivity` item. Returns null when the item is not one. */ @@ -111,42 +110,81 @@ export class CodexSubagentRoster { return null } // The root node is the parent turn itself, not a child it spawned. - if (isCodexRootAgentActivity(activity)) { + if ( + activity.agentThreadId === this.deps.primaryThreadId() || + isCodexRootAgentActivity(activity) + ) { return ADMITTED } - const group = this.groupFor(input.threadId, input.turnId) - const existing = group.entries.get(activity.agentThreadId) - const state = codexSubagentStateForKind(activity.kind) - if (!existing) { - // Rule: the first event for a child may be ANY kind. An `interacted` or - // `completed` with no prior `started` creates the entry in the state its - // kind implies rather than being dropped for lacking a roster row. - if (group.entries.size >= MAX_CODEX_SUBAGENTS_PER_GROUP) { - return ADMITTED - } - const now = this.now() - group.entries.set(activity.agentThreadId, { - id: activity.agentThreadId, - label: this.claimLabel(group, codexSubagentLabel(activity)), - state, - startedAt: now, - ...(isTerminalSubagentState(state) ? { settledAt: now } : {}) - }) - } else if (canReplaceSubagentState(existing.state, state)) { - // A child's own verdict latches. Re-applying the same non-terminal state - // is a no-op, which is what makes the duplicate `item/started` + - // `item/completed` delivery idempotent. `unverifiable` does not latch: a - // child swept when contact was lost can still report what it actually did - // if contact returns. - group.entries.set(activity.agentThreadId, { - ...existing, - state, - ...(isTerminalSubagentState(state) ? { settledAt: this.now() } : {}) - }) + const child = this.executions.register( + activity.agentThreadId, + codexSubagentLabel(activity), + activity.kind === 'started' || activity.kind === 'interacted' ? input.turnId : undefined + ) + if (!child?.execution) { + return ADMITTED + } + const group = + this.executionGroup(child.agentThreadId, child.execution.turnId) ?? + this.groupFor(input.threadId, input.turnId) + if (!group.entries.has(child.agentThreadId)) { + this.recordExecution(group, child, child.execution) } return this.write(group) } + handleTurnEvent(event: { + method: string + threadId: string + params: unknown + }): StructuredAgentSessionSinkAdmission { + const turnId = readCodexTurnId(event.params) + return turnId + ? this.handleTurn({ + threadId: event.threadId, + turnId, + state: + event.method === 'turn/started' + ? 'working' + : codexChildTurnState(readRecord(readRecord(event.params).turn).status) + }) + : ADMITTED + } + + handleTurn(input: { + threadId: string + turnId: string + state: NativeChatSubagentState + }): StructuredAgentSessionSinkAdmission { + if (input.threadId === this.deps.primaryThreadId()) { + return ADMITTED + } + const observed = this.executions.observeTurn(input.threadId, input.turnId, input.state) + if (!observed || !observed.child.registered) { + return ADMITTED + } + const { child, execution } = observed + if (input.state === 'working') { + const parent = this.deps.primaryThreadId() ?? input.threadId + const group = + this.executionGroup(child.agentThreadId, execution.turnId) ?? + this.groupFor(parent, this.deps.activeTurn(parent) ?? child.parentTurnId) + this.recordExecution(group, child, execution) + return this.write(group) + } + for (const group of this.groups.values()) { + if (group.executionTurns.get(input.threadId) !== input.turnId) { + continue + } + this.recordExecution(group, child, execution) + const admission = this.write(group) + if (!admission.accepted) { + return admission + } + } + return ADMITTED + } + /** Consume `thread/tokenUsage/updated`. Returns null when the params are not one. */ handleTokenUsage(params: unknown): StructuredAgentSessionSinkAdmission | null { const usage = readCodexThreadTokenTotal(params) @@ -187,6 +225,7 @@ export class CodexSubagentRoster { * routinely outlive their turn and keep reporting into the same group. */ settleSession(): StructuredAgentSessionSinkAdmission { + this.executions.settleSession() for (const group of this.groups.values()) { const admission = this.sweep(group) if (!admission.accepted) { @@ -233,6 +272,7 @@ export class CodexSubagentRoster { groupId, identity: codexSubagentGroupIdentity(groupId), entries: new Map(), + executionTurns: new Map(), labelCounts: new Map(), lastSerialized: null } @@ -247,15 +287,46 @@ export class CodexSubagentRoster { return group } + private executionGroup(threadId: string, turnId: string): RosterGroup | undefined { + return [...this.groups.values()].find((group) => group.executionTurns.get(threadId) === turnId) + } + /** Two children can share a trailing path segment; the ordinal keeps their * rows apart without inventing a name the provider never sent. */ private claimLabel(group: RosterGroup, label: string | null): string { - const base = label ?? UNLABELLED_AGENT + const base = label ?? 'subagent' const seen = group.labelCounts.get(base) ?? 0 group.labelCounts.set(base, seen + 1) return seen === 0 ? base : `${base} ${seen + 1}` } + private recordExecution( + group: RosterGroup, + child: CodexExecutionChild, + execution: CodexChildExecution | null + ): void { + const existing = group.entries.get(child.agentThreadId) + if (!existing && group.entries.size >= MAX_CODEX_SUBAGENTS_PER_GROUP) { + return + } + const turnId = execution?.turnId ?? null + const state = execution?.state ?? 'unverifiable' + const sameTurn = existing && group.executionTurns.get(child.agentThreadId) === turnId + if (sameTurn && existing.state === state) { + return + } + const now = this.now() + group.executionTurns.set(child.agentThreadId, turnId) + group.entries.set(child.agentThreadId, { + id: child.agentThreadId, + label: existing?.label ?? this.claimLabel(group, child.label), + state, + startedAt: sameTurn ? existing.startedAt : now, + ...(isTerminalSubagentState(state) ? { settledAt: now } : {}), + ...(existing?.tokens !== undefined ? { tokens: existing.tokens } : {}) + }) + } + private write(group: RosterGroup): StructuredAgentSessionSinkAdmission { const agents = [...group.entries].map(([id, entry]) => { const tokens = this.tokensByThread.get(id) @@ -300,48 +371,3 @@ export class CodexSubagentRoster { return published } } - -/** The roster row: the structured block plus the plain sentence an older client - * renders in its place. A message whose only block is the new variant would - * reach such a client with nothing it can draw. */ -export function codexSubagentGroupBody( - groupId: string, - agents: readonly NativeChatSubagentEntry[] -): AgentJournalItemBody { - const bounded = agents.map((agent, index) => ({ - ...agent, - id: boundSubagentField(agent.id, index), - label: boundSubagentField(agent.label, index) - })) - return { - kind: 'message', - role: 'system', - blocks: [ - { type: 'text', text: subagentGroupFallbackText(bounded) }, - { type: 'subagent-group', groupId, agents: bounded } - ] - } -} - -/** `id` and `label` are provider strings, so they take the bound both readers of - * this row already clip them to. A plain length check, not the tool-output - * bound: that one digests the whole value before it checks the length, and this - * runs twice per child on every streamed token-usage frame. - * - * A clip is not identity-preserving, so a clipped value carries the child's - * index: two ids sharing a long prefix collapse to one React key, and - * `claimLabel` writes its ordinal at the very tail the clip removes. The index - * is reserved out of the bound, not appended to it, because both readers - * re-clip to the same cap and would cut a suffix that overflowed it. */ -function boundSubagentField(value: string, index: number): string { - if (value.length <= MAX_SUBAGENT_FIELD_CHARS) { - return value - } - const suffix = `…~${index}` - const keep = MAX_SUBAGENT_FIELD_CHARS - suffix.length - // Slicing UTF-16 units can split a surrogate pair; a lone surrogate is - // malformed in a durable row and lossy through any non-JSON UTF-8 hop. - const last = value.charCodeAt(keep - 1) - const end = last >= 0xd800 && last <= 0xdbff ? keep - 1 : keep - return `${value.slice(0, end)}${suffix}` -} diff --git a/src/main/ipc/runtime.ts b/src/main/ipc/runtime.ts index 3901d8b1ffa..6237b8d040d 100644 --- a/src/main/ipc/runtime.ts +++ b/src/main/ipc/runtime.ts @@ -11,6 +11,7 @@ import type { RuntimeRpcResponse } from '../../shared/runtime-rpc-envelope' import type { ClientHostedBrowserRowsEvent } from '../../shared/client-hosted-browser-rows' import { TERMINAL_FIT_RESTORE_DEADLINE_MS } from '../../shared/terminal-fit-restore-deadline' import { + AGENT_SESSION_BACKGROUND_TASK_STOP_CAPABILITY, CLAUDE_STRUCTURED_AGENT_SESSION_RUNTIME_CAPABILITY, STRUCTURED_AGENT_SESSION_RUNTIME_CAPABILITY } from '../../shared/protocol-version' @@ -80,6 +81,7 @@ export function registerRuntimeHandlers(runtime: OrcaRuntimeService): void { clientKind: 'runtime', connectionId: desktopSenders.connectionIdFor(event.sender), clientCapabilities: [ + AGENT_SESSION_BACKGROUND_TASK_STOP_CAPABILITY, STRUCTURED_AGENT_SESSION_RUNTIME_CAPABILITY, CLAUDE_STRUCTURED_AGENT_SESSION_RUNTIME_CAPABILITY ] @@ -128,6 +130,7 @@ export function registerRuntimeHandlers(runtime: OrcaRuntimeService): void { clientKind: 'runtime', connectionId, clientCapabilities: [ + AGENT_SESSION_BACKGROUND_TASK_STOP_CAPABILITY, STRUCTURED_AGENT_SESSION_RUNTIME_CAPABILITY, CLAUDE_STRUCTURED_AGENT_SESSION_RUNTIME_CAPABILITY ] diff --git a/src/main/native-chat/agent-session-wire/agent-session-delta-coalescer-protected.test.ts b/src/main/native-chat/agent-session-wire/agent-session-delta-coalescer-protected.test.ts new file mode 100644 index 00000000000..ff82a8f63ed --- /dev/null +++ b/src/main/native-chat/agent-session-wire/agent-session-delta-coalescer-protected.test.ts @@ -0,0 +1,57 @@ +import { describe, expect, it } from 'vitest' +import { createAgentSessionDeltaCoalescer } from './agent-session-delta-coalescer' + +describe('protected provider streams', () => { + it('preserves protected prefixes beyond the ordinary count cap and evicts only ordinary streams', () => { + const protectedKeys = new Set() + const coalescer = createAgentSessionDeltaCoalescer({ + maxStreams: 1, + isProtected: (key) => protectedKeys.has(key), + emit: () => true, + schedule: () => () => {} + }) + // A typed start can arrive after the first delta, before the next output. + coalescer.append('command-0', 'before') + protectedKeys.add('command-0') + for (let index = 1; index < 448; index += 1) { + const key = `command-${index}` + protectedKeys.add(key) + expect(coalescer.append(key, 'before')).toBe(true) + } + coalescer.append('ordinary-1', 'temporary') + coalescer.append('ordinary-2', 'latest') + expect(coalescer.snapshot('ordinary-1')).toBeNull() + expect(coalescer.snapshot('ordinary-2')?.text).toBe('latest') + for (const key of protectedKeys) { + coalescer.append(key, 'after') + expect(coalescer.snapshot(key)?.text).toBe('beforeafter') + coalescer.forget(key) + expect(coalescer.snapshot(key)).toBeNull() + } + coalescer.dispose() + }) + + it('keeps aggregate and per-stream output byte ceilings under protected-key pressure', () => { + const coalescer = createAgentSessionDeltaCoalescer({ + maxStreams: 1, + maxRetainedBytes: 80, + maxTotalRetainedBytes: 120, + isProtected: () => true, + emit: () => true, + schedule: () => () => {} + }) + coalescer.append('first', 'a'.repeat(80)) + coalescer.append('second', 'b'.repeat(80)) + coalescer.append('third', 'c'.repeat(80)) + const snapshots = ['first', 'second', 'third'].map((key) => coalescer.snapshot(key)!) + expect(snapshots[0].text).toBe('a'.repeat(80)) + expect(snapshots.map((snapshot) => snapshot.truncated)).toEqual([false, true, true]) + expect( + snapshots.reduce((total, snapshot) => total + Buffer.byteLength(snapshot.text), 0) + ).toBeLessThanOrEqual(120) + coalescer.forget('first') + coalescer.append('new', 'd'.repeat(80)) + expect(coalescer.snapshot('new')?.text).toBe('d'.repeat(80)) + coalescer.dispose() + }) +}) diff --git a/src/main/native-chat/agent-session-wire/agent-session-delta-coalescer.ts b/src/main/native-chat/agent-session-wire/agent-session-delta-coalescer.ts index dbc63154b72..73dc08cc05c 100644 --- a/src/main/native-chat/agent-session-wire/agent-session-delta-coalescer.ts +++ b/src/main/native-chat/agent-session-wire/agent-session-delta-coalescer.ts @@ -37,6 +37,8 @@ export type AgentSessionDeltaCoalescerDeps = { maxTotalRetainedBytes?: number /** Maximum distinct item streams retained at once. */ maxStreams?: number + /** The caller byte-bounds protected metadata; only ordinary streams use the count cap. */ + isProtected?: (key: string) => boolean /** Injected by tests so a window can be driven without real time. */ schedule?: (run: () => void, ms: number) => () => void } @@ -83,8 +85,7 @@ export function createAgentSessionDeltaCoalescer( >() let totalRetainedBytes = 0 let cancelTimer: (() => void) | null = null - const streamOrder = new Map() - let nextOrder = 0 + const evictable = new Set() const flushKey = (key: string): boolean => { const stream = streams.get(key) @@ -130,9 +131,13 @@ export function createAgentSessionDeltaCoalescer( if (!stream) { // Evict the oldest stream before admitting a new attacker-controlled // id. Flush first so the retained prefix is durably visible. - if (streams.size >= maxStreams) { - const oldest = [...streamOrder.entries()].sort((a, b) => a[1] - b[1])[0]?.[0] + while (!deps.isProtected?.(key) && evictable.size >= maxStreams) { + const oldest = evictable.values().next().value if (oldest) { + if (deps.isProtected?.(oldest)) { + evictable.delete(oldest) + continue + } // Under sink backpressure the oldest stream must remain available // for a later retry; dropping it would lose already-observed output. if (!flushKey(oldest)) { @@ -143,8 +148,9 @@ export function createAgentSessionDeltaCoalescer( totalRetainedBytes -= evicted.retainedBytes } streams.delete(oldest) - streamOrder.delete(oldest) + evictable.delete(oldest) } + break } stream = { chunks: [], @@ -153,7 +159,11 @@ export function createAgentSessionDeltaCoalescer( truncated: false, dirty: false } - streamOrder.set(key, nextOrder++) + if (!deps.isProtected?.(key)) { + evictable.add(key) + } + } else if (deps.isProtected?.(key)) { + evictable.delete(key) } stream.observedBytes += Buffer.byteLength(delta, 'utf8') if (!stream.truncated) { @@ -184,14 +194,14 @@ export function createAgentSessionDeltaCoalescer( if (stream) { totalRetainedBytes -= stream.retainedBytes streams.delete(key) - streamOrder.delete(key) + evictable.delete(key) } }, dispose: () => { cancelTimer?.() cancelTimer = null streams.clear() - streamOrder.clear() + evictable.clear() totalRetainedBytes = 0 }, snapshot: (key) => { diff --git a/src/main/native-chat/agent-session-wire/structured-conversation-command-admission.test.ts b/src/main/native-chat/agent-session-wire/structured-conversation-command-admission.test.ts new file mode 100644 index 00000000000..42d03f46b5b --- /dev/null +++ b/src/main/native-chat/agent-session-wire/structured-conversation-command-admission.test.ts @@ -0,0 +1,48 @@ +import { describe, expect, it } from 'vitest' +import type { AgentSessionRecord } from '../../../shared/agent-session-record' +import type { AgentSessionBackgroundTaskState } from '../../../shared/agent-session-wire' +import { conversationCommandBlocked } from './structured-conversation-command-admission' +import type { AgentSessionTurnContext } from './structured-agent-session-turns' + +function contextWith( + backgroundTasks: AgentSessionBackgroundTaskState | null +): AgentSessionTurnContext { + return { + sessionId: 'session-1', + journal: { + snapshot: () => ({ items: [] }), + submissions: () => [] + }, + adapter: { backgroundTaskState: () => backgroundTasks } + } as unknown as AgentSessionTurnContext +} + +const RECORD = { lease: {} } as unknown as AgentSessionRecord + +describe('conversationCommandBlocked background tasks', () => { + it('admits the command when nothing is being monitored', () => { + expect(conversationCommandBlocked(contextWith(null), RECORD)).toBeNull() + }) + + it('asks for a stop when the host accepts targeted stops', () => { + const blocked = conversationCommandBlocked( + contextWith({ state: 'monitoring', supportsTaskStop: true }), + RECORD + ) + expect(blocked).toBe('Stop background tasks before using this command.') + }) + + it('asks for a stop on a host that predates the stop-capability field', () => { + const blocked = conversationCommandBlocked(contextWith({ state: 'monitoring' }), RECORD) + expect(blocked).toBe('Stop background tasks before using this command.') + }) + + it('asks the user to wait when the provider exposes no stop at all', () => { + // Codex: an instruction to stop would name a control that does not exist. + const blocked = conversationCommandBlocked( + contextWith({ state: 'monitoring', supportsStopAll: false }), + RECORD + ) + expect(blocked).toBe('Wait for background tasks to finish before using this command.') + }) +}) diff --git a/src/main/native-chat/agent-session-wire/structured-conversation-command-admission.ts b/src/main/native-chat/agent-session-wire/structured-conversation-command-admission.ts index a69a5163b67..78b9f4a59d0 100644 --- a/src/main/native-chat/agent-session-wire/structured-conversation-command-admission.ts +++ b/src/main/native-chat/agent-session-wire/structured-conversation-command-admission.ts @@ -38,8 +38,14 @@ export function conversationCommandBlocked( ) { return 'Resolve the pending question or approval before using this command.' } - if (ctx.adapter.backgroundTaskState?.(ctx.sessionId)?.state === 'monitoring') { - return 'Stop background tasks before using this command.' + const backgroundTasks = ctx.adapter.backgroundTaskState?.(ctx.sessionId) + if (backgroundTasks?.state === 'monitoring') { + // Only ask for a stop the host can actually perform. A provider that + // exposes neither a targeted nor an untargeted stop would otherwise leave + // the command refused behind an instruction nobody can follow. + return backgroundTasks.supportsTaskStop || backgroundTasks.supportsStopAll !== false + ? 'Stop background tasks before using this command.' + : 'Wait for background tasks to finish before using this command.' } if ( ctx.journal diff --git a/src/main/runtime/orchestration/db/contract-constants.ts b/src/main/runtime/orchestration/db/contract-constants.ts index 47e0cd6165a..56c2e2d542f 100644 --- a/src/main/runtime/orchestration/db/contract-constants.ts +++ b/src/main/runtime/orchestration/db/contract-constants.ts @@ -1,7 +1,17 @@ -import { ORCHESTRATION_LEGACY_RUN_ID } from '../../../../shared/orchestration-rpc-contract' +import { + ORCHESTRATION_LEGACY_RUN_ID, + ORCHESTRATION_UNBOUND_RUN_ID +} from '../../../../shared/orchestration-rpc-contract' import { ORCHESTRATION_CONTRACT_VERSION } from '../../../../shared/protocol-version' export const LEGACY_RUN_ID = ORCHESTRATION_LEGACY_RUN_ID +export const UNBOUND_RUN_ID = ORCHESTRATION_UNBOUND_RUN_ID + +// Why: a v1.4.198 coordinator sends no Run id, so its remote workers file mail under a per-attachment stub Run. +export const FEDERATED_STUB_HOME_RUN_ID_PREFIX = 'run_federated_' +export function federatedStubHomeRunId(dispatchId: string): string { + return `${FEDERATED_STUB_HOME_RUN_ID_PREFIX}${dispatchId}` +} export const LEGACY_CONTRACT_VERSION = 0 export const CURRENT_CONTRACT_VERSION = ORCHESTRATION_CONTRACT_VERSION diff --git a/src/main/runtime/orchestration/db/federation/federated-stub-home-run-backfill.ts b/src/main/runtime/orchestration/db/federation/federated-stub-home-run-backfill.ts new file mode 100644 index 00000000000..47c596e260d --- /dev/null +++ b/src/main/runtime/orchestration/db/federation/federated-stub-home-run-backfill.ts @@ -0,0 +1,16 @@ +import type Database from '../../../../sqlite/sync-database' +import { FEDERATED_STUB_HOME_RUN_ID_PREFIX } from '../contract-constants' + +// Why: a rolled-back v1.4.198 host inserts attachments with home_run_id='' after user_version is +// already 40, so this idempotent repair runs on every open, not only inside the v40 migration. +export function backfillFederatedStubHomeRuns(db: Database.Database): void { + db.exec(` + INSERT OR IGNORE INTO runs (id, objective, home_database, consumer_generation, legacy) + SELECT '${FEDERATED_STUB_HOME_RUN_ID_PREFIX}' || dispatch_id, + 'Coordinated from ' || home_peer_fingerprint, 'remote', 0, 0 + FROM remote_dispatch_attachments WHERE home_run_id = ''; + UPDATE remote_dispatch_attachments + SET home_run_id = '${FEDERATED_STUB_HOME_RUN_ID_PREFIX}' || dispatch_id + WHERE home_run_id = ''; + `) +} diff --git a/src/main/runtime/orchestration/db/federation/remote-dispatch-attachment-create.ts b/src/main/runtime/orchestration/db/federation/remote-dispatch-attachment-create.ts index d927cf96838..5a6e89b65ca 100644 --- a/src/main/runtime/orchestration/db/federation/remote-dispatch-attachment-create.ts +++ b/src/main/runtime/orchestration/db/federation/remote-dispatch-attachment-create.ts @@ -2,13 +2,15 @@ import type { WorkerDispatchState, RemoteDispatchAttachmentRow } from '../../typ import { OrchestrationError } from '../../orchestration-error' import { ensureMutationReceiptCapacity } from '../../mutation-receipt-capacity' import type { OrchestrationDb } from '../orchestration-db' +import { federatedStubHomeRunId } from '../contract-constants' import { insertRemoteDispatchAttachmentRow } from '../dispatch-row-writer' export function createRemoteDispatchAttachment( this: OrchestrationDb, params: { dispatchId: string - runId: string + /** Absent from a v1.4.198 coordinator; replaced by a per-attachment stub Run. */ + runId?: string taskId: string homePeerFingerprint: string protocolVersion: number @@ -44,7 +46,8 @@ export function createRemoteDispatchAttachment( `Remote attachment request ${params.mutationReceipt.requestId} already exists.` ) } - if (!params.runId?.trim()) { + const runId = params.runId ?? federatedStubHomeRunId(params.dispatchId) + if (!runId.trim()) { throw new OrchestrationError('invalid_argument', 'Missing Run ID') } this.db @@ -52,8 +55,8 @@ export function createRemoteDispatchAttachment( `INSERT OR IGNORE INTO runs (id, objective, home_database, consumer_generation, legacy) VALUES (?, ?, 'remote', 0, 0)` ) - .run(params.runId, `Coordinated from ${params.homePeerFingerprint}`) - this.requireRun(params.runId) + .run(runId, `Coordinated from ${params.homePeerFingerprint}`) + this.requireRun(runId) ensureMutationReceiptCapacity(this.db) this.db .prepare( @@ -70,7 +73,7 @@ export function createRemoteDispatchAttachment( ) insertRemoteDispatchAttachmentRow(this.db, { dispatchId: params.dispatchId, - runId: params.runId, + runId, taskId: params.taskId, homePeerFingerprint: params.homePeerFingerprint, protocolVersion: params.protocolVersion, diff --git a/src/main/runtime/orchestration/db/messages/message-insert.ts b/src/main/runtime/orchestration/db/messages/message-insert.ts index 82573305479..51ce7aaaa50 100644 --- a/src/main/runtime/orchestration/db/messages/message-insert.ts +++ b/src/main/runtime/orchestration/db/messages/message-insert.ts @@ -3,6 +3,7 @@ import { generateId } from '../generated-id' import { exposeMessageTimestamps } from '../utc-timestamp' import type { OrchestrationDb } from '../orchestration-db' import { runLifecycleWriteTransaction } from '../lifecycle-write-transaction-runner' +import { UNBOUND_RUN_ID } from '../contract-constants' // ── Messages ── @@ -25,9 +26,17 @@ export type MessageInsert = { } export function insertMessage(this: OrchestrationDb, msg: MessageInsert): MessageRow { - const runId = msg.runId - if (!runId) { - throw new Error('Run is required') + // A sender in no Run (two plain terminals, `send --to `) still gets durable mail. It is + // filed under the unbound Run, never the legacy one, which the schema-skew probe reads as pre-Runs. + // Created on first use so `run list` shows it only to a user who has such mail. + const runId = msg.runId ?? UNBOUND_RUN_ID + if (msg.runId == null) { + this.db + .prepare( + `INSERT OR IGNORE INTO runs (id, objective, home_database, consumer_generation, legacy) + VALUES (?, 'Mail from terminals in no Run', 'this_database', 0, 0)` + ) + .run(UNBOUND_RUN_ID) } const deliveryContract = msg.deliveryContract ?? 'current_delivery' this.requireRun(runId) diff --git a/src/main/runtime/orchestration/db/orchestration-db.ts b/src/main/runtime/orchestration/db/orchestration-db.ts index 2a970841a38..1ce52e96c4a 100644 --- a/src/main/runtime/orchestration/db/orchestration-db.ts +++ b/src/main/runtime/orchestration/db/orchestration-db.ts @@ -1,6 +1,7 @@ import Database from '../../../sqlite/sync-database' import { attachOrchestrationDbMethods } from './attach-orchestration-db-methods' import { hardenOrchestrationDatabaseFiles } from './database-file-permissions' +import { backfillFederatedStubHomeRuns } from './federation/federated-stub-home-run-backfill' import type { OrchestrationDbMethods } from './orchestration-db-methods' import { createCoordinatorMailRoutingTrigger, @@ -28,6 +29,7 @@ class OrchestrationDbCore { this.db.pragma('busy_timeout = 5000') createTables.call(this as unknown as OrchestrationDb) migrate.call(this as unknown as OrchestrationDb) + backfillFederatedStubHomeRuns(this.db) createCoordinatorMailRoutingTrigger.call(this as unknown as OrchestrationDb) rememberCurrentRunCoordinatorHandles.call(this as unknown as OrchestrationDb) hardenOrchestrationDatabaseFiles(dbPath) diff --git a/src/main/runtime/orchestration/db/schema/create-graph-tables-sql.ts b/src/main/runtime/orchestration/db/schema/create-graph-tables-sql.ts index 0897cb852de..d674a5298e7 100644 --- a/src/main/runtime/orchestration/db/schema/create-graph-tables-sql.ts +++ b/src/main/runtime/orchestration/db/schema/create-graph-tables-sql.ts @@ -38,7 +38,8 @@ CREATE TABLE IF NOT EXISTS federated_dispatches ( ); CREATE TABLE IF NOT EXISTS remote_dispatch_attachments ( - home_run_id TEXT NOT NULL, + -- DEFAULT: a rolled-back v1.4.198 host still inserts here without a home Run. + home_run_id TEXT NOT NULL DEFAULT '', dispatch_id TEXT PRIMARY KEY, task_id TEXT NOT NULL, home_peer_fingerprint TEXT NOT NULL, diff --git a/src/main/runtime/orchestration/db/schema/federated-home-run-migration.test.ts b/src/main/runtime/orchestration/db/schema/federated-home-run-migration.test.ts index b970435223a..4fcd78634f3 100644 --- a/src/main/runtime/orchestration/db/schema/federated-home-run-migration.test.ts +++ b/src/main/runtime/orchestration/db/schema/federated-home-run-migration.test.ts @@ -1,26 +1,105 @@ -import { afterEach, describe, expect, it } from 'vitest' +import { mkdtempSync, rmSync } from 'node:fs' +import { tmpdir } from 'node:os' +import { join } from 'node:path' +import { afterEach, beforeEach, describe, expect, it } from 'vitest' +import { ORCHESTRATION_CONTRACT_VERSION } from '../../../../../shared/protocol-version' import { OrchestrationDb } from '../orchestration-db' +import { SCHEMA_VERSION, federatedStubHomeRunId } from '../contract-constants' import { migrateV40 } from './migrate-v40' import { importFederatedControlMessage } from '../../federation-control-message' describe('federated home Run migration', () => { - const db = new OrchestrationDb(':memory:') + let db: OrchestrationDb + beforeEach(() => { + db = new OrchestrationDb(':memory:') + }) afterEach(() => db.close()) - it('adds the home Run column and refuses mail for a development placeholder', () => { + function importInstruction(target: OrchestrationDb, dispatchId: string, messageId: string): void { + expect( + importFederatedControlMessage(target, { + dispatchId, + messageId, + payload: JSON.stringify({ from: 'home', subject: 'Instruction', body: '', type: 'status' }) + }) + ).toEqual({ imported: true, type: 'status' }) + } + + function attachWithoutRunId(target: OrchestrationDb, dispatchId: string, runId?: string): void { + target.createRemoteDispatchAttachment({ + dispatchId, + runId, + taskId: `task_${dispatchId}`, + homePeerFingerprint: 'home', + protocolVersion: ORCHESTRATION_CONTRACT_VERSION, + runtimeEpoch: 'epoch', + mutationReceipt: { + callerFingerprint: 'home', + requestId: `request_${dispatchId}`, + method: 'orchestration.federationAttachStart', + payloadHash: `payload_${dispatchId}` + } + }) + } + + it('backfills a pre-upgrade attachment with a stub home Run that keeps its mailbox', () => { db.db.exec('ALTER TABLE remote_dispatch_attachments DROP COLUMN home_run_id') db.db.exec(`INSERT INTO remote_dispatch_attachments (dispatch_id, task_id, home_peer_fingerprint, runtime_epoch) VALUES ('ctx_old', 'task_old', 'home', 'epoch')`) migrateV40.call(db, 39) - expect(db.getRemoteDispatchAttachment('ctx_old')?.home_run_id).toBe('') - expect(() => - importFederatedControlMessage(db, { - dispatchId: 'ctx_old', - messageId: 'message_old', - payload: JSON.stringify({ from: 'home', subject: 'Instruction', body: '', type: 'message' }) - }) - ).toThrow('Run not found:') - expect(db.getMessageById('message_old')).toBeUndefined() + const stubRunId = federatedStubHomeRunId('ctx_old') + expect(db.getRemoteDispatchAttachment('ctx_old')?.home_run_id).toBe(stubRunId) + expect(db.getRunRaw(stubRunId)).toMatchObject({ home_database: 'remote', legacy: 0 }) + importInstruction(db, 'ctx_old', 'message_old') + expect(db.getMessageById('message_old')?.run_id).toBe(stubRunId) + }) + + it('mints a stub home Run when a v1.4.198 coordinator attaches without a Run id', () => { + attachWithoutRunId(db, 'ctx_legacy_home') + const stubRunId = federatedStubHomeRunId('ctx_legacy_home') + expect(db.getRemoteDispatchAttachment('ctx_legacy_home')?.home_run_id).toBe(stubRunId) + importInstruction(db, 'ctx_legacy_home', 'message_legacy_home') + expect(db.getMessageById('message_legacy_home')?.run_id).toBe(stubRunId) + }) + + it('rejects a whitespace-only Run id instead of minting a stub', () => { + expect(() => attachWithoutRunId(db, 'ctx_blank', ' ')).toThrow('Missing Run ID') + expect(db.getRemoteDispatchAttachment('ctx_blank')).toBeUndefined() + }) + + it('repairs rows a rolled-back v1.4.198 host inserted after user_version reached 40', () => { + const dir = mkdtempSync(join(tmpdir(), 'orca-federated-home-run-')) + const dbPath = join(dir, 'orchestration.db') + try { + const upgraded = new OrchestrationDb(dbPath) + expect(upgraded.db.pragma('user_version', { simple: true })).toBe(SCHEMA_VERSION) + // v1.4.198's insert shape: no home_run_id column, so the DEFAULT '' lands. + upgraded.db.exec(`INSERT INTO remote_dispatch_attachments + (dispatch_id, task_id, home_peer_fingerprint, runtime_epoch) + VALUES ('ctx_rolled_back', 'task_rolled_back', 'home', 'epoch')`) + expect(upgraded.getRemoteDispatchAttachment('ctx_rolled_back')?.home_run_id).toBe('') + expect(() => + importFederatedControlMessage(upgraded, { + dispatchId: 'ctx_rolled_back', + messageId: 'message_refused', + payload: JSON.stringify({ from: 'home', subject: 'x', body: '', type: 'status' }) + }) + ).toThrow(/Run not found/) + upgraded.close() + + const reopened = new OrchestrationDb(dbPath) + try { + const stubRunId = federatedStubHomeRunId('ctx_rolled_back') + expect(reopened.getRemoteDispatchAttachment('ctx_rolled_back')?.home_run_id).toBe(stubRunId) + expect(reopened.getRunRaw(stubRunId)).toMatchObject({ home_database: 'remote', legacy: 0 }) + importInstruction(reopened, 'ctx_rolled_back', 'message_rolled_back') + expect(reopened.getMessageById('message_rolled_back')?.run_id).toBe(stubRunId) + } finally { + reopened.close() + } + } finally { + rmSync(dir, { recursive: true, force: true }) + } }) }) diff --git a/src/main/runtime/orchestration/db/schema/migrate-v40.ts b/src/main/runtime/orchestration/db/schema/migrate-v40.ts index 50ef46f82cc..e3cd46ca30d 100644 --- a/src/main/runtime/orchestration/db/schema/migrate-v40.ts +++ b/src/main/runtime/orchestration/db/schema/migrate-v40.ts @@ -1,11 +1,15 @@ import type { OrchestrationDb } from '../orchestration-db' +import { backfillFederatedStubHomeRuns } from '../federation/federated-stub-home-run-backfill' export function migrateV40(this: OrchestrationDb, current: number): void { - if (current >= 40 || this.hasColumn('remote_dispatch_attachments', 'home_run_id')) { + if (current >= 40) { return } - // Federation is unreleased; any development-only rows fail Run validation until reattached. - this.db.exec( - "ALTER TABLE remote_dispatch_attachments ADD COLUMN home_run_id TEXT NOT NULL DEFAULT ''" - ) + if (!this.hasColumn('remote_dispatch_attachments', 'home_run_id')) { + this.db.exec( + "ALTER TABLE remote_dispatch_attachments ADD COLUMN home_run_id TEXT NOT NULL DEFAULT ''" + ) + } + // Why: workers attached by v1.4.198 keep a mailbox; without a Run their control mail is refused. + backfillFederatedStubHomeRuns(this.db) } diff --git a/src/main/runtime/orchestration/db/writer-run-required.test.ts b/src/main/runtime/orchestration/db/writer-run-required.test.ts index 21fcbeb28d6..682fabb1b60 100644 --- a/src/main/runtime/orchestration/db/writer-run-required.test.ts +++ b/src/main/runtime/orchestration/db/writer-run-required.test.ts @@ -1,5 +1,6 @@ import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' import { OrchestrationDb } from './orchestration-db' +import { UNBOUND_RUN_ID } from './contract-constants' describe('writers require a Run', () => { let db: OrchestrationDb @@ -8,11 +9,11 @@ describe('writers require a Run', () => { }) afterEach(() => db.close()) - it('rejects a message without a Run instead of using the legacy Run', () => { - expect(() => db.insertMessage({ from: 'sender', to: 'worker', subject: 'mail' })).toThrow( - 'Run is required' - ) - expect(db.db.prepare('SELECT id FROM messages').all()).toEqual([]) + it('files a message without a Run under the unbound Run, never the legacy one', () => { + const message = db.insertMessage({ from: 'sender', to: 'worker', subject: 'mail' }) + expect(message.run_id).toBe(UNBOUND_RUN_ID) + expect(db.getRun(UNBOUND_RUN_ID)).toMatchObject({ legacy: 0 }) + expect(db.getUnreadMessages('worker').map((row) => row.id)).toEqual([message.id]) }) it('rejects a Task without a Run instead of using the legacy Run', () => { diff --git a/src/main/runtime/orchestration/orchestration-federated-legacy-probe.test.ts b/src/main/runtime/orchestration/orchestration-federated-legacy-probe.test.ts index ad08a7383f0..5eff7569d27 100644 --- a/src/main/runtime/orchestration/orchestration-federated-legacy-probe.test.ts +++ b/src/main/runtime/orchestration/orchestration-federated-legacy-probe.test.ts @@ -3,7 +3,7 @@ import { tmpdir } from 'node:os' import { join } from 'node:path' import { afterEach, describe, expect, it } from 'vitest' import { LEGACY_RUN_ID, OrchestrationDb } from './db' -import { SCHEMA_VERSION } from './db/contract-constants' +import { federatedStubHomeRunId, SCHEMA_VERSION, UNBOUND_RUN_ID } from './db/contract-constants' import { resolveOrchestrationMigrationStartVersion } from './orchestration-schema-version-skew' describe('federated mailbox legacy-adoption probe', () => { @@ -17,15 +17,22 @@ describe('federated mailbox legacy-adoption probe', () => { } }) - function seedMailbox(handle: string, kind: 'message' | 'delivery'): string { + function seedMailbox( + handle: string, + kind: 'message' | 'delivery', + homeRunId = 'run_home', + mailRunId = LEGACY_RUN_ID + ): string { directory = mkdtempSync(join(tmpdir(), 'orca-federated-legacy-probe-')) const path = join(directory, 'orchestration.db') db = new OrchestrationDb(path) - db.db.exec(` - INSERT INTO remote_dispatch_attachments ( - dispatch_id, task_id, home_peer_fingerprint, home_run_id, runtime_epoch, state - ) VALUES ('ctx_remote', 'task_remote', 'peer_home', 'run_home', 'epoch', 'ready'); - `) + db.db + .prepare( + `INSERT INTO remote_dispatch_attachments ( + dispatch_id, task_id, home_peer_fingerprint, home_run_id, runtime_epoch, state + ) VALUES ('ctx_remote', 'task_remote', 'peer_home', ?, 'epoch', 'ready')` + ) + .run(homeRunId) if (kind === 'message') { db.db .prepare( @@ -33,14 +40,14 @@ describe('federated mailbox legacy-adoption probe', () => { id, run_id, delivery_contract, from_handle, to_handle, subject, type ) VALUES ('msg_probe', ?, 'current_delivery', 'term_home', ?, 'continue', 'dispatch')` ) - .run(LEGACY_RUN_ID, handle) + .run(mailRunId, handle) } else { db.db .prepare( `INSERT INTO deliveries (id, run_id, mailbox_handle, consumer_generation, message_ids) VALUES ('delivery_probe', ?, ?, 0, '[]')` ) - .run(LEGACY_RUN_ID, handle) + .run(mailRunId, handle) } return path } @@ -70,6 +77,27 @@ describe('federated mailbox legacy-adoption probe', () => { } ) + it.each(['message', 'delivery'] as const)( + 'does not treat a stub-home-Run attachment %s as pre-Runs evidence', + (kind) => { + const stubRunId = federatedStubHomeRunId('ctx_remote') + const path = seedMailbox('dispatch:ctx_remote', kind, stubRunId, stubRunId) + db!.db + .prepare( + `INSERT INTO runs (id, objective, home_database, consumer_generation, legacy) + VALUES (?, 'Coordinated from peer_home', 'remote', 0, 0)` + ) + .run(stubRunId) + expect( + resolveOrchestrationMigrationStartVersion(db!.db, SCHEMA_VERSION, SCHEMA_VERSION) + ).toBe(SCHEMA_VERSION) + db!.close() + db = new OrchestrationDb(path) + expect(db.getLegacyAdoption()).toBeUndefined() + expect(db.getRemoteDispatchAttachment('ctx_remote')?.home_run_id).toBe(stubRunId) + } + ) + it.each(['message', 'delivery'] as const)( 'still replays adoption for a genuine legacy %s', (kind) => { @@ -94,4 +122,19 @@ describe('federated mailbox legacy-adoption probe', () => { } } ) + + it('keeps mail from a terminal in no Run across a reopen without replaying adoption', () => { + directory = mkdtempSync(join(tmpdir(), 'orca-unbound-mail-probe-')) + const path = join(directory, 'orchestration.db') + db = new OrchestrationDb(path) + const sent = db.insertMessage({ from: 'term_a', to: 'term_b', subject: 'hi' }) + expect(sent.run_id).toBe(UNBOUND_RUN_ID) + expect(resolveOrchestrationMigrationStartVersion(db.db, SCHEMA_VERSION, SCHEMA_VERSION)).toBe( + SCHEMA_VERSION + ) + db.close() + db = new OrchestrationDb(path) + expect(db.getLegacyAdoption()).toBeUndefined() + expect(db.getUnreadMessages('term_b').map((row) => row.id)).toEqual([sent.id]) + }) }) diff --git a/src/main/runtime/rpc/methods/orchestration/federation/federation-start-schema.test.ts b/src/main/runtime/rpc/methods/orchestration/federation/federation-start-schema.test.ts new file mode 100644 index 00000000000..f00e5102d6d --- /dev/null +++ b/src/main/runtime/rpc/methods/orchestration/federation/federation-start-schema.test.ts @@ -0,0 +1,41 @@ +import { describe, expect, it } from 'vitest' +import { FederationAttachStartParams } from './federation-start-schema' + +// The request shape a v1.4.198 coordinator sends: no runId field at all. +const legacyRequest = { + dispatchId: 'ctx_legacy', + taskId: 'task_legacy', + taskSpec: 'Do the thing', + protocolVersion: 3, + worktree: 'feature-branch' +} + +describe('FederationAttachStartParams', () => { + it('parses a v1.4.198 request that carries no runId', () => { + const result = FederationAttachStartParams.safeParse(legacyRequest) + expect(result.success, result.success ? undefined : JSON.stringify(result.error.issues)).toBe( + true + ) + expect(result.success && result.data.runId).toBeUndefined() + }) + + it('keeps a v1.4.199 runId verbatim', () => { + const result = FederationAttachStartParams.parse({ ...legacyRequest, runId: 'run_home' }) + expect(result.runId).toBe('run_home') + }) + + // Pins OptionalString: '' and non-strings drop to undefined (a stub Run is minted downstream); + // whitespace-only passes the schema and is refused by createRemoteDispatchAttachment. + it('maps an empty or non-string runId to undefined but passes whitespace through', () => { + expect(FederationAttachStartParams.parse({ ...legacyRequest, runId: '' }).runId).toBeUndefined() + expect(FederationAttachStartParams.parse({ ...legacyRequest, runId: 7 }).runId).toBeUndefined() + expect(FederationAttachStartParams.parse({ ...legacyRequest, runId: ' ' }).runId).toBe(' ') + }) + + it('still requires the dispatch, task, spec, and worktree fields', () => { + for (const field of ['dispatchId', 'taskId', 'taskSpec', 'worktree'] as const) { + const { [field]: _dropped, ...rest } = legacyRequest + expect(FederationAttachStartParams.safeParse(rest).success, field).toBe(false) + } + }) +}) diff --git a/src/main/runtime/rpc/methods/orchestration/federation/federation-start-schema.ts b/src/main/runtime/rpc/methods/orchestration/federation/federation-start-schema.ts index 1e7257df27d..84ed57d58cc 100644 --- a/src/main/runtime/rpc/methods/orchestration/federation/federation-start-schema.ts +++ b/src/main/runtime/rpc/methods/orchestration/federation/federation-start-schema.ts @@ -3,7 +3,8 @@ import { OptionalFiniteNumber, OptionalString, requiredString } from '../../../s import { OptionalWorkerLaunchPreference } from '../worker/worker-start-schema' export const FederationAttachStartParams = z.object({ - runId: requiredString('Missing Run ID'), + /** Omitted by v1.4.198 coordinators; the worker host then mints a stub home Run. */ + runId: OptionalString, dispatchId: requiredString('Missing Dispatch ID'), taskId: requiredString('Missing Task ID'), taskSpec: requiredString('Missing Task spec'), diff --git a/src/main/runtime/rpc/methods/orchestration/messaging/send-unbound-terminals.test.ts b/src/main/runtime/rpc/methods/orchestration/messaging/send-unbound-terminals.test.ts new file mode 100644 index 00000000000..84cb250a19e --- /dev/null +++ b/src/main/runtime/rpc/methods/orchestration/messaging/send-unbound-terminals.test.ts @@ -0,0 +1,31 @@ +import { afterEach, describe, expect, it, vi } from 'vitest' +import { createOrchestrationRpcHarness } from '../rpc-test-harness' + +describe('orchestration.send between terminals in no Run', () => { + const h = createOrchestrationRpcHarness() + afterEach(() => h.cleanup()) + + it('delivers terminal-to-terminal mail when neither terminal is in a Run', async () => { + // Two plain panes and `send --to `: the first command the guide teaches. #19542 + // refused this with a bare "Run is required"; it files under the unbound Run instead. + const { db, runtime, ctx } = h.setup(false) + vi.spyOn(runtime, 'getTerminalPaneKey').mockImplementation((handle) => + handle === 'term_a' ? 'tab_a:leaf_a' : handle === 'term_b' ? 'tab_b:leaf_b' : null + ) + vi.spyOn(runtime, 'deliverPendingMessagesForHandle').mockImplementation(() => {}) + + const result = (await h.call( + 'orchestration.send', + { from: 'term_a', to: 'term_b', subject: 'hello from no Run' }, + ctx + )) as { message: { id: string; run_id: string; to_handle: string } } + + expect(result.message).toMatchObject({ run_id: 'run_unbound', to_handle: 'term_b' }) + expect(db.getRun('run_unbound')).toMatchObject({ legacy: 0 }) + expect(db.getUnreadMessages('term_b').map((row) => row.id)).toEqual([result.message.id]) + const checked = (await h.call('orchestration.check', { terminal: 'term_b' }, ctx)) as { + messages: { id: string }[] + } + expect(checked.messages.map((row) => row.id)).toEqual([result.message.id]) + }) +}) diff --git a/src/main/runtime/rpc/methods/structured-agent-session-background-task-capability.test.ts b/src/main/runtime/rpc/methods/structured-agent-session-background-task-capability.test.ts new file mode 100644 index 00000000000..cc5a7282080 --- /dev/null +++ b/src/main/runtime/rpc/methods/structured-agent-session-background-task-capability.test.ts @@ -0,0 +1,105 @@ +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' +import type { AgentSessionBackgroundTaskState } from '../../../../shared/agent-session-wire' +import { AGENT_SESSION_BACKGROUND_TASK_STOP_CAPABILITY } from '../../../../shared/protocol-version' +import { remoteRuntimeClientCapabilities } from '../../../../shared/remote-runtime-client-capabilities' +import type { AgentSessionSubscribeInput } from '../../../native-chat/agent-session-wire/structured-agent-session-subscribers' +import { + call, + clearStructuredHostStub, + hostCalls, + installStructuredHostStub, + SESSION, + STRUCTURED_CLIENT +} from './structured-agent-session-rpc.test-fixture' + +beforeEach(installStructuredHostStub) +afterEach(clearStructuredHostStub) + +const TASKS: AgentSessionBackgroundTaskState = { + state: 'monitoring', + supportsStopAll: false, + tasks: [{ id: 'child', kind: 'agent' }] +} +const CURRENT_CLIENT = { + ...STRUCTURED_CLIENT, + clientCapabilities: remoteRuntimeClientCapabilities(STRUCTURED_CLIENT.clientCapabilities) +} + +describe('background-task stop capability at the RPC boundary', () => { + it('advertises reader support on remote requests and subscriptions', () => { + expect(CURRENT_CLIENT.clientCapabilities).toContain( + AGENT_SESSION_BACKGROUND_TASK_STOP_CAPABILITY + ) + }) + + it.each([ + ['legacy reader', STRUCTURED_CLIENT, null], + ['current reader', CURRENT_CLIENT, TASKS], + ['in-process reader', undefined, TASKS] + ] as const)('projects history for a %s', async (_label, client, expected) => { + hostCalls.history.mockReturnValue({ ok: true, page: { items: [], backgroundTasks: TASKS } }) + expect( + await call('agentSession.history', { sessionId: SESSION, direction: 'tail' }, client) + ).toMatchObject({ ok: true, result: { page: { backgroundTasks: expected } } }) + }) + + it.each(['snapshot', 'batch', 'reset'] as const)( + 'gates the %s stream without changing the provider state', + async (type) => { + hostCalls.hold = vi.fn(async () => undefined) + hostCalls.subscribe.mockImplementation((input: AgentSessionSubscribeInput) => { + const base = { sessionId: SESSION, fence: 1, backgroundTasks: TASKS } + if (type === 'batch') { + input.emit({ + ...base, + type, + batch: { + cursor: { epoch: 'a', sequence: 0 }, + items: [], + removedItemIds: [], + submissions: [] + } + }) + } else { + const page = { + sessionId: SESSION, + epoch: 'a', + direction: 'tail' as const, + items: [], + removedItemIds: [], + submissions: [], + window: { oldest: null, newest: null, nextCursor: { epoch: 'a', sequence: 0 } }, + hasOlder: false, + hasNewer: false + } + input.emit( + type === 'snapshot' + ? { ...base, type, page } + : { ...base, type, page, reset: 'epoch_changed' } + ) + } + return () => {} + }) + for (const [client, expected] of [ + [STRUCTURED_CLIENT, null], + [CURRENT_CLIENT, TASKS] + ] as const) { + expect(await call('agentSession.subscribe', { sessionId: SESSION }, client)).toMatchObject({ + ok: true, + result: { type, backgroundTasks: expected } + }) + } + expect(TASKS.supportsStopAll).toBe(false) + } + ) + + it('preserves legacy stoppable state for both readers', async () => { + const stoppable = { state: 'monitoring', tasks: TASKS.tasks } + hostCalls.history.mockReturnValue({ ok: true, page: { items: [], backgroundTasks: stoppable } }) + for (const client of [STRUCTURED_CLIENT, CURRENT_CLIENT]) { + expect( + await call('agentSession.history', { sessionId: SESSION, direction: 'tail' }, client) + ).toMatchObject({ ok: true, result: { page: { backgroundTasks: stoppable } } }) + } + }) +}) diff --git a/src/main/runtime/rpc/methods/structured-agent-session-background-task-capability.ts b/src/main/runtime/rpc/methods/structured-agent-session-background-task-capability.ts new file mode 100644 index 00000000000..02afc5569bf --- /dev/null +++ b/src/main/runtime/rpc/methods/structured-agent-session-background-task-capability.ts @@ -0,0 +1,47 @@ +import type { + AgentSessionBackgroundTaskState, + AgentSessionHistoryResult, + AgentSessionSubscribeEvent +} from '../../../../shared/agent-session-wire' +import { AGENT_SESSION_BACKGROUND_TASK_STOP_CAPABILITY } from '../../../../shared/protocol-version' +import type { RpcContext } from '../core' + +type BackgroundTaskReader = Pick + +function supportsReadOnlyTasks(ctx: BackgroundTaskReader): boolean { + return ( + ctx.clientKind === undefined || + ctx.clientCapabilities?.includes(AGENT_SESSION_BACKGROUND_TASK_STOP_CAPABILITY) === true + ) +} + +function projectState( + state: AgentSessionBackgroundTaskState | null | undefined, + ctx: BackgroundTaskReader +): AgentSessionBackgroundTaskState | null | undefined { + // Legacy readers always offer a stop; retain their pre-producer empty strip. + return state?.supportsStopAll === false && !state.supportsTaskStop && !supportsReadOnlyTasks(ctx) + ? null + : state +} + +export function projectBackgroundTaskHistory( + result: AgentSessionHistoryResult, + ctx: BackgroundTaskReader +): AgentSessionHistoryResult { + const state = projectState(result.page.backgroundTasks, ctx) + return state === result.page.backgroundTasks + ? result + : { ...result, page: { ...result.page, backgroundTasks: state } } +} + +export function projectBackgroundTaskEvent( + event: AgentSessionSubscribeEvent, + ctx: BackgroundTaskReader +): AgentSessionSubscribeEvent { + if (!('backgroundTasks' in event)) { + return event + } + const state = projectState(event.backgroundTasks, ctx) + return state === event.backgroundTasks ? event : { ...event, backgroundTasks: state } +} diff --git a/src/main/runtime/rpc/methods/structured-agent-session.ts b/src/main/runtime/rpc/methods/structured-agent-session.ts index 3078c9fff61..d629b1c29a1 100644 --- a/src/main/runtime/rpc/methods/structured-agent-session.ts +++ b/src/main/runtime/rpc/methods/structured-agent-session.ts @@ -10,6 +10,10 @@ import { computeAgentSessionPayloadFingerprint } from '../../../../shared/agent-session-mutation-envelope' import type { z } from 'zod' +import { + projectBackgroundTaskEvent, + projectBackgroundTaskHistory +} from './structured-agent-session-background-task-capability' import { defineMethod, defineStreamingMethod, type RpcAnyMethod, type RpcContext } from '../core' import { ensureStructuredHostInstalled as ensureHostInstalled, @@ -251,7 +255,8 @@ export const STRUCTURED_AGENT_SESSION_METHODS: RpcAnyMethod[] = [ defineMethod({ name: 'agentSession.history', params: HistoryParams, - handler: async (params, ctx) => requireHost(ctx).history(params) + handler: async (params, ctx) => + projectBackgroundTaskHistory(requireHost(ctx).history(params), ctx) }), defineStreamingMethod({ name: 'agentSession.subscribe', @@ -278,7 +283,7 @@ export const STRUCTURED_AGENT_SESSION_METHODS: RpcAnyMethod[] = [ dispose = host.subscribe({ id: subscriptionId, sessionId: params.sessionId, - emit, + emit: (event) => emit(projectBackgroundTaskEvent(event, ctx)), ...(params.cursor ? { cursor: params.cursor } : {}) }) if (stream.isClosed()) { diff --git a/src/main/runtime/structured-agent-session-runtime.ts b/src/main/runtime/structured-agent-session-runtime.ts index d9b3e59186a..0245f15054b 100644 --- a/src/main/runtime/structured-agent-session-runtime.ts +++ b/src/main/runtime/structured-agent-session-runtime.ts @@ -225,6 +225,8 @@ async function install(deps: StructuredAgentSessionRuntimeDeps): Promise + host?.publishBackgroundTaskState(sessionId, state), onEvent: (event) => { if (event.type !== 'ended' || !('cause' in event) || event.cause !== 'unexpected-exit') { return diff --git a/src/renderer/src/components/native-chat/NativeChatBackgroundTasksStatus.test.tsx b/src/renderer/src/components/native-chat/NativeChatBackgroundTasksStatus.test.tsx index 65a59c8a6b9..a1b3a27228e 100644 --- a/src/renderer/src/components/native-chat/NativeChatBackgroundTasksStatus.test.tsx +++ b/src/renderer/src/components/native-chat/NativeChatBackgroundTasksStatus.test.tsx @@ -1,7 +1,11 @@ // @vitest-environment happy-dom -import { act, cleanup, render } from '@testing-library/react' + +import '@testing-library/jest-dom/vitest' + +import { act, cleanup, fireEvent, render, screen } from '@testing-library/react' import { Profiler } from 'react' -import { afterEach, expect, it, vi } from 'vitest' +import { afterEach, describe, expect, it, vi } from 'vitest' +import type { AgentSessionBackgroundTask } from '../../../../shared/agent-session-wire' import { NativeChatBackgroundTasksStatus } from './NativeChatBackgroundTasksStatus' afterEach(() => { @@ -9,6 +13,54 @@ afterEach(() => { vi.useRealTimers() }) +const TASKS: AgentSessionBackgroundTask[] = [ + { id: 'codex-agent:child-1', kind: 'agent', description: 'count_a' }, + { id: 'codex-command:exec-1', kind: 'command', description: 'sleep 90' } +] + +function renderStrip(props: { supportsTaskStop: boolean; supportsStopAll: boolean }): { + onStop: ReturnType +} { + const onStop = vi.fn() + render( + + ) + fireEvent.click(screen.getByRole('button', { expanded: false })) + return { onStop } +} + +describe('NativeChatBackgroundTasksStatus stop affordances', () => { + it('offers a per-task stop on a host that accepts targeted stops', () => { + renderStrip({ supportsTaskStop: true, supportsStopAll: true }) + expect(screen.getByLabelText('Stop count_a')).toBeInTheDocument() + expect(screen.queryByLabelText('Stop background tasks')).not.toBeInTheDocument() + }) + + it('falls back to a stop-all on a host that only accepts an untargeted stop', () => { + renderStrip({ supportsTaskStop: false, supportsStopAll: true }) + expect(screen.getByLabelText('Stop background tasks')).toBeInTheDocument() + }) + + it('offers no stop at all when the provider exposes none', () => { + // Codex: a Stop button here would be a control that cannot act. + renderStrip({ supportsTaskStop: false, supportsStopAll: false }) + expect(screen.queryByLabelText('Stop background tasks')).not.toBeInTheDocument() + expect(screen.queryByLabelText('Stop count_a')).not.toBeInTheDocument() + expect(screen.getByText('count_a')).toBeInTheDocument() + expect(screen.getByText('sleep 90')).toBeInTheDocument() + }) +}) + it('stops elapsed renders in a hidden pane and catches up on reveal', () => { vi.useFakeTimers() vi.setSystemTime(100_000) @@ -21,6 +73,7 @@ it('stops elapsed renders in a hidden pane and catches up on reveal', () => { settledTasks={[]} indicatorActive supportsTaskStop={false} + supportsStopAll={false} stoppingTaskIds={new Set()} stoppingAll={false} onStop={() => {}} diff --git a/src/renderer/src/components/native-chat/NativeChatBackgroundTasksStatus.tsx b/src/renderer/src/components/native-chat/NativeChatBackgroundTasksStatus.tsx index 4c34fa459c5..944f01b162e 100644 --- a/src/renderer/src/components/native-chat/NativeChatBackgroundTasksStatus.tsx +++ b/src/renderer/src/components/native-chat/NativeChatBackgroundTasksStatus.tsx @@ -113,6 +113,9 @@ 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 @@ -222,7 +225,7 @@ export function NativeChatBackgroundTasksStatus(props: { )}

)} - {!props.supportsTaskStop ? ( + {!props.supportsTaskStop && props.supportsStopAll ? (
0 ? 'mt-2 border-t border-border pt-2' : 'mt-2'}> -
- ) : null} - {messages.map((message, index) => { - const turnKey = turnKeys[index] - const isCurrentTurn = currentTurnKey - ? turnKey === currentTurnKey - : turnKey === undefined - const status = - index === latestUserIndex - ? turnStatuses.active - : message.role === 'user' && turnKey - ? turnStatuses.completedByTurn[turnKey] - : undefined - const receipt = receipts.get(message.id) - const turnDiff = - turnKey && turnKeys[index + 1] !== turnKey ? turnDiffs.get(turnKey) : undefined - return ( - - {receipt ? ( - - ) : ( - - )} - {showTurnStatus && - status && - (index !== latestUserIndex || showTypingIndicator || !isWorking) ? ( - toggleExpandedTurn(turnKey) - : undefined - } - /> - ) : null} - {turnDiff ? ( - - ) : null} - - ) - })} - {showTurnStatus && - latestUserIndex === -1 && - turnStatuses.active && - showTypingIndicator ? ( - - ) : null} - {showTurnStatus && isWorking ? ( - - ) : null} - {!showTurnStatus && showTypingIndicator ? : null} +
+ {hasMore ? ( +
+ +
+ ) : null} + {messages.map((message, index) => { + const turnKey = turnKeys[index] + const isCurrentTurn = currentTurnKey + ? turnKey === currentTurnKey + : turnKey === undefined + const status = + index === latestUserIndex + ? turnStatuses.active + : message.role === 'user' && turnKey + ? turnStatuses.completedByTurn[turnKey] + : undefined + const receipt = receipts.get(message.id) + const turnDiff = + turnKey && turnKeys[index + 1] !== turnKey ? turnDiffs.get(turnKey) : undefined + return ( + + {receipt ? ( + + ) : ( + + )} + {showTurnStatus && + status && + (index !== latestUserIndex || showTypingIndicator || !isWorking) ? ( + toggleExpandedTurn(turnKey) + : undefined + } + /> + ) : null} + {turnDiff ? ( + + ) : null} + + ) + })} + {showTurnStatus && + latestUserIndex === -1 && + turnStatuses.active && + showTypingIndicator ? ( + + ) : null} + {showTurnStatus && isWorking ? ( + + ) : null} + {!showTurnStatus && showTypingIndicator ? : null} +
+ {showJump ? ( + + ) : null} - {showJump ? ( - + {taskListState.list && taskListState.list.tasks.length > 0 ? ( +
+
+ +
+
) : null} ) diff --git a/src/renderer/src/components/native-chat/NativeChatMessageRow.tsx b/src/renderer/src/components/native-chat/NativeChatMessageRow.tsx index e3a4e78efb4..773e6f2c2ff 100644 --- a/src/renderer/src/components/native-chat/NativeChatMessageRow.tsx +++ b/src/renderer/src/components/native-chat/NativeChatMessageRow.tsx @@ -8,7 +8,11 @@ import { isSubagentGroupFallbackText, subagentGroupBlocks } from '../../../../shared/native-chat-subagent-summary' -import { isSubagentGroupBlock, type NativeChatMessage } from '../../../../shared/native-chat-types' +import { + isSubagentGroupBlock, + type NativeChatMessage, + type NativeChatToolCallBlock +} from '../../../../shared/native-chat-types' import { splitNativeChatBlocks } from './native-chat-tool-fold' import { NativeChatToolRun } from './NativeChatToolRun' import { NativeChatNoticeRow } from './NativeChatNoticeRow' @@ -29,6 +33,8 @@ import type { RuntimeFileOperationArgs } from '@/runtime/runtime-file-client' * keep their block identity, so only the changed row re-renders. */ export const MessageRow = memo(function MessageRow({ message, + previousTodoWrite, + previousUpdatePlan, revealedDiff, expandSignal, activeTurnIsWorking, @@ -41,6 +47,8 @@ export const MessageRow = memo(function MessageRow({ runtimeContext }: { message: NativeChatMessage + previousTodoWrite?: NativeChatToolCallBlock + previousUpdatePlan?: NativeChatToolCallBlock revealedDiff?: NativeChatDiffReveal expandSignal: boolean activeTurnIsWorking?: boolean @@ -202,6 +210,8 @@ export const MessageRow = memo(function MessageRow({ {tools.length > 0 || subagentGroups.length > 0 ? ( ({ monitoringBackgroundTasks: false, showBackgroundTasks: false, supportsBackgroundTaskStop: false, + supportsBackgroundTaskStopAll: true, backgroundTasks: [] as AgentSessionBackgroundTask[], settledBackgroundTasks: [] as AgentSessionBackgroundTask[], stopBackgroundTask: vi.fn() @@ -82,7 +83,8 @@ vi.mock('./use-structured-agent-session', async () => { isMonitoring: mocks.monitoringBackgroundTasks, tasks: mocks.backgroundTasks, settledTasks: mocks.settledBackgroundTasks, - supportsStop: mocks.supportsBackgroundTaskStop + supportsStop: mocks.supportsBackgroundTaskStop, + supportsStopAll: mocks.supportsBackgroundTaskStopAll }, turnId: null, cancel: vi.fn(), @@ -175,6 +177,7 @@ describe('NativeChatStructuredSession', () => { mocks.submissions = [] mocks.monitoringBackgroundTasks = false mocks.supportsBackgroundTaskStop = false + mocks.supportsBackgroundTaskStopAll = true mocks.stopBackgroundTask.mockReset() mocks.backgroundTasks = [] mocks.settledBackgroundTasks = [] diff --git a/src/renderer/src/components/native-chat/NativeChatStructuredSession.tsx b/src/renderer/src/components/native-chat/NativeChatStructuredSession.tsx index bef0a2afbf9..3f1fbb073c1 100644 --- a/src/renderer/src/components/native-chat/NativeChatStructuredSession.tsx +++ b/src/renderer/src/components/native-chat/NativeChatStructuredSession.tsx @@ -311,6 +311,7 @@ export function NativeChatStructuredSession( settledTasks={controller.backgroundTasks.settledTasks} indicatorActive={controller.backgroundTasks.isMonitoring} supportsTaskStop={controller.backgroundTasks.supportsStop} + supportsStopAll={controller.backgroundTasks.supportsStopAll} stoppingTaskIds={activeStoppingBackgroundTasks?.taskIds ?? NO_STOPPING_TASKS} stoppingAll={activeStoppingBackgroundTasks?.all ?? false} onStop={(taskId) => { diff --git a/src/renderer/src/components/native-chat/NativeChatTaskList.test.tsx b/src/renderer/src/components/native-chat/NativeChatTaskList.test.tsx new file mode 100644 index 00000000000..588f392f713 --- /dev/null +++ b/src/renderer/src/components/native-chat/NativeChatTaskList.test.tsx @@ -0,0 +1,75 @@ +// @vitest-environment happy-dom +import '@testing-library/jest-dom/vitest' +import { cleanup, fireEvent, render, screen } from '@testing-library/react' +import { afterEach, describe, expect, it } from 'vitest' +import { NativeChatTaskList } from './NativeChatTaskList' +import type { NativeChatTaskList as TaskList } from '../../../../shared/native-chat-task-list' + +afterEach(cleanup) +const previous: TaskList = { + tasks: [ + { content: 'Read', status: 'in_progress', activeForm: 'Reading' }, + { content: 'Write', status: 'pending', activeForm: 'Writing' }, + { content: 'Test', status: 'pending' } + ] +} +const current: TaskList = { + tasks: [ + { content: 'Read', status: 'completed', activeForm: 'Reading' }, + { content: 'Write', status: 'in_progress', activeForm: 'Writing' }, + { content: 'Test', status: 'pending' } + ] +} + +describe('NativeChatTaskList', () => { + it('shows tri-state glyphs, progress, and activeForm in the first checklist', () => { + const { container } = render() + expect(screen.getByText('Read')).toHaveClass('line-through') + expect(screen.getByText('Writing').closest('li')).toHaveClass('text-foreground') + expect(screen.getByText('Test')).toBeInTheDocument() + expect(screen.getByLabelText('1 of 3 tasks completed')).toHaveTextContent('1/3') + for (const glyph of ['circle', 'circle-dot', 'circle-check']) { + expect(container.querySelector(`.lucide-${glyph}`)).not.toBeNull() + } + expect(screen.getByText('In progress:')).toHaveClass('sr-only') + }) + + it('leads with the diff and expands the complete checklist on demand', () => { + render() + expect(screen.getByText('Completed Read')).toBeInTheDocument() + expect(screen.getByText('Started Write')).toBeInTheDocument() + expect(screen.queryByText('Test')).toBeNull() + const disclosure = screen.getByRole('button', { name: 'Full task list' }) + expect(disclosure).toHaveAttribute('aria-expanded', 'false') + fireEvent.click(disclosure) + expect(disclosure).toHaveAttribute('aria-expanded', 'true') + expect(screen.getByText('Writing')).toBeInTheDocument() + expect(screen.getByText('Test')).toBeInTheDocument() + }) + + it('shows unchanged feedback and the current explanation', () => { + render( + + ) + expect(screen.getByText('Tasks unchanged')).toBeInTheDocument() + expect(screen.getByText('Continuing verification')).toBeInTheDocument() + expect(screen.queryByText('Test')).toBeNull() + }) + + it('renders empty lists without claiming any task completed', () => { + render() + expect(screen.getByText('No tasks')).toBeInTheDocument() + expect(screen.getByLabelText('0 of 0 tasks completed')).toHaveTextContent('0/0') + }) + + it('switches from full list to diff when earlier history supplies a predecessor', () => { + const { rerender } = render() + expect(screen.getByText('Test')).toBeInTheDocument() + rerender() + expect(screen.queryByText('Test')).toBeNull() + expect(screen.getByText('Started Write')).toBeInTheDocument() + }) +}) diff --git a/src/renderer/src/components/native-chat/NativeChatTaskList.tsx b/src/renderer/src/components/native-chat/NativeChatTaskList.tsx new file mode 100644 index 00000000000..3ddaa8959e8 --- /dev/null +++ b/src/renderer/src/components/native-chat/NativeChatTaskList.tsx @@ -0,0 +1,183 @@ +import { Circle, CircleCheck, CircleDot, ChevronRight, ListChecks } from 'lucide-react' +import { Collapsible, CollapsibleContent, CollapsibleTrigger } from '@/components/ui/collapsible' +import { cn } from '@/lib/utils' +import { translate } from '@/i18n/i18n' +import { + diffNativeChatTaskLists, + nativeChatTaskLabel, + type NativeChatTask, + type NativeChatTaskChange, + type NativeChatTaskList as TaskList +} from '../../../../shared/native-chat-task-list' + +function statusLabel(task: NativeChatTask): string { + if (task.status === 'completed') { + return translate('components.native-chat.taskList.completed', 'Completed') + } + if (task.status === 'in_progress') { + return translate('components.native-chat.taskList.inProgress', 'In progress') + } + return translate('components.native-chat.taskList.pending', 'Pending') +} + +function changeLabel(change: NativeChatTaskChange): string { + const values = { task: change.task.content } + switch (change.kind) { + case 'added': + return translate('components.native-chat.taskList.added', 'Added {{task}}', values) + case 'removed': + return translate('components.native-chat.taskList.removed', 'Removed {{task}}', values) + case 'started': + return translate('components.native-chat.taskList.started', 'Started {{task}}', values) + case 'completed': + return translate('components.native-chat.taskList.finished', 'Completed {{task}}', values) + case 'pending': + return translate('components.native-chat.taskList.reset', 'Marked pending: {{task}}', values) + case 'updated': + return translate('components.native-chat.taskList.updated', 'Updated {{task}}', { + task: nativeChatTaskLabel(change.task) + }) + } +} + +function TaskRow({ task, label }: { task: NativeChatTask; label?: string }): React.JSX.Element { + const Icon = + task.status === 'completed' ? CircleCheck : task.status === 'in_progress' ? CircleDot : Circle + return ( +
  • + + {statusLabel(task)}: + + {label ?? nativeChatTaskLabel(task)} + +
    • + ) +} + +function Checklist({ list }: { list: TaskList }): React.JSX.Element { + return list.tasks.length === 0 ? ( +

    + {translate('components.native-chat.taskList.empty', 'No tasks')} +

    + ) : ( +
      + {list.tasks.map((task, index) => ( + + ))} +
    + ) +} + +export function NativeChatTaskList({ + list, + previous, + presentation = 'inline' +}: { + list: TaskList + previous?: TaskList + presentation?: 'inline' | 'composer' +}): React.JSX.Element { + const completed = list.tasks.filter((task) => task.status === 'completed').length + if (presentation === 'composer') { + return ( + + + + + {translate('components.native-chat.taskList.title', 'Tasks')} + + + {completed}/{list.tasks.length} + + + + +
    + + {list.explanation ? ( +

    + {list.explanation} +

    + ) : null} +
    +     +     + ) + } + const changes = previous ? diffNativeChatTaskLists(previous, list) : null + return ( +
    +
    + + + {translate('components.native-chat.taskList.title', 'Tasks')} + + + {completed}/{list.tasks.length} + +
    + {changes ? ( + <> + {changes.length > 0 ? ( +
      + {changes.map((change, index) => ( + + ))} +
    + ) : ( +

    + {translate('components.native-chat.taskList.unchanged', 'Tasks unchanged')} +

    + )} + + + + {translate('components.native-chat.taskList.showAll', 'Full task list')} + + + + + + + ) : ( + + )} + {list.explanation ? ( +

    + {list.explanation} +

    + ) : null} +
    + ) +} diff --git a/src/renderer/src/components/native-chat/NativeChatToolRun.test.tsx b/src/renderer/src/components/native-chat/NativeChatToolRun.test.tsx index cd819c5ced1..d5cf4ceb1eb 100644 --- a/src/renderer/src/components/native-chat/NativeChatToolRun.test.tsx +++ b/src/renderer/src/components/native-chat/NativeChatToolRun.test.tsx @@ -486,18 +486,28 @@ describe('NativeChatToolRun', () => { expect(runHeader(container)).toHaveTextContent('shell git log -1') }) - it('settles an orphaned running call when its turn lifecycle has ended', () => { + it('keeps a post-turn running call neutral until the item itself settles', () => { const blocks: NativeChatBlock[] = [ { type: 'tool-call', name: 'shell', input: { command: 'sleep 1' }, state: 'running' } ] - const { container } = render( + const { container, rerender } = render( ) expect(screen.queryByText('Running sleep 1')).toBeNull() - expect(container.querySelector('.lucide-check')).toBeInTheDocument() + expect(container.querySelector('.lucide-check')).toBeNull() expect(container.querySelector('.lucide-circle-alert')).toBeNull() + rerender( + + ) + expect(container.querySelector('.lucide-check')).toBeInTheDocument() }) it('shows the category glyph beside the word a classified row is named by', () => { @@ -730,3 +740,58 @@ describe('NativeChatToolRun', () => { expect(screen.getByTitle('ls')).toHaveTextContent('ls') }) }) + +describe('NativeChatToolRun task lists', () => { + it('renders task updates instead of JSON and consumes successful results', () => { + const blocks: NativeChatBlock[] = [ + { + type: 'tool-call', + name: 'update_plan', + input: { + plan: [ + { step: 'Read', status: 'in_progress' }, + { step: 'Test', status: 'pending' } + ] + } + }, + { type: 'tool-result', output: 'Plan updated' }, + { + type: 'tool-call', + name: 'update_plan', + input: { + plan: [ + { step: 'Read', status: 'completed' }, + { step: 'Test', status: 'in_progress' } + ] + } + } + ] + const { container } = render() + expect(screen.getByText('Completed Read')).toBeInTheDocument() + expect(screen.getByText('Started Test')).toBeInTheDocument() + expect(screen.getByText('1/2')).toBeInTheDocument() + expect(screen.queryByText('Plan updated')).toBeNull() + expect(container.querySelector('pre')).toBeNull() + }) + + it('keeps malformed calls and failed results visible in the generic view', () => { + render( + + ) + expect(screen.getByText('Invalid arguments', { selector: 'pre' })).toBeInTheDocument() + expect(screen.getByText('Update rejected', { selector: 'pre' })).toBeInTheDocument() + expect(screen.queryByText('1/1')).toBeNull() + }) +}) diff --git a/src/renderer/src/components/native-chat/NativeChatToolRun.tsx b/src/renderer/src/components/native-chat/NativeChatToolRun.tsx index 26ff8f40d6e..ad68b79e2d4 100644 --- a/src/renderer/src/components/native-chat/NativeChatToolRun.tsx +++ b/src/renderer/src/components/native-chat/NativeChatToolRun.tsx @@ -12,7 +12,8 @@ import { isToolCallBlock, isToolResultBlock, type NativeChatBlock, - type NativeChatSubagentGroupBlock + type NativeChatSubagentGroupBlock, + type NativeChatToolCallBlock } from '../../../../shared/native-chat-types' import { isRenderableSubagentGroup } from '../../../../shared/native-chat-subagent-summary' import { diffFromText, diffFromToolCall, type DiffLine } from './native-chat-diff' @@ -31,6 +32,8 @@ import { selectActiveToolCall } from '../../../../shared/native-chat-tool-activity' import { nativeChatToolRunIconName } from '../../../../shared/native-chat-tool-icon' +import { NativeChatTaskList } from './NativeChatTaskList' +import { buildNativeChatTaskListRows } from './native-chat-task-list-history' import { NativeChatDiffView } from './NativeChatDiffView' import { NativeChatSubagentRun } from './NativeChatSubagentRun' import { NativeChatToolIcon, NativeChatToolRunIcon } from './NativeChatToolIcon' @@ -158,6 +161,8 @@ function ToolLine({ * toolbar toggle drive every run at once while still allowing per-run override. */ export function NativeChatToolRun({ blocks, + previousTodoWrite, + previousUpdatePlan, revealedDiff, onRevealDiff, subagentGroups = NO_SUBAGENT_GROUPS, @@ -168,6 +173,8 @@ export function NativeChatToolRun({ onLinkClick }: { blocks: NativeChatBlock[] + previousTodoWrite?: NativeChatToolCallBlock + previousUpdatePlan?: NativeChatToolCallBlock revealedDiff?: NativeChatDiffReveal onRevealDiff?: (element: HTMLElement) => void /** Spawn-group rosters that belong with this run's activity, one row each. */ @@ -228,9 +235,22 @@ export function NativeChatToolRun({ ? selectActiveToolCall(blocks, { activeTurnIsWorking }) : null const isSettled = latestActiveCall == null + const hasRunningCall = blocks.some((block) => isToolCallBlock(block) && block.state === 'running') // The turn caret opens the activity group, while each child tool remains // collapsed. The global expand toolbar still opens child details together. const expandToolLines = expandOverride === undefined ? open : false + // Diffing every edit is the run's most expensive work, so a collapsed run — + // which renders none of it — never pays for it. + const taskLists = useMemo( + () => + open + ? buildNativeChatTaskListRows(blocks, { + todowrite: previousTodoWrite, + update_plan: previousUpdatePlan + }) + : null, + [open, blocks, previousTodoWrite, previousUpdatePlan] + ) // Rollups cache counts only; detailed diff rows are built when the run opens. const { editCards, consumedResults } = useMemo( () => (open ? buildEditCards(blocks) : NO_EDIT_CARDS), @@ -361,8 +381,8 @@ export function NativeChatToolRun({ {fallbackLabel} )} - {/* Completion reads as a trailing mark so the leading glyph can stay fixed. */} - {structuredActivityUi ? ( + {/* A running item cannot inherit completion from its turn. */} + {structuredActivityUi && !hasRunningCall ? ( ) : null} {/* Chevron is revealed on hover when collapsed and points down when open. */} @@ -381,7 +401,14 @@ export function NativeChatToolRun({
    {(() => { const seen = new Map() - return blocks.map((block) => { + return blocks.map((block, blockIndex) => { + const taskList = taskLists?.rows.get(block) + if (taskList) { + return + } + if (taskLists?.consumedResults.has(block)) { + return null + } const edit = editCards.get(block) if (edit) { return ( diff --git a/src/renderer/src/components/native-chat/native-chat-task-list-frames.ts b/src/renderer/src/components/native-chat/native-chat-task-list-frames.ts new file mode 100644 index 00000000000..f58c4a2bbdd --- /dev/null +++ b/src/renderer/src/components/native-chat/native-chat-task-list-frames.ts @@ -0,0 +1,36 @@ +import { normalizeNativeChatTaskList } from '../../../../shared/native-chat-task-list' +import type { NativeChatMessage } from '../../../../shared/native-chat-types' + +const projectedFrames = new WeakMap() + +/** Project after tool folding so a notification never takes another call's result. */ +export function projectNativeChatTaskListFrames( + messages: readonly NativeChatMessage[] +): NativeChatMessage[] { + return messages.map((message) => { + const cached = projectedFrames.get(message) + if (cached) { + return cached + } + const block = message.blocks.length === 1 ? message.blocks[0] : undefined + const frame = block?.type === 'text' ? block.providerFrame : undefined + if ( + message.role !== 'system' || + frame?.provider !== 'codex' || + frame.kind !== 'notification:turn/plan/updated' || + frame.payload.truncated || + !normalizeNativeChatTaskList('update_plan', frame.payload.head) + ) { + return message + } + const projected: NativeChatMessage = { + ...message, + role: 'assistant', + blocks: [ + { type: 'tool-call', name: 'update_plan', input: frame.payload.head, state: 'completed' } + ] + } + projectedFrames.set(message, projected) + return projected + }) +} diff --git a/src/renderer/src/components/native-chat/native-chat-task-list-history.test.ts b/src/renderer/src/components/native-chat/native-chat-task-list-history.test.ts new file mode 100644 index 00000000000..ef8eae291ad --- /dev/null +++ b/src/renderer/src/components/native-chat/native-chat-task-list-history.test.ts @@ -0,0 +1,114 @@ +import { describe, expect, it } from 'vitest' +import type { + NativeChatBlock, + NativeChatMessage, + NativeChatToolCallBlock +} from '../../../../shared/native-chat-types' +import { + buildNativeChatTaskListRows, + nativeChatTaskListPredecessors +} from './native-chat-task-list-history' + +function call(name = 'TodoWrite', status = 'pending'): NativeChatToolCallBlock { + return { + type: 'tool-call', + name, + input: + name === 'TodoWrite' + ? { todos: [{ content: 'Test', status }] } + : { plan: [{ step: 'Test', status }] } + } +} +function message( + id: string, + blocks: NativeChatBlock[], + role: NativeChatMessage['role'] = 'assistant' +): NativeChatMessage { + return { id, blocks, role, timestamp: 1, source: 'transcript' } +} + +describe('native chat task list history', () => { + it('carries predecessors across prose, ordinary tools, and user turns', () => { + const first = call() + const next = call('TodoWrite', 'completed') + const history = nativeChatTaskListPredecessors([ + message('a', [first]), + message('b', [{ type: 'text', text: 'Continue' }], 'user'), + message('c', [{ type: 'tool-call', name: 'Read', input: {} }]), + message('d', [next]) + ]) + expect(history.get('d')?.todowrite).toBe(first) + expect( + buildNativeChatTaskListRows([next], history.get('d')).rows.get(next)?.previous?.tasks[0] + .status + ).toBe('pending') + }) + + it('keeps interleaved tool families separate and ignores MCP lookalikes', () => { + const claude = call() + const codex = call('update_plan') + const next = call('TodoWrite', 'completed') + const model = buildNativeChatTaskListRows([claude, codex, call('mcp__x__TodoWrite'), next]) + expect(model.rows.get(codex)?.previous).toBeUndefined() + expect(model.rows.get(next)?.previous).toEqual(model.rows.get(claude)?.list) + const history = nativeChatTaskListPredecessors([ + message('a', [claude]), + message('b', [codex]), + message('c', [next]) + ]) + expect(history.get('c')).toEqual({ todowrite: claude, update_plan: codex }) + }) + + it('skips failed and malformed calls and keeps errors unconsumed', () => { + const first = call() + const failed = { ...call(), state: 'failed' as const } + const rejected = call('TodoWrite', 'completed') + const error: NativeChatBlock = { type: 'tool-result', output: 'Rejected', isError: true } + const next = call('TodoWrite', 'in_progress') + const blocks: NativeChatBlock[] = [ + first, + { type: 'tool-result', output: 'ok' }, + failed, + { type: 'tool-result', output: 'failed' }, + rejected, + error, + { ...call(), input: '{' }, + next + ] + const model = buildNativeChatTaskListRows(blocks) + expect(model.rows.has(failed)).toBe(false) + expect(model.rows.has(rejected)).toBe(false) + expect(model.consumedResults.has(error)).toBe(false) + expect(model.rows.get(next)?.previous).toEqual(model.rows.get(first)?.list) + const history = nativeChatTaskListPredecessors([ + message('a', blocks.slice(0, -1)), + message('b', [next]) + ]) + expect(history.get('b')?.todowrite).toBe(first) + }) + + it('updates predecessor identity after pagination and remains stable on rerender', () => { + const first = call() + const second = call('TodoWrite', 'in_progress') + const tail = message('b', [second]) + expect(nativeChatTaskListPredecessors([tail]).get('b')?.todowrite).toBeUndefined() + const history = nativeChatTaskListPredecessors([message('a', [first]), tail]) + expect(history.get('b')?.todowrite).toBe(first) + expect(nativeChatTaskListPredecessors([message('a', [first]), tail]).get('b')?.todowrite).toBe( + history.get('b')?.todowrite + ) + expect(nativeChatTaskListPredecessors([tail]).get('b')?.todowrite).toBeUndefined() + }) + + it('diffs a running call before its result arrives and consumes a successful result', () => { + const first = call() + const running = { ...call('TodoWrite', 'in_progress'), state: 'running' as const } + const result: NativeChatBlock = { type: 'tool-result', output: 'ok' } + const model = buildNativeChatTaskListRows([running, result], { + todowrite: first, + update_plan: undefined + }) + expect(model.rows.get(running)?.previous).toBeDefined() + expect(model.consumedResults.has(result)).toBe(true) + }) +}) diff --git a/src/renderer/src/components/native-chat/native-chat-task-list-history.ts b/src/renderer/src/components/native-chat/native-chat-task-list-history.ts new file mode 100644 index 00000000000..62582e2a7cf --- /dev/null +++ b/src/renderer/src/components/native-chat/native-chat-task-list-history.ts @@ -0,0 +1,83 @@ +import { + nativeChatTaskListTool, + normalizeNativeChatTaskList, + type NativeChatTaskList, + type NativeChatTaskListTool +} from '../../../../shared/native-chat-task-list' +import type { + NativeChatBlock, + NativeChatMessage, + NativeChatToolCallBlock +} from '../../../../shared/native-chat-types' +import { pairToolBlocks } from './native-chat-tool-fold' + +export type NativeChatTaskListPredecessors = Partial< + Record +> +export type NativeChatTaskListRow = { list: NativeChatTaskList; previous?: NativeChatTaskList } + +function taskListFromCall(call: NativeChatToolCallBlock): NativeChatTaskList | null { + return call.state === 'failed' ? null : normalizeNativeChatTaskList(call.name, call.input) +} + +/** Store call identities so unchanged rows stay memoized, while prepends replace their context. */ +export function nativeChatTaskListPredecessors( + messages: readonly NativeChatMessage[] +): Map { + const history = new Map() + const previous: NativeChatTaskListPredecessors = {} + for (const message of messages) { + history.set(message.id, { ...previous }) + if (message.role === 'user') { + continue + } + for (const { call, result } of pairToolBlocks(message.blocks)) { + if (!call || result?.isError) { + continue + } + const tool = nativeChatTaskListTool(call.name) + if (tool && taskListFromCall(call)) { + previous[tool] = call + } + } + } + return history +} + +export function buildNativeChatTaskListRows( + blocks: readonly NativeChatBlock[], + predecessors: NativeChatTaskListPredecessors = {} +): { + rows: Map + consumedResults: Set +} { + const rows = new Map() + const consumedResults = new Set() + const previous = new Map() + for (const call of Object.values(predecessors)) { + if (!call) { + continue + } + const tool = nativeChatTaskListTool(call.name) + const list = taskListFromCall(call) + if (tool && list) { + previous.set(tool, list) + } + } + for (const { call, result } of pairToolBlocks(blocks)) { + if (!call || result?.isError) { + continue + } + const tool = nativeChatTaskListTool(call.name) + const list = taskListFromCall(call) + if (!tool || !list) { + continue + } + rows.set(call, { list, previous: previous.get(tool) }) + previous.set(tool, list) + if (result) { + consumedResults.add(result) + } + } + return { rows, consumedResults } +} diff --git a/src/renderer/src/components/native-chat/native-chat-task-list-state.test.ts b/src/renderer/src/components/native-chat/native-chat-task-list-state.test.ts new file mode 100644 index 00000000000..c282c4d4532 --- /dev/null +++ b/src/renderer/src/components/native-chat/native-chat-task-list-state.test.ts @@ -0,0 +1,73 @@ +import { describe, expect, it } from 'vitest' +import type { NativeChatBlock, NativeChatMessage } from '../../../../shared/native-chat-types' +import { nativeChatTaskListState } from './native-chat-task-list-state' + +function message(id: string, blocks: NativeChatBlock[]): NativeChatMessage { + return { id, role: 'assistant', timestamp: 1, source: 'transcript', blocks } +} +function call(content: string, status = 'pending'): NativeChatBlock { + return { type: 'tool-call', name: 'TodoWrite', input: { todos: [{ content, status }] } } +} + +describe('nativeChatTaskListState', () => { + it('projects one latest snapshot, preserves prose and leaves source messages unchanged', () => { + const first = message('first', [call('Read')]) + const last = message('last', [ + { type: 'text', text: 'Here is the result' }, + call('Read', 'completed'), + { type: 'tool-result', output: 'Updated todos' } + ]) + const result = nativeChatTaskListState([first, last]) + expect(result.list?.tasks).toEqual([{ content: 'Read', status: 'completed' }]) + expect(result.messages[0]).toBe(first) + expect(result.messages[1]).toBe(last) + expect(first.blocks).toHaveLength(1) + expect(last.blocks).toHaveLength(3) + expect(nativeChatTaskListState([first, last]).messages[1]).toBe(result.messages[1]) + }) + + it('preserves latest state across user follow-ups and clears it on an explicit empty list', () => { + const first = message('first', [call('Read')]) + const user = { ...message('user', [{ type: 'text', text: 'Continue' }]), role: 'user' as const } + const empty = message('empty', [{ type: 'tool-call', name: 'TodoWrite', input: { todos: [] } }]) + expect(nativeChatTaskListState([first, user]).list?.tasks).toHaveLength(1) + expect(nativeChatTaskListState([first, user, empty]).list?.tasks).toEqual([]) + expect(nativeChatTaskListState([]).list).toBeNull() + }) + + it('does not replace valid state with malformed or failed calls, and retains their diagnostics', () => { + const first = message('first', [call('Read')]) + const malformed = message('malformed', [ + { type: 'tool-call', name: 'TodoWrite', input: '{' }, + { type: 'tool-result', output: 'Invalid arguments', isError: true } + ]) + const failed = message('failed', [ + call('Wrong', 'completed'), + { type: 'tool-result', output: 'Update rejected', isError: true } + ]) + const failedCall = message('failed-call', [ + { type: 'tool-call', name: 'TodoWrite', state: 'failed', input: { todos: [] } } + ]) + const result = nativeChatTaskListState([first, malformed, failed, failedCall]) + expect(result.list?.tasks[0].content).toBe('Read') + expect(result.messages.slice(1)).toEqual([malformed, failed, failedCall]) + }) + + it('retains task history and unrelated errors while selecting the paired snapshot', () => { + const tasks: NativeChatBlock = call('Read') + const shell: NativeChatBlock = { + type: 'tool-call', + name: 'shell', + input: {} + } + const error: NativeChatBlock = { + type: 'tool-result', + output: 'Failed', + isError: true + } + const success: NativeChatBlock = { type: 'tool-result', output: 'Updated' } + const result = nativeChatTaskListState([message('mixed', [tasks, success, shell, error])]) + expect(result.list?.tasks[0].content).toBe('Read') + expect(result.messages[0].blocks).toEqual([tasks, success, shell, error]) + }) +}) diff --git a/src/renderer/src/components/native-chat/native-chat-task-list-state.ts b/src/renderer/src/components/native-chat/native-chat-task-list-state.ts new file mode 100644 index 00000000000..f45d6531600 --- /dev/null +++ b/src/renderer/src/components/native-chat/native-chat-task-list-state.ts @@ -0,0 +1,43 @@ +import { + normalizeNativeChatTaskList, + type NativeChatTaskList +} from '../../../../shared/native-chat-task-list' +import type { NativeChatMessage } from '../../../../shared/native-chat-types' +import { pairToolBlocks } from './native-chat-tool-fold' + +const snapshots = new WeakMap() + +function latestSnapshot(message: NativeChatMessage): NativeChatTaskList | null { + if (snapshots.has(message)) { + return snapshots.get(message) ?? null + } + let list: NativeChatTaskList | null = null + if (message.role === 'assistant') { + for (const { call, result } of pairToolBlocks(message.blocks)) { + if (!call || call.state === 'failed' || result?.isError) { + continue + } + const snapshot = normalizeNativeChatTaskList(call.name, call.input) + if (snapshot) { + list = snapshot + } + } + } + snapshots.set(message, list) + return list +} + +/** Select composer progress without consuming historical transcript updates. */ +export function nativeChatTaskListState(messages: readonly NativeChatMessage[]): { + messages: readonly NativeChatMessage[] + list: NativeChatTaskList | null +} { + let list: NativeChatTaskList | null = null + for (const message of messages) { + const snapshot = latestSnapshot(message) + if (snapshot) { + list = snapshot + } + } + return { messages, list } +} diff --git a/src/renderer/src/components/native-chat/structured-session-background-tasks-view.ts b/src/renderer/src/components/native-chat/structured-session-background-tasks-view.ts index 0652129502a..3765d7daf6a 100644 --- a/src/renderer/src/components/native-chat/structured-session-background-tasks-view.ts +++ b/src/renderer/src/components/native-chat/structured-session-background-tasks-view.ts @@ -1,3 +1,9 @@ +// The background-tasks strip's view of one session's wire state. +// +// The strip stands for work that OUTLIVES a turn. It stays mounted through a +// running turn — a fan-out's children keep reporting long after the parent +// settles — but only an idle session lets it animate or speak for itself. + import type { AgentSessionBackgroundTask, AgentSessionBackgroundTaskState @@ -12,6 +18,7 @@ export type StructuredSessionBackgroundTasksView = { tasks: AgentSessionBackgroundTask[] settledTasks: AgentSessionBackgroundTask[] supportsStop: boolean + supportsStopAll: boolean } export function structuredSessionBackgroundTasksView( @@ -24,6 +31,9 @@ export function structuredSessionBackgroundTasksView( isMonitoring: turnId === null && monitoring, tasks: backgroundTasks?.tasks ?? [], settledTasks: backgroundTasks?.settledTasks ?? [], - supportsStop: backgroundTasks?.supportsTaskStop === true + supportsStop: backgroundTasks?.supportsTaskStop === true, + // Absent means the host predates the field and does accept an untargeted + // stop; only a host that says `false` has none to offer. + supportsStopAll: backgroundTasks?.supportsStopAll !== false } } diff --git a/src/renderer/src/components/native-chat/use-structured-agent-session-mutate.ts b/src/renderer/src/components/native-chat/use-structured-agent-session-mutate.ts new file mode 100644 index 00000000000..6f071e1a16b --- /dev/null +++ b/src/renderer/src/components/native-chat/use-structured-agent-session-mutate.ts @@ -0,0 +1,96 @@ +// One structured-session mutation, fenced and idempotent. +// +// The client operation id is keyed on (session, method, payload) so a retry of +// the same request reuses it and the host upserts one row instead of two, and +// every result is discarded unless the runtime fence it was issued against is +// still the current one. + +import { useCallback, useRef, useState } from 'react' +import * as conversationCommands from './structured-conversation-command-send' +import type { AgentSessionMutationResult } from '../../../../shared/agent-session-wire' +import { agentSessionRefusalOperationState } from '../../../../shared/agent-session-refusal-retry' +import { structuredAgentSessionPayloadFingerprint } from '../../../../shared/structured-agent-session-mutation' +import type { RuntimeClientTarget } from '@/runtime/runtime-rpc-client' +import { callStructuredAgentSession } from '@/runtime/structured-agent-session-client' +import { structuredSessionOperationId } from './use-structured-agent-session-outbox' + +export type StructuredAgentSessionMutate = ( + method: string, + fingerprintMethod: string, + fields: Record, + operationIdOverride?: string | null +) => Promise + +export function useStructuredAgentSessionMutate(args: { + sessionId: string + target: RuntimeClientTarget + /** Read at settle time, not at call time: the fence can move while a request + * is in flight, and a result from the previous fence is not this session's. */ + stateRef: { current: { fence: number | null } } +}): { mutate: StructuredAgentSessionMutate; writeError: string | null } { + const { sessionId, stateRef, target } = args + const [writeError, setWriteError] = useState(null) + const operationIds = useRef(new Map()) + + const mutate = useCallback( + async ( + method: string, + fingerprintMethod: string, + fields: Record, + operationIdOverride?: string | null + ): Promise => { + if (stateRef.current.fence === null) { + return null + } + const targetFence = stateRef.current.fence + const key = `${sessionId}:${fingerprintMethod}:${JSON.stringify(fields)}` + const clientOperationId = + operationIdOverride ?? operationIds.current.get(key) ?? structuredSessionOperationId() + operationIds.current.set(key, clientOperationId) + let result: AgentSessionMutationResult + try { + result = await callStructuredAgentSession>(target, method, { + envelope: { + sessionId, + clientOperationId, + expectedRuntimeFence: targetFence, + payloadFingerprint: structuredAgentSessionPayloadFingerprint({ + method: fingerprintMethod, + sessionId, + fields + }) + }, + ...fields + }) + } catch (error) { + if (stateRef.current.fence === targetFence) { + setWriteError(error instanceof Error ? error.message : 'Request was not sent') + } + return null + } + if (!result.ok) { + if ( + agentSessionRefusalOperationState(fingerprintMethod, result.refusal.code) === + 'settled-rejected' + ) { + operationIds.current.delete(key) + } + if (stateRef.current.fence === targetFence) { + setWriteError(result.refusal.message) + } + return null + } + if (stateRef.current.fence !== targetFence) { + return null + } + if (!conversationCommands.isUnconfirmedConversationCommand(fingerprintMethod, result.value)) { + operationIds.current.delete(key) + } + setWriteError(null) + return result.value + }, + [sessionId, stateRef, target] + ) + + return { mutate, writeError } +} 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 e08250f99e7..c05a564c098 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 @@ -1,20 +1,19 @@ -import * as conversationCommands from './structured-conversation-command-send' import { useCallback, useEffect, useMemo, useRef, useState } from 'react' +import * as conversationCommands from './structured-conversation-command-send' +import type { + AgentSessionOptionResult, + AgentSessionOptionsResult, + AgentSessionPromptResult +} from '../../../../shared/agent-session-wire' +import { useStructuredAgentSessionOutbox } from './use-structured-agent-session-outbox' +import { useStructuredAgentSessionMutate } from './use-structured-agent-session-mutate' import type { AgentSessionConversationCommand, AgentSessionConversationCommandResult } from '../../../../shared/agent-session-conversation-command' import type { AgentType } from '../../../../shared/agent-status-types' -import type { - AgentSessionMutationResult, - AgentSessionOptionResult, - AgentSessionOptionsResult, - AgentSessionPromptResult -} from '../../../../shared/agent-session-wire' import { getAgentSessionOptionCatalog } from '../../../../shared/agent-session-option-catalog' import type { SessionOptionsSurface } from '../../../../shared/native-chat-session-options' -import { agentSessionRefusalOperationState } from '../../../../shared/agent-session-refusal-retry' -import { structuredAgentSessionPayloadFingerprint } from '../../../../shared/structured-agent-session-mutation' import { applyStructuredAgentSessionOptions, canSetStructuredAgentSessionOption, @@ -26,19 +25,15 @@ import { import { activeStructuredAgentSessionTurnId } from '../../../../shared/structured-agent-session-projection' import type { RuntimeClientTarget } from '@/runtime/runtime-rpc-client' import { callStructuredAgentSession } from '@/runtime/structured-agent-session-client' -import { - structuredSessionOperationId, - useStructuredAgentSessionOutbox -} from './use-structured-agent-session-outbox' import { useStructuredAgentSessionHold } from './use-structured-agent-session-hold' import { useStructuredAgentSessionRead } from './use-structured-agent-session-read' import { pendingStructuredSessionPrompts, type StructuredPromptItem } from './structured-agent-session-message-projection' +import { structuredSessionBackgroundTasksView } from './structured-session-background-tasks-view' import { useStructuredAgentSessionMessages } from './use-structured-agent-session-messages' import { selectStructuredAgentTurnActivity } from './native-chat-turn-activity' -import { structuredSessionBackgroundTasksView } from './structured-session-background-tasks-view' import { enqueueSessionOptionSettingsWrite } from './native-chat-session-option-settings-write' export type { StructuredPromptItem } from './structured-agent-session-message-projection' @@ -55,8 +50,7 @@ export function useStructuredAgentSession(args: { useStructuredAgentSessionHold({ sessionId, target, surface: 'desktop-chat', enabled: isVisible }) const { state, loadingOlder, loadOlder } = useStructuredAgentSessionRead(args) const stateRef = useRef(state) - const [writeError, setWriteError] = useState(null) - const operationIds = useRef(new Map()) + const { mutate, writeError } = useStructuredAgentSessionMutate({ sessionId, target, stateRef }) const [conversationSupport, setConversationSupport] = useState<{ sessionId: string commands: readonly AgentSessionConversationCommand[] @@ -84,66 +78,6 @@ export function useStructuredAgentSession(args: { setOptionState(next) }, [agent, sessionId, state.fence]) - const mutate = useCallback( - async ( - method: string, - fingerprintMethod: string, - fields: Record, - operationIdOverride?: string | null - ): Promise => { - if (stateRef.current.fence === null) { - return null - } - const targetFence = stateRef.current.fence - const key = `${sessionId}:${fingerprintMethod}:${JSON.stringify(fields)}` - const clientOperationId = - operationIdOverride ?? operationIds.current.get(key) ?? structuredSessionOperationId() - operationIds.current.set(key, clientOperationId) - let result: AgentSessionMutationResult - try { - result = await callStructuredAgentSession>(target, method, { - envelope: { - sessionId, - clientOperationId, - expectedRuntimeFence: targetFence, - payloadFingerprint: structuredAgentSessionPayloadFingerprint({ - method: fingerprintMethod, - sessionId, - fields - }) - }, - ...fields - }) - } catch (error) { - if (stateRef.current.fence === targetFence) { - setWriteError(error instanceof Error ? error.message : 'Request was not sent') - } - return null - } - if (!result.ok) { - if ( - agentSessionRefusalOperationState(fingerprintMethod, result.refusal.code) === - 'settled-rejected' - ) { - operationIds.current.delete(key) - } - if (stateRef.current.fence === targetFence) { - setWriteError(result.refusal.message) - } - return null - } - if (stateRef.current.fence !== targetFence) { - return null - } - if (!conversationCommands.isUnconfirmedConversationCommand(fingerprintMethod, result.value)) { - operationIds.current.delete(key) - } - setWriteError(null) - return result.value - }, - [sessionId, target] - ) - // Refresh options each turn to confirm which model the provider actually selected. const turnId = activeStructuredAgentSessionTurnId(state.items) const turnActivity = useMemo( diff --git a/src/renderer/src/i18n/locales/en.json b/src/renderer/src/i18n/locales/en.json index f5990e50829..bfedd199022 100644 --- a/src/renderer/src/i18n/locales/en.json +++ b/src/renderer/src/i18n/locales/en.json @@ -16994,6 +16994,22 @@ "empty": "No users found" }, "native-chat": { + "taskList": { + "title": "Tasks", + "completed": "Completed", + "inProgress": "In progress", + "pending": "Pending", + "empty": "No tasks", + "progress": "{{completed}} of {{total}} tasks completed", + "added": "Added {{task}}", + "removed": "Removed {{task}}", + "started": "Started {{task}}", + "finished": "Completed {{task}}", + "reset": "Marked pending: {{task}}", + "updated": "Updated {{task}}", + "unchanged": "Tasks unchanged", + "showAll": "Full task list" + }, "turnDiff": { "one": "1 changed file", "many": "{{count}} changed files", diff --git a/src/renderer/src/store/slices/folder-workspace-activation-and-activity.test.ts b/src/renderer/src/store/slices/folder-workspace-activation-and-activity.test.ts index 35e9397c1a5..405007067d7 100644 --- a/src/renderer/src/store/slices/folder-workspace-activation-and-activity.test.ts +++ b/src/renderer/src/store/slices/folder-workspace-activation-and-activity.test.ts @@ -47,6 +47,29 @@ function makeFolderWorkspace(overrides: Partial = {}): FolderWo } } +function rememberedBrowserSurface(workspaceKey: string): Partial { + return { + browserTabsByWorktree: { + [workspaceKey]: [ + { + id: 'remembered', + worktreeId: workspaceKey, + url: 'about:blank', + title: 'Browser', + loading: false, + faviconUrl: null, + canGoBack: false, + canGoForward: false, + loadError: null, + createdAt: 1 + } + ] + }, + activeBrowserTabIdByWorktree: { [workspaceKey]: 'remembered' }, + activeTabTypeByWorktree: { [workspaceKey]: 'browser' } + } +} + type FolderWorkspaceUpdateArgs = { folderWorkspaceId: string updates: Partial @@ -121,6 +144,100 @@ describe('folder workspace generic activation and activity', () => { }) }) + it.each([ + ['simulator', 'local'], + ['agent-session', 'local'], + ['simulator', 'ssh:test-host'], + ['agent-session', 'ssh:test-host'] + ] as const)( + 'restores a folder %s tab on %s using its concrete visible type', + (contentType, executionHostId) => { + const folder = makeFolderWorkspace({ executionHostId }) + const workspaceKey = folderWorkspaceKey(folder.id) + const store = seedLocalFolderStore(folder) + store.getState().createUnifiedTab(workspaceKey, contentType, { id: 'selected' }) + store.setState({ activeTabTypeByWorktree: { [workspaceKey]: 'editor' } }) + + store.getState().setActiveFolderWorkspace(folder.id, executionHostId) + + expect(store.getState().activeWorkspaceExecutionHostId).toBe(executionHostId) + expect(store.getState().activeTabType).toBe(contentType) + expect(store.getState().activeTabTypeByWorktree[workspaceKey]).toBe(contentType) + expect(store.getState().getActiveTab(workspaceKey)?.id).toBe('selected') + } + ) + + it('does not let remembered browser state select content in an empty folder group', () => { + const folder = makeFolderWorkspace() + const workspaceKey = folderWorkspaceKey(folder.id) + const store = seedLocalFolderStore(folder) + store.setState({ + ...rememberedBrowserSurface(workspaceKey), + groupsByWorktree: { + [workspaceKey]: [ + { + id: 'empty', + worktreeId: workspaceKey, + activeTabId: null, + tabOrder: [] + } + ] + }, + activeGroupIdByWorktree: { [workspaceKey]: 'empty' } + } as Partial) + + store.getState().setActiveFolderWorkspace(folder.id) + + expect(store.getState().activeTabType).toBe('terminal') + expect(store.getState().activeBrowserTabId).toBe('remembered') + }) + + it('keeps layout-only folder ownership above remembered browser state', () => { + const folder = makeFolderWorkspace() + const workspaceKey = folderWorkspaceKey(folder.id) + const store = seedLocalFolderStore(folder) + store.setState({ + ...rememberedBrowserSurface(workspaceKey), + groupsByWorktree: {}, + layoutByWorktree: { [workspaceKey]: { type: 'leaf', groupId: 'pending' } } + } as Partial) + + store.getState().setActiveFolderWorkspace(folder.id) + + expect(store.getState().activeTabType).toBe('terminal') + expect(store.getState().activeBrowserTabId).toBe('remembered') + }) + + it('falls back to an open file when nothing else owns the folder surface', () => { + const folder = makeFolderWorkspace() + const workspaceKey = folderWorkspaceKey(folder.id) + const store = seedLocalFolderStore(folder) + store.setState({ + groupsByWorktree: {}, + layoutByWorktree: {}, + // Why: the remembered browser tab is gone, so only the open file is left to show. + activeBrowserTabIdByWorktree: { [workspaceKey]: 'closed' }, + browserTabsByWorktree: { [workspaceKey]: [] }, + activeTabTypeByWorktree: { [workspaceKey]: 'browser' }, + openFiles: [ + { + id: 'fallback-file', + worktreeId: workspaceKey, + filePath: '/workspace/folder/file', + relativePath: 'file', + language: 'plaintext', + isDirty: false, + mode: 'edit' + } + ] + } as Partial) + + store.getState().setActiveFolderWorkspace(folder.id) + + expect(store.getState().activeTabType).toBe('editor') + expect(store.getState().activeFileId).toBe('fallback-file') + }) + it('coalesces repeated activity persistence while keeping local activity current', async () => { vi.useFakeTimers() vi.setSystemTime(1_000) diff --git a/src/renderer/src/store/slices/tabs/tab-selection-contract.test.ts b/src/renderer/src/store/slices/tabs/tab-selection-contract.test.ts new file mode 100644 index 00000000000..3100301b50a --- /dev/null +++ b/src/renderer/src/store/slices/tabs/tab-selection-contract.test.ts @@ -0,0 +1,228 @@ +import { describe, expect, it } from 'vitest' +import type { Tab, TabContentType } from '../../../../../shared/tab-types' +import { buildHydratedTabState } from '../tabs-hydration' +import { resolveActivatedWorktreeSurface } from '../worktrees/session/active-worktree-surface' +import { deriveActiveSurfaceForWorktree } from './tabs-surface' + +type SelectionState = Parameters[0] + +const workspace = 'repo::/workspace' +function selectedTab(contentType: TabContentType): Tab { + return { + id: 'selected', + entityId: 'selected-entity', + groupId: 'group', + worktreeId: workspace, + contentType, + label: 'Selected', + customLabel: null, + color: null, + sortOrder: 0, + createdAt: 1 + } +} + +function selectionState(tab: Tab | null): SelectionState { + return { + rightSidebarExplorerViewByWorktree: {}, + activeGroupIdByWorktree: { [workspace]: 'group' }, + groupsByWorktree: { + [workspace]: [ + { + id: 'group', + worktreeId: workspace, + activeTabId: tab?.id ?? null, + tabOrder: tab ? [tab.id] : [] + } + ] + }, + unifiedTabsByWorktree: { [workspace]: tab ? [tab] : [] }, + layoutByWorktree: {}, + activeTabIdByWorktree: { [workspace]: 'remembered-terminal' }, + activeFileIdByWorktree: { [workspace]: 'remembered-file' }, + activeBrowserTabIdByWorktree: { [workspace]: 'remembered-browser' }, + activeTabTypeByWorktree: { [workspace]: 'browser' }, + tabsByWorktree: { + [workspace]: [ + { + id: 'remembered-terminal', + worktreeId: workspace, + ptyId: null, + title: 'Terminal', + customTitle: null, + color: null, + sortOrder: 0, + createdAt: 1 + } + ] + }, + browserTabsByWorktree: { + [workspace]: [ + { + id: 'remembered-browser', + worktreeId: workspace, + url: 'about:blank', + title: 'Browser', + loading: false, + faviconUrl: null, + canGoBack: false, + canGoForward: false, + loadError: null, + createdAt: 1 + } + ] + }, + openFiles: [ + { + id: 'remembered-file', + worktreeId: workspace, + filePath: '/workspace/file', + relativePath: 'file', + language: 'plaintext', + isDirty: false, + mode: 'edit' + } + ] + } +} + +function activate(state: SelectionState) { + const { restoredRightSidebarExplorerView: _view, ...surface } = resolveActivatedWorktreeSurface( + state, + workspace, + undefined, + null + ) + return surface +} + +describe('tab selection and hydration ownership', () => { + it.each([ + ['terminal', 'terminal'], + ['editor', 'editor'], + ['diff', 'editor'], + ['conflict-review', 'editor'], + ['check-details', 'editor'], + ['browser', 'browser'], + ['simulator', 'simulator'], + ['agent-session', 'agent-session'] + ] as const)( + 'projects %s selection while retaining other remembered surfaces', + (kind, visible) => { + const state = selectionState(selectedTab(kind)) + const expected = { + activeTabType: visible, + activeTabId: kind === 'terminal' ? 'selected-entity' : 'remembered-terminal', + activeFileId: visible === 'editor' ? 'selected-entity' : 'remembered-file', + activeBrowserTabId: kind === 'browser' ? 'selected-entity' : 'remembered-browser' + } + expect(deriveActiveSurfaceForWorktree(state, workspace)).toEqual(expected) + expect(activate(state)).toEqual(expected) + } + ) + + it('keeps empty groups authoritative over remembered browser/editor surfaces', () => { + const state = selectionState(null) + expect(activate(state).activeTabType).toBe('terminal') + expect(deriveActiveSurfaceForWorktree(state, workspace).activeTabType).toBe('terminal') + }) + + it('keeps layout-only ownership authoritative during staged hydration', () => { + const state = selectionState(null) + state.groupsByWorktree = {} + state.layoutByWorktree = { [workspace]: { type: 'leaf', groupId: 'pending' } } + expect(activate(state).activeTabType).toBe('terminal') + expect(deriveActiveSurfaceForWorktree(state, workspace).activeTabType).toBe('terminal') + }) + + it('distinguishes workspace restoration from group-focus legacy fallback', () => { + const state = selectionState(null) + state.groupsByWorktree = {} + state.activeTabTypeByWorktree[workspace] = 'terminal' + expect(activate(state).activeTabType).toBe('terminal') + expect(deriveActiveSurfaceForWorktree(state, workspace).activeTabType).toBe('browser') + }) + + it('does not select a preferred tab owned by another group', () => { + const state = selectionState(selectedTab('browser')) + state.unifiedTabsByWorktree[workspace].push({ + ...selectedTab('editor'), + id: 'foreign', + groupId: 'other' + }) + expect(resolveActivatedWorktreeSurface(state, workspace, 'foreign', null).activeTabType).toBe( + 'terminal' + ) + }) + + it('resolves stale active-group IDs to the first group consistently', () => { + const state = selectionState(selectedTab('simulator')) + state.activeGroupIdByWorktree[workspace] = 'removed' + expect(activate(state).activeTabType).toBe('simulator') + expect(deriveActiveSurfaceForWorktree(state, workspace).activeTabType).toBe('simulator') + }) + + it('requires group ownership even for an explicit preferred tab during hydration', () => { + const state = selectionState(selectedTab('simulator')) + state.groupsByWorktree = {} + state.activeTabTypeByWorktree[workspace] = 'terminal' + expect(resolveActivatedWorktreeSurface(state, workspace, 'selected', null).activeTabType).toBe( + 'terminal' + ) + }) + + it('honors a preferred tab in the selected group without mutating selection', () => { + const state = selectionState(selectedTab('browser')) + const preferred = { ...selectedTab('simulator'), id: 'preferred' } + state.unifiedTabsByWorktree[workspace].push(preferred) + state.groupsByWorktree[workspace][0].tabOrder.push(preferred.id) + expect( + resolveActivatedWorktreeSurface(state, workspace, preferred.id, null).activeTabType + ).toBe('simulator') + expect(state.groupsByWorktree[workspace][0].activeTabId).toBe('selected') + }) + + it.each([ + ['terminal', 'terminal', 'remembered-file'], + ['editor', 'editor', 'remembered-file'], + ['browser', 'browser', 'remembered-file'], + // Why: nothing renders a remembered agent-session/simulator once its tab is gone, so the browser + // surface takes over and must not leave the remembered file selected underneath it. + ['agent-session', 'browser', null], + ['simulator', 'browser', null] + ] as const)( + 'projects legacy %s memory as %s when unified groups are absent', + (activeTabType, visible, activeFileId) => { + const state = selectionState(null) + state.groupsByWorktree = {} + state.activeTabTypeByWorktree[workspace] = activeTabType + expect(activate(state)).toEqual({ + activeTabType: visible, + activeTabId: 'remembered-terminal', + activeFileId, + activeBrowserTabId: 'remembered-browser' + }) + } + ) + + it('hydrates unified selection without allowing conflicting legacy memories to choose it', () => { + const tab = selectedTab('simulator') + const state = selectionState(tab) + const session = { + activeRepoId: null, + activeWorktreeId: workspace, + activeTabId: 'remembered-terminal', + tabsByWorktree: {}, + terminalLayoutsByTabId: {}, + unifiedTabs: state.unifiedTabsByWorktree, + tabGroups: state.groupsByWorktree, + activeGroupIdByWorktree: state.activeGroupIdByWorktree, + activeTabTypeByWorktree: { [workspace]: 'browser' as const }, + activeTabIdByWorktree: state.activeTabIdByWorktree + } + const before = structuredClone(session) + const hydrated = buildHydratedTabState(session, new Set([workspace])) + expect(activate({ ...state, ...hydrated }).activeTabType).toBe('simulator') + expect(session).toEqual(before) + }) +}) diff --git a/src/renderer/src/store/slices/tabs/tabs-surface.ts b/src/renderer/src/store/slices/tabs/tabs-surface.ts index b870ca7bb32..793d0437fca 100644 --- a/src/renderer/src/store/slices/tabs/tabs-surface.ts +++ b/src/renderer/src/store/slices/tabs/tabs-surface.ts @@ -2,22 +2,26 @@ import type { AppState } from '../../types' import { toVisibleTabType } from '../../../../../shared/tab-types' import type { WorkspaceVisibleTabType } from '../../../../../shared/tab-types' +export type ActiveSurfaceSourceState = Pick< + AppState, + | 'activeBrowserTabIdByWorktree' + | 'activeFileIdByWorktree' + | 'activeGroupIdByWorktree' + | 'activeTabIdByWorktree' + | 'activeTabTypeByWorktree' + | 'browserTabsByWorktree' + | 'groupsByWorktree' + | 'layoutByWorktree' + | 'openFiles' + | 'tabsByWorktree' + | 'unifiedTabsByWorktree' +> + export function deriveActiveSurfaceForWorktree( - state: Pick< - AppState, - | 'activeBrowserTabIdByWorktree' - | 'activeFileIdByWorktree' - | 'activeGroupIdByWorktree' - | 'activeTabIdByWorktree' - | 'browserTabsByWorktree' - | 'groupsByWorktree' - | 'layoutByWorktree' - | 'openFiles' - | 'tabsByWorktree' - | 'unifiedTabsByWorktree' - >, + state: ActiveSurfaceSourceState, worktreeId: string, - preferredGroupId?: string | null + preferredGroupId?: string | null, + options?: { preferredTabId?: string; legacySelection?: 'remembered-type' } ): { activeBrowserTabId: string | null activeFileId: string | null @@ -28,10 +32,12 @@ export function deriveActiveSurfaceForWorktree( const activeGroupId = preferredGroupId ?? state.activeGroupIdByWorktree[worktreeId] ?? null const activeGroup = (activeGroupId ? groups.find((group) => group.id === activeGroupId) : null) ?? groups[0] ?? null + const activeUnifiedTabId = options?.preferredTabId ?? activeGroup?.activeTabId const activeUnifiedTab = - activeGroup?.activeTabId != null + activeUnifiedTabId != null ? ((state.unifiedTabsByWorktree[worktreeId] ?? []).find( - (tab) => tab.id === activeGroup.activeTabId && tab.groupId === activeGroup.id + (tab) => + tab.id === activeUnifiedTabId && activeGroup != null && tab.groupId === activeGroup.id ) ?? null) : null const restoredFileId = state.activeFileIdByWorktree[worktreeId] ?? null @@ -50,6 +56,14 @@ export function deriveActiveSurfaceForWorktree( : false const hasGroupOwnedSurface = groups.length > 0 || Boolean(state.layoutByWorktree[worktreeId]) + const restoreLegacyType = options?.legacySelection === 'remembered-type' + const restoredTabType = restoreLegacyType + ? (state.activeTabTypeByWorktree[worktreeId] ?? 'terminal') + : null + // Why: only a remembered browser type — or group focus, which remembers no type at all — may keep + // the remembered file selected under the browser surface; a stale agent-session/simulator clears it. + const keepRememberedFileUnderBrowser = restoredTabType === null || restoredTabType === 'browser' + let activeFileId: string | null let activeBrowserTabId: string | null let activeTabType: WorkspaceVisibleTabType @@ -76,8 +90,16 @@ export function deriveActiveSurfaceForWorktree( activeBrowserTabId = browserTabStillOpen ? restoredBrowserTabId : (browserTabs[0]?.id ?? null) // Why: focusing an empty split should target its default terminal area, not the previously active browser/editor in another group. activeTabType = 'terminal' - } else if (browserTabStillOpen) { + } else if (restoredTabType === 'terminal') { activeFileId = fileStillOpen ? restoredFileId : null + activeBrowserTabId = browserTabStillOpen ? restoredBrowserTabId : (browserTabs[0]?.id ?? null) + activeTabType = 'terminal' + } else if (restoredTabType === 'editor' && fileStillOpen) { + activeFileId = restoredFileId + activeBrowserTabId = browserTabStillOpen ? restoredBrowserTabId : (browserTabs[0]?.id ?? null) + activeTabType = 'editor' + } else if (browserTabStillOpen) { + activeFileId = keepRememberedFileUnderBrowser && fileStillOpen ? restoredFileId : null activeBrowserTabId = restoredBrowserTabId activeTabType = 'browser' } else if (fileStillOpen) { @@ -106,20 +128,7 @@ export function deriveActiveSurfaceForWorktree( } export function buildActiveSurfacePatch( - state: Pick< - AppState, - | 'activeBrowserTabIdByWorktree' - | 'activeFileIdByWorktree' - | 'activeGroupIdByWorktree' - | 'activeTabIdByWorktree' - | 'activeTabTypeByWorktree' - | 'browserTabsByWorktree' - | 'groupsByWorktree' - | 'layoutByWorktree' - | 'openFiles' - | 'tabsByWorktree' - | 'unifiedTabsByWorktree' - >, + state: ActiveSurfaceSourceState, worktreeId: string, preferredGroupId?: string | null ): Pick< diff --git a/src/renderer/src/store/slices/worktrees/session/active-worktree-surface.ts b/src/renderer/src/store/slices/worktrees/session/active-worktree-surface.ts index 6dcfc62d155..3e185435551 100644 --- a/src/renderer/src/store/slices/worktrees/session/active-worktree-surface.ts +++ b/src/renderer/src/store/slices/worktrees/session/active-worktree-surface.ts @@ -1,9 +1,10 @@ import type { AppState } from '../../../types' import type { WorkspaceVisibleTabType } from '../../../../../../shared/tab-types' -import { toVisibleTabType } from '../../../../../../shared/tab-types' +import type { ActiveSurfaceSourceState } from '../../tabs/tabs-surface' +import { deriveActiveSurfaceForWorktree } from '../../tabs/tabs-surface' export function resolveActivatedWorktreeSurface( - s: AppState, + s: ActiveSurfaceSourceState & Pick, worktreeId: string, preferredActiveUnifiedTabId: string | undefined, reconciledActiveTabId: string | null @@ -16,107 +17,11 @@ export function resolveActivatedWorktreeSurface( activeTabType: WorkspaceVisibleTabType activeTabId: string | null } { - // Why: Search lives under Explorer, so the files/search sub-route must switch with the worktree, not leak the prior one. - const restoredRightSidebarExplorerView = - s.rightSidebarExplorerViewByWorktree?.[worktreeId] ?? 'files' - const restoredFileId = s.activeFileIdByWorktree[worktreeId] ?? null - const restoredBrowserTabId = s.activeBrowserTabIdByWorktree[worktreeId] ?? null - const restoredTabType = s.activeTabTypeByWorktree[worktreeId] ?? 'terminal' - const activeGroupId = - s.activeGroupIdByWorktree[worktreeId] ?? s.groupsByWorktree[worktreeId]?.[0]?.id ?? null - const activeGroup = activeGroupId - ? ((s.groupsByWorktree[worktreeId] ?? []).find((group) => group.id === activeGroupId) ?? null) - : null - const activeUnifiedTabId = - preferredActiveUnifiedTabId ?? reconciledActiveTabId ?? activeGroup?.activeTabId ?? null - const activeUnifiedTab = - activeUnifiedTabId != null - ? ((s.unifiedTabsByWorktree[worktreeId] ?? []).find( - (tab) => tab.id === activeUnifiedTabId && (!activeGroup || tab.groupId === activeGroup.id) - ) ?? null) - : null - // Verify the restored file still exists in openFiles - const fileStillOpen = restoredFileId - ? s.openFiles.some((f) => f.id === restoredFileId && f.worktreeId === worktreeId) - : false - const browserTabs = s.browserTabsByWorktree[worktreeId] ?? [] - const browserTabStillOpen = restoredBrowserTabId - ? browserTabs.some((tab) => tab.id === restoredBrowserTabId) - : false - const hasGroupOwnedSurface = - (s.groupsByWorktree[worktreeId]?.length ?? 0) > 0 || Boolean(s.layoutByWorktree[worktreeId]) - - // Why: restore from the reconciled tab-group model first; preferring legacy fallbacks can show a blank worktree. - let activeFileId: string | null - let activeBrowserTabId: string | null - let activeTabType: WorkspaceVisibleTabType - if (activeUnifiedTab) { - activeFileId = - activeUnifiedTab.contentType === 'editor' || - activeUnifiedTab.contentType === 'diff' || - activeUnifiedTab.contentType === 'conflict-review' || - activeUnifiedTab.contentType === 'check-details' - ? activeUnifiedTab.entityId - : fileStillOpen - ? restoredFileId - : null - activeBrowserTabId = - activeUnifiedTab.contentType === 'browser' - ? activeUnifiedTab.entityId - : browserTabStillOpen - ? restoredBrowserTabId - : (browserTabs[0]?.id ?? null) - activeTabType = toVisibleTabType(activeUnifiedTab.contentType) - } else if (hasGroupOwnedSurface) { - activeFileId = fileStillOpen ? restoredFileId : null - activeBrowserTabId = browserTabStillOpen ? restoredBrowserTabId : (browserTabs[0]?.id ?? null) - activeTabType = 'terminal' - } else if (restoredTabType === 'terminal') { - activeFileId = fileStillOpen ? restoredFileId : null - activeBrowserTabId = browserTabStillOpen ? restoredBrowserTabId : (browserTabs[0]?.id ?? null) - activeTabType = 'terminal' - } else if (restoredTabType === 'browser' && browserTabStillOpen) { - activeFileId = fileStillOpen ? restoredFileId : null - activeBrowserTabId = restoredBrowserTabId - activeTabType = 'browser' - } else if (restoredTabType === 'editor' && fileStillOpen) { - activeFileId = restoredFileId - activeBrowserTabId = browserTabStillOpen ? restoredBrowserTabId : (browserTabs[0]?.id ?? null) - activeTabType = 'editor' - } else if (browserTabStillOpen) { - activeFileId = null - activeBrowserTabId = restoredBrowserTabId - activeTabType = 'browser' - } else if (fileStillOpen) { - activeFileId = restoredFileId - activeBrowserTabId = browserTabs[0]?.id ?? null - activeTabType = 'editor' - } else { - const fallbackFile = s.openFiles.find((f) => f.worktreeId === worktreeId) - const fallbackBrowserTab = browserTabs[0] ?? null - activeFileId = fallbackFile?.id ?? null - activeBrowserTabId = browserTabStillOpen - ? restoredBrowserTabId - : (fallbackBrowserTab?.id ?? null) - activeTabType = fallbackFile ? 'editor' : fallbackBrowserTab ? 'browser' : 'terminal' - } - - // Why: restore the last-active terminal tab so the user returns to where they left, not tab 0. - const restoredTabId = s.activeTabIdByWorktree[worktreeId] ?? null - const worktreeTabs = s.tabsByWorktree[worktreeId] ?? [] - const tabStillExists = restoredTabId ? worktreeTabs.some((t) => t.id === restoredTabId) : false - const activeTabId = - activeUnifiedTab?.contentType === 'terminal' - ? activeUnifiedTab.entityId - : tabStillExists - ? restoredTabId - : (worktreeTabs[0]?.id ?? null) - return { - restoredRightSidebarExplorerView, - activeFileId, - activeBrowserTabId, - activeTabType, - activeTabId + restoredRightSidebarExplorerView: s.rightSidebarExplorerViewByWorktree?.[worktreeId] ?? 'files', + ...deriveActiveSurfaceForWorktree(s, worktreeId, undefined, { + legacySelection: 'remembered-type', + preferredTabId: preferredActiveUnifiedTabId ?? reconciledActiveTabId ?? undefined + }) } } diff --git a/src/renderer/src/store/slices/worktrees/session/set-active-folder-workspace.ts b/src/renderer/src/store/slices/worktrees/session/set-active-folder-workspace.ts index 1ca9f11ee5a..d2b5af79114 100644 --- a/src/renderer/src/store/slices/worktrees/session/set-active-folder-workspace.ts +++ b/src/renderer/src/store/slices/worktrees/session/set-active-folder-workspace.ts @@ -8,6 +8,7 @@ import { folderWorkspaceMatchesHost } from '../listing/detected-worktree-meta' import { shouldDeferActivationTerminalPrep } from './activation-terminal-prep' +import { deriveActiveSurfaceForWorktree } from '../../tabs/tabs-surface' export function createSetActiveFolderWorkspace( set: WorktreeSliceSet, @@ -28,72 +29,11 @@ export function createSetActiveFolderWorkspace( const reconciledActiveTabId = get().reconcileWorktreeTabModel(workspaceKey).activeRenderableTabId set((s) => { - const restoredFileId = s.activeFileIdByWorktree[workspaceKey] ?? null - const restoredBrowserTabId = s.activeBrowserTabIdByWorktree[workspaceKey] ?? null - const restoredTabType = s.activeTabTypeByWorktree[workspaceKey] ?? 'terminal' - const activeGroupId = - s.activeGroupIdByWorktree[workspaceKey] ?? s.groupsByWorktree[workspaceKey]?.[0]?.id ?? null - const activeGroup = activeGroupId - ? ((s.groupsByWorktree[workspaceKey] ?? []).find((group) => group.id === activeGroupId) ?? - null) - : null - const activeUnifiedTabId = reconciledActiveTabId ?? activeGroup?.activeTabId ?? null - const activeUnifiedTab = - activeUnifiedTabId != null - ? ((s.unifiedTabsByWorktree[workspaceKey] ?? []).find( - (tab) => - tab.id === activeUnifiedTabId && (!activeGroup || tab.groupId === activeGroup.id) - ) ?? null) - : null - const fileStillOpen = restoredFileId - ? s.openFiles.some((file) => file.id === restoredFileId && file.worktreeId === workspaceKey) - : false - const browserTabs = s.browserTabsByWorktree[workspaceKey] ?? [] - const browserTabStillOpen = restoredBrowserTabId - ? browserTabs.some((tab) => tab.id === restoredBrowserTabId) - : false - const worktreeTabs = s.tabsByWorktree[workspaceKey] ?? [] - const restoredTabId = s.activeTabIdByWorktree[workspaceKey] ?? null - const tabStillExists = restoredTabId - ? worktreeTabs.some((tab) => tab.id === restoredTabId) - : false - const activeFileId = - activeUnifiedTab?.contentType === 'editor' || - activeUnifiedTab?.contentType === 'diff' || - activeUnifiedTab?.contentType === 'conflict-review' || - activeUnifiedTab?.contentType === 'check-details' - ? activeUnifiedTab.entityId - : fileStillOpen - ? restoredFileId - : null - const activeBrowserTabId = - activeUnifiedTab?.contentType === 'browser' - ? activeUnifiedTab.entityId - : browserTabStillOpen - ? restoredBrowserTabId - : (browserTabs[0]?.id ?? null) - const activeTabType = - activeUnifiedTab?.contentType === 'terminal' - ? 'terminal' - : activeUnifiedTab?.contentType === 'browser' - ? 'browser' - : activeUnifiedTab - ? 'editor' - : restoredTabType === 'browser' && browserTabStillOpen - ? 'browser' - : restoredTabType === 'editor' && fileStillOpen - ? 'editor' - : fileStillOpen - ? 'editor' - : browserTabs.length > 0 - ? 'browser' - : 'terminal' - const activeTabId = - activeUnifiedTab?.contentType === 'terminal' - ? activeUnifiedTab.entityId - : tabStillExists - ? restoredTabId - : (worktreeTabs[0]?.id ?? null) + const { activeFileId, activeBrowserTabId, activeTabType, activeTabId } = + deriveActiveSurfaceForWorktree(s, workspaceKey, undefined, { + legacySelection: 'remembered-type', + preferredTabId: reconciledActiveTabId ?? undefined + }) const nextEverActivated = s.everActivatedWorktreeIds.has(workspaceKey) ? s.everActivatedWorktreeIds : new Set([...s.everActivatedWorktreeIds, workspaceKey]) diff --git a/src/shared/agent-session-wire.ts b/src/shared/agent-session-wire.ts index 42b9c56475e..854f02e74e1 100644 --- a/src/shared/agent-session-wire.ts +++ b/src/shared/agent-session-wire.ts @@ -91,6 +91,11 @@ export type AgentSessionBackgroundTaskState = { settledTasks?: AgentSessionBackgroundTask[] /** Optional so clients only send targeted stops to hosts that accept them. */ supportsTaskStop?: boolean + /** Whether an untargeted "stop everything" is available at all. Absent means + * yes: every host that predates this field accepted one, and a client that + * read absence as "no stop" would hide a working control on those hosts. + * A host whose provider exposes no honest stop sends `false`. */ + supportsStopAll?: boolean } function backgroundTaskFieldsEqual( diff --git a/src/shared/native-chat-task-list.test.ts b/src/shared/native-chat-task-list.test.ts new file mode 100644 index 00000000000..0bbb7077f3c --- /dev/null +++ b/src/shared/native-chat-task-list.test.ts @@ -0,0 +1,138 @@ +import { describe, expect, it } from 'vitest' +import { + diffNativeChatTaskLists, + nativeChatTaskLabel, + normalizeNativeChatTaskList, + type NativeChatTask, + type NativeChatTaskList +} from './native-chat-task-list' + +const task = (content: string, status: NativeChatTask['status'] = 'pending'): NativeChatTask => ({ + content, + status +}) +const list = (...tasks: NativeChatTask[]): NativeChatTaskList => ({ tasks }) + +describe('normalizeNativeChatTaskList', () => { + it('normalizes Claude tasks and uses activeForm only while in progress', () => { + const result = normalizeNativeChatTaskList('TodoWrite', { + todos: [ + { content: 'Read', status: 'completed', activeForm: 'Reading' }, + { content: 'Write', status: 'in_progress', activeForm: 'Writing' }, + { content: 'Test', status: 'pending', activeForm: 'Testing' } + ] + })! + expect(result.tasks.map(nativeChatTaskLabel)).toEqual(['Read', 'Writing', 'Test']) + expect(result.tasks.map((entry) => entry.status)).toEqual([ + 'completed', + 'in_progress', + 'pending' + ]) + }) + + it('normalizes Codex JSON-string arguments and explanation', () => { + expect( + normalizeNativeChatTaskList( + 'update_plan', + JSON.stringify({ + explanation: 'Proceed with verification', + plan: [{ step: 'Test', status: 'in_progress' }] + }) + ) + ).toEqual({ explanation: 'Proceed with verification', tasks: [task('Test', 'in_progress')] }) + }) + + it('defaults unknown/missing statuses and ignores invalid entries', () => { + expect( + normalizeNativeChatTaskList(' TodoWrite ', { + todos: [ + null, + [], + 4, + {}, + { content: ' ' }, + { content: 7 }, + { content: ' One ', status: 'unknown', activeForm: 4 }, + { content: 'Two' } + ] + }) + ).toEqual(list(task('One'), task('Two'))) + }) + + it.each([undefined, null, 42, [], '{', '{}', { todos: null }, { todos: [{}] }])( + 'returns null for malformed input %j', + (input) => { + expect(normalizeNativeChatTaskList('TodoWrite', input)).toBeNull() + } + ) + + it('keeps empty lists valid and recognizes only exact tool families', () => { + expect(normalizeNativeChatTaskList('update_plan', { plan: [] })).toEqual(list()) + expect(normalizeNativeChatTaskList('TodoWrite', { todos: [] })).toEqual(list()) + expect(normalizeNativeChatTaskList('mcp__server__TodoWrite', { todos: [] })).toBeNull() + expect(normalizeNativeChatTaskList('ExitPlanMode', { plan: [] })).toBeNull() + expect(normalizeNativeChatTaskList('update_plan', { todos: [] })).toBeNull() + }) +}) + +describe('diffNativeChatTaskLists', () => { + it('reports completions and starts, omitting unchanged tasks', () => { + expect( + diffNativeChatTaskLists( + list(task('Read', 'in_progress'), task('Write'), task('Test')), + list(task('Read', 'completed'), task('Write', 'in_progress'), task('Test')) + ) + ).toEqual([ + { kind: 'completed', task: task('Read', 'completed') }, + { kind: 'started', task: task('Write', 'in_progress') } + ]) + }) + + it('ignores reorder-only updates and explanation changes', () => { + expect( + diffNativeChatTaskLists(list(task('A'), task('B')), { + tasks: [task('B'), task('A')], + explanation: 'Reordered' + }) + ).toEqual([]) + }) + + it('matches duplicate contents by occurrence', () => { + expect( + diffNativeChatTaskLists( + list(task('A'), task('A', 'in_progress')), + list(task('A', 'completed'), task('A', 'in_progress')) + ) + ).toEqual([{ kind: 'completed', task: task('A', 'completed') }]) + }) + + it('reports renamed content as an addition and removal', () => { + expect(diffNativeChatTaskLists(list(task('Old')), list(task('New')))).toEqual([ + { kind: 'added', task: task('New') }, + { kind: 'removed', task: task('Old') } + ]) + }) + + it('reports resets, reopening, and activeForm-only edits', () => { + const changed = { ...task('C', 'in_progress'), activeForm: 'Checking C' } + expect( + diffNativeChatTaskLists( + list(task('A', 'completed'), task('B', 'completed'), task('C', 'in_progress')), + list(task('A'), task('B', 'in_progress'), changed) + ) + ).toEqual([ + { kind: 'pending', task: task('A') }, + { kind: 'started', task: task('B', 'in_progress') }, + { kind: 'updated', task: changed } + ]) + }) + + it('reports clearing a list and removing a duplicate', () => { + expect(diffNativeChatTaskLists(list(task('A')), list())).toEqual([ + { kind: 'removed', task: task('A') } + ]) + expect(diffNativeChatTaskLists(list(task('A'), task('A')), list(task('A')))).toEqual([ + { kind: 'removed', task: task('A') } + ]) + }) +}) diff --git a/src/shared/native-chat-task-list.ts b/src/shared/native-chat-task-list.ts new file mode 100644 index 00000000000..4d4d470b246 --- /dev/null +++ b/src/shared/native-chat-task-list.ts @@ -0,0 +1,122 @@ +export type NativeChatTaskStatus = 'pending' | 'in_progress' | 'completed' +export type NativeChatTask = { + content: string + status: NativeChatTaskStatus + activeForm?: string +} +export type NativeChatTaskList = { tasks: NativeChatTask[]; explanation?: string } +export type NativeChatTaskChange = { + kind: 'added' | 'removed' | 'started' | 'completed' | 'pending' | 'updated' + task: NativeChatTask +} +export type NativeChatTaskListTool = 'todowrite' | 'update_plan' + +export function nativeChatTaskListTool(name: string): NativeChatTaskListTool | null { + const normalized = name.trim().toLowerCase() + return normalized === 'todowrite' || normalized === 'update_plan' ? normalized : null +} + +function record(value: unknown): Record | null { + return value !== null && typeof value === 'object' && !Array.isArray(value) + ? (value as Record) + : null +} + +function nonemptyString(value: unknown): string | undefined { + return typeof value === 'string' && value.trim() ? value.trim() : undefined +} + +export function normalizeNativeChatTaskList( + name: string, + input: unknown +): NativeChatTaskList | null { + const tool = nativeChatTaskListTool(name) + if (!tool) { + return null + } + if (typeof input === 'string') { + try { + input = JSON.parse(input) + } catch { + return null + } + } + const value = record(input) + const entries = tool === 'todowrite' ? value?.todos : value?.plan + if (!Array.isArray(entries)) { + return null + } + const tasks: NativeChatTask[] = [] + for (const entry of entries) { + const item = record(entry) + const content = nonemptyString(tool === 'todowrite' ? item?.content : item?.step) + if (!item || !content) { + continue + } + const status = + item.status === 'in_progress' || (tool === 'update_plan' && item.status === 'inProgress') + ? 'in_progress' + : item.status === 'completed' + ? 'completed' + : 'pending' + const activeForm = tool === 'todowrite' ? nonemptyString(item.activeForm) : undefined + tasks.push({ content, status, ...(activeForm ? { activeForm } : {}) }) + } + if (entries.length > 0 && tasks.length === 0) { + return null + } + const explanation = tool === 'update_plan' ? nonemptyString(value?.explanation) : undefined + return { tasks, ...(explanation ? { explanation } : {}) } +} + +export function nativeChatTaskLabel(task: NativeChatTask): string { + return task.status === 'in_progress' && task.activeForm ? task.activeForm : task.content +} + +/** Content plus occurrence is the only identity the providers give these entries. */ +export function diffNativeChatTaskLists( + previous: NativeChatTaskList, + current: NativeChatTaskList +): NativeChatTaskChange[] { + const byContent = new Map() + for (const task of previous.tasks) { + const matches = byContent.get(task.content) + if (matches) { + matches.push(task) + } else { + byContent.set(task.content, [task]) + } + } + const occurrences = new Map() + const consumed = new Set() + const changes: NativeChatTaskChange[] = [] + for (const task of current.tasks) { + const occurrence = occurrences.get(task.content) ?? 0 + occurrences.set(task.content, occurrence + 1) + const before = byContent.get(task.content)?.[occurrence] + if (!before) { + changes.push({ kind: 'added', task }) + continue + } + consumed.add(before) + if (before.status !== task.status) { + changes.push({ + kind: + task.status === 'completed' + ? 'completed' + : task.status === 'in_progress' + ? 'started' + : 'pending', + task + }) + } else if (before.activeForm !== task.activeForm) { + changes.push({ kind: 'updated', task }) + } + } + for (const task of previous.tasks) { + if (!consumed.has(task)) { + changes.push({ kind: 'removed', task }) + } + } + return changes +} diff --git a/src/shared/native-chat-tool-icon.test.ts b/src/shared/native-chat-tool-icon.test.ts index 695400c94ca..f8cabed950c 100644 --- a/src/shared/native-chat-tool-icon.test.ts +++ b/src/shared/native-chat-tool-icon.test.ts @@ -50,6 +50,9 @@ describe('native chat tool icons', () => { expect(nativeChatToolCategory('list')).toBe('listFiles') expect(nativeChatToolCategory('shell')).toBe('unknown') expect(nativeChatToolCategory('apply_patch')).toBe('fileChange') + expect(nativeChatToolCategory('update_plan')).toBe('todoList') + expect(nativeChatToolIconName('update_plan')).toBe('list-checks') + expect(nativeChatToolRunIconName([{ name: 'update_plan' }])).toBe('list-checks') expect(nativeChatToolCategory('web search')).toBe('webSearch') }) diff --git a/src/shared/native-chat-tool-icon.ts b/src/shared/native-chat-tool-icon.ts index d546a76e865..d52df9e52e9 100644 --- a/src/shared/native-chat-tool-icon.ts +++ b/src/shared/native-chat-tool-icon.ts @@ -81,6 +81,7 @@ const CATEGORY_BY_ROW_WORD = new Map([ ['task', 'subAgentActivity'], ['webfetch', 'webSearch'], ['todowrite', 'todoList'], + ['update_plan', 'todoList'], ['web search', 'webSearch'], ['websearch', 'webSearch'], ['web_search', 'webSearch'] diff --git a/src/shared/orchestration-rpc-contract.ts b/src/shared/orchestration-rpc-contract.ts index 3067d5ad6b0..fec4d4ed4e7 100644 --- a/src/shared/orchestration-rpc-contract.ts +++ b/src/shared/orchestration-rpc-contract.ts @@ -14,6 +14,9 @@ export const ORCHESTRATION_SKILL_COMMAND_ARGS = [ ] as const export const ORCHESTRATION_LEGACY_RUN_ID = 'run_legacy_local' +/** Files mail sent from a terminal in no Run. Distinct from the legacy Run on purpose: rows under + * the legacy id read as a pre-Runs database and trigger adoption on the next open. */ +export const ORCHESTRATION_UNBOUND_RUN_ID = 'run_unbound' const ORCHESTRATION_MUTATION_METHODS = new Set([ 'orchestration.runCreate', diff --git a/src/shared/protocol-version.ts b/src/shared/protocol-version.ts index 3aaeeb747f4..74317dd28c1 100644 --- a/src/shared/protocol-version.ts +++ b/src/shared/protocol-version.ts @@ -163,6 +163,9 @@ export const STRUCTURED_AGENT_SESSION_RESUME_HISTORY_RUNTIME_CAPABILITY = export const AGENT_SESSION_STATUS_FEED_RUNTIME_CAPABILITY = 'agent-session.status-feed.v1' as const // The RPC is registered unconditionally; per-session rewind support is a separate check. export const AGENT_SESSION_REWIND_RUNTIME_CAPABILITY = 'agent-session.rewind.v1' as const +// Readers must understand a monitoring roster with no available stop control. +export const AGENT_SESSION_BACKGROUND_TASK_STOP_CAPABILITY = + 'agent-session.background-task-stop.v1' as const // Why: adding kimi to RESUMABLE_TUI_AGENTS grows terminal.ensureAgentSession's enum, and an // older host answers the unknown member with invalid_argument — a code the launch fallback does // not retry on — so clients must probe before taking the host-authority path. @@ -269,6 +272,7 @@ export const RUNTIME_CAPABILITIES = [ STRUCTURED_AGENT_SESSION_RESUME_HISTORY_RUNTIME_CAPABILITY, AGENT_SESSION_STATUS_FEED_RUNTIME_CAPABILITY, AGENT_SESSION_REWIND_RUNTIME_CAPABILITY, + AGENT_SESSION_BACKGROUND_TASK_STOP_CAPABILITY, AGENT_SESSION_KIMI_RESUME_RUNTIME_CAPABILITY, FILE_MUTATION_OWNERSHIP_RUNTIME_CAPABILITY, GITHUB_MARK_PR_READY_RUNTIME_CAPABILITY, diff --git a/src/shared/remote-runtime-client-capabilities.ts b/src/shared/remote-runtime-client-capabilities.ts index 3797f08ffb9..456f90e25ad 100644 --- a/src/shared/remote-runtime-client-capabilities.ts +++ b/src/shared/remote-runtime-client-capabilities.ts @@ -1,4 +1,5 @@ import { + AGENT_SESSION_BACKGROUND_TASK_STOP_CAPABILITY, AGENT_SESSION_BOUNDARY_RUNTIME_CAPABILITY, AUTOMATION_OWNER_FENCING_RUNTIME_CAPABILITY, SESSION_TAB_CLOSE_INTENT_RUNTIME_CAPABILITY, @@ -16,6 +17,7 @@ export function remoteRuntimeClientCapabilities( ): RuntimeCapability[] { return Array.from( new Set([ + AGENT_SESSION_BACKGROUND_TASK_STOP_CAPABILITY, SESSION_TAB_CLOSE_INTENT_RUNTIME_CAPABILITY, SESSION_TABS_AUTHORITATIVE_INVENTORY_RUNTIME_CAPABILITY, AGENT_SESSION_BOUNDARY_RUNTIME_CAPABILITY, diff --git a/src/shared/structured-agent-session-reducer.ts b/src/shared/structured-agent-session-reducer.ts index 1f2fa7e6334..0fcb040b2dc 100644 --- a/src/shared/structured-agent-session-reducer.ts +++ b/src/shared/structured-agent-session-reducer.ts @@ -60,7 +60,8 @@ function backgroundTaskStatesEqual( !left || !right || left.state !== right.state || - left.supportsTaskStop !== right.supportsTaskStop + left.supportsTaskStop !== right.supportsTaskStop || + left.supportsStopAll !== right.supportsStopAll ) { return false } diff --git a/tests/e2e/orchestration-idle-mail-delivery.spec.ts b/tests/e2e/orchestration-idle-mail-delivery.spec.ts index 8ca6657ab7c..cffa6415208 100644 --- a/tests/e2e/orchestration-idle-mail-delivery.spec.ts +++ b/tests/e2e/orchestration-idle-mail-delivery.spec.ts @@ -35,7 +35,6 @@ import { waitForPaneIdentitySnapshot } from './helpers/terminal' import { RuntimeClient, type RuntimeRpcSuccess } from '../../src/cli/runtime-client' -import { RuntimeRpcFailureError } from '../../src/cli/runtime/types' import type { RuntimeTerminalListResult } from '../../src/shared/runtime-types' import { CODEX_IDLE_TITLE, @@ -354,7 +353,7 @@ test.describe('orchestration push-on-idle mail delivery', () => { // #19542 deleted the legacy-Run write fallback, so a sender in no Run has // nowhere to file mail to a bare handle: the send is refused outright, which // is what keeps an unsafe pointer out of the pane on the next idle frame. - test('refuses unbound direct mail from a sender in no Run instead of pushing it', async ({ + test('keeps unbound direct mail durable without pointing to an unsafe check', async ({ orcaPage, electronApp }) => { @@ -363,40 +362,22 @@ test.describe('orchestration push-on-idle mail delivery', () => { const pane = await openAgentPane() await driveToLiveIdle(client, pane) + // Two plain terminals, neither in a Run: `send --to ` must still land durably. It + // files under the unbound Run, so a reopen never reads it as pre-Runs state (#19542 regression). const stdinBeforeScan = pane.agent.readStdin() - const refusal = await sendMail(client, pane.handle, { subject: 'Unbound direct mail' }).then( - () => undefined, - (error: unknown) => error - ) - // Why runtime_error and not run_required: insertMessage throws a plain - // Error, which the dispatcher passes through with its message and no - // recovery data — the sibling no-recipient path is the one that adds it. - // The request-id suffix and stamp are the client's durable-mutation bookkeeping. - expect(refusal).toBeInstanceOf(RuntimeRpcFailureError) - expect(refusal).toMatchObject({ - code: 'runtime_error', - message: expect.stringContaining('Run is required') - }) - // Pins that the SERVER attached no orchestrationSkillRecoveryData, without - // freezing whatever else the client may stamp alongside its request id. - const refusalData = (refusal as RuntimeRpcFailureError).data - expect(refusalData).toMatchObject({ orchestrationRequestId: expect.any(String) }) - expect(refusalData).not.toHaveProperty('effectsApplied') - expect(refusalData).not.toHaveProperty('guide') - expect(refusalData).not.toHaveProperty('nextCommandArgs') - expect(refusalData).not.toHaveProperty('nextSteps') - expect(readMailbox(userDataDir, pane.handle)).toEqual([]) - - // A busy→idle edge is the push trigger. Walking one proves the refusal left - // nothing behind for the scan to point at, not merely that the push was slow. + const messageId = await sendMail(client, pane.handle, { subject: 'Unbound direct mail' }) pane.agent.setTitle(CODEX_WORKING_TITLE) await waitForObservedTitle(client, pane.handle, CODEX_WORKING_TITLE) pane.agent.setTitle(CODEX_IDLE_TITLE) await waitForObservedTitle(client, pane.handle, CODEX_IDLE_TITLE) - // The ledger only proves anything once the push window has fully elapsed. await orcaPage.waitForTimeout(NO_DELIVERY_SETTLE_MS) - expect(readMailbox(userDataDir, pane.handle)).toEqual([]) + expect(readMailRow(userDataDir, messageId)).toMatchObject({ + to_handle: pane.handle, + run_id: 'run_unbound', + read: 0, + delivered_at: null + }) expect(pane.agent.readStdin()).toBe(stdinBeforeScan) })