Skip to content

fix: preserve WebSocket bursts while consumers are paused - #49597

Open
maximilliangrand wants to merge 1 commit into
anomalyco:v2from
maximilliangrand:codex/websocket-burst-buffering
Open

maximilliangrand wants to merge 1 commit into
anomalyco:v2from
maximilliangrand:codex/websocket-burst-buffering

Conversation

@maximilliangrand

Copy link
Copy Markdown

Issue for this PR

Closes #49523

Type of change

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

What does this PR do?

A paused consumer receiving a burst of small WebSocket deltas fails at the 129th frame. A localhost regression reproduces this in the socket adapter; fixing that layer alone still overflows the session queue.

The socket adapter now admits bursts within a 64 MiB ingress accounting budget and a 4,096-frame ceiling, preserving the 16 MiB individual-frame guard. The async session reader waits for its consumer instead of failing a second queue. Both streams take one frame at a time: Effect's batched queue drain would otherwise release byte accounting early and hide frames after a terminal event.

Actual overload remains a typed error and closes the connection. Cancellation and post-terminal data still poison it; no silent drops or automatic retries. The ingress budget is not a total process-memory bound.

How did you verify your code works?

On macOS arm64 with the repository's Bun 1.4.2:

  • Full AI suite: 1,404 passed, 28 skipped.
  • Core transport, real WebSocket, checkpoint and session-runner suites: 254 passed.
  • Both affected package typechecks, repository-wide lint, changed-file formatting and diff checks passed.
  • New tests cover a paused 1,024-frame localhost burst and connection reuse, byte/count overload, budget reclamation, exact/oversized frame limits, cancellation and trailing data after completion.

Full bun run check reaches unrelated workspace typechecks but cannot finish with this filtered dependency installation (@tsconfig/node22 missing in console-core). No manifest or lockfile changes. CI is still needed for the full workspace/platform matrix.

AI-assisted implementation and independent review.

Screenshots / recordings

Not applicable; transport behavior only.

Checklist

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

@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.

@maximilliangrand

Copy link
Copy Markdown
Author

The issue section already contains Closes #49523; this PR targets v2 as required by CONTRIBUTING.md. The needs:issue warning appears to come from the default-branch standards workflow checking only closingIssuesReferences, which is empty for this non-default target. The version of that workflow on v2 already has a PR-body fallback for exactly this case.

The test and check workflow runs are currently action_required with no jobs started. Could a maintainer approve the fork runs and clear the incorrect issue label? Local Bun 1.4.2 test/typecheck results and the full-workspace validation limitation are recorded in the PR body.

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant