Could a sticky max-tokens state cause the agent loop to skip a required tool-result step?

[ShikangL]

Important

While reading the DSH 0.1.1-rc.1 code, I noticed a potential control-flow issue in the agent loop. A sticky max-tokens value may cause a later tool-calling step to end the turn before the model can read the tool result, even though step() returns null to request another step.

The same logic appears to still be present in DSH 0.1.1-rc.2.

The relevant code is in packages/core/agent-loop/src/agent.ts, around lines 285–299.

The code intentionally keeps max-tokens sticky at the turn level. Once a step reaches the token limit, a later step that completes normally does not downgrade the final turn outcome. Lines 285–290 implement this by retaining the earlier max-tokens value in turnEnds.

However, lines 295–299 also use turnEnds to decide whether the step loop should stop.

The potential failure sequence is:

1. Step 1 ends with max-tokens. Before the turn stops, a steer message causes the same turn to continue.

2. Step 2 then makes an ordinary tool call. The tool executes successfully, and step() returns null, indicating that another step is required so the model can read the tool result.

3. Because turnEnds already contains max-tokens, the null returned by Step 2 does not replace it. If the tool result is stored in the Session history but the tool does not add any additionalContexts, inbox.nextStep may be empty.

4. The condition on line 299 would then still see a truthy turnEnds value and an empty nextStep inbox, causing the loop to break immediately after Step 2. The tool result would exist in the Session history, but the model would not receive the next step needed to read it.

Is there another mechanism that guarantees continuation in this sequence, or can the sticky turn-level max-tokens state incorrectly terminate a step that explicitly returned null?

[nokkies]

Answering the question directly: no, nothing else guarantees continuation, and the sequence you describe reproduces.

I built the case you reasoned out — a step that ends max-tokens, a steer carrying the turn forward, then an ordinary tool call — and got two model calls where there should be three. The tool result is committed to the session log and no step ever reads it. Your reading of lines 285–299 is exactly right.

Why

One variable is answering two different questions.

step() returns null to mean "another step is required so the model can read the tool result". That null is the only continuation signal the loop has. It also returns a reason, which is what the turn should report, and max-tokens is deliberately sticky there so a later normal step cannot downgrade a truncated turn.

Because both ride turnEnds, an earlier ceiling suppresses the null. The loop then breaks in two places, not one — the stop check you identified, and also at the top of the next iteration:

if (turnEnds && decision.messages.length === 0) break

After a tool call decision.messages is typically empty (the result is in history, not the inbox), so even fixing only the stop check would leave the turn ending there.

Fix shape

Split the two roles. Keep turnEnds as the reported outcome, sticky as it is now, and introduce a separate variable holding the step's own signal:

let stopping: StepEndReason | null = null
...
const stepEnd = await this.step(decision.assembly)
stopping = stepEnd
if (stepEnd !== null && (turnEnds === null || turnEnds.kind !== 'max-tokens')) {
  turnEnds = stepEnd
}

then drive both break conditions off stopping rather than turnEnds. The turn still reports max-tokens; only the continuation decision changes.

Reachability, and who is most exposed

It needs a steer to carry the turn past the truncated step, which the suite's own sticky-max-tokens test already demonstrates is an ordinary thing to happen.

Worth flagging for anyone running a fork with context-overflow recovery: if you clamp maxTokens downward on recovery, you are manufacturing exactly the max-tokens step endings this window needs, so you will hit it far more often than stock defaults do. That is how I came to test it.

Verified against the full suite after the change: 844 files, 14161 tests, zero failures.

[ShikangL]

Thank you for taking the time to reproduce this and explain the root cause so clearly. The fix sketch and test results are very helpful!

[Jstn-1g]

I built a bounded reference implementation against official master at b150a551b8d465e31e418e1b2eaf5e79bbb7d28e.

Thank you @ShikangL for the exact failure sequence, and @nokkies for independently reproducing it and publishing the split-variable fix sketch. This implementation turns that analysis into a reviewable patch and cross-SDK receipt.

The root issue is the conflation of two values: the durable turn outcome (turnEnds, where max-tokens must remain sticky) and the current step's completion/continuation signal. The patch keeps the final turn outcome sticky while using a step-local stepEnd for the default stop checks and agent/turn-stopping. Error, rejection, cancellation, and disposal precedence remain unchanged.

Evidence:

• the focused regression fails before the fix with two model requests instead of three;
• after the fix, the exact same-turn sequence is max-tokens → steer → todo_write → third model request reads the real tool result, with no premature agent/turn-stopping;
• keyless assembled receipts cover ACP, the TypeScript SDK, and the packaged Python SDK, all pinning one turn, three balanced steps, and final max-tokens;
• agent-loop suite: 336/336; documentation gates: 28/28; typecheck, lint, and pre-push build/contracts checks pass.

Reference branch: https://github.com/Jstn-1g/deepseek-harness/tree/reference/discussion-4622-max-tokens-tool-continuation

Exact commit: Jstn-1g@91fa9ba

This is a verified reference implementation, not a claim of upstream adoption.

[ShikangL]

Thank you for turning this into a concrete, verified reference implementation. I really appreciate the thorough testing and all the effort you put into it!