From ad21d1a595196a1d27890c5ce561442fcf247808 Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 19 Sep 2026 19:51:27 +0000 Subject: [PATCH 1/2] fix(app-shell): recovery redirects follow the declared landing (objectui#7373) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `app.isDefault` declares where a deployment's home is. objectui#7256 (PR #7372) moved the console chrome's four Home affordances onto `useHomePath()`; the "you cannot be here" exits still named the environment launcher literally, so a control-plane customer refused an app — or sent off a surface their runtime does not serve — landed among "Build an app" / "Start from a template" cards that act on an environment their deployment does not have, beside "Your apps" tiles that are the control plane's own internal management apps. Measured first, per the ruling on the card: every `/home` occurrence in `packages/app-shell/src` + `apps/console/src` was classified before a line moved, and the classification ships in the pull request body. Nine live occurrences moved, one is held pending a decision, and the rest must not move — including the two this change is most likely to be misread as covering (`HOME_LAUNCHER_PATH`, and `RootRedirect`, which is `/`'s landing and has its own resolver). Retargeted onto the existing policy; no new policy was written: - `AppContent` — the access-denied screen's way back, and the bounce for a viewer with no app to enter; - `RequireAiSurface` — a runtime serving no AI agent. Only the DEFAULT moved: a host passing `redirectTo` still wins; - `AiChatPage` — the no-agent screen's Home, and the collapse-to-dock landing on a cold deep link. `resolveCollapseToDockTarget` takes the home path as a required argument rather than naming one, so a new call site cannot silently reintroduce the literal; - `StudioDesignSurface` — eviction when the package under the editor is deleted, and the header Home button; - `apps/console` — the `/studio` entry gate, and the Studio front door's wordmark beside it (two affordances one route apart may not name two different homes). `AcceptInvitationPage` is deliberately unchanged: it navigates immediately after an organization switch, where the app list in hand still belongs to the organization being left. The reading is recorded at the call site and on the card. Every ordinary environment is unchanged — where nothing is declared, and wherever the list is not yet an answer, the resolved path is `/home`. Each moved site gained a behavioural pin that fails on the previous implementation, and the existing `/home` pins are kept as the undeclared-deployment case rather than re-aimed. Co-Authored-By: Claude Claude-Session: https://claude.ai/code/session_018HrVaotisyhgmot9o2MLRq --- ...overy-redirects-follow-declared-landing.md | 36 ++++ .../src/components/StudioRoute.test.tsx | 91 ++++++++- apps/console/src/components/StudioRoute.tsx | 21 +- ...eAffordancesFollowDeclaration-7256.test.ts | 4 +- ...eryRedirectsFollowDeclaration-7373.test.ts | 179 ++++++++++++++++++ packages/app-shell/src/console/AppContent.tsx | 41 ++-- .../app-shell/src/console/ConsoleShell.tsx | 22 ++- .../AppContent.deniedVsUnpublished.test.tsx | 25 +++ .../AppContent.inaccessibleAppStrand.test.tsx | 21 +- .../__tests__/RequireAiSurface.test.tsx | 76 ++++++-- .../app-shell/src/console/ai/AiChatPage.tsx | 23 ++- .../resolveCollapseToDockTarget.test.ts | 46 +++-- .../manage/AcceptInvitationPage.tsx | 13 ++ ...nSurface.packageDeletionInference.test.tsx | 61 +++++- .../studio-design/StudioDesignSurface.tsx | 14 +- 15 files changed, 609 insertions(+), 64 deletions(-) create mode 100644 .changeset/7373-recovery-redirects-follow-declared-landing.md create mode 100644 packages/app-shell/src/__tests__/homeRecoveryRedirectsFollowDeclaration-7373.test.ts diff --git a/.changeset/7373-recovery-redirects-follow-declared-landing.md b/.changeset/7373-recovery-redirects-follow-declared-landing.md new file mode 100644 index 0000000000..cbdd2901f1 --- /dev/null +++ b/.changeset/7373-recovery-redirects-follow-declared-landing.md @@ -0,0 +1,36 @@ +--- +'@object-ui/app-shell': minor +'@object-ui/console': minor +--- + +The console's error-recovery exits follow the declared landing (objectui#7373). + +`app.isDefault` declares where a deployment's home is, and objectui#7256 (PR #7372) +made the console chrome's four Home affordances read it through `useHomePath()`. +The "you cannot be here" exits kept naming the environment launcher literally, so a +control-plane customer who hit one landed on a screen whose "Build an app" / +"Start from a template" cards act on an environment their deployment does not have +and whose "Your apps" tiles are the control plane's own internal management apps. + +Retargeted onto the existing policy — no new one was written: + +- `AppContent` — the access-denied screen's "Back to home", and the bounce for a + viewer with no app to enter; +- `RequireAiSurface` — a runtime that serves no AI agent (its `redirectTo` prop + still wins when a host passes one; only the default moved); +- `AiChatPage` — the no-agent screen's Home, and the collapse-to-dock landing on a + cold deep link. `resolveCollapseToDockTarget` now takes the home path as a + required argument instead of naming one; +- `StudioDesignSurface` — eviction when the package under the editor is deleted, + and the header's Home button; +- `apps/console`'s `/studio` entry gate and the Studio front door's wordmark. + +Every ordinary environment is unchanged: where nothing declares a landing, and +wherever the app list is not (yet) an answer, the resolved path IS `/home`. + +Two sites deliberately keep the launcher: `HOME_LAUNCHER_PATH` itself (it is the +launcher, and the fallback all of the above resolve through — ADR-0075), and +`RootRedirect`, which is `/`'s landing rather than a recovery exit and has its own +resolver. `AcceptInvitationPage` is unchanged pending a decision recorded on the +card: it navigates immediately after an organization switch, where the app list in +hand still belongs to the organization being left. diff --git a/apps/console/src/components/StudioRoute.test.tsx b/apps/console/src/components/StudioRoute.test.tsx index 34172289fb..06c24c6248 100644 --- a/apps/console/src/components/StudioRoute.test.tsx +++ b/apps/console/src/components/StudioRoute.test.tsx @@ -38,7 +38,9 @@ import '@testing-library/jest-dom/vitest'; import { describe, it, expect, beforeEach, vi } from 'vitest'; import { render, screen, waitFor } from '@testing-library/react'; +import React from 'react'; import { MemoryRouter, Routes, Route, useLocation } from 'react-router-dom'; +import { MetadataCtx, type MetadataContextValue } from '@object-ui/react'; /** Auth facts, swapped per test. `AuthGuard` itself stays real. */ let auth = { isAuthenticated: true, isLoading: false, user: { id: 'u1' } as unknown }; @@ -138,19 +140,53 @@ const pathname = () => screen.getByTestId('pathname').textContent; * `/studio` subtree, the home a non-holder is sent to, and the login surface an * unauthenticated visitor bounces to. */ -function renderStudioDeepLink(url: string) { +/** + * The app list the gate's home target resolves against (objectui#7373). + * `undefined` mounts the subtree with no metadata context at all — what every + * case written before that card saw, and what `useHomePath()` answers the + * launcher for. + */ +function withMetadata( + apps: MetadataContextValue['apps'] | undefined, + children: React.ReactNode, +) { + if (!apps) return <>{children}; + const value: MetadataContextValue = { + apps, + objects: [], dashboards: [], reports: [], pages: [], + loading: false, error: null, + refresh: async () => {}, invalidate: () => {}, ensureType: async () => [], + getItem: async () => null, getItemsByType: () => [], getTypeStatus: () => 'ready', + }; + return {children}; +} + +function renderStudioDeepLink(url: string, apps?: MetadataContextValue['apps']) { return render( - - {studioRoutes} - home} /> - login} /> - + {withMetadata( + apps, + + {studioRoutes} + home} /> + declared landing} + /> + login} /> + , + )} , ); } +/** A control plane: the landing is declared, and it is not the launcher. */ +const CONTROL_PLANE_APPS = [ + { name: 'cloud_control', label: 'Cloud', isDefault: true }, + { name: 'account', label: 'Account' }, +]; + /** A plain tenant org owner's real set on the measured shape — no `studio.access`. */ const TENANT_OWNER_CAPS = ['manage_org_users', 'setup.access', 'setup.write']; /** A platform operator: the same set plus the platform-exclusive entry capability. */ @@ -238,6 +274,35 @@ describe('/studio/* — the entry decision, both ways', () => { expect(designSurface).not.toHaveBeenCalled(); }); + it('a non-holder lands on the DECLARED landing where there is one (objectui#7373)', async () => { + // The card's case on this gate: a control-plane customer who follows a + // `/studio` link they may not enter. Pre-#7373 `redirectTo` defaulted to the + // `/home` literal, which on that deployment is the environment launcher — + // "Build an app" / "Start from a template" cards acting on an environment + // the control plane does not have. This pin fails on that implementation. + renderStudioDeepLink('/studio/hotcrm/data', CONTROL_PLANE_APPS); + + await waitFor(() => expect(pathname()).toBe('/apps/cloud_control')); + expect(screen.getByTestId('declared-landing')).toBeInTheDocument(); + expect(screen.queryByTestId('home-launcher')).not.toBeInTheDocument(); + // The load-bearing half is unchanged by the retarget: refusing is still + // refusing, and the builder is still never mounted. + expect(designSurface).not.toHaveBeenCalled(); + }); + + it('keeps the launcher for an environment that declares no landing', async () => { + // The status quo, as its own case: a real app list WITHOUT a declaration + // resolves to the launcher, exactly like the no-context cases above. + renderStudioDeepLink('/studio/hotcrm/data', [ + { name: 'crm', label: 'CRM' }, + { name: 'setup', label: 'Setup' }, + ]); + + await waitFor(() => expect(pathname()).toBe('/home')); + expect(screen.getByTestId('home-launcher')).toBeInTheDocument(); + expect(designSurface).not.toHaveBeenCalled(); + }); + it('NEGATIVE CONTROL: a holder still gets the front door, unchanged', async () => { // A gate that refused everyone would pass every assertion above. answerWith(OPERATOR_CAPS); @@ -257,6 +322,20 @@ describe('/studio/* — the entry decision, both ways', () => { expect(pathname()).toBe('/studio/hotcrm/data'); }); + it("the front door's wordmark walks back to the same home the gate bounces to", async () => { + // Two affordances one route apart — this wordmark and the pillar builder's + // header Home button — must not name two different homes; that asymmetry is + // the defect objectui#7256 measured and objectui#7373 finished removing. + answerWith(OPERATOR_CAPS); + renderStudioDeepLink('/studio/', CONTROL_PLANE_APPS); + + await waitFor(() => expect(screen.getByTestId('studio-front-door')).toBeInTheDocument()); + expect(screen.getByRole('link', { name: 'ObjectOS' })).toHaveAttribute( + 'href', + '/apps/cloud_control', + ); + }); + it('NEGATIVE CONTROL: the holder is answered ONCE for the whole subtree', async () => { answerWith(OPERATOR_CAPS); renderStudioDeepLink('/studio/hotcrm/data'); diff --git a/apps/console/src/components/StudioRoute.tsx b/apps/console/src/components/StudioRoute.tsx index 7a31e3c59b..898a638de6 100644 --- a/apps/console/src/components/StudioRoute.tsx +++ b/apps/console/src/components/StudioRoute.tsx @@ -42,6 +42,7 @@ import { LoadingScreen, StudioDesignSurface, getProductName, + useHomePath, } from '@object-ui/app-shell'; import { ProtectedRoute } from './ProtectedRoute'; @@ -56,12 +57,18 @@ import { holdsStudioAccess, useStudioEntry } from './studioEntry'; */ export function RequireStudioAccess({ children, - redirectTo = '/home', + redirectTo, }: { children: ReactNode; - /** Where a non-holder lands. Home, not a dead end — same posture as `RequireAiSurface`. */ + /** + * Where a non-holder lands. Home, not a dead end — same posture as + * `RequireAiSurface`. Defaults to the DECLARED landing (objectui#7373), which + * is the environment launcher wherever no app declares one; an explicit value + * still wins. + */ redirectTo?: string; }) { + const homePath = useHomePath(); const entry = useStudioEntry(); // Loading window. The builder must not mount for a single frame while the @@ -76,7 +83,7 @@ export function RequireStudioAccess({ } if (!holdsStudioAccess(entry.systemPermissions)) { - return ; + return ; } return <>{children}; @@ -98,13 +105,19 @@ export function StudioRoute() { * * Standalone frame — the landing must never be a navigation dead end, so the * wordmark walks back to the platform Home. + * + * Its sibling screen inside the same frame — `StudioDesignSurface`'s header + * Home button — follows the declared landing since objectui#7373, and two + * affordances one route apart must not name two different homes (the very + * defect objectui#7256 measured), so this one reads the same hook. */ function StudioLanding() { + const homePath = useHomePath(); return (
diff --git a/packages/app-shell/src/__tests__/homeAffordancesFollowDeclaration-7256.test.ts b/packages/app-shell/src/__tests__/homeAffordancesFollowDeclaration-7256.test.ts index 1d24d9fc75..074545a23e 100644 --- a/packages/app-shell/src/__tests__/homeAffordancesFollowDeclaration-7256.test.ts +++ b/packages/app-shell/src/__tests__/homeAffordancesFollowDeclaration-7256.test.ts @@ -33,7 +33,9 @@ * redirects in `console/AppContent.tsx` / `console/ConsoleShell.tsx`. Those are * error-recovery paths, not Home affordances, and retargeting them moves a * `/home` expectation that a dozen existing tests pin — a separate change with - * its own measurement. + * its own measurement. That change is objectui#7373, and its scan is the + * sibling file `homeRecoveryRedirectsFollowDeclaration-7373.test.ts`: the + * affordances stay this file's subject, the recovery exits are that one's. * * And it does not cover `apps/console`'s `/` resolver, which keeps its own * reading of the declaration — `landingHomeParity-7256.test.ts` compares the two diff --git a/packages/app-shell/src/__tests__/homeRecoveryRedirectsFollowDeclaration-7373.test.ts b/packages/app-shell/src/__tests__/homeRecoveryRedirectsFollowDeclaration-7373.test.ts new file mode 100644 index 0000000000..97316facd0 --- /dev/null +++ b/packages/app-shell/src/__tests__/homeRecoveryRedirectsFollowDeclaration-7373.test.ts @@ -0,0 +1,179 @@ +/** + * ObjectUI + * Copyright (c) 2024-present ObjectStack Inc. + * + * objectui#7373 — the "this app is gone / you may not be here" RECOVERY exits + * follow the declared landing too, and no longer name `/home` literally. + * + * ## The defect, and why it is a separate card from objectui#7256 + * + * #7256 (PR #7372) moved the four chrome Home AFFORDANCES — top-bar logo, both + * sidebar Home rows, the app-switcher Home entry — onto `useHomePath()`, and + * `homeAffordancesFollowDeclaration-7256.test.ts` pins them. It deliberately + * left the recovery redirects alone, in those words, because retargeting them + * moves a `/home` expectation that existing tests pin — a measurement this card + * did first and ships beside the change. + * + * What was left behind: a control-plane customer refused an app, or landed on a + * surface their runtime does not serve, was dropped on the ENVIRONMENT launcher + * (ADR-0075) — "Build an app" / "Start from a template" cards acting on an + * environment the control plane does not have, and "Your apps" tiles that are + * the control plane's own internal management apps. The declaration was already + * read by `/` and by the chrome; only the error paths still disagreed. + * + * ## ⚠ WHAT THIS FILE CANNOT ASSERT — read before adding a case here + * + * A SOURCE SCAN, in the shape of its #7256 sibling. It proves each site stopped + * naming the launcher and reads the shipped policy; it does not prove where any + * of them lands. Those are behavioural, one per site, and they are what fails on + * the pre-#7373 implementation: + * + * - `console/__tests__/AppContent.deniedVsUnpublished.test.tsx` — the denial + * screen's way back; + * - `console/__tests__/AppContent.inaccessibleAppStrand.test.tsx` — the + * no-app-to-enter bounce; + * - `console/__tests__/RequireAiSurface.test.tsx` — a runtime serving no + * agent; + * - `console/ai/__tests__/resolveCollapseToDockTarget.test.ts` — the dock + * landing on a cold deep link; + * - `views/studio-design/StudioDesignSurface.packageDeletionInference.test.tsx` + * — eviction when the edited package is deleted; + * - `apps/console/src/components/StudioRoute.test.tsx` — the `/studio` entry + * gate and the front door's wordmark. + * + * That the hook ANSWERS correctly is `hooks/__tests__/useHomePath.test.tsx`, and + * that the declaration is read correctly is `utils/__tests__/homePath.test.ts`. + * None of these replaces another. + * + * ## ⛔ Two sites in this file's own subject matter deliberately still name the + * launcher, and this file must not grow a case for either + * + * - `utils/homePath.ts` — `HOME_LAUNCHER_PATH` IS the launcher, and the `??` + * fallback every site above resolves through. Making it anything else is + * option C (redirecting `/home` itself), which the card's ruling excluded: + * it would strip the environment layer of its real launcher (ADR-0075). + * - `console/ConsoleShell.tsx`'s `RootRedirect` — the `/` LANDING, not a + * recovery redirect. `/`'s policy layers an emptiness heuristic + * (objectui#4048) and refuses to conclude from an unresolved list + * (objectui#4233); a third reading without those is a design question, + * raised on the card rather than settled inside it. + */ + +import { describe, it, expect } from 'vitest'; +import { readFileSync } from 'node:fs'; +import path from 'node:path'; +import { fileURLToPath } from 'node:url'; +// @ts-expect-error — plain-JS shared helper, intentionally untyped (`allowJs: false`) +import { maskComments } from '../../../../scripts/js-comment-mask.mjs'; + +/** Local annotation, since the import above is untyped — the call site stays checked. */ +const mask: (source: string) => string = maskComments; + +const here = path.dirname(fileURLToPath(import.meta.url)); +const repoRoot = path.resolve(here, '../../../..'); + +const read = (rel: string) => readFileSync(path.join(repoRoot, rel), 'utf8'); + +/** + * Drop block and line comments before scanning, for the reason the #7256 file + * gives: doc comments legitimately NAME the launcher while explaining which + * screen it is and why a site no longer points there, and a rule that forbids + * documenting its own subject only applies pressure to delete the explanation. + */ +function stripComments(src: string): string { + return mask(src); +} + +/** + * `RootRedirect`'s body, which lives in the same file as `RequireAiSurface` and + * keeps the launcher literal on purpose (see the header, and its own case at + * the bottom of this file). Cut out of the scan so the scan can stay a plain + * "no literal anywhere" over everything else in that file. + * + * If this stops matching — the function renamed or removed — the cut removes + * nothing and the scan gets STRICTER, never quieter. + */ +const ROOT_REDIRECT_BODY = /export function RootRedirect\(\)[\s\S]*?\n\}/; + +/** + * The recovery exits: file → the expression each must resolve its target by, + * and the region (if any) the launcher scan deliberately does not read. + */ +const RECOVERY_SITES: ReadonlyArray<{ + file: string; + site: string; + expression: RegExp; + except?: RegExp; +}> = [ + { + file: 'packages/app-shell/src/console/AppContent.tsx', + site: 'access-denied screen + the no-app-to-enter bounce', + expression: /const homePath = useHomePath\(\)/, + }, + { + file: 'packages/app-shell/src/console/ConsoleShell.tsx', + site: 'RequireAiSurface — a runtime that serves no agent', + expression: /redirectTo \?\? homePath/, + except: ROOT_REDIRECT_BODY, + }, + { + file: 'packages/app-shell/src/console/ai/AiChatPage.tsx', + site: 'the AI page: no-agent screen + collapse-to-dock landing', + expression: /const homePath = useHomePath\(\)/, + }, + { + file: 'packages/app-shell/src/views/studio-design/StudioDesignSurface.tsx', + site: 'Studio: deleted-package eviction + header Home', + expression: /const homePath = useHomePath\(\)/, + }, + { + file: 'apps/console/src/components/StudioRoute.tsx', + site: 'the /studio entry gate + the front door wordmark', + expression: /const homePath = useHomePath\(\)/, + }, +]; + +/** `'/home'`, `"/home"` or `` `/home` `` — the literal in any quoting. */ +const LAUNCHER_LITERAL = /['"`]\/home['"`]/; + +describe('objectui#7373 — recovery redirects follow the declared landing', () => { + it.each(RECOVERY_SITES)('$site does not hard-code the launcher path', ({ file, except }) => { + const src = stripComments(read(file)); + expect(except ? src.replace(except, '') : src).not.toMatch(LAUNCHER_LITERAL); + }); + + it.each(RECOVERY_SITES)('$site resolves its target through the policy', ({ file, expression }) => { + expect(stripComments(read(file))).toMatch(expression); + }); + + it('the scan can still fail — the literal is what it is looking for', () => { + // Without this, a `maskComments` that started returning '' would pass every + // case above as a scan of nothing. Same source, same masker, same regex. + expect(stripComments(`const x = '/home';`)).toMatch(LAUNCHER_LITERAL); + expect(stripComments(read('packages/app-shell/src/utils/homePath.ts'))).toMatch( + LAUNCHER_LITERAL, + ); + }); + + it('⛔ the launcher constant itself is untouched — option C stays excluded', () => { + // `HOME_LAUNCHER_PATH` is the fallback every site above resolves through. + // A card that "fixed" this line would redirect `/home` itself and strip the + // environment layer of its launcher (ADR-0075) — excluded by the ruling. + const src = stripComments(read('packages/app-shell/src/utils/homePath.ts')); + expect(src).toMatch(/export const HOME_LAUNCHER_PATH = '\/home';/); + }); + + it('⛔ the `/` landing keeps its own resolver', () => { + // `RootRedirect` is `/`'s redirect, not a recovery exit, and it may not be + // quietly folded into the hook: `/`'s answer is `resolveLandingPath`, whose + // extra rules this one does not have. Pinned by the comment that SAYS so, + // so deleting the reasoning is a visible act rather than a silent one. + const src = read('packages/app-shell/src/console/ConsoleShell.tsx'); + expect(src).toMatch(/Deliberately NOT retargeted onto `useHomePath\(\)`/); + // …and it is still the launcher it sends `/` to. This is the region the + // scan above cuts out, so without this case that cut would be unwatched. + const body = stripComments(src).match(ROOT_REDIRECT_BODY)?.[0]; + expect(body, 'RootRedirect no longer matches — the scan cut nothing').toBeTruthy(); + expect(body).toMatch(LAUNCHER_LITERAL); + }); +}); diff --git a/packages/app-shell/src/console/AppContent.tsx b/packages/app-shell/src/console/AppContent.tsx index 6dfa63ccce..363d5b8f6c 100644 --- a/packages/app-shell/src/console/AppContent.tsx +++ b/packages/app-shell/src/console/AppContent.tsx @@ -29,6 +29,7 @@ import { } from '../providers/ExpressionProvider.js'; import { buildExpressionUser } from '../providers/expressionUser.js'; import { useTrackRouteAsRecent } from '../hooks/useTrackRouteAsRecent.js'; +import { useHomePath } from '../hooks/useHomePath.js'; import { resolveRecordFormTarget, resolveFormViewLayout, resolveNavigateCreateUrl, resolveNavigateEditUrl, resolvePostCreateTarget } from '../utils/recordFormNavigation.js'; import { deriveRecordSurface, deriveRecordFlowSurface } from '@object-ui/plugin-view'; import { RECORD_FORM_PARAM, RECORD_FORM_OBJECT_PARAM, RECORD_FORM_LINK_PARAM } from '../urlParams.js'; @@ -170,6 +171,12 @@ export function AppContent({ extraRoutes, extraRoutesNoApp }: AppContentProps = const location = useLocation(); const { appName } = useParams(); const { apps, objects: allObjects, loading: metadataLoading, ensureType, error: metadataError, refresh: refreshMetadata } = useMetadata(); + // objectui#7373 — where this file's two "you cannot be here" exits land. Both + // sit BELOW the readiness gate further down (`metadataLoading` &c), so the + // list they resolve against has settled; an app list that failed to load is + // `[]`, which resolves to the launcher — the unchanged status quo, never a + // worse answer than the literal it replaced. + const homePath = useHomePath(); const previewDrafts = usePreviewDrafts(); const { t } = useObjectTranslation(); const { objectLabel } = useObjectLabel(); @@ -693,7 +700,7 @@ export function AppContent({ extraRoutes, extraRoutesNoApp }: AppContentProps = // objectui#5619 — `isWorkspaceAdminResolved` belongs in this readiness gate // for the same reason `metadataLoading` does: everything below branches on // the verdict. The guard at the "no active app" strand turns a `false` into a - // `` that the later flip to `true` cannot undo, + // replacing redirect to home that the later flip to `true` cannot undo, // and the chrome this mounts (ConsoleLayout -> UnifiedSidebar / AppHeader) // reads the same verdict to decide which navigation exists. Waiting for three // of four inputs and acting on the fourth mid-flight is the defect itself. @@ -745,13 +752,17 @@ export function AppContent({ extraRoutes, extraRoutesNoApp }: AppContentProps = // // No Retry here — retrying a permission decision cannot change it, and a // button that promises otherwise is the same misdirection one layer down. - // The way back is `/home` instead, because this screen (like every no-app + // The way back is HOME instead, because this screen (like every no-app // surface in this file) returns ABOVE the single `ConsoleLayout` mount and // so carries no header, no navigation and no workspace switcher — the - // objectui#4473 strand, which a dead end here would recreate. Router-relative - // for the same reason as that fix: ``/`navigate` resolve through - // the host's `basename`, and `/home` is part of the outer skeleton every - // host mounting this component provides (see this file's header). + // objectui#4473 strand, which a dead end here would recreate. Which home is + // the DECLARED one (objectui#7373): a control-plane customer bounced to the + // environment launcher lands among cards that act on an environment their + // deployment does not have. Router-relative for the same reason as the + // #4473 fix: ``/`navigate` resolve through the host's `basename`, + // and both the declared landing and the launcher fallback are part of the + // outer skeleton every host mounting this component provides (see this + // file's header). if (accessVerdict === 'denied') { return (
@@ -766,7 +777,7 @@ export function AppContent({ extraRoutes, extraRoutesNoApp }: AppContentProps = })}
-
@@ -850,16 +861,24 @@ export function AppContent({ extraRoutes, extraRoutesNoApp }: AppContentProps = // WORKSPACE-level fact ("no apps are registered") that a per-user-filtered // list cannot establish, and offers two actions — create an app, open system // settings — that a non-admin cannot perform. For a workspace admin it stays - // the deliberate first-run surface (#3573 / #3590). For everyone else `/home` - // is the honest destination: it renders inside the shell (top bar + workspace + // the deliberate first-run surface (#3573 / #3590). For everyone else HOME is + // the honest destination: it renders inside the shell (top bar + workspace // switcher) and already carries the role-aware copy for this state ("No // applications yet — your workspace is being set up…", `home/HomePage.tsx`). // `replace`, so the strand is not left in history behind them. // + // WHICH home is `homePath` (objectui#7373), not the launcher literal this + // line used to carry: on a deployment that DECLARES a landing the launcher is + // the wrong screen to strand someone on — cloud's control plane has no + // environment for its "Build an app" cards to act on. Where nothing is + // declared `homePath` IS the launcher, so this branch is unchanged for every + // ordinary environment. + // // Router-relative on purpose: `` resolves through the host's // `basename`, so the console's `/_console` mount is preserved without // building a URL by hand (`resolveConsoleUrl` is for full-page navigations - // that leave the router — see `organizations/resolveHomeUrl.ts`). `/home` is + // that leave the router — see `organizations/resolveHomeUrl.ts`). Both the + // declared landing and the launcher fallback are // part of the outer skeleton every host that mounts this component provides // (see this file's header), and `RequireAiSurface` in `ConsoleShell.tsx` // already bounces the same way for a surface this runtime cannot serve. @@ -880,7 +899,7 @@ export function AppContent({ extraRoutes, extraRoutesNoApp }: AppContentProps = // (`LegacyMetadataRedirect`, `ShorthandRecordRedirect`) deliberately do NOT // convert -- they fire INSIDE `ConsoleLayout`, with the console already on // screen, and a splash there would cover a layout that never went away. - return ; + return ; } if (!activeApp && !isCreateAppRoute && !isSystemRoute && !isMetadataRoute) return ( diff --git a/packages/app-shell/src/console/ConsoleShell.tsx b/packages/app-shell/src/console/ConsoleShell.tsx index a8509f201b..dee3dd1921 100644 --- a/packages/app-shell/src/console/ConsoleShell.tsx +++ b/packages/app-shell/src/console/ConsoleShell.tsx @@ -25,6 +25,7 @@ import { withSettleSignal } from '../observability/settleSignal.js'; import { MetadataProvider, useMetadata } from '../providers/MetadataProvider.js'; import { appRouteSegment } from '../utils/appRoute.js'; import { useAiSurfaceEnabled } from '../hooks/useAiSurface.js'; +import { useHomePath } from '../hooks/useHomePath.js'; import { PreviewModeProvider } from '../preview/PreviewModeContext.js'; import { NavigationProvider } from '../context/NavigationContext.js'; import { FavoritesProvider } from '../context/FavoritesProvider.js'; @@ -379,14 +380,21 @@ export function RequireOrganization({ children }: { children: ReactNode }) { * from exactly the entry points that are visible (no shown CTA that bounces back * to home, no hidden FAB to a working route). Waits for the catalog to resolve * before deciding so the redirect never flashes on first paint. + * + * WHICH home it bounces to is the DECLARED landing (objectui#7373), resolved by + * `useHomePath()` — the default is no longer the launcher literal. A caller that + * passes `redirectTo` still wins, and on a deployment that declares no landing + * the hook answers the launcher, so every ordinary environment is unchanged. */ export function RequireAiSurface({ children, - redirectTo = '/home', + redirectTo, }: { children: ReactNode; + /** Overrides the declared-home default. */ redirectTo?: string; }) { + const homePath = useHomePath(); const { enabled, isLoading } = useAiSurfaceEnabled(); if (isLoading) return ; // Splash-preserving handoff (objectui#6507). This is a BOOT-path redirect @@ -396,7 +404,7 @@ export function RequireAiSurface({ // runtime where this branch fires none of them is rendered. What reaches it // is a stale bookmark or an external link — a first navigation, with the // splash still up and no layout underneath. - if (!enabled) return ; + if (!enabled) return ; return <>{children}; } @@ -437,6 +445,16 @@ export function AuthenticatedRoute({ /** * RootRedirect — element for . Waits for metadata to load * then sends the user to /home. + * + * ⛔ Deliberately NOT retargeted onto `useHomePath()` by objectui#7373, which + * moved this file's `RequireAiSurface` bounce. This is not a recovery redirect: + * it is the `/` LANDING, and `/`'s policy is `resolveLandingPath` + * (`apps/console/src/components/RootLandingRedirect.tsx`), which layers a + * single-visible-app emptiness heuristic (objectui#4048) on the same + * declaration and refuses to conclude from an unresolved list (objectui#4233). + * Making this twin read the declaration WITHOUT those two would fork "where + * does `/` go" into a third answer for the consumers that mount it — a design + * question, raised on objectui#7373 rather than settled inside it. */ export function RootRedirect() { const { loading } = useMetadata(); diff --git a/packages/app-shell/src/console/__tests__/AppContent.deniedVsUnpublished.test.tsx b/packages/app-shell/src/console/__tests__/AppContent.deniedVsUnpublished.test.tsx index 3c01f398ad..db58331333 100644 --- a/packages/app-shell/src/console/__tests__/AppContent.deniedVsUnpublished.test.tsx +++ b/packages/app-shell/src/console/__tests__/AppContent.deniedVsUnpublished.test.tsx @@ -532,6 +532,8 @@ describe('AppContent — every screen here states what the probe measured (objec }); it('the denial screen offers a way back to /home', async () => { + // Nothing declared in `metadataApps` here (the `beforeEach` list is a plain + // `crm`), so home IS the launcher — the status quo objectui#7373 kept. byName.finance = () => json(403, DENIED_BODY); renderConsoleAt('/apps/finance'); @@ -540,4 +542,27 @@ describe('AppContent — every screen here states what the probe measured (objec await waitFor(() => expect(screen.getByTestId('pathname').textContent).toBe('/home')); }); + + it('…and that way back follows the DECLARED landing where there is one (objectui#7373)', async () => { + // The card's own case. On cloud's control plane `cloud_control` declares + // the landing, so a customer refused an app they may not open must land + // back on it — not on the environment launcher, whose "Build an app" / + // "Start from a template" cards act on an environment the control plane + // does not have and whose "Your apps" tiles are its own internal management + // apps. The pin fails on the pre-#7373 implementation, which named `/home` + // literally whatever the deployment declared. + metadataApps = [ + { name: 'cloud_control', label: 'Cloud', isDefault: true, navigation: [] }, + { name: 'account', label: 'Account', navigation: [] }, + ]; + byName.finance = () => json(403, DENIED_BODY); + + renderConsoleAt('/apps/finance'); + const home = await screen.findByTestId('app-access-denied-home'); + home.click(); + + await waitFor(() => + expect(screen.getByTestId('pathname').textContent).toBe('/apps/cloud_control'), + ); + }); }); diff --git a/packages/app-shell/src/console/__tests__/AppContent.inaccessibleAppStrand.test.tsx b/packages/app-shell/src/console/__tests__/AppContent.inaccessibleAppStrand.test.tsx index e03995586e..6e5ee98766 100644 --- a/packages/app-shell/src/console/__tests__/AppContent.inaccessibleAppStrand.test.tsx +++ b/packages/app-shell/src/console/__tests__/AppContent.inaccessibleAppStrand.test.tsx @@ -61,7 +61,7 @@ import '@testing-library/jest-dom/vitest'; import { describe, it, expect, vi, beforeEach } from 'vitest'; import React from 'react'; -import { render, screen } from '@testing-library/react'; +import { render, screen, waitFor } from '@testing-library/react'; import { MemoryRouter, Routes, Route, Navigate, useLocation, useNavigationType } from 'react-router-dom'; // --------------------------------------------------------------------------- @@ -270,4 +270,23 @@ describe('AppContent — an inaccessible landing app bounces to /home (objectui# expect(pathname()).toBe('/apps/setup'); expect(screen.queryByTestId('home-launcher')).not.toBeInTheDocument(); }); + + it('bounces to the DECLARED landing where the deployment declares one (objectui#7373)', async () => { + // The bounce and the declaration CAN both be true at once, and this is how: + // the branch fires on `!activeApp`, whose fallback list is + // `launcherApps` — `active !== false` and `hidden !== true` — while + // `resolveDeclaredHomePath` reads the whole list. A landing declared on an + // app that is hidden from the launcher (the shape + // `apps/console/src/components/landingHomeParity-7256.test.ts` already + // carries as its own case) therefore leaves nothing to enter here and a + // declared place to go. + // + // Pre-#7373 this landed on `/home` — for a control-plane customer, the + // environment launcher, which is the screen the card is about. + metadataApps = [{ name: 'cloud_control', label: 'Cloud', isDefault: true, hidden: true, navigation: [] }]; + renderConsoleAt('/apps/setup'); + + await waitFor(() => expect(pathname()).toBe('/apps/cloud_control')); + expect(screen.queryByTestId('home-launcher')).not.toBeInTheDocument(); + }); }); diff --git a/packages/app-shell/src/console/__tests__/RequireAiSurface.test.tsx b/packages/app-shell/src/console/__tests__/RequireAiSurface.test.tsx index 4017850434..f229911971 100644 --- a/packages/app-shell/src/console/__tests__/RequireAiSurface.test.tsx +++ b/packages/app-shell/src/console/__tests__/RequireAiSurface.test.tsx @@ -1,6 +1,8 @@ import { describe, it, expect, vi, beforeEach } from 'vitest'; +import React from 'react'; import { render, screen } from '@testing-library/react'; import { MemoryRouter, Routes, Route } from 'react-router-dom'; +import { MetadataCtx, type MetadataContextValue } from '@object-ui/react'; import { RequireAiSurface } from '../ConsoleShell'; import { useAiSurfaceEnabled } from '../../hooks/useAiSurface'; @@ -11,20 +13,45 @@ vi.mock('../../hooks/useAiSurface', () => ({ })); const mockSurface = vi.mocked(useAiSurfaceEnabled); -function renderGuardedAi() { +/** + * The app list this guard's home target is resolved from (objectui#7373). + * `undefined` renders the guard with no metadata context at all — the shape + * every case here had before that card, and the one a host that mounts the + * guard outside `MetadataProvider` still gets. + */ +function withMetadata( + apps: MetadataContextValue['apps'] | undefined, + children: React.ReactNode, +) { + if (!apps) return <>{children}; + const value: MetadataContextValue = { + apps, + objects: [], dashboards: [], reports: [], pages: [], + loading: false, error: null, + refresh: async () => {}, invalidate: () => {}, ensureType: async () => [], + getItem: async () => null, getItemsByType: () => [], getTypeStatus: () => 'ready', + }; + return {children}; +} + +function renderGuardedAi(apps?: MetadataContextValue['apps']) { return render( - - -
AI CHAT
- - } - /> - HOME
} /> - + {withMetadata( + apps, + + +
AI CHAT
+ + } + /> + HOME
} /> + DECLARED LANDING} /> + , + )} , ); } @@ -53,4 +80,29 @@ describe('RequireAiSurface', () => { expect(screen.queryByText('AI CHAT')).not.toBeInTheDocument(); expect(screen.queryByText('HOME')).not.toBeInTheDocument(); }); + + it('bounces to the DECLARED landing, not the launcher, where one is declared (objectui#7373)', () => { + // The card's case, on this guard: a stale `/ai` bookmark opened against a + // control plane that serves no agent. Pre-#7373 the default was the `/home` + // literal, so the customer landed among environment cards that cannot act + // on anything their deployment has. This pin fails on that implementation. + mockSurface.mockReturnValue({ enabled: false, isLoading: false }); + renderGuardedAi([ + { name: 'cloud_control', label: 'Cloud', isDefault: true }, + { name: 'account', label: 'Account' }, + ]); + expect(screen.getByText('DECLARED LANDING')).toBeInTheDocument(); + expect(screen.queryByText('HOME')).not.toBeInTheDocument(); + }); + + it('keeps the launcher for an ordinary environment that declares nothing', () => { + // The status quo, stated as its own case so the two answers cannot be + // confused for one: an app list WITHOUT a declaration still resolves to the + // launcher, and so does a guard mounted outside a metadata provider (every + // other case in this file). + mockSurface.mockReturnValue({ enabled: false, isLoading: false }); + renderGuardedAi([{ name: 'crm', label: 'CRM' }, { name: 'setup', label: 'Setup' }]); + expect(screen.getByText('HOME')).toBeInTheDocument(); + expect(screen.queryByText('DECLARED LANDING')).not.toBeInTheDocument(); + }); }); diff --git a/packages/app-shell/src/console/ai/AiChatPage.tsx b/packages/app-shell/src/console/ai/AiChatPage.tsx index 5fa6843de4..ec631ccc84 100644 --- a/packages/app-shell/src/console/ai/AiChatPage.tsx +++ b/packages/app-shell/src/console/ai/AiChatPage.tsx @@ -108,6 +108,7 @@ import { emitMetadataRefresh } from '../../assistant/assistantBus.js'; import { getRuntimeConfig, isAiStudioEnabled } from '../../runtime-config.js'; import { makerConvergedOnBuild, makerVisibleAgents } from '../../hooks/surfaceAgent.js'; import { useCanAuthorMetadata } from '../../hooks/useCanAuthorMetadata.js'; +import { useHomePath } from '../../hooks/useHomePath.js'; import { cloudConsoleUrl } from '../marketplace/marketplaceApi.js'; import { useNavigationContext } from '../../context/NavigationContext.js'; import { @@ -682,7 +683,13 @@ export function matchAiChatShortcut(e: { * 2. History back, when react-router has an in-app entry to return to * (`window.history.state.idx > 0` — the router stamps a monotonically * increasing `idx` on entries it creates). - * 3. `/home` — the page was the entry point (deep link, fresh tab). + * 3. `homePath` — the page was the entry point (deep link, fresh tab). + * + * `homePath` is a PARAMETER, not a literal, since objectui#7373: home is + * whatever the deployment declared (`useHomePath()` at the call site), and on a + * control plane the environment launcher is the wrong screen to land a customer + * on. Required rather than defaulted, so a new call site cannot silently + * reintroduce the literal this card removed. * * The dock itself is armed to open expanded separately * ({@link armChatDockExpanded}); this only picks the landing. Pure + exported @@ -690,10 +697,11 @@ export function matchAiChatShortcut(e: { */ export function resolveCollapseToDockTarget( historyIdx: unknown, - storedPath?: string, + storedPath: string | undefined, + homePath: string, ): string | -1 { if (storedPath) return storedPath; - return typeof historyIdx === 'number' && historyIdx > 0 ? -1 : '/home'; + return typeof historyIdx === 'number' && historyIdx > 0 ? -1 : homePath; } /** A composer submission held until the conversation id that will carry it exists. */ @@ -833,6 +841,10 @@ export function AiChatPage({ apiBase: apiBaseProp, defaultAgent: defaultAgentPro const handoffParentConversationId = searchParams.get('parentConversationId')?.trim() || undefined; const navigate = useNavigate(); + // objectui#7373 — both exits out of this page (the "no agent here" screen's + // Home button, and the collapse-to-dock landing on a cold deep link) follow + // the DECLARED landing. Undeclared deployments get the launcher, unchanged. + const homePath = useHomePath(); const { setContext } = useNavigationContext(); useEffect(() => { @@ -1258,7 +1270,7 @@ export function AiChatPage({ apiBase: apiBaseProp, defaultAgent: defaultAgentPro surface back into the dock. Arms the dock to mount expanded, then returns to the exact page the user maximized from (remembered by the dock's own maximize handlers; falls back to history-back, then - /home on a cold deep link) — the dock resolves the same + the declared home on a cold deep link) — the dock resolves the same (user, product) conversation scope, so it shows THE SAME THREAD. Visible on mobile too: under `md` the dock presents as a bottom sheet. */} @@ -1275,6 +1287,7 @@ export function AiChatPage({ apiBase: apiBaseProp, defaultAgent: defaultAgentPro const target = resolveCollapseToDockTarget( (window.history.state as { idx?: unknown } | null)?.idx, readDockReturnLocation(), + homePath, ); if (target === -1) navigate(-1); else navigate(target); @@ -1288,7 +1301,7 @@ export function AiChatPage({ apiBase: apiBaseProp, defaultAgent: defaultAgentPro navigate('/home')} + onHome={() => navigate(homePath)} t={t} /> ) : ( diff --git a/packages/app-shell/src/console/ai/__tests__/resolveCollapseToDockTarget.test.ts b/packages/app-shell/src/console/ai/__tests__/resolveCollapseToDockTarget.test.ts index e01fbcceba..8e1ae88731 100644 --- a/packages/app-shell/src/console/ai/__tests__/resolveCollapseToDockTarget.test.ts +++ b/packages/app-shell/src/console/ai/__tests__/resolveCollapseToDockTarget.test.ts @@ -4,34 +4,56 @@ * * ADR-0057 P3c — the `/ai` page's "collapse to dock" landing: the remembered * maximize origin wins, else history back when react-router has an in-app - * entry to return to, else /home (deep link / fresh tab — nothing behind us + * entry to return to, else home (deep link / fresh tab — nothing behind us * worth going "back" to). + * + * objectui#7373 made that last rung a PARAMETER. The page passes + * `useHomePath()`, so a deployment that declares a landing gets its own home + * here, and one that declares none gets `HOME_LAUNCHER_PATH` — which is why + * both spellings appear below: the declared answer is what the card changed, + * the launcher answer is the status quo it must not disturb. */ import { describe, it, expect } from 'vitest'; import { resolveCollapseToDockTarget } from '../AiChatPage'; +import { HOME_LAUNCHER_PATH } from '../../../utils/homePath'; + +/** A control plane's declared landing — what `useHomePath()` answers there. */ +const DECLARED = '/apps/cloud_control'; describe('resolveCollapseToDockTarget', () => { it('prefers the remembered maximize origin over history', () => { - expect(resolveCollapseToDockTarget(3, '/apps/crm/objects/deal')).toBe('/apps/crm/objects/deal'); + expect(resolveCollapseToDockTarget(3, '/apps/crm/objects/deal', DECLARED)).toBe( + '/apps/crm/objects/deal', + ); // Even a deep-linked page (idx 0) returns to the stored origin. - expect(resolveCollapseToDockTarget(0, '/studio/com.example/interfaces')).toBe( + expect(resolveCollapseToDockTarget(0, '/studio/com.example/interfaces', DECLARED)).toBe( '/studio/com.example/interfaces', ); }); it('goes back when react-router stamped a positive history index', () => { - expect(resolveCollapseToDockTarget(1)).toBe(-1); - expect(resolveCollapseToDockTarget(7, undefined)).toBe(-1); + expect(resolveCollapseToDockTarget(1, undefined, DECLARED)).toBe(-1); + expect(resolveCollapseToDockTarget(7, undefined, HOME_LAUNCHER_PATH)).toBe(-1); + }); + + it('lands on the DECLARED home when this page is the entry point (idx 0)', () => { + // objectui#7373: the case the card is about. A control-plane customer who + // deep-linked into `/ai` must not be tipped out into the environment + // launcher, whose cards act on an environment they do not have. + expect(resolveCollapseToDockTarget(0, undefined, DECLARED)).toBe(DECLARED); }); - it('lands on /home when this page is the entry point (idx 0)', () => { - expect(resolveCollapseToDockTarget(0)).toBe('/home'); + it('lands on the declared home when the index is missing or not a number', () => { + expect(resolveCollapseToDockTarget(undefined, undefined, DECLARED)).toBe(DECLARED); + expect(resolveCollapseToDockTarget(null, undefined, DECLARED)).toBe(DECLARED); + expect(resolveCollapseToDockTarget('2', undefined, DECLARED)).toBe(DECLARED); + expect(resolveCollapseToDockTarget(NaN, undefined, DECLARED)).toBe(DECLARED); }); - it('lands on /home when the index is missing or not a number', () => { - expect(resolveCollapseToDockTarget(undefined)).toBe('/home'); - expect(resolveCollapseToDockTarget(null)).toBe('/home'); - expect(resolveCollapseToDockTarget('2')).toBe('/home'); - expect(resolveCollapseToDockTarget(NaN)).toBe('/home'); + it('keeps the launcher where the deployment declares no landing — the status quo', () => { + // `useHomePath()` answers `HOME_LAUNCHER_PATH` for every ordinary + // environment, so this is the unchanged behaviour of every rung above. + expect(resolveCollapseToDockTarget(0, undefined, HOME_LAUNCHER_PATH)).toBe('/home'); + expect(resolveCollapseToDockTarget(NaN, undefined, HOME_LAUNCHER_PATH)).toBe('/home'); }); }); diff --git a/packages/app-shell/src/console/organizations/manage/AcceptInvitationPage.tsx b/packages/app-shell/src/console/organizations/manage/AcceptInvitationPage.tsx index c3821ef095..ffff91973e 100644 --- a/packages/app-shell/src/console/organizations/manage/AcceptInvitationPage.tsx +++ b/packages/app-shell/src/console/organizations/manage/AcceptInvitationPage.tsx @@ -89,6 +89,19 @@ export function AcceptInvitationPage() { await acceptInvitation(invitationId); await switchOrganization(invitation.organizationId).catch(() => null); toast.success(t('organization.accept.accepted', { defaultValue: 'Invitation accepted' })); + // ⛔ objectui#7373 retargeted this file's SIBLING recovery redirects onto + // the declared landing (`useHomePath()`) and deliberately did NOT touch + // this one. The reading, recorded on that card: the app list in hand here + // belongs to the organization the user is LEAVING. `switchOrganization` + // above has just resolved, `MetadataProvider` drops its cache on an org + // change (objectui#4486) and refetches, and this line runs before any of + // that can land — so a declared-landing answer read here would name the + // PREVIOUS org's app. The two other org-switch paths + // (`layout/WorkspaceSwitcher.tsx`, `console/organizations/ + // OrganizationsPage.tsx`) full-page-navigate to the console ROOT for + // exactly this reason and let `RootLandingRedirect` resolve the landing + // afterwards. Which of those two shapes this page should take is a + // decision, not an implementation detail. navigate('/home'); } catch (err) { // objectui#4474 — the card's site 5: a wrong recipient produced better-auth's diff --git a/packages/app-shell/src/views/studio-design/StudioDesignSurface.packageDeletionInference.test.tsx b/packages/app-shell/src/views/studio-design/StudioDesignSurface.packageDeletionInference.test.tsx index 9671d48257..1ba6ee12c2 100644 --- a/packages/app-shell/src/views/studio-design/StudioDesignSurface.packageDeletionInference.test.tsx +++ b/packages/app-shell/src/views/studio-design/StudioDesignSurface.packageDeletionInference.test.tsx @@ -41,7 +41,9 @@ import '@testing-library/jest-dom/vitest'; import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; import { cleanup, fireEvent, render, screen, waitFor } from '@testing-library/react'; +import React from 'react'; import { MemoryRouter, Route, Routes, useLocation } from 'react-router-dom'; +import { MetadataCtx, type MetadataContextValue } from '@object-ui/react'; const PACKAGE_ID = 'app.b2r4'; const SIBLING_ID = 'app.other'; @@ -184,14 +186,38 @@ function LocationProbe() { return
{useLocation().pathname}
; } -function renderSurface() { +/** + * The app list the eviction's destination resolves against (objectui#7373). + * `undefined` mounts the surface with no metadata context — every case written + * before that card, for which `useHomePath()` answers the launcher. + */ +function withMetadata( + apps: MetadataContextValue['apps'] | undefined, + children: React.ReactNode, +) { + if (!apps) return <>{children}; + const value: MetadataContextValue = { + apps, + objects: [], dashboards: [], reports: [], pages: [], + loading: false, error: null, + refresh: async () => {}, invalidate: () => {}, ensureType: async () => [], + getItem: async () => null, getItemsByType: () => [], getTypeStatus: () => 'ready', + }; + return {children}; +} + +function renderSurface(apps?: MetadataContextValue['apps']) { return render( - - } /> - } /> - + {withMetadata( + apps, + + } /> + } /> + } /> + , + )} , ); } @@ -205,9 +231,12 @@ const where = () => screen.getByTestId('location').textContent; * button. Every case shares this drive, so the ONLY difference between the * pins below is what the refresh does. */ -async function openLifecycleSheet(initial = [row(PACKAGE_ID)]): Promise { +async function openLifecycleSheet( + initial = [row(PACKAGE_ID)], + apps?: MetadataContextValue['apps'], +): Promise { fetchPackagesMock.mockResolvedValue(initial); - renderSurface(); + renderSurface(apps); await waitFor(() => expect(trigger()).toHaveAttribute('data-pkg-list-state', 'loaded')); fireEvent.click(trigger()); fireEvent.click(await screen.findByText('Package info & settings')); @@ -276,6 +305,24 @@ describe('Studio package lifecycle — a failed refresh is not a deletion (#7821 expect(toastError).not.toHaveBeenCalled(); }); + it('REAL deletion, nothing left, a DECLARED landing: evicts to it, not to the launcher (objectui#7373)', async () => { + // Same eviction as the case above — the only difference is that this + // deployment declares where home is. Pre-#7373 the destination was the + // `/home` literal either way, which on a control plane drops the author + // into the environment launcher. + const lifecycle = await openLifecycleSheet([row(PACKAGE_ID)], [ + { name: 'cloud_control', label: 'Cloud', isDefault: true }, + { name: 'account', label: 'Account' }, + ]); + + fetchPackagesMock.mockResolvedValue([]); + fireEvent.click(lifecycle); + + expect(await screen.findByTestId('declared-landing')).toBeInTheDocument(); + expect(where()).toBe('/apps/cloud_control'); + expect(toastError).not.toHaveBeenCalled(); + }); + it('REAL deletion, a sibling survives: still navigates to that sibling (behaviour unchanged)', async () => { const lifecycle = await openLifecycleSheet([row(PACKAGE_ID), row(SIBLING_ID)]); diff --git a/packages/app-shell/src/views/studio-design/StudioDesignSurface.tsx b/packages/app-shell/src/views/studio-design/StudioDesignSurface.tsx index 624e3b1bd6..1ea3a21216 100644 --- a/packages/app-shell/src/views/studio-design/StudioDesignSurface.tsx +++ b/packages/app-shell/src/views/studio-design/StudioDesignSurface.tsx @@ -108,6 +108,7 @@ import { emitMetadataRefresh, subscribeMetadataRefresh } from '../../assistant/a import { formatMetadataError, formatPublishFailures, type PublishFailure } from './metadataError.js'; import { loadPackageSurfaces } from './packageSurfaces.js'; import { useMetadataRefreshNonce } from './useMetadataRefreshNonce.js'; +import { useHomePath } from '../../hooks/useHomePath.js'; import { resolveSurface, findSurfaceInTree, type NavNode, type Surface } from './navSurface.js'; import { useSurfaceDeepLink, resolveSurfaceDeepLink, type SurfaceTarget } from './useSurfaceDeepLink.js'; import { SurfaceDeepLinkProvider, useRequestedSurface } from './surfaceDeepLinkChannel.js'; @@ -278,6 +279,10 @@ function PackageSwitcher({ }): React.ReactElement { const navigate = useNavigate(); const locale = useMetadataLocale(); + // objectui#7373 — where the deleted-package eviction below lands when no + // other package is left to open: the DECLARED landing, the launcher only when + // the deployment declares none. + const homePath = useHomePath(); const [open, setOpen] = React.useState(false); const [pkgs, setPkgs] = React.useState(null); /** @@ -549,7 +554,7 @@ function PackageSwitcher({ // Deleted — only navigate away if it was the package we're editing. if (managedId === packageId) { const next = list[0]; - navigate(next ? `/studio/${encodeURIComponent(next.id)}/${tab}` : '/home'); + navigate(next ? `/studio/${encodeURIComponent(next.id)}/${tab}` : homePath); } return; } @@ -628,7 +633,7 @@ function PackageSwitcher({ ); setManageOpen(false); } - }, [manage, packageId, tab, navigate, fetchFullPackage, locale]); + }, [manage, packageId, tab, navigate, fetchFullPackage, locale, homePath]); return ( // Radix Popover (portaled to ) — the top bar is `overflow-x-auto`, @@ -917,6 +922,9 @@ export function StudioDesignSurface({ aiSlot }: StudioDesignSurfaceProps): React // ships an app, offer 打开应用 — opened in a new tab so the builder context // survives. (App → builder is the reverse bridge, tracked separately.) const shellNavigate = useNavigate(); + // objectui#7373 — the header's Home button walks back to the DECLARED + // landing; the environment launcher only where nothing is declared. + const shellHomePath = useHomePath(); const shellClient = useMetadataClient(); const [packageApp, setPackageApp] = React.useState<{ name: string; label: string } | null>(null); // 创建应用 (package has no app yet): create a draft `app` item — the published @@ -1062,7 +1070,7 @@ export function StudioDesignSurface({ aiSlot }: StudioDesignSurfaceProps): React type="button" onClick={() => { if (!confirmLeavePillar()) return; - shellNavigate('/home'); + shellNavigate(shellHomePath); }} title={t('engine.studio.home', locale)} className="shrink-0 rounded-md p-1 text-muted-foreground hover:bg-muted hover:text-foreground" From 22b2f88849fa4603fd641f2ab9ed447edad6f8fa Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 19 Sep 2026 20:04:25 +0000 Subject: [PATCH 2/2] fix(app-shell): accepting an invitation lands on the console root (objectui#7373) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The held site from this branch's first round, now ruled: the seat took option A after verifying the basis on `origin/main` rather than taking the report's word for it. `handleAccept` ran `navigate('/home')` in-router, immediately after `switchOrganization` resolved. That is why it could not simply read `useHomePath()` like this card's nine other sites: the app list in memory at that instant belongs to the organization the user is LEAVING. `switchOrganization` only updates auth state, `MetadataProvider` drops its whole cache when the active org changes (objectui#4486) and refetches, and this line runs before any of that can land — so a declared-landing answer read here would name the PREVIOUS org's app, one the new organization may not even carry. Worse than the launcher it would have replaced. Landing on the console ROOT resolves the declaration AFTER the switch instead of before it: the full page load re-seats every data scope on the new organization, and `RootLandingRedirect` then reads that org's list through `resolveLandingPath`. So the page honours `app.isDefault` for the organization the user has just joined — which is what this card asked for — by the only route that can read it. This is not a new mechanism. `layout/WorkspaceSwitcher.tsx` (`handleSwitch`) and `console/organizations/OrganizationsPage.tsx` (`handleSelect`) both do `window.location.href = resolveRootUrl()` after the same call, and both say why in a comment. Accepting an invitation is the third site of one transition and was the only one not taking it. `resolveRootUrl()` rather than a bare `'/'` because a full-page navigation leaves React Router, so nothing applies the host's `basename`. The reading is written at the call site, not left in a thread. The pin that asserted "lands on /home" is rewritten rather than loosened: it now captures the full-page navigation, asks the RESOLVED target (the way a browser resolves it, so the embeddable build's relative `'./'` stays legal) whether it is the console root, and asserts the launcher route — still declared in the fixture on purpose — was not reached. The source scan gains this file keyed on `resolveRootUrl`, not on the hook, so a later edit that "unifies" this onto `useHomePath()` fails there instead of passing quietly. Co-Authored-By: Claude Claude-Session: https://claude.ai/code/session_018HrVaotisyhgmot9o2MLRq --- ...overy-redirects-follow-declared-landing.md | 14 +++- .../__tests__/AcceptInvitationRoute.test.tsx | 76 ++++++++++++++++++- ...eryRedirectsFollowDeclaration-7373.test.ts | 29 ++++++- .../manage/AcceptInvitationPage.tsx | 46 +++++++---- 4 files changed, 145 insertions(+), 20 deletions(-) diff --git a/.changeset/7373-recovery-redirects-follow-declared-landing.md b/.changeset/7373-recovery-redirects-follow-declared-landing.md index cbdd2901f1..ae4c26abce 100644 --- a/.changeset/7373-recovery-redirects-follow-declared-landing.md +++ b/.changeset/7373-recovery-redirects-follow-declared-landing.md @@ -25,12 +25,20 @@ Retargeted onto the existing policy — no new one was written: and the header's Home button; - `apps/console`'s `/studio` entry gate and the Studio front door's wordmark. +Accepting an organization invitation follows the declaration too, by a different +route and for a measured reason: it runs immediately after `switchOrganization`, +so the app list in hand there still belongs to the organization being LEFT +(`MetadataProvider` drops its cache on an org change and refetches after that line +has run). Reading the declaration in place would name the previous organization's +app. It now reloads onto the console ROOT — the shape `WorkspaceSwitcher` and +`OrganizationsPage` already use for this same transition — so +`RootLandingRedirect` resolves the landing for the organization the user has just +joined. + Every ordinary environment is unchanged: where nothing declares a landing, and wherever the app list is not (yet) an answer, the resolved path IS `/home`. Two sites deliberately keep the launcher: `HOME_LAUNCHER_PATH` itself (it is the launcher, and the fallback all of the above resolve through — ADR-0075), and `RootRedirect`, which is `/`'s landing rather than a recovery exit and has its own -resolver. `AcceptInvitationPage` is unchanged pending a decision recorded on the -card: it navigates immediately after an organization switch, where the app list in -hand still belongs to the organization being left. +resolver. diff --git a/apps/console/src/pages/auth/__tests__/AcceptInvitationRoute.test.tsx b/apps/console/src/pages/auth/__tests__/AcceptInvitationRoute.test.tsx index 9696dbae0f..728d226967 100644 --- a/apps/console/src/pages/auth/__tests__/AcceptInvitationRoute.test.tsx +++ b/apps/console/src/pages/auth/__tests__/AcceptInvitationRoute.test.tsx @@ -95,6 +95,41 @@ function Recorder() { return null; } +/** + * Records the FULL-PAGE navigations the page performs (objectui#7373). + * + * The accept path leaves React Router deliberately — see the comment at that + * call site — so "where did it go" cannot be read off `seen`, which only sees + * in-router transitions. Assigning `window.location.href` for real would make + * the environment try to navigate, so the accessor is swapped for the duration + * of each case and restored exactly as it was found (own property or + * prototype accessor, whichever it was). + */ +let navigations: string[] = []; +let ownHref: PropertyDescriptor | undefined; + +function captureFullPageNavigations() { + navigations = []; + ownHref = Object.getOwnPropertyDescriptor(window.location, 'href'); + const inherited = Object.getOwnPropertyDescriptor( + Object.getPrototypeOf(window.location), + 'href', + ); + Object.defineProperty(window.location, 'href', { + configurable: true, + get: () => (ownHref?.get ?? inherited?.get)?.call(window.location) ?? '', + set: (value: string) => { + navigations.push(String(value)); + }, + }); +} + +function releaseFullPageNavigations() { + if (ownHref) Object.defineProperty(window.location, 'href', ownHref); + else delete (window.location as unknown as Record).href; + ownHref = undefined; +} + /** * Mount the page exactly as `apps/console/src/App.tsx` does: inside the * console's `BrowserRouter`, on the real path, with NO layout wrapper. @@ -110,6 +145,10 @@ function renderRoute({ basename = '/', lang = 'en' }: { basename?: string; lang? } /> } /> + {/* ⛔ Keep this route declared even though nothing should land on it: + the accept case asserts the sentinel is ABSENT, and a negative + assertion against a route that does not exist passes for the + wrong reason (objectui#7373). */} } /> } /> @@ -121,6 +160,7 @@ function renderRoute({ basename = '/', lang = 'en' }: { basename?: string; lang? beforeEach(() => { vi.clearAllMocks(); seen.length = 0; + captureFullPageNavigations(); window.localStorage.clear(); authState = { isAuthenticated: true, @@ -133,6 +173,7 @@ beforeEach(() => { }); afterEach(() => { + releaseFullPageNavigations(); window.history.pushState({}, '', '/'); }); @@ -196,7 +237,16 @@ describe('objectui#3811 — console routes DefaultAcceptInvitationPage', () => { ).toBeInTheDocument(); }); - it('accept switches the user into the invited organization, then lands on /home', async () => { + it('accept switches the user into the invited organization, then reloads onto the console ROOT', async () => { + // objectui#7373 moved this landing. It used to be an in-router + // `navigate('/home')` — the environment launcher, reached without leaving + // the SPA, which is exactly why it could not honour the declaration: the + // app list in memory at that instant is the org the user just LEFT + // (`MetadataProvider` drops its cache on an org change, objectui#4486, + // and refetches after this line has already run). Landing on the root + // lets `RootLandingRedirect` resolve `app.isDefault` for the org the user + // just JOINED, which is the shape `WorkspaceSwitcher.handleSwitch` and + // `OrganizationsPage.handleSelect` already take for the same transition. const user = userEvent.setup(); renderRoute(); await screen.findByTestId('accept-invitation-page'); @@ -209,7 +259,29 @@ describe('objectui#3811 — console routes DefaultAcceptInvitationPage', () => { expect((authState.acceptInvitation as ReturnType).mock.invocationCallOrder[0]).toBeLessThan( (authState.switchOrganization as ReturnType).mock.invocationCallOrder[0], ); - await screen.findByTestId('home-sentinel'); + + // A full page load, not a router transition — the whole point is that + // every data scope is re-seated on the new organization before anything + // reads the app list. + await waitFor(() => expect(navigations).toHaveLength(1)); + // Asked of the RESOLVED target, the way the browser resolves it, rather + // than by string equality with the call — the shipped embeddable build's + // correct answer is a relative `'./'` (`utils/consoleBase.test.ts`), and + // a pin on the literal would forbid it while proving nothing. + const landed = new URL(navigations[0], document.baseURI); + expect(landed.pathname, 'accept must land on the console root').toBe('/'); + // …and specifically NOT on the launcher this replaced. Stated separately + // because that is the regression with a name: a root that is `/home` is + // the defect objectui#7373 was filed about, wearing a page load. + expect(landed.pathname).not.toBe('/home'); + expect(navigations[0]).not.toContain('/home'); + + // The router must NOT have handled it. `seen` records every in-router + // transition, so a `navigate()` regression shows up here even if a page + // load were also performed. + expect(screen.queryByTestId('home-sentinel')).not.toBeInTheDocument(); + expect(seen).not.toContain('/home'); + expect(toastSuccess).toHaveBeenCalledWith('Invitation accepted'); }); diff --git a/packages/app-shell/src/__tests__/homeRecoveryRedirectsFollowDeclaration-7373.test.ts b/packages/app-shell/src/__tests__/homeRecoveryRedirectsFollowDeclaration-7373.test.ts index 97316facd0..79e68b4fe9 100644 --- a/packages/app-shell/src/__tests__/homeRecoveryRedirectsFollowDeclaration-7373.test.ts +++ b/packages/app-shell/src/__tests__/homeRecoveryRedirectsFollowDeclaration-7373.test.ts @@ -39,12 +39,28 @@ * - `views/studio-design/StudioDesignSurface.packageDeletionInference.test.tsx` * — eviction when the edited package is deleted; * - `apps/console/src/components/StudioRoute.test.tsx` — the `/studio` entry - * gate and the front door's wordmark. + * gate and the front door's wordmark; + * - `apps/console/src/pages/auth/__tests__/AcceptInvitationRoute.test.tsx` — + * accepting an invitation, which lands on the console ROOT rather than + * reading the hook (see below). * * That the hook ANSWERS correctly is `hooks/__tests__/useHomePath.test.tsx`, and * that the declaration is read correctly is `utils/__tests__/homePath.test.ts`. * None of these replaces another. * + * ## ⭐ One site follows the declaration WITHOUT the hook, on purpose + * + * `console/organizations/manage/AcceptInvitationPage.tsx` runs immediately + * after `switchOrganization`, so the app list it could read still belongs to + * the organization the user is LEAVING (`MetadataProvider` drops its cache on + * an org change, objectui#4486, and refetches after that line has run). Reading + * the hook there would name the PREVIOUS org's app. It lands on the console + * root instead and lets `RootLandingRedirect` resolve the declaration for the + * NEW org — the shape `layout/WorkspaceSwitcher.tsx` and + * `console/organizations/OrganizationsPage.tsx` already take for this same + * transition. Its row below is keyed on THAT expression, so a later edit that + * folds it onto `useHomePath()` fails here rather than passing quietly. + * * ## ⛔ Two sites in this file's own subject matter deliberately still name the * launcher, and this file must not grow a case for either * @@ -131,6 +147,17 @@ const RECOVERY_SITES: ReadonlyArray<{ site: 'the /studio entry gate + the front door wordmark', expression: /const homePath = useHomePath\(\)/, }, + { + file: 'packages/app-shell/src/console/organizations/manage/AcceptInvitationPage.tsx', + site: 'accepting an invitation — the org-switch landing', + // ⭐ The ONE site here that may not read `useHomePath()`, and the expression + // says which policy it reads instead. Its app list belongs to the org being + // LEFT, so it resolves the declaration by landing on the console root and + // letting `RootLandingRedirect` read the new org's list — the shape both + // other org-switch paths take. A future edit that "unified" this onto the + // hook would pass a scan keyed on the hook; this one refuses it. + expression: /window\.location\.href = resolveRootUrl\(\)/, + }, ]; /** `'/home'`, `"/home"` or `` `/home` `` — the literal in any quoting. */ diff --git a/packages/app-shell/src/console/organizations/manage/AcceptInvitationPage.tsx b/packages/app-shell/src/console/organizations/manage/AcceptInvitationPage.tsx index ffff91973e..b018830961 100644 --- a/packages/app-shell/src/console/organizations/manage/AcceptInvitationPage.tsx +++ b/packages/app-shell/src/console/organizations/manage/AcceptInvitationPage.tsx @@ -19,6 +19,7 @@ import { Loader2, Building2, CheckCircle, XCircle } from 'lucide-react'; import { toast } from 'sonner'; import { resolveOrgRoleLabel } from '../orgRoleLabel.js'; import { resolveOrgErrorMessage } from '../orgErrorMessage.js'; +import { resolveRootUrl } from '../resolveHomeUrl.js'; type InvitationWithOrg = AuthInvitation & { organizationName?: string; @@ -89,20 +90,37 @@ export function AcceptInvitationPage() { await acceptInvitation(invitationId); await switchOrganization(invitation.organizationId).catch(() => null); toast.success(t('organization.accept.accepted', { defaultValue: 'Invitation accepted' })); - // ⛔ objectui#7373 retargeted this file's SIBLING recovery redirects onto - // the declared landing (`useHomePath()`) and deliberately did NOT touch - // this one. The reading, recorded on that card: the app list in hand here - // belongs to the organization the user is LEAVING. `switchOrganization` - // above has just resolved, `MetadataProvider` drops its cache on an org - // change (objectui#4486) and refetches, and this line runs before any of - // that can land — so a declared-landing answer read here would name the - // PREVIOUS org's app. The two other org-switch paths - // (`layout/WorkspaceSwitcher.tsx`, `console/organizations/ - // OrganizationsPage.tsx`) full-page-navigate to the console ROOT for - // exactly this reason and let `RootLandingRedirect` resolve the landing - // afterwards. Which of those two shapes this page should take is a - // decision, not an implementation detail. - navigate('/home'); + // The console ROOT, as a full-page navigation — NOT a router `navigate()` + // and NOT `useHomePath()` (objectui#7373). + // + // ⭐ WHY THE HOOK IS WRONG *HERE*, while it is right at this card's nine + // other sites: the app list this component can read belongs to the + // organization the user is LEAVING. `switchOrganization` one line up only + // updates auth state; `MetadataProvider` drops its whole cache when the + // active org changes (objectui#4486) and refetches, and this line runs + // before any of that can land. A declared-landing answer read here would + // therefore name the PREVIOUS org's app — an app the new organization may + // not even carry — which is worse than the launcher this replaced. + // + // Landing on the ROOT resolves the declaration AFTER the switch instead + // of before it: the full page load re-seats every data scope on the new + // org, then `RootLandingRedirect` reads the new org's list and applies + // `resolveLandingPath` — the declared landing, a single-app workspace's + // one app, or the launcher. So this page honours `app.isDefault` for the + // organization the user just joined, which is what objectui#7373 asked + // for, by the only route that can read it. + // + // This is the shape both other org-switch paths already take, for this + // same reason, and their comments say so: `layout/WorkspaceSwitcher.tsx` + // (`handleSwitch`) and `console/organizations/OrganizationsPage.tsx` + // (`handleSelect`). Accepting an invitation is the third site of one + // transition; it was the only one not taking it. + // + // `resolveRootUrl()` and not a bare `'/'`: a full-page navigation leaves + // React Router, so nothing applies the host's `basename`, and a console + // served under `` would drop the user at the + // origin root, outside the SPA. + window.location.href = resolveRootUrl(); } catch (err) { // objectui#4474 — the card's site 5: a wrong recipient produced better-auth's // English sentence under the translated title. Mapped by `code` now.