diff --git a/packages/cloud-agents/src/server/fast-agent/__tests__/fast-agent-conversation-repository.test.ts b/packages/cloud-agents/src/server/fast-agent/__tests__/fast-agent-conversation-repository.test.ts index 9dddfd352..6cf4a1de5 100644 --- a/packages/cloud-agents/src/server/fast-agent/__tests__/fast-agent-conversation-repository.test.ts +++ b/packages/cloud-agents/src/server/fast-agent/__tests__/fast-agent-conversation-repository.test.ts @@ -16,6 +16,7 @@ import { findFastAgentActiveInferenceRetryNotice, findFastAgentUnresolvedRequest, loadFastAgentTurnAttemptSummary, + loadFastAgentTaskMessageHistory, INTERRUPTED_INFERENCE_RETRY_MESSAGE, scheduleFastAgentDurableTurnRetry, markFastAgentDurableTurnDelivered, @@ -1595,6 +1596,215 @@ describe('Fast conversation repository', () => { }); }); + it('loads durable task-message receipts across turns in original call order, including silent pending work', async () => { + const user = await createUser(); + const session = await fastAgentConversationRepository.getOrCreate({ + userId: user.id, + conversation: slackConversation, + }); + const other = await fastAgentConversationRepository.getOrCreate({ + userId: user.id, + conversation: { + ...slackConversation, + conversationId: 'other-history', + replyTarget: { + ...slackConversation.replyTarget, + threadId: 'other-history', + }, + }, + }); + const write = async ( + turnId: string, + turnSeq: number, + metadata: Record, + options: { + result?: string; + failed?: boolean; + conversationId?: string; + toolName?: string; + eventType?: 'roomote_runtime.assistant_message'; + } = {}, + ) => { + await fastAgentConversationRepository.upsertMessage({ + conversationId: options.conversationId ?? session.id, + message: { + eventId: `${turnId}:tool:${turnSeq}`, + turnId, + turnSeq, + ts: options.result ? 99_000 : 1_000 - turnSeq, + eventType: + options.eventType ?? + (options.result + ? 'roomote_runtime.tool_result' + : 'roomote_runtime.tool_call'), + role: 'tool', + contentBlocks: options.result + ? [{ type: 'text', text: options.result }] + : [], + metadata: { visibleInTranscript: false, ...metadata }, + payload: { + toolCallId: `${turnId}:tool:${turnSeq}`, + toolName: options.toolName ?? 'send_task_message', + status: options.failed ? 'failed' : 'completed', + // Model input must never supply the trusted continuation stamp. + taskMessageContinuation: 'recovery', + rawInput: { + arguments: { + taskId: 'task-history', + message: turnId, + metadata: { taskMessageContinuation: 'recovery' }, + }, + }, + }, + source: 'slack', + }, + }); + }; + const recovery = { taskMessageContinuation: 'recovery' }; + const instruction = { + taskMessageContinuation: 'instruction', + taskMessageInstructionId: 'human-request-2', + }; + await write('recovery-turn', 8, recovery); + const original = await db.query.fastAgentMessages.findFirst({ + where: and( + eq(fastAgentMessages.conversationId, session.id), + eq(fastAgentMessages.eventId, 'recovery-turn:tool:8'), + ), + }); + await write('instruction-turn', 1, instruction); + await write('current-event-turn', 2, recovery); + // A late result replaces the earlier call without moving it after the instruction. + const result = JSON.stringify({ success: true, detail: 'x'.repeat(1_300) }); + await write('recovery-turn', 8, recovery, { result }); + await write('instruction-turn', 1, instruction, { + result: '{"success":true}', + }); + await write('legacy-turn', 3, {}, { result: '{"success":true}' }); + await write( + 'invalid-stamp', + 4, + { + taskMessageContinuation: 'automatic', + taskMessageInstructionId: 123, + }, + { result: '{"success":false}', failed: true }, + ); + await write('unrelated-tool', 5, {}, { toolName: 'launch_task' }); + await write( + 'unrelated-event', + 6, + {}, + { + eventType: 'roomote_runtime.assistant_message', + }, + ); + await write('other-conversation', 7, recovery, { + conversationId: other.id, + }); + + const updated = await db.query.fastAgentMessages.findFirst({ + where: eq(fastAgentMessages.id, original!.id), + }); + expect(updated!.createdAt).toEqual(original!.createdAt); + // Deterministic timestamps also exercise turnSeq as the equal-time tie breaker. + for (const [turnId, createdAt] of [ + ['recovery-turn', '2026-01-01T00:00:00Z'], + ['instruction-turn', '2026-01-02T00:00:00Z'], + ['current-event-turn', '2026-01-02T00:00:00Z'], + ['legacy-turn', '2026-01-03T00:00:00Z'], + ['invalid-stamp', '2026-01-04T00:00:00Z'], + ] as const) { + await db + .update(fastAgentMessages) + .set({ createdAt: new Date(createdAt) }) + .where( + and( + eq(fastAgentMessages.conversationId, session.id), + eq(fastAgentMessages.turnId, turnId), + ), + ); + } + const history = await loadFastAgentTaskMessageHistory(session.id); + expect(history).toEqual([ + { + kind: 'action', + tool: 'send_task_message', + arguments: { + taskId: 'task-history', + message: 'recovery-turn', + metadata: { taskMessageContinuation: 'recovery' }, + }, + status: 'completed', + continuation: 'recovery', + result, + }, + { + kind: 'action', + tool: 'send_task_message', + arguments: { + taskId: 'task-history', + message: 'instruction-turn', + metadata: { taskMessageContinuation: 'recovery' }, + }, + status: 'completed', + continuation: 'instruction', + instructionId: 'human-request-2', + result: '{"success":true}', + }, + { + kind: 'action', + tool: 'send_task_message', + arguments: { + taskId: 'task-history', + message: 'current-event-turn', + metadata: { taskMessageContinuation: 'recovery' }, + }, + status: 'unknown', + continuation: 'recovery', + }, + { + kind: 'action', + tool: 'send_task_message', + arguments: { + taskId: 'task-history', + message: 'legacy-turn', + metadata: { taskMessageContinuation: 'recovery' }, + }, + status: 'completed', + result: '{"success":true}', + }, + { + kind: 'action', + tool: 'send_task_message', + arguments: { + taskId: 'task-history', + message: 'invalid-stamp', + metadata: { taskMessageContinuation: 'recovery' }, + }, + status: 'failed', + result: '{"success":false}', + }, + ]); + for (const [index, turnId] of [ + 'recovery-turn', + 'instruction-turn', + 'current-event-turn', + 'legacy-turn', + 'invalid-stamp', + ].entries()) { + const summary = await loadFastAgentTurnAttemptSummary(session.id, turnId); + expect(summary.events).toEqual([ + index === 0 + ? { ...history[0], result: `${result.slice(0, 1_200)}…` } + : history[index], + ]); + } + await expect( + loadFastAgentTaskMessageHistory(crypto.randomUUID()), + ).resolves.toEqual([]); + }); + it('walks a durable turn row through claim, release, revoke, and delivery', async () => { const user = await createUser(); const session = await fastAgentConversationRepository.getOrCreate({ diff --git a/packages/cloud-agents/src/server/fast-agent/__tests__/fast-agent-prompt.test.ts b/packages/cloud-agents/src/server/fast-agent/__tests__/fast-agent-prompt.test.ts index 9d7659b2d..8d3d9d60c 100644 --- a/packages/cloud-agents/src/server/fast-agent/__tests__/fast-agent-prompt.test.ts +++ b/packages/cloud-agents/src/server/fast-agent/__tests__/fast-agent-prompt.test.ts @@ -1266,6 +1266,57 @@ describe('buildFastAgentSystemPrompt', () => { ); }); + it.each(['human', 'platform_event'] as const)( + 'bounds recovery of authorized work on %s turns', + (turnSource) => { + const prompt = buildFastAgentSystemPrompt({ + availableEnvironments: [], + turnSource, + }); + expect(prompt).toContain( + 'inspect the current task state and conversation history first', + ); + expect(prompt).toContain( + 'at most one automatic recovery attempt per task for the outstanding user request, across turns and run IDs', + ); + expect(prompt).toContain( + 'Duplicate events, later check-ins, and new run IDs do not reset this limit', + ); + expect(prompt).toContain( + 'Never auto-resume canceled or user-stopped work, needs-input or approval waits', + ); + expect(prompt).toContain( + 'A generic provider error is not proof it is transient', + ); + expect(prompt).toContain( + 'require a new explicit user instruction before another attempt', + ); + expect(prompt).toContain( + 'Do not launch a replacement task or schedule a retry loop', + ); + expect(prompt).toContain('Outside presentation-only events'); + expect(prompt).toContain('defaults to continuation="recovery"'); + expect(prompt).toContain( + 'never for a check-in, status question, or task event', + ); + expect(prompt).toContain( + 'only an accepted new human instruction resets that task', + ); + expect(prompt).toContain( + 'Do not change continuation mode or reword the message to bypass the refusal', + ); + expect(prompt).toContain( + 'Say work is continuing only when current execution or an accepted continuation supports it', + ); + if (turnSource === 'platform_event') { + expect(prompt).toContain( + 'A stored error that interrupted unfinished work is a changed outcome', + ); + expect(prompt).not.toContain('Report or ignore it without retrying'); + } + }, + ); + it('requires presentation-only platform events to stop after posting', () => { const prompt = buildFastAgentSystemPrompt({ availableEnvironments: [], diff --git a/packages/cloud-agents/src/server/fast-agent/__tests__/fast-agent-service.test.ts b/packages/cloud-agents/src/server/fast-agent/__tests__/fast-agent-service.test.ts index 8abdd049f..40a629355 100644 --- a/packages/cloud-agents/src/server/fast-agent/__tests__/fast-agent-service.test.ts +++ b/packages/cloud-agents/src/server/fast-agent/__tests__/fast-agent-service.test.ts @@ -45,6 +45,7 @@ const mocks = vi.hoisted(() => ({ scheduleDurableRetry: vi.fn(), findActiveRetryNotice: vi.fn(), loadTurnAttempt: vi.fn(), + loadTaskMessageHistory: vi.fn(), getUnifiedSession: vi.fn(), touchSessionActivity: vi.fn(), getSessionForTask: vi.fn(), @@ -123,6 +124,7 @@ vi.mock('../fast-agent-conversation-repository', () => ({ scheduleFastAgentDurableTurnRetry: mocks.scheduleDurableRetry, findFastAgentActiveInferenceRetryNotice: mocks.findActiveRetryNotice, loadFastAgentTurnAttemptSummary: mocks.loadTurnAttempt, + loadFastAgentTaskMessageHistory: mocks.loadTaskMessageHistory, })); vi.mock('../../available-environments', () => ({ @@ -466,6 +468,7 @@ describe('answerFastAgentQuestion native OpenCode tools', () => { mocks.setOpenCodeSession.mockResolvedValue(undefined); mocks.upsertMessage.mockResolvedValue({ initialHumanTurn: true }); mocks.reconcileRetryNotices.mockResolvedValue(0); + mocks.loadTaskMessageHistory.mockResolvedValue([]); mocks.markRetryNoticeInterruption.mockResolvedValue(undefined); mocks.renewRespondingLease.mockResolvedValue(true); mocks.findUnresolvedRequest.mockResolvedValue(null); @@ -8594,7 +8597,11 @@ describe('answerFastAgentQuestion native OpenCode tools', () => { message: 'Run tests', }), ).toMatchObject({ success: false }); - const args = { taskId: 'task-1', message: 'Run tests' }; + const args = { + taskId: 'task-1', + message: 'Run tests', + continuation: 'instruction', + }; const first = await invokeTool(nativeToolNames.sendTaskMessage, args); expect(await invokeTool(nativeToolNames.sendTaskMessage, args)).toEqual( first, @@ -8677,6 +8684,150 @@ describe('answerFastAgentQuestion native OpenCode tools', () => { }, ); + it('enforces recovery receipts across platform turns, check-ins, and new run IDs', async () => { + const persisted = new Map< + string, + { + eventType: string; + payload: { + toolName?: string; + rawInput?: { arguments?: unknown }; + output?: string; + }; + metadata: Record; + } + >(); + mocks.upsertMessage.mockImplementation(async ({ message }) => { + persisted.set(message.eventId, message); + return { initialHumanTurn: false }; + }); + mocks.loadTaskMessageHistory.mockImplementation(async () => + [...persisted.values()] + .filter((message) => message.payload.toolName === 'send_task_message') + .map((message) => ({ + kind: 'action', + tool: 'send_task_message', + arguments: message.payload.rawInput?.arguments, + continuation: message.metadata.taskMessageContinuation, + instructionId: message.metadata.taskMessageInstructionId, + status: + message.eventType === ACP_ENVELOPE_EVENT_TYPES.ToolCall + ? 'unknown' + : 'completed', + result: message.payload.output, + })), + ); + mocks.getActiveTasks.mockResolvedValue([ + { taskId: 'task-1', title: 'Checkout', status: 'idle' }, + ]); + const outcomes: unknown[] = []; + let human = false; + let instruction = false; + mocks.generateText.mockImplementation( + async (_params, _session, options) => { + await options.onSessionReady('opencode-session-1'); + if (human) { + await invokeTool(nativeToolNames.sendChatReply, { + purpose: 'ack', + message: 'Checking the existing work.', + }); + } + outcomes.push( + await invokeTool(nativeToolNames.sendTaskMessage, { + taskId: 'task-1', + message: 'Continue the saved work', + ...(instruction ? { continuation: 'instruction' } : {}), + }), + ); + await invokeTool(nativeToolNames.sendChatReply, { + purpose: 'closeout', + message: 'Reported the outcome.', + }); + return ''; + }, + ); + const turn = async (id: string) => + answerFastAgentQuestion({ + ...baseParams, + currentMessageId: id, + turnSource: human ? 'human' : 'platform_event', + question: human + ? instruction + ? 'Resume the task now.' + : 'Any update?' + : `task_settled runId=${id}`, + adapter: callbacks(), + }); + await turn('run-1'); + await turn('run-2'); + instruction = true; + await turn('run-3'); // A platform event cannot manufacture a human reset. + human = true; + instruction = false; + await turn('check-in'); + instruction = true; + await turn('explicit-request'); + human = false; + instruction = false; + await turn('run-4'); + await turn('run-5'); + + expect(outcomes[0]).toMatchObject({ success: true }); + for (const index of [1, 3, 6]) { + expect(outcomes[index]).toMatchObject({ + success: false, + delivery: 'not_accepted', + recovery: 'budget_exhausted', + }); + } + expect(outcomes[2]).toMatchObject({ + success: false, + delivery: 'not_accepted', + }); + expect(outcomes[4]).toMatchObject({ success: true }); + expect(outcomes[5]).toMatchObject({ success: true }); + expect(mocks.sendTaskMessage).toHaveBeenCalledTimes(3); + }); + + it('does not send a continuation when its durable claim cannot be stored', async () => { + mocks.getActiveTasks.mockResolvedValue([ + { taskId: 'task-1', title: 'Checkout', status: 'idle' }, + ]); + mocks.upsertMessage.mockImplementation(async ({ message }) => { + if (message.payload?.toolName === 'send_task_message') { + throw new Error('Claim storage unavailable'); + } + return { initialHumanTurn: false }; + }); + mocks.generateText.mockImplementation( + async (_params, _session, options) => { + await options.onSessionReady('opencode-session-1'); + await expect( + invokeTool(nativeToolNames.sendTaskMessage, { + taskId: 'task-1', + message: 'Continue', + }), + ).rejects.toThrow('Claim storage unavailable'); + return ''; + }, + ); + await answerFastAgentQuestion({ + ...baseParams, + turnSource: 'platform_event', + adapter: callbacks(), + }); + expect(mocks.sendTaskMessage).not.toHaveBeenCalled(); + }); + + it('does not execute tools when recovery history cannot be loaded', async () => { + mocks.loadTaskMessageHistory.mockRejectedValue( + new Error('History unavailable'), + ); + await answerFastAgentQuestion({ ...baseParams, adapter: callbacks() }); + expect(mocks.sendTaskMessage).not.toHaveBeenCalled(); + expect(mocks.generateText).not.toHaveBeenCalled(); + }); + it('does not execute tools when a resumed turn cannot load its delivery history', async () => { mocks.loadTurnAttempt.mockRejectedValueOnce( new Error('history unavailable'), @@ -10099,6 +10250,104 @@ describe('answerFastAgentQuestion native OpenCode tools', () => { } }); + it.each(['ignored event', 'empty response', 'reaction'] as const)( + 'retires a hidden retry marker after successful %s completion', + async (completion) => { + vi.useFakeTimers(); + try { + mocks.generateText + .mockRejectedValueOnce(new Error('TypeError: fetch failed')) + .mockImplementationOnce(async (_params, _session, options) => { + await options.onSessionReady('opencode-session-1'); + if (completion === 'ignored event') { + await invokeTool(nativeToolNames.ignoreEvent, { + reason: 'Duplicate task update.', + }); + } else if (completion === 'reaction') { + await invokeTool(nativeToolNames.sendChatReaction, { + name: 'thumbsup', + purpose: 'closeout', + }); + } + return ''; + }); + const adapter = callbacks(); + const result = answerFastAgentQuestion({ + ...baseParams, + ...(completion === 'reaction' + ? { allowSilentAmbientReply: true } + : { turnSource: 'platform_event' as const }), + adapter, + }); + await vi.runAllTimersAsync(); + await result; + + const retryWrites = mocks.upsertMessage.mock.calls + .map(([input]) => input.message) + .filter((message) => message.eventId === '100.2:retry-notice:0'); + expect(retryWrites[0]?.metadata).toMatchObject({ + inferenceRetryActive: true, + visibleInTranscript: false, + }); + expect(retryWrites.at(-1)?.metadata).toMatchObject({ + inferenceRetryNotice: true, + inferenceRetryActive: false, + visibleInTranscript: false, + }); + expect(adapter.postReply).not.toHaveBeenCalled(); + expect(mocks.reconcileRetryNotices).toHaveBeenCalledWith( + 'conversation-1', + 'turn_settled_reconcile', + {}, + ); + } finally { + vi.useRealTimers(); + } + }, + ); + + it('replaces a visible retry notice when an ambient turn recovers and is ignored', async () => { + vi.useFakeTimers(); + try { + const error = Object.assign(new Error('429 Too Many Requests'), { + providerError: { data: { responseHeaders: { 'retry-after': '45' } } }, + }); + mocks.generateText + .mockRejectedValueOnce(error) + .mockImplementationOnce(async (_params, _session, options) => { + await options.onSessionReady('opencode-session-1'); + await invokeTool(nativeToolNames.ignoreEvent, { + reason: 'No response needed.', + }); + return ''; + }); + const postReply = vi.fn().mockResolvedValue({ messageId: 'retry-1' }); + const replaceReply = vi.fn().mockResolvedValue({ messageId: 'retry-1' }); + const result = answerFastAgentQuestion({ + ...baseParams, + allowSilentAmbientReply: true, + adapter: callbacks({ postReply, replaceReply }), + }); + await vi.runAllTimersAsync(); + await result; + + expect(postReply).toHaveBeenCalledOnce(); + expect(replaceReply).toHaveBeenLastCalledWith( + { messageId: 'retry-1' }, + { purpose: 'closeout', message: 'The retry completed.' }, + ); + const retryWrites = mocks.upsertMessage.mock.calls + .map(([input]) => input.message) + .filter((message) => message.eventId === '100.2:retry-notice:0'); + expect(retryWrites.at(-1)?.metadata).toMatchObject({ + inferenceRetryActive: false, + platformMessageId: 'retry-1', + }); + } finally { + vi.useRealTimers(); + } + }); + it('rethrows native prompt failures for platform event retry', async () => { mocks.generateText.mockRejectedValue(new Error('OpenCode unavailable')); const activity = { diff --git a/packages/cloud-agents/src/server/fast-agent/__tests__/fast-agent-task-message-guard.test.ts b/packages/cloud-agents/src/server/fast-agent/__tests__/fast-agent-task-message-guard.test.ts index e28e710be..77f752f13 100644 --- a/packages/cloud-agents/src/server/fast-agent/__tests__/fast-agent-task-message-guard.test.ts +++ b/packages/cloud-agents/src/server/fast-agent/__tests__/fast-agent-task-message-guard.test.ts @@ -14,6 +14,30 @@ const action = ( }); describe('FastAgentTaskMessageGuard', () => { + it('does not authorize recovery after an instruction delivery was lost', async () => { + const guard = new FastAgentTaskMessageGuard(); + guard.restoreRecoveryHistory( + [ + action({ + continuation: 'instruction', + instructionId: 'human-1', + status: 'unknown', + result: undefined, + }), + ], + ['task-1'], + ); + const deliver = vi.fn(); + expect( + await guard.send('task-1', args, deliver, { kind: 'recovery' }), + ).toMatchObject({ + success: false, + delivery: 'not_accepted', + recovery: 'budget_exhausted', + }); + expect(deliver).not.toHaveBeenCalled(); + }); + it('caches accepted retries but allows distinct followups and attachment opt-in', async () => { const guard = new FastAgentTaskMessageGuard(); const deliver = vi.fn().mockResolvedValue({ success: true, queued: true }); @@ -192,4 +216,249 @@ describe('FastAgentTaskMessageGuard', () => { await guard.send('task-2', args, deliver); expect(deliver).toHaveBeenCalledTimes(2); }); + + const recovery = { kind: 'recovery' } as const; + const exhausted = { + success: false, + delivery: 'not_accepted', + recovery: 'budget_exhausted', + }; + + it('reconstructs the budget across runs without restoring cross-turn receipts', async () => { + const events = [ + action({ continuation: 'instruction', instructionId: 'human-1' }), + ]; + const first = new FastAgentTaskMessageGuard(); + first.restoreRecoveryHistory(events, ['task-1']); + const deliver = vi.fn().mockResolvedValue({ success: true }); + const receipt = await first.send('task-1', args, deliver, recovery); + events.push( + action({ continuation: 'recovery', result: JSON.stringify(receipt) }), + ); + expect(await first.send('task-1', args, deliver, recovery)).toEqual( + receipt, + ); + expect( + await first.send('task-1', { message: 'Different' }, deliver, recovery), + ).toMatchObject(exhausted); + first.clear(); + expect(await first.send('task-1', args, deliver, recovery)).toMatchObject( + exhausted, + ); + + const second = new FastAgentTaskMessageGuard(); + second.restoreRecoveryHistory(events, ['task-1']); + expect(await second.send('task-1', args, deliver, recovery)).toMatchObject( + exhausted, + ); + expect(deliver).toHaveBeenCalledOnce(); + expect(await second.send('task-2', args, deliver, recovery)).toMatchObject({ + success: true, + }); + }); + + it('combines same-turn restore and history without accepting a second recovery', async () => { + const event = action({ continuation: 'recovery' }); + const guard = new FastAgentTaskMessageGuard(); + guard.restore([event], ['task-1']); + guard.restoreRecoveryHistory([event], ['task-1']); + const deliver = vi.fn(); + expect(await guard.send('task-1', args, deliver, recovery)).toMatchObject({ + success: true, + }); + expect( + await guard.send('task-1', { message: 'Different' }, deliver, recovery), + ).toMatchObject(exhausted); + expect(deliver).not.toHaveBeenCalled(); + }); + + it('resets only for an accepted new explicit instruction ID, even with identical text', async () => { + const guard = new FastAgentTaskMessageGuard(); + const deliver = vi.fn().mockResolvedValue({ success: true }); + const instruction = { + kind: 'instruction', + instructionId: 'human-1', + } as const; + await guard.send('task-1', args, deliver, instruction); + await guard.send('task-1', args, deliver, recovery); + guard.clear(); + await guard.send('task-1', args, deliver, instruction); + expect(await guard.send('task-1', args, deliver, recovery)).toMatchObject( + exhausted, + ); + await guard.send('task-1', args, deliver, { + ...instruction, + instructionId: 'human-2', + }); + expect(await guard.send('task-1', args, deliver, recovery)).toMatchObject({ + success: true, + }); + guard.clear(); + await guard.send('task-1', args, deliver, instruction); + expect(await guard.send('task-1', args, deliver, recovery)).toMatchObject( + exhausted, + ); + expect(deliver).toHaveBeenCalledTimes(6); + }); + + it.each([ + { success: false, delivery: 'not_accepted' }, + { success: false }, + { success: true, delivery: 'unknown' }, + ])('does not reset for an instruction with result %j', async (result) => { + const guard = new FastAgentTaskMessageGuard(); + guard.restoreRecoveryHistory( + [action({ continuation: 'recovery' })], + ['task-1'], + ); + await guard.send('task-1', args, async () => result, { + kind: 'instruction', + instructionId: 'new', + }); + guard.clear(); + const deliver = vi.fn(); + expect(await guard.send('task-1', args, deliver, recovery)).toMatchObject( + exhausted, + ); + expect(deliver).not.toHaveBeenCalled(); + }); + + it.each([undefined, '', ' '])( + 'does not reset for empty instruction ID %j or default sends', + async (instructionId) => { + const guard = new FastAgentTaskMessageGuard(); + guard.restoreRecoveryHistory( + [action({ continuation: 'recovery' })], + ['task-1'], + ); + const deliver = vi.fn().mockResolvedValue({ success: true }); + await guard.send('task-1', args, deliver, { + kind: 'instruction', + instructionId, + }); + await guard.send('task-1', args, deliver); + expect(await guard.send('task-1', args, deliver, recovery)).toMatchObject( + exhausted, + ); + }, + ); + + it('reconstructs accepted instruction IDs without letting repeats reset the budget', async () => { + const initial = action({ + continuation: 'instruction', + instructionId: 'human-1', + }); + const recovered = action({ continuation: 'recovery' }); + const guard = new FastAgentTaskMessageGuard(); + guard.restoreRecoveryHistory([initial, recovered, initial], ['task-1']); + const deliver = vi.fn().mockResolvedValue({ success: true }); + expect(await guard.send('task-1', args, deliver, recovery)).toMatchObject( + exhausted, + ); + guard.restoreRecoveryHistory( + [action({ continuation: 'instruction', instructionId: 'human-2' })], + ['task-1'], + ); + expect(await guard.send('task-1', args, deliver, recovery)).toMatchObject({ + success: true, + }); + expect(deliver).toHaveBeenCalledOnce(); + }); + + it.each([ + { status: 'unknown', result: undefined }, + { status: 'completed', result: '{truncated' }, + { status: 'failed', result: JSON.stringify({ success: false }) }, + { status: 'unknown', result: JSON.stringify({ delivery: 'not_accepted' }) }, + ] as const)( + 'conservatively consumes all current tasks for untargeted $status recovery', + async (event) => { + const guard = new FastAgentTaskMessageGuard(); + guard.restoreRecoveryHistory( + [action({ ...event, continuation: 'recovery' })], + ['task-1', 'task-2'], + ); + guard.clear(); + const deliver = vi.fn(); + for (const taskId of ['task-1', 'task-2']) { + expect(await guard.send(taskId, args, deliver, recovery)).toMatchObject( + exhausted, + ); + } + expect(deliver).not.toHaveBeenCalled(); + }, + ); + + it('ignores legacy history and definite recovery rejections without resetting prior consumption', async () => { + const guard = new FastAgentTaskMessageGuard(); + const rejected = action({ + continuation: 'recovery', + status: 'failed', + result: JSON.stringify({ delivery: 'not_accepted' }), + }); + guard.restoreRecoveryHistory( + [action(), action({ status: 'unknown' }), rejected], + ['task-1'], + ); + const deliver = vi.fn().mockResolvedValue({ success: true }); + await guard.send('task-1', args, deliver, recovery); + guard.clear(); + guard.restoreRecoveryHistory( + [action({ instructionId: 'legacy' }), rejected], + ['task-1'], + ); + expect(await guard.send('task-1', args, deliver, recovery)).toMatchObject( + exhausted, + ); + expect(deliver).toHaveBeenCalledOnce(); + }); + + it('reserves recovery before await, survives clear in flight, and releases explicit rejection', async () => { + const guard = new FastAgentTaskMessageGuard(); + let resolve!: (result: Record) => void; + const deliver = vi.fn( + () => + new Promise>((done) => { + resolve = done; + }), + ); + const pending = guard.send('task-1', args, deliver, recovery); + expect(await guard.send('task-1', args, deliver, recovery)).toMatchObject( + exhausted, + ); + guard.clear(); + expect( + await guard.send('task-1', { message: 'Different' }, deliver, recovery), + ).toMatchObject(exhausted); + resolve({ success: false, delivery: 'not_accepted' }); + await pending; + expect( + await guard.send( + 'task-1', + args, + async () => ({ success: true }), + recovery, + ), + ).toMatchObject({ success: true }); + expect(deliver).toHaveBeenCalledOnce(); + }); + + it.each(['failure', 'throw'])( + 'keeps recovery consumed after %s and clear', + async (mode) => { + const guard = new FastAgentTaskMessageGuard(); + const deliver = vi.fn(async () => { + if (mode === 'throw') throw new Error('Lost'); + return { success: false }; + }); + expect(await guard.send('task-1', args, deliver, recovery)).toMatchObject( + { delivery: 'unknown' }, + ); + guard.clear(); + expect(await guard.send('task-1', args, deliver, recovery)).toMatchObject( + exhausted, + ); + expect(deliver).toHaveBeenCalledOnce(); + }, + ); }); diff --git a/packages/cloud-agents/src/server/fast-agent/fast-agent-conversation-repository.ts b/packages/cloud-agents/src/server/fast-agent/fast-agent-conversation-repository.ts index 00c8e1277..cd1bab895 100644 --- a/packages/cloud-agents/src/server/fast-agent/fast-agent-conversation-repository.ts +++ b/packages/cloud-agents/src/server/fast-agent/fast-agent-conversation-repository.ts @@ -268,6 +268,9 @@ export type FastAgentTurnAttemptAction = { /** 'unknown' when the call was recorded but the process died before its result. */ status: 'completed' | 'failed' | 'unknown'; result?: string; + /** Server-authored task-message intent; absent on legacy rows. */ + continuation?: 'instruction' | 'recovery'; + instructionId?: string; }; export type FastAgentTurnAttemptReplyPurpose = @@ -320,6 +323,86 @@ export type FastAgentTurnAttemptSummary = { const TURN_ATTEMPT_RESULT_MAX_CHARS = 1_200; +function turnAttemptText(blocks: unknown): string { + return Array.isArray(blocks) + ? blocks + .flatMap((block) => + block && + typeof block === 'object' && + (block as { type?: unknown }).type === 'text' + ? [String((block as { text?: unknown }).text ?? '')] + : [], + ) + .join('') + : ''; +} + +function turnAttemptAction(row: { + eventType: string; + payload: unknown; + metadata: unknown; + contentBlocks: unknown; +}): FastAgentTurnAttemptAction { + const payload = (row.payload ?? {}) as Record; + const metadata = (row.metadata ?? {}) as Record; + // Native/MCP input wraps arguments; subagent input is persisted directly. + const rawInput = payload.rawInput as Record | undefined; + const continuation = metadata.taskMessageContinuation; + const instructionId = metadata.taskMessageInstructionId; + const action: FastAgentTurnAttemptAction = { + kind: 'action', + tool: String(payload.toolName ?? payload.title ?? 'tool'), + arguments: + rawInput && typeof rawInput === 'object' + ? 'arguments' in rawInput + ? rawInput.arguments + : rawInput + : null, + status: + row.eventType === ACP_ENVELOPE_EVENT_TYPES.ToolCall + ? 'unknown' + : payload.status === 'failed' + ? 'failed' + : 'completed', + ...(continuation === 'instruction' || continuation === 'recovery' + ? { continuation } + : {}), + ...(typeof instructionId === 'string' ? { instructionId } : {}), + }; + if (action.status !== 'unknown') { + const output = turnAttemptText(row.contentBlocks); + if (output) action.result = output; + } + return action; +} + +/** Canonical task-message receipts across all turns, including hidden/current work. */ +export async function loadFastAgentTaskMessageHistory( + conversationId: string, +): Promise { + const rows = await db + .select({ + eventType: fastAgentMessages.eventType, + payload: fastAgentMessages.payload, + metadata: fastAgentMessages.metadata, + contentBlocks: fastAgentMessages.contentBlocks, + }) + .from(fastAgentMessages) + .where( + and( + eq(fastAgentMessages.conversationId, conversationId), + or( + eq(fastAgentMessages.eventType, ACP_ENVELOPE_EVENT_TYPES.ToolCall), + eq(fastAgentMessages.eventType, ACP_ENVELOPE_EVENT_TYPES.ToolResult), + ), + sql`${fastAgentMessages.payload}->>'toolName' = 'send_task_message'`, + ), + ) + // Result upserts preserve createdAt, so late results cannot reorder receipts. + .orderBy(fastAgentMessages.createdAt, fastAgentMessages.turnSeq); + return rows.map(turnAttemptAction); +} + /** * What an earlier attempt at this turn already did, for the run that resumes * it. Every tool call is recorded before it executes and its result after, @@ -351,19 +434,6 @@ export async function loadFastAgentTurnAttemptSummary( ) .orderBy(fastAgentMessages.turnSeq, fastAgentMessages.ts); - const text = (blocks: unknown) => - Array.isArray(blocks) - ? blocks - .flatMap((block) => - block && - typeof block === 'object' && - (block as { type?: unknown }).type === 'text' - ? [String((block as { text?: unknown }).text ?? '')] - : [], - ) - .join('') - : ''; - const events: FastAgentTurnAttemptEvent[] = []; // A call and its result share one canonical event, so normally only one row // per call survives; when both are present the later row wins in place. @@ -411,36 +481,12 @@ export async function loadFastAgentTurnAttemptSummary( // process died between starting the call and recording its outcome. const toolCallId = String(payload.toolCallId ?? ''); if (!toolCallId) continue; - // Native and MCP calls wrap their input as `rawInput.arguments`; - // subagent task calls persist the input object directly. - const rawInput = payload.rawInput as - | { arguments?: unknown } - | Record - | undefined; - const action: FastAgentTurnAttemptAction = { - kind: 'action', - tool: String(payload.toolName ?? payload.title ?? 'tool'), - arguments: - rawInput && typeof rawInput === 'object' - ? 'arguments' in rawInput - ? rawInput.arguments - : rawInput - : null, - status: - row.eventType === ACP_ENVELOPE_EVENT_TYPES.ToolCall - ? 'unknown' - : payload.status === 'failed' - ? 'failed' - : 'completed', - }; - if (action.status !== 'unknown') { - const output = text(row.contentBlocks); - if (output) { - action.result = - output.length > TURN_ATTEMPT_RESULT_MAX_CHARS - ? `${output.slice(0, TURN_ATTEMPT_RESULT_MAX_CHARS)}…` - : output; - } + const action = turnAttemptAction(row); + if ( + action.result && + action.result.length > TURN_ATTEMPT_RESULT_MAX_CHARS + ) { + action.result = `${action.result.slice(0, TURN_ATTEMPT_RESULT_MAX_CHARS)}…`; } const index = actionIndexByCallId.get(toolCallId); if (index === undefined) { @@ -455,7 +501,7 @@ export async function loadFastAgentTurnAttemptSummary( metadata.visibleInTranscript !== false && metadata.interruptionReason === undefined ) { - const reply = text(row.contentBlocks).trim(); + const reply = turnAttemptText(row.contentBlocks).trim(); if (!reply) continue; const purpose = payload.purpose ?? metadata.purpose; events.push({ diff --git a/packages/cloud-agents/src/server/fast-agent/fast-agent-native-tool-bridge.ts b/packages/cloud-agents/src/server/fast-agent/fast-agent-native-tool-bridge.ts index 4aac300d1..48839a337 100644 --- a/packages/cloud-agents/src/server/fast-agent/fast-agent-native-tool-bridge.ts +++ b/packages/cloud-agents/src/server/fast-agent/fast-agent-native-tool-bridge.ts @@ -358,6 +358,7 @@ export default { args: { taskId: z.string().nullable().optional(), message: z.string().min(1), + continuation: z.enum(["instruction", "recovery"]).optional().describe("Defaults to recovery, with a durable one-attempt budget per task and outstanding request. Use instruction only to forward a new explicit directive from the current human message, never for a status check or platform event."), includeAttachments: z.boolean().optional().describe("Set true to forward supported images and extracted file, audio, or video context from the active conversation turn; defaults to false"), }, execute: (args, context) => invoke("send_task_message", args, context), diff --git a/packages/cloud-agents/src/server/fast-agent/fast-agent-prompt.ts b/packages/cloud-agents/src/server/fast-agent/fast-agent-prompt.ts index 379226954..d643b270c 100644 --- a/packages/cloud-agents/src/server/fast-agent/fast-agent-prompt.ts +++ b/packages/cloud-agents/src/server/fast-agent/fast-agent-prompt.ts @@ -360,6 +360,9 @@ ${reactionGuidance} - You may launch multiple independent tasks in one turn after one acknowledgement that clearly covers them. Do not add a separate launch message for each task; the turn remains open for more tools. - Set "model" on "launch_task" only to an exact ID from Available Delegated Task Models when a specific model is useful or requested. Omit it to use the deployment default. Never invent or abbreviate model IDs. - Use "send_task_message" when an active or resumable task is listed above and the user clearly gives that task a new instruction. On a human-authored turn, acknowledge first, then send the instruction immediately. Set "includeAttachments" to true only when supported attachments from the active conversation turn are relevant to that instruction; omit it otherwise. A resumable settled task continues under the same task identity. Set "taskId" when needed; with exactly one listed task, omit it or use null. Afterward, post a concise closeout confirming the outcome when useful. +- "send_task_message" defaults to continuation="recovery". Set continuation="instruction" only when forwarding a new explicit directive from the current human message, never for a check-in, status question, or task event. The server rejects instruction mode without current human input and derives its request identity itself. Recovery claims and results are persisted before/after delivery and enforced across turns and run IDs; only an accepted new human instruction resets that task's budget. If the tool returns recovery="budget_exhausted", report the blocker and wait for a new explicit instruction. Do not change continuation mode or reword the message to bypass the refusal. A failed or unknown delivery does not establish a new instruction boundary. +- Bounded recovery of already-authorized work is a separate use of "send_task_message", not a failed-start retry. Outside presentation-only events, when a task event or human check-in reveals that unfinished authorized work stopped on an explicitly transient provider or transport error, inspect the current task state and conversation history first. If the task is resumable, not executing or already queued to resume, and prior effects are known, continue the same task from its saved work rather than offer to resume. Allow at most one automatic recovery attempt per task for the outstanding user request, across turns and run IDs; record that attempt in your reply after the send is accepted. Duplicate events, later check-ins, and new run IDs do not reset this limit. If prior recovery or delivery is uncertain, inspect it; if uncertainty remains, report the blocker instead of sending again. +- Never auto-resume canceled or user-stopped work, needs-input or approval waits, completed or superseded work, nonretryable errors (including authentication, permissions, billing, quota, configuration, or content-policy failures), or work with ambiguous side effects. A generic provider error is not proof it is transient. If the bounded recovery fails or its budget is exhausted, report the remaining blocker and require a new explicit user instruction before another attempt. Do not launch a replacement task or schedule a retry loop. Say work is continuing only when current execution or an accepted continuation supports it; accepting a continuation is not completion. - Use \`roomote_manage_tasks\` to inspect tasks in this deployment. Use "get_summary" for current status and failures, "get_messages" for transcript details, and "get_compute_logs" for runtime output when supported. Keep using "launch_task", "send_task_message", or "cancel_task" for task changes so Fast conversation association and follow-up behavior are preserved. - Use \`roomote_get_chat_message_context\` or \`roomote_get_chat_channel_messages\` for additional chat context. Pass the target channel or message reference required by the native tool schema. Slack channel history defaults to the previous 24 hours when \`oldest\` is omitted. - Never send conversational acknowledgements to a task. "Okay", "cool", "thanks", status questions, and similar conversation are addressed to you. Use a user-visible chat tool. @@ -418,7 +421,7 @@ ${ ${ retryTaskStartAvailable ? '- Call `retry_task_start` only when the failure appears transient; do not use it for clear configuration, authentication, permission, billing, quota, missing-resource, or other permanent failures. Report its result with one closeout.' - : '- No failed-start retry tool is available for this event. Report or ignore it without retrying.' + : '- No failed-start retry tool is available for this event. Do not retry startup; the separate bounded recovery rule may apply to already-started authorized work.' } - Launching creates a separate delegated task; it does not retry the task associated with this event. - Do not use the reaction tool because a platform event has no incoming chat message to react to. If the event warrants a response, post a text reply; otherwise stay silent according to the ignore rules above. @@ -455,7 +458,7 @@ ${ - Pull-request-feedback events contain triaged feedback for a delegated task's pull request. Summarize the findings only in one closeout, then stop. Do not ask a closing question, repeat or paraphrase a supplied question, or offer to resolve the issues in your message. The conversation adapter supplies any pending user-approvable actions. Do not launch a fix or call "send_task_message" until the user explicitly responds or clicks an action. These events are visibility-required and must never be ignored. - Pull-request-status-changed events contain an authoritative merged or closed status and should be presented unless that exact status was already reported for the pull request. When \`targetBranch\` is absent from the pull request metadata, do not infer or name a destination branch. Do not describe a closed pull request as merged or a merged pull request as merely closed. - A newer authoritative merged or closed pull-request event always takes precedence over an older child-authored report, even when that stale report arrives later. Keep useful child findings visible without repeating or endorsing stale claims that the pull request remains open, draft, or unpublished. -- Task-settled events include the task's current pull requests. Use them in a closeout only when there is a user-useful result or changed outcome, without describing an already-reported pull request as newly opened. Settled, stopped, or failed state by itself is not worth posting. +- Task-settled events include the task's current pull requests. Use them in a closeout only when there is a user-useful result or changed outcome, without describing an already-reported pull request as newly opened. Settled, stopped, or failed state by itself is not worth posting. A stored error that interrupted unfinished work is a changed outcome, even when the task remains idle/resumable: apply the bounded recovery rule or report the blocker, rather than ignore it as routine lifecycle state. Current execution/error evidence takes precedence over older child-authored progress claims. ` : reactionInput ? `## Human Reaction Input diff --git a/packages/cloud-agents/src/server/fast-agent/fast-agent-service.ts b/packages/cloud-agents/src/server/fast-agent/fast-agent-service.ts index b47b4a750..94696e2bf 100644 --- a/packages/cloud-agents/src/server/fast-agent/fast-agent-service.ts +++ b/packages/cloud-agents/src/server/fast-agent/fast-agent-service.ts @@ -138,6 +138,7 @@ import { type FastAgentTurnAttemptSummary, type FastAgentUnresolvedRequest, loadFastAgentTurnAttemptSummary, + loadFastAgentTaskMessageHistory, } from './fast-agent-conversation-repository'; import { bindFastAgentNativeToolExecutor, @@ -471,6 +472,10 @@ const taskMessageArgsSchema = z.object({ taskId: z.string().trim().min(1).nullable().optional(), message: z.string().trim().min(1), includeAttachments: z.boolean().optional().default(false), + continuation: z + .enum(['instruction', 'recovery']) + .optional() + .default('recovery'), }); const taskIdArgsSchema = z.object({ taskId: z.string().trim().min(1).nullable().optional(), @@ -1664,6 +1669,15 @@ export async function answerFastAgentQuestion({ const reactionInput = !platformEvent && humanInput.type === FAST_AGENT_REACTION_INPUT_TYPE; const substantiveHumanInput = !platformEvent && !reactionInput; + let taskMessageInstructionId = + substantiveHumanInput && !allowSilentAmbientReply ? turnId : undefined; + const taskMessageContinuation = (args: Record) => + args.continuation === 'instruction' && taskMessageInstructionId + ? { + kind: 'instruction' as const, + instructionId: taskMessageInstructionId, + } + : { kind: 'recovery' as const }; const currentMessageReactable = substantiveHumanInput; const transcriptPayload = reactionInput ? { externalInput: humanInput.externalInput } @@ -2413,6 +2427,7 @@ export async function answerFastAgentQuestion({ // directed. if (batch.some(({ followUp }) => followUp.directedAtRoomote !== false)) { steeredDirectedFollowUp = true; + taskMessageInstructionId = batch.at(-1)!.row.id; } injectedHumanFollowUpMessages.push(...batchMessages); injectedHumanFollowUpFiles.push(...batchFiles); @@ -2523,6 +2538,19 @@ export async function answerFastAgentQuestion({ title !== FAST_AGENT_NATIVE_TOOL_NAMES.sendChatReply && title !== FAST_AGENT_NATIVE_TOOL_NAMES.sendChatReaction; const canonicalEvent = allocateCanonicalEvent(`tool:${ordinal}`); + const continuation = + title === FAST_AGENT_NATIVE_TOOL_NAMES.sendTaskMessage + ? taskMessageContinuation(args) + : undefined; + const continuationMetadata = continuation + ? { + taskMessageContinuation: continuation.kind, + taskMessageInstructionId: + 'instructionId' in continuation + ? continuation.instructionId + : undefined, + } + : {}; await persistCanonicalMessage( { ...canonicalEvent, @@ -2531,7 +2559,7 @@ export async function answerFastAgentQuestion({ eventType: ACP_ENVELOPE_EVENT_TYPES.ToolCall, role: 'tool', contentBlocks: [], - metadata: { visibleInTranscript }, + metadata: { visibleInTranscript, ...continuationMetadata }, payload: { toolCallId, title, @@ -2551,7 +2579,7 @@ export async function answerFastAgentQuestion({ source: conversation.surface, nativeSessionId: nativeSessionId ?? activeOpenCodeSessionId, }, - true, + !continuation, ); return { ordinal, @@ -2564,6 +2592,7 @@ export async function answerFastAgentQuestion({ kind, visibleInTranscript, canonicalEvent, + continuationMetadata, }; }; const finishCanonicalToolEvent = async ( @@ -2585,7 +2614,11 @@ export async function answerFastAgentQuestion({ eventType: ACP_ENVELOPE_EVENT_TYPES.ToolResult, role: 'tool', contentBlocks: output ? [{ type: 'text', text: output }] : [], - metadata: { visibleInTranscript: event.visibleInTranscript, truncated }, + metadata: { + visibleInTranscript: event.visibleInTranscript, + truncated, + ...event.continuationMetadata, + }, payload: { toolCallId: event.toolCallId, title: event.title, @@ -2916,6 +2949,19 @@ export async function answerFastAgentQuestion({ ? await loadFastAgentTurnAttemptSummary(session.id, turnId) : null; if (previousAttempt) { + // A human steer may have advanced the request inside this same turn. + // Replaying it must not mint a fresh instruction identity/reset budget. + const lastInstruction = [...previousAttempt.events] + .reverse() + .find( + (event) => + event.kind === 'action' && + event.continuation === 'instruction' && + event.instructionId, + ); + if (lastInstruction?.kind === 'action') { + taskMessageInstructionId = lastInstruction.instructionId; + } nextAssistantOrdinal = previousAttempt.next.assistantOrdinal; nextToolOrdinal = previousAttempt.next.toolOrdinal; nextRetryNoticeOrdinal = previousAttempt.next.retryNoticeOrdinal; @@ -3081,6 +3127,10 @@ export async function answerFastAgentQuestion({ const currentTasks = new Map( resolvedActiveTasks.map((task) => [task.taskId, task]), ); + taskMessageGuard.restoreRecoveryHistory( + await loadFastAgentTaskMessageHistory(session.id), + [...currentTasks.keys()], + ); taskMessageGuard.restore(previousAttempt?.events ?? [], [ ...currentTasks.keys(), ]); @@ -4189,6 +4239,17 @@ export async function answerFastAgentQuestion({ }; } const args = parsed.data; + if ( + args.continuation === 'instruction' && + !taskMessageInstructionId + ) { + return { + success: false, + delivery: 'not_accepted', + error: + 'Only a current human instruction may reset the recovery budget.', + }; + } const target = selectActiveTaskId(args.taskId, currentTasks); if (!target.taskId) { return { @@ -4205,17 +4266,21 @@ export async function answerFastAgentQuestion({ attachmentTexts, }) : args.message; - return await taskMessageGuard.send(taskId, args, () => - sendFastAgentTaskMessage( - { userId, apiBaseUrl }, - { - taskId, - message, - ...(args.includeAttachments && images.length > 0 - ? { images } - : {}), - }, - ), + return await taskMessageGuard.send( + taskId, + args, + () => + sendFastAgentTaskMessage( + { userId, apiBaseUrl }, + { + taskId, + message, + ...(args.includeAttachments && images.length > 0 + ? { images } + : {}), + }, + ), + taskMessageContinuation(args), ); } @@ -5099,6 +5164,17 @@ export async function answerFastAgentQuestion({ ); } } + // Silent/ignored turns have no text reply to retire their retry marker. + // Leaving it active would make reconciliation report a false interruption. + if (inferenceRetryCanonicalEvent) { + await replaceInferenceRetryReply( + { purpose: 'closeout', message: 'The retry completed.' }, + true, + ); + inferenceRetryReply = undefined; + inferenceRetryMessageIndex = undefined; + inferenceRetryCanonicalEvent = undefined; + } await settleDurableTurn(); await mirrorPendingMessages(); return lastVisibleMessage; diff --git a/packages/cloud-agents/src/server/fast-agent/fast-agent-task-message-guard.ts b/packages/cloud-agents/src/server/fast-agent/fast-agent-task-message-guard.ts index d1b66cc9a..30dca5970 100644 --- a/packages/cloud-agents/src/server/fast-agent/fast-agent-task-message-guard.ts +++ b/packages/cloud-agents/src/server/fast-agent/fast-agent-task-message-guard.ts @@ -2,28 +2,96 @@ import type { FastAgentTurnAttemptEvent } from './fast-agent-conversation-reposi type Result = Record; type MessageArgs = { message: string; includeAttachments?: boolean }; +type Continuation = { + kind: 'instruction' | 'recovery'; + instructionId?: string; +}; const unknownGuidance = 'Delivery is unknown. Do not resend or reword the instruction. Check the task status and transcript before taking further action.'; -function signature(args: MessageArgs): string { +function signature(args: MessageArgs, continuation?: Continuation): string { // Attachments are fixed within the human instruction boundary. Keep the // signature private; persisted receipts already contain the original args. return JSON.stringify([ args.message.trim(), args.includeAttachments ?? false, + ...(continuation + ? [continuation.kind, continuation.instructionId ?? null] + : []), ]); } export class FastAgentTaskMessageGuard { private readonly unresolved = new Set(); private readonly receipts = new Map>(); + private readonly recoveryReservations = new Map(); + private readonly acceptedInstructions = new Map>(); clear(): void { this.unresolved.clear(); this.receipts.clear(); } + restoreRecoveryHistory( + events: FastAgentTurnAttemptEvent[], + currentTaskIds: string[], + ): void { + for (const event of events) { + if ( + event.kind !== 'action' || + event.tool !== 'send_task_message' || + !event.continuation + ) + continue; + let result: Result = {}; + try { + const parsed: unknown = JSON.parse(event.result ?? '{}'); + if (parsed && typeof parsed === 'object' && !Array.isArray(parsed)) { + result = parsed as Result; + } + } catch { + // A truncated recovery receipt still spends the durable budget. + } + if (event.status !== 'unknown' && result.delivery === 'not_accepted') { + continue; + } + const args = event.arguments as { taskId?: unknown } | undefined; + const taskId = [result.taskId, args?.taskId] + .find( + (value): value is string => + typeof value === 'string' && !!value.trim(), + ) + ?.trim(); + if (event.continuation === 'recovery') { + for (const id of taskId ? [taskId] : currentTaskIds) { + this.recoveryReservations.set(id, Symbol()); + } + } else if ( + taskId && + event.status === 'completed' && + result.success === true && + result.delivery !== 'unknown' + ) { + this.acceptInstruction(taskId, event.instructionId); + } else { + // An instruction whose outcome was lost cannot authorize recovery. + for (const id of taskId ? [taskId] : currentTaskIds) { + this.recoveryReservations.set(id, Symbol()); + } + } + } + } + + private acceptInstruction(taskId: string, instructionId?: string): void { + if (!instructionId?.trim()) return; + const accepted = this.acceptedInstructions.get(taskId) ?? new Set(); + if (accepted.has(instructionId)) return; + accepted.add(instructionId); + this.acceptedInstructions.set(taskId, accepted); + this.recoveryReservations.delete(taskId); + } + restore(events: FastAgentTurnAttemptEvent[], currentTaskIds: string[]): void { for (const event of events) { if (event.kind !== 'action' || event.tool !== 'send_task_message') @@ -58,15 +126,28 @@ export class FastAgentTaskMessageGuard { if ( event.status === 'completed' && result.success === true && + result.delivery !== 'unknown' && typeof args?.message === 'string' && args.message.trim() && (args.includeAttachments === undefined || typeof args.includeAttachments === 'boolean') ) { - this.remember(taskId, signature(args as MessageArgs), { - ...result, + this.remember( taskId, - }); + signature( + args as MessageArgs, + event.continuation + ? { + kind: event.continuation, + instructionId: event.instructionId, + } + : undefined, + ), + { + ...result, + taskId, + }, + ); } else { this.unresolved.add(taskId); } @@ -83,7 +164,24 @@ export class FastAgentTaskMessageGuard { taskId: string, args: MessageArgs, deliver: () => Promise, + continuation?: Continuation, ): Promise { + const key = signature(args, continuation); + const receipt = this.receipts.get(taskId)?.get(key); + if ( + continuation?.kind === 'recovery' && + this.recoveryReservations.has(taskId) + ) { + if (receipt && !this.unresolved.has(taskId)) return receipt; + return { + success: false, + taskId, + delivery: 'not_accepted', + recovery: 'budget_exhausted', + error: + 'Recovery budget exhausted. A new explicit instruction is required.', + }; + } if (this.unresolved.has(taskId)) { return { success: false, @@ -92,9 +190,14 @@ export class FastAgentTaskMessageGuard { error: unknownGuidance, }; } - const key = signature(args); - const receipt = this.receipts.get(taskId)?.get(key); if (receipt) return receipt; + // Same-turn receipts from before continuation metadata was shipped remain + // replayable, but never override an already-consumed recovery budget. + const legacyReceipt = this.receipts.get(taskId)?.get(signature(args)); + if (legacyReceipt) return legacyReceipt; + const reservation = + continuation?.kind === 'recovery' ? Symbol() : undefined; + if (reservation) this.recoveryReservations.set(taskId, reservation); this.unresolved.add(taskId); let result: Result; try { @@ -108,9 +211,18 @@ export class FastAgentTaskMessageGuard { result = { ...result, taskId }; if (result.delivery === 'not_accepted') { this.unresolved.delete(taskId); - } else if (result.success === true) { + if ( + reservation && + this.recoveryReservations.get(taskId) === reservation + ) { + this.recoveryReservations.delete(taskId); + } + } else if (result.success === true && result.delivery !== 'unknown') { this.unresolved.delete(taskId); this.remember(taskId, key, result); + if (continuation?.kind === 'instruction') { + this.acceptInstruction(taskId, continuation.instructionId); + } } else { result = { ...result, delivery: 'unknown', guidance: unknownGuidance }; } diff --git a/packages/sdk/src/server/lib/task-runs/__tests__/notify-fast-agent-parent-on-settle.test.ts b/packages/sdk/src/server/lib/task-runs/__tests__/notify-fast-agent-parent-on-settle.test.ts index 30c1bee82..b3f43094f 100644 --- a/packages/sdk/src/server/lib/task-runs/__tests__/notify-fast-agent-parent-on-settle.test.ts +++ b/packages/sdk/src/server/lib/task-runs/__tests__/notify-fast-agent-parent-on-settle.test.ts @@ -189,6 +189,98 @@ describe('notifyFastAgentParentOnSettle', () => { ); }); + it.each([ + undefined, + TaskPayloadKind.GithubPrReview, + TaskPayloadKind.GithubPrReviewSync, + ])( + 'preserves redacted idle errors without retry controls for payload kind %s', + async (payloadKind) => { + mocks.canRetryFailedStart.mockResolvedValue(true); + await notifyFastAgentParentOnSettle( + makeRun( + { fastAgentParent: fastParent }, + { + payloadKind, + error: + ' Invalid credential xoxb-1234567890-abcdefghijklmnop while running. ', + errorCode: TaskRunErrorCode.DockerWorkerStartTimeout, + }, + ), + RunStatus.Idle, + ); + + expect(mocks.enqueueParentEvent).toHaveBeenCalledExactlyOnceWith({ + parent: fastParent, + event: { + type: 'task_settled', + taskId: 'child-task', + runId: 200, + status: RunStatus.Idle, + error: 'Invalid credential [redacted] while running.', + errorCode: TaskRunErrorCode.DockerWorkerStartTimeout, + taskUrl: 'https://roomote.example/task/child-task', + pullRequests: [], + }, + }); + expect(mocks.canRetryFailedStart).not.toHaveBeenCalled(); + }, + ); + + it.each([null, ' '])( + 'does not synthesize an idle error for %s', + async (error) => { + await notifyFastAgentParentOnSettle( + makeRun({ fastAgentParent: fastParent }, { error }), + RunStatus.Idle, + ); + + const notification = mocks.enqueueParentEvent.mock.calls[0]?.[0]; + expect(notification.event).not.toHaveProperty('error'); + expect(notification.event).not.toHaveProperty('errorCode'); + expect(notification).not.toHaveProperty('retryTaskStartRunId'); + expect(mocks.canRetryFailedStart).not.toHaveBeenCalled(); + }, + ); + + it('preserves an idle review error code without synthesizing an error', async () => { + await notifyFastAgentParentOnSettle( + makeRun( + { fastAgentParent: fastParent }, + { + payloadKind: TaskPayloadKind.GithubPrReview, + errorCode: TaskRunErrorCode.DockerWorkerStartTimeout, + }, + ), + RunStatus.Idle, + ); + + const notification = mocks.enqueueParentEvent.mock.calls[0]?.[0]; + expect(notification.event).toMatchObject({ + status: RunStatus.Idle, + errorCode: TaskRunErrorCode.DockerWorkerStartTimeout, + }); + expect(notification.event).not.toHaveProperty('error'); + expect(notification).not.toHaveProperty('retryTaskStartRunId'); + expect(mocks.canRetryFailedStart).not.toHaveBeenCalled(); + }); + + it('keeps canceled fallback diagnostics without retry controls', async () => { + await notifyFastAgentParentOnSettle( + makeRun({ fastAgentParent: fastParent }), + RunStatus.Canceled, + ); + + const notification = mocks.enqueueParentEvent.mock.calls[0]?.[0]; + expect(notification.event).toMatchObject({ + status: RunStatus.Canceled, + error: + 'The task stopped without a detailed error. Open the task for diagnostics.', + }); + expect(notification).not.toHaveProperty('retryTaskStartRunId'); + expect(mocks.canRetryFailedStart).not.toHaveBeenCalled(); + }); + it('carries custom automation identity into the settlement event', async () => { await notifyFastAgentParentOnSettle( makeRun({ diff --git a/packages/sdk/src/server/lib/task-runs/notify-fast-agent-parent-on-settle.ts b/packages/sdk/src/server/lib/task-runs/notify-fast-agent-parent-on-settle.ts index 2739c7757..b54d2d817 100644 --- a/packages/sdk/src/server/lib/task-runs/notify-fast-agent-parent-on-settle.ts +++ b/packages/sdk/src/server/lib/task-runs/notify-fast-agent-parent-on-settle.ts @@ -57,12 +57,15 @@ export async function notifyFastAgentParentOnSettle( return; } + const hasIdleError = + status === RunStatus.Idle && Boolean(run.error?.trim() || run.errorCode); + // A review child's outcome reaches the session through exactly one pipe, // the PR feedback relay built from its summary comment. That holds for // automatic reviews of a session-owned PR and for reviews the session // requested itself, so a successful settle never announces here; only - // failures do, because a failed review never posts a summary. - if (isPrReviewRun(run) && status !== RunStatus.Failed) { + // failures and idle runs with stored errors do, since they may lack a summary. + if (isPrReviewRun(run) && status !== RunStatus.Failed && !hasIdleError) { return; } @@ -123,7 +126,14 @@ export async function notifyFastAgentParentOnSettle( error: formatFastAgentTerminalError(run), ...(run.errorCode ? { errorCode: run.errorCode } : {}), } - : {}), + : hasIdleError + ? { + ...(run.error?.trim() + ? { error: formatFastAgentTerminalError(run) } + : {}), + ...(run.errorCode ? { errorCode: run.errorCode } : {}), + } + : {}), taskUrl: getTaskUrl({ taskId: run.taskId, utm: {