Add /_/* redirect route - #4523
Conversation
Resolves the signed-in user's current organization, project and environment
and redirects to the canonical page, so /deeplink/apikeys lands on
/orgs/{org}/projects/{project}/env/{env}/apikeys.
Only the environment page segments navigation already knows about are
followed (derived from ENV_PAGE_META), so an unrecognised path redirects to
the environment root rather than becoming the redirect target. Deeper
segments and the query string are preserved.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01MvNi2jRtYatPQdXFJRK9SW
|
The splat arrives percent-decoded from the router, so the preserved remainder could contain "." / ".." segments and literal "?" or "#" characters. Traversal segments let a crafted suffix climb back out of the resolved environment path (/deeplink/apikeys/../../../../../../x resolved to /orgs/x), and a decoded "?" or "#" became part of the target's query or hash rather than its path. Drop traversal segments and re-encode each remaining segment when rebuilding the path. The redirect can no longer leave the resolved environment path. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MvNi2jRtYatPQdXFJRK9SW
|
@coderabbitai review Generated by Claude Code |
The allowlist came from ENV_PAGE_META, which omits tasks, agents and settings because those URL shapes are special-cased when resolving page metadata. Deeplinks to them fell through to the environment root. Move the list to its own module and populate it from the environment layout route segments instead, so task, agent and project settings deeplinks resolve.
The allowlist is maintained by hand, so it can fall behind when a page is added. The test derives the expected set from the environment layout's route filenames and names any segment that drifts.
tasks, waitpoints and metrics only exist as the parent of param routes, so a bare /deeplink/tasks redirected to a URL matching no route and rendered a 404 — worse than the environment root it used to fall back to. Replace the segment allowlist with a map from deeplink name to target path. tasks points at the environment root, which is the task list; waitpoints points at waitpoints/tokens; metrics is dropped, being only a legacy redirect shim with no page of its own. Deeper segments are still kept as given, so task and run detail links keep working. The test now checks that every target resolves to a real route and that no environment page is missing, instead of comparing bare segment names.
Observability mapAs of 18/100 over 413 measured of 429 entry points (base 18, no change) What this PR changed
FIX FIRST
AUDIT 3 of 50 sensitive mutations record an actor. 47 without one. What the score is made ofThe score and findings here are report-only and never gate the merge. Separately, a required test suite keeps this tool's symbol and route lists in sync with the code they name, and can fail a pull request that renames or removes a symbol they reference, or that adds the first route with a segment they anticipate. Each failure names the list to edit. The rules and their reasons: internal-packages/observability-map/README.md. |
A mapped target only applied to a bare name, so /deeplink/waitpoints landed on waitpoints/tokens but /deeplink/waitpoints/waitpoint_123 became waitpoints/waitpoint_123, which matches no route. Prefixing the target unconditionally would break tasks, whose landing is the environment root but whose detail pages are under /tasks, so each entry now carries both a landing path and a prefix for deeper segments. An unrecognised name also kept no query string while a mapped one did, which became visible once tasks started resolving to the environment root: three links to the same page behaved two ways. Both now keep it. The test walks the real child routes under each prefix rather than probing one synthetic segment, so a target whose deep form stops resolving fails.
React Router decodes the splat param, so an id containing an escaped slash arrived split in two: /deeplink/tasks/standard/group%2Fmy-task became tasks/standard/group/my-task and matched no route, breaking a link copied from the dashboard. The suffix now comes from the request pathname, which keeps %2F intact, and its segments are passed through as they arrived rather than being encoded a second time. Traversal rejection now also covers the escaped spellings: a segment is dropped when it decodes to . or .. , or when it is not decodable at all. new URL already normalises %2e%2e and resolves it, which can move the pathname out of /deeplink entirely, so a suffix outside the prefix is treated as absent. A user with pending invites is also sent to the invites page first, as the dashboard index does, so an invitee following a deeplink before joining an organization is not offered organization creation instead.
React Router compiles route paths with the `i` flag unless a route opts into
`caseSensitive`, so /Deeplink/apikeys reaches this loader. The prefix strip
required the literal lowercase /deeplink/, so the suffix came back empty and the
link landed on the environment root instead of the page. The page-name lookup
had the same problem one level down: /deeplink/APIKeys fell through even though
/env/{env}/APIKeys would have matched.
Fold the case of the prefix and of the first segment only, and resolve the name
to the map's own spelling. Everything after the first segment is left exactly as
written, since task and run ids are case-sensitive.
Folding only the first segment left a prefix that spans more than one segment half-matched: `Waitpoints/Tokens/wp_123` did not equal `waitpoints/tokens`, so the graft branch fired on top of it and produced `waitpoints/tokens/Tokens/wp_123`, which matches no route. The all-lowercase spelling worked, so this was a regression the previous commit introduced. Compare as many leading segments as the prefix spans, lowercased, and return the prefix in the map's own spelling with everything past it exactly as written. Driven off `prefix` rather than special-cased for waitpoints, so a second multi-segment entry is covered when it is added.
Escape the underscore in the route filename so it stays a literal URL segment: a flat-route segment beginning with `_` is a pathless layout, so `_.$` would have mounted the route at `/*`. `createRoutePath` skips a segment only when its cooked and raw spellings both start with `_`, and the raw spelling of `[_]` does not, so `[_].$` serves `/_/*`. The prefix comparison no longer folds case. It existed because React Router matches routes case-insensitively and `/Deeplink/apikeys` really did reach the loader, but `_` has no case, so the fold is dead. Page-name folding stays, since `/_/APIKeys` still has a route to agree with. Drops `/deeplink/*` rather than keeping it as an alias; the route has not shipped, so nothing links to it yet.
The previous test built the route pattern out of DEEPLINK_PATH_PREFIX and then asserted that pattern equalled the same constant, so it could not fail if Remix compiled `[_].$.ts` to something else. That is the one thing worth guarding here: an unescaped `_` reads as a pathless layout, which would mount this loader at `/*` — a splat over the whole site whose loader redirects unconditionally. Run the real routes directory through the same `flatRoutes` the vite plugin uses, derive the mounted path from the manifest entry for the route file, and check the constant against that. Also assert no route anywhere compiles to a bare site-wide splat, which is the assertion that catches the failure independently of this route. Verified by mutation: unescaping the filename fails the suite, and a decoy pathless splat route fails the splat assertion.
Delete the explanatory blocks and every comment that restated the line below it. What survives: why the route file's underscore is escaped, and why `.`/`..` segments are dropped. Rename `isUsableSegment` to `isSafeSegment` and put the intent that was commented into the test names instead.
Requested by Chris Arderne · Slack thread
Before: there's no stable URL you can put in docs, an email or a Slack message that lands someone on their own project's API keys page — every dashboard URL contains the org, project and environment slugs, so you have to tell people to navigate there by hand.
After:
/_/apikeysresolves the signed-in user's current organization, project and environment and redirects to/orgs/{org}/projects/{project}/env/{env}/apikeys. Deeper segments are preserved (/_/runs/run_abc123) and the query string is passed through. If you're not signed in, login happens first and you land on the deeplink afterwards. The preserved suffix drops.and..segments in both their plain and escaped spellings, so a crafted URL can't escape the resolved environment path.The
/_/prefix matches how Supabase spells the same idea.How
New loader-only route
apps/webapp/app/routes/[_].$.ts, modelled on the existing redirect-only routes (orgs.$organizationSlug.projects.$projectParam.apikeys.ts,orgs.$organizationSlug.projects.v3.$.ts):requireUser(app/services/session.server.ts) — already redirects to/login?redirectTo=…, so the deeplink survives login.SelectBestEnvironmentPresenter.call({ user })resolves org + project + environment from dashboard preferences, falling back to the most recently updated project and the user's dev/prod environment.resolveDeeplinkPageagainstENV_PAGE_TARGETSinapp/utils/deeplinkPages.ts, which mirrors the environment layout's routes (_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.*)./_,/_/nonsenseand/_/tasksall behave alike.invitesPath()first, mirroring_app._index/route.tsx— a deeplink is exactly the URL a new invitee is sent, and without this they were offered organization creation while their invite sat unaccepted.newProjectPath/newOrganizationPathexactly like_app._index/route.tsxdoes.new URL(request.url).pathnamerather than the splat param, which React Router decodes. The dashboard writes a task id containing a slash asgroup%2Fmy-task, and the decoded param split that into two segments that matched no route. Segments are passed through as they arrived — re-encoding would turn%2Finto%252F.Why the filename is
[_].$.tsThe brackets are Remix's character escape, and they're load-bearing. In the flat-route convention a segment beginning with
_is a pathless layout that contributes nothing to the URL, so a plain_.$.tswould mount this route at/*and swallow the entire site.createRoutePathskips a segment only when the cooked and the raw spelling both start with_:For
[_].$the escape parser yieldssegment = "_"butrawSegment = "[_]", so the skip doesn't fire and the segment survives as a literal. Running the repo's own routes directory throughflatRoutes()on@remix-run/dev2.17.5 confirms it:The same entry appears in the production server build's route manifest, no route-path collisions are reported across the other 482 routes, and nothing else in the app claims
/_or anything under it. No route file used[…]escaping before this one, so the test asserts the resulting pattern (/_/*) directly rather than leaving it implicit.One consequence: the prefix comparison in
deeplinkSuffixno longer folds case. It used to, because React Router compiles route paths with theiflag and/Deeplink/apikeysreally did reach the loader — but_has no case, so the fold became dead code. Page-name folding stays, since/_/APIKeysstill has a route to agree with.Why the map has two paths per entry
Most names are a page in their own right. Three segments under the environment layout are not — they exist only as the parent of param or child routes, so a bare URL matches no route and would 404:
taskstasks.dashboard,tasks.{standard,scheduled}.$taskParam,tasks.stream)taskswaitpointswaitpoints.tokens[.$waitpointParam])waitpoints/tokenswaitpoints/tokensmetricsmetrics.$dashboardKey,metrics.custom.$dashboardId)So each entry carries a
landingused for a bare/_/<name>and aprefixthat deeper segments hang off; for an ordinary page both are just the name. The two differ only where the page and the things underneath it live in different places: the task list is the environment root (that route is the env_index, titled "Tasks") while task detail pages are under/tasks. A single "prefix the target" rule would fixwaitpointsand breaktasks.metricsis left out entirely — it's only a legacy 301 shim todashboardswith no page of its own, so it falls back to the environment root rather than being given an invented target. A suffix that already spells out a path under the prefix is kept as written, so the shorthand/_/waitpoints/waitpoint_123and the longhand/_/waitpoints/tokens/waitpoint_123agree.ENV_PAGE_METAinapp/components/navigation/favoritePages.tsxlooked like the natural source for this list, but it only lists segments that need an icon and a label —tasks,agentsandsettingsget theirs from special cases when resolving page metadata — and it says nothing about whether a segment resolves on its own. The route files answer both questions, so they are the source of truth.No
/deeplink/alias. The route hasn't shipped, so nothing links to the old spelling and there's no compatibility to preserve. Keeping both would mean two route files and a two-valued prefix constant for no gain.No project-picker fallback page. The presenter always resolves something for a user with at least one project, and the zero-project case already has a well-defined destination (create project / create org), so an intermediate "which project did you mean?" page would never be shown in practice.
✅ Checklist
Testing
pnpm run format,pnpm run lint:fix,pnpm run typecheck --filter webappandpnpm run build --filter webappare clean — the build matters here because the route filename is what produces the URL. Not yet exercised against a running dashboard — worth clicking through/_/apikeys,/_/tasks,/_/waitpoints,/_/runs?statuses=COMPLETED_SUCCESSFULLY,/_/runs/run_…,/_/nonsenseand/_while signed out and signed in.app/utils/deeplinkPages.test.tsreads the environment layout's route files and enforces the invariants that keep the map honest:_indexchild, since a param route likemetrics.$dashboardKeyisn't somewhere you can arrive without the param;settings/generalandalerts/new, need that);/_/*, asserted withmatchPathagainst the prefix constant the loader strips — the one invariant with no in-repo precedent to lean on.Deliberate exclusions are named in the test:
_index(the environment root, whichtasksalready points at) andqueues_(Remix's layout-opt-out spelling ofqueues, not a distinct URL). Failures name the offending entry.Traversal is covered in both spellings, because the two escapes behave differently and it's worth knowing which layer stops each.
%2Fsurvives inpathname, so an escaped slash inside an id stays one segment.%2e%2edoes not: WHATWG normalises it to..and resolves it, so/_/runs/%2e%2e/%2e%2e/etcreaches the loader as/etc— outside the prefix, and treated as no suffix. Individually, a segment is dropped when it is./.., when it decodes to one, or when it isn't decodable. Both parser behaviours are asserted rather than assumed. The redirect loader itself is still untested: there is no existing pattern for testing a redirect-only Remix loader in this repo.Changelog
Short links like
/_/apikeysnow take you straight to that page in your current project and environment (.server-changes/deeplink-routes.md).Generated by Claude Code