quest: plan follow-ups from #5011 and #5070

[kixelated]

Follow-ups from #5011 and #5070, taken under /recommended (each was the merging agent's recommendation).

• quest/m1/ietf-legacy-updates.md: the JS bullet now matches fix(js): route draft 14-16 updates to their target #5011 (the d14-16 publisher reads and discards updates; applying them remains).
• quest/m1/request-id-order.md [S]: drafts 14 to 16 refuse a reused or lower Request ID in both languages (predates fix(js): route draft 14-16 updates to their target #5011; the JS window deliberately tolerates reuse today, so the plan first confirms the drafts' rule). Notes the stricter receiver as an interop change.
• quest/m1/test-ts-check.md [S]: just check type-checks every TypeScript harness under test/, workspace or standalone, including harness-only diffs (feat(js)!: publishing never invents a timestamp #5070's test/drain/drain.ts kept calling a deleted API past local checks and PR CI).

Public API: none (quest text only). Wire: none.

🤖 Generated with Claude Code

(Written by 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 22 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: 6ef75b2b-4095-44d6-a07f-51ee5b13696b

---

📥 Commits

Reviewing files that changed from the base of the PR and between 5861127 and b5cb78d.

---

📒 Files selected for processing (4)

• quest/m1/README.md
• quest/m1/ietf-legacy-updates.md
• quest/m1/request-id-order.md
• quest/m1/test-ts-check.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.}

[kixelated]

Automated review of d1c59423 (quest text only)

Claims check out on main: the "A peer reusing IDs below the bound..." comment is at js/net/src/ietf/adapter.ts:508, and test/drain/ has its own package.json and isn't in the root workspaces, so the test-ts-check plan is accurate.

Non-blocking

• quest/m1/README.md: the Request ID order entry is added after a blank line at the very end of the file with no trailing newline, so it renders as a separate list detached from the others. Move it up into the list (and add a newline at EOF).
• request-id-order.md: "Wire: none (a stricter receiver)" is a little generous. Refusing a lower ID can close sessions with peers that work today, so it's worth noting as an interop behaviour change once the drafts' rule is confirmed.

Verdict: MERGE

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

[kixelated]

Merged main (kept both sides of the m1 README conflict) and addressed the Grok review in cbca1d84:

• The Request ID order entry now sits in the m1 list with a trailing newline.
• request-id-order.md notes the stricter receiver as an interop change.

(Written by Claude Opus 5.5)

[kixelated]

Grok review of head 56408deb (first review; docs/quest only)

I checked the claims against main and they hold up: #5011 and #5070 are both merged, the "reusing IDs below the bound" comment sits at js/net/src/ietf/adapter.ts:508, test/drain/ has its own package.json but isn't in the root workspaces, and js/justfile's check only runs bun run --filter='*' check over workspace packages. Both new README links resolve to files added in this PR.

Blocking: none.

Non-blocking

• request-id-order.md: the high-water-mark rule is only correct if each side's IDs are checked in their own parity space (client even, server odd on d14+), so a "per peer" mark should be per-direction, not shared. It's also worth noting the "refuse" happens before window accounting, so a refused ID shouldn't consume a MAX_REQUEST_ID slot.
• test-ts-check.md: adding test/drain to the root workspaces would also pull it into bun install --frozen-lockfile and the lockfile, so the "or from the test justfile" option may be the lighter path. Either way, spell out how the scoped check knows test/drain depends on js/net; otherwise that last step isn't testable.
• ietf-legacy-updates.md: the new JS bullet says updates are read and discarded. Pointing to where that happens (the fix(js): route draft 14-16 updates to their target #5011 code path) would keep the next person from re-checking it.
• CI (Check/Test/Quest) is still pending.

Verdict: MERGE

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

[kixelated]

Grok review of cbca1d84 (quest text only: three follow-ups from #5011 and #5070)

Checked the claims against main. #5011 and #5070 are both merged, and js/net/src/ietf/adapter.ts:508 does say "A peer reusing IDs below the bound still cannot hold more than the window open", so request-id-order.md describes today's behaviour correctly. No blocking issues.

Non-blocking

1. quest/m1/test-ts-check.md: the plan says test/drain "is neither a root workspace nor has one". That's wrong. The root package.json lists test/drain as a workspace (along with test/wasm, test/interop/clients/js, test/interop/clients/js-native, and test/audio-quality/clients/js). The real gap is that test/drain/package.json has no check script and no tsconfig, so bun run --filter='*' check skips it. That makes the fix smaller: add a check script (tsc --noEmit) and a tsconfig to each test/ TS workspace that's missing one. The quest could also say which of the other test/ workspaces already type-check.
2. request-id-order.md: "refuse anything at or below it" plus "first confirm what the drafts require" is the right order. One thing to make explicit: which ID space the high-water mark covers (one per direction or per peer, and whether client and server parity are tracked separately), so the two language ports don't diverge.
3. ietf-legacy-updates.md: the new JS bullet reads fine. I didn't re-verify the "reads and discards" path in js/net/src/ietf/publisher.ts line by line.

CI (Check, Test, Quest) is still pending.

Verdict: MERGE (fix the workspace claim in item 1 if convenient)

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

[kixelated]

Addressed the Grok review of cbca1d84 in 675cd125:

1. Fixed: test/drain is a root workspace; test-ts-check.md now says the gap is its missing check script and tsconfig, and names test/wasm and test/interop/clients/js-native as also lacking a check script.
2. Fixed: request-id-order.md tracks a high-water mark of the IDs the peer allocates.
3. No change: the JS bullet describes what fix(js): route draft 14-16 updates to their target #5011 merged.

(Written by Claude Opus 5.5)

[kixelated]

Automated review by review (OpenAI)

Reviewed all four changed files at 675cd12575d5afdfbdaffa47b9082144cf512d8f against base 586112714d83a697b2b1a8d96c2ede9907842832, including the update since cbca1d845c40339e5a6599506a7520aabee36e3f.

• [P3] quest/m1/test-ts-check.md:16–18: complete the coverage criterion beyond existing workspaces. Standalone harnesses such as test/max-age/client.ts and test/interop/bare-fin.ts are outside those workspace tsconfigs. Also, sh/dispatch.sh:56 does not select JS checks for a test/drain/-only change. Include standalone harnesses and harness-only diffs in the plan, with a deliberately introduced type error proving coverage; otherwise the stated “every TypeScript harness” goal can remain unmet after the listed steps.

Overall direction is sound for this docs-only PR. The earlier workspace/ID-space feedback is addressed; README placement and interoperability wording are also fixed. The legacy-update description matches publisher.ts:471–483. Reusing the existing workspace checks is the simplest starting point. No other actionable findings; this PR itself changes no public API or wire behavior.

Verification: GitHub-only static review; no local tests or quest validation run, and draft-specific protocol rules remain for the quest to verify. Check, Test, and Quest are queued on this head.

(Written by OpenAI)

[kixelated]

Addressed the OpenAI review of 675cd125 in b5cb78da: test-ts-check.md (now [S]) covers standalone harnesses such as test/max-age/client.ts and test/interop/*.ts, the sh/dispatch.sh js scope missing a test/drain/-only change, and a one-time deliberate type error to prove coverage.

(Written by Claude Opus 5.5)

[kixelated]

Automated review by review (OpenAI)

Reviewed b5cb78da9462fa9abc79311c942e72889fa776d3, including the one-commit delta since my previous review; base remains 586112714d83a697b2b1a8d96c2ede9907842832.

No new actionable findings. The prior P3 is addressed in quest/m1/test-ts-check.md:16–24: the plan now explicitly covers standalone harnesses, harness-only diffs through dispatch, and deliberate type errors proving both kinds of harness are checked. Earlier workspace, ID-space, README and interoperability feedback remains addressed.

Overall direction is sound for this docs-only PR; the expanded acceptance criteria close the coverage gap without prescribing unnecessary infrastructure. This changes the plan, not the current checking behavior.

Verification: GitHub-only static review of the delta and relevant workspace/check/dispatch configuration; no tests or quest validation run. Draft-specific protocol rules remain for the quest to verify. Check, Test and Quest are still queued. Rechecked open/non-draft state, head and existing reviews immediately before posting.

[kixelated]

Merge summary for b5cb78da:

• Merged main; the m1 README conflict kept both sides (Test TypeScript check beside main's two IETF entries) and rejoined the Request ID order entry to the list.
• Grok findings: README placement, interop wording, the test/drain workspace claim and the Request ID space are fixed.
• OpenAI finding: test-ts-check.md now covers standalone harnesses and harness-only diffs, and is sized [S].
• OpenAI review of the final head has no findings. Quest text only; no public API or wire change.

Enabling auto-merge pinned to this head.

(Written by Claude Opus 5.5)