Skip to content

quest(m1): abandon moq-transport cluster-off - #4657

Merged
kixelated merged 3 commits into
mainfrom
quest/m1/ietf-cluster-off
Oct 4, 2026
Merged

kixelated merged 3 commits into
mainfrom
quest/m1/ietf-cluster-off

Conversation

@kixelated

@kixelated kixelated commented Oct 1, 2026 •

Copy link
Copy Markdown
Collaborator

Abandoned

Abandons quest/m1/ietf-cluster-off.md. Not worth it: letting moq-transport peers negotiate the optional MoQ Cluster extension is harmless enough that a 15-file change isn't justified.

The code change that stopped negotiating the extension was dropped. This PR now only deletes the quest and removes its references:

  • quest/m1/README.md: drop it from Required.
  • quest/m1/cluster-routing/README.md, transit.md: drop the "cluster links are moq-lite only" links to it.
  • quest/m2/ietf-cluster-peers.md: no longer says moq-transport peers are plain clients until it lands.

Public API: none. Wire: none.

(Written by Claude Opus 5.5)

🤖 Generated with Claude Code

@kixelated

Copy link
Copy Markdown
Collaborator Author

Completed quest/m1/ietf-cluster-off.md. Removed Rust and JS MoQ Cluster negotiation and routing payloads from moq-transport, retained plain-client lifecycles and authorization updates, and kept the extension draft with an implementation note. Cluster mesh tests now exercise lite links.

The six modern SETUP/announcement cases fail against the original negotiation and pass with this change. Nix checks, all 4,831 Rust tests, networking JS tests, draft checks, quest validation and the full interop matrix passed. No public API change; the wire impact is withdrawal of the optional cluster extension on drafts 17 through 22.

No unresolved decisions or new follow-up quests. The existing quest/m2/ietf-cluster-peers.md covers future extension restoration. This PR remains draft for review as requested.

(Written by GPT-6)

@kixelated
kixelated changed the base branch from release to main October 2, 2026 22:03
@kixelated
kixelated marked this pull request as ready for review October 3, 2026 16:01
@coderabbitai

coderabbitai Bot commented Oct 3, 2026 •

Copy link
Copy Markdown
Contributor

Review in Change Stack →

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

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 50 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: b69255e2-611a-4c38-944e-e2620ace01a1
📥 Commits

Reviewing files that changed from the base of the PR and between 43b2f2f and 6e27b2e.

📒 Files selected for processing (5)
  • quest/m1/README.md
  • quest/m1/cluster-routing/README.md
  • quest/m1/cluster-routing/transit.md
  • quest/m1/ietf-cluster-off.md
  • quest/m2/ietf-cluster-peers.md

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: 1a33eab8-b751-4365-954c-b2b15d2c6aa3
📥 Commits

Reviewing files that changed from the base of the PR and between e2e77a4 and 43b2f2f.

📒 Files selected for processing (5)
  • quest/m1/README.md
  • quest/m1/cluster-routing/README.md
  • quest/m1/cluster-routing/transit.md
  • quest/m1/ietf-cluster-off.md
  • quest/m2/ietf-cluster-peers.md
💤 Files with no reviewable changes (3)
  • quest/m1/README.md
  • quest/m1/cluster-routing/transit.md
  • quest/m1/ietf-cluster-off.md

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


Walkthrough

The quest documentation removes the M1 plain-client quest entry and its proposal document. It also removes references that describe cluster links as lite-only. The M2 quest now describes a moq-transport relay joining a cluster as a peer through an extended cluster draft.

Priority: ⬇️ Low

Merge Risk: ⚪ Minimal · up to 43b2f

This documentation-only change frames peer participation as planned quest work and leaves the related routing policies coherent. No material merge risk is evident.

Security Architecture Review

Security architecture risk: ⚪ Minimal · up to 43b2f

The changes revise future cluster-peer plans rather than runtime negotiation or authorization. The session-scoping, grant-based authorization, and member-safe failover requirements remain intact. No material security risk is shown to be introduced or worsened by this PR.

Retained concerns
No architecture-level concerns identified.

Security review details

Security Blast Radius

  • observed — The documented authority boundary is broad for cluster-peer links, which may advertise any node, but session-scoped for clients unless grants authorize additional node identities. The stated threat is an authorized publisher attracting another node’s subscriptions through a cheap route advertisement; its runtime reachability is not established.

Trust Boundaries and Controls

  • observed — The unchanged plan requires scoped client identities not to shadow trusted routes. Promotion depends on node scope in a hop-bound peer grant, and promoted links remain subject to upstream and down-only restrictions. The grant shape and enforcement remain planned rather than demonstrated controls.
🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly states that the moq-transport cluster-off quest is being abandoned, which matches the main change.
Description check ✅ Passed The description explains that the quest is deleted and its references are removed, matching the changeset.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 0…
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.
✨ Finishing Touches 💡 1
⚔️ Resolve merge conflicts 💡
  • Resolve merge conflict in branch quest/m1/ietf-cluster-off
✨ Simplify code
  • Commit to this branch
  • Create a new PR
  • 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

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

Letting moq-transport peers negotiate the optional cluster extension is
harmless enough that a 15-file change isn't justified.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@kixelated
kixelated force-pushed the quest/m1/ietf-cluster-off branch from 12f4101 to 2b135c3 Compare October 3, 2026 16:28
@kixelated kixelated changed the title fix(net): keep moq-transport peers outside clusters quest(m1): abandon moq-transport cluster-off Oct 3, 2026
@kixelated

Copy link
Copy Markdown
Collaborator Author

Dropped the code change: the branch is now main plus only the quest deletion. Letting moq-transport peers negotiate the optional cluster extension is harmless enough that the 15-file change isn't worth it.

(Written by Claude Opus 5.5)

@kixelated

Copy link
Copy Markdown
Collaborator Author

Review (head 2b135c3bf344d36bdd8ca4b29bfe333882345b52)

Quest abandon only: deletes quest/m1/ietf-cluster-off.md and drops its references. Matches the updated PR body; no wire/API change. Extension negotiation and doc/bin/relay/cluster.md on main stay as-is, which is consistent with abandoning the turn-off.

Blocking

  1. quest/m2/ietf-cluster-peers.md still says cluster links are lite-only. Plan still has: “and its selection, since cluster links are lite-only there,” pointing at the cluster-routing line. This PR removes that lite-only claim from quest/m1/cluster-routing/README.md and transit.md, so the clause is now false relative to the m1 line it cites. Drop or rephrase the “lite-only” premise (the dependency on the route layer can stay without it).

Non-blocking

  • CI (Check / Test / Quest) was still queued at review time.
  • The 2026-10-01 completion comment still describes the dropped 15-file code change; the newer abandon comment corrects it, but the old one can confuse skimmers.

Verdict: ITERATE

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

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@kixelated

Copy link
Copy Markdown
Collaborator Author

Merged main in and resolved the quest/m1/README.md conflict: main already deleted the finished epoch quest, and this PR drops ietf-cluster-off.md, so both lines go.

Review nit: quest/m2/ietf-cluster-peers.md no longer says "since cluster links are lite-only there". No other references to quest/m1/ietf-cluster-off.md remain. quest check passes.

Public API: none. Wire: none.

(Written by Claude Opus 5.5)

@kixelated
kixelated enabled auto-merge (squash) October 4, 2026 05:47
@kixelated

Copy link
Copy Markdown
Collaborator Author

Re-review (head 43b2f2f06c01da61606b88992a80d712a17e772b)

Follow-up after push from 2b135c3bf344d36bdd8ca4b29bfe333882345b52 (merge main + drop stale lite-only claim). PR-unique delta is the quest/m2/ietf-cluster-peers.md rephrase; the rest of the compare is base-branch noise.

Earlier findings

  1. Blocking — m2 still said cluster links are lite-only: fixed. Plan no longer has “since cluster links are lite-only there.” ietf-cluster-off links are gone from the m1 README / cluster-routing files, and the deleted quest file is absent on this head.

Non-blocking

  • Check / Test still in progress at review time (Quest already passed).
  • The 2026-10-01 completion comment still describes the dropped 15-file code change; the later abandon / merge comments correct it.

No new issues in the PR-unique quest edits.

Verdict: MERGE

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

…ests

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@kixelated

Copy link
Copy Markdown
Collaborator Author

Merged main again after #4761 rewrote the cluster quests. Took main's versions and removed the ietf-cluster-off.md references they reintroduced: the m1 Required line, the cluster-routing "moq-lite only" clause, and in ietf-cluster-peers.md the "Until this lands, moq-transport peers are plain clients" paragraph plus the "again" in its Goal, since moq-transport peers keep negotiating the extension. quest check passes.

(Written by Claude Opus 5.5)

@kixelated
kixelated merged commit aac5eba into main Oct 4, 2026
4 checks passed
@kixelated
kixelated deleted the quest/m1/ietf-cluster-off branch October 4, 2026 06:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant