From 6b34db2c5ee8b05bc57165b76d41308a66026bbd Mon Sep 17 00:00:00 2001 From: daniel-lxs Date: Wed, 8 Jul 2026 12:54:12 -0500 Subject: [PATCH 1/2] [Fix] Treat a clarification question as a valid end of turn in Slack hooks MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Asking the user a question was not an accepted terminal state: after a send_chat_reply with purpose "clarification", the stop hook blocked with current_turn_nonterminal_reply, the harness injected up to three closeout reminders, and the model dutifully posted redundant "Paused here — I'm waiting on your answer" echo messages right after the question. A clarification is a visible handoff exactly like a closeout — the turn ends waiting on the user's answer — so the anti-silence hooks now treat it as terminal, with the same staleness rule: non-Slack work after the question invalidates it and a fresh reply is required before stopping. - recordChatReplySatisfaction stamps terminal state for clarification replies, so staleness tracking applies to questions too. - The stop hook accepts clarification in its terminal-satisfaction checks (including legacy states written by pre-deploy workers); ack and progress still block. - The silence hook's terminal check mirrors the same policy so post-question work is stamped consistently. - OpenCode todo bookkeeping (todowrite/todoread) no longer stales a terminal reply: the exemption list only knew the legacy update_plan name, so a post-closeout todo update forced a redundant "closed this turn" follow-up message. - The send_chat_reply tool description now says a clarification ends the turn when the next step depends on the user's answer, and not to follow it with a separate "waiting on your answer" message. Verified end-to-end against the mock Slack harness with a live planner task: the question turn now ends cleanly (terminal state stamped, no echo for 3+ minutes where it previously arrived within one), and the answer round-trip completes with a single closeout. Co-Authored-By: Claude Fable 5 --- .../slack-reply-satisfaction.test.ts | 61 +++++++++++++++ .../chat-reply-satisfaction.ts | 5 +- .../src/mcp/roomote-mcp-server/index.ts | 2 +- .../slack-silence-hook-script.test.ts | 34 +++++++++ .../__tests__/slack-stop-hook-script.test.ts | 75 ++++++++++++++++++- .../src/run-task/slack-silence-hook-script.ts | 15 +++- .../src/run-task/slack-stop-hook-script.ts | 16 +++- 7 files changed, 200 insertions(+), 8 deletions(-) diff --git a/apps/worker/src/mcp/roomote-mcp-server/__tests__/slack-reply-satisfaction.test.ts b/apps/worker/src/mcp/roomote-mcp-server/__tests__/slack-reply-satisfaction.test.ts index d7de9c59f..1d3db3af0 100644 --- a/apps/worker/src/mcp/roomote-mcp-server/__tests__/slack-reply-satisfaction.test.ts +++ b/apps/worker/src/mcp/roomote-mcp-server/__tests__/slack-reply-satisfaction.test.ts @@ -373,6 +373,67 @@ describe('Slack reply satisfaction state', () => { }); }); + it('writes terminal state when a current-turn clarification reply is recorded', () => { + const tempDir = fs.mkdtempSync(path.join(os.tmpdir(), 'roomote-slack-')); + tempDirs.push(tempDir); + const stateFilePath = path.join(tempDir, 'reply-state.json'); + process.env[SLACK_REPLY_SATISFACTION_STATE_FILE_ENV] = stateFilePath; + fs.writeFileSync( + stateFilePath, + JSON.stringify({ + currentTurnMessageTs: '111.222', + currentTurnStartedAtMs: 1000, + }), + 'utf8', + ); + + recordSlackReplySatisfaction({ + messageTs: 'bot-333.444', + tool: 'send_chat_reply', + replyPurpose: 'clarification', + nowMs: 1200, + }); + + expect(JSON.parse(fs.readFileSync(stateFilePath, 'utf8'))).toEqual({ + currentTurnMessageTs: '111.222', + currentTurnStartedAtMs: 1000, + satisfiedTurnMessageTs: '111.222', + terminalSatisfiedTurnMessageTs: '111.222', + terminalSatisfiedAtMs: 1200, + terminalSatisfactionTool: 'send_chat_reply', + messageTs: 'bot-333.444', + tool: 'send_chat_reply', + replyPurpose: 'clarification', + recordedAtMs: 1200, + }); + }); + + it('does not write terminal state for ack or progress replies', () => { + const tempDir = fs.mkdtempSync(path.join(os.tmpdir(), 'roomote-slack-')); + tempDirs.push(tempDir); + const stateFilePath = path.join(tempDir, 'reply-state.json'); + process.env[SLACK_REPLY_SATISFACTION_STATE_FILE_ENV] = stateFilePath; + fs.writeFileSync( + stateFilePath, + JSON.stringify({ + currentTurnMessageTs: '111.222', + currentTurnStartedAtMs: 1000, + }), + 'utf8', + ); + + recordSlackReplySatisfaction({ + messageTs: 'bot-333.444', + tool: 'send_chat_reply', + replyPurpose: 'progress', + nowMs: 1200, + }); + + const state = JSON.parse(fs.readFileSync(stateFilePath, 'utf8')); + expect(state.terminalSatisfiedTurnMessageTs).toBeUndefined(); + expect(state.terminalSatisfiedAtMs).toBeUndefined(); + }); + it('does not clobber terminal closeout state when a later reaction overwrites the latest tool markers', () => { const tempDir = fs.mkdtempSync(path.join(os.tmpdir(), 'roomote-slack-')); tempDirs.push(tempDir); diff --git a/apps/worker/src/mcp/roomote-mcp-server/chat-reply-satisfaction.ts b/apps/worker/src/mcp/roomote-mcp-server/chat-reply-satisfaction.ts index 35932609d..0a5b62669 100644 --- a/apps/worker/src/mcp/roomote-mcp-server/chat-reply-satisfaction.ts +++ b/apps/worker/src/mcp/roomote-mcp-server/chat-reply-satisfaction.ts @@ -203,8 +203,11 @@ export function recordChatReplySatisfaction(input: { lastNonSlackWorkAfterSatisfactionAtMs: undefined, } : {}), + // A clarification reply is a terminal handoff like a closeout: the turn + // ends waiting on the user's answer, so ending after one is not silence. ...(input.tool === 'send_chat_reply' && - input.replyPurpose === 'closeout' && + (input.replyPurpose === 'closeout' || + input.replyPurpose === 'clarification') && satisfiesCurrentTurn ? { terminalSatisfiedTurnMessageTs: currentTurnMessageTs, diff --git a/apps/worker/src/mcp/roomote-mcp-server/index.ts b/apps/worker/src/mcp/roomote-mcp-server/index.ts index e70ce359f..e72732698 100644 --- a/apps/worker/src/mcp/roomote-mcp-server/index.ts +++ b/apps/worker/src/mcp/roomote-mcp-server/index.ts @@ -1103,7 +1103,7 @@ if (shouldRegisterSlackThreadReplyTool()) { description: `${chatReplySurfaceLabel}-visible: posts a lifecycle reply in the originating ${chatReplySurfaceLabel} thread. ` + `Choose the current ${chatReplySurfaceLabel} turn purpose before writing: ack, progress, closeout, or clarification. ` + - `Use ack for the first visible response when work will continue; use progress only when the message adds new decision-useful state or prevents a 10-minute silence gap; use closeout for the answer, result, blocker, or handoff; use clarification for lightweight non-secret questions. Only closeout is terminal for final task completion; ack, progress, and clarification keep the ${chatReplySurfaceLabel} turn open. ` + + `Use ack for the first visible response when work will continue; use progress only when the message adds new decision-useful state or prevents a 10-minute silence gap; use closeout for the answer, result, blocker, or handoff; use clarification for lightweight non-secret questions. Use closeout to finish a turn with an outcome; a clarification also ends the turn when the next step depends on the user's answer — do not follow it with a separate "waiting on your answer" message. Ack and progress keep the ${chatReplySurfaceLabel} turn open. ` + `Use it again on later ${chatReplySurfaceLabel} turns when they need another direct reply; an earlier thread reply does not count as the reply for the current turn. ` + "For routine successful closeouts, focus on the shipped change and any blocker or delivery outcome that changes the user's next step; do not include exact validation commands, passed-check ledgers, or proof-applicability narration unless the user asked or that detail materially changes what they should do next. " + chatReplyMarkdownGuidance + diff --git a/apps/worker/src/run-task/__tests__/slack-silence-hook-script.test.ts b/apps/worker/src/run-task/__tests__/slack-silence-hook-script.test.ts index d8c95095d..845a970a4 100644 --- a/apps/worker/src/run-task/__tests__/slack-silence-hook-script.test.ts +++ b/apps/worker/src/run-task/__tests__/slack-silence-hook-script.test.ts @@ -731,6 +731,40 @@ describe('SLACK_SILENCE_HOOK_SCRIPT', () => { ).toBeUndefined(); }); + it.each(['todowrite', 'todoread'])( + 'ignores %s todo bookkeeping after a terminal closeout on PostToolUse', + (toolName) => { + const stateFilePath = writeState({ + recordedAtMs: Date.now() - 60_000, + messageTs: 'bot-111.222', + tool: 'send_chat_reply', + replyPurpose: 'closeout', + satisfiedTurnMessageTs: 'web:client-1', + currentTurnMessageTs: 'web:client-1', + terminalSatisfiedTurnMessageTs: 'web:client-1', + terminalSatisfiedAtMs: Date.now() - 60_000, + terminalSatisfactionTool: 'send_chat_reply', + }); + + const result = runHook({ + input: { + hook_event_name: 'PostToolUse', + tool_name: toolName, + }, + env: { + ROOMOTE_SLACK_REPLY_SATISFACTION_STATE_FILE: stateFilePath, + }, + }); + + expect(result.status).toBe(0); + expect(result.stdout).toBe(''); + expect( + JSON.parse(fs.readFileSync(stateFilePath, 'utf8')) + .lastNonSlackWorkAfterTerminalAtMs, + ).toBeUndefined(); + }, + ); + it.each(['request_user_input', 'request_user_input_handoff'])( 'ignores %s bookkeeping after a terminal closeout on PostToolUse', (toolName) => { diff --git a/apps/worker/src/run-task/__tests__/slack-stop-hook-script.test.ts b/apps/worker/src/run-task/__tests__/slack-stop-hook-script.test.ts index 211875f7a..b075ed978 100644 --- a/apps/worker/src/run-task/__tests__/slack-stop-hook-script.test.ts +++ b/apps/worker/src/run-task/__tests__/slack-stop-hook-script.test.ts @@ -310,7 +310,7 @@ describe('SLACK_STOP_HOOK_SCRIPT', () => { expect(result.stderr).toBe(''); }); - it.each(['ack', 'progress', 'clarification'])( + it.each(['ack', 'progress'])( 'blocks Stop when the recent send_chat_reply purpose is %s', (replyPurpose) => { const stateFilePath = path.join( @@ -350,6 +350,79 @@ describe('SLACK_STOP_HOOK_SCRIPT', () => { }, ); + it('allows Stop when the current turn ends on a clarification question', () => { + const stateFilePath = path.join( + fs.mkdtempSync(path.join(os.tmpdir(), 'roomote-stop-state-')), + 'state.json', + ); + tempDirs.push(path.dirname(stateFilePath)); + fs.writeFileSync( + stateFilePath, + JSON.stringify({ + currentTurnMessageTs: 'user-111.222', + satisfiedTurnMessageTs: 'user-111.222', + messageTs: 'bot-333.444', + tool: 'send_chat_reply', + replyPurpose: 'clarification', + recordedAtMs: Date.now(), + }), + 'utf8', + ); + + const result = runHook({ + env: { + ROOMOTE_SLACK_HOOK_DEBUG: 'true', + ROOMOTE_SLACK_REPLY_SATISFACTION_STATE_FILE: stateFilePath, + }, + }); + + expect(result.status).toBe(0); + expect(result.stdout).toBe(''); + expect(result.stderr).toContain('decision="allow"'); + expect(result.stderr).toContain('reason="terminal_reply_satisfied"'); + }); + + it('blocks Stop when non-Slack work happened after a clarification question', () => { + const clarificationAtMs = Date.now() - 60_000; + const stateFilePath = path.join( + fs.mkdtempSync(path.join(os.tmpdir(), 'roomote-stop-state-')), + 'state.json', + ); + tempDirs.push(path.dirname(stateFilePath)); + fs.writeFileSync( + stateFilePath, + JSON.stringify({ + currentTurnMessageTs: 'user-111.222', + satisfiedTurnMessageTs: 'user-111.222', + messageTs: 'bot-333.444', + tool: 'send_chat_reply', + replyPurpose: 'clarification', + recordedAtMs: clarificationAtMs, + terminalSatisfiedTurnMessageTs: 'user-111.222', + terminalSatisfiedAtMs: clarificationAtMs, + terminalSatisfactionTool: 'send_chat_reply', + lastNonSlackWorkAfterTerminalAtMs: clarificationAtMs + 30_000, + }), + 'utf8', + ); + + const result = runHook({ + env: { + ROOMOTE_SLACK_REPLY_SATISFACTION_STATE_FILE: stateFilePath, + }, + }); + + expect(result.status).toBe(0); + expect(JSON.parse(result.stdout)).toEqual({ + decision: 'block', + reason: reminder, + }); + expect(result.stderr).toContain('decision="block"'); + expect(result.stderr).toContain( + 'reason="current_turn_terminal_reply_stale"', + ); + }); + it('blocks Stop when the current Slack turn only has a recent successful reaction', () => { const stateFilePath = path.join( fs.mkdtempSync(path.join(os.tmpdir(), 'roomote-stop-state-')), diff --git a/apps/worker/src/run-task/slack-silence-hook-script.ts b/apps/worker/src/run-task/slack-silence-hook-script.ts index 7ca6b4b06..80a9ae86d 100644 --- a/apps/worker/src/run-task/slack-silence-hook-script.ts +++ b/apps/worker/src/run-task/slack-silence-hook-script.ts @@ -190,10 +190,15 @@ function isCloseoutBookkeepingTool(input) { const toolName = input && typeof input.tool_name === 'string' ? input.tool_name : ''; + // Todo bookkeeping (OpenCode's todowrite/todoread, legacy update_plan) does + // not add user-visible outcome, so it must not stale a terminal reply and + // force a redundant follow-up closeout. return ( toolName.endsWith('update_plan') || toolName.includes('/update_plan') || - toolName.includes('.update_plan') + toolName.includes('.update_plan') || + toolName.endsWith('todowrite') || + toolName.endsWith('todoread') ); } @@ -298,8 +303,14 @@ function hasCurrentTurnTerminalCloseout(state) { return false; } + // Clarification is a terminal handoff like closeout: the turn ends waiting + // on the user's answer (kept in sync with the stop hook's policy). const replyPurpose = trimString(state.replyPurpose); - if (replyPurpose && replyPurpose !== 'closeout') { + if ( + replyPurpose && + replyPurpose !== 'closeout' && + replyPurpose !== 'clarification' + ) { return false; } diff --git a/apps/worker/src/run-task/slack-stop-hook-script.ts b/apps/worker/src/run-task/slack-stop-hook-script.ts index 267cd785c..0830446e6 100644 --- a/apps/worker/src/run-task/slack-stop-hook-script.ts +++ b/apps/worker/src/run-task/slack-stop-hook-script.ts @@ -212,6 +212,16 @@ function isCurrentTurnSatisfied(state) { return trimString(state && state.satisfiedTurnMessageTs) === currentTurnMessageTs; } +// Closeout ends the turn with an outcome; clarification ends it waiting on +// the user's answer. Both are visible handoffs, so neither is "silence" — +// blocking on a fresh clarification only forces a redundant "I'm waiting on +// your answer" echo message. +function isTerminalReplyPurpose(replyPurpose) { + return ( + !replyPurpose || replyPurpose === 'closeout' || replyPurpose === 'clarification' + ); +} + function getLegacyTerminalSatisfaction(state, currentTurnMessageTs) { const tool = trimString(state && state.tool); if (tool !== 'send_chat_reply') { @@ -219,7 +229,7 @@ function getLegacyTerminalSatisfaction(state, currentTurnMessageTs) { } const replyPurpose = trimString(state && state.replyPurpose); - if (replyPurpose && replyPurpose !== 'closeout') { + if (!isTerminalReplyPurpose(replyPurpose)) { return null; } @@ -248,7 +258,7 @@ function getCurrentTurnTerminalSatisfaction(state) { } const replyPurpose = trimString(state && state.replyPurpose); - if (replyPurpose && replyPurpose !== 'closeout') { + if (!isTerminalReplyPurpose(replyPurpose)) { return null; } @@ -337,7 +347,7 @@ function getTerminalCurrentTurnFailureReason(state) { } const replyPurpose = trimString(state && state.replyPurpose); - if (replyPurpose && replyPurpose !== 'closeout') { + if (!isTerminalReplyPurpose(replyPurpose)) { return 'current_turn_nonterminal_reply'; } From 16fab24f2ab5d5bd4b8aea338d13a41438cadc55 Mon Sep 17 00:00:00 2001 From: daniel-lxs Date: Wed, 8 Jul 2026 14:42:40 -0500 Subject: [PATCH 2/2] Update pinned send_chat_reply descriptions in tool-descriptions test Co-Authored-By: Claude Fable 5 --- .../roomote-mcp-server/__tests__/tool-descriptions.test.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/apps/worker/src/mcp/roomote-mcp-server/__tests__/tool-descriptions.test.ts b/apps/worker/src/mcp/roomote-mcp-server/__tests__/tool-descriptions.test.ts index 02685ac31..475d98f69 100644 --- a/apps/worker/src/mcp/roomote-mcp-server/__tests__/tool-descriptions.test.ts +++ b/apps/worker/src/mcp/roomote-mcp-server/__tests__/tool-descriptions.test.ts @@ -237,7 +237,7 @@ describe('roomote MCP tool descriptions', () => { 'Choose the current Slack turn purpose before writing: ack, progress, closeout, or clarification.', ); expect(replyTool.config.description).toContain( - 'Use ack for the first visible response when work will continue; use progress only when the message adds new decision-useful state or prevents a 10-minute silence gap; use closeout for the answer, result, blocker, or handoff; use clarification for lightweight non-secret questions. Only closeout is terminal for final task completion; ack, progress, and clarification keep the Slack turn open.', + `Use ack for the first visible response when work will continue; use progress only when the message adds new decision-useful state or prevents a 10-minute silence gap; use closeout for the answer, result, blocker, or handoff; use clarification for lightweight non-secret questions. Use closeout to finish a turn with an outcome; a clarification also ends the turn when the next step depends on the user's answer — do not follow it with a separate "waiting on your answer" message. Ack and progress keep the Slack turn open.`, ); expect(replyTool.config.description).toContain( "For routine successful closeouts, focus on the shipped change and any blocker or delivery outcome that changes the user's next step; do not include exact validation commands, passed-check ledgers, or proof-applicability narration unless the user asked or that detail materially changes what they should do next.", @@ -605,7 +605,7 @@ describe('roomote MCP tool descriptions', () => { const chatReplyTool = getRegisteredTool(registeredTools, 'send_chat_reply'); expect(chatReplyTool.config.description).toBe( - "Slack-visible: posts a lifecycle reply in the originating Slack thread. Choose the current Slack turn purpose before writing: ack, progress, closeout, or clarification. Use ack for the first visible response when work will continue; use progress only when the message adds new decision-useful state or prevents a 10-minute silence gap; use closeout for the answer, result, blocker, or handoff; use clarification for lightweight non-secret questions. Only closeout is terminal for final task completion; ack, progress, and clarification keep the Slack turn open. Use it again on later Slack turns when they need another direct reply; an earlier thread reply does not count as the reply for the current turn. For routine successful closeouts, focus on the shipped change and any blocker or delivery outcome that changes the user's next step; do not include exact validation commands, passed-check ledgers, or proof-applicability narration unless the user asked or that detail materially changes what they should do next. Supports the modern Slack Markdown contract from the Slack instructions. Use rich Markdown when it improves scanability. When the reply mentions actionable code references, follow the Slack prompt source-linking rule. Write the message so its content clearly matches the selected purpose.", + `Slack-visible: posts a lifecycle reply in the originating Slack thread. Choose the current Slack turn purpose before writing: ack, progress, closeout, or clarification. Use ack for the first visible response when work will continue; use progress only when the message adds new decision-useful state or prevents a 10-minute silence gap; use closeout for the answer, result, blocker, or handoff; use clarification for lightweight non-secret questions. Use closeout to finish a turn with an outcome; a clarification also ends the turn when the next step depends on the user's answer — do not follow it with a separate "waiting on your answer" message. Ack and progress keep the Slack turn open. Use it again on later Slack turns when they need another direct reply; an earlier thread reply does not count as the reply for the current turn. For routine successful closeouts, focus on the shipped change and any blocker or delivery outcome that changes the user's next step; do not include exact validation commands, passed-check ledgers, or proof-applicability narration unless the user asked or that detail materially changes what they should do next. Supports the modern Slack Markdown contract from the Slack instructions. Use rich Markdown when it improves scanability. When the reply mentions actionable code references, follow the Slack prompt source-linking rule. Write the message so its content clearly matches the selected purpose.`, ); expect(getInputSchemaField(chatReplyTool, 'message').description).toBe( "Markdown text to post in the Slack thread. Match the selected purpose, lead with the useful takeaway, and keep it conversational like a teammate in a thread. For routine successful closeouts, focus on the shipped change and any blocker or delivery outcome that changes the user's next step instead of listing exact validation commands, passed checks, or proof-applicability notes unless the user asked for them or they materially change what the user should do next. Use the modern Slack Markdown contract from the Slack instructions; tables, headings, blockquotes, and fenced code blocks are allowed when they make the reply clearer.",