Skip to content

feat(studio): renew a joined guest's token while they stay in the room - #28

Merged
kixelated merged 18 commits into
mainfrom
quest/m0/guest-refresh
Nov 4, 2026
Merged

kixelated merged 18 commits into
mainfrom
quest/m0/guest-refresh

Conversation

@kixelated

@kixelated kixelated commented Nov 4, 2026 •

Copy link
Copy Markdown
Contributor

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/join on the moq submodule'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/room and @moq/signals are now workspace:* links into moq/js, like the Rust path dependencies. The root workspace lists every moq package they reach.
  • bunfig.toml installs hoisted, as moq does, so the submodule's worklet Vite plugin finds rolldown.
  • postinstall builds @moq/libav (nix build of moq's flake), as moq's root does. It isn't on npm.
  • vite.config.ts uses moq's worklet plugin, and tsconfig.json includes its ?worklet declarations and vite/client.
  • Root check/test scripts filter to ./js/*, so they skip moq's own packages.
  • join.yml checks out the submodule and redeploys when the gitlink moves. just upgrade-moq now refreshes bun.lock too. The README's Development section notes the submodule and Nix are needed.

Renewal (ee34030)

  • Narrower guest grant: a guest's token still publishes <identity>/**, but it can now subscribe only to .renew/<identity>/<generation>, where its renewals are published. It was moq-room's **.
  • Studio session: a second session dials the room URL with a token the room key signs (publish .renew/**, nothing else).
    • It carries none of the --connect credentials: no URL, --connect-token or mTLS identity.
    • It offers only moq-lite-07-wip and renews its own token in band.
    • A fixed --connect-bind port is refused with guests, since the two sessions would share it.
  • Schedule: each token lasts one --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.
  • Remove from scenes stops a guest's renewals, so they leave within one expiry.
    • Removals are kept in room.guests.toml beside 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.
    • New link resumes renewal under the next generation, which is kept in the same file and carried in the link (generation=). A token from before the removal cannot read the new renewals. Undo of Remove from scenes does not resume renewal.
  • Guests panel: a renewed guest's Link column reads "renewing" instead of their link's expiry. Show link appears only while the link works. The panel also shows when the renewing session can't connect.
  • Expiry: each token's expiry rounds up to the whole second, and --guest-expiry must be at least a minute. A renewal every half expiry then always lands before the token it replaces expires.
  • Join page: offers only 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.
  • Docs and quests: README says renewal needs a relay built from moq main accepting --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-refresh is 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 carry generation=. 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 accepting moq-lite-07-wip.

Decisions

  • Links issued before this PR, whose claims subscribe to the whole room, could read other guests' renewals (OpenAI P1):
    • Rotate the room key on upgrade, documented in the README's Guests section ✅ (maintainer)
    • Encrypt renewals with moq-e2ee to a per-guest key in the link (built in c9e7e92, then removed)

Choices made without asking

  • Renewals under .renew/: outside each guest's own <identity>/** publish space, so a guest can't spoof them, and hidden from moq-room listings.
  • Removals file room.guests.toml beside 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.
  • The join page offers only moq-lite-07-wip: falling back to older versions would join but get disconnected at expiry. Refusing up front matches "supported or refused".
  • Renewal follows the camera broadcast: the page announces it for as long as it's joined. A guest offline at a tick is renewed when they return, if their last token still works.
  • Undoing Remove from scenes doesn't resume renewal: renewal isn't scene state, and the hover text and README say so. New link resumes it.
  • Minimum --guest-expiry of 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.lock refreshed. Nothing needed porting: the studio's guest code uses none of the APIs it changed.

Validation

  • Integration test (a_guest_joins_and_renews_through_a_relay_that_verifies_tokens), against a relay that verifies tokens:
    • alice reads her renewal, and the relay accepts it in band;
    • alice can't read bob's renewals;
    • widening alice's grant back to ** makes this test fail (checked earlier).
  • Unit tests:
    • fractional-second phases, where a token always outlasts the next renewal;
    • remove, then restart: the guest stays unrenewed until a new link, which moves them to the next generation, and a restart keeps it;
    • the earlier generation's renewals stop once a removed guest is let back in;
    • a removal the file can't hold isn't made.
  • Unit test (manual clock, so wall time moves): renewal starts on join; Remove ends it, New link resumes it, and leaving ends it. Neither invited nor screen-only counts.
  • JS tests:
    • the page receives each token at its own name and generation, never another's;
    • a group superseded before it completes doesn't end the renewals (it failed before the fix);
    • links with a malformed generation are refused.
  • just ci at 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. Head d30e16c959c3a13df6a46cdc06cbd70c322093f5, 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):

  • Round 1: Leave then Join re-sent the original token. Fixed in 4883616.
  • Round 3: a failed removals write still marked the guest removed. Fixed in 253754d.

Online review of aeb8438:

  • OpenAI P1 (old links read renewals): answered by the maintainer's pick, rotate the room key (README).
  • OpenAI P2 (removals lost on restart): fixed with a removals file. Later moved beside the room key; see below.
  • OpenAI P2 (whole-second expiry): fixed with rounded-up expiry and a one-minute minimum.
  • Pullfrog README.md:188, ui.rs:1352, and undo: fixed.

Online review of 03800b7:

  • OpenAI P1 (admitted old sessions outlive the key rotation): no studio has shipped and the join page isn't live, so no such sessions exist. The README's upgrade steps now end them anyway (restart the relay, or wait out their expiry) before the upgraded studio starts.
  • OpenAI P2 (New link reuses the renewal path): fixed in 7c674f3. Renewals move to a new generation when a removed guest is let back in.
  • OpenAI P2 (a superseded group ends the reader): fixed in 7c674f3 and tested.

Online review of a8f555d:

  • OpenAI P2 (removal state keyed to the scene file): fixed in d30e16c. It now lives beside the room key, and the restart test keeps the key in its own directory.
  • Pullfrog guests.rs:15 (stale .renew/<identity> docs): fixed in d30e16c, along with the RENEW doc and quest/m0/guest-return.md.

(Written by Claude Opus 5.5)

kixelated and others added 2 commits November 3, 2026 18:13
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>
@kixelated kixelated changed the title feat(studio): renew a joined guest's token while they stay in the room quest: guest token refresh waits on in-band renewal being released Nov 4, 2026
kixelated and others added 2 commits November 3, 2026 18:27
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>
@kixelated kixelated changed the title quest: guest token refresh waits on in-band renewal being released feat(studio): renew a joined guest's token while they stay in the room Nov 4, 2026
@kixelated
kixelated marked this pull request as ready for review November 4, 2026 02:58
@coderabbitai

coderabbitai Bot commented Nov 4, 2026 •

Copy link
Copy Markdown

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 5 minutes.

Check out review usage here.

View limit details

Limit details: You’ve used the included review 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: dea6bbe4-7098-4ba6-9fdd-527e6d7b12b5
Commits

Reviewing files that changed from the base of the PR and between d1dd353 and d44baf8.

Files ignored due to path filters: 2
  • Cargo.lock is excluded by !**/*.lock
  • bun.lock is excluded by !**/*.lock
Files selected for processing: 21
  • .github/workflows/join.yml
  • README.md
  • bunfig.toml
  • js/join/package.json
  • js/join/src/index.ts
  • js/join/src/link.test.ts
  • js/join/src/link.ts
  • js/join/src/renew.test.ts
  • js/join/src/renew.ts
  • js/join/tsconfig.json
  • js/join/vite.config.ts
  • justfile
  • package.json
  • quest/m0/README.md
  • quest/m0/guest-refresh.md
  • quest/m0/guest-return.md
  • rs/moq-studio/Cargo.toml
  • rs/moq-studio/src/guests.rs
  • rs/moq-studio/src/main.rs
  • rs/moq-studio/src/ui.rs
  • sh/upgrade-moq.sh
  • 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

@kixelated kixelated left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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). removed lives 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.

@pullfrog pullfrog Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 — adds Renewal/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 offers moq-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 the moq submodule instead of npm.
  • rs/moq-studio/src/ui.rs and main.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.

Pullfrog  | Fix all ➔ | Fix 👍s ➔ | View workflow run | Using big-pickle (free) | 𝕏

Comment thread README.md Resolved
Comment thread rs/moq-studio/src/ui.rs Resolved
kixelated and others added 4 commits November 3, 2026 19:39
…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>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@kixelated

Copy link
Copy Markdown
Contributor Author

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:

  • P1 (old broad grants read renewals): fixed in c9e7e92.
    • Each guest's renewals are encrypted to a key derived from the room key (HMAC-SHA256 over the identity). Only that guest's links carry it, as key= in the fragment.
    • They're published under a name only that key derives.
    • a_guest_joins_and_renews_through_a_relay_that_verifies_tokens signs a token with the old claims, reaches alice's renewal broadcast through the relay, and fails to read it without her key.
    • A shared Rust/JS vector pins the two sides to the same path.
  • P2 (removals lost on restart): fixed in c9e7e92 and 253754d.
    • Removals are kept in show.guests.toml beside the scene file until a New link.
    • A failed write leaves the guest as they were and reports the error.
    • Tested in a_removal_outlasts_a_restart.
  • P2 (whole-second flooring): fixed in c9e7e92.
    • Expiry now rounds up, so every token lasts at least --guest-expiry.
    • --guest-expiry must be at least a minute, so the half-expiry renewal has 30s or more to land.
    • a_guest_in_the_room_is_renewed_until_removed starts 750 ms into a second and checks each token outlasts the next renewal.
  • Pullfrog, undo of Remove from scenes: undo restores the scenes but not renewal, by design: renewal isn't scene state. The hover text and README now say so, and New link resumes it.

just ci passed at 253754d, and the local Codex review round 4 found nothing.

(Written by Claude Opus 5.5)

@kixelated kixelated left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 throw Old when 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))

kixelated and others added 2 commits November 3, 2026 19:58
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>

@pullfrog pullfrog Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ℹ️ 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.toml and reloads them across restarts, and makes stop_renewing take effect only once the temp-file write is persisted.
  • Whole-second renewal timing — floors issued, rounds expires up, and rejects --guest-expiry under 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/join renewal-path vectors name the same broadcast.

Pullfrog  | Fix it ➔ | View workflow run | Using big-pickle (free) | 𝕏

@kixelated

Copy link
Copy Markdown
Contributor Author

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 moq-e2ee, no @moq/e2ee, and no key= in the link.

  • Renewals are published in the clear at .renew/<name>. A guest's new token may subscribe only to their own.
  • A link from before this version carries a subscribe grant over the whole room (**), so it could read every guest's renewals.
  • The README's Guests section now tells operators upgrading to generate a new room key and drop the old public key from --key-dir, so those links die with it.
  • It also says anything else the relay lets subscribe in the room can read renewals, so grant that only to the studio.

The other fixes stay:

  • removals kept in show.guests.toml;
  • the one-minute --guest-expiry minimum, with each token's expiry rounded up;
  • the Guests panel and README fixes from Pullfrog.

just ci passed at 03800b7, after merging main's #8. A fresh quest review --new round on that head found nothing.

(Written by Claude Opus 5.5)

@kixelated kixelated left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 Old and 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))

@pullfrog pullfrog Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ℹ️ 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/rind and the identity-derived key, publishing each guest's JWT as a plaintext token frame 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-e2ee from the workspace, bun.lock, and Cargo.lock, and rewrites the join page test for the unencrypted path, dropping the cross-language path vector.

Pullfrog  | Fix it ➔ | View workflow run | Using big-pickle (free) | 𝕏

kixelated and others added 2 commits November 3, 2026 22:25
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>
@kixelated

Copy link
Copy Markdown
Contributor Author

Responses to the OpenAI review of 03800b7 (5763075754):

  • P1, admitted legacy sessions outlive the key rotation: no studio has shipped and the join page isn't live, so no such sessions exist today. The finding still holds for anyone upgrading, so the README's upgrade steps now say to end them before starting the upgraded studio. After rotating the key: restart the relay, or wait out the old links' --guest-expiry. A relay keeps a session it already admitted until that token expires. I didn't add a relay test: whether the auth server revalidates is moq's behavior, not the studio's.
  • P2, New link reuses .renew/<identity>: fixed in 7c674f3.
    • Renewals now live at .renew/<name>/<generation>.
    • Letting a removed guest back in with New link bumps the generation. It's kept in show.guests.toml, and the link carries it as generation=.
    • A token from before the removal can subscribe only to the old generation, which is no longer published.
    • Tested in a_guest_in_the_room_is_renewed_until_removed: the old path stops resolving. Also in a_removal_outlasts_a_restart: the generation survives a restart and moves only when a removed guest is let back in.
  • P2, a superseded partial group ends the only reader: fixed in 7c674f3. The join page skips a group that fails with StreamCode.Old and reads the next one; other errors still end the reader loudly. renew.test.ts adds an incomplete group followed by good ones. That test failed before the fix with the Old error and passes after.

just ci passed at e2b4183, after merging main's #23. The local Codex review round on that head found nothing.

(Written by Claude Opus 5.5)

@kixelated kixelated left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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>

@pullfrog pullfrog Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ℹ️ 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.toml now stores generation beside removed (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::change replaces set_removed, bumping the generation only when the guest is removed, signing for generation + u64::from(removed) before mutating, and restoring the whole Participant if the sidecar write fails.
  • Old-group continuation — renew.ts catches Net.Error.Stream with Net.StreamCode.Old around the group read, so a superseded partial group no longer ends the only reader; renew.test.ts covers 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.

Pullfrog  | Fix all ➔ | Fix 👍s ➔ | View workflow run | Using big-pickle (free) | 𝕏

Comment thread rs/moq-studio/src/guests.rs Resolved

@kixelated kixelated left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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>
@kixelated

Copy link
Copy Markdown
Contributor Author

Response to the OpenAI review of a8f555d (5764232822):

  • P2, removal state keyed to the scene file: fixed in d30e16c.
    • A removal bounds tokens the room key signed, so its state now lives beside the key: room.guests.toml for room.jwk. Opening another scene file for the same room sees the same removals and generations.
    • Config::load no longer takes the scene path at all.
    • a_removal_outlasts_a_restart keeps the key in its own directory and checks that the state lands beside it and survives a restart.
    • The README notes the key's directory must be writable.
  • Pullfrog guests.rs:15 (stale .renew/<identity>): fixed in the same commit, with the RENEW doc and quest/m0/guest-return.md.

just ci passed at d30e16c, and the local Codex review round 4 on that head found nothing.

(Written by Claude Opus 5.5)

@pullfrog pullfrog Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ 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.toml for room.jwk instead 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.rs and the restart test were updated (the test now uses keys/room.jwk and keys/room.guests.toml).
  • Stale renewal-path docs corrected — the module doc, RENEW doc, and quest/m0/guest-return.md now 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.

Pullfrog  | View workflow run | Using big-pickle (free) | 𝕏

@kixelated kixelated left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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))

@kixelated
kixelated merged commit 18f2520 into main Nov 4, 2026
4 checks passed
@kixelated
kixelated deleted the quest/m0/guest-refresh branch November 4, 2026 07:20
kixelated added a commit that referenced this pull request Nov 4, 2026
Brings in #28 (guest token renewal) beside the recordings directory.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
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