Fix: route /mr-<iid>/ and /v<X.Y.Z>/ to their own builds via serve.json (!12) · Merge requests · Daanvr / upload-workbench

Summary

Per-MR preview deploys (/mr-<iid>/) and per-version archives (/v<X.Y.Z>/) on upload-workbench.toolforge.org were silently serving the root build's HTML, with the wrong asset hashes.
Fix: ship public/serve.json that Vite copies into dist/, where Toolforge's vercel/serve container reads it. Two redirects (which run before rewrites in serve-handler) catch /mr-<digits> and /v<X.Y.Z> and 301 them to their explicit /<subdir>/index.html before the --single SPA fallback can hijack them.
Unblocks the three sibling tasks parked in Needs input waiting on a working preview URL.

Phabricator

T425851 — Merge Request live previeuw bug (Unbreak Now!)
Unblocks (in Needs input, waiting on this fix to verify their own previews):
- T425827 — Depicts pill popover
- T425828 — Frozen leftmost columns
- T425829 — Category pill popover

Root cause

Toolforge's static-files serving for <tool>.toolforge.org is vercel/serve -s (the SPA-fallback flag) — confirmed by the response shape (Files within static/v0.4.0/ listing HTML matches serve-handler's built-in template byte-for-byte; the Vary / Content-Disposition headers match too).

The -s flag prepends { source: "**", destination: "/index.html" } to the serve.json rewrites. Combined with the lookup order in serve-handler (early-stat-if-extname → rewrites → findRelated):

URL	What happens
`/mr-9/`	extname `""` → skip early stat → rewrite `` matches → resolve `/index.html` (the root** one) → serve root build HTML, referencing `/assets/...` (root paths). Looks like the live app, but it isn't the MR's build.
`/v0.4.0/`	extname `.0` (last dot wins) → early stat fires → directory listing. Listing has a clickable `index.html`, but cleanUrls strips `.html`, then trailing-slash normalisation puts you back at the listing. Loop.
`/mr-9/index.html`	cleanUrls 301 → `/mr-9/index` → 301 → `/mr-9` → root SPA. Same dead end.

The --exclude='/v*/' --exclude='/mr-*/' on the root deploy's rsync is correct — it preserves the per-MR / per-version directories under --delete. The build files land where they should. Only the routing is broken.

Fix

public/serve.json:

{
  "cleanUrls": false,
  "redirects": [
    { "source": "/mr-:iid(\\d+)", "destination": "/mr-:iid/index.html", "type": 301 },
    { "source": "/v:version(\\d+\\.\\d+\\.\\d+)", "destination": "/v:version/index.html", "type": 301 }
  ],
  "rewrites": [
    { "source": "**", "destination": "/index.html" }
  ]
}

serve-handler runs shouldRedirect before applyRewrites, so the redirect fires first. The redirect target is a real file (<subdir>/index.html exists on disk after rsync), and cleanUrls: false keeps serve from stripping the .html back into the loop. After the second hop the per-subdir HTML is served with the right asset paths.

The trailing ** → /index.html rewrite preserves the root SPA fallback for unknown paths (so /foo still loads the live app); it's harmlessly redundant with the container's --single flag.

Pattern constraints ((\d+) for the IID, (\d+\.\d+\.\d+) for SemVer) keep the redirects narrow — /version-info or /mr-feedback still fall through to the root SPA.

Verification

SSH to Toolforge wasn't available in the subagent's environment (the local SSH agents on the Claude Code host were stale / had no identities, and the user's encrypted key needs a passphrase that can't be entered non-interactively). I also can't fetch this MR's own preview URL until after the fix lands — that's the chicken-and-egg the parent task flagged.

What I did instead:

Read serve-handler's source (src/index.js and @zeit/schemas/deployment/config-static.js) to confirm:
- redirects are processed before rewrites.
- --single always prepends its own ** → /index.html rewrite (no override available from a config file).
- cleanUrls toggles whether .html redirects fire at all.
- The schema rejects unknown top-level keys (so no comment field; rationale is in the commit body and CHANGELOG).
Reproduced the production layout locally — built dist/ with VITE_BASE_PATH=/, VITE_BASE_PATH=/v0.4.0/, and VITE_BASE_PATH=/mr-9/, dropped them into a fixture mirroring www/static/.

Ran serve-handler directly against the fixture, both with the simulated --single rewrite prepended and without it. All cases pass:

Request	Status	Body
`/`	200	root index.html, `/assets/...`
`/mr-9/` and `/mr-9`	301 → `/mr-9/index.html` → 200	mr-9 build HTML, `/mr-9/assets/...`
`/v0.4.0/` and `/v0.4.0`	301 → `/v0.4.0/index.html` → 200	v0.4.0 build HTML, `/v0.4.0/assets/...`
`/mr-9/assets/index-…js`	200	real bundle bytes
`/foo` (unknown SPA path)	200	root index.html (SPA fallback preserved)
`/version-info`, `/mr-feedback`	200 (no 301)	root index.html (constraints prevent false matches)

Validated dist/serve.json against the real @zeit/schemas/deployment/config-static.js schema with AJV. Passes.

Once this MR's pipeline runs, the maintainer can confirm by:

curl -sIL https://upload-workbench.toolforge.org/mr-<this-iid>/
# expect:  301 -> /mr-<iid>/index.html  -> 200, content-length ~520, etag != root etag
curl -s https://upload-workbench.toolforge.org/mr-<this-iid>/ | grep src=
# expect:  src="/mr-<iid>/assets/index-<hash>.js"  (not the root hash)

If the --single flag is somehow not in use on the deployed container, cleanUrls: false in our serve.json could surface previously-redirected /index URLs as 404s — there are no such URLs in this app's source (grep'd src/).

Test plan

CI's deploy:mr job for this MR finishes green.
curl -sIL https://upload-workbench.toolforge.org/mr-<IID>/ follows a single 301 to /mr-<IID>/index.html and returns 200 with a content-length that doesn't match the root index.html's 510 bytes.
curl -s https://upload-workbench.toolforge.org/mr-<IID>/ | grep src= shows the asset path under /mr-<IID>/assets/....
Live root / is unaffected — same etag, same SPA behaviour for unknown paths.
After merge: /v<new-version>/ resolves the same way as /mr-<IID>/.

🤖 Generated with Claude Code

Admin message