Skip to content

fix(core): honour agent.steps and inject last-step signal in V2 runner#31328

Open
lexlian wants to merge 2 commits into
anomalyco:devfrom
lexlian:fix/v2-step-cap-and-last-step-signal
Open

fix(core): honour agent.steps and inject last-step signal in V2 runner#31328
lexlian wants to merge 2 commits into
anomalyco:devfrom
lexlian:fix/v2-step-cap-and-last-step-signal

Conversation

@lexlian
Copy link
Copy Markdown

@lexlian lexlian commented Jun 8, 2026

Issue for this PR

Closes #30865
Closes #30866

Type of change

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

What does this PR do?

V2's session runner (packages/core/src/session/runner/llm.ts) was hardcoded to a 25-step cap regardless of the agent's steps field, and never warned the model on its final configured turn. V1 reads agent.steps ?? Infinity per iteration and appends a synthetic assistant message from max-steps.txt on the last step — this PR restores that behavior in V2 so the planned V1→V2 cutover does not regress user-facing behavior.

The fix adds a MAX_STEPS_PROMPT constant (inline copy of V1's max-steps.txt), reads the resolved step cap from the agent at the top of run() to use as the for-loop bound, threads a Ref<number> step counter so runTurnAttempt can read the current iteration, and injects the synthetic assistant message when the current step is the last. StepLimitExceededError.limit now reports the resolved cap instead of always 25.

V1 already has the correct behavior; no V1 change is needed.

How did you verify your code works?

  • bun typecheck in packages/core — clean
  • bun turbo typecheck (23 packages) — all pass
  • bun test test/session-runner.test.ts — 95 passed, 0 failed (4 new regression tests + 91 pre-existing)
  • bun test test/session-runner-{model,message,tool-events,tool-registry,recorded}.test.ts — 37 passed
  • git diff --check — clean

The 4 new tests cover:

  1. agent.steps = 3 raises StepLimitExceededError({ limit: 3 }) — regression for llm.ts: MAX_STEPS=25 hard cutoff with no partial-result preservation #30865
  2. No agent.steps override falls back to the 25-step default
  3. agent.steps = 2 injects the max-steps prompt on the final (2nd) turn — regression for llm.ts: V2 runner dropped V1's 'last step' signal — AI never warned to finalize work #30866
  4. agent.steps = 5 does NOT inject the signal on non-final turns

Checklist

  • I have tested my changes locally
  • I have not included unrelated changes in this PR

V2's session runner was hardcoded to 25 steps regardless of the agent's
configured `steps` field, and never warned the model on its final
configured turn. V1 (`packages/opencode/src/session/prompt.ts:1314-1315,
1426`) does both — read `agent.steps ?? Infinity` per iteration and
append a synthetic assistant message from `max-steps.txt` on the last
step. This restores V1 parity so the planned V1→V2 cutover does not
regress user behavior.

## What changed

- `packages/core/src/session/runner/llm.ts`
  - Add `MAX_STEPS_PROMPT` constant — inlined verbatim copy of V1's
    `packages/opencode/src/session/prompt/max-steps.txt`.
  - At the top of `run()`, read the per-agent cap
    (`runAgent.info?.steps ?? MAX_STEPS`) once and use it as the for-loop
    bound. Previously hardcoded `MAX_STEPS = 25`.
  - Thread a `Ref<number>` (closure-scoped, reset on each `run()` call)
    so `runTurnAttempt` can read the current iteration index.
  - At the request build site in `runTurnAttempt`, read the index and
    re-read the agent's `steps` per call (to pick up mid-session swaps),
    then append the synthetic assistant message when
    `currentStep + 1 >= maxStepsForAgent`.
  - `StepLimitExceededError.limit` now reports the resolved cap (was
    always `25`).

- `packages/core/test/session-runner.test.ts`
  - 4 new tests in the existing `SessionRunnerLLM` describe block:
    1. `agent.steps = 3` raises `StepLimitExceededError({ limit: 3 })`
       (regression for anomalyco#30865).
    2. No `agent.steps` override → still defaults to 25.
    3. `agent.steps = 2` → 2nd (final) request's last message contains
       "CRITICAL - MAXIMUM STEPS REACHED" (regression for anomalyco#30866).
    4. `agent.steps = 5` → non-final turns do NOT carry the signal.

## Why

- The hardcoded cap ignored user-configured step limits (anomalyco#30865).
- Without the last-step signal, the model emits tool calls on the
  cutoff step that are killed mid-flight, wasting the final turn (anomalyco#30866).
- Both are V1↔V2 architecture gaps that block the planned V1→V2
  cutover. V1 already has the correct behavior; V2 now matches.

## V1-parity notes

- V1 reads `agent.steps ?? Infinity` per iteration; V2 reads once in
  `run()` for the for-loop bound and re-reads in `runTurnAttempt` for
  the `isLastStep` check. If the agent is swapped mid-session, the
  for-loop bound stays at the original cap (acceptable: it is a safety
  net; per-call cap detection is the actual enforcement).
- V1 reads `agent` from the last user message; V2 reads from the
  session. Minor known deviation; per-message agent switching in V2 is
  not yet supported.
- V2 retains its 25-step default when `agent.steps` is unset. V1 has no
  default cap (`?? Infinity`); V2's 25-step default is a strict
  improvement and is preserved.

## Verification

- `bun typecheck` in `packages/core` — clean
- `bun turbo typecheck` (23 packages) — all pass
- `bun test test/session-runner.test.ts` — 95 passed, 0 failed
  (4 new + 91 pre-existing)
- `bun test test/session-runner-model.test.ts
         test/session-runner-message.test.ts
         test/session-runner-tool-events.test.ts
         test/session-runner-tool-registry.test.ts
         test/session-runner-recorded.test.ts` — 37 passed, 0 failed
- `git diff --check` — clean

Closes anomalyco#30865
Closes anomalyco#30866
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

1 participant