Repository navigation
Conversation
…, and count only sockets Codex opened
…, Codex drop and status-less close, and the counter notes
…ing upgrade reach its 503 whatever the Home last advertised
…in as its one exception
…ild-t6-5 # Conflicts: # scripts/test-layout/layout-clients.json
Contributor
|
Important Draft PR not reviewedDraft PRs are not automatically reviewed by default.
To automatically review draft PRs, update your CodeRabbit configuration: reviews:
auto_review:
drafts: true
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. |
Contributor
|
✅ Deterministic PR hygiene checks passed. |
…the Child, as on the Home
…t closes both legs, counted once
…el spawned, start-up check done, link ended), not on microtask counts or a zero timer
… Home and its two timers
…eadable link id included
This branch has not been deployed
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
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.
Summary
Follow-up to the Child side of the Responses WebSocket over a Remote Link (#8614, #8619, #8620). The final reviews of #8619 and #8620 left items that must be fixed before the Child's release gate opens; this PR fixes them. The gate stays closed, so nothing changes for anyone today.
LinkTunnelGatehas an optionalonStoppinghook. The supervisor calls it whenstop()begins, before it signals the tunnel, and the relay drains: open link sockets close with 1001 on both legs, setups are aborted, and every upgrade from then on, new or still in setup, gets the transient 503 (link_websocket_unavailable, counted asdraining). The supervisor also calls it when the link ends (its sidecar is gone or names another link) and the runtime is about to recycle. The hook lives in the supervisor rather than inruntime.ts, so every path that stops the supervisor (restart, recycle, signal) drains first. While draining, an upgrade reaches its 503 even if the Home's last answer turned the advertisement off; a closed Child gate or the Child's opt-out still gives today's 426 with no network I/O. Eachserve()builds a new listener and supervisor, so a drained relay is never reused.server.upgradereturned false, the Child countedcloses.stopped, wrotelink_ws_close:stoppedand counted a complete request in the traffic book for a socket Codex never had. When it threw, the leg stayed in the set with a paused Home socket. Both now go throughHomeLeg.abandon(): the Home leg closes with 1001, only thehandshakefallback is counted, and the slot is released once the Home connection closes. A socket's traffic flow now begins when Codex's socket opens.HomeLegarms the 5 s close bound when it is created and stops itself ifattachhas not run by then. Scratch runs on the pinned Bun 1.4.4 show that Bun runsopensynchronously insideserver.upgradewhen it returns true, and thatserver.upgradereturns false (withreq.signalaborted) if Codex left before the call. The bound is therefore a guard for runtimes that behave differently.server.upgradethrew after runningopen) closes both legs as a stop does and counts the socket once; it used to close only the Home leg.LinkWebsocketCountersJSDoc andstructure/remote-link.mdnow say that a non-localOriginrefused with 403 is not counted, and thatfromCodexBytescounts text messages arriving while the Home leg is open. The structure page also explains why the Child's?transport=1shape differs from the Home's per-rowws: the Home counts what it did with upgrades per link (refused), while a Child serves one link and itsfallbacksare the attempts it sent back to HTTP or told to retry, with reasons only a Child sees. The name is kept.What keeps it inert:
linkWebsocketChildReady()is still false, so the ingress answers today's 426 and the relay never runs. The relay subscribes to the supervisor's stopping hook in every supervised link-mode listener; with the gate closed, the hook flips two flags and walks two empty sets: no I/O, no counting, no diagnostics.Verification
Tests that failed first (
tests/clients/link-ws-relay-edges.test.ts):server.upgradereturning false answers 426, counts no close and no request, and frees the slot. Failed:link_ws_close:stoppedwas recorded and a request was counted.server.upgradethrowing does the same and leaves no leg behind. Failed: the leg stayed.stop()notified.Final-review fixes (failed first unless marked):
incomplete: 1, the 1005 halfincomplete: 0. Failed: the drop counted complete.server.upgradethrowing after Bun opened Codex's socket closes both legs and counts the socket once. Failed: Codex's socket was never closed.onInitializedtest seam inClientLinkSupervisorDeps; link ended) instead of microtask counts and a zero timer. Test-only.Existing tests updated:
tests/clients/link-ws-relay.test.ts(the slot-release case awaits the newonReleasedseam instead of retrying until a 101; the stop-in-hold case asserts exactly 426; the oversized-message case asserts that the Home gets 1011; two directHomeLegcases take a flow factory and mark the socket upgraded, as the relay does) andtests/clients/link-ws-child-status.test.ts(waits on the Child's own release before readingopen: 0).Commands (exit codes) on 03a7cf0 (the T6-5-1 head 8494612 plus one commit per final-review item; it merges cleanly with dev 6f58f62):
bun install --frozen-lockfile: exit 0bun run typecheck: exit 0bun run privacy:scan: exit 0bun run structure:check: exit 0bun scripts/test.ts --parallel=2over tests/clients/link-ws-relay, link-ws-relay-edges, link-ws-child-status, link-ws-fallback, link-ws-transport, link-ws-framer, link-ws-advertisement, link-ws-activity-holds, client-link-tunnel, client-link-runtime, client-machine-listener, client-link-relay, link-relay-bound-transport, link-boundary, tests/e2e-style/link-ws-child-home, tests/e2e-style/link-websocket-release-gate, tests/lab/core-lab-boundary, tests/test-layout, test-layout-tooling, ci-workflows/structure-ssot, ci-workflows/file-size-ratchet: exit 0 (384 pass, 8 skip, 0 fail; the skips are seven release-gate cases that need the pinned codex-rs binary and one macOS-only case)Review: a gpt-6.1-terra reviewer (xhigh, read-only, its own worktree) reviewed the whole slice. Round 1: no P0-P2; two P3s (the link-ending path did not drain; the ingress's advertisement check could bypass a drain), both accepted and fixed with the tests above, which failed first. Round 2: no P0-P2; one documentation P3 (the structure page's earlier 426 sentence did not name the drain), fixed in cb79c1f. The coordinator's fresh final review of 8494612 found one P2 (a dropped Codex counted complete) and five P3s; each is fixed in its own commit, and a gpt-6.1-terra round on those commits found none. Open P0-P2: 0.
NOT RUN: the full
bun run testsuite (4-core VM rule: focused files only); macOS/Windows; the release-gate cases intests/e2e-style/link-websocket-release-gate.test.tsthat need the pinned codex-rs binary (OCX_LINK_GATE_CODEX, absent here; they skip).Checklist