fix(worker): gate text-only exits on explicit outcome signal#327
Merged
fix(worker): gate text-only exits on explicit outcome signal#327
Conversation
Contributor
|
Caution Review failedPull request was closed or merged during review WalkthroughThe changes implement a new tool-nudging/outcome-gate: workers must explicitly call Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![tembo[bot]](https://avatars.githubusercontent.com/in/1223475?s=60&v=4){kind=small}
Comment on lines
+48
to
+50
| pub const TOOL_NUDGE_PROMPT: &str = "You have not completed the task yet. Continue working using the available tools. \ | ||
| When you have reached a final result, call set_status with kind \"outcome\" \ | ||
| before finishing."; |
Contributor
There was a problem hiding this comment.
Small nit: the nudge prompt wording is close, but it’d be nice to align it exactly with the worker docs + tool schema (kind: "outcome") and format set_status consistently.
Suggested change
| pub const TOOL_NUDGE_PROMPT: &str = "You have not completed the task yet. Continue working using the available tools. \ | |
| When you have reached a final result, call set_status with kind \"outcome\" \ | |
| before finishing."; | |
| pub const TOOL_NUDGE_PROMPT: &str = "You have not completed the task yet. Continue working using the available tools. \ | |
| When you have reached a final result, call `set_status` with `kind: \"outcome\"` before finishing."; |
| @@ -36,13 +36,18 @@ pub struct SpacebotHook { | |||
| event_tx: broadcast::Sender<ProcessEvent>, | |||
| tool_nudge_policy: ToolNudgePolicy, | |||
| completion_calls: std::sync::Arc<std::sync::atomic::AtomicUsize>, | |||
Contributor
There was a problem hiding this comment.
completion_calls looks unused now (it no longer gates nudging, and I don’t see it read anywhere). Might be worth removing to avoid future confusion, unless you’re keeping it for telemetry/debug output elsewhere.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
tests/tool_nudge.rs (1)
17-20: Consider assertingoutcomeguidance explicitly as well.The current check is flexible, but adding a second substring assertion for
kind/outcomewould better protect the core contract.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/tool_nudge.rs` around lines 17 - 20, Add a second assertion guarding the nudge prompt's explicit guidance for signaling the outcome: in the test alongside spacebot::hooks::SpacebotHook::TOOL_NUDGE_PROMPT.contains("set_status"), assert that TOOL_NUDGE_PROMPT also contains the substring "outcome" (or "kind"/equivalent token your prompt uses) so the prompt explicitly documents the outcome/kind signaling contract; update the assertion message accordingly to indicate the nudge prompt should reference outcome/kind for outcome signaling.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/hooks/spacebot.rs`:
- Around line 476-484: The current code sets outcome_signaled based on the
intent to call the "set_status" tool (checking tool_name and parsing args),
which lets a malformed or failed call still mark outcome signaled; instead, move
the check so outcome_signaled is set only when the tool actually returns a
successful result. Locate the call site that processes tool results (the
function/method that receives the tool response/result for "set_status" — e.g.,
where tool responses are handled or where apply_tool_result/handle_tool_response
is implemented), parse the returned tool output there (inspect the successful
result payload), and only call self.outcome_signaled.store(true,
Ordering::Relaxed) when the tool execution succeeded and the parsed JSON has
kind == "outcome"; remove or stop using the current intent-time check that uses
tool_name/args for this purpose.
---
Nitpick comments:
In `@tests/tool_nudge.rs`:
- Around line 17-20: Add a second assertion guarding the nudge prompt's explicit
guidance for signaling the outcome: in the test alongside
spacebot::hooks::SpacebotHook::TOOL_NUDGE_PROMPT.contains("set_status"), assert
that TOOL_NUDGE_PROMPT also contains the substring "outcome" (or
"kind"/equivalent token your prompt uses) so the prompt explicitly documents the
outcome/kind signaling contract; update the assertion message accordingly to
indicate the nudge prompt should reference outcome/kind for outcome signaling.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 74fa9c90-ad6b-4a6a-bece-b79fd899fa6f
📒 Files selected for processing (8)
AGENTS.mddocs/design-docs/tool-nudging.mdprompts/en/tools/set_status_description.md.j2prompts/en/worker.md.j2src/hooks/spacebot.rssrc/tools.rssrc/tools/set_status.rstests/tool_nudge.rs
Workers could silently reach Done state without completing their task by returning text-only responses mid-work. The tool nudge system only protected against cold-start text (saw_tool_call disabled it after the first tool call), so workers that made a few calls then narrated instead of acting would exit as successful with no useful output. Replace the saw_tool_call/completion_calls guards with an outcome gate: workers must call set_status(kind: "outcome") before the system allows a text-only exit. Without it, every text-only response triggers a nudge retry. After retries exhaust, the worker fails loudly instead of silently succeeding. - Add StatusKind (progress|outcome) to set_status tool, backward compat - Track outcome_signaled flag in SpacebotHook, detected from tool args - Remove saw_tool_call field and completion_calls guard from nudge logic - Update worker prompt to require outcome signal before finishing - Rewrite tests to cover the exact bug case and outcome gate behavior
jamiepine
force-pushed
the
fix/worker-outcome-gate
branch
from
bafef6e to
fac4e23
Compare
This was referenced Mar 5, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Donestate without completing their task — the model would return text-only after a few tool calls and the system accepted it as a legitimate completionsaw_tool_callpermanently disabled it after the first tool call), leaving workers unprotected mid-tasksaw_tool_call/completion_callsguards with an outcome gate: workers must callset_status(kind: "outcome")before the system allows a text-only exitProblem
Observed in production: workers spawned for email-sending tasks would call
read_skill+set_status("Researching...")then return narration text like "Let me create the email now..." — and the worker would exit asDonewithsuccess: true. The email was never sent. This happened 7+ times in a row on the same task.Root cause: Rig's agent loop treats any text-only response (no tool calls) as the termination signal. The
should_nudge_tool_usagehook was supposed to catch this, but it had two guards that disabled nudging too early:saw_tool_call— any tool call ever → nudge permanently disabledcompletion_calls > 2— more than 2 LLM completions → nudge disabledBoth meant the nudge only worked on the very first 1-2 LLM calls when the model had never touched a tool.
Changes
src/hooks/spacebot.rs— Core fixsaw_tool_callfield entirelycompletion_callsguard fromshould_nudge_tool_usageoutcome_signaledatomic flag, set whenset_status(kind: "outcome")is detected inon_tool_callargssrc/tools/set_status.rs— Outcome signal mechanismStatusKindenum (Progress|Outcome) with#[serde(default)]kindfield toSetStatusArgsand tool schemakinddefaults toProgressprompts/en/worker.md.j2— Worker instructionsset_status(kind: "outcome")with a result summary before finishingdocs/design-docs/tool-nudging.md— Full rewrite reflecting outcome-gated designBehavior change
read_skill+set_status(progress)Doneset_status(kind: "outcome")then returns textDonePromptCancelledTesting
cargo fmtclean,cargo clippyzero warningsgate-pr.shall green