diff --git a/apps/worker/src/callbacks/__tests__/linear-agent.test.ts b/apps/worker/src/callbacks/__tests__/linear-agent.test.ts index b90eb87a9..64a04fbde 100644 --- a/apps/worker/src/callbacks/__tests__/linear-agent.test.ts +++ b/apps/worker/src/callbacks/__tests__/linear-agent.test.ts @@ -216,6 +216,10 @@ describe('linearAgentCallbacks', () => { context, ); + // Streaming placeholders are skipped for integration elicitations. + expect(setPendingLinearRequestUserInputMock).not.toHaveBeenCalled(); + expect(emitElicitationMock).not.toHaveBeenCalled(); + await linearAgentCallbacks.onMessage?.( taskRun, 'task_123', @@ -234,7 +238,7 @@ describe('linearAgentCallbacks', () => { context, ); - expect(setPendingLinearRequestUserInputMock).toHaveBeenCalledTimes(2); + expect(setPendingLinearRequestUserInputMock).toHaveBeenCalledTimes(1); expect(setPendingLinearRequestUserInputMock).toHaveBeenLastCalledWith({ runId: 123, sessionId: 'session_123', @@ -242,7 +246,7 @@ describe('linearAgentCallbacks', () => { taskId: 'task_123', questions: [richQuestion], }); - expect(emitElicitationMock).toHaveBeenCalledTimes(2); + expect(emitElicitationMock).toHaveBeenCalledTimes(1); expect(emitElicitationMock).toHaveBeenLastCalledWith( 'session_123', expect.stringContaining('Which language should I use?'), diff --git a/apps/worker/src/callbacks/__tests__/request-user-input.test.ts b/apps/worker/src/callbacks/__tests__/request-user-input.test.ts index d918fcdf2..43462a7fc 100644 --- a/apps/worker/src/callbacks/__tests__/request-user-input.test.ts +++ b/apps/worker/src/callbacks/__tests__/request-user-input.test.ts @@ -1,8 +1,10 @@ import type { TaskRun } from '@roomote/sdk/client'; import { + STREAMING_PLACEHOLDER_QUESTION_TEXT, buildRequestUserInputTaskUrl, formatRequestUserInputPrompt, + isStreamingPlaceholderRequestUserInput, } from '../request-user-input'; describe('formatRequestUserInputPrompt', () => { @@ -213,3 +215,52 @@ describe('buildRequestUserInputTaskUrl', () => { ); }); }); + +describe('isStreamingPlaceholderRequestUserInput', () => { + it('detects the harness streaming placeholder', () => { + expect( + isStreamingPlaceholderRequestUserInput({ + questions: [ + { + id: 'response', + header: 'Response', + question: STREAMING_PLACEHOLDER_QUESTION_TEXT, + isOther: true, + isSecret: false, + }, + ], + }), + ).toBe(true); + }); + + it('rejects real questions with options or custom wording', () => { + expect( + isStreamingPlaceholderRequestUserInput({ + questions: [ + { + id: 'language', + header: 'Language', + question: 'Which language should I use?', + isOther: true, + isSecret: false, + options: [{ label: 'TypeScript', description: 'App stack' }], + }, + ], + }), + ).toBe(false); + + expect( + isStreamingPlaceholderRequestUserInput({ + questions: [ + { + id: 'response', + header: 'Response', + question: 'Anything else?', + isOther: true, + isSecret: false, + }, + ], + }), + ).toBe(false); + }); +}); diff --git a/apps/worker/src/callbacks/__tests__/slack-mention.test.ts b/apps/worker/src/callbacks/__tests__/slack-mention.test.ts index 41d75d7b4..d467f55a3 100644 --- a/apps/worker/src/callbacks/__tests__/slack-mention.test.ts +++ b/apps/worker/src/callbacks/__tests__/slack-mention.test.ts @@ -15,6 +15,7 @@ const { mockSlackInstallationsFindFirst, mockSupportsIntegrationRequestUserInput, mockUpdateMessage, + mockDeleteMessage, } = vi.hoisted(() => ({ mockBuildSlackRequestUserInputBlocks: vi.fn(() => [ { @@ -64,6 +65,7 @@ const { .mockResolvedValue({ botAccessToken: 'xoxb-test' }), mockSupportsIntegrationRequestUserInput: vi.fn().mockReturnValue(true), mockUpdateMessage: vi.fn().mockResolvedValue(undefined), + mockDeleteMessage: vi.fn().mockResolvedValue(true), })); vi.mock('@roomote/sdk/client', () => ({ @@ -92,6 +94,7 @@ vi.mock('@roomote/slack/client', () => ({ fetchThreadMessages = mockFetchThreadMessages; removeReaction = mockRemoveReaction; updateMessage = mockUpdateMessage; + deleteMessage = mockDeleteMessage; postMessage = mockPostMessage; }, buildSlackRequestUserInputBlocks: mockBuildSlackRequestUserInputBlocks, @@ -99,18 +102,15 @@ vi.mock('@roomote/slack/client', () => ({ convertMarkdownToSlack: vi.fn((text: string) => text), })); -vi.mock('../request-user-input', () => ({ - buildRequestUserInputTaskUrl: mockBuildRequestUserInputTaskUrl, - getRequestUserInputPromptSignature: (request: { - requestId: string; - questions: unknown[]; - }) => - JSON.stringify({ - requestId: request.requestId, - questions: request.questions, - }), - supportsIntegrationRequestUserInput: mockSupportsIntegrationRequestUserInput, -})); +vi.mock('../request-user-input', async (importOriginal) => { + const actual = await importOriginal(); + return { + ...actual, + buildRequestUserInputTaskUrl: mockBuildRequestUserInputTaskUrl, + supportsIntegrationRequestUserInput: + mockSupportsIntegrationRequestUserInput, + }; +}); import { RunStatus, TaskPayloadKind } from '@roomote/types'; import { type TaskRun, sdk } from '@roomote/sdk/client'; @@ -573,6 +573,10 @@ describe('slackMentionCallbacks', () => { context, ); + // Streaming placeholders must not post Slack buttons or pending state. + expect(mockPostMessage).not.toHaveBeenCalled(); + expect(sdk.taskRuns.setPendingSlackRequestUserInput).not.toHaveBeenCalled(); + await slackMentionCallbacks.onMessage?.( taskRun, 'task_123', @@ -592,25 +596,162 @@ describe('slackMentionCallbacks', () => { ); expect(mockPostMessage).toHaveBeenCalledTimes(1); + expect(mockUpdateMessage).not.toHaveBeenCalled(); + expect(mockBuildSlackRequestUserInputBlocks).toHaveBeenCalledWith( + expect.objectContaining({ + requestId, + questions: [richQuestion], + }), + ); + expect( + sdk.taskRuns.setPendingSlackRequestUserInput, + ).toHaveBeenLastCalledWith({ + runId: 123, + threadId: '1710000000.123', + requestId, + taskId: 'task_row_123', + questions: [richQuestion], + promptMessageTs: 'posted-ts', + }); + }); + + it('skips Slack buttons for streaming placeholder request_user_input prompts', async () => { + const taskRun = createTaskRun(); + const context = {}; + + await slackMentionCallbacks.onMessage?.( + taskRun, + 'task_123', + { + type: 'request_user_input', + request: { + requestId: 'rui:session:turn:call', + sessionId: 'session_1', + turnId: 'turn_1', + callId: 'call_1', + questions: [ + { + id: 'response', + header: 'Response', + question: 'Provide the requested input.', + isOther: true, + isSecret: false, + }, + ], + status: 'pending', + }, + ts: 1000, + }, + context, + ); + + expect(mockPostMessage).not.toHaveBeenCalled(); + expect(mockUpdateMessage).not.toHaveBeenCalled(); + expect(sdk.taskRuns.setPendingSlackRequestUserInput).not.toHaveBeenCalled(); + }); + + it('neutralizes the previous Slack card when enriching via a new post after update fails', async () => { + const taskRun = createTaskRun(); + const context = {}; + const requestId = 'rui:session:turn:call'; + mockPostMessage + .mockReset() + .mockResolvedValueOnce('first-ts') + .mockResolvedValueOnce('second-ts'); + mockUpdateMessage.mockReset().mockResolvedValue(false); + + const firstQuestion = { + id: 'language', + header: 'Language', + question: 'Which language should I use?', + isOther: true, + isSecret: false, + options: [ + { + label: 'TypeScript', + description: 'Use the existing app stack.', + }, + ], + }; + const secondQuestion = { + id: 'language-v2', + header: 'Language', + question: 'Which language should I use?', + isOther: true, + isSecret: false, + options: [ + { + label: 'TypeScript', + description: 'Use the existing app stack.', + }, + { + label: 'Rust', + description: 'Use the OpenCode runtime.', + }, + ], + }; + + await slackMentionCallbacks.onMessage?.( + taskRun, + 'task_123', + { + type: 'request_user_input', + request: { + requestId, + sessionId: 'session_1', + turnId: 'turn_1', + callId: 'call_1', + questions: [firstQuestion], + status: 'pending', + }, + ts: 1000, + }, + context, + ); + + await slackMentionCallbacks.onMessage?.( + taskRun, + 'task_123', + { + type: 'request_user_input', + request: { + requestId, + sessionId: 'session_1', + turnId: 'turn_1', + callId: 'call_1', + questions: [secondQuestion], + status: 'pending', + }, + ts: 1001, + }, + context, + ); + + expect(mockPostMessage).toHaveBeenCalledTimes(2); + // Failed in-place update attempt, then neutralize superseded card. expect(mockUpdateMessage).toHaveBeenCalledWith({ channel: 'C123', - ts: 'posted-ts', + ts: 'first-ts', + message: { + blocks: expect.any(Array), + }, + }); + expect(mockUpdateMessage).toHaveBeenCalledWith({ + channel: 'C123', + ts: 'first-ts', message: { blocks: [ { type: 'markdown', - text: 'native request_user_input blocks', + text: '_This prompt was updated. Please use the newest question in the thread._', }, ], }, }); - expect(mockBuildSlackRequestUserInputBlocks).toHaveBeenNthCalledWith( - 2, - expect.objectContaining({ - requestId, - questions: [richQuestion], - }), - ); + expect(mockDeleteMessage).toHaveBeenCalledWith({ + channel: 'C123', + ts: 'first-ts', + }); expect( sdk.taskRuns.setPendingSlackRequestUserInput, ).toHaveBeenLastCalledWith({ @@ -618,8 +759,8 @@ describe('slackMentionCallbacks', () => { threadId: '1710000000.123', requestId, taskId: 'task_row_123', - questions: [richQuestion], - promptMessageTs: 'posted-ts', + questions: [secondQuestion], + promptMessageTs: 'second-ts', }); }); diff --git a/apps/worker/src/callbacks/linear-agent.ts b/apps/worker/src/callbacks/linear-agent.ts index 799d60c94..6bebe4664 100644 --- a/apps/worker/src/callbacks/linear-agent.ts +++ b/apps/worker/src/callbacks/linear-agent.ts @@ -20,6 +20,7 @@ import { formatRequestUserInputPrompt, getRequestUserInputPromptSignature, isSingleQuestionRequestUserInput, + isStreamingPlaceholderRequestUserInput, supportsIntegrationRequestUserInput, } from './request-user-input'; @@ -265,6 +266,10 @@ export const linearAgentCallbacks: RunTaskCallbacks = { return; } + if (isStreamingPlaceholderRequestUserInput(event.request)) { + return; + } + try { if (!supportsIntegrationRequestUserInput(event.request)) { const taskUrl = buildRequestUserInputTaskUrl(taskRun, 'linear'); diff --git a/apps/worker/src/callbacks/request-user-input.ts b/apps/worker/src/callbacks/request-user-input.ts index 4b67dbbab..695b2035b 100644 --- a/apps/worker/src/callbacks/request-user-input.ts +++ b/apps/worker/src/callbacks/request-user-input.ts @@ -71,6 +71,26 @@ export function getRequestUserInputPromptSignature( }); } +export const STREAMING_PLACEHOLDER_QUESTION_TEXT = + 'Provide the requested input.'; + +export function isStreamingPlaceholderRequestUserInput( + request: Pick, +): boolean { + if (request.questions.length !== 1) { + return false; + } + + const question = request.questions[0]!; + const hasNoOptions = !question.options || question.options.length === 0; + + return ( + hasNoOptions && + question.id === 'response' && + question.question === STREAMING_PLACEHOLDER_QUESTION_TEXT + ); +} + export function isSingleQuestionRequestUserInput( request: AcpRequestUserInputPayload, ): boolean { diff --git a/apps/worker/src/callbacks/slack-mention.ts b/apps/worker/src/callbacks/slack-mention.ts index 506198fd3..d3f6545c5 100644 --- a/apps/worker/src/callbacks/slack-mention.ts +++ b/apps/worker/src/callbacks/slack-mention.ts @@ -24,6 +24,7 @@ import { captureWorkerException } from '../monitoring/sentry'; import { buildRequestUserInputTaskUrl, getRequestUserInputPromptSignature, + isStreamingPlaceholderRequestUserInput, supportsIntegrationRequestUserInput, } from './request-user-input'; @@ -302,6 +303,10 @@ async function handleRequestUserInput( return; } + if (isStreamingPlaceholderRequestUserInput(event.request)) { + return; + } + try { const slack = await getSlackNotifier(); const { channel, thread_ts: threadTs } = getSlackConversation(taskRun); @@ -349,6 +354,33 @@ async function handleRequestUserInput( blocks: promptBlocks, }); + if ( + !didUpdatePrompt && + existingPromptMessageTs && + promptMessageTs && + promptMessageTs !== existingPromptMessageTs + ) { + const neutralized = await slack.updateMessage({ + channel, + ts: existingPromptMessageTs, + message: { + blocks: [ + { + type: 'markdown', + text: '_This prompt was updated. Please use the newest question in the thread._', + }, + ], + }, + }); + + if (!neutralized) { + await slack.deleteMessage({ + channel, + ts: existingPromptMessageTs, + }); + } + } + if (!promptMessageTs) { await sdk.taskRuns.clearPendingSlackRequestUserInput({ runId: taskRun.id, diff --git a/packages/slack/src/__tests__/handle-followup-answer.test.ts b/packages/slack/src/__tests__/handle-followup-answer.test.ts index 0878aa1bb..3ab83b742 100644 --- a/packages/slack/src/__tests__/handle-followup-answer.test.ts +++ b/packages/slack/src/__tests__/handle-followup-answer.test.ts @@ -561,6 +561,70 @@ describe('handleFollowupAnswer', () => { expect(consoleErrorMock).not.toHaveBeenCalled(); }); + it('accepts a Pick answer with a stale question id when the option still matches the current prompt', async () => { + findActiveSlackTaskRunMock.mockResolvedValue({ + id: 42, + machineId: null, + taskId: 'task-1', + }); + selectLimitMock + .mockResolvedValueOnce([{ userId: 'user-1' }]) + .mockResolvedValueOnce([{ botAccessToken: 'xoxb-test' }]); + getPendingSlackRequestUserInputMock.mockResolvedValue({ + requestId: 'rui:session:turn:call', + runId: 42, + taskId: 'task-1', + questions: [ + { + id: 'language-v2', + header: 'Language', + question: 'Which language should I use?', + isOther: true, + isSecret: false, + options: [ + { + label: 'TypeScript', + description: 'Use the app stack.', + }, + ], + }, + ], + currentQuestionIndex: 0, + answers: {}, + status: 'pending', + createdAt: 123, + promptMessageTs: 'prompt-ts', + }); + + await handleFollowupAnswer( + buildPayload( + JSON.stringify({ + requestId: 'rui:session:turn:call', + questionId: 'language', + questionIndex: 0, + answer: 'TypeScript', + }), + ), + ); + + expect(submitPendingSlackRequestUserInputAnswerMock).toHaveBeenCalledWith( + '111.222', + expect.objectContaining({ + requestId: 'rui:session:turn:call', + }), + expect.objectContaining({ + answers: { + 'language-v2': { + answers: ['TypeScript'], + }, + }, + user: 'U123', + userId: 'user-1', + }), + ); + expect(consoleErrorMock).not.toHaveBeenCalled(); + }); + it('shows already-received copy when a structured final answer loses the atomic claim', async () => { findActiveSlackTaskRunMock.mockResolvedValue({ id: 42, diff --git a/packages/slack/src/handle-followup-answer.ts b/packages/slack/src/handle-followup-answer.ts index b5ec2a214..6432120b3 100644 --- a/packages/slack/src/handle-followup-answer.ts +++ b/packages/slack/src/handle-followup-answer.ts @@ -311,14 +311,28 @@ export async function handleFollowupAnswer(payload: SlackInteractivePayload) { return; } - if (structuredAnswer.questionId !== currentQuestion.question.id) { - throw new Error( - 'This prompt is out of date. Please answer the newest question in the thread.', + let answerQuestionId = structuredAnswer.questionId!; + if (answerQuestionId !== currentQuestion.question.id) { + const answerText = structuredAnswer.answer ?? ''; + const options = currentQuestion.question.options ?? []; + const matchesOption = options.some( + (option) => option.label === answerText, ); + const allowsCustomAnswer = + options.length === 0 || currentQuestion.question.isOther === true; + const freeformOk = allowsCustomAnswer && answerText.trim().length > 0; + + if (!matchesOption && !freeformOk) { + throw new Error( + 'This prompt is out of date. Please answer the newest question in the thread.', + ); + } + + answerQuestionId = currentQuestion.question.id; } const nextAnswers = mergeRequestUserInputAnswers(pendingRequest.answers, { - [structuredAnswer.questionId]: { + [answerQuestionId]: { answers: [structuredAnswer.answer!], }, });