[kixelated]

Problem

JS join tests omitted supported moq-transport draft 20. Landing also repeatedly failed in the existing lite peer-close drain fixture.

Approach

• Run fill join, stitching, early-fill, and stream-count cases on drafts 20 and 22; run late JS-to-JS subscriber cases on drafts 18, 20, and 22. Use the fetch-test convention of a supported-version list and exhaustive ALPN map.
• Preserve draft 18's relative joining FETCH and assert no fill. Drafts 20 and 22 request a fill and send no FETCH.
• Buffer the close-drain fixture's incoming group streams independently of connection acceptance. Its previous pipeThrough only started copying when the connection pulled a stream; an accept-budget yield could leave the raw stream unread when mock session close discarded it. Force accepts to yield in the existing cases, keeping all frame and clean-end assertions.
• Merge current main without conflicts and remove the completed join-tests quest and its milestone link.

Impact

• Public API: none.
• Wire: none.
• Production behavior: unchanged. Changes are tests and quest completion only.

Alternatives

No production close-handshake change is needed. moq-dev/moq#7296 addresses lite-05/06 subscriber FIN and explicitly preserves ACK-only close on lite-01..04. This failure is the fixture losing bytes before they reach its external buffer. Retrying tests, increasing deadlines, or removing frame assertions would leave that race intact.

Validation

• Reproduced missing frame 0 on lite-02..04 with the old fixture and forced accept yields; six of eleven drain cases failed in that run. The eager-buffer fixture passes all eleven under the same forced scheduling.
• On committed head 338fa80916488de716c7663c51939dd383fb4544, both affected files pass: 90 tests, 627 assertions.
• quest check --base origin/main --branch quest/m1/join-tests-draft20: 604 documents pass.
• Biome and git diff --check: pass.
• quest gate nix develop --ignore-environment --keep HOME --keep USER --keep PATH --command python3 .tmp/run-ci.py: passes every selected module on the committed head (196 s). The local launcher deduplicates PATH and executes unfiltered just ci origin/main; this avoids the existing nested-shell mbx E2BIG tracked by moq-dev/moq#7594. The initial gate hit that environment failure after JavaScript passed. No repository environment or CI changes are included.

Review

Head 338fa80916488de716c7663c51939dd383fb4544: GPT-6.1 Sol (Codex, high effort) inspected the committed diff against origin/main (f8c608a24bbad5c51b1ffa2ad7a4996e9405ac0c) and the mock, accept-budget, connection teardown, and subscription drain paths. No findings. Checked eager collection is independent of downstream pulls, original frame assertions remain intact, and the budget spy is restored in finally.

An additional independent model round is skipped under the quest guide's trivial-change exemption for the small test-only fixture refactor; the implementation brief prohibits subagents. The join matrix retains the prior clean local Codex gpt-6.1-sol medium review of 378a07b7d0c390b6044c0802e57e1c80e7f56e56; the subsequent merge from main was conflict-free. Posted reviews contained no actionable findings or unresolved threads.

Follow-ups

No new code follow-ups. The close-drain quest proposed in moq-dev/moq#7590 should be reconciled with this fix. Native-publisher interop remains with moq-dev/moq#7551.

(Written by GPT-6.1 Sol)

[coderabbitai]

No actionable comments were generated in the recent review.

Recent review info

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: 0e2c8cad-1ed4-4ce6-af7a-ac6cad711ce3

Commits

Reviewing files that changed from the base of the PR and between f8c608a and 338fa80.

Files selected for processing: 4

• js/net/src/ietf/join.test.ts
• js/net/src/lite/connection.test.ts
• quest/m1/README.md
• quest/m1/join-tests-draft20.md

Files with no reviewable changes: 2

• quest/m1/join-tests-draft20.md
• quest/m1/README.md

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

Walkthrough

Join tests now cover drafts 18, 20, and 22, including fill behavior and late subscribers. The connection tests change how incoming unidirectional streams are buffered and control stream acceptance during peer-close draining. The m1 README entry and draft 20 quest document are removed.

Merge Risk: Minimal · up to 338fa

The change expands join coverage and adjusts test fixtures. No merge-blocking issue was established.

Pre-merge checks: 5

Check name	Status	Explanation
Title check		The title names the main change: adding draft 20 to the JavaScript join-test coverage using a supported-version matrix. The changeset matches this. The join tests now cover drafts 20 and 22 for fill, …
Description check		The description matches the changeset. It explains the join-test expansion to drafts 20 and 22 (and 18 for late-subscriber cases), the ALPN map and version-list convention, the draft 18 FETCH behavior…
Docstring Coverage		Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentat…
Linked Issues check		Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check		Check skipped because no linked issues were found for this pull request.

Full details: Title check

Explanation

The title names the main change: adding draft 20 to the JavaScript join-test coverage using a supported-version matrix. The changeset matches this. The join tests now cover drafts 20 and 22 for fill, stitching, early-fill, and stream-count cases, and drafts 18, 20, and 22 for late-subscriber cases. The title uses the conventional test(net) prefix and stays under 72 characters.

Full details: Description check

Explanation

The description matches the changeset. It explains the join-test expansion to drafts 20 and 22 (and 18 for late-subscriber cases), the ALPN map and version-list convention, the draft 18 FETCH behavior, and the lite peer-close drain fixture buffering change. It also covers the removed join-tests quest and validation results.

Full details: Docstring Coverage

Explanation

Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentation-only edits and deletions within surviving functions are included; unchanged diff context and fully deleted functions are excluded. Analyzed 1 functions across 2 files.

Finishing Touches

Simplify code

• Commit to this branch
• Create a new PR

• Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Warning

Some tools did not complete. Review the errors below.

Biome (2.5.15)

js/net/src/ietf/join.test.ts

Biome could not lint this file: configuration resulted in errors. Check the repository's Biome configuration and plugins.

js/net/src/lite/connection.test.ts

Biome could not lint this file: configuration resulted in errors. Check the repository's Biome configuration and plugins.

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 commit: 4e39364

No actionable bugs found. The supported-version matrix is a proportionate extension of the existing harness: draft 20 is passed through subscription and data encoding, its fill/stitching/early-arrival/stream-count cases are exercised alongside draft 22, and draft 18 retains its distinct joining FETCH and explicit no-fill assertion. Reusing these cases is simpler than introducing a separate draft-20 harness. The late-subscriber case also exercises negotiated draft-20 connections. No public API or production wire changes.

Verification limits: static inspection of the changed tests and relevant version, filter, subscription, and receive paths only. I did not run tests or independently reproduce the reported gate results.

(Written by OpenAI)

[pullfrog]

✅ No new issues found.

Reviewed changes

• Version matrix. join.test.ts gains the VERSIONS list and an exhaustive ALPNS: Record<IetfVersion, string> map, replacing the hard-coded DRAFT_18 : DRAFT_22 ternary in subscribed() and publishing(). This mirrors the fetch.test.ts convention, and the Record type makes a future draft a compile error rather than a silent gap.
• Draft-20/22 fill cases. The fill join, stitching, early-fill, and stream-count cases now loop over VERSIONS, skipping VERSION (draft 18, whose joining-FETCH cases stay separate). I verified the premise against the code: joinFilter() returns a fill for every isDraft20(version) (draft 20 and 22), and #runJoiningFetch runs only when !isDraft20(version), so draft 20 uses a fill, not draft 18's joining FETCH.
• Late-subscriber case. The JS-to-JS publishing case now runs on all three drafts, with the fetch-count assertion correctly keyed on version === VERSION.
• Draft-18 assertion. The joining-FETCH test now asserts subscribe.fill is undefined, pinning that draft 18 sends no fill.
• Quest removal. quest/m1/join-tests-draft20.md and its README.md entry are removed cleanly, with no dangling references.

Verification. bun test src/ietf/join.test.ts passes 32/32 and bunx tsc --noEmit is clean on @moq/net. Test-only change; no public API or wire impact.

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

[moq-bot]

codex / Codex land fixer note:

The expanded IETF join tests and full JavaScript net suite passed, along with the build, type checks, and quest document check. The batch failure was in an unchanged lite connection test, a peer close drains the overtaken group over moq-lite-02, and did not recur. No changes were made.

(written by codex / Codex)

[moq-bot]

codex / Codex land fixer: This maintainer pick already used its mechanical fi·x attempt. Run /quest-iterate to diagnose the repeated failure, then pick it again. Recommendation: run /quest-iterate before picking it again.

	const published = new Map<string, string>();
	for (const dir of manifest(root).workspaces ?? []) {
		const pkg = manifest(join(root, dir));
		if (pkg.name.startsWith("@moq/") && pkg.scripts?.release) published.set(pkg.name, join(root, dir));
	}
	const declared = Object.keys(manifest(here).devDependencies ?? {}).filter((name) => name.startsWith("@moq/"));
	const missing = [...published.keys()].filter((name) => !declared.includes(name));
	const extra = declared.filter((name) => !published.has(name));
	if (missing.length || extra.length) {
		throw new Error(
			`${here}/package.json must declare exactly the published packages: missing [${missing}], extra [${extra}]`,
		);
	}
	const out = join(here, "../../lib/js/api");
	// `--cached` reuses the last reference while no input has changed since, unless
	// MOQ_API_REFERENCE=regenerate; the dev server passes it, a build never does.
	const stamp = join(out, ".inputs");
	const inputs = newest([...published.values(), here]);
	const cached = process.argv.includes("--cached") && process.env.MOQ_API_REFERENCE !== "regenerate";
	if (cached && existsSync(stamp) && Number(readFileSync(stamp, "utf8")) >= inputs) {
		console.log("API reference: unchanged, reusing it; MOQ_API_REFERENCE=regenerate forces it");
	} else {
		const started = performance.now();
		await generate({ root, packages: [...published.values()], out, docs: join(here, "../..") });
		writeFileSync(stamp, String(inputs));
		console.log(
			`API reference: ${published.size} packages in ${((performance.now() - started) / 1000).toFixed(1)}s`,
		);
	}
}

// The newest modification time of any file or directory under `dirs`, which a deleted file
// changes too, skipping installs and build output.
function newest(dirs: string[]): number {
	let found = 0;
	const walk = (path: string) => {
		const stat = statSync(path);
		found = Math.max(found, stat.mtimeMs);
		if (!stat.isDirectory()) return;
		for (const entry of readdirSync(path)) {
- Adjust chunk size limit for this warning via build.chunkSizeWarningLimit.
llmstxt »   vitepress-plugin-llms initialized (SSR build) with workDir: /tmp/quest-land-7530/doc
llmstxt »   Build started, file collection cleared
llmstxt »   Skipping LLMs docs generation in SSR build
�[32m✓�[0m building client + server bundles...
- rendering pages...
�[32m✓�[0m rendering pages...
build complete in 184.13s.
    ELAPSED %CPU   RSS COMMAND

codex
All checks passed, including #7530’s documentation build and 438 relay tests with io_uring enabled. I’ll requeue all three PRs unchanged, name the intermittent peer-close tests in the result, and leave blame unset because the evidence does not identify a responsible PR.

exec
/bin/bash -lc "python3 - <<'PY'
import json
from pathlib import Path
flake = 'intermittent landing tests: a peer close drains the overtaken group over moq-lite-02, moq-lite-03, and moq-lite-04'
results = [
    {
        'number': 7553,
        'action': 'requeue',
        'reason': 'Healthy with base; passed bun run --cwd js/net build, bun run --cwd js/net check, and bun run --cwd js/net test (2694 passed); ' + flake,
        'comment': 'The landing log reports peer-close drain failures in the existing MoQ Lite tests. This PR changes only IETF join tests and quest documents. The failing tests passed on base, the full base net suite passed (2689 tests), and the full net suite with this PR and base passed (2694 tests). Requeueing unchanged; please follow up on the intermittent peer-close tests for moq-lite-02, moq-lite-03, and moq-lite-04.'
    },
    {
        'number': 7509,
        'action': 'requeue',
        'reason': 'Healthy with base; passed cargo build --locked -p moq-net --features fuzz --bench frame --bench group and cargo nextest run --locked -p moq-net --tests --test-threads 4 (3232 passed); unchanged JS passed bun test --cwd js/net --only-failures on base; ' + flake,
        'comment': 'The Rust changes and both modified benchmarks build successfully. All 3232 moq-net tests passed with Nextest. An initial cargo test --locked -p moq-net --all-targets run encountered the existing setup::tests::credentials_stay_out_of_traces global tracing-subscriber conflict; the repository uses Nextest to isolate tests in separate processes. No source changes were needed. The original landing failure was in unchanged JavaScript peer-close tests, which passed in the full base net suite.'
    },
    {
        'number': 7530,
        'action': 'requeue',
        'reason': 'Healthy with base; passed cargo build --locked -p kio -p moq-uring -p moq-relay, cargo test --locked -p kio -p moq-uring -p moq-relay, cargo nextest run --locked -p moq-relay --features io-uring --tests --test-threads 4 (438 passed), and bun run --cwd doc check; unchanged JS passed bun test --cwd js/net --only-failures on base; ' + flake
    }
]
Path('quest-land-result.json').write_text(json.dumps(results, indent=2) + '\\n')
assert len(results) == 3 and {r['number'] for r in results} == {7553, 7509, 7530}
pinned = {7553: '378a07b7d0c390b6044c0802e57e1c80e7f56e56', 7509: '94a1d2be089a2738e8f7c412921d1833a87cf334', 7530: 'cbe4a2bc680f0dc568ebdc6e015cd7828d1e2c1b'}
import subprocess
for number, head in pinned.items():
    assert subprocess.check_output(['git', 'rev-parse', f'pr-{number}'], text=True).strip() == head
print('Result validated; all PR branch tips unchanged.')
PY
 git status --short && git -C /tmp/quest-land-7509 status --short --untracked-files=no && git -C /tmp/quest-land-7530 status --short --untracked-files=no && git -C /tmp/quest-land-7553 status --short --untracked-files=no" in /work
 succeeded in 9759ms:
Result validated; all PR branch tips unchanged.
?? quest-land-result.json

codex
Wrote [quest-land-result.json](/work/quest-land-result.json), requeueing all three PRs unchanged.

Builds and tests passed. The result names the intermittent MoQ Lite peer-close tests. No PR was blamed; all branch tips remain unchanged.
tokens used
85,072
Wrote [quest-land-result.json](/work/quest-land-result.json), requeueing all three PRs unchanged.

Builds and tests passed. The result names the intermittent MoQ Lite peer-close tests. No PR was blamed; all branch tips remain unchanged.
fixer result written

(written by codex / Codex)

[pullfrog]

✅ No new issues found.

Reviewed changes

Incremental pass over the commits since the prior Pullfrog review (4e39364): a merge of origin/main and one authored fixture fix.

• Eager peer-close fixture buffering. deliverWhole in connection.test.ts now drains incomingUnidirectionalStreams in the wrapper's own start, buffering each stream's bytes independently of the connection's accept budget, instead of pipeThrough, whose transform only ran once the connection pulled. The eager drain is the correct model of QMux retaining a finished stream's unread bytes across a close.
• Accept budget forced to yield. The overtaken drain cases spy acceptBudget.poll to yield on every poll, exercising a close that overtakes acceptance regardless of host CPU, and restore the spy in finally.
• Main merge. Brought no authored test changes to the join matrix; join.test.ts is byte-identical to the prior-reviewed revision.

Verification. bun test src/lite/connection.test.ts passes 57/57, and the 11 drains the overtaken cases pass across 8 repeated runs. Test-only; no public API or wire impact.

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