mirror of
https://github.com/NanmiCoder/cc-haha
synced 2026-07-17 13:13:35 +08:00
fix(agent): stabilize background task continuations
Fixes #870, #884, #886. Route backgrounded sync Agent tool activity through the same parent-tagged event path as detached agents, recover incomplete tool-use streams with the non-streaming fallback, and suppress structured-output local-agent terminal notifications from re-entering the model. Tested: bun test src/tools/AgentTool/agentToolUtils.test.ts src/utils/taskNotificationPolicy.test.ts src/services/api/streamFallback.test.ts src/server/__tests__/proxy-streaming.test.ts Tested: bun run check:server Confidence: high Scope-risk: moderate
This commit is contained in:
parent
a090070352
commit
c2355205dc
@ -238,6 +238,10 @@ import {
|
||||
runElicitationResultHooks,
|
||||
} from 'src/services/mcp/elicitationHandler.js'
|
||||
import { executeNotificationHooks } from 'src/utils/hooks.js'
|
||||
import {
|
||||
parseTaskNotificationXml,
|
||||
shouldForwardTaskNotificationToModel,
|
||||
} from 'src/utils/taskNotificationPolicy.js'
|
||||
import {
|
||||
ElicitRequestSchema,
|
||||
ElicitationCompleteNotificationSchema,
|
||||
@ -1005,6 +1009,7 @@ function runHeadlessStreaming(
|
||||
setSDKStatus?: (status: SDKStatus) => void
|
||||
promptSuggestions?: boolean | undefined
|
||||
workload?: string | undefined
|
||||
outputFormat: string | undefined
|
||||
},
|
||||
turnInterruptionState?: TurnInterruptionState,
|
||||
): AsyncIterable<StdoutMessage> {
|
||||
@ -2008,61 +2013,14 @@ function runHeadlessStreaming(
|
||||
notifyCommandLifecycle(uuid, 'started')
|
||||
}
|
||||
|
||||
// Task notifications arrive when background agents complete.
|
||||
// Emit an SDK system event for SDK consumers, then fall through
|
||||
// to ask() so the model sees the agent result and can act on it.
|
||||
// This matches TUI behavior where useQueueProcessor always feeds
|
||||
// notifications to the model regardless of coordinator mode.
|
||||
// Task notifications arrive when background tasks complete.
|
||||
// Emit SDK system events for structured consumers, then only feed
|
||||
// notifications back to the model for task types that still need a
|
||||
// model follow-up (for example background shell commands).
|
||||
if (command.mode === 'task-notification') {
|
||||
const notificationText =
|
||||
typeof command.value === 'string' ? command.value : ''
|
||||
// Parse the XML-formatted notification
|
||||
const taskIdMatch = notificationText.match(
|
||||
/<task-id>([^<]+)<\/task-id>/,
|
||||
)
|
||||
const toolUseIdMatch = notificationText.match(
|
||||
/<tool-use-id>([^<]+)<\/tool-use-id>/,
|
||||
)
|
||||
const outputFileMatch = notificationText.match(
|
||||
/<output-file>([^<]+)<\/output-file>/,
|
||||
)
|
||||
const statusMatch = notificationText.match(
|
||||
/<status>([^<]+)<\/status>/,
|
||||
)
|
||||
const summaryMatch = notificationText.match(
|
||||
/<summary>([^<]+)<\/summary>/,
|
||||
)
|
||||
const resultMatch = notificationText.match(
|
||||
/<result>([\s\S]*?)<\/result>/,
|
||||
)
|
||||
|
||||
const isValidStatus = (
|
||||
s: string | undefined,
|
||||
): s is 'completed' | 'failed' | 'stopped' | 'killed' =>
|
||||
s === 'completed' ||
|
||||
s === 'failed' ||
|
||||
s === 'stopped' ||
|
||||
s === 'killed'
|
||||
const rawStatus = statusMatch?.[1]
|
||||
const status = isValidStatus(rawStatus)
|
||||
? rawStatus === 'killed'
|
||||
? 'stopped'
|
||||
: rawStatus
|
||||
: 'completed'
|
||||
|
||||
const usageMatch = notificationText.match(
|
||||
/<usage>([\s\S]*?)<\/usage>/,
|
||||
)
|
||||
const usageContent = usageMatch?.[1] ?? ''
|
||||
const totalTokensMatch = usageContent.match(
|
||||
/<total_tokens>(\d+)<\/total_tokens>/,
|
||||
)
|
||||
const toolUsesMatch = usageContent.match(
|
||||
/<tool_uses>(\d+)<\/tool_uses>/,
|
||||
)
|
||||
const durationMsMatch = usageContent.match(
|
||||
/<duration_ms>(\d+)<\/duration_ms>/,
|
||||
)
|
||||
const notification = parseTaskNotificationXml(notificationText)
|
||||
|
||||
// Only emit a task_notification SDK event when a <status> tag is
|
||||
// present — that means this is a terminal notification (completed/
|
||||
@ -2071,31 +2029,31 @@ function runHeadlessStreaming(
|
||||
// default to 'completed' and falsely close the task for SDK
|
||||
// consumers. Terminal bookends are now emitted directly via
|
||||
// emitTaskTerminatedSdk, so skipping statusless events is safe.
|
||||
if (statusMatch) {
|
||||
if (notification.status) {
|
||||
output.enqueue({
|
||||
type: 'system',
|
||||
subtype: 'task_notification',
|
||||
task_id: taskIdMatch?.[1] ?? '',
|
||||
tool_use_id: toolUseIdMatch?.[1],
|
||||
status,
|
||||
output_file: outputFileMatch?.[1] ?? '',
|
||||
summary: summaryMatch?.[1] ?? '',
|
||||
result: resultMatch?.[1],
|
||||
usage:
|
||||
totalTokensMatch && toolUsesMatch
|
||||
? {
|
||||
total_tokens: parseInt(totalTokensMatch[1]!, 10),
|
||||
tool_uses: parseInt(toolUsesMatch[1]!, 10),
|
||||
duration_ms: durationMsMatch
|
||||
? parseInt(durationMsMatch[1]!, 10)
|
||||
: 0,
|
||||
}
|
||||
: undefined,
|
||||
task_id: notification.taskId,
|
||||
tool_use_id: notification.toolUseId,
|
||||
status: notification.status,
|
||||
output_file: notification.outputFile,
|
||||
summary: notification.summary,
|
||||
result: notification.result,
|
||||
usage: notification.usage,
|
||||
session_id: getSessionId(),
|
||||
uuid: randomUUID(),
|
||||
})
|
||||
}
|
||||
// No continue -- fall through to ask() so the model processes the result
|
||||
if (
|
||||
!shouldForwardTaskNotificationToModel(notification, {
|
||||
structuredOutput: options.outputFormat === 'stream-json',
|
||||
})
|
||||
) {
|
||||
for (const uuid of batchUuids) {
|
||||
notifyCommandLifecycle(uuid, 'completed')
|
||||
}
|
||||
continue
|
||||
}
|
||||
}
|
||||
|
||||
const input = command.value
|
||||
|
||||
@ -211,6 +211,7 @@ import {
|
||||
startSessionActivity,
|
||||
stopSessionActivity,
|
||||
} from "../../utils/sessionActivity.js";
|
||||
import { shouldTriggerNonStreamingFallbackForEmptyStream } from "./streamFallback.js";
|
||||
import { jsonStringify } from "../../utils/slowOperations.js";
|
||||
import {
|
||||
isBetaTracingEnabled,
|
||||
@ -2496,11 +2497,22 @@ async function* queryModel(
|
||||
// Note: We must check stopReason to avoid false positives. For example, with
|
||||
// structured output (--json-schema), the model calls a StructuredOutput tool
|
||||
// on turn 1, then on turn 2 responds with end_turn and no content blocks.
|
||||
// That's a legitimate empty response, not an incomplete stream.
|
||||
if (!partialMessage || (newMessages.length === 0 && !stopReason)) {
|
||||
// That's a legitimate empty response, not an incomplete stream. However,
|
||||
// stop_reason=tool_use with no completed tool block is incomplete: some
|
||||
// OpenAI-compatible streams send only finish_reason=tool_calls, and we
|
||||
// need the non-streaming fallback to recover the full tool call.
|
||||
if (
|
||||
shouldTriggerNonStreamingFallbackForEmptyStream({
|
||||
hasMessageStart: partialMessage !== undefined,
|
||||
assistantMessageCount: newMessages.length,
|
||||
stopReason,
|
||||
})
|
||||
) {
|
||||
logForDebugging(
|
||||
!partialMessage
|
||||
? "Stream completed without receiving message_start event - triggering non-streaming fallback"
|
||||
: stopReason === "tool_use"
|
||||
? "Stream completed with tool_use stop but no completed tool block - triggering non-streaming fallback"
|
||||
: "Stream completed with message_start but no content blocks completed - triggering non-streaming fallback",
|
||||
{ level: "error" },
|
||||
);
|
||||
|
||||
54
src/services/api/streamFallback.test.ts
Normal file
54
src/services/api/streamFallback.test.ts
Normal file
@ -0,0 +1,54 @@
|
||||
import { describe, expect, test } from 'bun:test'
|
||||
import { shouldTriggerNonStreamingFallbackForEmptyStream } from './streamFallback.js'
|
||||
|
||||
describe('stream fallback policy', () => {
|
||||
test('falls back when a stream never produced message_start', () => {
|
||||
expect(
|
||||
shouldTriggerNonStreamingFallbackForEmptyStream({
|
||||
hasMessageStart: false,
|
||||
assistantMessageCount: 0,
|
||||
stopReason: null,
|
||||
}),
|
||||
).toBe(true)
|
||||
})
|
||||
|
||||
test('falls back when a stream starts but produces no terminal reason or content', () => {
|
||||
expect(
|
||||
shouldTriggerNonStreamingFallbackForEmptyStream({
|
||||
hasMessageStart: true,
|
||||
assistantMessageCount: 0,
|
||||
stopReason: null,
|
||||
}),
|
||||
).toBe(true)
|
||||
})
|
||||
|
||||
test('falls back when a stream reports tool_use without any tool block', () => {
|
||||
expect(
|
||||
shouldTriggerNonStreamingFallbackForEmptyStream({
|
||||
hasMessageStart: true,
|
||||
assistantMessageCount: 0,
|
||||
stopReason: 'tool_use',
|
||||
}),
|
||||
).toBe(true)
|
||||
})
|
||||
|
||||
test('keeps legitimate empty end_turn responses', () => {
|
||||
expect(
|
||||
shouldTriggerNonStreamingFallbackForEmptyStream({
|
||||
hasMessageStart: true,
|
||||
assistantMessageCount: 0,
|
||||
stopReason: 'end_turn',
|
||||
}),
|
||||
).toBe(false)
|
||||
})
|
||||
|
||||
test('does not fall back after yielding an assistant message', () => {
|
||||
expect(
|
||||
shouldTriggerNonStreamingFallbackForEmptyStream({
|
||||
hasMessageStart: true,
|
||||
assistantMessageCount: 1,
|
||||
stopReason: 'tool_use',
|
||||
}),
|
||||
).toBe(false)
|
||||
})
|
||||
})
|
||||
13
src/services/api/streamFallback.ts
Normal file
13
src/services/api/streamFallback.ts
Normal file
@ -0,0 +1,13 @@
|
||||
export function shouldTriggerNonStreamingFallbackForEmptyStream({
|
||||
hasMessageStart,
|
||||
assistantMessageCount,
|
||||
stopReason,
|
||||
}: {
|
||||
hasMessageStart: boolean
|
||||
assistantMessageCount: number
|
||||
stopReason: string | null
|
||||
}): boolean {
|
||||
if (!hasMessageStart) return true
|
||||
if (assistantMessageCount > 0) return false
|
||||
return stopReason === null || stopReason === 'tool_use'
|
||||
}
|
||||
File diff suppressed because one or more lines are too long
@ -45,7 +45,7 @@ import { BackgroundHint } from '../BashTool/UI.js';
|
||||
import { FILE_READ_TOOL_NAME } from '../FileReadTool/prompt.js';
|
||||
import { spawnTeammate } from '../shared/spawnMultiAgent.js';
|
||||
import { setAgentColor } from './agentColorManager.js';
|
||||
import { agentToolResultSchema, classifyHandoffIfNeeded, emitTaskProgress, extractPartialResult, finalizeAgentTool, getLastToolUseName, runAsyncAgentLifecycle } from './agentToolUtils.js';
|
||||
import { agentToolResultSchema, classifyHandoffIfNeeded, emitAgentToolActivitiesForMessage, emitTaskProgress, extractPartialResult, finalizeAgentTool, getLastToolUseName, runAsyncAgentLifecycle } from './agentToolUtils.js';
|
||||
import { GENERAL_PURPOSE_AGENT } from './built-in/generalPurposeAgent.js';
|
||||
import { AGENT_TOOL_NAME, LEGACY_AGENT_TOOL_NAME, ONE_SHOT_BUILTIN_AGENT_TYPES } from './constants.js';
|
||||
import { buildForkedMessages, buildWorktreeNotice, FORK_AGENT, isForkSubagentEnabled, isInForkChild } from './forkSubagent.js';
|
||||
@ -943,6 +943,7 @@ export const AgentTool = buildTool({
|
||||
// Track progress for backgrounded agents
|
||||
updateProgressFromMessage(tracker, msg, resolveActivity2, toolUseContext.options.tools);
|
||||
updateAsyncAgentProgress(backgroundedTaskId, getProgressUpdate(tracker), rootSetAppState);
|
||||
emitAgentToolActivitiesForMessage(msg, backgroundedTaskId, toolUseContext.toolUseId);
|
||||
const lastToolName = getLastToolUseName(msg);
|
||||
if (lastToolName) {
|
||||
emitTaskProgress(tracker, backgroundedTaskId, toolUseContext.toolUseId, description, startTime, lastToolName);
|
||||
|
||||
@ -12,7 +12,11 @@ import {
|
||||
resetCommandQueue,
|
||||
} from '../../utils/messageQueueManager.js'
|
||||
import { createAssistantMessage, createUserMessage } from '../../utils/messages.js'
|
||||
import { extractAgentToolActivities, runAsyncAgentLifecycle } from './agentToolUtils.js'
|
||||
import {
|
||||
emitAgentToolActivitiesForMessage,
|
||||
extractAgentToolActivities,
|
||||
runAsyncAgentLifecycle,
|
||||
} from './agentToolUtils.js'
|
||||
import { SYNTHETIC_OUTPUT_TOOL_NAME } from '../SyntheticOutputTool/SyntheticOutputTool.js'
|
||||
|
||||
describe('runAsyncAgentLifecycle', () => {
|
||||
@ -92,6 +96,9 @@ describe('runAsyncAgentLifecycle', () => {
|
||||
expect(String(getCommandQueue()[0]?.value)).toContain(
|
||||
'<status>completed</status>',
|
||||
)
|
||||
expect(String(getCommandQueue()[0]?.value)).toContain(
|
||||
'<task-type>local_agent</task-type>',
|
||||
)
|
||||
expect(String(getCommandQueue()[0]?.value)).toContain('Review complete.')
|
||||
})
|
||||
|
||||
@ -242,3 +249,49 @@ describe('extractAgentToolActivities', () => {
|
||||
expect(extractAgentToolActivities(message)).toEqual([])
|
||||
})
|
||||
})
|
||||
|
||||
describe('emitAgentToolActivitiesForMessage', () => {
|
||||
test('emits child tool activity for backgrounded sync agents', () => {
|
||||
const emitSpy = spyOn(sdkEventQueue, 'emitAgentToolActivity').mockImplementation(
|
||||
() => {},
|
||||
)
|
||||
try {
|
||||
const message = createAssistantMessage({
|
||||
content: [
|
||||
{ type: 'tool_use', id: 'toolu_child', name: 'Bash', input: { command: 'pwd' } },
|
||||
],
|
||||
}) as Message
|
||||
|
||||
emitAgentToolActivitiesForMessage(message, 'agent-foregrounded', 'toolu_parent')
|
||||
|
||||
expect(emitSpy.mock.calls).toEqual([
|
||||
[
|
||||
'agent-foregrounded',
|
||||
'toolu_parent',
|
||||
{ kind: 'tool_use', tool_name: 'Bash', tool_use_id: 'toolu_child', input: { command: 'pwd' } },
|
||||
],
|
||||
])
|
||||
} finally {
|
||||
emitSpy.mockRestore()
|
||||
}
|
||||
})
|
||||
|
||||
test('does nothing without a parent tool use id', () => {
|
||||
const emitSpy = spyOn(sdkEventQueue, 'emitAgentToolActivity').mockImplementation(
|
||||
() => {},
|
||||
)
|
||||
try {
|
||||
const message = createAssistantMessage({
|
||||
content: [
|
||||
{ type: 'tool_use', id: 'toolu_child', name: 'Bash', input: { command: 'pwd' } },
|
||||
],
|
||||
}) as Message
|
||||
|
||||
emitAgentToolActivitiesForMessage(message, 'agent-foregrounded', undefined)
|
||||
|
||||
expect(emitSpy).not.toHaveBeenCalled()
|
||||
} finally {
|
||||
emitSpy.mockRestore()
|
||||
}
|
||||
})
|
||||
})
|
||||
|
||||
@ -410,6 +410,17 @@ export function extractAgentToolActivities(
|
||||
return []
|
||||
}
|
||||
|
||||
export function emitAgentToolActivitiesForMessage(
|
||||
message: MessageType,
|
||||
taskId: string,
|
||||
parentToolUseId: string | undefined,
|
||||
): void {
|
||||
if (!parentToolUseId) return
|
||||
for (const activity of extractAgentToolActivities(message)) {
|
||||
emitAgentToolActivity(taskId, parentToolUseId, activity)
|
||||
}
|
||||
}
|
||||
|
||||
export function emitTaskProgress(
|
||||
tracker: ProgressTracker,
|
||||
taskId: string,
|
||||
@ -630,12 +641,11 @@ export async function runAsyncAgentLifecycle({
|
||||
// agents are detached (void runAsyncAgentLifecycle), so we route through
|
||||
// the SDK event queue → stdout → ws handler, tagged with the parent
|
||||
// Agent tool_use_id so the UI groups it under the right card.
|
||||
const parentToolUseId = toolUseContext.toolUseId
|
||||
if (parentToolUseId) {
|
||||
for (const activity of extractAgentToolActivities(message)) {
|
||||
emitAgentToolActivity(taskId, parentToolUseId, activity)
|
||||
}
|
||||
}
|
||||
emitAgentToolActivitiesForMessage(
|
||||
message,
|
||||
taskId,
|
||||
toolUseContext.toolUseId,
|
||||
)
|
||||
const lastToolName = getLastToolUseName(message)
|
||||
if (lastToolName) {
|
||||
emitTaskProgress(
|
||||
|
||||
42
src/utils/taskNotificationPolicy.test.ts
Normal file
42
src/utils/taskNotificationPolicy.test.ts
Normal file
@ -0,0 +1,42 @@
|
||||
import { describe, expect, test } from 'bun:test'
|
||||
import {
|
||||
parseTaskNotificationXml,
|
||||
shouldForwardTaskNotificationToModel,
|
||||
} from './taskNotificationPolicy.js'
|
||||
|
||||
describe('task notification policy', () => {
|
||||
test('does not re-enter the model for local agent terminal notifications in structured output', () => {
|
||||
const notification = parseTaskNotificationXml(`<task-notification>
|
||||
<task-id>agent-1</task-id>
|
||||
<task-type>local_agent</task-type>
|
||||
<output-file>/tmp/agent-1.out</output-file>
|
||||
<status>completed</status>
|
||||
<summary>Agent "Probe" completed</summary>
|
||||
</task-notification>`)
|
||||
|
||||
expect(shouldForwardTaskNotificationToModel(notification, { structuredOutput: true })).toBe(false)
|
||||
})
|
||||
|
||||
test('keeps local agent notifications as model input for plain print mode', () => {
|
||||
const notification = parseTaskNotificationXml(`<task-notification>
|
||||
<task-id>agent-1</task-id>
|
||||
<task-type>local_agent</task-type>
|
||||
<output-file>/tmp/agent-1.out</output-file>
|
||||
<status>completed</status>
|
||||
<summary>Agent "Probe" completed</summary>
|
||||
</task-notification>`)
|
||||
|
||||
expect(shouldForwardTaskNotificationToModel(notification, { structuredOutput: false })).toBe(true)
|
||||
})
|
||||
|
||||
test('continues forwarding background shell notifications to the model', () => {
|
||||
const notification = parseTaskNotificationXml(`<task-notification>
|
||||
<task-id>bash-1</task-id>
|
||||
<output-file>/tmp/bash-1.out</output-file>
|
||||
<status>completed</status>
|
||||
<summary>Background command "bun test" completed</summary>
|
||||
</task-notification>`)
|
||||
|
||||
expect(shouldForwardTaskNotificationToModel(notification, { structuredOutput: true })).toBe(true)
|
||||
})
|
||||
})
|
||||
71
src/utils/taskNotificationPolicy.ts
Normal file
71
src/utils/taskNotificationPolicy.ts
Normal file
@ -0,0 +1,71 @@
|
||||
import {
|
||||
OUTPUT_FILE_TAG,
|
||||
STATUS_TAG,
|
||||
SUMMARY_TAG,
|
||||
TASK_ID_TAG,
|
||||
TASK_TYPE_TAG,
|
||||
TOOL_USE_ID_TAG,
|
||||
} from '../constants/xml.js'
|
||||
|
||||
type TaskNotificationStatus = 'completed' | 'failed' | 'stopped'
|
||||
|
||||
export type ParsedTaskNotification = {
|
||||
taskId: string
|
||||
toolUseId?: string
|
||||
taskType?: string
|
||||
outputFile: string
|
||||
status?: TaskNotificationStatus
|
||||
summary: string
|
||||
result?: string
|
||||
usage?: {
|
||||
total_tokens: number
|
||||
tool_uses: number
|
||||
duration_ms: number
|
||||
}
|
||||
}
|
||||
|
||||
function getTagValue(text: string, tag: string): string | undefined {
|
||||
return text.match(new RegExp(`<${tag}>([\\s\\S]*?)</${tag}>`))?.[1]
|
||||
}
|
||||
|
||||
function normalizeStatus(rawStatus: string | undefined): TaskNotificationStatus | undefined {
|
||||
if (rawStatus === 'killed') return 'stopped'
|
||||
if (rawStatus === 'completed' || rawStatus === 'failed' || rawStatus === 'stopped') {
|
||||
return rawStatus
|
||||
}
|
||||
return undefined
|
||||
}
|
||||
|
||||
export function parseTaskNotificationXml(text: string): ParsedTaskNotification {
|
||||
const usageContent = getTagValue(text, 'usage') ?? ''
|
||||
const totalTokens = getTagValue(usageContent, 'total_tokens')
|
||||
const toolUses = getTagValue(usageContent, 'tool_uses')
|
||||
const durationMs = getTagValue(usageContent, 'duration_ms')
|
||||
|
||||
return {
|
||||
taskId: getTagValue(text, TASK_ID_TAG) ?? '',
|
||||
toolUseId: getTagValue(text, TOOL_USE_ID_TAG),
|
||||
taskType: getTagValue(text, TASK_TYPE_TAG),
|
||||
outputFile: getTagValue(text, OUTPUT_FILE_TAG) ?? '',
|
||||
status: normalizeStatus(getTagValue(text, STATUS_TAG)),
|
||||
summary: getTagValue(text, SUMMARY_TAG) ?? '',
|
||||
result: getTagValue(text, 'result'),
|
||||
usage:
|
||||
totalTokens && toolUses
|
||||
? {
|
||||
total_tokens: parseInt(totalTokens, 10),
|
||||
tool_uses: parseInt(toolUses, 10),
|
||||
duration_ms: durationMs ? parseInt(durationMs, 10) : 0,
|
||||
}
|
||||
: undefined,
|
||||
}
|
||||
}
|
||||
|
||||
export function shouldForwardTaskNotificationToModel(
|
||||
notification: ParsedTaskNotification,
|
||||
options: { structuredOutput: boolean },
|
||||
): boolean {
|
||||
if (!options.structuredOutput) return true
|
||||
if (!notification.status) return true
|
||||
return notification.taskType !== 'local_agent'
|
||||
}
|
||||
Loading…
x
Reference in New Issue
Block a user