Repository navigation
feat(studio): renew a joined guest's token while they stay in the room - #28
Conversation
Quest-Claim: 735f2244e053282e
moq's main already renews a token on a live session, but the published @moq/net and moq-relay (moq's release branch) have neither the Auth handle nor in-band renewal. Add a waiting quest for that release, require it from guest-refresh, and record the delivery-track design found while scoping. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The join page took @moq/* from npm, which publishes moq's release branch, so any studio feature needing newer moq JS waited on a release cut. Its @moq/* dependencies are now workspace links into moq/js, like the Rust crates' path dependencies: the workspace lists every moq package they reach, installs hoisted as moq does so the submodule's worklet plugin resolves its tools, and builds @moq/libav on install. The join page's deploy checks out the submodule and redeploys when the gitlink moves, and `just upgrade-moq` refreshes bun.lock too. Drop the waiting quest for a moq release, which this makes moot. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
A signed guest token cannot be revoked and the relay ends a session at its expiry, so a show could not outlast --guest-expiry. While a guest is in the room, the studio now signs them a fresh token every half --guest-expiry and publishes it at `.renew/<identity>`, over a session of its own in the room that the room key signs for and that renews its own token the same way. A guest's token subscribes only to its own renewals, so no guest reads another's. The join page offers moq-lite-07, carries its token in SETUP rather than the URL, and presents each renewal in band without reconnecting, storing it so a reload rejoins with it. Remove from scenes stops a guest's renewals, so their last token bounds how long they stay; a new link resumes them. The Guests panel reports when the renewing session cannot connect. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
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 5 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. |
kixelated
left a comment
There was a problem hiding this comment.
Automated review by review (OpenAI)
Reviewed head: aeb8438
- [P1] Protect renewal JWTs from previously issued room-wide subscriptions (rs/moq-studio/src/guests.rs:615–620). Before this change, guest links used moq_room::claims with subscribe
**. An unexpired old link can therefore directly request.renew/<other-guest>and read that guest's new bearer token, then publish as them. Hiding the announcement does not restrict a direct subscription, and narrowing only newly minted claims does not change existing signed tokens. The code explicitly supports guests invited before this session, so upgrade/restart can expose their peers. Put renewals behind an authorization boundary that excludes old broad grants, or invalidate/rotate those grants before publishing, and test with a pre-change guest token. - [P2] Preserve removals across a Studio restart (rs/moq-studio/src/guests.rs:249–255, 330–335).
removedlives only in the in-memory participant map. If Studio restarts after removing a guest but before their last token expires, the still-online camera is observed as a default, unremoved participant and immediately receives fresh tokens again. They can remain connected indefinitely, contrary to the stated expiry bound. Persist the removal/revocation state until an explicit new invite, and test remove → restart while the old session remains online. - [P2] Schedule renewal before the actual whole-second expiry (rs/moq-studio/src/guests.rs:542–545). One-second expiry is accepted, but signing floors the issued time to whole seconds. For example, starting at T+0.75 gives the studio session a token expiring at T+1.0 while its first renewal is at T+1.25; even earlier phases can mint the same expiry at the half-second tick. The renewal session can expire before it can refresh, dropping active guests. Base the schedule on the actual encoded expiration with a safety margin, or reject lifetimes too short to renew reliably, and test fractional wall-clock phases.
Verification: static review of the PR and pinned MoQ sources only; I did not run a relay, browser, or timing tests.
There was a problem hiding this comment.
Important
Two user-facing surfaces still assume the original link's token is the guest's last: the README says a link works only until it expires, and the Guests panel keeps showing and re-sharing the original link after renewal replaced it. Neither breaks the protocol, but both mislead the host about who can still be in the room.
Reviewed changes
rs/moq-studio/src/guests.rs— addsRenewal/Renewals: the studio dials a room session under the room key, signs each joined guest a fresh token every half--guest-expiry, and publishes it on.renew/<identity>; guest tokens now subscribe only to their own literal renewal path.js/join— the join page offersmoq-lite-07, carries its token in SETUP, follows.renew/<name>in band, presents each renewal without reconnecting, and stores it so Leave and reload rejoin with the newest token;@moq/*now comes from themoqsubmodule instead of npm.rs/moq-studio/src/ui.rsandmain.rs— the Guests panel surfaces renewal failure and "Remove from scenes" stops renewal; CLI wiring for the new options.- Build, CI, docs, and quests —
bun.lock,bunfig.toml,vite.config.ts,join.yml,justfile,sh/upgrade-moq.sh, README, and the quest tree updates.
ℹ️ Undoing Remove from scenes leaves the guest un-renewed
stop_renewing sets Participant::removed, and only Room::invite clears it; undoing the scene edit restores sources and mutes but never touches that flag. A host who undoes the removal sees the guest back on air, yet the guest is still dropped within --guest-expiry and cannot be renewed again with their current token. The README calls removal "your edit, so it undoes", which invites that expectation.
Technical details
# Undo does not resume renewal
## Affected sites
- `rs/moq-studio/src/ui.rs:1374` — `room.stop_renewing(&name)` runs beside the scene draft, outside the edit history.
- `rs/moq-studio/src/guests.rs:249-254` — `stop_renewing` sets `removed = true`.
- `rs/moq-studio/src/guests.rs:128-131` — `Participant::renewed()` requires `!removed`.
- `rs/moq-studio/src/guests.rs:239-241` — only `invite()` clears `removed`.
- `README.md:199-202` — "Remove from scenes is your edit, so it undoes" followed by the disconnect behavior.
## Required outcome
- Either undoing the removal also resumes renewal (clear `removed`), or the UI and docs state that the removal is not undoable for the connection and only **New link** resumes it.
## Open questions for the human
- Is a guest returning via undo supposed to be renewed again without a new link? If not, the undo affordance should say so.big-pickle (free) | 𝕏
…expiry Address review of #28: - A link signed before renewals existed subscribes to the whole room, as can anything else the relay lets subscribe there, so a path grant alone could not keep renewals private. Each guest's renewals are now encrypted with moq-e2ee to a key derived from the room key and carried only in that guest's links, and published under a name only that key derives. The join page decrypts them with the key from its link. - Removals from scenes are kept beside the scene file (`show.guests.toml`), so a restart does not renew a removed guest until a new link. - Tokens expire rounded up to the whole second, and --guest-expiry must be at least a minute, so a renewal every half expiry always reaches a guest, and the studio's own session, before the token it replaces expires. - The Guests panel shows a renewed guest as renewing rather than their link's expiry, offers Show link only while the link works, and says undo does not resume renewal; the README says a link holder stays while renewed. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
# Conflicts: # rs/moq-studio/Cargo.toml
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Design change: guest renewals are now encrypted with moq-e2ee. A path grant alone could not keep them private, because anything else allowed to subscribe in the room can read them. That includes a link signed before this PR, whose claims subscribe Responses to the automated review of aeb8438:
(Written by Claude Opus 5.5) |
kixelated
left a comment
There was a problem hiding this comment.
Automated review by review (OpenAI)
Reviewed head: 253754dd70e6139e6463eff1b029eb9b5c12dcf2, following aeb84387d11d3ad700ffbd24cccd476f65840db5.
The earlier cross-guest disclosure is addressed by encrypting renewals to each guest, removal is persisted across restart, and expiration rounds up with a one-minute minimum. Two issues remain in the revised flow:
- [P1] Rotate renewal access when re-admitting a removed guest (
rs/moq-studio/src/guests.rs:332–346,369–379). A new link for a name reuses the same identity-derived key and renewal path and clears its removal. If someone copied Alice's old link and still has a valid token when the host removes Alice and issues her a new link, that holder can decrypt the new renewals and stay connected indefinitely without receiving the new link. The new publication epoch does not revoke the old secret; this also contradicts the promise that an earlier link works only until it expires. Rotate a persisted per-invite generation or secret and its renewal path on re-admission, retaining it across restarts. Test a copied old link against the new invitation. - [P2] Continue after a renewal group is superseded (
js/join/src/renew.ts:64–73). With the pinned MoQ client's default zero max delay, a partially read group can throwOldwhen the next group arrives. That exception exits the only renewal loop and closes its track; the effect still sees the same active broadcast, so it does not restart. A slow renewal followed by a good one can therefore leave a healthy session with no further tokens until it expires. Handle a superseded group inside the loop and continue to the newer group; cover an incomplete group followed by a complete one.
Verification: static GitHub inspection of Studio and the pinned MoQ sources and tests. I did not run a relay, browser, fault-injection test, or timing test.
(Written by review (OpenAI))
The maintainer picked documenting a room key rotation over moq-e2ee for links issued before renewal, whose claims subscribe to the whole room. Drop moq-e2ee, `@moq/e2ee`, and the link's `key=`: renewals are published in the clear at `.renew/<name>`, which only that guest's new token may subscribe to. The README tells operators upgrading to rotate the room key, and to grant subscribe in the room only to the studio. Persisted removals, the one-minute --guest-expiry minimum with rounded-up expiries, and the Guests panel fixes stay. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
There was a problem hiding this comment.
ℹ️ No new issues in the incremental delta. I'm not approving because the author's OpenAI review at this same head (5762960491) still has two open findings; they aren't re-raised here.
Reviewed changes
- Per-guest encrypted renewals — derives each guest's renewal key with HMAC over the room key's private material, publishes tokens on a credential-named
.renew/path, and narrows new guest tokens to subscribe only to their own renewals. - Persisted removals — writes removed guests to
<scene stem>.guests.tomland reloads them across restarts, and makesstop_renewingtake effect only once the temp-file write is persisted. - Whole-second renewal timing — floors
issued, roundsexpiresup, and rejects--guest-expiryunder a minute so each renewal lands before the token it replaces expires. - Guests panel and docs — shows "renewing"/"not renewed", hides Show link once the original link expires, and rewrites the README and hover text for the new lifetime and undo behavior.
- Cross-language path check — asserts the Rust and
js/joinrenewal-path vectors name the same broadcast.
big-pickle (free) | 𝕏
|
Design change: per the maintainer's pick, the OpenAI P1 (links issued before this version read other guests' renewals) is answered by rotating the room key, not by encryption. Commit 39693b4 removes the moq-e2ee encryption added in c9e7e92: no
The other fixes stay:
(Written by Claude Opus 5.5) |
kixelated
left a comment
There was a problem hiding this comment.
Automated review by review (OpenAI)
Reviewed head: 03800b745d00a1d70561c52cf0568c0c49ef9892, following 253754dd70e6139e6463eff1b029eb9b5c12dcf2.
[P1] End existing legacy sessions before publishing plaintext renewals (README.md:192–195). The selected upgrade procedure removes the old verification key, which blocks new admissions, but an already admitted old guest with subscribe ** can retain its relay grant until expiry. The pinned moq auth serve has revalidation off by default, and its lease driver does not re-read the key on file changes. If that guest is connected while the upgraded studio starts, it can read another guest's new plaintext JWT and continue as them after the old token expires. Keep the chosen key-rotation design, but require the operator to terminate or force revalidate and confirm all affected old sessions have ended before publishing renewals, or wait for all old grants to expire. Test an admitted legacy session across key removal and upgrade.
The two findings in the previous review remain in the new design:
- A new link clears removal but uses the same
.renew/<identity>grant, so a holder of an earlier still-valid token for that identity can read the new renewals without the new link (rs/moq-studio/src/guests.rs:316–319,341–343,356–361). Separate re-admission with a persisted invitation generation/path if old holders must be excluded. - A superseded partial renewal group can still throw
Oldand end the only reader task, leaving later tokens unread (js/join/src/renew.ts:43–50). Handle the per-group supersession and test an incomplete group followed by a good one.
Verification: static GitHub inspection of the revised Studio code and pinned MoQ auth/track sources. I did not run a relay, browser, migration, or fault-injection test.
(Written by review (OpenAI))
There was a problem hiding this comment.
ℹ️ No new issues in the incremental delta. I'm not approving because the author's OpenAI review at this head (5763075754) still has an open P1 on the upgrade window; it isn't re-raised here.
Reviewed changes
- Plaintext renewals replace per-guest encryption — drops
moq-e2ee/rindand the identity-derived key, publishing each guest's JWT as a plaintexttokenframe at.renew/<identity>; new guest tokens still subscribe only to that literal path. - Room-key rotation is the upgrade path — the README now tells operators to generate a new room key and drop the old public half so pre-PR
**-subscribe links die, instead of encrypting renewals to each guest. - Dependency and test cleanup — removes
@moq/e2ee/moq-e2eefrom the workspace,bun.lock, andCargo.lock, and rewrites the join page test for the unencrypted path, dropping the cross-language path vector.
big-pickle (free) | 𝕏
Address the review of 03800b7: - New link after Remove from scenes reused `.renew/<name>`, which an earlier token for that name could still read. Renewals now live at `.renew/<name>/<generation>`; letting a removed guest back in bumps the generation, kept in `show.guests.toml`, and the link carries it. - The join page skips a renewal group superseded before it completed (`Old`) rather than ending its only reader. - The README's upgrade steps end sessions the relay already admitted with an old link (restart the relay, or wait out their expiry) before the upgraded studio starts. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Responses to the OpenAI review of 03800b7 (5763075754):
(Written by Claude Opus 5.5) |
kixelated
left a comment
There was a problem hiding this comment.
Automated review by review (OpenAI)
Reviewed head e2b4183bddc4b3472754eec30f679c10dfc5fe80 against 03800b745d00a1d70561c52cf0568c0c49ef9892 and the updated base. The upgrade instructions now require ending admitted legacy sessions, the re-admission path advances a persisted generation, and the renewal reader continues after an Old group. Those address the three issues in my previous review for the same persisted show.
P2: Keep revocation generations scoped to the room, not only the scene file (rs/moq-studio/src/guests.rs:93–103, 241–252, 320–337, 445–455). Tokens authorize .renew/<identity>/<generation> under the relay room and signing key, but the removal/generation state is selected from the scene file's name, and a missing sidecar defaults to an empty state. If Alice is removed while her generation-0 token is still valid, then the operator starts another scene file (or renames the existing one without its sidecar) using the same relay room and key, Alice's still-announced camera is accepted as an unremoved generation-0 participant. The new studio publishes .renew/alice/0 again, which her old token can read, so she can keep renewing without a new invitation. Persist revocation/generation state with the room/key across scene files, or establish and enforce a fresh authorization namespace when switching shows. Add a remove → switch scene file in the same room → old token cannot read new renewals test.
Verification: GitHub-only static inspection of the Studio and pinned MoQ paths and tests. I did not run a relay, browser, migration, or fault-injection test.
(Written by review (OpenAI))
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
There was a problem hiding this comment.
ℹ️ No new behavioral issues in this incremental pass (
03800b7 → e2b4183). I'm not approving because the author's OpenAI review at this head (5764232822) still has an open P2 about scoping revocation state to the room rather than the scene file; it isn't re-raised here. One stale-doc nit is inline.
Reviewed changes
- Generation-scoped renewals — a new link after Remove from scenes bumps a persisted per-guest generation, and the studio publishes at
.renew/<identity>/<generation>; the link carries the generation, so an earlier token for that name cannot read the new renewals (rs/moq-studio/src/guests.rs,js/join/src/{link,index,renew}.ts). - Persisted generation —
show.guests.tomlnow storesgenerationbesideremoved(with#[serde(default)], so existing sidecars still load), and the renewal loop retains by(identity, generation). - One write path for removal and re-admission —
Room::changereplacesset_removed, bumping the generation only when the guest is removed, signing forgeneration + u64::from(removed)before mutating, and restoring the wholeParticipantif the sidecar write fails. Old-group continuation —renew.tscatchesNet.Error.StreamwithNet.StreamCode.Oldaround the group read, so a superseded partial group no longer ends the only reader;renew.test.tscovers an incomplete group followed by a complete one.- Docs — the README upgrade steps now require ending already-admitted legacy sessions before the upgraded studio starts, and describe re-admission under a new generation.
big-pickle (free) | 𝕏
kixelated
left a comment
There was a problem hiding this comment.
Automated review by review (OpenAI)
Reviewed head a8f555ddd7fc73fe98e6f346450679b74706e11a against e2b4183bddc4b3472754eec30f679c10dfc5fe80 and the updated base. The guest code is unchanged; the update incorporates the new MoQ submodule pin and refreshes the JS workspace lockfile. I found no additional actionable issue in the reviewed renewal/auth integration with that pin.
The scene-file revocation-generation issue in my previous review remains: changing scene files while reusing the room and key can restore an old generation's renewal path. The base merge does not alter that state scope.
Verification: GitHub-only static inspection of the lockfile, Studio guest paths and relevant pinned MoQ auth/token lifecycle. I did not run the relay, browser, or tests.
(Written by review (OpenAI))
A removal bounds tokens the room key signed, but it was kept beside the scene file, so opening another scene file for the same room renewed a removed guest again. Keep it beside the room key instead (`room.guests.toml` for `room.jwk`), and bring the renewal path's docs up to `.renew/<identity>/<generation>`. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Response to the OpenAI review of a8f555d (5764232822):
(Written by Claude Opus 5.5) |
There was a problem hiding this comment.
✅ No new issues found.
Reviewed changes
Changes since the prior pullfrog review (a8f555d → d30e16c):
- Revocation state moved beside the room key — removed guests and their generations now persist to
room.guests.tomlforroom.jwkinstead of<scene stem>.guests.toml, so opening a different scene file in the same room no longer re-admits a still-connected removed guest at generation 0. - Scene path dropped from
Config::load— guest configuration no longer keys removals off the scene file;main.rsand the restart test were updated (the test now useskeys/room.jwkandkeys/room.guests.toml). - Stale renewal-path docs corrected — the module doc,
RENEWdoc, andquest/m0/guest-return.mdnow name.renew/<identity>/<generation>.
This addresses the last open finding (the earlier OpenAI P2 on scene-file-scoped revocation state) and the outstanding Pullfrog doc nit.
big-pickle (free) | 𝕏
kixelated
left a comment
There was a problem hiding this comment.
Automated review by review (OpenAI)
Reviewed head d30e16c959c3a13df6a46cdc06cbd70c322093f5 against a8f555ddd7fc73fe98e6f346450679b74706e11a. The previous scene-file revocation issue is addressed for a room key kept at the same path: removal and generation state now live beside that key, independently of the selected scene file, and the restart test exercises the persisted state. No new actionable issue found in this update.
Operationally, that state is tied to the key file's path; if the key is moved or copied to another path, its companion .guests.toml must be preserved with it to retain removals and generations.
Verification: GitHub-only static inspection of loading, persistence and the tests. I did not run a relay, browser or migration test.
(Written by review (OpenAI))
Brings in #28 (guest token renewal) beside the recordings directory. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

Completes Guest token refresh: a show can outlast
--guest-expiry. While a guest stays in the room, the studio renews their token in band. The host sends no new link, and the guest doesn't reconnect.The maintainer picked building
js/joinon themoqsubmodule's JS now, over waiting for a moq release (relayed by the coordinator), and made "use the moq submodule going forward" a standing rule. Both are applied here, and the waiting quest from the first push is dropped.Changes
The join page builds on the submodule's JS (
1afe9b4)js/join's@moq/net,@moq/publish,@moq/roomand@moq/signalsare nowworkspace:*links intomoq/js, like the Rust path dependencies. The root workspace lists every moq package they reach.bunfig.tomlinstalls hoisted, as moq does, so the submodule's worklet Vite plugin findsrolldown.postinstallbuilds@moq/libav(nix buildof moq's flake), as moq's root does. It isn't on npm.vite.config.tsuses moq's worklet plugin, andtsconfig.jsonincludes its?workletdeclarations andvite/client.check/testscripts filter to./js/*, so they skip moq's own packages.join.ymlchecks out the submodule and redeploys when the gitlink moves.just upgrade-moqnow refreshesbun.locktoo. The README's Development section notes the submodule and Nix are needed.Renewal (
ee34030)<identity>/**, but it can now subscribe only to.renew/<identity>/<generation>, where its renewals are published. It was moq-room's**..renew/**, nothing else).--connectcredentials: no URL,--connect-tokenor mTLS identity.moq-lite-07-wipand renews its own token in band.--connect-bindport is refused with guests, since the two sessions would share it.--guest-expiry. The studio publishes one the moment a guest joins (camera announced), then every half expiry. It holds one track per guest in the room.room.guests.tomlbeside the room key (room.jwk), so neither a restart nor opening another scene file renews a removed guest. A failed write leaves the guest as they were and reports the error.generation=). A token from before the removal cannot read the new renewals. Undo of Remove from scenes does not resume renewal.--guest-expirymust be at least a minute. A renewal every half expiry then always lands before the token it replaces expires.moq-lite-07-wip, carries its token in SETUP (no longer in the URL), and follows.renew/<name>/<generation>. It skips a renewal group superseded before it completed (Old) instead of ending its reader. It replaces its connection's tokens with each renewal (presented in band, no reconnect) and stores the newest so a reload rejoins with it.mainaccepting--listen-version moq-lite-07-wip, and that the join page speaks only that WIP wire. It tells operators upgrading to rotate the room key and end sessions the relay already admitted (restart the relay, or wait out the old links' expiry) before starting the upgraded studio, and to grant subscribe in the room only to the studio. Updated--guest-expiry, Remove from scenes and the panel hover text.guest-refreshis deleted.guest-return's plan now notes the narrowed guest grant.Public API / wire: no published API. Guest tokens' subscribe claim narrows from
**to.renew/<identity>/<generation>, and links carrygeneration=. Links from before this PR don't parse on the new join page, and die with the rotated room key. The join page needs a relay acceptingmoq-lite-07-wip.Decisions
Choices made without asking
.renew/: outside each guest's own<identity>/**publish space, so a guest can't spoof them, and hidden from moq-room listings.room.guests.tomlbeside the room key: the state bounds tokens the key signed, so it lives with the key, not the scene file. It holds only names and generations. The key's directory must be writable.moq-lite-07-wip: falling back to older versions would join but get disconnected at expiry. Refusing up front matches "supported or refused".--guest-expiryof one minute: it leaves a renewal at least 30s to reach the guest and the relay.Order
#27 (moq bump to fd07f5b) has landed and is merged in, with
bun.lockrefreshed. Nothing needed porting: the studio's guest code uses none of the APIs it changed.Validation
a_guest_joins_and_renews_through_a_relay_that_verifies_tokens), against a relay that verifies tokens:**makes this test fail (checked earlier).just ciat d30e16c: passed (148 Rust tests, 5 JS tests). The relay renewal test passes against moq fd07f5b.Review
Codex (
quest review, session started after the design change), gpt-6.1-sol, medium effort. Headd30e16c959c3a13df6a46cdc06cbd70c322093f5, round 4: no findings. Rounds 1-3 (03800b7,e2b4183,a8f555d) also found none. Round 4 is the design checkpoint: the approach holds, and the PR is ready to merge as is.Earlier session (rounds 1-4, ending clean at
253754d):4883616.253754d.Online review of
aeb8438:Online review of
03800b7:7c674f3. Renewals move to a new generation when a removed guest is let back in.7c674f3and tested.Online review of
a8f555d:d30e16c. It now lives beside the room key, and the restart test keeps the key in its own directory..renew/<identity>docs): fixed ind30e16c, along with theRENEWdoc andquest/m0/guest-return.md.(Written by Claude Opus 5.5)