Add support for redirection in host routing rules - #5652
Conversation
A hostRoutingRules entry can now declare { path, redirectTo, statusCode? }
alongside the existing { path, instance } form. A matched redirect rule
answers 302 by default (301/308 opt-in); a realm-relative target resolves
against the realm's mount pathname while external http(s) URLs pass
through verbatim (other schemes are dropped). The request query string
carries over when the target has none, and the declared redirect fires
from either trailing-slash form in a single hop.
The SPA mirrors the server: an in-app transition to a redirect-ruled
path navigates to the target instead of rendering a card. The realm
config editor gains a card/redirect toggle with advisory validation of
the target.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Preview deploymentsHost Test Results 1 files ± 0 1 suites ±0 2h 57m 32s ⏱️ + 4m 1s Results for commit 2d4d864. ± Comparison against earlier commit b898a63. Realm Server Test Results 1 files ± 0 1 suites ±0 14m 15s ⏱️ -1s Results for commit 2d4d864. ± Comparison against earlier commit b898a63. |
Side by side, the URL input's and the select's combined minimum width overflows the rule container at operator-mode stack width — the select escaped past the container edge. Stacking them lets the 1fr track shrink the input instead. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Routed paths only ever serve GET/HEAD page requests — card mutations go through the JSON API on different URLs — so the method-preserving 308 behaves identically to 301 here and just clutters the choice. Redirect rules now offer 301 or 302 (the default). The trailing-slash canonicalization redirect keeps its 308; it is not author-configurable. Co-Authored-By: Claude Fable 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: d3d532cec9
ℹ️ 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 server copies the request query onto a redirect rule's target when the target has none of its own; the SPA branch dropped it. The query is sourced from the transition target (transition.to.queryParams), not window.location.search — with HistoryLocation the location still shows the URL being navigated away from while the transition is in flight. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
FadhlanR
left a comment
FadhlanR
left a comment
There was a problem hiding this comment.
[Claude Code 🤖] LGTM — clean layering across the shared vocabulary, map builder, server redirect, and SPA path, and the comments explaining the non-obvious bits (redirect-wins-over-canonicalization, protocol-relative steering, the transition-vs-location.search choice) are excellent.
One gap worth addressing, ideally before merge:
No test coverage for the SPA-side redirect path. The server HTTP redirect, the validators, and the editor UI are all well covered, but the in-app redirect — routes/index.gts (resolveRoutedPath → the 'redirectTo' in routed branch) → HostModeService#redirectTo — has no test exercising it. Nothing in host-mode-test.gts / host-submode-test.gts navigates in-app to a redirect-ruled path.
This is the subtlest new code in the PR: redirectTo deliberately takes transition.to.queryParams rather than window.location.search to dodge a HistoryLocation staleness bug (per the comment on the method). That kind of subtlety regresses silently. An acceptance test that transitions in-app to a redirect-ruled path and asserts the resulting location would lock it in.
While adding it, worth confirming server↔SPA parity on query carry-over: the server copies the raw requestURL.search verbatim, whereas the SPA re-serializes transition.to.queryParams via String(value) with a value != null guard — different data shapes over different code paths, so it's worth verifying they produce the same Location for the same inbound request.
burieberry
left a comment
burieberry
left a comment
There was a problem hiding this comment.
Reviewed the redirect-rule support end to end (server handler, SPA path, realm map building, and the config editor). Seven findings below as inline comments. One more lands in a file this PR doesn't touch, so noting it here:
packages/host/app/components/operator-mode/publish-realm-modal.gts (~L562) — pre-publish dangling-rule check not updated for redirect rules. Two gaps: (1) a rule carrying both redirectTo and a dangling instance link — the exact state realm.ts documents as "redirect wins" — still has its instance resolved by checkDanglingRoutingRulesTask, 404s, and triggers a spurious "degrades to 404" warning even though the published path redirects fine; (2) a pure redirect rule is skipped entirely (if (!reference) continue;), so a realm-relative target pointing at nothing publishes silently and the path 302s into a 404 — the failure class this check exists to prevent.
Two other candidates were investigated and ruled out, for the record: afterModel does not wipe state after the SPA redirect (restore() is a no-op once initialized), and a cached ETag 304 cannot answer before the redirect branch (the ETag key bumps on last_published_at, which any rule change necessarily changes via republish).
🤖 Reviewed by Claude (Fable 5)
Adds acceptance coverage for the in-app leg of a redirect routing rule, which had none: serve rule still renders, realm-relative and external targets, query carry-over, and a target whose own query wins. Two parity gaps with serve-index surfaced while writing them. The router hydrates every declared query param onto transition.to.queryParams using its controller default whether or not it was in the URL, so every redirect gained a debug=false the server never sends — and internal state, including the sid/clientSecret password-reset tokens, would have been handed to external targets. Forwarding now drops the app's own params, whose list moves to an exported constant. Repeated params also serialized as tag=a,b instead of the server's tag=a&tag=b. Realm-relative targets are joined onto the host-mode origin by concatenation, matching how the index route builds a routed card's URL; new URL(target, base) discarded any path the origin carries under ?hostModeOrigin= simulation. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
A rule pointing at its own path, or a ring of rules pointing at each other, answered a redirect to a URL matching the same rule again — the client bounced until it gave up, and a permanent 301 kept doing so from cache after the config was fixed. Nothing caught it: the target validator is shape-only and never sees the rule's own path. findRedirectCycles walks the relative-target graph and reports every path in a ring. The realm drops those rules when it reads the config, so the path resolves like any unrouted one, and the editor shows a banner alongside the existing duplicate-path and dangling-target warnings. Paths that only lead into a ring keep working once the ring is gone, and an external target ends a chain. A target spelled as a full URL back to the same site stays undetectable — the published host isn't known where the map is built. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
hostModeOrigin returns the ?hostModeOrigin= query param verbatim whenever one is present, so using it as the base for a realm-relative redirect target made location.replace() a navigation sink on visitor input: the param is a declared query param and sticks across in-app transitions, sending every redirect rule to an origin the visitor picked. An empty ?hostModeOrigin= yielded '', which is not nullish, so the fallback beside it never ran and URL construction threw inside the model hook instead. Realm-relative targets now resolve against window.location.origin, the way a browser resolves a Location: /path header. The two values are identical in host mode — only the simulation affordance separates them, and a simulated session should stay on the page it runs from. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The SPA already dropped these; the server still forwarded the request's search verbatim, so the two answered different URLs for the same inbound request. A redirect target may be an external site, and sid/clientSecret are password-reset tokens — neither side should pass them on. The rest are meaningless off the page that owns them. The list moves to runtime-common so the index controller and serve-index read the same one, with foreignQueryParams as the shared filter. It round-trips through URLSearchParams, so repeated keys stay repeated and a query made up entirely of app params leaves no empty ? behind. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The tracked override was set on every toggle and never cleared, so once clicked it shadowed the model for the life of the component. With the config card open in two places, clearing a redirect in one left the other showing an empty redirect editor, where the next keystroke wrote a redirectTo that beats the instance the first had chosen. Its justification was wrong: setField writes to the data bucket and calls notifyCardTracking inline, so a field write is readable and rendered without a mirror in component state. StringField has no empty value, so an unset redirectTo reads as undefined and the seeded empty string still reads as a redirect. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
RadioInput takes the group name as @name and falls back to naming the group after itself; a bare name= attribute lands on the fieldset through ...attributes, where it does nothing for grouping. So the counter and the field feeding it never reached an input. The guid fallback already gives each rule its own group, which is the behavior in place today. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
A rule carrying both an instance and a malformed redirectTo was dropped whole, so a path that had been serving a card started resolving as if unrouted — the dead end the branch's own comment disclaims, and a regression against the previous behavior of ignoring an unrecognized redirectTo key. Only a hand-edited realm.json can produce the pairing, since the editor clears one target when the other is chosen. An unusable target now falls through to the instance instead of taking the rule down with it. With no usable instance either, the rule still drops. The log line says ignoring rather than dropping to match. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The server worked the target out twice — once for the Location header, once for the routing map it injects for the SPA — from two copies of the same expression. Editing one would have sent a full-page visitor and an in-app one to different places. resolveRedirectTarget now backs both, with isExternalRedirectTarget shared by the SPA's own resolution. The SPA also re-declared the rule union locally, widening statusCode to number, and discriminated on a raw 'redirectTo' in check. Both now use HostRoutingRule and isRedirectRoutingRule, so the shape and its discriminant are declared once. No behavior change: the helper reproduces both former expressions for external, relative, multi-slash, query-bearing and root-mounted targets. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
thanks, 13ac5dd has this |
3 participants
This adds redirection to the routing map:
In raw JSON:
Here’s the 301:
and 302: