fix(exec): stop false INCOMPLETE downgrades on conversational final messages#608
Conversation
…urrent objective
The detector matched inability stems anywhere in the final message, so a
conversational recap of a past exchange ("You asked if I could work
autonomously ... so I couldn't actually do it at the time") downgraded a
completed run to INCOMPLETE. Match per sentence instead, drop quoted spans,
and skip sentences that retell a past exchange. The real transcript is now a
regression test, and genuine admissions still fire, both present tense and
past tense about the current task.
A chat frontend driving zero exec has an operator present, so a final
message that hands the turn back ("the sandbox blocks network egress,
approve it and I'll continue") is a complete conversational answer, not an
unfinished task. The headless completion gate still stamped such runs
INCOMPLETE (exit 4) on the honest inability phrasing, and no phrase
heuristic can fix that case because the admission is true. The new flag
maps to RequireCompletionSignal=false so conversational callers can opt
out. Default behavior is unchanged.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (4)
🚧 Files skipped from review as they are similar to previous changes (3)
WalkthroughThis PR refines self-reported incompletion detection by filtering quoted and narrative fragments before matching admissions, and adds a ChangesSelf-report detection and completion gate flag
Estimated code review effort: 3 (Moderate) | ~25 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ 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 |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@internal/agent/guardrails.go`:
- Around line 209-225: The stripQuoted helper in guardrails.go is over-stripping
when it sees an opening quote/backtick without a matching closer, which can
remove later content needed by selfReportedIncompletion. Update stripQuoted to
only remove balanced quoted segments and preserve the original text when the
input contains dangling delimiters, using the existing stripQuoted and
selfReportedIncompletion flow as the main touchpoints. Add a regression test
covering truncated double quotes, curly quotes, and backticks to ensure
unbalanced input is left intact.
In `@internal/cli/exec.go`:
- Around line 558-561: Reject the combination of --no-completion-gate and
--use-spec during exec parsing so the flag is not silently ignored on the
runExecSpecDraft path. Add the validation alongside the existing --self-correct
+ --use-spec check in exec_parse.go, using the same option parsing flow that
populates options.noCompletionGate and options.useSpec. Ensure the parser
returns an explicit error before exec reaches runExecSpecDraft or agent.Run,
matching the existing early-rejection behavior for incompatible flags.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 0c7b5a88-a301-4143-9198-04071eb07464
📒 Files selected for processing (6)
internal/agent/acceptance_gate_test.gointernal/agent/guardrails.gointernal/cli/app.gointernal/cli/exec.gointernal/cli/exec_parse.gointernal/cli/exec_parse_completion_gate_test.go
…combination stripQuoted now removes only balanced quoted spans: an opening delimiter that never closes is restored verbatim, so a stray quote cannot swallow the rest of the message and hide a genuine admission from the detector. Regression tests cover dangling straight, curly, and backtick delimiters. --no-completion-gate combined with --use-spec is now rejected at parse time, mirroring the --self-correct check: the spec-draft path never consults the completion gate, so the flag would be silently ignored.
anandh8x
left a comment
There was a problem hiding this comment.
LGTM.
Sentence-scoped self-report detection (strip balanced quotes, skip narrative recap sentences) fixes the real false positive without gutting the gate. Present-tense / current-task admissions still fire, and the chess-style cases stay covered.
--no-completion-gate is the right opt-out for chat frontends with an operator present; default zero exec keeps RequireCompletionSignal on. Rejecting it with --use-spec matches the existing --self-correct pattern.
Tests pin both sides. All CI green. Ship it.
gnanam1990
left a comment
There was a problem hiding this comment.
Review
Verdict: Approve with notes. Agree with @anandh8x.
Sentence-scoped self-report detection (balanced quote strip + skip narrative-recap sentences) fixes the real conversational false positive without removing the gate. Present-tense / current-task admissions still fire in the new tests; chess-style coverage still passes. --no-completion-gate is the right opt-out for chat frontends; default zero exec keeps RequireCompletionSignal. Rejecting it with --use-spec matches the --self-correct pattern. CI green; focused agent/cli tests pass with -race.
Notes
- Residual FN risk (non-blocking): whole-sentence skip on markers like
you asked/at the time/last timecan miss a single sentence that both recaps and admits current failure (e.g. "You asked me to port the parser and I couldn't finish the second half.", "I couldn't complete the refactor at the time."). Split sentences still fire. Worth watching / tightening markers if CI incompletes go silent. - Process: no linked issue (
issue-approved); author claims contributor exception.
No disagreement with shipping the core fix.
Summary
Headless
zero execruns driven by a chat frontend get wrongly downgraded to INCOMPLETE (exit 4) by the completion gate's self-report check. Two distinct causes, fixed by the two commits:selfReportedIncompletionmatched inability stems anywhere in the final message, so a recap of a past exchange tripped it. Real transcript: "You asked if I could work autonomously on zero if you gave me a GitHub account. I was honest that my sandbox had no repo, no build system, and no network wired up yet, so I couldn't actually do it at the time." The turn's objective (summarize where we left off) was fully met, but the run endedincomplete: the final message admits the objective was not met ("i couldn't ..."). The detector is now sentence-scoped: quoted spans are dropped and sentences that retell a past exchange ("you asked", "at the time", "when we last") are skipped. That transcript is a regression test. Present-tense admissions and genuine past-tense failure reports about the current task still fire, and the existing chess-incident regression tests still pass.Some admissions are true and still should not fail the run. With an operator present, "the sandbox blocks network egress, so I can't run
gh auth status; approve network and I'll continue" is a complete answer that hands the turn back. No phrase heuristic can distinguish that, so this addszero exec --no-completion-gate, mapping toRequireCompletionSignal=false, for conversational callers. Plainzero execis unchanged and keeps full gate semantics for CI and cron use.Linked issue
None. Opening directly under my contributor exception to the community PR policy.
Checklist
issue-approvedlabel. (Not applicable, see above.)go build ./...,go vet ./..., andgo test ./...pass locally. (internal/agentandinternal/clipass. Three failures elsewhere are local-environment issues untouched by this diff: twoprovideroauthChatGPT loopback-callback tests that cannot bind port 1455 on this machine, andinternal/sandboxTestWindowsUnelevatedSetupMarkerEvictsOldestPastCap.)gofmtclean.-racewhere relevant).zero exec --help.)Summary by CodeRabbit
--no-completion-gateoption forzero exec, allowing a final no-tool-call reply to be accepted without requiring a completion signal.zero exechelp text for the new--no-completion-gateflag, and added validation that it can’t be combined with--use-spec.