Repository navigation
quest: plan refusing a reused moq-transport alias at once - #8453
Conversation
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
kixelated
left a comment
kixelated
left a comment
There was a problem hiding this comment.
Automated review by review (OpenAI)
Reviewed ac02934. No actionable findings in this documentation-only plan and index entry.
Direction: immediate cleanup for the nonfatal alias-binding refusal matches the neighboring refusal paths. The existing OnceStep::Unbound retains the track until cancellation completes; the proposed never-acknowledged-FIN test targets that failure. Preserve the separate fatal Error::Duplicate behavior for an alias naming a different track when implementing the plan.
Verification limits: GitHub-only static review of both changed files and the relevant register_alias, refusal, and Unbound paths in subscriber.rs. No builds, tests, or runtime reproduction performed.
|
Warning Review limit reachedYou'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 11 minutes. View limit details
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. |
![pullfrog[bot]](https://avatars.githubusercontent.com/in/1768019?s=60&v=4){kind=small}
There was a problem hiding this comment.
ℹ️ No critical issues — one body-level note.
Reviewed changes
- New quest plan —
quest/m1/ietf-alias-refuse.mdplans failing a rejected SUBSCRIBE_OK alias's readers, removing the subscription, and aborting the track before waiting on the peer's FIN acknowledgement, then deletingOnceStep::Unboundin favor of the existingOnceStep::Refusepath. - m1 index entry —
quest/m1/README.mdlists the quest with a matching one-line summary at its rank.
I checked the plan's premise against rs/moq-net/src/ietf/subscriber.rs at this head. The Unbound arm really does defer remove_subscribe and track.abort until cancelling.poll is ready, and Closing::poll resolves only once the peer acknowledges everything written. The property-refusal and error branches just above it already clean up first and return Refuse. Placement in m1 matches the additive-only rule, and [S] fits the change.
ℹ️ The PR body is missing its ## Models section
The body ends at ## Review with no ## Models, so the PR's size and the planning session's implementer line are not recorded. Comparable quest-plan PRs (#8415, #8422) carry it above their ## Review section.
Technical details
# Missing `## Models` in the PR body
## Affected sites
- PR #8453 body — goes from `## Impact` straight to `## Review`
## Required outcome
- Add a `## Models` section before `## Review` carrying `Size: S` and an `Implementer:` line for this session (the body credits Claude Opus 5.5 with medium effort).deepseek-v4.1-flash-1017 (free via Pullfrog for OSS) ｜ 𝕏
Summary
Plans one m1 quest, A reused moq-transport alias refuses its track at once, from moq-dev/moq#8402's follow-ups: when
register_aliasfails after a SUBSCRIBE_OK, the track is only aborted once the peer acknowledges the FIN, so a peer that never does leaves readers on a dead copy and a registered entry.Decisions
Impact
Public API: none. Wire: none. Quest files only.
(Written by Claude Opus 5.5 with medium effort)
🤖 Generated with Claude Code
Review
Rewritten by
quest reviewafter each round.01a1c65c-34d6-7083-8a42-4d95623a52b5, round 1The documentation-only change adds a follow-up plan and index entry consistent with the existing delayed-cleanup path, with no actionable defects.