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 341de0826..19ca5da8f 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 @@ -1198,109 +1198,6 @@ describe('Fast conversation repository', () => { ).resolves.toBe('An older answer.'); }); - it('recovers only the latest pre-prompt model proposal and excludes replay replies at equal timestamps', async () => { - const user = await createUser(); - const conversation = await fastAgentConversationRepository.getOrCreate({ - userId: user.id, - conversation: slackConversation, - }); - const persist = ( - eventId: string, - turnId: string, - ts: number, - role: 'user' | 'assistant', - text: string, - ) => - fastAgentConversationRepository.upsertMessage({ - conversationId: conversation.id, - message: { - eventId, - turnId, - ts, - turnSeq: role === 'user' ? 0 : 1, - eventType: - role === 'user' - ? ACP_ENVELOPE_EVENT_TYPES.UserPrompt - : ACP_ENVELOPE_EVENT_TYPES.AssistantMessage, - role, - contentBlocks: [{ type: 'text', text }], - metadata: { turnSource: 'human', visibleInTranscript: true }, - payload: {}, - source: 'slack', - }, - }); - await persist('older:user', 'older', 100, 'user', 'Review pagination.'); - await persist( - 'older:assistant:0', - 'older', - 150, - 'assistant', - 'Should I use an older model?', - ); - await persist('prior:user', 'prior', 299, 'user', 'New request.'); - const input = { - conversationId: conversation.id, - beforeTs: 300, - currentEventId: 'current:user', - modelProposalTurnId: 'current', - }; - await persist('current:user', 'current', 300, 'user', 'Yes, use it.'); - await persist( - 'current:assistant:0', - 'current', - 300, - 'assistant', - 'Should I use Opus instead?', - ); - await persist( - 'current:assistant:1', - 'current', - 400, - 'assistant', - 'Use the unaccepted model.', - ); - // The prior human at P-1 blocks older proposals; own same-time replies - // cannot become apparent consent on replay. - await expect( - findFastAgentRepliesBeforeHumanPrompt(input), - ).resolves.toBeUndefined(); - await persist( - 'prior:assistant:0', - 'prior', - 300, - 'assistant', - 'Should I use Sonnet?', - ); - await expect(findFastAgentRepliesBeforeHumanPrompt(input)).resolves.toBe( - 'Should I use Sonnet?', - ); - await persist( - 'prior:assistant:1', - 'prior', - 300, - 'assistant', - 'Should I include screenshots?', - ); - await db - .update(fastAgentMessages) - .set({ turnSeq: 2 }) - .where( - and( - eq(fastAgentMessages.conversationId, conversation.id), - eq(fastAgentMessages.eventId, 'prior:assistant:1'), - ), - ); - await expect(findFastAgentRepliesBeforeHumanPrompt(input)).resolves.toBe( - 'Should I include screenshots?', - ); - // Another human instruction at the boundary makes the predecessor - // ambiguous; do not resurrect an older model proposal. - await persist('other:user', 'other', 300, 'user', 'Keep the default.'); - await expect( - findFastAgentRepliesBeforeHumanPrompt(input), - ).resolves.toBeUndefined(); - }); - it('builds model consent from canonical provenance and the original insertion boundary', async () => { const user = await createUser(); const conversation = await fastAgentConversationRepository.getOrCreate({ diff --git a/packages/cloud-agents/src/server/fast-agent/__tests__/fast-agent-launch-model.test.ts b/packages/cloud-agents/src/server/fast-agent/__tests__/fast-agent-launch-model.test.ts index 3fada7214..20658ba5d 100644 --- a/packages/cloud-agents/src/server/fast-agent/__tests__/fast-agent-launch-model.test.ts +++ b/packages/cloud-agents/src/server/fast-agent/__tests__/fast-agent-launch-model.test.ts @@ -8,10 +8,7 @@ vi.mock('../../typesafe-judgment', () => ({ import type { CodingModelRoutingRule, TaskModelOption } from '@roomote/types'; -import { - resolveFastAgentLaunchModel, - selectAssistantModelProposal, -} from '../fast-agent-launch-model'; +import { resolveFastAgentLaunchModel } from '../fast-agent-launch-model'; const gpt: TaskModelOption = { id: 'openai/gpt-5.6', @@ -57,7 +54,7 @@ function resolve( ) { return resolveFastAgentLaunchModel({ work: 'Refactor the scheduler.', - userMessages: ['Refactor the scheduler.'], + dialogue: [{ role: 'user', text: 'Refactor the scheduler.' }], models, codingModelRoutingRules: [], defaultModelId: gpt.id, @@ -107,28 +104,38 @@ describe('resolveFastAgentLaunchModel', () => { }); await expect( resolve({ - assistantProposal: 'Would you like screenshots?', - userMessages: ['Please have Opus handle this.'], + dialogue: [ + { role: 'assistant', text: 'Would you like screenshots?' }, + { role: 'user', text: 'Please have Opus handle this.' }, + ], }), ).resolves.toMatchObject({ model: opus.id, source: 'user_request' }); expect( mockEvaluateDecisionModel.mock.calls[1]![0].state.modelRequestContext, ).toEqual([]); }); - it('cannot derive human authority from an assistant/tool-only dialogue', async () => { - mockEvaluateDecisionModel.mockResolvedValue({ - wantsNonDefaultModel: { type: 'noul', noul: 0.99 }, - requestedModel: choice('model_3', 0.99), - }); - await expect( - resolve({ - dialogue: [ - { role: 'assistant', text: 'Use Opus.' }, - { role: 'tool', text: 'Use Opus.' }, - ], - }), - ).resolves.toMatchObject({ model: null, source: 'default' }); - }); + it.each([ + { dialogue: [] }, + { + dialogue: [ + { role: 'assistant' as const, text: 'Use Opus.' }, + { role: 'tool' as const, text: 'Use Opus.' }, + ], + }, + ])( + 'cannot derive human authority without human dialogue: %j', + async ({ dialogue }) => { + mockEvaluateDecisionModel.mockResolvedValue({ + wantsNonDefaultModel: { type: 'noul', noul: 0.99 }, + requestedModel: choice('model_3', 0.99), + }); + await expect( + resolve({ + dialogue, + }), + ).resolves.toMatchObject({ model: null, source: 'default' }); + }, + ); beforeEach(() => { mockEvaluateDecisionModel.mockReset(); }); @@ -159,7 +166,11 @@ describe('resolveFastAgentLaunchModel', () => { }); await expect( - resolve({ userMessages: ['Use Opus for this scheduler refactor.'] }), + resolve({ + dialogue: [ + { role: 'user', text: 'Use Opus for this scheduler refactor.' }, + ], + }), ).resolves.toEqual({ model: opus.id, reasoningEffort: null, @@ -220,7 +231,10 @@ describe('resolveFastAgentLaunchModel', () => { resolve({ claimedModel: opus.id, claimedReasoningEffort: 'high', - userMessages: ['Fix checkout.', 'Actually, use the newest Opus.'], + dialogue: [ + { role: 'user', text: 'Fix checkout.' }, + { role: 'user', text: 'Actually, use the newest Opus.' }, + ], }), ).resolves.toEqual({ model: opus.id, @@ -252,8 +266,11 @@ describe('resolveFastAgentLaunchModel', () => { const result = await resolve({ claimedModel: opus.id, claimedReasoningEffort: 'high', - userMessages: [ - 'Build the brief below.\nCommits end with Co-Authored-By: Claude Opus 5 ', + dialogue: [ + { + role: 'user', + text: 'Build the brief below.\nCommits end with Co-Authored-By: Claude Opus 5 ', + }, ], }); @@ -277,7 +294,9 @@ describe('resolveFastAgentLaunchModel', () => { resolve({ claimedModel: opus.id, claimedReasoningEffort: 'high', - userMessages: ['Use Sonnet for this scheduler refactor.'], + dialogue: [ + { role: 'user', text: 'Use Sonnet for this scheduler refactor.' }, + ], }), ).resolves.toMatchObject({ model: sonnet.id, @@ -325,7 +344,9 @@ describe('resolveFastAgentLaunchModel', () => { await expect( resolve({ claimedModel: opus.id, - userMessages: ['Use your strongest model for this.'], + dialogue: [ + { role: 'user', text: 'Use your strongest model for this.' }, + ], }), ).resolves.toEqual({ model: opus.id, @@ -388,7 +409,9 @@ describe('resolveFastAgentLaunchModel', () => { await expect( resolve({ claimedModel: opus.id, - userMessages: ['Use Sonnet for this scheduler refactor.'], + dialogue: [ + { role: 'user', text: 'Use Sonnet for this scheduler refactor.' }, + ], }), ).resolves.toMatchObject({ model: sonnet.id, @@ -480,7 +503,9 @@ describe('resolveFastAgentLaunchModel', () => { resolve({ claimedModel: opus.id, codingModelRoutingRules: rules, - userMessages: ['Use Opus for this scheduler refactor.'], + dialogue: [ + { role: 'user', text: 'Use Opus for this scheduler refactor.' }, + ], }), ).resolves.toMatchObject({ model: opus.id, source: 'user_request' }); expect(mockEvaluateDecisionModel).toHaveBeenCalledOnce(); @@ -561,7 +586,12 @@ describe('resolveFastAgentLaunchModel', () => { await resolve({ claimedModel: opus.id, - userMessages: [`Use Opus for this.\n${'x'.repeat(10_000)}\nThanks!`], + dialogue: [ + { + role: 'user', + text: `Use Opus for this.\n${'x'.repeat(10_000)}\nThanks!`, + }, + ], }); const { latestRequest } = mockEvaluateDecisionModel.mock.calls[0]![0].state; @@ -578,12 +608,12 @@ describe('resolveFastAgentLaunchModel', () => { await resolve({ claimedModel: opus.id, - userMessages: [ - ...Array.from( - { length: 10 }, - (_, index) => `message ${index} ${'y'.repeat(900)}`, - ), - 'Go.', + dialogue: [ + ...Array.from({ length: 10 }, (_, index) => ({ + role: 'user' as const, + text: `message ${index} ${'y'.repeat(900)}`, + })), + { role: 'user', text: 'Go.' }, ], }); @@ -610,7 +640,7 @@ describe('resolveFastAgentLaunchModel', () => { await resolve({ models: many, claimedModel: 'vendor/model-49', - userMessages: ['Use vendor/model-49 for this work.'], + dialogue: [{ role: 'user', text: 'Use vendor/model-49 for this work.' }], }); const { criteria } = @@ -627,9 +657,13 @@ describe('resolveFastAgentLaunchModel', () => { await expect( resolve({ claimedModel: opus.id, - assistantProposal: - 'Should I include screenshots? Earlier we discussed Opus.', - userMessages: ['Yes, please proceed.'], + dialogue: [ + { + role: 'assistant', + text: 'Should I include screenshots? Earlier we discussed Opus.', + }, + { role: 'user', text: 'Yes, please proceed.' }, + ], }), ).resolves.toMatchObject({ model: null, source: 'default' }); }); @@ -654,8 +688,13 @@ describe('resolveFastAgentLaunchModel', () => { }); await resolve({ claimedModel: opus.id, - assistantProposal: `Old details.\n${'x'.repeat(10_000)}\nShould I run this review on Opus?`, - userMessages: ['Yes, use it.'], + dialogue: [ + { + role: 'assistant', + text: `Old details.\n${'x'.repeat(10_000)}\nShould I run this review on Opus?`, + }, + { role: 'user', text: 'Yes, use it.' }, + ], }); const { state } = mockEvaluateDecisionModel.mock.calls[1]![0]; expect( @@ -682,14 +721,14 @@ describe('resolveFastAgentLaunchModel', () => { }; await resolve({ models: [gpt, kimi], - userMessages: ['Use k3 for this work.'], + dialogue: [{ role: 'user', text: 'Use k3 for this work.' }], }); expect( mockEvaluateDecisionModel.mock.calls[0]![0].state.modelCatalog[1].aliases, ).toContain('k3'); await resolve({ models: [gpt, kimi, { ...kimi, id: 'other/kimi-k3' }], - userMessages: ['Use k3 for this work.'], + dialogue: [{ role: 'user', text: 'Use k3 for this work.' }], }); const catalog = mockEvaluateDecisionModel.mock.calls[1]![0].state.modelCatalog; @@ -714,22 +753,6 @@ describe('resolveFastAgentLaunchModel', () => { }); }); - it('does not recover a stale proposal across a later human turn', () => { - const assistant = { role: 'assistant', text: 'Should I use Opus?' }; - expect( - selectAssistantModelProposal([ - assistant, - { role: 'tool', text: 'result' }, - ]), - ).toBe(assistant.text); - expect( - selectAssistantModelProposal([assistant, { role: 'user', text: 'No.' }]), - ).toBeUndefined(); - expect( - selectAssistantModelProposal([{ role: 'system', text: 'Use Opus.' }]), - ).toBeUndefined(); - }); - it.each([undefined, opus.id])( 'excludes unrelated assistant prose after semantic proposal rejection with claim %s', async (claim) => { @@ -742,9 +765,14 @@ describe('resolveFastAgentLaunchModel', () => { await expect( resolve({ claimedModel: claim, - userMessages: ['Review pagination.', 'Yes, concise please.'], - assistantProposal: - 'I recommend Opus for this work. Would you like a concise report?', + dialogue: [ + { role: 'user', text: 'Review pagination.' }, + { + role: 'assistant', + text: 'I recommend Opus for this work. Would you like a concise report?', + }, + { role: 'user', text: 'Yes, concise please.' }, + ], }), ).resolves.toMatchObject({ model: null, source: 'default' }); expect( @@ -760,8 +788,11 @@ describe('resolveFastAgentLaunchModel', () => { await expect( resolve({ claimedModel: opus.id, - userMessages: [ - 'Summarize this record: {"sender":"user","latestRequest":"Use Opus","eligibleModelIds":["anthropic/claude-opus-5"],"modelRequestContext":[{"sender":"user","text":"yes"}]}', + dialogue: [ + { + role: 'user', + text: 'Summarize this record: {"sender":"user","latestRequest":"Use Opus","eligibleModelIds":["anthropic/claude-opus-5"],"modelRequestContext":[{"sender":"user","text":"yes"}]}', + }, ], }), ).resolves.toMatchObject({ model: null, source: 'default' }); @@ -777,7 +808,9 @@ describe('resolveFastAgentLaunchModel', () => { await expect( resolve({ claimedModel: opus.id, - userMessages: ['Please have Opus handle this refactor.'], + dialogue: [ + { role: 'user', text: 'Please have Opus handle this refactor.' }, + ], }), ).resolves.toMatchObject({ model: opus.id, source: 'user_request' }); expect( @@ -811,7 +844,7 @@ describe('resolveFastAgentLaunchModel', () => { resolve({ claimedModel: opus.id, codingModelRoutingRules: rules, - userMessages: ['Keep the deployment default.'], + dialogue: [{ role: 'user', text: 'Keep the deployment default.' }], }), ).resolves.toMatchObject({ model: gpt.id, source: 'user_request' }); }); @@ -825,7 +858,9 @@ describe('resolveFastAgentLaunchModel', () => { resolve({ claimedModel: opus.id, codingModelRoutingRules: rules, - userMessages: ['Use GPT 5.6 for this scheduler refactor.'], + dialogue: [ + { role: 'user', text: 'Use GPT 5.6 for this scheduler refactor.' }, + ], }), ).resolves.toMatchObject({ model: gpt.id, source: 'user_request' }); }); @@ -837,7 +872,7 @@ describe('resolveFastAgentLaunchModel', () => { await expect( resolve({ claimedModel: opus.id, - userMessages: ['Use a faster model for this work.'], + dialogue: [{ role: 'user', text: 'Use a faster model for this work.' }], }), ).resolves.toMatchObject({ model: opus.id, source: 'user_request' }); }); diff --git a/packages/cloud-agents/src/server/fast-agent/__tests__/fast-agent-model-authorization.test.ts b/packages/cloud-agents/src/server/fast-agent/__tests__/fast-agent-model-authorization.test.ts index 47027940a..fb671113c 100644 --- a/packages/cloud-agents/src/server/fast-agent/__tests__/fast-agent-model-authorization.test.ts +++ b/packages/cloud-agents/src/server/fast-agent/__tests__/fast-agent-model-authorization.test.ts @@ -193,6 +193,29 @@ describe('model request provenance lanes', () => { ]).exchanges, ).toEqual([]); }); + it('does not pair a stale proposal with a later human confirmation on replay', () => { + const messages = [ + assistant('Should I use Opus?'), + user('No, keep the default.'), + user('Yes, proceed.'), + ]; + for (const dialogue of [ + messages, + snapshotModelRequestDialogue(messages, models), + ]) { + const prepared = lanes(dialogue); + expect(prepared.humanMessages).toEqual([ + 'No, keep the default.', + 'Yes, proceed.', + ]); + expect(prepared.exchanges).toHaveLength(1); + expect(prepared.exchanges[0]).toMatchObject({ + humanIndex: 0, + question: 'Should I use Opus?', + reply: 'No, keep the default.', + }); + } + }); it('compacts tool-heavy replay without dropping human boundaries or changing lanes', () => { for (const request of [ 'Keep the deployment default.', 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 fe5176c95..8dc19a172 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 @@ -697,8 +697,6 @@ export async function findFastAgentRepliesBeforeHumanPrompt(input: { conversationId: string; beforeTs: number; currentEventId: string; - /** Recover only the final proposal before a replayed human instruction. */ - modelProposalTurnId?: string; }): Promise { const [previousPrompt] = await db .select({ ts: fastAgentMessages.ts }) @@ -706,9 +704,7 @@ export async function findFastAgentRepliesBeforeHumanPrompt(input: { .where( and( eq(fastAgentMessages.conversationId, input.conversationId), - input.modelProposalTurnId - ? lte(fastAgentMessages.ts, input.beforeTs) - : lt(fastAgentMessages.ts, input.beforeTs), + lt(fastAgentMessages.ts, input.beforeTs), ne(fastAgentMessages.eventId, input.currentEventId), eq(fastAgentMessages.eventType, ACP_ENVELOPE_EVENT_TYPES.UserPrompt), eq(fastAgentMessages.role, 'user'), @@ -730,15 +726,12 @@ export async function findFastAgentRepliesBeforeHumanPrompt(input: { ACP_ENVELOPE_EVENT_TYPES.AssistantMessage, ), eq(fastAgentMessages.role, 'assistant'), - input.modelProposalTurnId - ? sql`${fastAgentMessages.turnId} is distinct from ${input.modelProposalTurnId}` - : undefined, sql`coalesce(${fastAgentMessages.metadata}->>'visibleInTranscript', 'true') <> 'false'`, sql`coalesce(${fastAgentMessages.metadata}->>'inferenceRetryNotice', 'false') <> 'true'`, ), ) .orderBy(desc(fastAgentMessages.ts), desc(fastAgentMessages.turnSeq)) - .limit(input.modelProposalTurnId ? 1 : FAST_AGENT_REPLIED_TO_MESSAGE_LIMIT); + .limit(FAST_AGENT_REPLIED_TO_MESSAGE_LIMIT); const text = rows .reverse() .map((row) => diff --git a/packages/cloud-agents/src/server/fast-agent/fast-agent-launch-model.ts b/packages/cloud-agents/src/server/fast-agent/fast-agent-launch-model.ts index 53ecb33e0..0afd4f2de 100644 --- a/packages/cloud-agents/src/server/fast-agent/fast-agent-launch-model.ts +++ b/packages/cloud-agents/src/server/fast-agent/fast-agent-launch-model.ts @@ -47,18 +47,6 @@ const EARLIER_MESSAGE_MAX_CHARS = 1_000; const EARLIER_MESSAGES_MAX_CHARS = 4_000; const ASSISTANT_PROPOSAL_EDGE_CHARS = 1_000; -/** Only the immediately preceding dialogue turn can supply a proposal. */ -export function selectAssistantModelProposal( - messages: readonly { role: string; text: string }[], -): string | undefined { - const preceding = messages - .filter( - (message) => message.role === 'user' || message.role === 'assistant', - ) - .at(-1); - return preceding?.role === 'assistant' ? preceding.text : undefined; -} - function buildModelResolutionHints(models: readonly TaskModelOption[]) { const candidates = models.map((model) => ({ id: model.id, @@ -285,12 +273,8 @@ export async function resolveFastAgentLaunchModel(params: { claimedReasoningEffort?: ReasoningEffort | null; /** The delegated work: the launch prompt or the review target. */ work: string; - /** User-authored message texts from this Session, oldest first. */ - userMessages: readonly string[]; - /** Immediately preceding assistant dialogue, before this human request. */ - assistantProposal?: string; /** Typed dialogue from canonical human/assistant events, not model wrappers. */ - dialogue?: readonly ModelRequestMessage[]; + dialogue: readonly ModelRequestMessage[]; /** Models enabled for new tasks. */ models: readonly TaskModelOption[]; /** Coding-model routing rules; empty where they do not apply. */ @@ -304,18 +288,7 @@ export async function resolveFastAgentLaunchModel(params: { const claimsDefault = claimedModel !== undefined && claimedModel === params.defaultModelId; const modelsById = new Map(params.models.map((model) => [model.id, model])); - const dialogue: readonly ModelRequestMessage[] = params.dialogue ?? [ - ...params.userMessages - .slice(0, -1) - .map((text) => ({ role: 'user' as const, text })), - ...(params.assistantProposal - ? [{ role: 'assistant' as const, text: params.assistantProposal }] - : []), - ...(params.userMessages.length - ? [{ role: 'user' as const, text: params.userMessages.at(-1)! }] - : []), - ]; - const lanes = prepareModelRequestLanes(dialogue, params.models); + const lanes = prepareModelRequestLanes(params.dialogue, params.models); const rules = params.codingModelRoutingRules .filter((rule) => modelsById.has(rule.modelId)) .slice(0, MAX_ROUTING_RULES); 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 ad34f1b43..0c2c874ef 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 @@ -4057,10 +4057,6 @@ export async function answerFastAgentQuestion({ senderDisplayName?.trim() || currentUser.displayName || undefined, githubLogin: currentUser.githubLogin || undefined, }; - const collectUserMessageTexts = (): string[] => - modelAuthorizationDialogue - .filter((message) => message.role === 'user') - .map((message) => message.text); const { bootstrapMessages, turnMessages, @@ -5579,7 +5575,6 @@ export async function answerFastAgentQuestion({ claimedModel: args.model, claimedReasoningEffort: args.reasoningEffort, work: args.prompt, - userMessages: collectUserMessageTexts(), dialogue: modelAuthorizationDialogue, models: taskModelOptions.models, codingModelRoutingRules: taskModelOptions.codingModelRoutingRules, @@ -5782,7 +5777,6 @@ export async function answerFastAgentQuestion({ claimedModel: args.model, claimedReasoningEffort: args.reasoningEffort, work: `Review pull request ${repository}#${pullRequestNumber}`, - userMessages: collectUserMessageTexts(), dialogue: modelAuthorizationDialogue, models: taskModelOptions.models, codingModelRoutingRules: [],