From dbd2439cd4616e962bc2058c54cb0ed5ef0585ff Mon Sep 17 00:00:00 2001 From: Brennan Benson <79079362+brennanb2025@users.noreply.github.com> Date: Wed, 30 Sep 2026 02:57:50 -0700 Subject: [PATCH] fix(native-chat): a stop-all that times out stops asking, and "no children" is sent once A stop-all kept asking after a request timed out, so a Claude CLI that stopped answering control requests cost one full deadline per task, while the chat's sends, Stop and /compact waited behind it. A timeout now ends the loop; other request failures still let the remaining tasks be stopped. The chat strip channel never remembered that it had sent "no children", so every change in a chat with none re-sent that frame to each subscriber. It now remembers it, and forgets only when the conversation closes. Also renames agent-child-work-stop.ts to agent-child-work-stop-targets.ts, which says what it answers: the provider ids a background Stop reaches. --- .../claude-background-stop-records.test.ts | 2 +- .../claude-structured-control-actions.test.ts | 25 +++++++++++++++++++ .../claude-structured-control-actions.ts | 8 +++++- ...nt-session-background-task-channel.test.ts | 24 ++++++++++++++++-- ...d-agent-session-background-task-channel.ts | 10 +++----- .../structured-agent-session-turns-cancel.ts | 2 +- ...ructured-conversation-command-admission.ts | 2 +- src/shared/agent-child-row-model.ts | 2 +- ...op.ts => agent-child-work-stop-targets.ts} | 0 9 files changed, 62 insertions(+), 13 deletions(-) rename src/shared/{agent-child-work-stop.ts => agent-child-work-stop-targets.ts} (100%) diff --git a/src/main/claude/claude-background-stop-records.test.ts b/src/main/claude/claude-background-stop-records.test.ts index 092915dc1be..0092d9e2f29 100644 --- a/src/main/claude/claude-background-stop-records.test.ts +++ b/src/main/claude/claude-background-stop-records.test.ts @@ -3,7 +3,7 @@ // order through the real adapter and a real hook server. import { afterEach, describe, expect, it, vi } from 'vitest' -import { agentChildWorkStopTargets } from '../../shared/agent-child-work-stop' +import { agentChildWorkStopTargets } from '../../shared/agent-child-work-stop-targets' import type { AgentSessionRecord } from '../../shared/agent-session-record' import { conversationCommandBlocked } from '../native-chat/agent-session-wire/structured-conversation-command-admission' import type { CapturedFrame } from './claude-captured-frame-builders.test-fixture' diff --git a/src/main/claude/claude-structured-control-actions.test.ts b/src/main/claude/claude-structured-control-actions.test.ts index 7cc154f27a5..66913919a42 100644 --- a/src/main/claude/claude-structured-control-actions.test.ts +++ b/src/main/claude/claude-structured-control-actions.test.ts @@ -6,6 +6,10 @@ import { } from './claude-structured-control-actions' import { dispatchClaudeTurn } from './claude-structured-dispatch' import { ClaudeControlRequestError } from './claude-stream-json-connection' +import { + ClaudeControlRequestTimeoutError, + runClaudeControl +} from './claude-agent-sdk-control-requests' import { buildClaudePromptReply, ClaudePromptRegistry } from './claude-structured-prompt-replies' import type { ClaudeDispatchWaiter, ClaudeSession } from './claude-structured-session-state' import { ClaudeChildWorkDecoder } from './claude-child-work-decoder' @@ -389,4 +393,25 @@ describe('stopClaudeBackgroundTasks', () => { { type: 'ended', handle: { id: 'task-2' }, outcome: 'cancelled' } ]) }) + + it('stops asking after a request times out: a CLI not answering costs one deadline, not one per task', async () => { + const childWork = liveChildWork(['task-0', 'task-1', 'task-2', 'task-3', 'task-4']) + // Never answered, behind the real deadline wrapper, with a short deadline. + const stopTask = vi.fn((_taskId: string, options?: { timeoutMs?: number }) => + runClaudeControl('stop_task', () => new Promise(() => {}), options?.timeoutMs) + ) + const session = stoppingSession(childWork, stopTask) + + await expect( + stopClaudeBackgroundTasks(session, 50, () => true, [ + 'task-0', + 'task-1', + 'task-2', + 'task-3', + 'task-4' + ]) + ).rejects.toBeInstanceOf(ClaudeControlRequestTimeoutError) + expect(stopTask).toHaveBeenCalledTimes(1) + expect(childWork.drain(2)).toEqual([]) + }) }) diff --git a/src/main/claude/claude-structured-control-actions.ts b/src/main/claude/claude-structured-control-actions.ts index 6aac249c232..8c9c9c7afeb 100644 --- a/src/main/claude/claude-structured-control-actions.ts +++ b/src/main/claude/claude-structured-control-actions.ts @@ -1,6 +1,7 @@ import type { PermissionResult } from '@anthropic-ai/claude-agent-sdk' import type { ClaudePromptClaim } from './claude-structured-prompt-replies' import { ClaudeControlRequestError } from './claude-stream-json-connection' +import { ClaudeControlRequestTimeoutError } from './claude-agent-sdk-control-requests' import { settleCancelledClaudeDispatchWaiters } from './claude-structured-dispatch' import type { ClaudeLateDispatchSettlement } from './claude-replay-turn-resolution' import type { ClaudeSession } from './claude-structured-session-state' @@ -67,7 +68,9 @@ export async function stopClaudeBackgroundTasks( taskIds: readonly string[] ): Promise<{ cancelled: boolean }> { let cancelled = false - // A failed request for one task still leaves the others to stop; it is reported after them. + // A failed request for one task still leaves the others to stop; it is reported after them. A + // timeout ends the loop: a CLI that is not answering would make each id wait out its own + // deadline while the session's other actions queue behind this one. let failure: { error: unknown } | undefined for (const taskId of taskIds) { if (!isCurrent()) { @@ -78,6 +81,9 @@ export async function stopClaudeBackgroundTasks( session.childWork.stopAcknowledged(taskId) cancelled = true } catch (error) { + if (error instanceof ClaudeControlRequestTimeoutError) { + throw error + } if (!(error instanceof ClaudeControlRequestError)) { failure ??= { error } } diff --git a/src/main/native-chat/agent-session-wire/structured-agent-session-background-task-channel.test.ts b/src/main/native-chat/agent-session-wire/structured-agent-session-background-task-channel.test.ts index a021f635a56..129dbb64167 100644 --- a/src/main/native-chat/agent-session-wire/structured-agent-session-background-task-channel.test.ts +++ b/src/main/native-chat/agent-session-wire/structured-agent-session-background-task-channel.test.ts @@ -23,7 +23,7 @@ const child: AgentChildWorkView = { invocation: { invocationId: 'spawn-1', generation: 1 } } -function channelOver() { +function channelOver(children: () => AgentChildWorkView[] = () => [child]) { const sessions = new StructuredAgentSessionConversations({ deliver: () => {}, onDeliveryError: () => {}, @@ -42,7 +42,7 @@ function channelOver() { async () => { throw new Error('not opened here') }, - () => [child] + children ) // oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: the map reads only the journal's commit observer and the session's provider. const session = { @@ -67,4 +67,24 @@ describe('the background-task channel', () => { channel.publish('session-1') expect(sent).toHaveBeenCalledTimes(2) }) + + it('sends "no children" once, not again on every change that leaves none', () => { + let views: AgentChildWorkView[] = [] + const { sessions, sent, channel, session } = channelOver(() => views) + sessions.set('session-1', session) + channel.publish('session-1') + channel.publish('session-1') + expect(sent.mock.calls.map(([, state]) => state)).toEqual([null]) + + views = [child] + channel.publish('session-1') + views = [] + channel.publish('session-1') + channel.publish('session-1') + expect(sent.mock.calls.map(([, state]) => state?.children?.length ?? null)).toEqual([ + null, + 1, + null + ]) + }) }) diff --git a/src/main/native-chat/agent-session-wire/structured-agent-session-background-task-channel.ts b/src/main/native-chat/agent-session-wire/structured-agent-session-background-task-channel.ts index 7b8e9d12bb6..843aede4a3f 100644 --- a/src/main/native-chat/agent-session-wire/structured-agent-session-background-task-channel.ts +++ b/src/main/native-chat/agent-session-wire/structured-agent-session-background-task-channel.ts @@ -85,7 +85,7 @@ export class StructuredAgentSessionBackgroundTaskChannel { /** Re-read after the session's child records changed; an unchanged roster sends nothing. */ publish(sessionId: string): void { const session = this.sessions.get(sessionId) - // Explicit null once rows were sent, not silence: a reader keeps its last roster on + // Explicit null once a roster was sent, not silence: a reader keeps its last roster on // `undefined`, and a closing provider stops answering before its records are gone. const read = this.state(sessionId) const state = read === undefined && this.published.has(sessionId) ? null : read @@ -96,11 +96,9 @@ export class StructuredAgentSessionBackgroundTaskChannel { if (this.published.get(sessionId) === fingerprint) { return } - if (state === null) { - this.published.delete(sessionId) - } else { - this.published.set(sessionId, fingerprint) - } + // "None" is remembered too, so a session with no children sends it once, not on every change; + // the entry goes when the conversation closes. + this.published.set(sessionId, fingerprint) this.subscribers.backgroundTasks( sessionId, state, diff --git a/src/main/native-chat/agent-session-wire/structured-agent-session-turns-cancel.ts b/src/main/native-chat/agent-session-wire/structured-agent-session-turns-cancel.ts index 76b4a74fc9b..ee6e7bf1179 100644 --- a/src/main/native-chat/agent-session-wire/structured-agent-session-turns-cancel.ts +++ b/src/main/native-chat/agent-session-wire/structured-agent-session-turns-cancel.ts @@ -1,4 +1,4 @@ -import { agentChildWorkStopTargets } from '../../../shared/agent-child-work-stop' +import { agentChildWorkStopTargets } from '../../../shared/agent-child-work-stop-targets' import { agentSessionFailureFact } from '../../../shared/agent-session-failure' import type { AgentChildWorkView } from '../../../shared/agent-status-child-work-view' import { agentSessionFailureWords } from '../../../shared/agent-session-failure-words' 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 21419ffb32b..7aca4e23884 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 @@ -1,4 +1,4 @@ -import { agentChildWorkViewOffersStop } from '../../../shared/agent-child-work-stop' +import { agentChildWorkViewOffersStop } from '../../../shared/agent-child-work-stop-targets' import type { AgentSessionRecord } from '../../../shared/agent-session-record' import { isQueuedAgentJournalSubmission } from '../../../shared/agent-session-queued-submission' import { agentChildWorkLiveness } from '../../../shared/agent-status-child-work-liveness' diff --git a/src/shared/agent-child-row-model.ts b/src/shared/agent-child-row-model.ts index bbdc471f80d..c0dcfed2d04 100644 --- a/src/shared/agent-child-row-model.ts +++ b/src/shared/agent-child-row-model.ts @@ -19,7 +19,7 @@ import { deriveAgentChildDisplayState, type AgentChildDisplayState } from './agent-status-child-work-display' -import { agentChildWorkViewOffersStop } from './agent-child-work-stop' +import { agentChildWorkViewOffersStop } from './agent-child-work-stop-targets' import type { AgentChildWorkView } from './agent-status-child-work-view' import { agentStatusAuthorityObservedAt, diff --git a/src/shared/agent-child-work-stop.ts b/src/shared/agent-child-work-stop-targets.ts similarity index 100% rename from src/shared/agent-child-work-stop.ts rename to src/shared/agent-child-work-stop-targets.ts