Skip to content

fix(responses): bound closing handshakes for non-reused upstream WebSockets - #7681

Closed
luvs01 wants to merge 12 commits into
lidge-jun:devfrom
luvs01:fix/private-ws-close-grace
Closed

luvs01 wants to merge 12 commits into
lidge-jun:devfrom
luvs01:fix/private-ws-close-grace

Conversation

@luvs01

@luvs01 luvs01 commented Oct 24, 2026 •

Copy link
Copy Markdown
Collaborator

Summary

Bound the closing handshake for a WebSocket that will not be reused. Detach the logical session and its existing listeners before starting a graceful close; if a native socket remains open after the one-second grace timer, terminate that specific socket.

Keep retained-session disposal immediate and successful pooled release reusable. Clean up the close timer/listener on early close, timeout and exceptions. A fake without native termination receives only a close attempt, avoiding a leftover listener/timer. The timer is not a real-time upper bound under event-loop stalls.

Update the owning wire contract and add native loopback stalled-peer regressions plus receipt, cancellation and ownership checks.

Verification

Candidate 90ad57a2a23e4a490426e8330c06189c9bccb688 normally merges dev@0aeaf0cb6b0a7c2f117d8c75f8fa396cf2ea6e39 and shared #7627 (f011a94be1b468b7b785cb54976d4cb4a9b6a563). Runtime and testRunnerBun are both pinned to 1.4.4. The PR remains based on dev.

Current candidate validation: Official native Windows Bun 1.4.4, CI=true bun test --isolate --timeout 60000, fresh processes for the six focused WebSocket files: 215 passed / 0 failed / 1 existing conditional skip / 1,418 assertions. Reference TypeScript, official Structure and skill-surface checks, and all 14 changed-file privacy/size/diff checks passed. All ten units returned native exit 0 and complete child output, with no dependency reinstall. This focused result does not settle the earlier broad-run failures described below.

All three change-owned blobs remain unchanged; the 11-file Bun baseline is shared with #7627.

Previously reported at 91c871e514a7b97f18645c49bcf3b8eedc8a612e: official Bun 1.4.4 on Windows passed 215 tests / 1 conditional skip / 0 failures / 1,418 assertions across handshake validation, dispatch receipts, upstream reuse, pool contention, handshake and upstream tests. Native fixtures used loopback; external URLs used fake WebSocket/fetch paths. Proxy variables were removed only from the test process, with repository preload, home guard and shared queue retained.

Earlier TypeScript, Structure, diff and changed-file privacy/size checks passed; unrelated sparse-absent paths were not evaluated. The source reviewer subsequently ran the Windows checks, so this was not a second independent review. Earlier Linux focused results on Bun 1.4.3/1.4.4 and the corrected 534d5f9e fake-socket pre-send receipt-refusal failure remain historical evidence.

Broader validation remains incomplete

The earlier Bun 1.4.3 test:changed run selected 693 files and reported 16,538 passed / 37 skipped / 269 failed / 22 errors. Reported categories included inherited-proxy ingress, HTTP-only mocks reaching WebSocket paths, discovery expectations and timeout cascades. An eight-case follow-up changed dependencies/environment and did not establish one common cause.

Those failures remain unresolved. Focused Windows results or hosted shard success do not classify them as environmental or establish macOS/full-suite success. Escaped-mock raw output is not republished. Focused checks used the shared-worktree resource exception; broader classification and final-head validation remain pending. Keep Draft; this is not merge or deployment approval.

Checklist

  • Scope stays focused and avoids unrelated cleanup.
  • Docs or release notes were updated when needed.
  • Security-sensitive changes were reviewed for secrets, auth, and unsafe defaults.

Co-authored-by: luvs01 27862058+luvs01@users.noreply.github.com

luvs01 and others added 9 commits October 23, 2026 21:25
Keep graceful close for one-shot sessions, then terminate after a one-second
unrefed grace if the peer leaves the connection open. Preserve retained-pool
immediate disposal and completed-session reuse. Cover timer ownership, failure
paths, idempotency and native Bun physical teardown with an injected stalled
close operation.

Co-authored-by: luvs01 <27862058+luvs01@users.noreply.github.com>
Explicitly widen the fresh readyState read after close(), which can mutate
it synchronously despite TypeScript retaining the pre-close narrowing.
Preserve the early-close timer cleanup behavior.

Co-authored-by: luvs01 <27862058+luvs01@users.noreply.github.com>
Refresh with dev and preserve private one-second close grace while avoiding
an observer and timer when native terminate is unavailable. Exercise real
peers that withhold Close replies for dispose and completed release, and
document private versus retained teardown in the owning Structure contract.

Co-authored-by: luvs01 <27862058+luvs01@users.noreply.github.com>
@coderabbitai

coderabbitai Bot commented Oct 24, 2026

Copy link
Copy Markdown
Contributor

Important

Draft PR not reviewed

Draft PRs are not automatically reviewed by default.

  • Trigger a manual review

To automatically review draft PRs, update your CodeRabbit configuration:

reviews:
  auto_review:
    drafts: true
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@github-actions github-actions Bot added the bug Something isn't working label Oct 24, 2026
@github-actions

Copy link
Copy Markdown
Contributor

✅ Deterministic PR hygiene checks passed.

lidge-jun added a commit that referenced this pull request Oct 26, 2026
…ockets (#7743)

A private Codex WebSocket now gets a one-second graceful close window before the native socket is terminated; retained-pool disposal stays immediate. Carries #7681.

Co-authored-by: luvs01 <27862058+luvs01@users.noreply.github.com>
@lidge-jun

Copy link
Copy Markdown
Owner

Release triage for v2.93.0: carried and merged. Your change landed on dev through #7743 as squash commit 852bf9d, with a Co-authored-by trailer crediting you.

The source change is byte-identical to this PR's dev-relative diff; only the base moved. The carried head passed exact-head PR CI and a cross-platform lane=all run, including the Windows and macOS shards, plus an independent review. Closing this draft in favour of the carry. Thank you!

@lidge-jun lidge-jun closed this Oct 26, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants