fix(session): cap retries on repeated unrecognized finish reason - #49418
alparslanozturk wants to merge 1 commit into
Conversation
SessionPrompt.run's loop-exit check treats an "unknown" finish reason the
same as "tool-calls" (i.e. "there might be more work to do, keep going"),
which is the right call for a one-off glitch (a provider that drops its
finish signal on an otherwise-fine stream - see the existing "loop
continues when finish is unknown" test). But that retry had no upper
bound: if a provider's finish_reason never maps to one of our recognized
FinishReason values, every single attempt reproduces the same "unknown"
finish, and the loop re-sends the same request forever with no backoff.
Cap consecutive unresolved ("unknown", no tool calls) attempts at
MAX_UNKNOWN_FINISH_RETRIES before giving up, instead of retrying
indefinitely.
Fixes anomalyco#49414.
|
This PR doesn't fully meet our contributing guidelines and PR template. What needs to be fixed:
Please edit this PR description to address the above within 2 hours, or it will be automatically closed. If you believe this was flagged incorrectly, please let a maintainer know. |
|
The following comment was made by an LLM, it may be inaccurate: Based on the search results, I found some related PRs addressing session retry logic: Related PRs (not duplicates of #49418):
These are related to the same session/retry domain but appear to address different specific scenarios. PR #49418 is the current PR, which uniquely addresses the cap on consecutive unknown finish reasons specifically. No exact duplicates found. No duplicate PRs found |
|
This pull request has been automatically closed because it was not updated to meet our contributing guidelines within the 2-hour window. Feel free to open a new pull request that follows our guidelines. |
Fixes #49414.
What
SessionPrompt.run's loop-exit check treats an"unknown"finish reasonthe same as
"tool-calls"— i.e. "there might be more work to do, keepgoing." That's the right call for a one-off glitch: the existing test
"loop continues when finish is unknown"models a provider that ends itsstream without a proper finish signal, and the loop correctly retries once
and recovers.
But that retry has no upper bound. If a provider's
finish_reasonnevermaps to one of opencode's recognized
FinishReasonvalues (stop,length,tool-calls,content-filter,error) — which happensdeterministically with some self-hosted/vLLM-served OpenAI-compatible
backends — every single attempt reproduces the same
"unknown"finish,and the loop re-sends the same request forever with no backoff, no cap,
and no visible error. Reported in #49414 with a live repro (86–138
requests/10s against a real endpoint).
This caps consecutive unresolved (
"unknown", no tool calls) attempts atMAX_UNKNOWN_FINISH_RETRIES(3) before giving up, instead of retryingindefinitely. Normal finish reasons (
stop,length, etc.) are unaffectedand still exit the loop immediately, same as before.
Testing
asserts the loop gives up after 4 total attempts (1 + the retry cap)
instead of consuming the whole queue or hanging.
after the fix.
bun test test/session/ test/agent/— 463 pass, 0 fail (no regressions,including the existing
"loop continues when finish is unknown"test).bun run typecheck— clean.oxlinton both changed files — 0 errors (pre-existing warningselsewhere in the file are untouched by this diff).
Marking as draft since this is my first PR here — happy to adjust the
retry count, logging, or approach if you'd prefer something different
(e.g. a time-based cap instead of a count-based one, or surfacing a
distinct error/finish state when the cap is hit rather than reusing
"unknown").