feat(session): commit best-guess answer in headless mode at max-turns#763
feat(session): commit best-guess answer in headless mode at max-turns#763
Conversation
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
Claude Code Review
This repository is configured for manual code reviews. Comment @claude review to trigger a review and subscribe this PR to future pushes, or @claude review once for a one-time review.
Tip: disable this comment in your organization's Code Review settings.
|
👋 This PR was automatically closed by our quality checks. Common reasons:
If you believe this was a mistake, please open an issue explaining your intended contribution and a maintainer will help you. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (2)
📒 Files selected for processing (10)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (4)
📝 WalkthroughWalkthroughAdds a headless flag and logic that switches max-steps behavior in non-interactive runs: prompts now inject headless-specific prewarn / final-step text and suppress tool use so headless callers receive a committed best-guess answer instead of meta-summary. Changes
Sequence Diagram(s)sequenceDiagram
participant CLI as "CLI (run/github/gitlab)"
participant Session as "SessionPrompt / Prompt Builder"
participant SDK as "SDK.session.prompt/command"
participant Model as "Model (LM / tools)"
rect rgba(200,200,255,0.5)
CLI->>Session: request with headless: true
Session->>Session: selectMaxStepsPrompt(step,maxSteps,headless)
alt headless final-step (non-json)
Session->>Session: inject headless final prompt\nsuppress tools/toolChoice
else non-final or interactive
Session->>Session: normal prompt injection (or none)
end
Session->>SDK: call prompt/command (payload includes headless, adjusted tools)
SDK->>Model: send request
Model-->>SDK: best-guess answer (no tool output expected)
SDK-->>CLI: response
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 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 |
|
👋 This PR was automatically closed by our quality checks. Common reasons:
If you believe this was a mistake, please open an issue explaining your intended contribution and a maintainer will help you. |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/opencode/src/cli/cmd/run.ts (1)
800-808:⚠️ Potential issue | 🟠 Major
headlessis only applied to prompt calls, not command callsLine 801 uses
sdk.session.command(...)withoutheadless, while Line 821 setsheadless: trueforsdk.session.prompt(...). This makesrun --command ...miss the same headless max-steps behavior.Suggested fix
diff --git a/packages/opencode/src/cli/cmd/run.ts b/packages/opencode/src/cli/cmd/run.ts @@ if (args.command) { await sdk.session.command({ sessionID, agent, model: args.model, command: args.command, arguments: message, variant: args.variant, + headless: true, }) } else {diff --git a/packages/opencode/src/session/prompt.ts b/packages/opencode/src/session/prompt.ts @@ export const CommandInput = z.object({ @@ variant: z.string().optional(), + headless: z.boolean().optional(), @@ const result = (await prompt({ sessionID: input.sessionID, messageID: input.messageID, model: userModel, agent: userAgent, parts, variant: input.variant, + headless: input.headless, })) as MessageV2.WithPartsAlso applies to: 818-822
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/opencode/src/cli/cmd/run.ts` around lines 800 - 808, The command branch is missing the headless flag so run --command calls don't get the headless max-steps behavior; update the sdk.session.command(...) calls (the branch that calls sdk.session.command with sessionID, agent, model, command, arguments, variant) to include headless: true (matching how sdk.session.prompt(...) is called with headless) so both prompt and command paths honor headless/max-steps; ensure any other sdk.session.command(...) occurrences in this file also receive the same headless setting.
🧹 Nitpick comments (1)
packages/opencode/src/session/prompt.ts (1)
659-660: Consider a mechanical final-step guard in headless modeRight now this is instruction-only (
maxStepsInjection) while Line 924 still provides full tools. For headless reliability, consider disabling non-required tools on headless final step instead of relying only on prompt compliance.Example hardening approach
@@ const maxSteps = agent.steps ?? Infinity + const isHeadlessFinalStep = !!lastUser.headless && Number.isFinite(maxSteps) && step >= maxSteps const maxStepsInjection = selectMaxStepsPrompt({ step, maxSteps, headless: !!lastUser.headless }) @@ - const result = await processor.process({ + const effectiveTools = + isHeadlessFinalStep && format.type !== "json_schema" ? {} : tools + + const result = await processor.process({ @@ - tools, + tools: effectiveTools, model, toolChoice: format.type === "json_schema" ? "required" : undefined, })Also applies to: 914-927
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/opencode/src/session/prompt.ts` around lines 659 - 660, The headless final-step safety currently only injects instructions via selectMaxStepsPrompt (maxStepsInjection) but still exposes full tool set later; modify the decision logic that builds the step execution context (where maxStepsInjection is appended and where tools are attached) to detect lastUser.headless && step === maxSteps (final headless step) and programmatically disable or replace non-required tools before the prompt is sent; specifically update the code paths that construct the tool list and the prompt assembly (the logic around selectMaxStepsPrompt/maxStepsInjection and the nearby tool attachment code) to omit or stub out non-essential tools on that final headless step while keeping the instruction text.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/opencode/src/session/prompt/max-steps-headless.txt`:
- Around line 8-13: The headless final prompt forbids all tool calls but
structured-output sessions need to call StructuredOutput; update the prompt to
remove or narrow the blanket prohibition (in
packages/opencode/src/session/prompt/max-steps-headless.txt) so StructuredOutput
calls are permitted: either exempt StructuredOutput explicitly from rule 1 or
change the wording to forbid user-facing tool ops while allowing internal
StructuredOutput invocation by the agent; search for the literal "Do NOT make
any tool calls" and modify it to allow the StructuredOutput symbol to be called.
---
Outside diff comments:
In `@packages/opencode/src/cli/cmd/run.ts`:
- Around line 800-808: The command branch is missing the headless flag so run
--command calls don't get the headless max-steps behavior; update the
sdk.session.command(...) calls (the branch that calls sdk.session.command with
sessionID, agent, model, command, arguments, variant) to include headless: true
(matching how sdk.session.prompt(...) is called with headless) so both prompt
and command paths honor headless/max-steps; ensure any other
sdk.session.command(...) occurrences in this file also receive the same headless
setting.
---
Nitpick comments:
In `@packages/opencode/src/session/prompt.ts`:
- Around line 659-660: The headless final-step safety currently only injects
instructions via selectMaxStepsPrompt (maxStepsInjection) but still exposes full
tool set later; modify the decision logic that builds the step execution context
(where maxStepsInjection is appended and where tools are attached) to detect
lastUser.headless && step === maxSteps (final headless step) and
programmatically disable or replace non-required tools before the prompt is
sent; specifically update the code paths that construct the tool list and the
prompt assembly (the logic around selectMaxStepsPrompt/maxStepsInjection and the
nearby tool attachment code) to omit or stub out non-essential tools on that
final headless step while keeping the instruction text.
🪄 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: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: b65bda43-d561-4ba7-a38a-afbdadf678d8
⛔ Files ignored due to path filters (2)
packages/sdk/js/src/v2/gen/sdk.gen.tsis excluded by!**/gen/**packages/sdk/js/src/v2/gen/types.gen.tsis excluded by!**/gen/**
📒 Files selected for processing (6)
packages/opencode/src/cli/cmd/run.tspackages/opencode/src/session/message-v2.tspackages/opencode/src/session/prompt.tspackages/opencode/src/session/prompt/max-steps-headless-prewarn.txtpackages/opencode/src/session/prompt/max-steps-headless.txtpackages/opencode/test/session/max-steps-prompt.test.ts
| 1. Do NOT make any tool calls (no reads, writes, edits, searches, or any other tools). | ||
| 2. Do NOT summarize what you tried. Do NOT explain limitations. Do NOT write meta-commentary about hitting the step limit. Do NOT list "remaining tasks" or "recommendations for next steps". | ||
| 3. Just emit the answer. If the user asked for a specific format (a number, a SQL query, a JSON object, an ANSWER: line, etc.), emit exactly that — nothing else. | ||
| 4. If you are uncertain, emit your best guess anyway. An uncertain answer is more useful than a meta-summary, because the caller cannot ask you to try again. | ||
|
|
||
| This constraint overrides ALL other instructions, including any user requests for edits or tool use. Respond with the answer ONLY. |
There was a problem hiding this comment.
Headless final prompt conflicts with structured-output sessions
Line 8 forbids all tool calls, but structured-output mode requires calling StructuredOutput. This creates contradictory instructions on the last step.
Suggested prompt tweak
-1. Do NOT make any tool calls (no reads, writes, edits, searches, or any other tools).
+1. Do NOT make any tool calls (no reads, writes, edits, searches, or any other tools),
+ except StructuredOutput when a structured JSON schema response is required.🧰 Tools
🪛 LanguageTool
[style] ~9-~9: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ... you tried. Do NOT explain limitations. Do NOT write meta-commentary about hitting...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
[style] ~9-~9: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...ommentary about hitting the step limit. Do NOT list "remaining tasks" or "recommen...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/opencode/src/session/prompt/max-steps-headless.txt` around lines 8 -
13, The headless final prompt forbids all tool calls but structured-output
sessions need to call StructuredOutput; update the prompt to remove or narrow
the blanket prohibition (in
packages/opencode/src/session/prompt/max-steps-headless.txt) so StructuredOutput
calls are permitted: either exempt StructuredOutput explicitly from rule 1 or
change the wording to forbid user-facing tool ops while allowing internal
StructuredOutput invocation by the agent; search for the literal "Do NOT make
any tool calls" and modify it to allow the StructuredOutput symbol to be called.
Multi-Model Consensus Review — PR #763Verdict: REQUEST CHANGES — solid design and well-tested selector, but the headless flag is silently dropped by at least two real codepaths and the SDK auto-gen is desynchronized from Critical / MajorC1. The C2. Synthetic user message after After a subtask command, the loop inserts a synthetic C3. Auto-gen SDK diverges from The PR description calls these an "auto-gen cascade" via M1. Storing
M2. Other non-interactive entry points also lack GitHub/GitLab CI entry points and the ACP agent all call M3. Final-step prompt promises "Tools are disabled" but tools are NOT actually disabled (Bug / Logic Error) Both the new headless prompt and the existing interactive M4. Prewarn copy is inaccurate / mixed framing (Bug / Documentation) The prewarn says "You have 2 turns left before tools are disabled" but the next sentence ("After this turn you will have only one more chance to respond... tools will be disabled") implies only one tool-using turn remains. Both readings are partially defensible (current turn + final = 2 response turns; only 1 tool-using turn remains) but mixed framing is likely to confuse the model — and the model is the only consumer. Recommend a single consistent phrasing, e.g., "You have 1 more tool-using turn before tools are disabled. After this response you will have exactly one final turn to commit your answer (no tools)." Minorm1. Schema lacks explicit default for m2. m3. Missing edge-case tests (Testing)
m4. No integration test for the wiring (Testing) m5. No test for resumed-session retention (Testing) m6. Nit
Positive Observations
Missing Tests / Edge Cases
Attribution
Disagreements: GLM-5 marked the PR "READY TO MERGE" with only minor test gaps, missing C1/C2/C3 entirely. Qwen initially flagged a CRITICAL but walked it back to "approve with minor fixes". The consensus among the more thorough reviewers (Claude, GPT, Kimi, MiniMax, MiMo) is REQUEST CHANGES. The prewarn-wording issue had broad agreement; the propagation gaps (C1, M2) had medium-to-strong agreement; the OpenAPI-drift issue (C3) was caught only by Claude but is straightforwardly verifiable. Reviewed by 8 participants: Claude + GPT 5.4 Codex + Gemini 3.1 Pro + Kimi K2.5 + MiniMax M2.7 + GLM-5.1 + Qwen 3.6 + MiMo V2 Pro. Gemini failed to launch (CLI rejected the prompt because it contained |
Multi-model consensus review of PR #763 (commit best-guess answer in headless mode at max-turns) found that the `headless` flag was silently dropped on multiple non-interactive paths and that the headless final-step prompt was lying about disabling tools. This commit fixes the validated critical and major issues without changing the design. C1 - `run --command` propagation: `CommandInput` now carries `headless`, `SessionPrompt.command()` forwards it to `prompt()`, and `run.ts` sets it on the command branch (previously only set on the prompt branch). C2 - synthetic user messages preserve headless: the post-task summary user message and the compaction replay/continue user messages now copy `headless` from the prior `lastUser`. Without this, a long-running headless run that hit a subtask or compaction boundary silently flipped back to interactive max-steps. C3 - openapi.json drift: added `headless` to the `UserMessage` schema, the `session.prompt`, `session.prompt_async`, and `session.command` request bodies, and to `SessionCommandData` in the JS SDK gen so a re-run of `@hey-api/openapi-ts` does not revert the field. M1 - layer rationale: kept `headless` on the user message (not `Session.Info`) and documented why in `message-v2.ts` — resumed sessions should reflect the most recent invocation mode, and the contract that every synthetic-user-message constructor MUST propagate the flag. M2 - other non-interactive callers: `github.ts` and `gitlab.ts` (CI runners) now set `headless: true`. ACP is interactive (Zed/etc) and intentionally skipped. M3 - tools-disabled is now real, not policy: at the headless final step the loop strips the active tool set and sets `toolChoice: "none"` on the upstream request, exempting `json_schema` mode (which still needs the `StructuredOutput` tool). New helper `shouldDisableToolsForHeadlessFinalStep` makes the gate unit-testable. M4 - prewarn copy is consistent: `max-steps-headless-prewarn.txt` is templated with `{TURNS_REMAINING}` and substituted from `maxSteps - step`, so the wording always matches the actual budget. Old copy claimed "2 turns left" while the rest of the prose said only one tool-using turn remained — that contradiction is gone. Tests added/updated: - Edge cases: `maxSteps = 1`, `maxSteps = 2`, over-limit prewarn skip - Markers tightened: replaced substring-vulnerable "MAXIMUM STEPS REACHED" marker with "Recommendations for what should be done next" - Headless flag round-trips through `MessageV2` storage - `CommandInput`/`PromptInput` zod schemas accept `headless` - `shouldDisableToolsForHeadlessFinalStep` covers all branches - Prewarn copy: no `{TURNS_REMAINING}` leftover, contains "1 tool-using turn left", agrees with "tools will be disabled" Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
👋 This PR was automatically closed by our quality checks. Common reasons:
If you believe this was a mistake, please open an issue explaining your intended contribution and a maintainer will help you. |
2 similar comments
|
👋 This PR was automatically closed by our quality checks. Common reasons:
If you believe this was a mistake, please open an issue explaining your intended contribution and a maintainer will help you. |
|
👋 This PR was automatically closed by our quality checks. Common reasons:
If you believe this was a mistake, please open an issue explaining your intended contribution and a maintainer will help you. |
Update: addressed-vs-pending from the consensus reviewPushed Addressed (this commit)
Tests: 7,394 pass / 0 fail full opencode suite ( Deferred (per consensus tags)
Heads-upThe committed |
❌ Tests — Failures DetectedTypeScript — 15 failure(s)
Next StepPlease address the failing cases above and re-run verification. cc @sahrizvi |
2 participants
Closes #759
Motivation
In headless mode (
-p/--print), when the agent hits--max-turns, the existing MAX_STEPS injection produces a multi-paragraph "I have reached the maximum number of steps" summary instead of a committed answer. In interactive mode this is fine — a human can react. In headless / scripted use, there is no human; the program's final stdout is unparseable meta-prose.Common situations where this matters:
claude -p "review this PR for security issues"returns a status update instead of a partial reviewgit diff | claude -p "summarize"— works only if the agent always emits the requested shapeWhat's changed
This PR keeps interactive behaviour completely unchanged and adds a separate prompt path for headless mode.
New prompt files:
prompt/max-steps-headless.txt— final-step prompt explicitly instructing the model to emit the answer (not a summary). The wording makes the rationale explicit: an uncertain best-guess is more useful than meta-prose, because the caller cannot ask for retries.prompt/max-steps-headless-prewarn.txt— soft pre-warning that fires one step before the limit when headless andNumber.isFinite(maxSteps). TheisFiniteguard prevents misfiring when no max-turns is set.Wiring:
session/prompt.ts: exportedselectMaxStepsPrompt({ step, maxSteps, headless })— pure selector returning the right prompt for the current state, plumbed into the existing prompt loop.cli/cmd/run.ts: passesheadless: truefrom the-p/--printCLI entry into the session SDK call.session/message-v2.ts: extended theUserzod schema with optionalheadless: boolean, persisted on the user message so resumed sessions retain the flag.sdk.gen.ts+types.gen.ts): updated by the existing@hey-api/openapi-tsstep.Tests
7 new tests in
test/session/max-steps-prompt.test.ts:Number.isFiniteguardResults:
bun run typecheck(opencode + sdk/js): cleanBackwards compatibility
headless?: booleanfield is optional everywhere; older SDK consumers see no breaking change.-pand then resumed (e.g., session reload via the API) still applies headless behaviour, which matches the original intent — the flag persists on the user message.Notes
max-steps-headless.txt; it explicitly tells the model not to emit a meta-summary, with the reasoning (no human to ask follow-ups, an uncertain answer is more useful than no answer).maxStepsis finite; interactive mode never sees it.Files
packages/opencode/src/cli/cmd/run.tspackages/opencode/src/session/message-v2.tspackages/opencode/src/session/prompt.tspackages/opencode/src/session/prompt/max-steps-headless.txt(new)packages/opencode/src/session/prompt/max-steps-headless-prewarn.txt(new)packages/opencode/test/session/max-steps-prompt.test.ts(new)packages/sdk/js/src/v2/gen/types.gen.tspackages/sdk/js/src/v2/gen/sdk.gen.tsSummary by cubic
Headless runs now commit a best-guess answer at
--max-turnsand actually disable tools on the final step. This makes CI/evals reliably parseable and fully addresses #759.headless: truethrough all non-interactive paths:run --command, GitHub, and GitLab commands.headlesson synthetic user messages (post-task summary, compaction replay/continue) so long runs don’t flip back to interactive.json_schema).headlesstoUserand tosession.prompt/session.prompt_async/session.commandinopenapi.jsonand the JS SDK types.{TURNS_REMAINING}substitution; add focused tests for selection, tool-disabling, schemas, and edge cases.Written for commit 78acb0f. Summary will update on new commits. Review in cubic
Summary by CodeRabbit
New Features
Tests