Skip to content

fix(session): abort-safe stream processing and resilient cleanup - #20101

Closed
ESRE-dev wants to merge 1 commit into
anomalyco:devfrom
ESRE-dev:pr/abort-safe-stream
Closed

fix(session): abort-safe stream processing and resilient cleanup#20101
ESRE-dev wants to merge 1 commit into
anomalyco:devfrom
ESRE-dev:pr/abort-safe-stream

Conversation

@ESRE-dev

@ESRE-devESRE-dev commented Mar 30, 2026

Copy link
Copy Markdown

Issue for this PR

Closes#20094

Type of change

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

What does this PR do?

Fixes three tightly related session processor correctness problems:

  1. Abort-safe stream iteration

    • Replaces the blocking stream iteration pattern with an abort-race loop so cancellation can interrupt pending iterator waits immediately.
  2. Resilient cleanup finalization

    • Wraps cleanup-time snapshot patching in error handling, so snapshot failures do not skip message/session finalization.
  3. Abort-aware doom-loop permission wait

    • Races doom-loop permission prompts against abort so cancellation cannot be blocked by a pending prompt.

This PR is intentionally scoped to stream-loop and cleanup correctness in session/processor.ts.

Related (not duplicates):

How did you verify your code works?

  • Added and ran packages/opencode/test/session/iterator-cleanup.test.ts to validate iterator abort and cleanup behavior.
  • Ran targeted session reliability tests in the same area (cancel/watchdog/processor paths).
  • Manually verified finalization still runs when snapshot patching throws.

Screenshots / recordings

Not a UI change.

Checklist

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

If you do not follow this template your PR will be automatically rejected.

- Replace for-await with Promise.race abort-race pattern to prevent
hangs when abort fires during outstanding tool results
- Wrap post-loop Snapshot.patch in try/catch to prevent zombie sessions
when snapshot cleanup fails
- Make doom loop permission prompt abort-aware
- Add sweep() for recursive child session abort on cancellation
@github-actionsgithub-actionsBot added needs:issue needs:compliance This means the issue will auto-close after 2 hours. labels Mar 30, 2026
@github-actions

Copy link
Copy Markdown
Contributor

Thanks for your contribution!

This PR doesn't have a linked issue. All PRs must reference an existing issue.

Please:

  1. Open an issue describing the bug/feature (if one doesn't exist)
  2. Add Fixes #<number> or Closes #<number> to this PR description

See CONTRIBUTING.md for details.

@github-actionsgithub-actionsBot removed needs:compliance This means the issue will auto-close after 2 hours. needs:issue labels Mar 30, 2026
@github-actions

Copy link
Copy Markdown
Contributor

Thanks for updating your PR! It now meets our contributing guidelines. 👍

@ESRE-dev

Copy link
Copy Markdown
Author

Closing: Obsoleted by upstream Effect/Stream migration

This PR is no longer needed. The upstream codebase has undergone a major architectural migration to Effect/Stream/Runner that addresses all the race conditions this PR targeted. Our PR branches forked from 0bbf26a1ce (Mar 20) and were pushed Mar 30 — the same day the final migration commit landed, making them effectively obsolete before they were even reviewed.

Upstream commits that supersede this PR

CommitDateDescription
860531c275Mar 28refactor(session): effectify session processor (#19485) — Replaces raw for await iteration with Effect/Stream (Stream.tapStream.runDrain), making stream interruption native via Fiber.interrupt. The cleanup() function handles all dangling state (currentText, reasoningMap, pending tool parts).
c5442d418dMar 30refactor(session): effectify SessionPrompt service (#19483) — Introduces the Runner state machine with SynchronizedRef-based atomic state transitions. Runner.cancelFiber.interrupt propagates cleanly through the stream processing chain.

What this PR did vs what upstream now handles

Our fixUpstream solution
Promise.race([iter.next(), aborted]) to prevent for await hangEffect/Stream with Stream.runDrain — interruption is native via Fiber.interrupt
abortChildren() to kill child tool fiberscleanup() in processor.ts marks all pending tools as error on abort
sweep() for dangling statecleanup() handles currentText, reasoningMap, and pending tool parts

Related: #20095, #20094
Superseded by: #19485, #19483

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[BUG] Stream iteration hangs on abort; cleanup failure leaves zombie sessions

1 participant

@ESRE-dev