Real 404 pages and per-route document titles#1261
Conversation
Deploying with
|
| Status | Name | Latest Commit | Preview URL | Updated (UTC) |
|---|---|---|---|---|
| ✅ Deployment successful! View logs |
executor-marketing | 11d7e4d | Commit Preview URL Branch Preview URL |
Jul 02 2026, 07:24 PM |
Deploying with
|
| Status | Name | Latest Commit | Updated (UTC) |
|---|---|---|---|
| ✅ Deployment successful! View logs |
executor-cloud | 11d7e4d | Jul 02 2026, 07:25 PM |
Greptile SummaryThis PR fixes two distinct UX issues: garbage URLs silently rendered the Integrations dashboard (root cause:
Confidence Score: 5/5Safe to merge — the gate change is well-scoped and well-documented, and the title hook is a thin, dependency-free effect with no side-effects beyond The No files require special attention beyond what prior review threads have already flagged. Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[URL navigated to] --> B{Router resolves URL}
B -->|Matched a route| C{OrgSlugGate: isNotFound?}
B -->|No match| D[globalNotFound = true on match]
D --> C
C -->|isNotFound = true| E[needsCanonicalize = false]
C -->|isNotFound = false| F{urlSlug === activeSlug?}
F -->|Yes| G[needsCanonicalize = false - Render children normally]
F -->|No - e.g. bare URL or wrong slug| H[needsCanonicalize = true - navigate replace to activeSlug URL]
E --> I[notFoundComponent renders - 404 page]
G --> J[Page renders + document title set via useExecutorDocumentTitle]
H --> A
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
flowchart TD
A[URL navigated to] --> B{Router resolves URL}
B -->|Matched a route| C{OrgSlugGate: isNotFound?}
B -->|No match| D[globalNotFound = true on match]
D --> C
C -->|isNotFound = true| E[needsCanonicalize = false]
C -->|isNotFound = false| F{urlSlug === activeSlug?}
F -->|Yes| G[needsCanonicalize = false - Render children normally]
F -->|No - e.g. bare URL or wrong slug| H[needsCanonicalize = true - navigate replace to activeSlug URL]
E --> I[notFoundComponent renders - 404 page]
G --> J[Page renders + document title set via useExecutorDocumentTitle]
H --> A
Reviews (2): Last reviewed commit: "Show real not found pages and route titl..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| import { useEffect } from "react"; | ||
|
|
||
| const APP_NAME = "Executor"; | ||
|
|
||
| export function executorDocumentTitle(page: string): string { | ||
| return `${page} · ${APP_NAME}`; | ||
| } | ||
|
|
||
| export function useExecutorDocumentTitle(page: string | null | undefined): void { | ||
| useEffect(() => { | ||
| if (!page || typeof document === "undefined") return; | ||
| document.title = executorDocumentTitle(page); | ||
| }, [page]); | ||
| } |
There was a problem hiding this comment.
.ts, not .tsx
The file contains no JSX — it only imports useEffect and returns void. The .tsx extension enables the JSX transform unnecessarily. Renaming to document-title.ts would keep the extension honest and slightly shorten the generated output.
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
| notFoundComponent: NotFoundPage, | ||
| component: RootComponent, | ||
| }); | ||
|
|
||
| function NotFoundPage() { | ||
| return ( | ||
| <main className="flex min-h-screen items-center justify-center bg-background px-6 py-10"> | ||
| <section className="w-full max-w-md text-center"> | ||
| <p className="text-xs font-medium uppercase tracking-wide text-muted-foreground">404</p> | ||
| <h1 className="mt-2 text-xl font-semibold text-foreground">Page not found</h1> | ||
| <p className="mt-2 text-sm leading-6 text-muted-foreground"> | ||
| There's nothing at this address. | ||
| </p> | ||
| <a | ||
| href="/" | ||
| className="mt-6 inline-flex h-9 items-center justify-center rounded-md bg-primary px-4 text-sm font-medium text-primary-foreground shadow transition-colors hover:bg-primary/90" | ||
| > | ||
| Go home | ||
| </a> | ||
| </section> |
There was a problem hiding this comment.
NotFoundPage is duplicated verbatim across two root files
The identical component exists in both packages/app/src/routes/__root.tsx and apps/host-selfhost/web/routes/__root.tsx. Any future copy/style change will need to be made in both places. AGENTS.md recommends keeping package boundaries clear and extracting shared logic when the behavior is genuinely shared — this is a good candidate. A single export from packages/react (e.g. alongside the other page components) would keep both roots in sync automatically.
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
| export function useExecutorDocumentTitle(page: string | null | undefined): void { | ||
| useEffect(() => { | ||
| if (!page || typeof document === "undefined") return; | ||
| document.title = executorDocumentTitle(page); | ||
| }, [page]); |
There was a problem hiding this comment.
document.title is never reset on unmount, so navigating to the 404 page shows the previous page's title
The hook sets document.title on mount but has no cleanup. The NotFoundPage components in both roots don't call useExecutorDocumentTitle, so any user who navigates from a real page to a 404 URL will see the previous page's title persisting in the browser tab (e.g. "Policies · Executor" while the 404 content is shown). Adding a return function that resets to APP_NAME (or calling the hook with a fixed string like "404" inside NotFoundPage) would close this gap.
| }, [namespace]); | ||
|
|
||
| const integrationData = AsyncResult.isSuccess(integration) ? integration.value : null; | ||
| useExecutorDocumentTitle(integrationData?.name || namespace); |
There was a problem hiding this comment.
|| coerces empty-string names to the namespace fallback
|| treats an empty string as falsy, so an integration whose name is "" would silently fall back to showing the namespace slug as the document title. Use ?? to only fall back when the name is null/undefined, which is the actual sentinel for "not yet loaded or not present".
| useExecutorDocumentTitle(integrationData?.name || namespace); | |
| useExecutorDocumentTitle(integrationData?.name ?? namespace); |
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
Cloudflare preview
Sign-in is Cloudflare Access (one-time PIN to an allowed email). The preview has its own database and encryption key; it is destroyed when this PR closes. |
@executor-js/cli
@executor-js/config
@executor-js/execution
@executor-js/sdk
@executor-js/codemode-core
@executor-js/runtime-quickjs
@executor-js/plugin-file-secrets
@executor-js/plugin-graphql
@executor-js/plugin-keychain
@executor-js/plugin-mcp
@executor-js/plugin-onepassword
@executor-js/plugin-openapi
executor
commit: |
RhysSullivan
force-pushed
the
ux/not-found-and-titles
branch
from
ebe36bd to
a511bb1
Compare
RhysSullivan
force-pushed
the
ux/setup-status-retry
branch
from
ff9ad76 to
8fe8dc4
Compare
RhysSullivan
force-pushed
the
ux/setup-status-retry
branch
from
8fe8dc4 to
376f68c
Compare
RhysSullivan
force-pushed
the
ux/not-found-and-titles
branch
from
a511bb1 to
11d7e4d
Compare
1 participant
Unknown URLs (e.g. /default/does-not-exist, /default/org) silently rendered the Integrations dashboard under the wrong URL. Root cause: the org-slug gate treated any URL without an orgSlug param as a bare URL and canonicalized it back to the dashboard, which destroyed the router's not-found state before it could render. The browser tab title was also a single static string on every page.
The gate now skips canonicalization when the router reports a global not-found, self-host and the local app get a proper 404 page (404 / Page not found / Go home), and a small dependency-free helper sets document.title per route (Policies · Executor, API keys · Executor, and so on), including the integration name on detail pages.
Verified in the browser: garbage URLs render the 404 with the URL preserved, one-segment garbage canonicalizes cleanly, and all real routes still work. Typecheck is green.
Stacked on #1260.