diff --git a/src/cli/runtime/websocket-transport.test.ts b/src/cli/runtime/websocket-transport.test.ts index 4c178b71fd8..ed4330c8f1a 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_ROW_STOP_CAPABILITY, AGENT_SESSION_BACKGROUND_TASK_STOP_CAPABILITY, AGENT_SESSION_BOUNDARY_RUNTIME_CAPABILITY, AUTOMATION_OWNER_FENCING_RUNTIME_CAPABILITY, @@ -72,6 +73,7 @@ describe('CLI remote WebSocket transport', () => { expect.objectContaining({ clientCapabilities: [ AGENT_SESSION_BACKGROUND_TASK_STOP_CAPABILITY, + AGENT_SESSION_BACKGROUND_TASK_ROW_STOP_CAPABILITY, SESSION_TAB_CLOSE_INTENT_RUNTIME_CAPABILITY, SESSION_TABS_AUTHORITATIVE_INVENTORY_RUNTIME_CAPABILITY, AGENT_SESSION_BOUNDARY_RUNTIME_CAPABILITY, diff --git a/src/main/claude/claude-background-task-tracker.test.ts b/src/main/claude/claude-background-task-tracker.test.ts index df89a64a968..43912bfa560 100644 --- a/src/main/claude/claude-background-task-tracker.test.ts +++ b/src/main/claude/claude-background-task-tracker.test.ts @@ -21,6 +21,24 @@ function trackerAt(times: number[]): ClaudeBackgroundTaskTracker { return new ClaudeBackgroundTaskTracker(() => times[Math.min(index++, times.length - 1)]) } +/** The identity and stoppability of each published row, which is what the + * foreground cases below are about; `startedAt` and `state` have their own + * tests and would only make these brittle. */ +function rows(tracker: ClaudeBackgroundTaskTracker): { id: string; stoppable?: boolean }[] { + return (tracker.state?.tasks ?? []).map((task) => ({ + id: task.id, + ...(task.stoppable === undefined ? {} : { stoppable: task.stoppable }) + })) +} + +function started(id: string, backgrounded: boolean): Record { + return system('task_started', { + task_id: id, + task_type: 'local_agent', + is_backgrounded: backgrounded + }) +} + describe('ClaudeBackgroundTaskTracker', () => { it('classifies SDK task types without inferring them from descriptions', () => { expect(classifyClaudeBackgroundTaskKind('local_agent')).toBe('agent') @@ -581,4 +599,137 @@ describe('ClaudeBackgroundTaskTracker', () => { expect(tracker.clear()).toBe(true) expect(tracker.state).toBeNull() }) + it('marks a foreground row not stoppable and leaves a backgrounded row alone', () => { + // `stopTask` has no foreground target, so the row must not offer a Stop that + // resolves to an empty list and silently reports nothing cancelled. A + // backgrounded row stays untouched on the wire: absent means stoppable. + const tracker = new ClaudeBackgroundTaskTracker() + tracker.observe({ type: 'user' }, true) + tracker.observe(started('fore-1', false)) + tracker.observe(started('back-1', true)) + + expect(rows(tracker)).toEqual([{ id: 'fore-1', stoppable: false }, { id: 'back-1' }]) + expect(tracker.stoppableTaskIds).toEqual(['back-1']) + }) + + it('keeps live foreground work across an aggregate roster that never lists it', () => { + // `background_tasks_changed` enumerates BACKGROUNDED work only, so it is + // authoritative over that class alone. Treating it as the whole world wiped + // every in-flight foreground row and then dropped every later start. + const tracker = new ClaudeBackgroundTaskTracker() + tracker.observe({ type: 'user' }, true) + tracker.observe(started('fore-1', false)) + tracker.observe( + aggregate([{ task_id: 'back-1', task_type: 'local_bash', description: 'bash' }]) + ) + + // A retained row also keeps the place the user is already reading it in. + expect(rows(tracker)).toEqual([{ id: 'fore-1', stoppable: false }, { id: 'back-1' }]) + + // A foreground start after the roster is new work, not a stale echo. + tracker.observe(started('fore-2', false)) + expect(rows(tracker)).toEqual([ + { id: 'fore-1', stoppable: false }, + { id: 'back-1' }, + { id: 'fore-2', stoppable: false } + ]) + + // Turn end still retires the foreground rows and only those. + tracker.observe(result()) + expect(rows(tracker)).toEqual([{ id: 'back-1' }]) + }) + + it('drops a backgrounded start the roster no longer lists but bounds what it retains', () => { + const tracker = new ClaudeBackgroundTaskTracker() + tracker.observe({ type: 'user' }, true) + for (let index = 0; index < 300; index += 1) { + tracker.observe(started(`fore-${index}`, false)) + } + tracker.observe( + aggregate([{ task_id: 'back-1', task_type: 'local_bash', description: 'bash' }]) + ) + + const ids = rows(tracker).map((row) => row.id) + // 255 retained foreground rows plus the roster's own entry: retention is + // real and still counts against the cap. + expect(ids).toHaveLength(256) + // When the cap bites, the STALEST retained row goes, not the newest. + expect(ids).toContain('fore-299') + expect(ids).not.toContain('fore-44') + expect(ids).toContain('back-1') + + // Aggregate authority over its OWN class is unchanged. + tracker.observe(started('stale', true)) + expect(tracker.stoppableTaskIds).toEqual(['back-1']) + }) + + it('keeps a finished foreground id dead across a roster that never listed it', () => { + // The start guard only convicts BACKGROUNDED starts now, so terminal + // evidence is the only thing left defending a finished foreground id — and + // the roster carries no evidence about one, so it must not wipe it. + const tracker = new ClaudeBackgroundTaskTracker() + tracker.observe({ type: 'user' }, true) + tracker.observe(started('fore-1', false)) + tracker.observe(system('task_notification', { task_id: 'fore-1', status: 'completed' })) + expect(tracker.state).toBeNull() + + tracker.observe( + aggregate([{ task_id: 'back-1', task_type: 'local_bash', description: 'bash' }]) + ) + tracker.observe(started('fore-1', false)) + + expect(rows(tracker)).toEqual([{ id: 'back-1' }]) + }) + + it('retires a phantom foreground row when the next turn starts', () => { + // A foreground `task_started` with no turn open has no `result` coming to + // retire it, so it would sit in the strip — with no stop of its own — and + // refuse a conversation command. Turn start is the same evidence `result` + // is, and settling on it is cleanup only: nothing gates visibility on it. + const tracker = new ClaudeBackgroundTaskTracker() + tracker.observe(started('phantom', false)) + expect(rows(tracker)).toEqual([{ id: 'phantom', stoppable: false }]) + + tracker.observe({ type: 'user' }, true) + expect(tracker.state).toBeNull() + }) + + it('settles a previous turn the way the subagent roster settles it', () => { + // On this same frame the roster's `settleTurn` moves a still-working + // FOREGROUND child to `unverifiable` and leaves a backgrounded one alone. + // The strip has no `unverifiable` row, so keeping one would assert `live` + // for work Orca has already stopped vouching for. + const tracker = new ClaudeBackgroundTaskTracker() + tracker.observe({ type: 'user' }, true) + tracker.observe(started('fore', false)) + tracker.observe(started('back', true)) + + // No `result` for that turn; the next one starting is its only end. + tracker.observe({ type: 'user' }, true) + expect(rows(tracker)).toEqual([{ id: 'back' }]) + }) + + it('empties only between one task retiring and the next starting', () => { + // The strip's mid-turn unmount in a sequential fan-out is TRUTHFUL: A leaves + // on the provider's own terminal frame, B does not exist yet, and nothing + // sweeps A early. Foreground work is not retained as a settled row either, + // so an empty roster means no task is running. + const tracker = new ClaudeBackgroundTaskTracker() + tracker.observe({ type: 'user' }, true) + tracker.observe(started('A', false)) + expect(rows(tracker)).toEqual([{ id: 'A', stoppable: false }]) + tracker.observe(system('task_notification', { task_id: 'A', status: 'completed' })) + expect(tracker.state).toBeNull() + tracker.observe(started('B', false)) + expect(rows(tracker)).toEqual([{ id: 'B', stoppable: false }]) + + // Backgrounded work spanning the same gap holds the roster open, so an + // empty one is never work the strip is hiding. + const spanned = new ClaudeBackgroundTaskTracker() + spanned.observe({ type: 'user' }, true) + spanned.observe(started('bg', true)) + spanned.observe(started('A', false)) + spanned.observe(system('task_notification', { task_id: 'A', status: 'completed' })) + expect(rows(spanned)).toEqual([{ id: 'bg' }]) + }) }) diff --git a/src/main/claude/claude-background-task-tracker.ts b/src/main/claude/claude-background-task-tracker.ts index 68c7f4f1185..14f4271b7d8 100644 --- a/src/main/claude/claude-background-task-tracker.ts +++ b/src/main/claude/claude-background-task-tracker.ts @@ -66,6 +66,16 @@ export class ClaudeBackgroundTaskTracker { // Background work publishes through a foreground turn: the strip stays // honest mid-fan-out and the client alone decides when the idle-only // monitoring label may speak. + // + // A new turn is the same evidence `result` is: nothing the previous turn + // left foreground is still that turn's work. CLEANUP ONLY — a row's + // visibility never consults `startsTurn`, which is Orca's own + // dispatch-correlation bookkeeping and false by design for undispatched + // turns, so a missed one degrades to the old behaviour and can never hide + // live work. + if (startsTurn || message.type === 'result') { + this.settleForegroundTasks() + } if (message.type === 'system') { if (!this.observeSystemFrame(message) && !startsTurn) { return false @@ -84,6 +94,17 @@ export class ClaudeBackgroundTaskTracker { return this.refreshMonitoring() } + /** `result` is the outcome of every task the provider marked foreground, so + * they stop being live work. Backgrounded tasks outlive the turn and are + * never swept here — only their own terminal frame retires them. */ + private settleForegroundTasks(): void { + for (const task of this.tasks.values()) { + if (!task.backgrounded) { + task.liveInTurn = false + } + } + } + private settle( id: string, state: AgentSessionBackgroundTaskRunState, @@ -131,12 +152,17 @@ export class ClaudeBackgroundTaskTracker { this.finish(id) return true } - if (this.aggregateRosterObserved && !this.tasks.has(id)) { + const kind = classifyClaudeBackgroundTaskKind(message.task_type) + const backgrounded = + message.is_backgrounded === true || kind === 'workflow' || kind === 'monitor' + // The aggregate roster enumerates BACKGROUND work only, so it is authoritative + // over that class alone. A foreground start it could never have listed is not + // stale evidence, and dropping it here silently killed foreground rows. + if (this.aggregateRosterObserved && backgrounded && !this.tasks.has(id)) { return false } - const kind = classifyClaudeBackgroundTaskKind(message.task_type) this.upsert(id, { - backgrounded: message.is_backgrounded === true || kind === 'workflow' || kind === 'monitor', + backgrounded, kind, description: taskDescription(message.description), name: taskName(message), @@ -189,9 +215,9 @@ export class ClaudeBackgroundTaskTracker { const prior = new Map(this.tasks) this.aggregateRosterObserved = true this.tasks.clear() - this.terminalTaskIds.clear() + const roster = new Map() for (const valueTask of value) { - if (this.tasks.size >= MAX_TRACKED_TASKS) { + if (roster.size >= MAX_TRACKED_TASKS) { break } const task = record(valueTask) @@ -202,12 +228,16 @@ export class ClaudeBackgroundTaskTracker { if (!id) { continue } - // An authoritative live roster supersedes an earlier terminal edge. - const retained = this.retention.resume(id) - const existing = prior.get(id) ?? retained + // An authoritative live roster supersedes an earlier terminal edge — for + // the ids it actually lists. Wiping the whole set left a finished + // FOREGROUND id undefended, since the start guard now convicts only + // backgrounded starts. + this.terminalTaskIds.delete(id) + const existing = prior.get(id) ?? this.retention.resume(id) const kind = classifyClaudeBackgroundTaskKind(task.task_type) - this.tasks.set(id, { + roster.set(id, { backgrounded: true, + liveInTurn: true, kind: kind !== 'unknown' ? kind : (existing?.kind ?? 'unknown'), description: taskDescription(task.description) ?? existing?.description, name: taskName(task) ?? existing?.name, @@ -216,6 +246,28 @@ export class ClaudeBackgroundTaskTracker { totalTokens: existing?.totalTokens }) } + // Live foreground work is not in a BACKGROUND roster and is not superseded + // by one. Budget counted up front so eviction drops the STALEST retained + // rows rather than the newest, and roster entries are never starved. + const retainable = [...prior].filter( + ([id, task]) => !task.backgrounded && task.liveInTurn && !roster.has(id) + ) + let evict = Math.max(0, roster.size + retainable.length - MAX_TRACKED_TASKS) + // Retained rows keep their own relative order and stay ahead of the roster, + // so a live row the user is reading does not drop below it when a roster + // frame lands. Within the roster the PROVIDER's order wins — including for + // a task it reports live again, which belongs where the provider lists it + // rather than appended after the rows that outlived it. + for (const [id, task] of retainable) { + if (evict > 0) { + evict -= 1 + continue + } + this.tasks.set(id, task) + } + for (const [id, task] of roster) { + this.tasks.set(id, task) + } for (const [id, task] of prior) { if (task.backgrounded && !this.tasks.has(id)) { this.retention.rememberRemoved(id, task) @@ -223,7 +275,7 @@ export class ClaudeBackgroundTaskTracker { } } - private upsert(id: string, task: TrackedClaudeBackgroundTask): void { + private upsert(id: string, task: Omit): void { if (!this.tasks.has(id) && this.tasks.size >= MAX_TRACKED_TASKS) { let foregroundId: string | undefined for (const [candidateId, candidate] of this.tasks) { @@ -242,6 +294,8 @@ export class ClaudeBackgroundTaskTracker { if (existing) { this.tasks.set(id, { backgrounded: existing.backgrounded || task.backgrounded, + // A settled foreground task is not revived by a late edge frame. + liveInTurn: existing.liveInTurn, kind: task.kind !== 'unknown' ? task.kind : existing.kind, description: task.description ?? existing.description, name: task.name ?? existing.name, @@ -251,7 +305,7 @@ export class ClaudeBackgroundTaskTracker { }) return } - this.tasks.set(id, task) + this.tasks.set(id, { ...task, liveInTurn: true }) } private finish(id: string): void { @@ -284,7 +338,7 @@ export class ClaudeBackgroundTaskTracker { private backgroundTaskDetails(): AgentSessionBackgroundTask[] { const details: AgentSessionBackgroundTask[] = [] for (const [id, task] of this.tasks) { - if (!task.backgrounded) { + if (!task.backgrounded && !task.liveInTurn) { continue } details.push(claudeBackgroundTaskDetail(id, task)) diff --git a/src/main/claude/claude-settled-background-tasks.ts b/src/main/claude/claude-settled-background-tasks.ts index 91e975d07bb..73064becec4 100644 --- a/src/main/claude/claude-settled-background-tasks.ts +++ b/src/main/claude/claude-settled-background-tasks.ts @@ -17,6 +17,10 @@ const MAX_RETAINED_TASKS = 256 export type TrackedClaudeBackgroundTask = { backgrounded: boolean + /** Foreground work is turn-scoped: the provider's `result` (or the next turn + * starting) is its outcome, so it stays visible only until that frame. + * Backgrounded work ignores this and is retired only by its own edge. */ + liveInTurn: boolean kind: AgentSessionBackgroundTask['kind'] description?: string name?: string @@ -38,7 +42,9 @@ export function claudeBackgroundTaskDetail( ...(task.name ? { name: task.name } : {}), state: task.state ?? (task.kind === 'monitor' ? 'monitoring' : 'working'), startedAt: task.startedAt, - ...(task.totalTokens !== undefined ? { totalTokens: task.totalTokens } : {}) + ...(task.totalTokens !== undefined ? { totalTokens: task.totalTokens } : {}), + // Only a backgrounded row has a stop the host can target; absent means yes. + ...(task.backgrounded ? {} : { stoppable: false }) } } @@ -103,7 +109,15 @@ export class ClaudeSettledBackgroundTasks { if (!source || source.startedAt === undefined) { return undefined } - return { ...source, backgrounded: true, state: undefined, startedAt: source.startedAt } + // Positive live evidence, so it re-enters live in this turn too; a resumed + // task is always backgrounded, which is what actually gates its visibility. + return { + ...source, + backgrounded: true, + liveInTurn: true, + state: undefined, + startedAt: source.startedAt + } } get hasSettled(): boolean { diff --git a/src/main/codex/codex-background-command-tracker.ts b/src/main/codex/codex-background-command-tracker.ts index becbb18a67c..844134881ad 100644 --- a/src/main/codex/codex-background-command-tracker.ts +++ b/src/main/codex/codex-background-command-tracker.ts @@ -10,9 +10,33 @@ 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. */ +/** The label's reserved share of the description. Reserved, not merely capped: + * a label free to spend the whole budget clips away the command it qualifies, + * leaving a command row naming an agent and no command — the failure this + * qualification exists to remove, in the other direction. `bytes` is counted + * before qualification, so this share is also what a published row may exceed + * the admitted count by. */ +const MAX_LABEL_CHARS = 96 + +/** Every cut in this file goes through here, clipped the way `boundSubagentField` + * clips the same provider string on the agent row: never mid surrogate pair, + * since a lone surrogate is lossy through any non-JSON UTF-8 hop. A composed + * row is cut a SECOND time, so a clip that is safe only where the label is + * bounded is not safe. No ordinal, because a row's identity is its `id`. */ +function boundText(value: string, max: number): string { + if (value.length <= max) { + return value + } + const keep = max - 1 + const last = value.charCodeAt(keep - 1) + const end = last >= 0xd800 && last <= 0xdbff ? keep - 1 : keep + return `${value.slice(0, end)}…` +} + +/** Resolved on read, and capped at the bound the admitted description already respects. */ function qualifiedDescription(label: string, description: string | undefined): string { - return (description ? `${label} — ${description}` : label).slice(0, MAX_DESCRIPTION_CHARS) + const name = boundText(label, MAX_LABEL_CHARS) + return boundText(description ? `${name} — ${description}` : name, MAX_DESCRIPTION_CHARS) } export class CodexBackgroundCommandTracker { @@ -122,8 +146,7 @@ export class CodexBackgroundCommandTracker { } 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) + const description = boundText(readString(item, 'command') ?? '', MAX_DESCRIPTION_CHARS) .replace(/\s+/g, ' ') .trim() const value = { diff --git a/src/main/codex/codex-background-task-tracker.test.ts b/src/main/codex/codex-background-task-tracker.test.ts index 47987fe3fc0..0a70442a0b3 100644 --- a/src/main/codex/codex-background-task-tracker.test.ts +++ b/src/main/codex/codex-background-task-tracker.test.ts @@ -22,7 +22,8 @@ function turn( function activity( kind = 'started', parentTurn = PARENT_TURN, - child = CHILD + child = CHILD, + name = 'count_a' ): CodexBackgroundTaskEvent { return { method: 'item/started', @@ -35,7 +36,7 @@ function activity( id: `activity-${kind}`, kind, agentThreadId: child, - agentPath: '/root/count_a' + agentPath: `/root/${name}` } } } @@ -49,7 +50,11 @@ function runningChild(): CodexBackgroundTaskTracker { return tracker } -function command(threadId = PRIMARY, method = 'item/started'): CodexBackgroundTaskEvent { +function command( + threadId = PRIMARY, + method = 'item/started', + commandText = 'sleep 90' +): CodexBackgroundTaskEvent { return { method, threadId, @@ -61,7 +66,7 @@ function command(threadId = PRIMARY, method = 'item/started'): CodexBackgroundTa id: 'exec-1', processId: '71831', source: 'unifiedExecStartup', - command: 'sleep 90', + command: commandText, status: method === 'item/started' ? 'inProgress' : 'completed' } } @@ -103,15 +108,19 @@ describe('CodexBackgroundTaskTracker child execution ownership', () => { expect(tracker.state).toBeNull() }) - it('reports an executing child only after the foreground turn ends', () => { + it('reports an executing child while the spawning turn is still open', () => { const tracker = runningChild() - expect(tracker.state).toBeNull() - expect(tracker.observe(turn('turn/completed', PRIMARY, PARENT_TURN))).toBe(true) - expect(tracker.state).toEqual({ + const running = { state: 'monitoring', supportsStopAll: false, tasks: [{ id: `codex-agent:${CHILD}`, kind: 'agent', description: 'count_a' }] - }) + } + // The strip is a live view: a fan-out is reported while it runs, not once + // the parent turn happens to end. + expect(tracker.state).toEqual(running) + // Turn end reveals children, it never settles them; the child is unchanged. + expect(tracker.observe(turn('turn/completed', PRIMARY, PARENT_TURN))).toBe(false) + expect(tracker.state).toEqual(running) }) it('never settles a child when a primary turn ends', () => { @@ -220,15 +229,16 @@ describe('CodexBackgroundTaskTracker child execution ownership', () => { }) describe('CodexBackgroundTaskTracker command integration', () => { - it('keeps a primary shell visible after the turn until its own completion', () => { + it('keeps a primary shell visible from launch until its own completion', () => { const tracker = new CodexBackgroundTaskTracker(PRIMARY) + const shell = [{ id: 'codex-command:primary:exec-1', kind: 'command', description: 'sleep 90' }] tracker.observe(turn('turn/started', PRIMARY, PARENT_TURN)) tracker.observe(command()) - expect(tracker.state).toBeNull() + // Visible while the turn that launched it is still running. + expect(tracker.state?.tasks).toEqual(shell) tracker.observe(turn('turn/completed', PRIMARY, PARENT_TURN)) - expect(tracker.state?.tasks).toEqual([ - { id: 'codex-command:primary:exec-1', kind: 'command', description: 'sleep 90' } - ]) + expect(tracker.state?.tasks).toEqual(shell) + // Only the shell's own completion retires the row. tracker.observe(command(PRIMARY, 'item/completed')) expect(tracker.state).toBeNull() }) @@ -261,6 +271,63 @@ describe('CodexBackgroundTaskTracker command integration', () => { }) }) + it('keeps the command visible under a label that would otherwise fill the row', () => { + const tracker = new CodexBackgroundTaskTracker(PRIMARY) + tracker.observe(turn('turn/started', PRIMARY, PARENT_TURN)) + tracker.observe(turn('turn/started', CHILD, CHILD_TURN)) + tracker.observe(activity('started', PARENT_TURN, CHILD, 'L'.repeat(600))) + tracker.observe(command(CHILD)) + tracker.observe(turn('turn/completed', PRIMARY, PARENT_TURN)) + tracker.observe(turn('turn/completed', CHILD, CHILD_TURN)) + const description = tracker.state?.tasks?.[0]?.description + expect(description).toContain('sleep 90') + expect(description).toBe(`${'L'.repeat(95)}… — sleep 90`) + }) + + it('never cuts a label mid surrogate pair', () => { + const tracker = new CodexBackgroundTaskTracker(PRIMARY) + tracker.observe(turn('turn/started', PRIMARY, PARENT_TURN)) + tracker.observe(turn('turn/started', CHILD, CHILD_TURN)) + tracker.observe(activity('started', PARENT_TURN, CHILD, `${'L'.repeat(94)}\u{1F600}bad`)) + tracker.observe(command(CHILD)) + tracker.observe(turn('turn/completed', PRIMARY, PARENT_TURN)) + tracker.observe(turn('turn/completed', CHILD, CHILD_TURN)) + const description = tracker.state?.tasks?.[0]?.description ?? '' + expect(description.isWellFormed()).toBe(true) + expect(description).toBe(`${'L'.repeat(94)}… — sleep 90`) + }) + + it('never cuts a qualified command mid surrogate pair', () => { + // The label is bounded, then the COMPOSED row is bounded again. That second + // cut lands inside the description, so clipping only the label side leaves a + // lone surrogate — lossy through any non-JSON UTF-8 hop. + const tracker = new CodexBackgroundTaskTracker(PRIMARY) + tracker.observe(turn('turn/started', PRIMARY, PARENT_TURN)) + tracker.observe(turn('turn/started', CHILD, CHILD_TURN)) + tracker.observe(activity('started', PARENT_TURN, CHILD, 'L'.repeat(96))) + // Places the pair exactly where a raw slice of the composed row splits it. + tracker.observe(command(CHILD, 'item/started', `${'C'.repeat(412)}\u{1F600}${'D'.repeat(200)}`)) + tracker.observe(turn('turn/completed', PRIMARY, PARENT_TURN)) + tracker.observe(turn('turn/completed', CHILD, CHILD_TURN)) + const description = tracker.state?.tasks?.[0]?.description ?? '' + expect(description.length).toBeLessThanOrEqual(512) + expect(description.startsWith(`${'L'.repeat(96)} — `)).toBe(true) + expect(description.isWellFormed()).toBe(true) + }) + + it('never cuts an unqualified primary command mid surrogate pair', () => { + const tracker = new CodexBackgroundTaskTracker(PRIMARY) + tracker.observe(turn('turn/started', PRIMARY, PARENT_TURN)) + // The pair straddles the raw description bound itself. + tracker.observe( + command(PRIMARY, 'item/started', `${'C'.repeat(511)}\u{1F600}${'D'.repeat(50)}`) + ) + tracker.observe(turn('turn/completed', PRIMARY, PARENT_TURN)) + const description = tracker.state?.tasks?.[0]?.description ?? '' + expect(description.length).toBeLessThanOrEqual(512) + expect(description.isWellFormed()).toBe(true) + }) + 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)) diff --git a/src/main/codex/codex-background-task-tracker.ts b/src/main/codex/codex-background-task-tracker.ts index 2918f087809..a972b7bb4c1 100644 --- a/src/main/codex/codex-background-task-tracker.ts +++ b/src/main/codex/codex-background-task-tracker.ts @@ -12,7 +12,6 @@ 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 @@ -44,29 +43,22 @@ export class CodexBackgroundTaskTracker { } 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 { + } else if (frame.threadId !== this.primaryThreadId) { this.executions.observeTurn(frame.threadId, frame.turnId, frame.state) } + // A primary-turn frame only prompts a republish: turn end reveals children, + // it never settles them. Codex `spawn_agent` children keep reporting well + // past their parent turn, so nothing here may sweep the roster. 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}`, diff --git a/src/main/codex/codex-structured-session-background-tasks.test.ts b/src/main/codex/codex-structured-session-background-tasks.test.ts index aa6156027e8..380abb1da6f 100644 --- a/src/main/codex/codex-structured-session-background-tasks.test.ts +++ b/src/main/codex/codex-structured-session-background-tasks.test.ts @@ -215,29 +215,27 @@ describe('codex background tasks reach the strip', () => { } }) - it('publishes the orphaned fan-out once the spawning turn completes', async () => { + it('publishes the fan-out while it runs and keeps it past the spawning turn', async () => { const published: { sessionId: string; state: AgentSessionBackgroundTaskState | null }[] = [] const { adapter, codex } = await adapterWithSession(published) + const running = { + state: 'monitoring', + supportsStopAll: false, + tasks: [{ id: `codex-agent:${CHILD_ID}`, kind: 'agent', description: 'count_a' }] + } 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() + // Mid-turn: the child is running, so the strip reports it now. + expect(published).toEqual([{ sessionId: 'session-1', state: running }]) + expect(adapter.backgroundTaskState('session-1')).toEqual(running) + published.length = 0 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) + // Turn end is not the child's outcome: no republish and no settle. + expect(published).toEqual([]) + expect(adapter.backgroundTaskState('session-1')).toEqual(running) }) it('clears the strip when the session closes', async () => { diff --git a/src/main/ipc/runtime.ts b/src/main/ipc/runtime.ts index 6237b8d040d..81de25d66c0 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_ROW_STOP_CAPABILITY, AGENT_SESSION_BACKGROUND_TASK_STOP_CAPABILITY, CLAUDE_STRUCTURED_AGENT_SESSION_RUNTIME_CAPABILITY, STRUCTURED_AGENT_SESSION_RUNTIME_CAPABILITY @@ -82,6 +83,7 @@ export function registerRuntimeHandlers(runtime: OrcaRuntimeService): void { connectionId: desktopSenders.connectionIdFor(event.sender), clientCapabilities: [ AGENT_SESSION_BACKGROUND_TASK_STOP_CAPABILITY, + AGENT_SESSION_BACKGROUND_TASK_ROW_STOP_CAPABILITY, STRUCTURED_AGENT_SESSION_RUNTIME_CAPABILITY, CLAUDE_STRUCTURED_AGENT_SESSION_RUNTIME_CAPABILITY ] @@ -131,6 +133,7 @@ export function registerRuntimeHandlers(runtime: OrcaRuntimeService): void { connectionId, clientCapabilities: [ AGENT_SESSION_BACKGROUND_TASK_STOP_CAPABILITY, + AGENT_SESSION_BACKGROUND_TASK_ROW_STOP_CAPABILITY, STRUCTURED_AGENT_SESSION_RUNTIME_CAPABILITY, CLAUDE_STRUCTURED_AGENT_SESSION_RUNTIME_CAPABILITY ] 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 index 42d03f46b5b..5a3370c910f 100644 --- 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 @@ -45,4 +45,26 @@ describe('conversationCommandBlocked background tasks', () => { ) expect(blocked).toBe('Wait for background tasks to finish before using this command.') }) + + it('still refuses on the open turn, not on the work the strip now shows', () => { + // The strip reports subagents while a turn runs. That must not change which + // refusal the user sees: an open turn already refuses, and it refuses first, + // so a live fan-out never re-labels the reason or blocks anything new. + const ctx = contextWith({ state: 'monitoring', supportsTaskStop: true }) + ctx.journal.snapshot = () => + ({ + items: [ + { + id: 'turn-1', + body: { + kind: 'status', + turnLifecycle: { turnId: 'turn-1', state: 'running' } + } + } + ] + }) as unknown as ReturnType + expect(conversationCommandBlocked(ctx, RECORD)).toBe( + 'Wait for the current turn to finish before using this command.' + ) + }) }) 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 index cc5a7282080..2b62b00525e 100644 --- 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 @@ -1,6 +1,9 @@ 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 { + AGENT_SESSION_BACKGROUND_TASK_ROW_STOP_CAPABILITY, + 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 { @@ -24,6 +27,20 @@ const CURRENT_CLIENT = { ...STRUCTURED_CLIENT, clientCapabilities: remoteRuntimeClientCapabilities(STRUCTURED_CLIENT.clientCapabilities) } +/** Understands a stopless roster, but predates per-row stoppability. */ +const STOP_ONLY_CLIENT = { + ...STRUCTURED_CLIENT, + clientCapabilities: remoteRuntimeClientCapabilities(STRUCTURED_CLIENT.clientCapabilities).filter( + (capability) => capability !== AGENT_SESSION_BACKGROUND_TASK_ROW_STOP_CAPABILITY + ) +} +const FOREGROUND_ROW = { id: 'fore', kind: 'agent', stoppable: false } as const +const BACKGROUNDED_ROW = { id: 'back', kind: 'agent' } as const +const MIXED_ROWS: AgentSessionBackgroundTaskState = { + state: 'monitoring', + supportsTaskStop: true, + tasks: [FOREGROUND_ROW, BACKGROUNDED_ROW] +} describe('background-task stop capability at the RPC boundary', () => { it('advertises reader support on remote requests and subscriptions', () => { @@ -93,6 +110,58 @@ describe('background-task stop capability at the RPC boundary', () => { } ) + it('advertises row-stop support separately from stop support', () => { + // A client can advertise the stop capability and still predate `stoppable`, + // so the two must not be conflated. + expect(CURRENT_CLIENT.clientCapabilities).toContain( + AGENT_SESSION_BACKGROUND_TASK_ROW_STOP_CAPABILITY + ) + expect(STOP_ONLY_CLIENT.clientCapabilities).not.toContain( + AGENT_SESSION_BACKGROUND_TASK_ROW_STOP_CAPABILITY + ) + }) + + it.each([ + ['row-stop reader', () => CURRENT_CLIENT, MIXED_ROWS], + [ + 'stop-only reader', + () => STOP_ONLY_CLIENT, + { state: 'monitoring', tasks: [BACKGROUNDED_ROW] } + ], + ['in-process reader', () => undefined, MIXED_ROWS] + ] as const)('projects unstoppable rows for a %s', async (_label, client, expected) => { + hostCalls.history.mockReturnValue({ + ok: true, + page: { items: [], backgroundTasks: MIXED_ROWS } + }) + expect( + await call('agentSession.history', { sessionId: SESSION, direction: 'tail' }, client()) + ).toMatchObject({ ok: true, result: { page: { backgroundTasks: expected } } }) + }) + + it('hands a reader that predates the field no strip when every row is unstoppable', async () => { + // Its pre-feature view exactly: the host published no foreground rows at all. + const foregroundOnly = { + state: 'monitoring' as const, + supportsTaskStop: true, + tasks: [{ id: 'fore', kind: 'agent' as const, stoppable: false }] + } + hostCalls.history.mockReturnValue({ + ok: true, + page: { items: [], backgroundTasks: foregroundOnly } + }) + expect( + await call( + 'agentSession.history', + { sessionId: SESSION, direction: 'tail' }, + STOP_ONLY_CLIENT + ) + ).toMatchObject({ ok: true, result: { page: { backgroundTasks: null } } }) + expect( + await call('agentSession.history', { sessionId: SESSION, direction: 'tail' }, CURRENT_CLIENT) + ).toMatchObject({ ok: true, result: { page: { backgroundTasks: foregroundOnly } } }) + }) + 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 } }) 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 index 02afc5569bf..ae69de4a069 100644 --- 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 @@ -3,7 +3,10 @@ import type { AgentSessionHistoryResult, AgentSessionSubscribeEvent } from '../../../../shared/agent-session-wire' -import { AGENT_SESSION_BACKGROUND_TASK_STOP_CAPABILITY } from '../../../../shared/protocol-version' +import { + AGENT_SESSION_BACKGROUND_TASK_ROW_STOP_CAPABILITY, + AGENT_SESSION_BACKGROUND_TASK_STOP_CAPABILITY +} from '../../../../shared/protocol-version' import type { RpcContext } from '../core' type BackgroundTaskReader = Pick @@ -15,14 +18,37 @@ function supportsReadOnlyTasks(ctx: BackgroundTaskReader): boolean { ) } +function honoursRowStop(ctx: BackgroundTaskReader): boolean { + return ( + ctx.clientKind === undefined || + ctx.clientCapabilities?.includes(AGENT_SESSION_BACKGROUND_TASK_ROW_STOP_CAPABILITY) === true + ) +} + +/** A reader that predates `stoppable` draws a per-row stop on every row it is + * handed, and the host cannot honour one on a row marked unstoppable — the + * dead button the field exists to remove. The host publishing such rows at all + * is new, so withholding them hands that reader exactly its pre-feature view; + * a state whose every row is withheld becomes no strip, as it was. */ +function withoutUnstoppableRows( + state: AgentSessionBackgroundTaskState +): AgentSessionBackgroundTaskState | null { + if (!state.tasks?.some((task) => task.stoppable === false)) { + return state + } + const tasks = state.tasks.filter((task) => task.stoppable !== false) + return tasks.length > 0 ? { ...state, tasks } : null +} + function projectState( state: AgentSessionBackgroundTaskState | null | undefined, ctx: BackgroundTaskReader ): AgentSessionBackgroundTaskState | null | undefined { + const rows = !state || honoursRowStop(ctx) ? state : withoutUnstoppableRows(state) // Legacy readers always offer a stop; retain their pre-producer empty strip. - return state?.supportsStopAll === false && !state.supportsTaskStop && !supportsReadOnlyTasks(ctx) + return rows?.supportsStopAll === false && !rows.supportsTaskStop && !supportsReadOnlyTasks(ctx) ? null - : state + : rows } export function projectBackgroundTaskHistory( diff --git a/src/renderer/src/components/native-chat/NativeChatBackgroundTasksStatus.test.tsx b/src/renderer/src/components/native-chat/NativeChatBackgroundTasksStatus.test.tsx index 96be91820a3..365be31cc47 100644 --- a/src/renderer/src/components/native-chat/NativeChatBackgroundTasksStatus.test.tsx +++ b/src/renderer/src/components/native-chat/NativeChatBackgroundTasksStatus.test.tsx @@ -3,7 +3,7 @@ import '@testing-library/jest-dom/vitest' import { act, cleanup, fireEvent, render, screen, within } from '@testing-library/react' -import { Profiler } from 'react' +import { Profiler, useState } from 'react' import { afterEach, describe, expect, it, vi } from 'vitest' import type { AgentSessionBackgroundTask } from '../../../../shared/agent-session-wire' import { NativeChatBackgroundTasksStatus } from './NativeChatBackgroundTasksStatus' @@ -13,6 +13,24 @@ afterEach(() => { vi.useRealTimers() }) +/** The strip's disclosure is parent-owned, because the strip unmounts whenever + * live work momentarily drops to nothing; this stands in for that owner. */ +function DisclosureHost( + props: Omit< + Parameters[0], + 'expanded' | 'onExpandedChange' + > +): React.JSX.Element { + const [expanded, setExpanded] = useState(false) + return ( + + ) +} + const TASKS: AgentSessionBackgroundTask[] = [ { id: 'codex-agent:child-1', kind: 'agent', description: 'count_a' }, { id: 'codex-command:exec-1', kind: 'command', description: 'sleep 90' } @@ -23,7 +41,7 @@ function renderStrip(props: { supportsTaskStop: boolean; supportsStopAll: boolea } { const onStop = vi.fn() render( - { expect(screen.getByLabelText('Stop background tasks')).toBeInTheDocument() }) + it('withholds a row stop the host reported it cannot act on', () => { + // Claude publishes in-turn foreground rows with `stoppable: false`: the + // session accepts targeted stops, but `stopTask` has no target for this row, + // so a Stop here resolves to an empty list and reports nothing cancelled. + render( + + ) + fireEvent.click(screen.getByRole('button', { expanded: false })) + + expect(screen.getByText('in-turn subagent')).toBeInTheDocument() + expect(screen.queryByLabelText('Stop in-turn subagent')).not.toBeInTheDocument() + expect(screen.getByLabelText('Stop backgrounded subagent')).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 }) @@ -64,7 +109,7 @@ describe('NativeChatBackgroundTasksStatus stop affordances', () => { describe('background-tasks strip header', () => { function renderHeader(tasks: AgentSessionBackgroundTask[]): HTMLElement { render( - { it('dims the monitor amber while a turn owns the voice', () => { render( - { // usage it ended on, and stops claiming a clock or a stop control. it('keeps a settled row with its final usage, no clock and no stop', () => { render( - { describe('background-task row reasons', () => { function expandedRows(tasks: AgentSessionBackgroundTask[]): HTMLElement[] { render( -