Move moq.pub and moq.watch into the repo, keyed by path - #119
Merged
Conversation
The two player sites lived in a separate repo and deployed by hand. Bring them in as sites/pub and sites/watch, each still its own Worker but built and uploaded by the same `just deploy`, and reading the repo-root .env files so PUBLIC_RELAY_URL is the single place the relay is configured. The old copies hardcoded cdn.moq.dev, stale since the move to cdn.moq.pro. Broadcasts are now named by the path rather than the query string: /<project>/<name>, e.g. moq.pub/anon/lazy-otter-4f21.hang. The same path on moq.watch plays it back, so a publisher can share its own URL with one substitution, and the publisher page links to it. A bare moq.pub/ redirects to an invented name server-side, so the shareable URL exists before any JS runs. Old ?project=&name= links redirect into the path. Everything that isn't part of a broadcast's identity - relay, jwt, source - stays in the query string. sites/lib holds the scheme once, shared by both sites and by both the Worker and the Vite dev server so the two can't drift. Two things this had to work around: - assets.run_worker_first is load-bearing. `/` matches index.html, so without it Cloudflare's asset server answers first and the Worker never runs, meaning no redirect and no invented name. - The Worker can't tell an asset from a page by looking for a dot, since names end in .hang. It asks the asset store and falls back to the page on a 404. Declaring vite as a direct devDependency (it was only reachable through astro) re-hoists esbuild in the lockfile: vite wants ^0.25.0 and astro wants ^0.27.3, so they were always on separate copies and each still resolves to the same version as before. Only the tree position swapped. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e19e61ffba
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
The share link on moq.pub was built from the broadcast path alone, so a broadcast published to a non-default relay produced a link that opened moq.watch against the wrong relay and found nothing. `relay` now carries over. `jwt` deliberately does not: moq tokens are prefix-scoped with separate publish (`put`) and subscribe (`get`) grants, so a token being used to publish carries `put`, and copying it into a link meant to be passed around would hand every recipient the right to publish rather than just to watch. Reviewers suggested carrying the whole query; that trades a broken link for a credential leak. Instead, a publisher who wants to share access to a private broadcast passes a subscribe-only token as `?viewer=`, which becomes the `?jwt=` on the moq.watch end. With a `jwt` and no `viewer` the page says viewers need their own token, so the remaining gap is visible rather than looking like a working link that silently won't connect. Also deploy the player sites before moq.dev. Snapshot, moq.dev upload and announce are effectively a transaction: a failure in the middle leaves posts live but unannounced, and the snapshot expires after an hour, so a later retry reads those posts as already-published and never mails them. The two new deploys sat inside that window; now they run ahead of it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Contributor
|
Warning Review limit reached
Next review available in: 43 minutes You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (6)
WalkthroughAdded shared broadcast URL and relay utilities with route redirection and generated names. Added 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches✨ Simplify code
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@CLAUDE.md`:
- Around line 72-79: Update the development-command description in CLAUDE.md to
identify the site-specific Vite commands—bun run dev:pub, bun run dev:watch,
just dev-pub, and just dev-watch—as the commands that use sites/lib/dev.ts, and
clarify that the root bun run dev starts Astro rather than moq.pub or moq.watch.
In `@justfile`:
- Around line 49-54: The _announce flow must skip subscriber notifications for
staging: gate the announcement credential/1Password setup behind env == live,
then exit or return before invoking notify-subscribers.ts send when env is
staging. Preserve the existing live announcement behavior and update the
_announce recipe rather than deploy.
In `@sites/lib/broadcast.ts`:
- Around line 20-23: Update parse() in broadcast.ts so malformed percent escapes
do not throw from decodeURIComponent while splitting pathname. Catch URIError
inside the path-parsing flow and return undefined for invalid broadcast paths,
preserving the existing successful parse behavior for valid inputs and letting
redirect() fall through on bad shared routes.
In `@sites/pub/src/main.ts`:
- Around line 28-36: Update the share-link construction in the main page flow to
preserve the selected relay as a query parameter in the watch URL, so
non-default relays remain usable by recipients. Keep the relay parameter
alongside the existing URL parameters and do not automatically include the JWT;
leave token distribution as an explicit policy decision.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: b55a4a44-2c9f-45e8-a0bc-6ed860f8a871
⛔ Files ignored due to path filters (9)
bun.lockis excluded by!**/*.locksites/pub/public/favicon.pngis excluded by!**/*.pngsites/pub/public/favicon.svgis excluded by!**/*.svgsites/pub/public/logo.svgis excluded by!**/*.svgsites/pub/public/wordmark.svgis excluded by!**/*.svgsites/watch/public/favicon.pngis excluded by!**/*.pngsites/watch/public/favicon.svgis excluded by!**/*.svgsites/watch/public/logo.svgis excluded by!**/*.svgsites/watch/public/wordmark.svgis excluded by!**/*.svg
📒 Files selected for processing (23)
.env.development.env.live.env.stagingCLAUDE.mdREADME.mdjustfilepackage.jsonsites/env.d.tssites/lib/broadcast.tssites/lib/dev.tssites/lib/name.tssites/lib/route.tssites/lib/worker.tssites/pub/index.htmlsites/pub/src/main.tssites/pub/src/worker.tssites/pub/vite.config.tssites/pub/wrangler.jsoncsites/watch/index.htmlsites/watch/src/main.tssites/watch/src/worker.tssites/watch/vite.config.tssites/watch/wrangler.jsonc
`new URL` keeps a bad percent escape in the pathname, so a request to moq.pub/anon/% reached decodeURIComponent in parse() and threw a URIError straight out of the fetch handler. Verified against wrangler dev: 500 before, 200 after. It's now treated as "not a broadcast", which lands on the invented-name redirect or the usage hint. relay() had the same shape one function down: `?relay=` is whatever was in the address bar and `new URL` throws on garbage, blanking the page. It now falls back to the site's own relay. It also only accepts http and https -- `javascript:` and `data:` parse fine as URLs, mean nothing to WebTransport, and are the shape that becomes an XSS sink as soon as something renders the value. Also correct CLAUDE.md: the dev plugin serves `just dev-pub` and `just dev-watch`, not `bun run dev`, which is the Astro site. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The link was never asked for; I added it as a "nice touch" and it earned its keep in review findings rather than use. To work it had to carry ?relay=, and the obvious next step of carrying the whole query would have copied ?jwt= too, leaking a publish token to everyone the link reached. The ?viewer= parameter existed only to escape that bind: a second, subscribe-only token supplied purely so the link could be built. None of that machinery is needed. The path symmetry is the actual feature -- the same /<project>/<name> works on both sites -- and swapping the hostname is something a person can do unaided. Removes the link, the note, ?viewer=, Broadcast.watch(), PUBLIC_WATCH_URL from all three .env files, and the layout reserve they needed. moq.watch's usage hint still points at moq.pub; that's the empty state of a page with nothing else to say, not a link hung over a working player. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The two player sites lived in a separate repo (
../moq.watch, which held both) and deployed by hand. This brings them in and makes them part ofjust deploy, and changes broadcasts to be named by the path instead of the query string.There is no
../moq.pub— that repo held bothpub/andwatch/. Its content is copied here; the repo itself is untouched on disk and can be archived.URL scheme
/<project>/<name>— project is the relay tenant, name is everything after it (slashes allowed).moq.pub/→ 302 →/anon/lazy-otter-4f21.hang. Server-side, so the shareable URL exists before any JS runs, rather than being patched in byhistory.replaceStateafter load.moq.watch/anon/lazy-otter-4f21.hangplays it back. The publisher page shows that link at the top, so sharing is one substitution.moq.watch/still shows a usage hint.relay,jwt,sourcestay query params — they aren't part of a broadcast's identity.?project=&name=links 302 into the path, preserving the rest of the query.Invented names are
adjective-animal-hex.hang. I kept a random suffix because the original code deliberately used a UUID to avoid collisions in the openanonproject, and two dictionary words alone is only ~500k combinations.Layout
sites/libholds the scheme once, used by both sites and by both the Worker and the Vite dev server, so production andjust dev-pubcan't drift on what a URL means.Two things worth a reviewer's attention
assets.run_worker_first: ["/"]in eachwrangler.jsoncis load-bearing. My first version returned 200 formoq.pub/with no redirect —/matchesindex.html, so Cloudflare's asset server answers before the Worker ever runs. Easy to delete as noise later; it isn't.The Worker can't use a dot to tell an asset from a page, since names end in
.hang. The inheritedpathname.includes(".")heuristic would have served/anon/x.hangas a static file. It now asks the asset store and falls back to the page on a 404.Deploy
just build/just deploydo all three sites;just dev-pub(:5174) andjust dev-watch(:5173) added. The player sites now read the repo-root.env.<mode>files, soPUBLIC_RELAY_URLis the single place the relay is set — the old copies hardcodedcdn.moq.dev, stale since the switch tocdn.moq.pro. AddedPUBLIC_WATCH_URLfor the cross-site link.Staging is
new.moq.pub/new.moq.watch, matchingnew.moq.dev. Wrangler will create those custom-domain DNS records on the first staging deploy — worth knowing before running it.On the lockfile diff
163 lines, all esbuild, and it is not a version change.
vitewas only reachable through astro; declaring it directly (we import from it in three files and invoke its binary) re-hoists esbuild. vite requires^0.25.0and astro requires^0.27.3, so they were always on separate copies:esbuildvite/esbuild0.25.12astro/esbuild0.27.3Each consumer resolves to the same version as before; only the tree position swapped.
Testing
just checkandtscclean. I probe-tested that biome actually coverssites/rather than trusting the pass.just build stagingandjust build livegreen, correct env baked into each bundle.--dry-run.wrangler devfor both Workers and against the Vite dev server: redirects, fresh name per request, nested names, percent-encoded names, hashed assets, favicons, legacy links, module/HMR requests. This caught a dev-only bug where/logo.svgwas mistaken for a broadcast path and redirected — the dev plugin now only claims navigations, mirroringrun_worker_first.cdn.moq.pro/anonwith the watch link; player connects and shows LIVE. No console errors.One caveat:
/demo/bbb.hangrenders a black canvas. That's the demo not currently broadcasting, not this change — the live moq.dev/watch page shows the same black canvas for the same relay and name.🤖 Generated with Claude Code
(written by Opus 5)