fix(net): drain IETF serves before session close

[kixelated]

Problem

Session::close() on IETF sessions closes immediately, dropping a finished track's final group and FIN. It also skips the shared one-second drain deadline.

Approach

Count dispatched SUBSCRIBE and FETCH serves until completion or cancellation, as lite::Publisher does with its owed count. The private ietf::Driver reports drained() once both its namespace withdrawals (already tracked on main) and those serves are done, so the existing session supervisor waits for both. Update close documentation and remove the completed quest.

Impact

• Session::close() now drains IETF sessions, with the same one-second timeout and abort behavior as moq-lite.
• No exported API shape or wire-format changes.

Alternatives

Converting the entire IETF protocol to a named state machine is unnecessary for this lifecycle fix; its existing boxed future remains inside a private driver.

Validation

• Three new paused-time IETF session tests fail before the fix and pass afterward: final-group delivery, live-track timeout, and abort during drain.
• Publisher tests cover dispatch before first poll, cancellation, malformed requests, and FETCH completion across draft-14, draft-19, and draft-20.
• nix develop --command just check passed, including 4,836 Rust tests and native/wasm feature builds.
• Cross-language interop is not required: this changes lifecycle accounting only, with no wire, FFI, or gateway changes.

Follow-ups

None identified.

(Written by GPT-6)

[kixelated]

Completed: IETF sessions now drain dispatched SUBSCRIBE and FETCH serves before closing, within the shared one-second deadline. The new paused-time regressions reproduce the dropped final payload, skipped deadline, and ineffective abort before this fix, and pass afterward. Dispatch accounting also releases on cancellation, malformed input, and FETCH completion.

nix develop --command just check passed, including all 4,836 Rust tests. The first run found two dangling references to the deleted quest in close-tail; both are corrected. No public API shape or wire changes, and no open implementation decisions or new follow-up quests.

Recommended next action: /quest-merge after review and CI. Left as a draft for the spawning session to coordinate.

(Written by GPT-6)

[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 54 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: e3ce4abb-6c69-480b-aa9a-4ff2d72265e3

📥 Commits

Reviewing files that changed from the base of the PR and between 585e496 and 10e846c.

📒 Files selected for processing (9)

• doc/lib/rs/moq-net.md
• quest/m1/README.md
• quest/m1/close-tail.md
• quest/m1/ietf-drain-before-close.md
• rs/moq-net/src/driver.rs
• rs/moq-net/src/ietf/publisher.rs
• rs/moq-net/src/ietf/session.rs
• rs/moq-net/src/session.rs
• rs/moq-net/tests/session_close.rs

• 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

• X
• Mastodon
• Reddit
• LinkedIn

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

[kixelated]

Merged current main (post trunk flip) and rebuilt the fix on main's IETF drain shape instead of restoring the old one:

• ietf::Driver (already on main, carrying withdrawal and local_close) gains the owed serve count; its new drained() is withdrawal.drained() && owed == 0, mirroring lite::Publisher::drained. Protocol::drained calls it.
• Docs (doc/lib/rs/moq-net.md, Session::close) drop "IETF media streams are not drained yet"; the quest is deleted and its README and close-tail references removed.
• The dispatch-count unit test now builds a draft-20 FETCH as FetchType::Filtered, since draft-20 dropped the Standalone form on main.

Checked: the three IETF session_close regressions fail with drained() reduced to withdrawals only and pass with the fix; cargo test -p moq-net (all pass), clippy -D warnings, rustfmt, and quest check are clean. No public API or wire change.

Decisions: none beyond the rebuild above. No review had landed (CodeRabbit is rate-limited); merging on the maintainer's approval in session.

(Written by Claude Opus 5.5)

[kixelated]

Automated review by review (OpenAI)

Reviewed commit: 461ae1b

No actionable introduced bugs found. The dispatch-time guard in rs/moq-net/src/ietf/publisher.rs:479-512 counts SUBSCRIBE/FETCH before their first poll and releases on malformed input, cancellation, and completion. Both IETF driver branches share that counter; the supervisor checks it together with withdrawal delivery. Existing serve paths retain the guard through media completion and request-stream close.

Direction: extending the existing private Driver is a proportionate fix for graceful-close accounting. It preserves the current timeout/abort policy without a broader protocol rewrite. The updated draft-20 test now uses the filtered FETCH form; the tests cover the important dispatch and lifecycle boundaries. This remains transport-acknowledgement draining, not a subscriber application-consumption guarantee.

Verification: static review of the current nine-file diff and surrounding publisher/session/supervisor code, including the adaptation to current main since the earlier read-only inspection. No tests run here; the reported validation was not independently reproduced. Rechecked open/non-draft state, head, and review history before submission.

(Written by review (OpenAI))

[kixelated]

Review: fix(net): drain IETF serves before session close

Makes IETF Session::close() wait on dispatched SUBSCRIBE/FETCH serves (plus existing namespace withdrawals) under the shared 1s drain, matching lite’s owed pattern via a RAII Serve guard and ietf::Driver::drained().

Findings

No issues found.

Drain accounting looks correct: count at dispatch (before first poll / decode), release on cancel, malformed refuse, and completion; Driver.owed is wired onto both draft-14–16 and draft-17+ publisher paths before tasks run; subscribe completion still waits on writer.close() for PUBLISH_DONE, so owed stays ack-tied like lite. Quest removal and doc updates match the code. New paused-time IETF session tests plus the publisher unit test cover the intended races.

CI: still pending at review time (Android, Check, Test, WASM, Windows, macOS, Quest; Auto-merge skipping).

Reviewed head SHA: 461ae1b04de9a8b7396f497d046503304bd24c75

MERGE

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

[kixelated]

Merge summary:

• The previous Check failure on 461ae1b was infrastructure: the hosted runner lost communication after ~59 minutes and no step log was retained. The same changes passed Check in 16 minutes on 9d126ec, and the only commits since were main merges plus a docs rewrap; Test passed on 461ae1b.
• Merged current main (clean, including fix(net): close IETF sessions on unknown bidi types #4703 and fix(net): cancel abandoned IETF group fetches #4711 in ietf/session.rs) instead of re-running the stale job, so CI now covers the interaction with those IETF changes.
• Grok review on 461ae1b: MERGE, no findings. Auto-merge re-enabled on 10e846c.

(Written by Claude Opus 5.5)