[kixelated]

Finishes quest quest/m2/api-shape/session-closed.md (deleted here).

Problem

A binding saw MoqError::Closed for a session that closed only when two endpoints shared one in-process origin (moq-dev/moq#8373). Across a real session the same event surfaced as a protocol error, so is_shutdown / IsShutdown treated an expected teardown as a failure:

How the session behind a track ended	moq-net error the reader gets	Binding saw	Binding sees now
Socket endpoint closed, same origin	Stream(Session(Cancel))	Closed	Closed
Peer's session closed upstream of a relay (lite)	Stream(Session(Internal)), a received SESSION_CLOSED reset	Protocol { Stream, SessionClosed, 0x3 }	Closed
Direct session closed cleanly, by either side, lite or moq-transport	Session(Cancel)	Protocol { Session, Cancel, 0x0 }	Closed
Direct session failed (for example Unauthorized)	Session(<code>)	Protocol { Session, <kind> }	unchanged

I measured each row with temporary probes over the mock transport before changing anything: lite-05, lite-07, moq-transport-18 and -22, with the publisher aborting, dropping, or closing, and the subscriber closing.

Approach

• rs/moq-ffi/src/error.rs: From<moq_net::Error> maps Session(SessionError::Cancel) and every Stream(StreamError::Session(_)) to Closed. A SESSION_CLOSED reset carries no reason, so a local cause and a received one now read the same. A session that failed keeps its scope, code, and kind.
• MoqProtocolKind::SessionClosed is removed, because no conversion can produce it any more. The Go ProtocolKindSessionClosed alias goes with it, and the checked-in Dart bindings are regenerated.
• No moq-net change: both wires already decode to these variants.

Tests:

• a_closed_session_is_closed_on_either_side (unit) replaces local_session_close_is_closed, which pinned a received SESSION_CLOSED to Protocol.
• a_closed_session_ends_its_tracks_as_closed (moq-ffi, in-memory sessions on lite-07 and moq-transport-18): a reader whose session is closed by the publisher or by itself gets Closed. Before this change it got Protocol { Session, Cancel }.
• closing_an_endpoint_ends_its_sessions (moq-net, through a relay) now asserts that the peer reads Stream(Session(_)), which is the moq-net contract the mapping relies on. Before, it asserted only is_err().

js/net and moq-c

• js/net: no change needed. It has no Closed error. A socket endpoint close and a received reset are both Stream(SessionClosed), and a clean session close is Session(Cancel) on either side, so local and peer closes already read the same.
• moq-c: no new kind. It reports both a local and a received SESSION_CLOSED as MOQ_ERROR_MOQ with {STREAM, 0x3, MOQ_PROTOCOL_KIND_SESSION_CLOSED}, and a clean close as {SESSION, 0, CANCEL}. Its callbacks already end with 0 for a clean end. moq-c has no local/peer split to fix, and a new status would break every C caller that matches MOQ_ERROR_MOQ.

Impact

• Public API (breaking, moq-ffi and every binding): a track or group whose session closed, cleanly or through a SESSION_CLOSED reset, raises Closed instead of Protocol. MoqProtocolKind::SessionClosed is removed from Rust, Python, Kotlin, Swift, and Dart, and ProtocolKindSessionClosed from Go. All later MoqProtocolKind ordinals shift by one, so regenerate the bindings together with the native library.
  • Migration: code that matched Protocol with kind SessionClosed, or with session-scope kind Cancel, should match Closed. is_shutdown / isShutdown / IsShutdown now cover these cases with no change.
• Wire: none.

Alternatives

• Keep SessionClosed as a kind that is never reported. Rejected: a kind no conversion produces is misleading API. Removing it turns stale matches into compile errors in the typed bindings instead of branches that silently never run.
• Map every Session(_) close, failures included, to Closed. Rejected: that would hide the reason (Unauthorized, Expired, ProtocolViolation) a caller can act on.
• Teach moq-transport to carry a session-closed track end. Rejected: PUBLISH_DONE has no status for it (see below), and AGENTS.md rules out moq-transport-only machinery.

Choices made without asking

• Removed MoqProtocolKind::SessionClosed, rather than keeping a variant nothing reports.
• Clean session closes (Session(Cancel)) map to Closed. Failed sessions keep their Protocol details.
• moq-c gets no Closed kind (reasons above).
• Corrected two claims in the quest's text. A relayed peer close over moq-transport does not arrive as SESSION_CLOSED (next section). The Python socket test the quest called skipped is no longer skipped since moq-dev/moq#8373.

moq-transport limit (not fixed)

When the peer whose session closed sits behind a relay, the relay ends a moq-transport subscription with PUBLISH_DONE INTERNAL_ERROR, because that registry has no session-closed status. The reader then sees Remote(0), which is an internal error in the bindings, not Closed. Direct moq-transport sessions are covered, since a clean close decodes to Session(Cancel). Mixed lite/moq-transport chains lose the cause at the moq-transport hop the same way. Per AGENTS.md, moq-transport is a compatibility layer, so this PR leaves it alone.

Follow-ups

Pre-existing failures that block a clean local just check on macOS. No open PR fixes them:

• moq-carla-bridge's module docs link items that are compiled only on Linux, so cargo doc -D warnings fails on macOS.
• The py/moq-rs docs build fails on the missing moq.media.catalog autosummary stub.
• sh/version.test.py: two release-cut tests fail on main.
• check cpp fails on macOS hosts on a -Wl,-lintl flag.

Checks

Run on macOS:

• Passed: cargo fmt --all --check; cargo clippy -p moq-ffi -p moq-net --all-targets -D warnings; cargo doc -D warnings -p moq-ffi -p moq-net; cargo nextest run -p moq-ffi (323 passed); moq-net socket:: integration tests (28 passed); just py test (127 passed).
• just check modules that passed: go, kt, swift, dart (Dart bindings regenerated with just dart generate), c, obs_compile, biome, markdown, gh, quest, shell, toml, nix, justfile, drill, large_files, text.
• just test interop --all: 182 cases passed. The only failures are the finish cpp -> * cases, run under env -u MOQ_QUEST because the session leaks that variable and the binaries reject it.
• Failures that reproduce unchanged on base origin/main at d9800c1:
  • check rs lint: cargo doc -D warnings -p moq-carla-bridge fails with three unresolved links. The crate is #![cfg(target_os = "linux")], so on macOS its module docs link items that are not compiled. Linux CI is unaffected.
  • check py docs: autosummary reports stub file not found 'moq.media.catalog' (py/moq-rs/docs/index.md:147).
  • check cpp: clang++: error: -Wl,-lintl: 'linker' input unused from the host toolchain.
  • test gh: two cases in sh/version.test.py (test_cut_promotes_only_published_library_dependencies_and_preserves_exemptions and test_release_plz_keeps_manual_cut_despite_breaking_commits).
  • Interop finish cpp -> *: all 9 fail on base too. moq-dev/moq#8407 quarantines them.

Review

• Reviewer: Codex, gpt-6.1-sol, medium effort (quest review)
• Session: 01a1c371-c2fa-7a12-959c-80ce712f71bd, round 1
• Head: 3be989f
• Base: origin/main at c734d2a
• Findings: 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 4 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: 6dce9c18-2cc3-4b52-9a03-de811b321fba

Commits

Reviewing files that changed from the base of the PR and between d9800c1 and 3be989f.

Files selected for processing: 7

• dart/moq_ffi/lib/src/moq.dart
• go/wrapper/types.go
• quest/m2/api-shape/README.md
• quest/m2/api-shape/session-closed.md
• rs/moq-ffi/src/error.rs
• rs/moq-ffi/src/test.rs
• rs/moq-net/tests/integration/socket.rs

• 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: 3be989f

No actionable findings.

Direction: the binding boundary is the right place to normalize clean session closes and reasonless SESSION_CLOSED resets to Closed. Direct session failures retain their structured reason. Removing the now-unreachable SessionClosed kind is consistent with the documented breaking API change. The regression tests cover both sides of direct lite/IETF sessions and the relayed lite reset contract.

Verification limits: GitHub-only static review of the conversion, wire mappings, affected tests, and binding impact. I did not run builds, binding generation, tests, or interop. The documented relayed moq-transport limitation remains outside this change.

[pullfrog]

✅ No new issues found.

Reviewed changes

• A session end reads as Closed from either side: From<moq_net::Error> folds Session(SessionError::Cancel) and every Stream(StreamError::Session(_)) into MoqError::Closed, so a clean close and a received SESSION_CLOSED reset read the same, while a session that failed keeps its scope, code, and kind.
• MoqProtocolKind::SessionClosed is removed: the variant and its stream_kind arm go, the Go ProtocolKindSessionClosed alias goes with them, and the checked-in Dart bindings are regenerated against the shifted ordinals (the lift's index + 1 cases still line up).
• Tests: the unit test pins both sides of the mapping and that a failed session keeps Unauthorized; a_closed_session_ends_its_tracks_as_closed covers publisher- and subscriber-side closes on moq-lite-07-wip and moq-transport-18; the moq-net socket test now asserts the Stream(Session(_)) variant the mapping relies on instead of is_err().
• Quest: quest/m2/api-shape/session-closed.md and its milestone bullet are deleted, and nothing else in the tree still links to the slug.

I checked the removed reason-carrying case: StreamError::Session only decodes from stream code 0x3, and moq-net's local construction sites pass Session(Cancel)/Session(Internal), so folding it into Closed cannot hide a reason a reader could otherwise act on; real failures still arrive as Error::Session(<code>).

ℹ️ The hand-written moq-c does not carry the Closed reading into the C line

c/moq and cpp/moq are generated from moq-ffi, so they adopt MOQ_ERROR_CLOSED for the session-end rows and drop MOQ_PROTOCOL_KIND_SESSION_CLOSED without a Rust change. rs/moq-c, the hand-written line that replaces them after this cut, keeps the kind and still reports a clean session close as MOQ_ERROR_MOQ with {SESSION, 0, CANCEL}, so a C caller moving between the two surfaces gets the old reading back. Nothing here is blocking — the track path already reads the same on both sides in rs/moq-c, which is what the quest asked for — but the generated-to-hand-written transition is where that difference will surface.

Technical details

# C surfaces disagree on how a session end reads

## Affected sites
- `rs/moq-c/src/error.rs:426` — `protocol_of_net` returns `Some` for `Error::Session(Cancel)` and `Error::Stream(StreamError::Session(_))`, both reaching callbacks as `MOQ_ERROR_MOQ`.
- `rs/moq-c/src/error.rs:129` — `MOQ_PROTOCOL_KIND_SESSION_CLOSED = 12` now exists only here; the generated `c/moq` header loses it with this PR.
- `c/moq/README.md` — generated from `rs/moq-ffi`, so `c/moq` and `cpp/moq` pick up `MoqError::Closed` automatically.
- `doc/lib/c/index.md:112` — documents `MOQ_ERROR_*` and `moq_error_protocol()` without describing a session end on either surface.

## Required outcome
- A caller migrating from the generated C package to `rs/moq-c` is not surprised by `MOQ_ERROR_MOQ` plus a protocol record where it used to get a closed code.

## Open questions for the human
- Is the verbatim `{scope, code, kind}` report deliberate for the low-level C API, or should `rs/moq-c` gain a `Closed`-shaped status at the cut? Either answer is fine here; a sentence in the C docs at the migration point would pin it.

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