Skip to content

[Fix] Treat a clarification question as a valid end of turn in Slack hooks - #3

Merged
daniel-lxs merged 2 commits into
developfrom
fix/clarification-terminal-turn
Jul 8, 2026
Merged

[Fix] Treat a clarification question as a valid end of turn in Slack hooks#3
daniel-lxs merged 2 commits into
developfrom
fix/clarification-terminal-turn

Conversation

@daniel-lxs

Copy link
Copy Markdown
Member

Problem

When an agent asks the user a question via send_chat_reply purpose clarification and ends its turn, the stop hook blocks (current_turn_nonterminal_reply — only closeout counted as terminal). The harness then injects up to 3 closeout reminders, and the model posts redundant echo messages right after its own question: "Paused here for now — I'm waiting on your answer to the question above."

Investigation notes (from the OpenRoomote debugging session; ported here per repo targeting):

  • This is the only live echo path: the request_user_input branches in both hooks are dead code (nothing writes that tool into the satisfaction state), and the structured question tool blocks the turn in-flight so the stop hook never runs while it waits.
  • A second echo source was caught live during e2e verification: after a closeout, a todowrite bookkeeping call staled the terminal reply (the exemption list only knew the legacy update_plan name), forcing a redundant "closed this turn" follow-up.

Fix

Principle: the hooks exist to prevent turns from ending silently. A clarification question is a visible handoff exactly like a closeout — the turn ends waiting on the user's answer — so it now counts as terminal, with the same staleness rule as closeout (non-Slack work after the question invalidates it). ack and progress still block; automation no-turn closeout enforcement is untouched.

  • recordChatReplySatisfaction stamps terminal state for clarification replies
  • Stop hook accepts clarification in terminal-satisfaction checks (incl. legacy states written by pre-deploy workers)
  • Silence hook mirrors the same policy; OpenCode todowrite/todoread count as closeout bookkeeping and no longer stale a terminal reply
  • send_chat_reply description updated: a clarification ends the turn when the next step depends on the user's answer — don't follow it with a separate "waiting on your answer" message

Verification

  • Hook-script tests exercise the real generated scripts: clarification at turn end → allow (terminal_reply_satisfied); non-Slack work after the question → block (current_turn_terminal_reply_stale); ack/progress still block; todowrite/todoread don't stale a closeout
  • Verified end-to-end on a live planner task via the mock Slack harness: question turn ends cleanly with the terminal state stamped and no echo for 3+ minutes (previously within one minute), and the answer round-trip completes with a single closeout
  • apps/worker run-task + satisfaction tests: 228 passed; tsc --noEmit and prettier clean

🤖 Generated with Claude Code

…hooks

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 <noreply@anthropic.com>
@roomote-roomote-v1

roomote-roomote-v1 Bot commented Jul 8, 2026

Copy link
Copy Markdown

2 issues outstanding (1 medium, 1 low). No new issues in the latest update (test-expectation sync only). See task

  • Automation no-turn tasks can now stop on a clarificationapps/worker/src/run-task/slack-stop-hook-script.ts:252-275. The no-inbound-turn branch of getCurrentTurnTerminalSatisfaction now runs through isTerminalReplyPurpose, which accepts clarification. For a requiresTerminalCloseoutWithoutTurn automation task with no currentTurnMessageTs, a send_chat_reply purpose clarification is now treated as terminal, so the Stop hook allows completion where it previously blocked with current_turn_nonterminal_reply. This diverges from the silence hook's hasNoTurnAutomationTerminalCloseout (still closeout-only) and from the PR's stated goal that "automation no-turn closeout enforcement is untouched" — the automation could end after only asking a question, with no self-contained outcome. Consider gating clarification-as-terminal on the presence of a current turn (or excluding it when requiresTerminalCloseoutWithoutTurn is set).
  • todowrite/todoread exemption misses namespaced variantsapps/worker/src/run-task/slack-silence-hook-script.ts:196-202. isCloseoutBookkeepingTool matches the todo tools with endsWith('todowrite')/endsWith('todoread') only, while update_plan in the same function also handles /update_plan and .update_plan namespaced forms. If the todo tools are ever surfaced under an MCP/namespaced prefix they would not be exempted and could stale a terminal reply, reintroducing the redundant follow-up this PR fixes. Low risk today since OpenCode's todo tools are unprefixed built-ins.

@github-actions

github-actions Bot commented Jul 8, 2026

Copy link
Copy Markdown
Contributor

All contributors have signed the CLA.
Posted by the CLA Assistant Lite bot.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@daniel-lxs

Copy link
Copy Markdown
Member Author

I have read the CLA Document and I hereby sign the CLA

@daniel-lxs
daniel-lxs merged commit 0742f8c into develop Jul 8, 2026
@daniel-lxs
daniel-lxs deleted the fix/clarification-terminal-turn branch July 8, 2026 19:45
mrubens pushed a commit that referenced this pull request Jul 11, 2026
Co-authored-by: Roomote <roomote@roomote.dev>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant