[RaghwendraSingh]

Issue for this PR

Closes #1634

Type of change

• Bug fix
• New feature
• Refactor / code improvement
• Documentation

What does this PR do?

An agent with a steps cap is meant to end with a text answer. On the Altimate gateway it ended with no answer instead, and a capped subagent handed its parent an empty result.

1. The request at the cap failed (session/prompt.ts).
At step N the loop appended MAX_STEPS as a trailing assistant message and still sent every tool, with no toolChoice. On the gateway that request failed with UnknownError "Could not get a response from the agent" and 0 tokens, so the session ended without an answer. In our headless runs these were the only two gateway errors in about 1,000 requests, and the only two requests sent at the cap. The answer-echo path a few lines above already notes that a conversation must end on a user turn.

Fix: at the cap the instruction is a user turn, sent with no tools and toolChoice: "none", the same toolless shape the answer echo, compaction and title requests use; llm.ts then flattens historical tool calls, which the gateway requires for a toolless request. A json_schema request keeps its tools at the cap, because its answer is the StructuredOutput tool call.

2. The cap counted iterations, not requests (session/prompt.ts).
step also increments on loop iterations that run a compaction or a subtask and continue. A capped agent that compacted twice reached its cap several requests early. The cap now uses a separate modelSteps counter that advances only on the normal model-call path. step is unchanged for everything else that reads it (title, telemetry, plan-mode checks).

3. An errored or silent child returned an empty result (tool/task.ts, session/task-output.ts).
The parent received only the child's last text part, so a child that failed at its cap produced <task_result>\n\n</task_result> with status completed, which reads as "done, nothing found". Now, when the child produced no final text, the result says why ("Task stopped with an error before giving a final answer (UnknownError: …)" or "Task ended without a final text answer") and carries the child's last written text if there is one. The <task_result> tag itself is unchanged, since cli/cmd/run/tool.ts parses it.

Not in this PR:

• The step count still restarts when a child is resumed with task_id. Making it persist per session would also cap interactive sessions for good after N steps in total, so that belongs with a per-run budget that counts child sessions.
• run --max-turns: passing the remaining turns into the loop would make the last allowed turn the toolless answer, but it changes the documented --max-turns contract (N work turns, then an abort and one toolless finalization reply, exit 1) that budget-finalization.test.ts pins, so it is left for a separate change.

How did you verify your code works?

• New prompt.test.ts cases (the step cap asks for a final text answer …), plain and with a usage-triggered compaction, on an OpenAI-compatible mock: the last request ends with a user message carrying MAXIMUM STEPS REACHED and declares no tools; no earlier request carries it; with a compaction the agent still gets its full step count. Both fail on main (role assistant; one request short with the compaction).
• New test/tool/task-empty-result.test.ts: an errored child with earlier text, a child with no text, and a normal answer. The first two fail on main.
• bun test test/session test/tool test/agent: 2,131 pass, 0 fail. test/cli: the same 2 --time-budget failures in json-output.test.ts as main. budget-finalization.test.ts: 46/46. An apply_patch case timed out once under load and passes alone (27/27).
• Typecheck clean; script/check-tracker-leaks.ts and analyze.ts --markers --strict pass.

Cost side effects

None added. The toolless final request sends no tool schemas, so it does not reuse the cached tools prefix; it is one request per capped session, and it replaces a request that failed and a parent turn that continued without the child's findings.

Screenshots / recordings

N/A

Checklist

• I have tested my changes locally
• I have not included unrelated changes in this PR
• I checked for cost side effects (model calls, queries or cloud resources that grow with user data) and described them, or wrote "none"

🤖 Generated with Claude Code

[coderabbitai]

Important

Draft PR not reviewed

Draft PRs are not automatically reviewed by default.

• Trigger a manual review

To automatically review draft PRs, update your CodeRabbit configuration:

reviews:
  auto_review:
    drafts: true

• Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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.

Share

• X
• Mastodon
• Reddit
• LinkedIn

[github-actions]

Thanks for your contribution!

This PR doesn't have a linked issue. All PRs must reference an existing issue.

Please:

1. Open an issue describing the bug/feature (if one doesn't exist)
2. Add Fixes #<number> or Closes #<number> to this PR description

See CONTRIBUTING.md for details.

[github-actions]

👋 This PR was automatically closed by our quality checks.

Common reasons:

• New GitHub account with limited contribution history
• PR description doesn't meet our guidelines
• Contribution appears to be AI-generated without meaningful review

If you believe this was a mistake, please open an issue explaining your intended contribution and a maintainer will help you.

[github-actions]

Thanks for updating your PR! It now meets our contributing guidelines. 👍

[RaghwendraSingh]

Gateway run: the step cap now ends with an answer

Four short headless runs on the real gateway (model altimate-backend/altimate-default), in a throwaway project, with the task "run echo step N for N = 1 to 30, one call per step, then reply DONE". The steps pair uses a custom primary agent with steps: 5 (bash only); the turns pair uses the default agent with --max-turns 5. main = origin/main at bdb7dcc7b9; fix = a local branch stacking this PR with #1631, #1639, #1641 and #1643.

run	build	requests	billable tokens	how it ended
agent steps: 5	main	4	6K	rc=1, gateway error Could not get a response from the agent, no final text
agent steps: 5	fix	5	8K	rc=0, final text: what was done (steps 1–5), what remains, no DONE claimed
--max-turns 5	main	6	118K	rc=1, final text produced by the existing finalization turn
--max-turns 5	fix	6	118K	rc=1, final text produced; unchanged by this PR, as intended

The main step-cap failure is the same error that ended two of our long runs at their cap (the cap request was sent as a trailing assistant message with tools still declared). With this PR the cap request is a user message with no tools, and the model answers it.

The --max-turns rows show that the finalization turn already produces an answer at this context size on both builds; the no-answer cases seen at --max-turns in longer runs (~150K context) are therefore not a missing final turn and are tracked separately.

[sahrizvi]

Approved. No critical or major finding is open, and the fix does what it claims on the main paths: the request at the cap is a trailing user message with no tools, only model requests count toward the cap, and an empty child result now says why.

Five minors are posted inline. None of them blocks:

• lastTaskText loses text after a compaction in the child: edge input, the error reason still reaches the parent.
• A tool call at the cap keeps the loop going with no bound: edge input, and main loops the same way while actually running the tools.
• An errored child with a closed text block reads as an answer: pre-existing, safe in a follow-up.
• A request queued after the cap gets no tools: edge input, the model can still answer.
• Test gaps on those paths: test-only.

Tests at this head against its base: test/session, test/tool and test/agent show the same 2 failures on both sides (scratch-store tests), and the only difference in counts is this PR's 5 new tests. See the PR comment for nits, sibling PR notes and what was checked.

[sahrizvi]

Minor: after a compaction in the child this loses all text, and the parent is told "It wrote no text".

After a compaction the newest user message is the compaction continuation. Post-compaction assistants carry parentID = continuation, so this filter skips them, and line 30 then breaks at that continuation, so text under the original prompt is never reached.

I ran it through the real prompt loop on a mock provider: the child writes "Found 3 candidates so far.", a usage-triggered compaction runs, and the next request returns 400. lastTaskText returns undefined, so emptyTaskStatus says "It wrote no text". The same run without the compaction returns the text. The PR says the result "carries the child's last written text if there is one".

Suggested fix: walk past user messages that carry a compaction part or are synthetic, stop only at the task's prompt id, and drop the parentID equality check.

Not blocking: it needs a compaction, then an error, with no later text. The error reason still reaches the parent, and the wording errs toward "unfinished".

[sahrizvi]

Minor: a tool call in answer to the toolless request keeps the loop going, with no bound.

If the provider returns a tool call anyway, the repair path maps it to invalid, the tool part errors with "Model tried to call unavailable tool 'invalid'", finish stays tool-calls, and the loop sends the same toolless request again. With steps: 3 on a mock that always returns a tool call, I saw 2 normal requests and then 10 capped requests, until the mock stopped. Repeating the identical call gives the same 12 requests, so the doom-loop guard does not end it.

Base also makes 12 requests, and there the tools really run, so this PR is an improvement, but the cap still does not end the run.

Suggested fix: after a textFinalStep request, break whatever the finish reason, and record an error if there is no text.

Not blocking: edge input (a provider returning tool calls with none declared), and main has the same loop in a worse form.

[sahrizvi]

Minor: an errored child with a closed text block still reads as an answer.

status depends only on text.trim(). With a scripted stream of text-start, text-delta, text-end, error, result.info.error is set and the parts hold "Let me check the remaining tables.". The parent gets that narration inside <task_result> with no mention of the error. On an OpenAI-compatible stream that dies before the text block closes, the text is blanked to "", so that case does get the new status. This needs a provider that closes blocks before the failure.

Suggested fix: emit the reason whenever result.info.error is set, and keep text below it.

Not blocking: pre-existing on main, and the PR's claim is about empty results, so this is safe in a follow-up.

[sahrizvi]

Minor: a user request that joins the running loop after the cap gets no tools.

modelSteps lives for the generation and does not reset when a new request joins (where currentTurn changes). With steps: 2 and a request queued during the capped request, the follow-up at this head goes out with 0 tools and MAX_STEPS as the trailing user message, after the real question. At base it went out with all 94 tools and the assistant MAX_STEPS.

MAX_STEPS says "Tools are disabled until next user input", and a follow-up sent after idle gets a fresh count, so the outcome depends on timing. The cap text on that request is pre-existing; losing the tools is new.

Suggested fix: reset modelSteps where currentTurn moves to a new request.

Not blocking: edge input (a capped agent plus a follow-up queued in that window), and the model can still answer.

[sahrizvi]

Minor: the new tests miss the paths above, and this mock hides the compaction case.

The new tests do what they claim. Run against base, the 4 failing cases fail for the stated reasons (role assistant, 3 requests instead of 4, empty <task_result>), and the normal-answer case passes on both.

This mock gives every assistant the prompt's parentID, which real history does not do after a compaction, so lastTaskText never sees that shape. Not covered: a tool call at the cap, json_schema at the cap, a compaction followed by lastTaskText, a queued follow-up after the cap, and an error alongside text.

Not blocking: test-only.

[sahrizvi]

Notes from the review that don't belong on a single diff line, plus the nits.

Nits

• modelSteps++ runs before the newerUserRequest continue at session/prompt.ts:2132, so that iteration counts without a request. It needs an analyst with a configured steps, a committed answer, and a request landing in that window.
• errorText passes the provider's raw message to the parent, without the masking and 300-character slice telemetry applies to the same message (lastMessageError). It stays in the same user's session, so this is not a new trust boundary.
• No machine-readable flag: an abort sets metadata.interrupted, but an errored or silent child returns normal metadata and tool status completed. The parent model can read the text; the TUI, tracing and run JSON can't tell the difference. Consider something like metadata.incomplete: true.
• tool/task.txt item 5 still says agent outputs "should generally be trusted" and doesn't mention the new status lines.
• steps: N means N model requests: N-1 with tools, then the answer. That matches main, but the docs only say "Max agentic iterations". steps: 1 now sends no tools at all. Worth one line in docs/docs/configure/agents.md.
• json_schema at the cap keeps all tools with toolChoice: "required" while MAX_STEPS says to make no tool calls. The wording is pre-existing, and a mock run still returned StructuredOutput.
• The inner altimate_change block at session/prompt.ts:2051 sits inside one whose comment describes only the echo. Also, analyze.ts --markers --strict reports "No upstream-shared files modified" here, so it isn't checking these files.

Sibling PRs

• fix(tool): run background subagents without blocking the parent when the flag is on #1643 re-indents the same tool/task.ts block into a run() closure and edits the same import line, so this is a real textual conflict. Whichever lands second has to carry status into run().
• fix(session): keep the cached system prompt prefix stable across steps #1631 edits the environment line of the same processor.process call, about 20 lines above, with no overlap. fix(session): compact once per overflow and list shell-written files in the ledger #1639 sets lastFinished earlier in the loop and fix(session): drop the clarification option from the run-mode nudge and apply the completion contract to every agent in run mode #1641 changes completionInstruction. Neither overlaps or undoes this.

Checked, no finding

• Toolless shape: at the cap the wire request has no tools and no tool_choice, flattened history, and a trailing user message, the same shape as compaction and title requests.
• An empty answer at the cap stops the loop after 3 requests with steps: 3.
• Step accounting: compaction and subtask iterations don't count, retries stay inside one process() call, and a non-retryable error stops the loop.
• --max-turns: run.ts and run-accounting.ts are untouched. By code reading, a cap answer counts as a turn and satisfies answered(), so it gets no extra finalization, and a silent one at the limit gets the existing single finalization.
• explore and general have no steps value, so they are never capped. All subagents share the same status path, and the <task_result> regex in cli/cmd/run/tool.ts still matches.

Test runs (local, at this head and its base)

• test/session, test/tool, test/agent except answer-commit.test.ts: head 2,042 pass, base 2,037 pass. Both show the same 2 failures ("four concurrent scratch tools persist BigInt metadata and complete the next model turn" and "removing a session closes only its scratch store") and the same unhandled error between tests.
• test/agent/answer-commit.test.ts plus test/cli/run/budget-finalization.test.ts: base 133/133. Head 132/133 on that run, with one --time-budget 5 case failing on a heavily loaded machine. That test sets no steps cap, and budget-finalization.test.ts then passed 46/46 three times in a row at both commits.
• CI TypeScript job at this head: 19,870 pass, 0 fail.

Not verified

• The real-gateway table in the PR comment ran on a stack of this PR with fix(session): keep the cached system prompt prefix stable across steps #1631, fix(session): compact once per overflow and list shell-written files in the ledger #1639, fix(session): drop the clarification option from the run-mode nudge and apply the completion contract to every agent in run mode #1641 and fix(tool): run background subagents without blocking the parent when the flag is on #1643, not on this PR alone, and I didn't reproduce it. Everything above used mock providers, so the claim that the gateway accepts this shape rests on that stacked run. A run of this PR alone would close that gap.