Skip to content

fix(opencode): discard a failed attempt's output before retrying the stream - #51548

Open
iceteaSA wants to merge 1 commit into
anomalyco:devfrom
iceteaSA:stream-retry
Open

iceteaSA wants to merge 1 commit into
anomalyco:devfrom
iceteaSA:stream-retry

Conversation

@iceteaSA

Copy link
Copy Markdown

Issue for this PR

Closes #44894
Closes #37852

Related: #48454 / #48453 cover this among a wider set of changes (+6.8k lines); this PR is the retry part alone. The empty-stream half takes the same approach as #43881 (closed by the stale-PR bot), reusing ResponseStreamError.

Type of change

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

What does this PR do?

SessionProcessor wraps the whole stream in Effect.retry(SessionRetry.policy(...)). Each attempt resets currentText and reasoningMap and publishes new parts, but nothing removes what a failed attempt already wrote. So when a stream fails after producing output (a dropped connection, a 5xx mid-stream):

This PR:

  1. Records which parts exist when each attempt starts. Before a retry, it removes the parts the failed attempt created through session.removePart (so clients get the normal removal events), and restores the assistant message's finish, cost, tokens and the file snapshot to their values at the start of the attempt. Parts from earlier completed steps are not touched.
  2. Does not retry if a tool call from the failed attempt had started running (any tool part past pending). The turn ends with the error, as it does today for non-retryable errors.
  3. Treats a stream that ends with no finish reason, no usage, and no text or tool output as a retryable ResponseStreamError (Aborted provider stream recorded as clean stop (finish=unknown, zero usage, no text) — subagent returns empty with no error #37852). Today that is stored as a normal finish: "unknown" stop, so a dropped connection looks like the model said nothing. Streams that end without a finish reason but did produce text or tool calls behave as before.

The snapshot restore matters when a failed attempt got as far as step-finish: that clears ctx.snapshot and writes a patch part, and after the discard removes the patch, re-tracking at the retry's step-start would take the snapshot after the edits and drop them from the turn's patch.

Behavior change: for an empty finish-less stream, the processor now retries within the turn with the normal backoff and ends with an error once retries run out, instead of the prompt loop starting another turn. The existing loop continues when finish is unknown test still passes; its second request now comes from the processor's retry.

How did you verify your code works?

  • New tests in test/session/processor-effect.test.ts, each shown failing before the change: duplicate text after a retry, removal events for the failed attempt's parts, no retry after a tool ran (the tool runs once, not twice), an empty stream retried and then surfaced as an error, and a failed attempt's patch surviving the retry. Retry waits use TestClock, so no test sleeps for real.
  • Mutations that fail the tests: removing the discard, allowing a retry after a tool ran, removing the empty-stream check, discarding only text (keeping reasoning), and not restoring the snapshot.
  • bun test test/session (424 pass, 0 fail) and bun typecheck in packages/opencode.

Screenshots / recordings

Not a UI change.

Checklist

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

…stream

The processor retries a whole stream when it fails with a retryable
error, but it never removed what the failed attempt had already
written. A retry after partial output left duplicate text and
reasoning, parts that were never closed, and could run a tool the
failed attempt had already started a second time.

Before a retry, remove the parts the failed attempt created and restore
the message's finish, cost, tokens and file snapshot to what they were
when the attempt started. If a tool from that attempt had started
running, don't retry; end the turn with the error.

A stream that ends with no finish reason, no usage and no text or tool
output is now a retryable stream error instead of a normal stop, so a
dropped connection is retried and, once retries run out, shows an
error.
@github-actions

Copy link
Copy Markdown
Contributor

The following comment was made by an LLM, it may be inaccurate:

This branch has not been deployed

No deployments
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