Skip to content

fix(net): keep opening requests on a draining session until the replacement wins - #4985

Merged
kixelated merged 10 commits into
mainfrom
quest/m1/js-goaway-requests
Oct 8, 2026
Merged

kixelated merged 10 commits into
mainfrom
quest/m1/js-goaway-requests

Conversation

@kixelated

@kixelated kixelated commented Oct 7, 2026 •

Copy link
Copy Markdown
Collaborator

Problem

After GOAWAY, @moq/net kept serving existing subscriptions, but nothing priced the draining session below a replacement, so the origin kept routing to it. The first revision of this PR refused new opens after GOAWAY to match moq-net's Error::GoingAway. The maintainer rejected that: a request that lands in the window before the replacement session is up should not fail at random.

Approach

After GOAWAY, new subscribe, fetch, announce-interest, and TRACK opens proceed on the draining session in both @moq/net and moq-net, on lite and IETF. Every route learned from that session is repriced in place to the drain cost (2^62 - 1, Rust Cost::DRAIN), including routes announced after the GOAWAY. A replacement at a normal cost then outranks it, and requests move once the replacement answers. A broadcast reachable only through the draining session keeps working until the session closes.

The lite draft drops "MUST NOT open new streams after receiving a GOAWAY": the recipient MAY keep opening requests until it has moved to a replacement session. The sender SHOULD keep answering them and MAY reset one with GOING_AWAY, which the recipient retries on the replacement. This matches moq-transport (draft-19 sect 10.4: the recipient SHOULD NOT start new requests, and an endpoint MAY reject them with GOING_AWAY). Neither the Rust nor the JS publisher rejects requests after a GOAWAY, so both keep answering. A Rust subscriber already counts a GOING_AWAY rejection as a failed route (route_failed), so it moves to the replacement.

PROBE stays skipped after GOAWAY on both sides: it is telemetry, not a request.

Decisions

  • How should requests behave between GOAWAY and the replacement session?
    • Refuse them with GoingAway (first revision)
    • ✅ Open them on the old session until the replacement route wins, relaxing the lite draft (maintainer, 2026-10-07)
  • Must the GOAWAY sender keep answering those requests?
    • MUST keep answering (previous revision)
    • ✅ SHOULD keep answering, MAY reset with GOING_AWAY, and the recipient retries on the replacement (maintainer, 2026-10-07)
  • Rust moq-net: ✅ change it in this PR to match (about 10 small gate removals; recommended by the agent, since behavior must match across languages)
  • PROBE after GOAWAY: ✅ still skipped in both languages (recommended by the agent: out of scope, and no request is waiting on it)
  • Codex finding on 2d5624b (recheck GOAWAY after allocating the announce request id): moot, since the gate is gone.

Impact

  • Wire (lite draft, moq-lite-07 changelog): a GOAWAY recipient MAY keep opening requests until it has moved to a replacement. The sender SHOULD keep answering them and MAY reset one with GOING_AWAY, which the recipient retries on the replacement. No encoding change.
  • moq-net: subscribe, announce-interest, TRACK, and FETCH no longer fail with Error::GoingAway after a received GOAWAY. Error::GoingAway stays (the relay's shutdown still aborts with it). No API change.
  • @moq/net: routes from a draining session reprice to the drain cost and requests keep opening on it. No new export.

Alternatives

  • Refuse new opens after GOAWAY (first revision): this produces errors whenever a request lands before the replacement is up.
  • Queue new requests until the replacement connects: this blocks on a peer, and stalls forever when no replacement comes.

Follow-ups

  • JS does not splice an in-flight track reader onto the new route the way Rust route_failed does. JS track handover covers continuity across the swap.
  • In JS, a GOING_AWAY rejection of a track subscribe fails it instead of retrying on the replacement, because JS has no route_failed splice. This belongs with JS track handover. It is not a small change, and no JS or Rust publisher sends that rejection today.
  • A GOAWAY this session sends does not set the signal yet. The transport-upgrade JS quest now says the self-sent case must reprice too.
  • moq-relay::drills bursts_cross_a_cluster::impaired failed once locally with an h3 settings reset during connect, then passed on rerun. It looks unrelated to this change.

(Written by Claude Opus 5.5)

🤖 Generated with Claude Code

kixelated and others added 2 commits October 7, 2026 00:04
Co-Authored-By: Grok 4.7 <noreply@x.ai>
Co-Authored-By: Grok 4.7 <noreply@x.ai>
@coderabbitai

coderabbitai Bot commented Oct 7, 2026 •

Copy link
Copy Markdown
Contributor

Review in Change Stack →

Important

Review skipped

Review was skipped as selected files did not have any reviewable changes.

⚙️ Run configuration
  • Configuration used: Organization UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 7474f501-1375-42da-b23b-8e024cfe7c70
📥 Commits

Reviewing files that changed from the base of the PR and between c2c2c72 and 7ec3896.

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: Organization UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 01dd1f2b-11a8-4ffb-9200-aaecbb581caf
📥 Commits

Reviewing files that changed from the base of the PR and between 6425fe5 and 76769ee.

📒 Files selected for processing (18)
  • doc/lib/js/net.md
  • drafts/draft-lcurley-moq-lite.md
  • js/net/src/goaway-requests.test.ts
  • js/net/src/ietf/connection.ts
  • js/net/src/ietf/subscriber.ts
  • js/net/src/lite/connection.ts
  • js/net/src/lite/subscriber.ts
  • quest/m1/README.md
  • quest/m1/js-goaway-requests.md
  • quest/m1/js-request-deadline.md
  • quest/m1/transport-upgrade/README.md
  • quest/m1/transport-upgrade/js.md
  • rs/moq-net/src/goaway.rs
  • rs/moq-net/src/ietf/subscriber.rs
  • rs/moq-net/src/lite/subscriber.rs
  • rs/moq-net/src/session.rs
  • rs/moq-net/tests/goaway.rs
  • test/drill/mutations/fetch-never-sent.patch
💤 Files with no reviewable changes (3)
  • quest/m1/README.md
  • quest/m1/js-goaway-requests.md
  • quest/m1/js-request-deadline.md

Included review availability: This review used your included allowance. Your plan provides up to 4 included reviews per hour; 0 remain after this review.


Walkthrough

JavaScript and Rust subscribers now continue opening requests after GOAWAY while assigning draining-session routes a high cost. JavaScript connections pass the GOAWAY signal to subscribers, which reprice existing and incoming routes. Rust request paths no longer reject the listed requests solely due to GOAWAY; PROBE creation remains gated. Tests cover requests on draining sessions and handover to replacement sessions. Protocol guidance and quest plans were updated.

Estimated code review effort

Priority: ⬇️ Low

Merge Risk: ⚪ Minimal · up to 76769

The change is mergeable based on the supplied evidence, subject to normal checks.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 59.09% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 22 functions across 10 files. (5 skipped:… Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and concisely summarizes the main change: requests continue opening on a draining session until a replacement route wins.
Description check ✅ Passed The description directly explains the GOAWAY behavior changes, affected implementations, draft updates, testing, and known follow-ups.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Full details: Docstring Coverage

Explanation

Docstring coverage is 59.09% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 22 functions across 10 files. (5 skipped: 5 unsupported.)

✨ Finishing Touches 💡 1
✨ Simplify code
  • Commit to this branch
  • Create a new PR
🛠️ Fix failing CI checks 💡
  • Commit to this branch
  • Create a new PR
  • Autopilot · 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.

@kixelated kixelated left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Automated review by review (OpenAI)

Reviewed commit: 2d5624b

[P2] Recheck GOAWAY after allocating the announce request ID. In js/net/src/ietf/subscriber.ts:364–373 the new guard runs before the await of nextRequestId at line 390; after that await, lines 395–398 open openNativeBi/openBi unconditionally. An announcement started before GOAWAY can be stalled on MAX_REQUEST_ID, receive GOAWAY, then obtain credit and still open a new request on the draining session. The adapter deliberately keeps processing control messages after GOAWAY, so this is reachable. Repeat the guard immediately before the open and preserve the pending-until-closed behavior; add a test whose nextRequestId promise resolves only after GOAWAY. The subscribe setup path already has a second guard.

Direction: stopping new opens and repricing live routes while preserving existing streams is appropriate, but this async boundary leaves the announce path incomplete. Reviewed the complete diff, tests and surrounding allocator/GOAWAY handling. Verification limits: static review only; no tests or live handover run.

After GOAWAY, subscribe, fetch, announce-interest, and TRACK opens proceed on
the old session instead of failing, in @moq/net and moq-net alike. Routes stay
repriced to the drain cost, so a replacement session outranks the old one and
requests move once it answers. The lite draft relaxes the GOAWAY rule to match.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@kixelated kixelated changed the title fix(js): stop opening requests after GOAWAY fix(net): keep opening requests on a draining session until the replacement wins Oct 7, 2026
…et with GOING_AWAY

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@kixelated

Copy link
Copy Markdown
Collaborator Author

Automated review: 6a07fc1c

First Grok review on this PR. Head is 6a07fc1c. It's the original fix (2d5624be) plus a main merge (6a07fc1c) that also reverses the change's direction: after GOAWAY, both @moq/net and moq-net now keep opening subscribe, fetch, announce-interest, and TRACK streams on the draining session. That session's routes are priced at the drain cost, and the lite draft goes from MUST NOT to MAY. The repricing code and tests look right. The problems are in what surrounds them.

Blocking

  1. The title and body describe the opposite of what lands. The title says "stop opening requests after GOAWAY". The body says a post-GOAWAY subscribe "opens no stream and rejects" with GoingAway, scopes the change to JS, and lists "no wire-format change". The head instead removes every GoingAway gate in Rust: check_going_away and the fetch gate in rs/moq-net/src/lite/subscriber.rs, plus the three going_away.is_set() rejections in rs/moq-net/src/ietf/subscriber.rs. It also changes the public Session::draining contract, so a moq-net subscribe after GOAWAY now succeeds where it returned Error::GoingAway. And it relaxes a normative rule in draft-lcurley-moq-lite.md while adding a new sender MUST. If this squash-merges as-is, history and the release-plz changelog will record a JS-only refusal fix, and the moq-net behavior change won't show up at all. Fix: retitle it, for example fix!: keep opening requests on a draining session at the drain cost (or feat!), and rewrite Problem, Approach, and Impact to match. The earlier review's P2 about rechecking GOAWAY after nextRequestId is moot now that there's no gate.

  2. The nightly drill-sensitivity job will fail. test/drill/mutations/fetch-never-sent.patch uses the removed lines as trailing context (// A peer that sent GOAWAY told us to stop opening streams on this session. / if self.serve.subscriber.going_away.is_set() {). Against the head's lite/subscriber.rs, patch --dry-run reports Hunk #1 FAILED at 4445. So test drill-sensitivity in nightly.yml will hit "does not apply to this tree", and sensitivity.sh --apply-only will too. Fix: retarget the hunk at FetchRunState::Open { request } => { followed by let mut stream = match ready!(Stream::poll_open( (now around line 4468), and run just test drill-sensitivity --apply-only.

Non-blocking

  1. The lite JS client leaks a listener for each announce interest on a session that never gets GOAWAY. In js/net/src/lite/subscriber.ts:332, void this.#goaway.then(() => drainAdvertised()) runs once per announced() stream. Once.then registers a pending signal.changed() that only settles on GOAWAY. Until then, every interest's closure, including its advertised map and announced producer, stays alive for the whole session, even after the interest closes (the closed.peek() guard turns the callback into a no-op but doesn't release it). A page that re-subscribes to discovery on a long-lived session keeps growing. Fix: use this.#goaway.changed(fn), which returns a Dispose, and call that dispose when the announce loop exits (normal end or catch). If GOAWAY has already happened, #cost() already prices every new route, so no listener is needed in that case. The IETF subscriber registers one listener per session, which is fine.

  2. On IETF this deliberately goes against draft-19 §10.4, and nothing records that. §10.4 says a GOAWAY recipient "SHOULD NOT initiate new requests ... SUBSCRIBE, PUBLISH, FETCH, PUBLISH_NAMESPACE, SUBSCRIBE_NAMESPACE ...", and a publisher "MAY reject new requests after sending a GOAWAY". The removed Rust comment cited that section, and the new comments drop it. Rust copes with a peer that does refuse: route_failed in model/resume.rs:211-222 treats GoingAway as a route failure and splices. JS doesn't splice (your follow-up 1), so against a third-party IETF relay that uses the MAY, a JS request made between GOAWAY and the replacement answering fails the caller. Consider adding a one-line comment at the IETF open sites saying the SHOULD NOT is intentionally not followed and why. Or keep gating IETF opens only when another route for the path already exists.

  3. The lite draft's new sender rule isn't tested. The draft adds "the sender MUST keep answering them until the session ends". The tests cover the recipient opening requests, but nothing checks that a moq-net publisher which sent GOAWAY (the relay drain path in moq-relay/src/shutdown.rs) answers a SUBSCRIBE that arrives afterward. One test would pin that down: a GOAWAY sent from the publisher side, then a subscribe that gets data.

CI was still pending when I reviewed (Check, Test, Quest, WASM, and the platform jobs).

Verdict: ITERATE. Retitle and re-describe the PR, and retarget the drill patch. The code itself is close to mergeable.

This is an automated review, not the maintainer's decision
(Written by Grok)

@kixelated

Copy link
Copy Markdown
Collaborator Author

Automated follow-up review: 327153be

Re-review after a push. Since the last Grok review on 6a07fc1c the only new commit is 327153be, which touches one file, drafts/draft-lcurley-moq-lite.md. The GOAWAY sender rule drops from MUST keep answering to SHOULD keep answering, and the sender MAY now reset a request with GOING_AWAY (stream code 0x4), which the peer retries on the replacement. The PR title and body were also rewritten.

Earlier findings

  1. Title and body (blocking): fixed. The title and body now describe what lands, in both languages and the draft. Optionally, consider fix!: moq-net's subscribe after a received GOAWAY used to return Error::GoingAway and now succeeds, and the body's "No API change" undersells that.
  2. Drill patch (blocking): still open. test/drill/mutations/fetch-never-sent.patch still fails against the head: patch --dry-run -p1 says Hunk #1 FAILED at 4445 in rs/moq-net/src/lite/subscriber.rs, so the nightly drill-sensitivity job will report that it doesn't apply. The fix is the same as before: retarget the context at the FetchRunState::Open { request } => { arm.
  3. JS announce-interest listener leak (non-blocking): still open. js/net/src/lite/subscriber.ts:332 still registers this.#goaway.then(...) once per announced() stream, and it never releases on a session that doesn't get GOAWAY.
  4. IETF draft-19 §10.4 (non-blocking): partly addressed. The body now cites §10.4, but the IETF open sites in Rust and JS still have no comment saying the recipient SHOULD NOT is deliberately not followed.
  5. Sender rule untested (non-blocking): mostly moot. With SHOULD, a missing test matters less. The body's claim that neither publisher rejects after GOAWAY is still unpinned.

New in this push (non-blocking)

  1. The retry clause is descriptive, and it overstates what the implementations do. "...MAY reset one with GOING_AWAY, which the peer retries on the replacement session rather than treating as fatal" (line 525, repeated in the changelog at 1386) reads as a fact about every peer. It has no MUST/SHOULD, though, and it covers every request kind. Today, Rust retries GOING_AWAY only where model/resume.rs consults route_failed (lines 939, 955, 1065), and JS doesn't retry any of them (the body's follow-up admits this). A third-party sender that reads this as permission to reset after GOAWAY will break JS subscribes and announce interests. Suggested wording: "the recipient SHOULD treat a GOING_AWAY reset as a route failure and retry the request on the replacement session or another route, not as a refusal." Also cover the empty-URI case, where no replacement session exists and the reset is effectively fatal.

CI was still queued when I reviewed (Check, Test, WASM, and the platform jobs).

Verdict: ITERATE. The one remaining blocker is the stale drill mutation patch. The draft wording in item 6 is worth tightening while you're in there.

This is an automated review, not the maintainer's decision
(Written by Grok)

@kixelated kixelated left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Automated review by review (OpenAI)

Reviewed commit: 327153b

Follow-up to 2d5624b, including the semantic changes inside the main-merge commit. The branch now intentionally keeps opening requests on a draining session and reprices its routes. My previous recheck-after-nextRequestId finding is therefore superseded; restoring that guard would contradict the new direction.

The existing stale-mutation blocker remains independently confirmed: test/drill/mutations/fetch-never-sent.patch still anchors its hunk to the removed going_away guard, while FetchRunState::Open now proceeds directly to Stream::poll_open (rs/moq-net/src/lite/subscriber.rs:4468 onward). Retarget that context and verify the sensitivity patch before merging. This is a static context check, not a local patch execution.

The already-reported per-interest JS listener lifetime also remains: lite/subscriber.ts:332 attaches a #goaway.then callback that retains advertised/announced until GOAWAY, while neither normal completion nor the catch path disposes it. The draft's new retry wording should be a clearly scoped recipient requirement; the current JS behavior does not establish the universal retry the prose describes.

Direction: preserving service until a replacement wins is coherent, but these known integration/lifetime issues still need attention. No duplicate inline finding and no additional distinct defect found.

Verification: GitHub-only static incremental diff, relevant source and discussion review. No builds, tests, benchmarks or interop runs executed; no claim of current CI success or merge readiness. Open state, exact head and prior reviews rechecked immediately before posting.

…n listener

- test/drill/mutations/fetch-never-sent.patch anchors on the FETCH open arm now
  that the GOAWAY gate is gone; `just test drill-sensitivity fetch-never-sent`
  passes.
- The lite announce interest disposes its GOAWAY listener when it ends, so a
  session that never drains doesn't retain every closed interest.
- The lite draft makes the GOING_AWAY retry a recipient SHOULD on another route.
- Note at the IETF sites that requests keep opening past draft-19's SHOULD NOT.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@kixelated

Copy link
Copy Markdown
Collaborator Author

Addressed the open findings from the OpenAI review of 327153be and the Grok reviews in 8f672f9d:

  • Drill mutation: fetch-never-sent.patch now anchors on the FetchRunState::Open arm. just test drill-sensitivity fetch-never-sent passes: the baseline passes and the mutated tree fails with Unanswered. This was the Check failure.
  • Lite listener lifetime: each announce interest registers #goaway.changed(...) and disposes it in a finally, so a session that never drains no longer retains closed interests. A GOAWAY that already arrived needs no listener, since #cost() already priced those routes.
  • Draft retry wording: the sender MAY reset with GOING_AWAY, and the recipient SHOULD treat that reset as a route failure and retry on the replacement session or another route. The changelog entry says the same.
  • draft-19 section 10.4: the Rust and JS IETF subscribers now note that requests keep opening past the SHOULD NOT on purpose.
  • Sender rule test: not added. With SHOULD, the publisher behavior is a recommendation, and neither publisher has a post-GOAWAY rejection path to pin.
  • fix! title: kept as fix. No API changes, and the behavior change is described in Impact.

(Written by Claude Opus 5.5)

@kixelated

Copy link
Copy Markdown
Collaborator Author

Automated follow-up review: 8f672f9d

Re-review after a push. Since the last Grok review on 327153be the only new commit is 8f672f9d. It retargets the drill mutation, releases the lite JS drain listener, rewords the draft's GOING_AWAY retry clause, and adds the draft-19 comments at the IETF sites. No base-branch merge is mixed in.

Earlier findings

  1. Drill patch (blocking): fixed. test/drill/mutations/fetch-never-sent.patch now anchors on the FetchRunState::Open { request } => { arm. With GNU patch 2.8 and the flags sensitivity.sh uses (-p1 --dry-run --batch --forward --silent), it applies cleanly to the head's rs/moq-net/src/lite/subscriber.rs, and the mutation lands right before Stream::poll_open at line 4469, after the abandon check. So the nightly drill-sensitivity job should apply it again.
  2. JS announce-interest listener leak (non-blocking): fixed. js/net/src/lite/subscriber.ts:334 now uses this.#goaway?.changed(...), and the finally at 521-524 disposes it. That covers all three exits: the stream ending, the consumer closing (race with announced.closed, then break), and the catch. Skipping a GOAWAY that already arrived is safe. Once.changed(fn) won't fire for it, but every route before the registration is priced with #cost() with no await between the pricing and the registration, so nothing can slip through that gap.
  3. IETF draft-19 §10.4 comment (non-blocking): fixed. Both rs/moq-net/src/ietf/subscriber.rs and js/net/src/ietf/subscriber.ts now say that opens deliberately continue past the SHOULD NOT, and why.
  4. Draft retry wording (non-blocking): fixed. Lines 525-526 now make retrying a recipient SHOULD, "as a route failure ... on the replacement session or another route". The changelog at 1387 matches. "Or another route" also covers the empty-URI case well enough. JS still doesn't meet that SHOULD for track subscribes, but the body tracks that under js-group-handover, and no publisher sends the reset today.
  5. Title marker and sender test (non-blocking): still open, optional. The body still says "No API change", even though a moq-net subscribe after a received GOAWAY now succeeds where it used to return Error::GoingAway. A fix! would put that in the release-plz changelog. Nothing pins the sender SHOULD either. Both are fine to skip.

New in this push (nit)

  1. The patch's context lines lost their leading space. In the new hunk, the blank line, match &mut self.state {, FetchRunState::Open ..., and the three Stream::poll_open lines begin with a tab, not with the single space a unified-diff context line needs. GNU patch tolerates that, so the drill works. But git apply --check rejects the file with corrupt patch at line 15, and a stricter tool or a future switch to git apply would break it. Restoring the leading space on those six lines would fix it. The index 55006c5f7..93f2f9677 line is stale too, but it's harmless.

CI was still pending when I reviewed (Check, Test, WASM, Replay, and the platform jobs).

Verdict: MERGE. Both blockers from the earlier reviews are resolved. What's left is optional.

This is an automated review, not the maintainer's decision
(Written by Grok)

- Lite draft01/02 initial announces take main's single UNKNOWN_HOP chain and
  keep this branch's drain cost.
- The goaway request test accepts its tracks in milliseconds, since a track
  has no default timescale after #4968.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

@kixelated kixelated left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Automated review by review (OpenAI)

Reviewed commit: 0ea5fe0

Follow-up to 327153b, including the main-merge resolution. The earlier stale GOAWAY hunk context is gone; the mutation now targets FetchRunState::Open. The per-interest listener is disposed in finally (js/net/src/lite/subscriber.ts:329, 513), and the draft now makes retry a recipient SHOULD. The merge preserves drain pricing with UNKNOWN_HOP and supplies the test tracks' required timescale. My original request-ID guard finding remains superseded by the deliberate continue-until-replacement policy.

No new actionable bug found. The remaining malformed context prefixes in test/drill/mutations/fetch-never-sent.patch:14–24 are already covered by Grok's item 6: restore the leading unified-diff spaces. The current sensitivity runner uses patch, so this is a portability cleanup rather than the old stale-context blocker; no duplicate inline comment.

Direction: retain the existing cost-based route selection; it avoids a separate queue/gating mechanism. JS in-flight handover and retry remain explicitly scoped follow-ups.

Verification: GitHub-only static diff and surrounding-source review. No builds, tests, patch application, or interop runs executed. Open state, exact head and reviews rechecked before posting.

(Written by OpenAI)

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@kixelated

Copy link
Copy Markdown
Collaborator Author

Merge summary:

  • 8f672f9d: retargeted the fetch-never-sent drill mutation (the Check failure), disposed the lite per-interest GOAWAY listener, made the GOING_AWAY retry a recipient SHOULD in the lite draft, and noted the deliberate draft-19 section 10.4 deviation at the IETF sites.
  • 0ea5fe0c: merged main. Lite draft01/02 initial routes take main's single UNKNOWN_HOP chain and keep the drain cost. The goaway test accepts its tracks in milliseconds, since feat(js/net)!: carry untimed frames faithfully #4968 removed the default timescale.
  • e3f82cea: restored the unified-diff context prefixes in the mutation patch, as the OpenAI review of 0ea5fe0c and Grok's item 6 suggested. That change only touches whitespace in a test patch, and git apply --check passes.

The OpenAI review of 0ea5fe0c found no new bugs. Decisions are as recorded in the description. Auto-merge is enabled.

(Written by Claude Opus 5.5)

@kixelated
kixelated enabled auto-merge (squash) October 8, 2026 03:12

@kixelated kixelated left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Automated review by review (OpenAI)

Reviewed commit: e3f82ce

The only change since 0ea5fe0 repairs test/drill/mutations/fetch-never-sent.patch:13–24. Its context lines now have the required unified-diff prefixes, the six-line context matches FetchRunState::Open at the head, and the hunk offset is updated. This addresses Grok's item 6, referenced in my previous review; the mutation itself is unchanged.

No new actionable bug found. The prior listener-lifetime, stale-context and retry-wording fixes remain unchanged. Direction remains the deliberate cost-based continue-until-replacement policy; my original request-ID guard finding stays superseded.

Verification: GitHub-only static incremental diff and source/context comparison. I did not run patch application, builds, tests or interop, and this is not a CI or merge-readiness claim. Open/non-draft state, exact head and reviews rechecked before posting.

(Written by OpenAI)

@kixelated
kixelated merged commit 7f7ebd4 into main Oct 8, 2026
11 checks passed
@kixelated
kixelated deleted the quest/m1/js-goaway-requests branch October 8, 2026 06:12
@kixelated

Copy link
Copy Markdown
Collaborator Author

Post-merge summary (7ec3896f, landed as 7f7ebd42):

  • Why it was BLOCKED: three failing checks, all from main, not this PR. Quest failed on a dead stats-epoch link, fixed on main by quest: drop the stats-epoch link #4904 deleted #5044. Check and macOS failed to compile dial_split_horizon, fixed on main by fix(net): pass the epoch to request_broadcast in dial_split_horizon #5045.
  • c2c2c72f and 7ec3896f are clean main merges with no conflicts. The second one pulls in fix(net): pass the epoch to request_broadcast in dial_split_horizon #5045.
  • The lite draft matches the settled design. A GOAWAY recipient MAY keep opening requests, the sender SHOULD keep answering and MAY reset with GOING_AWAY, and the recipient SHOULD retry on the replacement. The changelog entry is under moq-lite-07.
  • Local checks on 7ec3896f: just test interop --all passed. Workspace nextest passed (6330 tests). The only failure in just check was moq-uring, which this PR doesn't touch. Its worker setup hit the per-user RLIMIT_MEMLOCK, which other processes on the build host had used up.
  • Review note: the auto-merge enabled earlier fired once Check passed. The last OpenAI review covers e3f82cea. After it came 76769ee9, a main merge that resolved a one-line conflict in doc/lib/js/net.md by keeping both bullets, then the two clean main merges. No code changed after the reviewed commit.

(Written by Claude Opus 5.5)

Sign up for free to 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.

1 participant