From a95e32e7890cfabf55bb373202109caeba501a02 Mon Sep 17 00:00:00 2001 From: Sawyer Date: Thu, 17 Sep 2026 22:25:09 -0700 Subject: [PATCH 1/5] refactor(web): settings sections read through TanStack Query --- apps/web/src/query-client.ts | 4 ++ apps/web/src/settings/access.ts | 43 ------------- apps/web/src/settings/account-section.tsx | 35 ++-------- apps/web/src/settings/credentials-section.tsx | 61 +++++++++--------- apps/web/src/settings/grants-section.tsx | 58 ++++++++--------- apps/web/src/settings/index.ts | 2 +- apps/web/src/settings/people-section.tsx | 64 +++++++------------ apps/web/src/settings/roles-section.tsx | 55 +++++++--------- 8 files changed, 113 insertions(+), 209 deletions(-) diff --git a/apps/web/src/query-client.ts b/apps/web/src/query-client.ts index 0bb5e98fd..32d1552e8 100644 --- a/apps/web/src/query-client.ts +++ b/apps/web/src/query-client.ts @@ -99,6 +99,10 @@ export const tenantKeys = { // `invalidateQueries({ queryKey: tenantKeys.artifacts(tenantId) })` after // an upload covers both the list and the kind-nav counts. artifactCounts: (tenantId: string) => ["tenant", tenantId, "artifacts", "counts"] as const, + credentials: (tenantId: string) => ["tenant", tenantId, "credentials"] as const, + principals: (tenantId: string) => ["tenant", tenantId, "principals"] as const, + roles: (tenantId: string) => ["tenant", tenantId, "roles"] as const, + grants: (tenantId: string) => ["tenant", tenantId, "grants"] as const, /** Settings section-nav gating (People/Roles/Grants/Credentials). Keyed * so col2's nav band and the settings stage — mounted in separate * subtrees — share one cached probe instead of each firing its own. */ diff --git a/apps/web/src/settings/access.ts b/apps/web/src/settings/access.ts index 9ef2479e9..3111b7cab 100644 --- a/apps/web/src/settings/access.ts +++ b/apps/web/src/settings/access.ts @@ -9,8 +9,6 @@ // together made the gated nav vanish as if the principal were // unauthorized. -import { useEffect, useState } from "react"; - import { evaluate } from "./tenancy-api"; export type SectionAccess = "loading" | "allowed" | "denied" | "error"; @@ -48,44 +46,3 @@ export function coalesceSectionAccess(previous: SectionAccess, next: SectionAcce return next; } -function useResourceAccess( - tenantId: string | null, - principalId: string | null, - resource: string, -): SectionAccess { - const [access, setAccess] = useState("loading"); - - useEffect(() => { - if (tenantId === null || principalId === null) { - setAccess("loading"); - return; - } - let cancelled = false; - setAccess("loading"); - void probeSectionAccess(tenantId, principalId, resource).then((next) => { - if (!cancelled) setAccess(next); - }); - return () => { - cancelled = true; - }; - }, [tenantId, principalId, resource]); - - return access; -} - -/** One probe per section, run in parallel — a section stays out of the nav - * until its probe resolves `allowed`, so a slow probe reads as "not shown - * yet", never as a visible-but-disabled tab. A failed probe is `error`, - * not `denied`: the registry withholds the section but marks the group so - * a host can show a couldn't-check state instead of looking unauthorized. */ -export function useTenancyAccess( - tenantId: string | null, - principalId: string | null, -): TenancyAccess { - return { - people: useResourceAccess(tenantId, principalId, "principal"), - roles: useResourceAccess(tenantId, principalId, "role"), - grants: useResourceAccess(tenantId, principalId, "grant"), - credentials: useResourceAccess(tenantId, principalId, "credential"), - }; -} diff --git a/apps/web/src/settings/account-section.tsx b/apps/web/src/settings/account-section.tsx index c1315934d..45ef1a30a 100644 --- a/apps/web/src/settings/account-section.tsx +++ b/apps/web/src/settings/account-section.tsx @@ -18,10 +18,9 @@ import { } from "@corbits/react-ui"; import { Select } from "@corbits/react-ui/ui/select"; import { ChatCircleDots, Copy, SignOut } from "@/lib/icons"; -import { useCallback, useEffect, useState } from "react"; +import { useQuery } from "@tanstack/react-query"; -import type { APIQuery } from "@/lib/api-query"; -import { QueryView, UnauthenticatedError, describeQueryError } from "@/lib/api-query"; +import { QueryView, toAPIQuery } from "@/lib/api-query"; import { resolveAvatarFill } from "@/chat"; import webPackage from "../../package.json"; import { getAccount, type Account } from "./api"; @@ -32,33 +31,9 @@ import { SETTINGS_STRINGS } from "./strings"; const FEEDBACK_URL = `${webPackage.repository.url}/issues`; export function AccountSection({ onSignOut }: { readonly onSignOut?: () => void }) { - const [query, setQuery] = useState>({ kind: "loading" }); - - const load = useCallback(() => { - setQuery({ kind: "loading" }); - let cancelled = false; - getAccount() - .then((account) => { - if (!cancelled) setQuery({ kind: "ready", data: account }); - }) - .catch((cause: unknown) => { - if (cancelled) return; - if (cause instanceof UnauthenticatedError) { - setQuery({ kind: "unauthenticated" }); - return; - } - setQuery({ - kind: "error", - message: describeQueryError(cause), - retry: load, - }); - }); - return () => { - cancelled = true; - }; - }, []); - - useEffect(() => load(), [load]); + const query = toAPIQuery( + useQuery({ queryKey: ["me", "account"], queryFn: getAccount }), + ); return ( <> diff --git a/apps/web/src/settings/credentials-section.tsx b/apps/web/src/settings/credentials-section.tsx index 25f4e945f..1073f4777 100644 --- a/apps/web/src/settings/credentials-section.tsx +++ b/apps/web/src/settings/credentials-section.tsx @@ -26,10 +26,11 @@ import { TableRow, } from "@corbits/react-ui"; import type { CredentialType } from "@intx/types"; -import { useEffect, useState } from "react"; +import { useQuery, useQueryClient } from "@tanstack/react-query"; +import { useState } from "react"; -import type { APIQuery } from "@/lib/api-query"; -import { QueryView, UnauthenticatedError, describeQueryError } from "@/lib/api-query"; +import { QueryView, toAPIQuery } from "@/lib/api-query"; +import { tenantKeys } from "@/query-client"; import { createCredential, createProvider, @@ -42,41 +43,39 @@ import { } from "./credentials-api"; import { SETTINGS_STRINGS } from "./strings"; +type CredentialsData = { + readonly credentials: readonly Credential[]; + readonly providers: readonly Provider[]; +}; + export function CredentialsSection({ tenantId }: { readonly tenantId: string | null }) { - const [query, setQuery] = useState>({ kind: "loading" }); - const [reloadKey, setReloadKey] = useState(0); + const queryClient = useQueryClient(); const [createOpen, setCreateOpen] = useState(false); const [creating, setCreating] = useState(false); const [createError, setCreateError] = useState(null); const [rowError, setRowError] = useState(null); - const [providers, setProviders] = useState([]); - function reload() { - setReloadKey((value) => value + 1); - } + // Credentials and their providers load together: creating one needs the + // provider row for the typed name, so a split read would race the form. + const result = useQuery({ + queryKey: tenantKeys.credentials(tenantId ?? "none"), + queryFn: async (): Promise => { + if (tenantId === null) return { credentials: [], providers: [] }; + const [credentials, providers] = await Promise.all([ + listCredentials(tenantId), + listProviders(tenantId), + ]); + return { credentials, providers }; + }, + enabled: tenantId !== null, + }); + const query = toAPIQuery(result); + const providers = result.data?.providers ?? []; - useEffect(() => { + function reload() { if (tenantId === null) return; - let cancelled = false; - setQuery({ kind: "loading" }); - Promise.all([listCredentials(tenantId), listProviders(tenantId)]) - .then(([credentials, providerRows]) => { - if (cancelled) return; - setProviders(providerRows); - setQuery({ kind: "ready", data: credentials }); - }) - .catch((cause: unknown) => { - if (cancelled) return; - if (cause instanceof UnauthenticatedError) { - setQuery({ kind: "unauthenticated" }); - return; - } - setQuery({ kind: "error", message: describeQueryError(cause), retry: reload }); - }); - return () => { - cancelled = true; - }; - }, [tenantId, reloadKey]); + void queryClient.invalidateQueries({ queryKey: tenantKeys.credentials(tenantId) }); + } if (tenantId === null) { return ( @@ -114,7 +113,7 @@ export function CredentialsSection({ tenantId }: { readonly tenantId: string | n return ( - {(credentials) => ( + {({ credentials }) => ( ({}); - const [query, setQuery] = useState>({ - kind: "loading", - }); - const [reloadKey, setReloadKey] = useState(0); + const queryClient = useQueryClient(); const [createOpen, setCreateOpen] = useState(false); const [creating, setCreating] = useState(false); const [createError, setCreateError] = useState(null); const [rowError, setRowError] = useState(null); + // Filters are part of the key, so narrowing refetches instead of the list + // silently keeping the previous filter's rows. const filtersKey = JSON.stringify(filters); - function reload() { - setReloadKey((value) => value + 1); - } + const query = toAPIQuery( + useQuery({ + queryKey: [...tenantKeys.grants(tenantId ?? "none"), filtersKey], + queryFn: async (): Promise => { + if (tenantId === null) return { grants: [], roles: [], principals: [] }; + const [grants, roles, principals] = await Promise.all([ + listGrants(tenantId, filters), + listRoles(tenantId), + listPrincipals(tenantId), + ]); + return { grants, roles, principals }; + }, + enabled: tenantId !== null, + }), + ); - useEffect(() => { + function reload() { if (tenantId === null) return; - let cancelled = false; - setQuery({ kind: "loading" }); - Promise.all([listGrants(tenantId, filters), listRoles(tenantId), listPrincipals(tenantId)]) - .then(([grants, roles, principals]) => { - if (!cancelled) setQuery({ kind: "ready", data: { grants, roles, principals } }); - }) - .catch((cause: unknown) => { - if (cancelled) return; - if (cause instanceof UnauthenticatedError) { - setQuery({ kind: "unauthenticated" }); - return; - } - setQuery({ - kind: "error", - message: describeQueryError(cause), - retry: reload, - }); - }); - return () => { - cancelled = true; - }; - }, [tenantId, reloadKey, filtersKey]); + void queryClient.invalidateQueries({ queryKey: tenantKeys.grants(tenantId) }); + } if (tenantId === null) { return ( diff --git a/apps/web/src/settings/index.ts b/apps/web/src/settings/index.ts index d60f2d365..fd325b8a8 100644 --- a/apps/web/src/settings/index.ts +++ b/apps/web/src/settings/index.ts @@ -28,7 +28,7 @@ export type { PrincipalLabel } from "./identity"; export { GRANT_RESOURCES, GRANT_ACTIONS } from "./resource-vocabulary"; export type { GrantResource, GrantAction } from "./resource-vocabulary"; -export { useTenancyAccess, probeSectionAccess, coalesceSectionAccess } from "./access"; +export { probeSectionAccess, coalesceSectionAccess } from "./access"; export type { SectionAccess, TenancyAccess } from "./access"; export { diff --git a/apps/web/src/settings/people-section.tsx b/apps/web/src/settings/people-section.tsx index e47f27fc3..1b05c869e 100644 --- a/apps/web/src/settings/people-section.tsx +++ b/apps/web/src/settings/people-section.tsx @@ -20,10 +20,11 @@ import { TableHeader, TableRow, } from "@corbits/react-ui"; -import { useEffect, useState } from "react"; +import { useQuery, useQueryClient } from "@tanstack/react-query"; +import { useState } from "react"; -import type { APIQuery } from "@/lib/api-query"; -import { QueryView, UnauthenticatedError, describeQueryError } from "@/lib/api-query"; +import { QueryView, toAPIQuery } from "@/lib/api-query"; +import { tenantKeys } from "@/query-client"; import { reportError } from "@corbits/error-sink"; import { PRINCIPAL_KIND_LABEL, principalLabel } from "./identity"; @@ -59,47 +60,30 @@ type PeopleData = { }; export function PeopleSection({ tenantId }: { readonly tenantId: string | null }) { - const [query, setQuery] = useState>({ - kind: "loading", - }); - const [reloadKey, setReloadKey] = useState(0); + const queryClient = useQueryClient(); const [rowError, setRowError] = useState(null); - function reload() { - setReloadKey((value) => value + 1); - } + // People are the user-kind principals; the role picker needs this tenant's + // roles alongside them, so both read under one key every write invalidates. + const query = toAPIQuery( + useQuery({ + queryKey: tenantKeys.principals(tenantId ?? "none"), + queryFn: async (): Promise => { + if (tenantId === null) return { people: [], roles: [] }; + const [principals, roles] = await Promise.all([ + listPrincipals(tenantId), + listRoles(tenantId), + ]); + return { people: principals.filter((p) => p.kind === "user"), roles }; + }, + enabled: tenantId !== null, + }), + ); - useEffect(() => { + function reload() { if (tenantId === null) return; - let cancelled = false; - setQuery({ kind: "loading" }); - Promise.all([listPrincipals(tenantId), listRoles(tenantId)]) - .then(([principals, roles]) => { - if (!cancelled) - setQuery({ - kind: "ready", - data: { - people: principals.filter((p) => p.kind === "user"), - roles, - }, - }); - }) - .catch((cause: unknown) => { - if (cancelled) return; - if (cause instanceof UnauthenticatedError) { - setQuery({ kind: "unauthenticated" }); - return; - } - setQuery({ - kind: "error", - message: describeQueryError(cause), - retry: reload, - }); - }); - return () => { - cancelled = true; - }; - }, [tenantId, reloadKey]); + void queryClient.invalidateQueries({ queryKey: tenantKeys.principals(tenantId) }); + } if (tenantId === null) { return ( diff --git a/apps/web/src/settings/roles-section.tsx b/apps/web/src/settings/roles-section.tsx index 67a7b0126..d9b24a453 100644 --- a/apps/web/src/settings/roles-section.tsx +++ b/apps/web/src/settings/roles-section.tsx @@ -24,10 +24,11 @@ import { TableHeader, TableRow, } from "@corbits/react-ui"; -import { useEffect, useState } from "react"; +import { useQuery, useQueryClient } from "@tanstack/react-query"; +import { useState } from "react"; -import type { APIQuery } from "@/lib/api-query"; -import { QueryView, UnauthenticatedError, describeQueryError } from "@/lib/api-query"; +import { QueryView, toAPIQuery } from "@/lib/api-query"; +import { tenantKeys } from "@/query-client"; import { principalLabel } from "./identity"; import { SETTINGS_STRINGS } from "./strings"; import { @@ -48,41 +49,33 @@ type RolesData = { }; export function RolesSection({ tenantId }: { readonly tenantId: string | null }) { - const [query, setQuery] = useState>({ kind: "loading" }); - const [reloadKey, setReloadKey] = useState(0); + const queryClient = useQueryClient(); const [createOpen, setCreateOpen] = useState(false); const [creating, setCreating] = useState(false); const [createError, setCreateError] = useState(null); const [rowError, setRowError] = useState(null); - function reload() { - setReloadKey((value) => value + 1); - } + // Roles and principals read together: the assignment table needs both, and + // every write below invalidates this one key. + const query = toAPIQuery( + useQuery({ + queryKey: tenantKeys.roles(tenantId ?? "none"), + queryFn: async (): Promise => { + if (tenantId === null) return { roles: [], principals: [] }; + const [roles, principals] = await Promise.all([ + listRoles(tenantId), + listPrincipals(tenantId), + ]); + return { roles, principals }; + }, + enabled: tenantId !== null, + }), + ); - useEffect(() => { + function reload() { if (tenantId === null) return; - let cancelled = false; - setQuery({ kind: "loading" }); - Promise.all([listRoles(tenantId), listPrincipals(tenantId)]) - .then(([roles, principals]) => { - if (!cancelled) setQuery({ kind: "ready", data: { roles, principals } }); - }) - .catch((cause: unknown) => { - if (cancelled) return; - if (cause instanceof UnauthenticatedError) { - setQuery({ kind: "unauthenticated" }); - return; - } - setQuery({ - kind: "error", - message: describeQueryError(cause), - retry: reload, - }); - }); - return () => { - cancelled = true; - }; - }, [tenantId, reloadKey]); + void queryClient.invalidateQueries({ queryKey: tenantKeys.roles(tenantId) }); + } if (tenantId === null) { return ( From a6ccb3ae0f8c9711b63caa4140c22ac71a49421c Mon Sep 17 00:00:00 2001 From: Sawyer Date: Thu, 17 Sep 2026 22:29:08 -0700 Subject: [PATCH 2/5] refactor(web): retired paths resolve in the router; redirects hop during render --- apps/web/src/app.tsx | 31 ++--- apps/web/src/main.tsx | 21 +-- apps/web/src/pages/home-page.tsx | 14 +- .../src/pages/legacy-settings-redirects.tsx | 80 ----------- apps/web/src/pages/settings-page.tsx | 22 ++- apps/web/src/redirect.tsx | 23 ++++ apps/web/src/router-store.ts | 59 ++++++++ apps/web/src/routes.tsx | 128 ++++-------------- 8 files changed, 143 insertions(+), 235 deletions(-) delete mode 100644 apps/web/src/pages/legacy-settings-redirects.tsx create mode 100644 apps/web/src/redirect.tsx create mode 100644 apps/web/src/router-store.ts diff --git a/apps/web/src/app.tsx b/apps/web/src/app.tsx index da9d265d2..18527fcc4 100644 --- a/apps/web/src/app.tsx +++ b/apps/web/src/app.tsx @@ -7,7 +7,7 @@ import { Button, EmptyState } from "@corbits/react-ui"; import { WorkbenchLoadingState } from "@/chat"; import { QueryClientProvider } from "@tanstack/react-query"; import { BoldIconProvider, WarningCircle } from "@/lib/icons"; -import { useEffect, useMemo } from "react"; +import { useMemo } from "react"; import { AuthScreen } from "./auth-screen"; import { BenchProvider } from "./bench-context"; @@ -18,32 +18,13 @@ import { NotFoundPage } from "./pages/not-found-page"; import { OnboardingPage } from "./pages/onboarding-page"; import { ProvisioningErrorPage } from "./pages/provisioning-error-page"; import { createAppQueryClient } from "./query-client"; +import { Redirect } from "./redirect"; import { APP_ROUTES, LOGIN_PATH, matchesRoute, ONBOARDING_PATH } from "./routes"; import type { SessionState, SessionUser } from "./session"; import { AppShell } from "./shell/app-shell"; import { ComposerInsertionProvider } from "./shell/composer-insertion"; import { ShellChromeProvider } from "./shell/shell-chrome-provider"; -/** Any signed-out request for a path other than `/login` itself bounces - * there with `?next=` so a successful sign-in returns to where the visitor - * meant to go — the URL is the source of truth for "where was I headed", - * not an implicit conditional swap in `App`. */ -function LoginRedirect({ path, navigate }: { readonly path: string; readonly navigate: Navigate }) { - useEffect(() => { - navigate(buildLoginRedirect(path)); - }, [path, navigate]); - return null; -} - -/** An already-authed visit to `/login` (a stale tab, a bookmark) bounces - * home rather than showing the sign-in form to someone already signed in. */ -function LoginBounceHome({ navigate }: { readonly navigate: Navigate }) { - useEffect(() => { - navigate("/"); - }, [navigate]); - return null; -} - /** * Onboarding renders above the shell entirely — no rail, no col2, no bench * dock, nothing that implies a workbench already exists. An account the @@ -150,8 +131,10 @@ export function App({ ); case "signed-out": + // The URL is the source of truth for "where was I headed": a + // signed-out request for another path carries `?next=` to login. if (path !== LOGIN_PATH) { - return ; + return ; } return ; case "error": @@ -170,8 +153,10 @@ export function App({ ); case "signed-in": + // An already-authed visit to `/login` (a stale tab, a bookmark) + // bounces home rather than showing the sign-in form again. if (path === LOGIN_PATH) { - return ; + return ; } if (path === ONBOARDING_PATH) { return ; diff --git a/apps/web/src/main.tsx b/apps/web/src/main.tsx index 58f61df7d..d02525e6d 100644 --- a/apps/web/src/main.tsx +++ b/apps/web/src/main.tsx @@ -3,7 +3,7 @@ import "./app.css"; import "./tailwind.css"; import { ThemeProvider, Toaster, toast } from "@corbits/react-ui"; -import { StrictMode, useCallback, useEffect, useState } from "react"; +import { StrictMode, useCallback, useEffect, useState, useSyncExternalStore } from "react"; import { createRoot } from "react-dom/client"; import { getLogger } from "@/lib/client-log"; @@ -11,6 +11,7 @@ import { AppErrorBoundary } from "./app-error-boundary"; import { App } from "./app"; import { validatedNextPath } from "./login-next"; import { triggerFirstLoginProvisioning } from "./onboarding"; +import { getPath, navigateTo, subscribeToPath } from "./router-store"; import { ONBOARDING_PATH } from "./routes"; import { fetchSession, signOut } from "./session"; import type { SessionState, SessionUser } from "./session"; @@ -18,20 +19,10 @@ import type { SessionState, SessionUser } from "./session"; const log = getLogger("web.session"); function Root() { - const [path, setPath] = useState(window.location.pathname); - useEffect(() => { - const handlePopState = () => setPath(window.location.pathname); - window.addEventListener("popstate", handlePopState); - return () => window.removeEventListener("popstate", handlePopState); - }, []); - const navigate = useCallback((to: string) => { - window.history.pushState(null, "", to); - // `path` state is pathname-only (every comparison against it — - // `matchesRoute`, `LOGIN_PATH`, `ONBOARDING_PATH` — expects a bare - // path); a query string like `/login?next=...` still lands on the - // URL bar via `pushState` above, just not in this state. - setPath(new URL(to, window.location.origin).pathname); - }, []); + // History lives outside React (`./router-store`), so the path is a + // subscription, not an effect that starts listening after first paint. + const path = useSyncExternalStore(subscribeToPath, getPath); + const navigate = navigateTo; const [session, setSession] = useState({ kind: "loading" }); const probe = useCallback(() => { diff --git a/apps/web/src/pages/home-page.tsx b/apps/web/src/pages/home-page.tsx index f539c008d..92f1402d8 100644 --- a/apps/web/src/pages/home-page.tsx +++ b/apps/web/src/pages/home-page.tsx @@ -5,7 +5,6 @@ import { Button, EmptyState, PageShell } from "@corbits/react-ui"; import { WarningCircle } from "@/lib/icons"; import { useQuery } from "@tanstack/react-query"; -import { useEffect } from "react"; import { WorkbenchLoadingState } from "@/chat"; import { listChats } from "@/chat/threads-api"; @@ -13,6 +12,7 @@ import { listChats } from "@/chat/threads-api"; import { useBench } from "../bench-context"; import { chatKeys, chatPath, NEW_CHAT_PATH } from "../chat-path"; import { useNavigate } from "../navigation"; +import { Redirect } from "../redirect"; export function HomeRoute() { const navigate = useNavigate(); @@ -23,12 +23,6 @@ export function HomeRoute() { queryFn: () => listChats(selectedTenantId ?? ""), }); - const newest = chats.data?.[0]; - useEffect(() => { - if (chats.data === undefined) return; - navigate(newest === undefined ? NEW_CHAT_PATH : chatPath(newest.id)); - }, [chats.data, newest, navigate]); - if (memberships.kind === "error" || chats.isError) { const cause: unknown = chats.error; const message = @@ -53,6 +47,12 @@ export function HomeRoute() { ); } + if (chats.data !== undefined) { + const newest = chats.data[0]; + const to = newest === undefined ? NEW_CHAT_PATH : chatPath(newest.id); + return ; + } + return (
diff --git a/apps/web/src/pages/legacy-settings-redirects.tsx b/apps/web/src/pages/legacy-settings-redirects.tsx deleted file mode 100644 index 164ad4d53..000000000 --- a/apps/web/src/pages/legacy-settings-redirects.tsx +++ /dev/null @@ -1,80 +0,0 @@ -// Old links that must still land somewhere real. Agents and Skills were -// Settings sections for a stretch (: `/settings/agents[/:id]`, -// `/settings/skills[/:id]`); / moved both back out to their -// own rail destinations, so any deep link into the old Settings home now -// bounces to the new one, preserving a deep-linked id. Library was renamed -// Files, then Artifacts, moving off `/library` and `/files` in turn — both -// old prefixes bounce to `/artifacts` the same way. - -import { useEffect } from "react"; - -function legacyRedirectTarget(path: string, oldPrefix: string, newPrefix: string): string { - if (path === oldPrefix) return newPrefix; - if (path.startsWith(`${oldPrefix}/`)) { - return `${newPrefix}/${path.slice(oldPrefix.length + 1)}`; - } - return newPrefix; -} - -export function LegacyRedirect({ - path, - navigate, - oldPrefix, - newPrefix, -}: { - readonly path: string; - readonly navigate: (to: string) => void; - readonly oldPrefix: string; - readonly newPrefix: string; -}) { - useEffect(() => { - navigate(legacyRedirectTarget(path, oldPrefix, newPrefix)); - }, [path, navigate, oldPrefix, newPrefix]); - return null; -} - -export function LegacySettingsAgentsRedirect({ - path, - navigate, -}: { - readonly path: string; - readonly navigate: (to: string) => void; -}) { - return ( - - ); -} - -export function LegacySettingsSkillsRedirect({ - path, - navigate, -}: { - readonly path: string; - readonly navigate: (to: string) => void; -}) { - return ( - - ); -} - -export function LegacyLibraryRedirect({ - path, - navigate, -}: { - readonly path: string; - readonly navigate: (to: string) => void; -}) { - return ( - - ); -} diff --git a/apps/web/src/pages/settings-page.tsx b/apps/web/src/pages/settings-page.tsx index feb04ddf5..60cedc178 100644 --- a/apps/web/src/pages/settings-page.tsx +++ b/apps/web/src/pages/settings-page.tsx @@ -9,10 +9,10 @@ import { flattenSettingsSections, resolveActiveSection, SettingsShell } from "@/settings"; import { PageShell } from "@corbits/react-ui"; -import { useEffect } from "react"; import { useBench } from "../bench-context"; import { useSignOut } from "../navigation"; +import { Redirect } from "../redirect"; import { SETTINGS_PATH_PREFIX, settingsEntityIdFromPath, @@ -58,17 +58,15 @@ export function SettingsRoute({ // Bare /settings, and an unknown or gate-denied /settings/:section, both // correct to the first allowed section's own URL — never a fallback - // rendered under a URL the section nav disagrees with. Depends on - // `activeSectionId` (a primitive), not `activeSection` (a fresh object - // every render, since `resolveSettingsSectionGroups` isn't memoized) — - // otherwise an unrelated re-render (e.g. BenchProvider persisting the - // resolved tenant id) would refire this and double-navigate. - useEffect(() => { - if (activeSectionId === null) return; - if (requestedId !== null && requestedSectionExists) return; - if (requestedId !== null && !accessSettled) return; - navigate(`${SETTINGS_PATH_PREFIX}/${activeSectionId}`); - }, [requestedId, requestedSectionExists, accessSettled, activeSectionId, navigate]); + // rendered under a URL the section nav disagrees with. + if ( + activeSectionId !== null && + (requestedId === null || (!requestedSectionExists && accessSettled)) + ) { + return ( + + ); + } return (
diff --git a/apps/web/src/redirect.tsx b/apps/web/src/redirect.tsx new file mode 100644 index 000000000..bd6b79294 --- /dev/null +++ b/apps/web/src/redirect.tsx @@ -0,0 +1,23 @@ +// A screen that only exists to forward. The hop happens during render, not +// from an effect, so the route it leaves never paints a frame of its own — +// the router store publishes the new path on a microtask, so nothing is +// updated mid-render. + +import type { Navigate } from "./navigation"; + +/** Idempotent across re-renders: the hop only fires while the current path + * still differs from the target. */ +export function Redirect({ + to, + from, + navigate, +}: { + readonly to: string; + readonly from: string; + readonly navigate: Navigate; +}) { + if (from !== new URL(to, window.location.origin).pathname) { + navigate(to); + } + return null; +} diff --git a/apps/web/src/router-store.ts b/apps/web/src/router-store.ts new file mode 100644 index 000000000..edc1f25c6 --- /dev/null +++ b/apps/web/src/router-store.ts @@ -0,0 +1,59 @@ +// The browser history as an external store: the path lives outside React, +// so the shell subscribes with `useSyncExternalStore` instead of an effect +// that registers a `popstate` listener after the first paint. Retired paths +// are canonicalized here, before any screen mounts — a bookmark to +// `/files/a1` becomes `/artifacts/a1` in the URL bar and in the store, and +// no route entry exists just to bounce it. + +import { redirectTargetFor } from "./routes"; + +function canonicalPath(pathname: string): string { + return redirectTargetFor(pathname) ?? pathname; +} + +const listeners = new Set<() => void>(); + +let currentPath = canonicalPath(window.location.pathname); +if (currentPath !== window.location.pathname) { + window.history.replaceState(null, "", currentPath); +} + +function setPath(next: string): void { + if (next === currentPath) return; + currentPath = next; + // Published on a microtask so a render-phase `navigateTo` (a forwarding + // route resolving its target) never updates a subscriber mid-render. + queueMicrotask(() => { + for (const listener of listeners) listener(); + }); +} + +window.addEventListener("popstate", () => { + const canonical = canonicalPath(window.location.pathname); + if (canonical !== window.location.pathname) { + window.history.replaceState(null, "", canonical); + } + setPath(canonical); +}); + +export function subscribeToPath(listener: () => void): () => void { + listeners.add(listener); + return () => listeners.delete(listener); +} + +export function getPath(): string { + return currentPath; +} + +/** + * Push a new entry and publish the new path. The store keeps the pathname + * only — every comparison against it (`matchesRoute`, `LOGIN_PATH`, + * `ONBOARDING_PATH`) expects a bare path — while a query string like + * `/login?next=...` still reaches the URL bar. + */ +export function navigateTo(to: string): void { + const url = new URL(to, window.location.origin); + const canonical = canonicalPath(url.pathname); + window.history.pushState(null, "", canonical === url.pathname ? to : canonical); + setPath(canonical); +} diff --git a/apps/web/src/routes.tsx b/apps/web/src/routes.tsx index 4fe5dc09f..0d789de02 100644 --- a/apps/web/src/routes.tsx +++ b/apps/web/src/routes.tsx @@ -8,16 +8,12 @@ // open her conversation) for a bench with a workbench already, or the // guided first-workbench describe screen for a bench with none — never a // Home dashboard. -// Approvals has no page — the Activity band owns them. Agents -// and Skills are their own rail destinations again — they spent -// a stretch as Settings sections and `/settings/agents[/:id]` / -// `/settings/skills[/:id]` stay routable only as redirects back here, so -// old links and bookmarks still land somewhere real. Library was renamed -// Files, then Workbench renamed Routines to Workflows and Files to -// Artifacts — `/routines` and `/files` (and the older `/library`) all -// redirect to their current homes. Inbox is gone too (tasks + approvals -// don't flow into workbenches); `/inbox` stays routable only as a -// redirect to `/`. +// Approvals has no page — the Activity band owns them. Agents and Skills +// are their own rail destinations again. Every path this app has retired +// along the way (`/settings/agents`, `/routines`, `/files`, `/library`, +// `/inbox`, `/mission-control`) lives in `RETIRED_PREFIXES` below, which +// the router resolves before a screen mounts — never a route entry whose +// only job is to bounce. import { ChatCircle, @@ -31,7 +27,7 @@ import { } from "@/lib/icons"; import { CHAT_STRINGS } from "@/chat"; import type { Slug } from "@/lib/slug"; -import { lazy, useEffect, type ReactElement, type ReactNode } from "react"; +import { lazy, type ReactElement, type ReactNode } from "react"; import { AGENTS_PATH_PREFIX, @@ -42,12 +38,6 @@ import { } from "./path-ids"; import { WORKBENCH_PATH_PREFIX, isWorkbenchPath } from "./workbench-path"; import { CHATS_PATH_PREFIX, isChatPath } from "./chat-path"; -import { - LegacyRedirect, - LegacyLibraryRedirect, - LegacySettingsAgentsRedirect, - LegacySettingsSkillsRedirect, -} from "./pages/legacy-settings-redirects"; // Each signed-in page is a dynamic import so Vite emits one chunk per // screen. Static imports here pulled chat-ui, artifact-ui, settings-ui, @@ -204,15 +194,10 @@ export function matchesRoute(routePath: string, path: string): boolean { } if ( routePath === "/workflows" || - routePath === "/routines" || - routePath === "/library" || routePath === "/artifacts" || - routePath === "/files" || routePath === "/insights" || routePath === "/agents" || routePath === "/skills" || - routePath === "/settings/agents" || - routePath === "/settings/skills" || routePath === SETTINGS_PATH ) { return path === routePath || path.startsWith(`${routePath}/`); @@ -220,12 +205,29 @@ export function matchesRoute(routePath: string, path: string): boolean { return routePath === path; } -/** Bounces old `/inbox` links and bookmarks home (: the Inbox page - * is gone — tasks and approvals don't flow into a workbench). */ -function InboxRedirect({ navigate }: { readonly navigate: (to: string) => void }) { - useEffect(() => { - navigate("/"); - }, [navigate]); +/** Every retired path prefix and where it lives now. Resolved by the router + * before a screen mounts, so a bookmark never renders a page just to bounce + * off it. */ +const RETIRED_PREFIXES: readonly (readonly [string, string])[] = [ + ["/mission-control", "/"], + ["/inbox", "/"], + ["/routines", "/workflows"], + ["/files", "/artifacts"], + ["/library", "/artifacts"], + ["/settings/agents", "/agents"], + ["/settings/skills", "/skills"], +]; + +/** The current home for a retired path, `null` for a path that is still its + * own. A deep-linked id is carried across (`/files/a1` → `/artifacts/a1`). */ +export function redirectTargetFor(path: string): string | null { + for (const [oldPrefix, newPrefix] of RETIRED_PREFIXES) { + if (path === oldPrefix) return newPrefix; + if (path.startsWith(`${oldPrefix}/`)) { + const rest = path.slice(oldPrefix.length + 1); + return newPrefix === "/" ? "/" : `${newPrefix}/${rest}`; + } + } return null; } @@ -237,16 +239,6 @@ export const APP_ROUTES: readonly AppRoute[] = [ render: () => , hasStageTopBar: false, }, - { - // Mission Control is gone — pending approvals and activity live in the - // workbench view now; old links and bookmarks bounce home. - path: "/mission-control", - label: "Mission Control", - icon: , - render: (path: string, navigate: (to: string) => void) => ( - - ), - }, { path: NEW_WORKBENCH_PATH, label: CHAT_STRINGS.newWorkbenchAction, @@ -267,14 +259,6 @@ export const APP_ROUTES: readonly AppRoute[] = [ icon: , render: (path: string) => , }, - { - path: "/inbox", - label: "Inbox", - icon: , - render: (_path: string, navigate: (to: string) => void) => ( - - ), - }, { // Detail routes come before their roster: the roster prefix matches // everything beneath it, so the more specific slug route has to be @@ -290,20 +274,6 @@ export const APP_ROUTES: readonly AppRoute[] = [ icon: , render: () => , }, - { - // Old `/routines` links and bookmarks (its rename) land here. - path: "/routines", - label: "Workflows", - icon: , - render: (path: string, navigate: (to: string) => void) => ( - - ), - }, { // The renamed, remounted Library page — "Library" stays out // of user-facing copy, but the underlying artifact machinery @@ -313,24 +283,6 @@ export const APP_ROUTES: readonly AppRoute[] = [ icon: , render: (path: string) => , }, - { - // Old `/files` links and bookmarks (its rename) land here. - path: "/files", - label: "Artifacts", - icon: , - render: (path: string, navigate: (to: string) => void) => ( - - ), - }, - { - // Old `/library` links and bookmarks (its rename) land here. - path: "/library", - label: "Artifacts", - icon: , - render: (path: string, navigate: (to: string) => void) => ( - - ), - }, { path: AGENT_DETAIL_PATH, label: "Agent", @@ -347,16 +299,6 @@ export const APP_ROUTES: readonly AppRoute[] = [ ), }, - { - // Agents spent through as a Settings section — this - // entry keeps old `/settings/agents[/:id]` links routable. - path: "/settings/agents", - label: "Agents", - icon: , - render: (path: string, navigate: (to: string) => void) => ( - - ), - }, { path: SKILL_DETAIL_PATH, label: "Skill", @@ -369,16 +311,6 @@ export const APP_ROUTES: readonly AppRoute[] = [ icon: , render: (_path: string, navigate: (to: string) => void) => , }, - { - // Skills spent through as a Settings section — this - // entry keeps old `/settings/skills[/:id]` links routable. - path: "/settings/skills", - label: "Skills", - icon: , - render: (path: string, navigate: (to: string) => void) => ( - - ), - }, { path: "/tools", label: "Tools", From 147d48a9e3cc1a21ccd6bf53c238ba7f16de7f15 Mon Sep 17 00:00:00 2001 From: Sawyer Date: Thu, 17 Sep 2026 22:32:06 -0700 Subject: [PATCH 3/5] refactor(web): shell layout, canvas scope and stage search drop their effects --- apps/web/src/router-store.ts | 4 + apps/web/src/shell/app-shell.tsx | 83 +++++++++++-------- apps/web/src/shell/layout/index.ts | 1 - apps/web/src/shell/layout/use-scroll-reset.ts | 16 ---- apps/web/src/shell/layout/use-shell-layout.ts | 43 ++++++---- apps/web/src/shell/shell-chrome-provider.tsx | 50 +++++------ apps/web/src/shell/stage-search.tsx | 29 ++++--- 7 files changed, 117 insertions(+), 109 deletions(-) delete mode 100644 apps/web/src/shell/layout/use-scroll-reset.ts diff --git a/apps/web/src/router-store.ts b/apps/web/src/router-store.ts index edc1f25c6..12e1ed477 100644 --- a/apps/web/src/router-store.ts +++ b/apps/web/src/router-store.ts @@ -54,6 +54,10 @@ export function getPath(): string { export function navigateTo(to: string): void { const url = new URL(to, window.location.origin); const canonical = canonicalPath(url.pathname); + // A hop to the path already showing is a no-op, which is what makes a + // render-phase `navigate` safe to run twice (StrictMode, a re-render): + // it can never stack duplicate history entries. + if (canonical === currentPath) return; window.history.pushState(null, "", canonical === url.pathname ? to : canonical); setPath(canonical); } diff --git a/apps/web/src/shell/app-shell.tsx b/apps/web/src/shell/app-shell.tsx index eb3309ab8..0ee4d5251 100644 --- a/apps/web/src/shell/app-shell.tsx +++ b/apps/web/src/shell/app-shell.tsx @@ -11,7 +11,7 @@ // and this component only reads it through the same hooks page code // already uses. -import { lazy, Suspense, useEffect, useRef, useState, type ReactNode } from "react"; +import { lazy, Suspense, useRef, useState, type ReactNode, type RefObject } from "react"; import type { ArtifactSaveState } from "@/library"; import { WorkbenchLoadingState } from "@/chat"; @@ -23,7 +23,6 @@ import { saveArtifactContent } from "./library-artifacts"; import { APP_ROUTES, matchesRoute } from "../routes"; import type { SessionUser } from "../session"; import { StageTopBar } from "./stage-top-bar"; -import { useScrollReset } from "@/shell/layout"; import { useCanvasColumnArtifact, useCanvasColumnAvailable, @@ -55,6 +54,24 @@ function routeLabel(path: string): string { return route?.label ?? "Workbench"; } +/** Route changes must not inherit the previous page's scroll position. Keyed + * on the path, so it remounts per route and resets the scroll container as + * its ref attaches. */ +function ScrollToTop({ + containerRef, +}: { + readonly containerRef: RefObject; +}) { + return ( +