fix(net): a truncated spliced group fails instead of ending cleanly

[kixelated]

Completes /quest/m1/finished-truncated.md.

A spliced group whose continuation can never arrive (its segment was pruned, or the successor skip-declared the group) used to report a clean end at the seam. Now it fails with the error the dead route recorded, or Error::Dropped when the seam's segment was pruned away and no route is left to blame.

Changes

• poll_finished's no-route arm returns the loss instead of Ok(cap), and its doc comment now says finished() answers for the original group.
• Beyond the quest's listed work: a reader that had already read up to the cap still got Ok(cap) from finished(), and read_frame()/next_frame() returned a clean None. Both went through give_up()'s Ok(false) arm. No test exercises a real clean end through that arm (the full suite passes without it), because a group only ends cleanly through its unbounded tail copy. So give_up() now always returns an error, and poll_current returns Result<()> instead of Result<bool>. The no-route arm in poll_finished reuses give_up(), so the warning is logged there too.
• Tests: two tests are renamed to finished_fails_* and now expect Dropped and NotFound. A new test, finished_fails_for_a_drained_pruned_bounded_group, covers the drained path for finished, read_frame and next_frame. It returned Ok(2) / Ok(None) before this fix.

moq-mux

container::consumer already treats a read error as a skip when poll_finished errors (poll_aborted). A truncated group now gets that treatment: it is dropped as aborted. Before, it was either read as a short clean group or, in the min/max timestamp paths, turned into a Decode(Short) error. There is no moq-mux test for a spliced group yet.

Public API / wire

• Public API: behavior only. group::Consumer::finished(), read_frame() and next_frame() on a resumed group return an error where a truncated group used to end cleanly. No signatures change.
• Wire: none.

Overlap

#4491 (resume-latest) landed first; main is merged in. The no-route arm in poll_finished now releases the replacement demand (waiting = None) before failing with give_up().

just check passes.

(Written by Claude Opus 5.5)

🤖 Generated with Claude Code

[kixelated]

Decisions confirmed by the maintainer:

• Keep the read-path change: give_up() always errors, so finished(), read_frame() and next_frame() fail on a truncated group instead of ending cleanly.
• A pruned seam with no route left to blame reports Error::Dropped.
• Don't thread the track's abort error through poll_covering.

This lands before #4491 (resume-latest), which will need to merge main and resolve the overlap in resume.rs.

(Written by Claude Opus 5.5)

[kixelated]

Automated review by review (OpenAI)

Reviewed 61f8304. No actionable findings.

Direction: making the no-route outcome uniformly fail is simpler and keeps finished(), read_frame(), and next_frame() consistent. The retained unbounded-copy path still provides clean completion; a lost continuation uses the recorded route error or Dropped. The read-path extension is necessary to cover an already-drained bounded copy, and the new regression exercises that case. The existing moq-mux read-error/poll_aborted path handles this outcome as a skipped group.

Verification: inspected the full diff, route resolution/pruning/latching, group cursor contracts, nearby regressions, and moq-mux consumers. I did not run builds or tests; Rust/Nix tooling is unavailable here. The reported local just check pass is not independently verified, CI is still queued/running, and there is no dedicated spliced-group moq-mux regression. Preserve these semantics when resolving the documented overlap with #4491.

[coderabbitai]

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Advanced

Run ID: 42b981bb-6539-4480-8cfb-9e93b6f8cac4

📥 Commits

Reviewing files that changed from the base of the PR and between 61f8304 and c238d86.

📒 Files selected for processing (3)

• quest/m1/README.md
• quest/m1/finished-truncated.md
• rs/moq-net/src/model/resume.rs

💤 Files with no reviewable changes (2)

• quest/m1/finished-truncated.md
• quest/m1/README.md

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

---

Walkthrough

Group::poll_current and frame reads now return route loss errors when no route can serve the current position. For bounded groups, Group::poll_finished checks the seam and returns a loss error if no route covers it. Tests cover pruned seams and successors that skip the seam. The related quest entry and documentation were removed.

Priority: ➖ Normal

Merge Risk: ⚪ Minimal · up to c238d

Unavailable continuations now fail instead of appearing complete, with regression tests covering pruned and skipped seams. No merge-blocking issue is established; merge after normal checks pass.

Security Architecture Review

Security architecture risk: 🔵 Low · up to c238d

Truncated groups now fail explicitly rather than appearing complete. The inspected recovery paths remain within the subscribed track, and no new security vulnerability was established. Additional continuation-subscription behavior and incomplete baseline coverage leave limited uncertainty.

Retained concerns
No architecture-level concerns identified.

Security review details

Security Blast Radius

• inferred — Publisher or route progress can make a group's seam unavailable. In the inspected downstream path, the resulting effect is failure or skipping within the same subscribed track, not selection of another track or peer. Peer authentication and broader service exposure were not established by this source slice.

Trust Boundaries and Controls

• observed — Recovered copies must match the requested frame position and are guarded by the logical subscription, drift anchor, and route bound before use. A copy starting above the requested seam is rejected as lagged rather than accepted as a contiguous continuation.

Resilience and Maintainability Implications

• observed — Inspected terminal paths release continuation demand on unavailable routes, explicit read bounds, clean read completion, and latency expiry. Completion also releases demand after an underlying success or error; neither completion branch uses error propagation before cleanup.

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Docstring Coverage	✅ Passed	Docstring coverage is 80.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 10 functions across 1 files.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Title check	✅ Passed	The title clearly and concisely describes the primary change: truncated spliced groups now fail instead of ending cleanly.
Description check	✅ Passed	The description directly explains the truncation behavior, affected APIs, error handling, tests, and related mux behavior.

✨ Finishing Touches

✨ Simplify code

• Commit to this branch
• Create a new PR

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

Autopilot is currently an internal CodeRabbit preview.

---

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]

MERGE — reviewed c238d86d0e50abfc06706b6c3c0c4603c3559908

Truncated spliced groups now fail (Dropped / dead-route error) from finished(), read_frame(), and next_frame() instead of looking complete at the seam. Making give_up() always error is the right companion to the quest’s poll_finished no-route change; a clean end still only comes from an unbounded tail copy.

Blocking

None.

Non-blocking

1. CI still pending on this head (Check/Test/WASM/Platform/Android queued at review time; Quest already green). Local just check is claimed but not independently verified here.
2. moq-mux spliced-group coverage is still absent on this PR (acknowledged). Main already has follow-up quest quest/m1/mux-truncated-group.md (quest: plan unknown bidi, lite early streams, mux truncated group #4692) to pin that the consumer skips these as aborted — land that after this.
3. mux min/max timestamp paths (container/consumer.rs ~643/661): they used to see Ok(cap) then synthesize Decode(Short); they now get the truncation error via ? (Dropped / route error). More accurate, but any caller matching on Short specifically will need to widen.
4. Head is behind main by 3 (early IETF streams, qmux bump, mux quest) — unrelated to resume.rs, but rebase/merge before land if you want a clean auto-merge.

Notes

• Merge resolution with fix(net): a group-only reader subscribes to its replacement copy #4491 looks correct: the no-route arm clears waiting before Err(give_up()), so replacement demand is not pinned after truncation.
• Renamed + new drained-path tests cover the three surfaces (finished / read_frame / next_frame) and the pruned vs skip-declared error variants.

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

[kixelated]

Merge summary: a spliced group whose continuation can never arrive now fails finished(), read_frame() and next_frame() (the dead route's error, or Error::Dropped for a pruned seam) instead of ending cleanly at the seam. Behavior-only public API change; no wire change.

#4491 landed first, so main is merged in (c238d86). The resolution keeps both changes: the no-route arm in poll_finished releases the replacement demand (waiting = None) and then fails with give_up(). Both quest entries are gone from quest/m1/README.md. cargo test -p moq-net and just check pass locally, and CI is green.

The automated review on 61f8304 had no findings. Only the merge resolution has changed since then.

(Written by Claude Opus 5.5)