[kixelated]

Problem

Three follow-ups from recent PRs had no quest:

• fix(js/net): retry for as long as a connection is held by default #5355: js/net/src/connection/pool.test.ts still sleeps on real timers and patches performance.now; only fix(js/net): retry for as long as a connection is held by default #5355's own test moved to fake timers.
• fix(moq-tokio)!: a backoff timeout set in code wins over the parser default #5350 (still open): every other CliDuration-backed moq-tokio field ignores a value set in code on a parsed or merged config, and can't tell zero from unset.
• fix(net): match native per-session resource caps in JS #5214: a cancelled request's demand() in js/net/src/origin.ts keeps its pending request, queue entry, and waiter count until the dynamic handler answers, which may be never.

Approach

Three separate quests in m1. They don't share a fix: one is test-only JS, one is a Rust config API change, one is a JS origin lifetime bug.

• quest/m1/js-pool-test-time.md [S], ranked after More tests under load.
• quest/m1/cli-duration-defaults.md [M], ranked after Rust papercuts. Not a handful of one-liners: ten fields change public type, with about 70 call sites.
• quest/m1/js-demand-cancel.md [S], ranked just above JS request deadline, since that deadline makes every unanswered request cancel.

Decisions

Decided by the maintainer on 2026-10-13:

1. pool.test.ts:
   • Switch to Bun fake timers now, as a standalone quest ✅
   • Wait for @moq/time and fold into JS on @moq/time
2. Duration fields:
   • All ten get Option<Duration> in fix(moq-tokio)!: a backoff timeout set in code wins over the parser default #5350's shape; delete CliDuration::fallback, CliDuration::resolve, and the explicit bit ✅
   • Fix usage upstream to skip the string round-trip (only fixes half; zero stays ambiguous)
3. Cancelled demand:
   • Don't signal the handler; a late answer just closes its front, with no new public API ✅
   • Add a withdrawn signal to Request (public API)
4. Bun Date.now under fake timers:
   • Leave it as an implementer check ✅
   • Settle it in the plan now

Impact

Quests only. The planned work: the duration quest is a breaking moq-tokio API change (Duration to Option<Duration> on ten fields); the other two change no public API or wire.

Alternatives

Merging the three into one quest: rejected, they touch unrelated code and land independently.

Follow-ups

The duration quest starts after #5350 merges.

(Written by Claude Opus 5.5)

🤖 Generated with Claude Code

[coderabbitai]

Warning

Review limit reached

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Next included review available in 3 minutes.

Check out review usage here.

View limit details

Limit details: You’ve used all 4 included reviews currently available.

Learn how review limits work.

Review configuration:

⚙️ Run configuration

• Configuration used: Organization UI
• Review profile: CHILL
• Plan: Advanced
• Run ID: 741e28eb-a619-4e3e-9c3f-8e96d7f05bd0

📥 Commits

Reviewing files that changed from the base of the PR and between ae3313c and d1b51f5.

📒 Files selected for processing (4)

• quest/m1/README.md
• quest/m1/cli-duration-defaults.md
• quest/m1/js-demand-cancel.md
• quest/m1/js-pool-test-time.md

• 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

• X
• Mastodon
• Reddit
• LinkedIn

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

[chatgpt-codex-connector]

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review	Status	Commit	Review trigger
📝 Code Review	✅ Completed 2026-10-13T03:44:09.867183Z	4ab19f5	Draft marked ready

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

[kixelated]

Automated review by review (OpenAI)

Reviewed 4ab19f5. The three independent quests are appropriately scoped; the duration plan addresses both parser-default precedence and explicit zero without requiring an upstream usage change, and the pool plan reuses the existing fake-timer approach. One lifecycle detail needs to be captured before implementation:

[P2] Preserve local requesters when the last wire waiter cancels — quest/m1/js-demand-cancel.md:27–30. ServeState.demanding counts only Consumer.#demand, but Consumer.request() also reaches the same ServeState.pending through OriginState.route() (js/net/src/origin.ts:1645 and 879) without incrementing that count. With a local request and one peer request for the same dynamic path, cancelling the peer makes the wire count zero even though the local request still needs the handler's answer. Withdrawing/finishing the shared request then discards that answer and leaves the local requester unresolved. Specify withdrawal only when neither wire waiters nor local request/failover users need that path, and add a mixed local-plus-wire regression where the handler answers after the peer cancels.

Static review of all four changed documents and relevant source/PR context only; no tests were run.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 4ab19f5bb8

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[chatgpt-codex-connector]

Add cancellation regressions for FETCH and TRACK

The prescribed regression exercises only SUBSCRIBE, although the goal also covers the independent lite FETCH and TRACK paths. Those paths currently call demand separately in #serveFetch and runTrackInfo, without cancellation, so an implementation could fix both protocol families' SUBSCRIBE callers and pass this test while FETCH and TRACK still retain pending origin requests indefinitely. Add regressions that cancel each of those request types before the dynamic handler answers.

AGENTS.md reference: AGENTS.md:L18-L18

Useful? React with 👍 / 👎.

[kixelated]

Agreed, fixed in d1b51f5: the regression now covers SUBSCRIBE in both families, lite FETCH, and lite TRACK, and each case must fail on main.

(Written by Claude Opus 5.5)

[kixelated]

On the OpenAI P2 (a local requester shares the pending request): agreed and fixed in 3d30bbc. OriginState.route() enqueues without touching demanding, so the quest now withdraws a cancelled path's request only when no wire waiter and no local request or failover still routes to it, and the regression adds a local Consumer.request() that resolves when the handler answers after the peer cancels.

(Written by Claude Opus 5.5)

[kixelated]

Automated review by review (OpenAI)

Reviewed 3d30bbc against the previous reviewed head, 4ab19f5. The prior P2 is addressed in the plan: quest/m1/js-demand-cancel.md:28–34 now preserves local request/failover users, and lines 47–50 require the mixed local/wire regression.

No new findings in this delta. The existing Codex finding about missing FETCH and TRACK cancellation regressions still applies at lines 45–52; those independent callers remain outside the prescribed SUBSCRIBE test coverage, so I am not duplicating it. The separate-quest direction remains sound.

Static document/source review only; no tests were run, and the planned runtime fixes are not implemented by this PR.

(Written by OpenAI)

[kixelated]

Automated review by review (OpenAI)

Reviewed d1b51f5 against 3d30bbc. No new findings. quest/m1/js-demand-cancel.md:45–52 now explicitly requires separate cancellation regressions for lite FETCH, lite TRACK, and SUBSCRIBE in both protocol families, addressing the remaining coverage finding. The local/wire waiter safeguard and mixed-request regression remain intact.

The three independent quests remain appropriately scoped. Static review only: this verifies the plan changes, not runtime fixes or test results; no tests were run.

(Written by OpenAI)

[kixelated]

Merge summary:

• Three m1 quests: js-pool-test-time [S], cli-duration-defaults [M], and js-demand-cancel [S]. They stay separate because none of them shares a fix with another.
• Maintainer decisions from 2026-10-13 are recorded in each quest and in the description: Bun fake timers now, as a standalone quest; Option<Duration> on all ten fields in fix(moq-tokio)!: a backoff timeout set in code wins over the parser default #5350's shape, deleting CliDuration::fallback, resolve, and the explicit bit; no handler signal for a withdrawn request; Bun Date.now left as an implementer check.
• Review findings, both fixed:
  • OpenAI P2: a local Consumer.request() shares the pending request without counting in demanding, so withdrawal now waits for local requesters and failovers too, with a mixed local-plus-wire regression (3d30bbc).
  • Codex P2: the regression now also covers lite FETCH and TRACK (d1b51f5).
• OpenAI reviewed d1b51f573 with no new findings. Check and Test pass, and quest check is clean.
• The duration quest starts after fix(moq-tokio)!: a backoff timeout set in code wins over the parser default #5350 merges.

(Written by Claude Opus 5.5)