fix(session): bounded retry and safe recovery - #48453
Open
yixiao5428 wants to merge 3 commits into
Open
Conversation
Legacy SessionProcessor could silently accept incomplete provider streams, replay turns that already ran tools or hooks, lose the interruption identity when a retry race or a cleanup fault landed, and repeat overflow recovery without a bound. This adds: - typed classification for incomplete streams (canonical SDK other/raw-undefined marker and clean-EOF settlement rules); detected cases settle fail-closed with error + finish=error - replay vetoes shared by ordinary and incomplete retries (observed tool activity, dispatched text-complete hook, compaction, blocked); mixed failure+interrupt causes never retry and keep both identities - per-attempt ownership: parts, accounting baseline and deferred diff summaries are rolled back before any authorized retry; the original failure survives cleanup faults via combined causes - output-length errors produced from the real step-finish path and classified overflow preserved across the provider-error boundary - reactive overflow recovery admitted at most once per executed prompt loop, with old-history admission and a started-output replay veto - failed turns without a finish no longer restart on loop re-entry, while genuinely new user turns keep working 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
A StructuredOutput capture ran inside the attempt, so it could be captured before the provider terminal arrived; promoting it unconditionally then overwrote that terminal: finish was forced to "stop" on an errored turn and the content-filter/json_schema error promotions were skipped entirely. Gate the promotion on the attempt not having established a terminal outcome: no durable error, and no content-filter finish (the processor sets only the finish there; ContentFilterError is created by the later promotion, so the plain error check is blind to it). Captured output on a clean stop/unknown turn is unchanged. Amend the canonical incomplete-stream oracle to the settlement semantics: finish is "error" with the recognition message, and the durable step-finish part keeps its own "unknown" reason. The message is asserted before any asymmetric matcher, whose write-back was replacing the received string. Co-Authored-By: Claude <noreply@anthropic.com>
These three documented prompt-waiter/write-point behavior that this change explicitly does not cover; the coordinator keeps internal copies. - G6 failed driver cancels and drains actual runner - G6 driver and drain failures remain observable - G8 fatal final write failure preserves primary Cause (with its now unreachable fatal+fault assertion block, which no remaining fixture reaches) The shared driver/delayed/fault machinery stays: the settled-tool G6 controls and the cancel G8 rows still use it. Co-Authored-By: Claude <noreply@anthropic.com>
Contributor
|
Thanks for your contribution! This PR doesn't have a linked issue. All PRs must reference an existing issue. Please:
See CONTRIBUTING.md for details. |
github-actions
Bot
added
the
needs:compliance
Contributor
|
The following comment was made by an LLM, it may be inaccurate: Based on the search results, no duplicate PRs were found for PR #48453. The searches returned the current PR itself as the top result, but no other open PRs addressing the same bounded retry and safe recovery issues for session processing. The related PRs mentioned in the description (#26167, #43881, #33667, #47832, #27254, #45861, #47813, #47797, #47880) are referenced as complementary or previous work, but these are tracked directly in the PR description rather than being open duplicates. No duplicate PRs found |
github-actions
Bot
removed
needs:compliance
Contributor
|
Thanks for updating your PR! It now meets our contributing guidelines. 👍 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Issue for this PR
Closes #48454 — the issue lists the individual symptoms; this PR fixes all five with regression coverage per case.
Type of change
What does this PR do?
Legacy
SessionProcessorhas several failure-handling gaps that compound: incomplete provider streams (finishotherwith no raw reason, or a stream that just ends) are accepted as if they had settled; retries replay turns that already executed tools or dispatched hooks; a retry racing a cancellation, or a failing cleanup write, loses the original interrupt/failure; output-length finishes never produce an error; an overflow can trigger compaction recovery repeatedly because nothing remembers one already ran; and re-entering the prompt loop on a failed turn that has nofinishissues a fresh request instead of surfacing the stored failure.Changes:
error+finish: "error"instead of being acceptedlengthfinishes now produceMessageOutputLengthError, and a classified overflow keeps its classification across the provider-error boundaryfinishstops on loop re-entry while genuinely new user turns keep workingOne deliberate behavior change: an incomplete turn now settles with
finish: "error"(previously it was accepted withunknown), and a failed turn is no longer re-requested on loop re-entry — the existing test that relied on loop-level continuation now passes via the processor's own bounded retry (same two requests, same observable outcome).Related: #26167, #43881 (open, overlapping symptoms — different approach), #33667, #47832 (closed), #27254, #45861, #47813 (merged, complementary), #47797, #47880 (our earlier session fixes).
How did you verify your code works?
bun run typecheckinpackages/llmandpackages/opencodebun testover the six session suites and the llm package: 466 pass / 0 fail / 2 skip, including new regression coverage for each behavior abovepackages/clientgenerate, sdk build): no generated diff; contract-identity tests passScreenshots / recordings
N/A — no UI change.
Checklist