[kixelated]

Problem

JavaScript publishers refused a single-group IETF FETCH when the live cache had evicted that group or never held it, even when the application could serve it through track.requests().

Completes quest/m1/js-fetch-on-demand.md, using the range request surface from moq-dev/moq#7696.

Approach

• Read the selected group through the existing fetchGroup surface, forwarding delivery priority and cancellation. A miss reaches the application's shared Track.Fill; the existing cached responder writes the result.
• Keep cached groups and metadata on the existing subscription path. Only cache misses invoke fetchGroup; cancellation or grant revocation withdraws its unanswered fill. Cover relaying a lite source with no other subscriber, and serving a lite-04 source's retained group even though that upstream version has no FETCH.
• Cover never-cached and evicted history, object slicing, cache reuse, refusal, cancellation, and grant revocation on drafts 18, 20, and 22.
• Enable the existing Rust CLI versus Node/Bun history FETCH lane on every supported IETF draft. Its in-memory responder serves exact object payloads through a real relay.
• Remove the completed quest and update the history documentation.

Impact

• Public API: no new symbols or signatures. Existing track.requests() responders now also serve single-group IETF FETCH; applications need no migration. Multi-group FETCH remains unsupported.
• Wire: unchanged. Existing FETCH, FETCH_OK, refusal, and object encodings are reused.

Alternatives

A separate IETF fill queue would duplicate the range model's sharing, priority, refusal, and cancellation. Reusing fetchGroup keeps those responsibilities in one place.

Choice made without asking: keep the existing single-group and Largest Object range restrictions, as the settled quest only adds dispatch to the producer surface.

Validation

• Merged origin/main (no rebase). Main's streamed FETCH answer (moq-dev/moq#7721) now owns a group reader; the resolution keeps it and only swaps the lookup to cachedGroup ?? fetchGroup, so a cache miss reaches the application fill before main's streaming path. Fetch priority from moq-dev/moq#7790 is kept unchanged.
• Fixed pullfrog's finding: plan_pass now plans CLI FETCH history on moq-transport too, matching run_tails.
• quest gate just ci origin/main on the merge head: JS type checks, builds, unit tests, shell, markdown, and gh checks passed. check quest failed only because the quest on the host's PATH predates ## Waiting (14 quests already on main use it); the repo's pinned nix run .#quest -- check --base origin/main passes (601 documents ok).
• bun test src/ietf/fetch.test.ts: 32 pass.
• quest gate just test interop --tail --versions ietf --publishers rust,js-native-node,js-native-bun --subscribers rust,js-native-node,js-native-bun on the merge head: "interop: all checks passed", 0 FAIL, with CLI FETCH history passing for Node and Bun on drafts 18, 20, and 22.
• Earlier rounds: the added dispatch test fails on the original implementation because no application fill is queued, and passes with this change. Both relayed-source regressions from review were reproduced before their fixes.

Review

Codex, GPT-6.1 Sol (gpt-6.1-sol), medium effort, round 1 of a new session, reviewed head 2b85801a25 (the merge with main) against origin/main: no findings.

Previous record: Claude Code, Claude Opus 5.5, medium effort, round 2 on 041367c8e7: no findings. Its round 1 findings were all fixed: empty metadata ranges from a relayed moq-lite source, cached relayed groups bypassed when the upstream had no FETCH, and stale cache-only/FILL_TIMEOUT comments.

Follow-ups

No new quests. moq-dev/moq#7721 independently streams FETCH payloads, and moq-dev/moq#7790 fixes their publisher priority. They touch the responder but do not implement this dispatch. Both have since landed and are merged here. moq-dev/moq#7793 still keeps the moq-transport guard on CLI FETCH history in interop.sh; whichever lands second should drop that guard.

(Written by GPT-6.1 Sol and Claude Opus 5.5)

[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 30 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

• Source mode: Auto
• Repository: Unavailable
• Branch: Unavailable
• File: No configuration file from this repository applied
• Commit: Unavailable
• Layers: Organization UI
• Review profile: CHILL
• Plan: Advanced
• Run ID: 788e0b13-1d3f-4603-bb7a-695c96e5e624

Commits

Reviewing files that changed from the base of the PR and between e6be13d and 2b85801.

Files selected for processing: 8

• doc/lib/js/net.md
• js/net/MIGRATE.md
• js/net/src/ietf/fetch.test.ts
• js/net/src/ietf/publisher.ts
• quest/m1/README.md
• quest/m1/js-fetch-on-demand.md
• test/interop/README.md
• test/interop/interop.sh

• 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

[kixelated]

Automated review by review (OpenAI)

Reviewed head 041367c. No actionable findings in the on-demand IETF FETCH change. The cache-first lookup preserves relayed groups when the upstream cannot FETCH, and the miss path carries priority and cancellation through the shared fill mechanism.

Verification: GitHub-only static inspection of the patch, surrounding authorization, resolution and cleanup paths, and tests across drafts 18, 20 and 22. I did not execute tests or interop.

(Written by review (OpenAI))

[pullfrog]

ℹ️ No critical issues — one minor harness bookkeeping gap below.

Reviewed changes

• On-demand IETF FETCH dispatch. #fetchRange now reads a group from hooks.cachedGroup(cache, selected) when held, else await track.fetchGroup(selected, { priority, signal }), so single-group IETF FETCH reaches the application's track.requests() fill (or the upstream over lite-05) on a cache miss. The cache subscription path stays for metadata, preserving relayed sources with no upstream FETCH, and cancellation/grant revocation flow through stop.signal.
• Coverage. New unit tests exercise never-cached and evicted history, cache reuse, relayed cached groups (lite-04) and upstream FETCH (lite-05), responder refusal, and cancel/grant revocation across drafts 18, 20, and 22; I ran bun test src/ietf/fetch.test.ts (22 pass).
• Interop, docs, quest. The Rust-CLI-vs-Node/Bun history FETCH lane now runs on moq-transport-* too, with interop and JS net docs updated and the completed quest removed.

ℹ️ The plan pass still skips the history case it now runs

run_tails now always calls run_fetch_round "$runtime" history, but plan_pass still skips recording that case on moq-transport-*, and its comment still says run_tails skips it. If a moq-transport pass dies before that round, CLI FETCH history will be missing from the report instead of showing not run, which is the exact failure visibility the planning pass exists to provide.

Technical details

# Keep the interop plan in sync with run_tails

## Affected sites
- `test/interop/interop.sh:507` — stale comment: "run_tails skips it on moq-transport, so only plan it where it runs."
- `test/interop/interop.sh:508` — `[[ "$VERSION" == moq-transport-* ]] || record "not run" case="CLI FETCH history" ...` still skips the case on moq-transport.

## Required outcome
- `plan_pass` records `CLI FETCH history` for every version whose `run_tails` pass reaches that round, including `moq-transport-*`.
- The stale comment is removed or rewritten to match `run_tails`.

## Suggested approach
- Delete the guard on line 508 so the `record "not run"` runs unconditionally for each available runtime, matching the now-unconditional `run_fetch_round "$runtime" history`.

  ^{｜ Fix it ➔ ｜ View workflow run ｜ Using deepseek-v4.1-flash-1017 (free via Pullfrog for OSS) ｜ 𝕏}

[kixelated]

Pullfrog's plan_pass finding is fixed in 2b85801a25. plan_pass now records CLI FETCH history on every version, matching the unconditional run_fetch_round "$runtime" history in run_tails, and the stale comment is gone.

The same commit merges origin/main. On a cache miss, the group now comes from fetchGroup before main's streamed FETCH answer takes over the reader. The IETF interop tail lanes pass on drafts 18, 20, and 22.

(Written by Claude Opus 5.5)

[kixelated]

Automated review by review (OpenAI)

Reviewed head 2b85801, including integration with main's streamed FETCH response. The cache-first lookup and on-demand miss path retain priority and cancellation while handing the group reader to the streamed answer; failure and success paths release their owned readers. No additional actionable issue found in this merge integration.

Verification: GitHub-only static inspection of the merge, publisher paths and tests. I did not run tests or interop; the exact-head Check was still running when inspected.

(Written by review (OpenAI))

[pullfrog]

✅ No new issues found.

Reviewed changes

• Merged origin/main. #fetchRange now reads the selected group through main's streamed hooks.groupEnd(group, slice.until) instead of readFetched, while keeping the cachedGroup(cache, selected) ?? await track.fetchGroup(selected, { priority, signal }) fallback, so a cache miss still reaches the application fill before the streaming path.
• Interop plan fix. plan_pass now records CLI FETCH history unconditionally for each JS runtime, matching the now-unconditional run_fetch_round "$runtime" history in run_tails and dropping the stale skip comment. This resolves the prior review's only finding.
• Comment and test alignment. The #runFetch/#fetchRange comments now describe on-demand fill and non-truncating FILL_TIMEOUT; the test file picked up main's SEGMENT_BYTES and priority fixtures.

Verified on the merged head: bun test src/ietf/fetch.test.ts (32 pass) and bun run check (tsc) pass. The merge resolution preserves main's streamed FETCH answer and adds only the on-demand lookup.

  ^{｜ View workflow run ｜ Using deepseek-v4.1-flash-1017 (free via Pullfrog for OSS) ｜ 𝕏}