mirror of
https://github.com/NanmiCoder/cc-haha
synced 2026-07-18 13:23:33 +08:00
Keep goal sessions alive until background work reports back
The goal evaluator now treats TaskCreate/TaskUpdate transcript state as a hard liveness gate, so a goal cannot complete while task entries remain pending or in_progress. Background agent completion now notifies the parent before classifier or worktree cleanup, which keeps the main session from waiting forever when post-completion cleanup hangs. The desktop store also marks tabs idle on message_complete so completed transcripts do not leave stale running chrome. Constraint: Desktop /goal relies on transcript task notifications to resume after background agents. Rejected: Let optional cleanup run before notification | cleanup can hang and leaves the parent loop stuck. Confidence: high Scope-risk: moderate Directive: Do not gate task-notification delivery on classifier or worktree cleanup without a timeout-backed liveness test. Tested: bun test src/goals/goalEvaluator.test.ts src/tools/AgentTool/agentToolUtils.test.ts Tested: bun run check:server Tested: cd desktop && bun run test src/stores/chatStore.test.ts -t "marks the tab idle when a message completes"
This commit is contained in:
parent
3ebab4366d
commit
e9ac5739bc
@ -445,6 +445,12 @@ describe('chatStore history mapping', () => {
|
||||
summary: 'Agent completed',
|
||||
},
|
||||
},
|
||||
{
|
||||
id: 'goal-complete',
|
||||
type: 'goal_event',
|
||||
action: 'completed',
|
||||
message: 'Goal marked complete.',
|
||||
},
|
||||
])
|
||||
expect(session?.activeGoal).toMatchObject({
|
||||
action: 'completed',
|
||||
@ -1945,6 +1951,22 @@ describe('chatStore history mapping', () => {
|
||||
vi.useRealTimers()
|
||||
})
|
||||
|
||||
it('marks the tab idle when a message completes', () => {
|
||||
useChatStore.setState({
|
||||
sessions: {
|
||||
[TEST_SESSION_ID]: makeSession({ chatState: 'thinking' }),
|
||||
},
|
||||
})
|
||||
|
||||
useChatStore.getState().handleServerMessage(TEST_SESSION_ID, {
|
||||
type: 'message_complete',
|
||||
usage: { input_tokens: 1, output_tokens: 2 },
|
||||
})
|
||||
|
||||
expect(useChatStore.getState().sessions[TEST_SESSION_ID]?.chatState).toBe('idle')
|
||||
expect(updateTabStatusMock).toHaveBeenCalledWith(TEST_SESSION_ID, 'idle')
|
||||
})
|
||||
|
||||
it('flushes pending text before appending a thinking block', () => {
|
||||
vi.useFakeTimers()
|
||||
|
||||
|
||||
@ -254,6 +254,27 @@ function mergeBackgroundTaskMessages(
|
||||
return [...merged].sort((a, b) => a.timestamp - b.timestamp)
|
||||
}
|
||||
|
||||
function mergeRestoredTerminalGoalEvents(
|
||||
messages: UIMessage[],
|
||||
restoredMessages: UIMessage[],
|
||||
): UIMessage[] {
|
||||
const existingKeys = new Set(messages
|
||||
.filter((message): message is Extract<UIMessage, { type: 'goal_event' }> =>
|
||||
message.type === 'goal_event')
|
||||
.map((message) => `${message.action}:${message.message ?? ''}:${message.objective ?? ''}`))
|
||||
|
||||
const missingTerminalEvents = restoredMessages.filter((
|
||||
message,
|
||||
): message is Extract<UIMessage, { type: 'goal_event' }> =>
|
||||
message.type === 'goal_event' &&
|
||||
(message.action === 'completed' || message.action === 'cleared') &&
|
||||
!existingKeys.has(`${message.action}:${message.message ?? ''}:${message.objective ?? ''}`))
|
||||
|
||||
return missingTerminalEvents.length > 0
|
||||
? [...messages, ...missingTerminalEvents]
|
||||
: messages
|
||||
}
|
||||
|
||||
function normalizeMemoryEventFiles(data: unknown): MemoryEventFile[] {
|
||||
if (!data || typeof data !== 'object') return []
|
||||
const writtenPaths = (data as { writtenPaths?: unknown }).writtenPaths
|
||||
@ -592,7 +613,10 @@ export const useChatStore = create<ChatStore>((set, get) => ({
|
||||
s.backgroundAgentTasks ?? {},
|
||||
restoredBackgroundTasks,
|
||||
),
|
||||
messages: mergeBackgroundTaskMessages(s.messages, restoredBackgroundTasks),
|
||||
messages: mergeRestoredTerminalGoalEvents(
|
||||
mergeBackgroundTaskMessages(s.messages, restoredBackgroundTasks),
|
||||
uiMessages,
|
||||
),
|
||||
})) }
|
||||
}
|
||||
return { sessions: updateSessionIn(state.sessions, sessionId, (s) => ({
|
||||
@ -908,6 +932,7 @@ export const useChatStore = create<ChatStore>((set, get) => ({
|
||||
pendingComputerUsePermission: null,
|
||||
elapsedTimer: null,
|
||||
}))
|
||||
useTabStore.getState().updateTabStatus(sessionId, 'idle')
|
||||
const notification = wasAgentRunning
|
||||
? buildAgentCompletionNotification(sessionId, completionMessages, text)
|
||||
: null
|
||||
|
||||
@ -134,6 +134,61 @@ describe('goalEvaluator', () => {
|
||||
)
|
||||
})
|
||||
|
||||
test('continues an active goal before evaluating when tasks are still incomplete', async () => {
|
||||
setThreadGoal('thread-eval-open-task', {
|
||||
objective: 'finish all task-list work',
|
||||
now: 1_000,
|
||||
})
|
||||
let evaluatorCalled = false
|
||||
|
||||
const decision = await evaluateThreadGoalAfterTurn({
|
||||
threadId: 'thread-eval-open-task',
|
||||
messages: [
|
||||
createUserMessage({
|
||||
content: [
|
||||
{
|
||||
type: 'tool_result',
|
||||
tool_use_id: 'task-create-3',
|
||||
content: 'Task #3 created successfully: Perform code review',
|
||||
} as unknown as BetaContentBlock,
|
||||
],
|
||||
}),
|
||||
createAssistantMessage({
|
||||
content: [
|
||||
{
|
||||
type: 'tool_use',
|
||||
id: 'task-update-3',
|
||||
name: 'TaskUpdate',
|
||||
input: { taskId: '3', status: 'in_progress' },
|
||||
} as unknown as BetaContentBlock,
|
||||
],
|
||||
}),
|
||||
],
|
||||
assistantMessages: [
|
||||
createAssistantMessage({
|
||||
content: [{ type: 'text', text: 'The implementation is complete.' }],
|
||||
}),
|
||||
],
|
||||
signal: new AbortController().signal,
|
||||
now: 3_000,
|
||||
evaluate: async () => {
|
||||
evaluatorCalled = true
|
||||
return {
|
||||
complete: true,
|
||||
reason: 'The final answer claims the work is done.',
|
||||
}
|
||||
},
|
||||
})
|
||||
|
||||
expect(decision.action).toBe('continue')
|
||||
expect(evaluatorCalled).toBe(false)
|
||||
if (decision.action === 'continue') {
|
||||
expect(decision.reason).toContain('Task #3 (Perform code review) is in_progress')
|
||||
expect(decision.prompt).toContain('The task list is not complete yet')
|
||||
}
|
||||
expect(getThreadGoal('thread-eval-open-task')?.status).toBe('active')
|
||||
})
|
||||
|
||||
test('hydrates an active goal from persisted slash command history before continuing', async () => {
|
||||
const threadId = 'thread-eval-hydrate'
|
||||
|
||||
|
||||
@ -78,6 +78,25 @@ export async function evaluateThreadGoalAfterTurn(input: {
|
||||
return { action: 'budget_limited', goal: limited }
|
||||
}
|
||||
|
||||
const taskState = summarizeTaskState([
|
||||
...input.messages,
|
||||
...input.assistantMessages,
|
||||
])
|
||||
if (taskState.incomplete.length > 0) {
|
||||
const reason = formatIncompleteTaskReason(taskState.incomplete)
|
||||
const continued =
|
||||
incrementThreadGoalContinuation(input.threadId, {
|
||||
reason,
|
||||
now,
|
||||
}) ?? accounted
|
||||
return {
|
||||
action: 'continue',
|
||||
goal: continued,
|
||||
reason,
|
||||
prompt: buildGoalContinuationPrompt(continued, reason),
|
||||
}
|
||||
}
|
||||
|
||||
const transcript = formatTranscript([
|
||||
...input.messages,
|
||||
...input.assistantMessages,
|
||||
@ -259,6 +278,88 @@ function assistantVisibleText(content: readonly BetaContentBlock[]): string {
|
||||
.join('\n')
|
||||
}
|
||||
|
||||
type TaskSummary = {
|
||||
id: string
|
||||
subject: string | null
|
||||
status: string
|
||||
}
|
||||
|
||||
function summarizeTaskState(messages: Message[]): { incomplete: TaskSummary[] } {
|
||||
const tasks = new Map<string, TaskSummary>()
|
||||
|
||||
for (const message of messages) {
|
||||
if (message.type === 'assistant') {
|
||||
for (const block of message.message.content) {
|
||||
if (block.type !== 'tool_use') continue
|
||||
if (block.name !== 'TaskUpdate') continue
|
||||
const input = block.input
|
||||
if (!input || typeof input !== 'object') continue
|
||||
const taskId = (input as { taskId?: unknown }).taskId
|
||||
const status = (input as { status?: unknown }).status
|
||||
if (typeof taskId !== 'string' || typeof status !== 'string') continue
|
||||
const existing = tasks.get(taskId)
|
||||
tasks.set(taskId, {
|
||||
id: taskId,
|
||||
subject: existing?.subject ?? null,
|
||||
status,
|
||||
})
|
||||
}
|
||||
continue
|
||||
}
|
||||
|
||||
if (message.type !== 'user') continue
|
||||
const content = message.message.content
|
||||
if (!Array.isArray(content)) continue
|
||||
for (const block of content) {
|
||||
if (block.type !== 'tool_result') continue
|
||||
const text = toolResultText(block.content)
|
||||
const created = text.match(/Task #(\S+) created successfully:\s*(.+)/)
|
||||
if (!created) continue
|
||||
const [, id, subject] = created
|
||||
const existing = tasks.get(id)
|
||||
tasks.set(id, {
|
||||
id,
|
||||
subject: subject.trim(),
|
||||
status: existing?.status ?? 'pending',
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
return {
|
||||
incomplete: [...tasks.values()].filter(task =>
|
||||
task.status === 'pending' || task.status === 'in_progress',
|
||||
),
|
||||
}
|
||||
}
|
||||
|
||||
function toolResultText(content: unknown): string {
|
||||
if (typeof content === 'string') return content
|
||||
if (!Array.isArray(content)) return ''
|
||||
return content
|
||||
.map(item =>
|
||||
item &&
|
||||
typeof item === 'object' &&
|
||||
'text' in item &&
|
||||
typeof item.text === 'string'
|
||||
? item.text
|
||||
: '',
|
||||
)
|
||||
.filter(Boolean)
|
||||
.join('\n')
|
||||
}
|
||||
|
||||
function formatIncompleteTaskReason(tasks: TaskSummary[]): string {
|
||||
const taskList = tasks
|
||||
.slice(0, 3)
|
||||
.map(task => {
|
||||
const label = task.subject ? `Task #${task.id} (${task.subject})` : `Task #${task.id}`
|
||||
return `${label} is ${task.status}`
|
||||
})
|
||||
.join('; ')
|
||||
const suffix = tasks.length > 3 ? `; ${tasks.length - 3} more task(s) are incomplete` : ''
|
||||
return `The task list is not complete yet: ${taskList}${suffix}.`
|
||||
}
|
||||
|
||||
function getMaxContinuations(): number {
|
||||
const raw = process.env.CLAUDE_CODE_GOAL_MAX_CONTINUES
|
||||
if (!raw) return DEFAULT_MAX_CONTINUATIONS
|
||||
|
||||
File diff suppressed because one or more lines are too long
95
src/tools/AgentTool/agentToolUtils.test.ts
Normal file
95
src/tools/AgentTool/agentToolUtils.test.ts
Normal file
@ -0,0 +1,95 @@
|
||||
import { afterEach, describe, expect, test } from 'bun:test'
|
||||
import type { AppState } from '../../state/AppState.js'
|
||||
import { IDLE_SPECULATION_STATE } from '../../state/AppStateStore.js'
|
||||
import { createTaskStateBase } from '../../Task.js'
|
||||
import type { ToolUseContext } from '../../Tool.js'
|
||||
import type { LocalAgentTaskState } from '../../tasks/LocalAgentTask/LocalAgentTask.js'
|
||||
import type { Message } from '../../types/message.js'
|
||||
import { getEmptyToolPermissionContext } from '../../Tool.js'
|
||||
import {
|
||||
getCommandQueue,
|
||||
resetCommandQueue,
|
||||
} from '../../utils/messageQueueManager.js'
|
||||
import { createAssistantMessage } from '../../utils/messages.js'
|
||||
import { runAsyncAgentLifecycle } from './agentToolUtils.js'
|
||||
|
||||
describe('runAsyncAgentLifecycle', () => {
|
||||
afterEach(() => {
|
||||
resetCommandQueue()
|
||||
})
|
||||
|
||||
test('notifies the parent before post-completion cleanup finishes', async () => {
|
||||
const taskId = 'agent-notify-first'
|
||||
const abortController = new AbortController()
|
||||
const task: LocalAgentTaskState = {
|
||||
...createTaskStateBase(taskId, 'local_agent', 'Review code', 'toolu_agent'),
|
||||
status: 'running',
|
||||
agentId: taskId,
|
||||
prompt: 'Review code',
|
||||
agentType: 'general-purpose',
|
||||
abortController,
|
||||
retrieved: false,
|
||||
lastReportedToolCount: 0,
|
||||
lastReportedTokenCount: 0,
|
||||
isBackgrounded: true,
|
||||
pendingMessages: [],
|
||||
retain: false,
|
||||
diskLoaded: false,
|
||||
}
|
||||
let appState = {
|
||||
tasks: { [taskId]: task },
|
||||
toolPermissionContext: getEmptyToolPermissionContext(),
|
||||
speculation: IDLE_SPECULATION_STATE,
|
||||
} as unknown as AppState
|
||||
const setAppState = (updater: (prev: AppState) => AppState): void => {
|
||||
appState = updater(appState)
|
||||
}
|
||||
const message = createAssistantMessage({
|
||||
content: [{ type: 'text', text: 'Review complete.' }],
|
||||
}) as Message
|
||||
let cleanupStarted = false
|
||||
|
||||
async function* makeStream(): AsyncGenerator<Message, void> {
|
||||
yield message
|
||||
}
|
||||
|
||||
const result = await Promise.race([
|
||||
runAsyncAgentLifecycle({
|
||||
taskId,
|
||||
abortController,
|
||||
makeStream,
|
||||
metadata: {
|
||||
prompt: 'Review code',
|
||||
resolvedAgentModel: 'test-model',
|
||||
isBuiltInAgent: true,
|
||||
startTime: Date.now(),
|
||||
agentType: 'general-purpose',
|
||||
isAsync: true,
|
||||
},
|
||||
description: 'Review code',
|
||||
toolUseContext: {
|
||||
options: { tools: [] },
|
||||
toolUseId: 'toolu_agent',
|
||||
getAppState: () => appState,
|
||||
} as unknown as ToolUseContext,
|
||||
rootSetAppState: setAppState,
|
||||
agentIdForCleanup: taskId,
|
||||
enableSummarization: false,
|
||||
getWorktreeResult: () => {
|
||||
cleanupStarted = true
|
||||
return new Promise(() => {})
|
||||
},
|
||||
}).then(() => 'completed'),
|
||||
new Promise(resolve => setTimeout(() => resolve('timed-out'), 50)),
|
||||
])
|
||||
|
||||
expect(result).toBe('completed')
|
||||
expect(cleanupStarted).toBe(true)
|
||||
expect(appState.tasks[taskId]?.status).toBe('completed')
|
||||
expect(getCommandQueue()).toHaveLength(1)
|
||||
expect(String(getCommandQueue()[0]?.value)).toContain(
|
||||
'<status>completed</status>',
|
||||
)
|
||||
expect(String(getCommandQueue()[0]?.value)).toContain('Review complete.')
|
||||
})
|
||||
})
|
||||
@ -597,44 +597,44 @@ export async function runAsyncAgentLifecycle({
|
||||
const agentResult = finalizeAgentTool(agentMessages, taskId, metadata)
|
||||
|
||||
// Mark task completed FIRST so TaskOutput(block=true) unblocks
|
||||
// immediately. classifyHandoffIfNeeded (API call) and getWorktreeResult
|
||||
// (git exec) are notification embellishments that can hang — they must
|
||||
// not gate the status transition (gh-20236).
|
||||
// immediately, then notify the parent before any optional cleanup. The
|
||||
// parent session depends on this notification to resume its loop.
|
||||
completeAsyncAgent(agentResult, rootSetAppState)
|
||||
|
||||
let finalMessage = extractTextContent(agentResult.content, '\n')
|
||||
|
||||
if (feature('TRANSCRIPT_CLASSIFIER')) {
|
||||
const handoffWarning = await classifyHandoffIfNeeded({
|
||||
agentMessages,
|
||||
tools: toolUseContext.options.tools,
|
||||
toolPermissionContext:
|
||||
toolUseContext.getAppState().toolPermissionContext,
|
||||
abortSignal: abortController.signal,
|
||||
subagentType: metadata.agentType,
|
||||
totalToolUseCount: agentResult.totalToolUseCount,
|
||||
})
|
||||
if (handoffWarning) {
|
||||
finalMessage = `${handoffWarning}\n\n${finalMessage}`
|
||||
}
|
||||
}
|
||||
|
||||
const worktreeResult = await getWorktreeResult()
|
||||
|
||||
enqueueAgentNotification({
|
||||
taskId,
|
||||
description,
|
||||
status: 'completed',
|
||||
setAppState: rootSetAppState,
|
||||
finalMessage,
|
||||
finalMessage: extractTextContent(agentResult.content, '\n'),
|
||||
usage: {
|
||||
totalTokens: getTokenCountFromTracker(tracker),
|
||||
toolUses: agentResult.totalToolUseCount,
|
||||
durationMs: agentResult.totalDurationMs,
|
||||
},
|
||||
toolUseId: toolUseContext.toolUseId,
|
||||
...worktreeResult,
|
||||
})
|
||||
|
||||
void (async () => {
|
||||
try {
|
||||
await getWorktreeResult()
|
||||
if (feature('TRANSCRIPT_CLASSIFIER')) {
|
||||
await classifyHandoffIfNeeded({
|
||||
agentMessages,
|
||||
tools: toolUseContext.options.tools,
|
||||
toolPermissionContext:
|
||||
toolUseContext.getAppState().toolPermissionContext,
|
||||
abortSignal: abortController.signal,
|
||||
subagentType: metadata.agentType,
|
||||
totalToolUseCount: agentResult.totalToolUseCount,
|
||||
})
|
||||
}
|
||||
} catch (cleanupError) {
|
||||
logForDebugging(
|
||||
`Async agent post-completion cleanup failed: ${errorMessage(cleanupError)}`,
|
||||
)
|
||||
}
|
||||
})()
|
||||
} catch (error) {
|
||||
stopSummarization?.()
|
||||
if (error instanceof AbortError) {
|
||||
@ -654,7 +654,6 @@ export async function runAsyncAgentLifecycle({
|
||||
reason:
|
||||
'user_kill_async' as AnalyticsMetadata_I_VERIFIED_THIS_IS_NOT_CODE_OR_FILEPATHS,
|
||||
})
|
||||
const worktreeResult = await getWorktreeResult()
|
||||
const partialResult = extractPartialResult(agentMessages)
|
||||
enqueueAgentNotification({
|
||||
taskId,
|
||||
@ -663,13 +662,16 @@ export async function runAsyncAgentLifecycle({
|
||||
setAppState: rootSetAppState,
|
||||
toolUseId: toolUseContext.toolUseId,
|
||||
finalMessage: partialResult,
|
||||
...worktreeResult,
|
||||
})
|
||||
void getWorktreeResult().catch(cleanupError =>
|
||||
logForDebugging(
|
||||
`Async agent post-cancel cleanup failed: ${errorMessage(cleanupError)}`,
|
||||
),
|
||||
)
|
||||
return
|
||||
}
|
||||
const msg = errorMessage(error)
|
||||
failAsyncAgent(taskId, msg, rootSetAppState)
|
||||
const worktreeResult = await getWorktreeResult()
|
||||
enqueueAgentNotification({
|
||||
taskId,
|
||||
description,
|
||||
@ -677,8 +679,12 @@ export async function runAsyncAgentLifecycle({
|
||||
error: msg,
|
||||
setAppState: rootSetAppState,
|
||||
toolUseId: toolUseContext.toolUseId,
|
||||
...worktreeResult,
|
||||
})
|
||||
void getWorktreeResult().catch(cleanupError =>
|
||||
logForDebugging(
|
||||
`Async agent post-failure cleanup failed: ${errorMessage(cleanupError)}`,
|
||||
),
|
||||
)
|
||||
} finally {
|
||||
clearInvokedSkillsForAgent(agentIdForCleanup)
|
||||
clearDumpState(agentIdForCleanup)
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user