diff --git a/.changeset/26183-one-app-address.md b/.changeset/26183-one-app-address.md new file mode 100644 index 00000000000..ee13e674da3 --- /dev/null +++ b/.changeset/26183-one-app-address.md @@ -0,0 +1,16 @@ +--- +'@object-ui/app-shell': patch +'@object-ui/console': patch +--- + +fix(app-shell,console): one app opens under one address — a name-alias route moves onto the package id + +Clause-②: no +Metadata-protocol: none + +An app with a package id answered under two address families in one session. ADR-0048 keys `/apps/` on the package id, and every sidebar entry builds on it, but the app's `name` still resolves as an alias, and a page opened on the alias built every link of its own (list rows, record pages, related lists) on the alias. Landing on `/apps/clm/…` therefore opened contracts under the name and obligations and the approvals inbox under `/apps/app.objectstack.hotclm/…`. + +- **`@object-ui/app-shell`:** a route under `/apps//…` whose app carries a package id is rewritten, with `replace`, to the same path under `/apps//…`, keeping the query string, hash and history state. Old name-based bookmarks and shared links keep opening, now on the one address. The top-bar logo and every Home affordance (`resolveDeclaredHomePath`) name the declared app by its package id. An app without a package id, an app whose name is its package id, and every app after the first of a package that ships several (the package id opens the first) keep their name as their address. +- **`@object-ui/console`:** the `/` landing (`resolveLandingPath`) names the landing app by the same address, so post-login and the logo agree and no redirect hop follows. + +What to do: nothing. Links that name an app by its name keep working; they now arrive on its package-id address. diff --git a/ui/apps/console/src/components/RootLandingRedirect.test.ts b/ui/apps/console/src/components/RootLandingRedirect.test.ts index 643b976a660..6fdcbbf63f6 100644 --- a/ui/apps/console/src/components/RootLandingRedirect.test.ts +++ b/ui/apps/console/src/components/RootLandingRedirect.test.ts @@ -97,13 +97,25 @@ describe('resolveLandingPath', () => { // The declared landing is the escape hatch: an admin-console deployment // that genuinely wants Setup as its first screen says so with // `isDefault`, and rule 1 is deliberately untouched by this fix. - expect(resolveLandingPath([{ ...SETUP, isDefault: true }])).toBe('/apps/setup'); + // On Setup's package id, its one address (objectstack-ai/objectstack#26183). + expect(resolveLandingPath([{ ...SETUP, isDefault: true }])).toBe('/apps/com.objectstack.setup'); }); it('CONTROL: a one-app PRODUCT deployment still lands in its app (#2027 rule 2 intact)', () => { expect(resolveLandingPath([{ name: 'crm' }])).toBe('/apps/crm'); }); + it('lands on the package id, never the name alias (objectstack-ai/objectstack#26183)', () => { + // The name alias left the whole session on `/apps//…` while the + // sidebar built every entry on the package id. + expect(resolveLandingPath([{ name: 'clm', _packageId: 'app.objectstack.hotclm' }])).toBe( + '/apps/app.objectstack.hotclm', + ); + expect( + resolveLandingPath([{ name: 'clm', _packageId: 'app.objectstack.hotclm', isDefault: true }, SETUP]), + ).toBe('/apps/app.objectstack.hotclm'); + }); + it('CONTROL: Setup alongside a product app is unchanged — this fix only reads the SINGLE-app case', () => { // Every real deployment ships Setup. Excluding it from the *count* would // silently re-route `[crm, setup]` from /home into /apps/crm — a far @@ -122,7 +134,7 @@ describe('resolveLandingPath', () => { // not. Widening this to every platform app is a separate judgment call // that this card does not authorize. expect(resolveLandingPath([{ name: 'studio', _packageId: 'com.objectstack.studio' }])).toBe( - '/apps/studio', + '/apps/com.objectstack.studio', ); }); }); diff --git a/ui/apps/console/src/components/RootLandingRedirect.tsx b/ui/apps/console/src/components/RootLandingRedirect.tsx index 174b92dd9f1..487e397a08f 100644 --- a/ui/apps/console/src/components/RootLandingRedirect.tsx +++ b/ui/apps/console/src/components/RootLandingRedirect.tsx @@ -65,6 +65,24 @@ function isPlatformSetupApp(app: LandingApp): boolean { return app._packageId === SETUP_APP_PACKAGE_ID || app.name === SETUP_APP_NAME; } +/** + * The ONE segment that addresses `app` among `apps` (ADR-0048 option A): its + * package id whenever that resolves back to this very app, else its name — a + * package shipping several apps leaves the name as the only address that + * reaches the later ones. The same answer as app-shell's `appAddressSegment`, + * spelled out here for the reason rule 1 below gives for not importing the + * chrome's policy; `landingHomeParity-7256.test.ts` holds the two equal. + * + * Landing on the name used to leave the whole session on the name alias while + * the sidebar built every link on the package id (objectstack-ai/objectstack#26183). + */ +function landingSegment(apps: readonly LandingApp[], app: LandingApp & { name: string }): string { + const pkg = app._packageId; + if (!pkg) return app.name; + const resolved = apps.find((a) => a?._packageId === pkg) ?? apps.find((a) => a?.name === pkg); + return resolved === app ? pkg : app.name; +} + /** * The path `/` should redirect to, resolved purely from the App list. Extracted * from the component so the policy is unit-testable without a router/render. @@ -84,7 +102,7 @@ export function resolveLandingPath(apps: readonly LandingApp[] | null | undefine // chrome-free hand-rolled mock, so importing the policy from there would // make the fixture, not the shipped function, the thing under test. const defaultApp = list.find((a) => a.isDefault === true); - if (defaultApp) return `/apps/${defaultApp.name}`; + if (defaultApp) return `/apps/${landingSegment(apps ?? [], defaultApp)}`; // 2. A single-app deployment lands straight in that App (no one-tile // launcher) — but Setup alone is NOT a one-app deployment. @@ -111,7 +129,7 @@ export function resolveLandingPath(apps: readonly LandingApp[] | null | undefine // deployment from `/home` into the product app — a far larger change than // this card asks for, and one nobody measured. const visible = list.filter((a) => a.active !== false && a.hidden !== true); - if (visible.length === 1 && !isPlatformSetupApp(visible[0])) return `/apps/${visible[0].name}`; + if (visible.length === 1 && !isPlatformSetupApp(visible[0])) return `/apps/${landingSegment(apps ?? [], visible[0])}`; // 3. Multi-app default: the workspace launcher. return '/home'; diff --git a/ui/apps/console/src/components/landingHomeParity-7256.test.ts b/ui/apps/console/src/components/landingHomeParity-7256.test.ts index 1b94f456afd..a3c113c347d 100644 --- a/ui/apps/console/src/components/landingHomeParity-7256.test.ts +++ b/ui/apps/console/src/components/landingHomeParity-7256.test.ts @@ -44,6 +44,8 @@ const LISTS: ReadonlyArray<{ label: string; apps: any[] }> = [ { label: 'ordinary multi-app environment, nothing declared', apps: [{ name: 'crm' }, { name: 'ops' }, SETUP] }, { label: 'one product app + Setup, nothing declared', apps: [{ name: 'crm' }, SETUP] }, { label: 'Setup only — a fresh environment', apps: [SETUP] }, + { label: 'declared packaged app (objectstack-ai/objectstack#26183)', apps: [{ name: 'clm', _packageId: 'app.objectstack.hotclm', isDefault: true }, SETUP] }, + { label: 'declared second app of a two-app package', apps: [{ name: 'crm', _packageId: 'com.acme.suite' }, { name: 'ops', _packageId: 'com.acme.suite', isDefault: true }, SETUP] }, { label: 'empty list', apps: [] }, ]; @@ -78,6 +80,21 @@ describe('objectui#7256 — one declaration, one home', () => { expect(resolveLandingPath(apps)).toBe('/apps/alpha'); }); + it('DRIFT CANARY: both land on the package id, the address the sidebar builds (objectstack-ai/objectstack#26183)', () => { + const apps = [{ name: 'clm', _packageId: 'app.objectstack.hotclm', isDefault: true }, SETUP]; + expect(resolveDeclaredHomePath(apps)).toBe('/apps/app.objectstack.hotclm'); + expect(resolveLandingPath(apps)).toBe('/apps/app.objectstack.hotclm'); + }); + + it('DRIFT CANARY: a package id another app answers to is not this app\'s address', () => { + const apps = [ + { name: 'crm', _packageId: 'com.acme.suite' }, + { name: 'ops', _packageId: 'com.acme.suite', isDefault: true }, + ]; + expect(resolveDeclaredHomePath(apps)).toBe('/apps/ops'); + expect(resolveLandingPath(apps)).toBe('/apps/ops'); + }); + it('DRIFT CANARY: a nameless declaration is a declaration to neither', () => { const apps = [{ isDefault: true }, { name: 'crm' }, SETUP]; expect(resolveDeclaredHomePath(apps)).toBeNull(); diff --git a/ui/packages/app-shell/src/console/AppContent.tsx b/ui/packages/app-shell/src/console/AppContent.tsx index 66183e3229a..a6fbdb59482 100644 --- a/ui/packages/app-shell/src/console/AppContent.tsx +++ b/ui/packages/app-shell/src/console/AppContent.tsx @@ -37,7 +37,7 @@ import { useSignedInUserLocale } from '../hooks/useUserLocale.js'; import { resolveRecordFormTarget, resolveRecordFormModalLayout, resolveNavigateCreateUrl, resolveNavigateEditUrl, resolvePostCreateTarget, recordFormCreateValues, readRecordFormCreateValues, RECORD_FORM_CREATE_VALUES_STATE } 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'; -import { appRouteSegment, matchAppBySegment } from '../utils/appRoute.js'; +import { appAddressSegment, appRouteSegment, canonicalAppPathname, matchAppBySegment } from '../utils/appRoute.js'; import { resolveHref, type NavTemplateContext } from '@object-ui/layout'; import type { NavigationItem } from '@object-ui/types'; @@ -376,11 +376,28 @@ export function AppContent({ extraRoutes, extraRoutesNoApp }: AppContentProps = : undefined); // ADR-0048 (A) — the segment every `/apps/…` path this component builds is - // keyed on: the URL's own (`appName` — the package id, or the name alias the - // user followed), else the resolved app's route segment. Never `activeApp.name` - // raw: on a package-id route that is a second address for the same app, and - // the recent-items tracker below matches it against the URL (objectui#11818). - const appSegment = appName ?? appRouteSegment(activeApp); + // keyed on: the matched app's one address (`appAddressSegment` — its package + // id), else the URL's own, else the resolved app's route segment. Never + // `activeApp.name` raw: on a package-id route that is a second address for + // the same app, and the recent-items tracker below matches it against the URL + // (objectui#11818). + const appSegment = appAddressSegment(apps, matchedApp) ?? appName ?? appRouteSegment(activeApp); + + // objectstack-ai/objectstack#26183 — a URL that reached the app through its + // NAME alias moves to the app's one address, path, search, hash and history + // state kept. The alias still opens (old bookmarks and shared links keep + // working), but a page left on it built every link on the alias while the + // sidebar built on the package id: one app, two address families. `replace`, + // so Back never returns to the alias. An effect rather than a `` + // return, so a layout already on screen is not unmounted for the swap. + const canonicalPathname = canonicalAppPathname(apps, appName, location.pathname); + useEffect(() => { + if (!canonicalPathname) return; + navigate(`${canonicalPathname}${location.search}${location.hash}`, { + replace: true, + state: location.state, + }); + }, [canonicalPathname, location.search, location.hash, location.state, navigate]); // objectui#12809 — the lazy metadata buckets the CURRENT route reads at // render (`appRouteMetadataReads.ts` declares them per route). This used to diff --git a/ui/packages/app-shell/src/console/__tests__/AppContent.emptyNavigationLanding-12079.test.tsx b/ui/packages/app-shell/src/console/__tests__/AppContent.emptyNavigationLanding-12079.test.tsx index e05f1f1f7cc..6ec820247f8 100644 --- a/ui/packages/app-shell/src/console/__tests__/AppContent.emptyNavigationLanding-12079.test.tsx +++ b/ui/packages/app-shell/src/console/__tests__/AppContent.emptyNavigationLanding-12079.test.tsx @@ -28,7 +28,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, useLocation } from 'react-router-dom'; // AGENTS.md §测试纪律 — the lazy page modules are stubbed so no unbounded @@ -160,8 +160,9 @@ describe('AppContent — an app with empty navigation lands on an empty state (o const emptyState = await screen.findByTestId('app-no-access-empty-state'); expect(emptyState).toHaveTextContent('Nothing in HotCLM is available to you yet'); - // The app root, not a redirect somewhere else. - expect(screen.getByTestId('pathname').textContent).toBe('/apps/clm'); + // The app root, not a redirect somewhere else — on the app's package id, + // its one address (objectstack-ai/objectstack#26183). + await waitFor(() => expect(screen.getByTestId('pathname').textContent).toBe('/apps/app.objectstack.hotclm')); expect(screen.getByTestId('console-layout')).toHaveAttribute('data-active-app', 'clm'); }); @@ -215,7 +216,9 @@ describe('AppContent — an app with empty navigation lands on an empty state (o renderConsoleAt('/apps/clm'); expect(await screen.findByTestId('object-view')).toBeInTheDocument(); - expect(screen.getByTestId('pathname').textContent).toBe('/apps/clm/contract'); + await waitFor(() => + expect(screen.getByTestId('pathname').textContent).toBe('/apps/app.objectstack.hotclm/contract'), + ); expect(screen.queryByTestId('app-no-access-empty-state')).not.toBeInTheDocument(); }); diff --git a/ui/packages/app-shell/src/console/__tests__/AppContent.nameAliasRedirect-26183.test.tsx b/ui/packages/app-shell/src/console/__tests__/AppContent.nameAliasRedirect-26183.test.tsx new file mode 100644 index 00000000000..56eb8fa7866 --- /dev/null +++ b/ui/packages/app-shell/src/console/__tests__/AppContent.nameAliasRedirect-26183.test.tsx @@ -0,0 +1,194 @@ +/** + * ObjectUI + * Copyright (c) 2024-present ObjectStack Inc. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + */ + +/** + * objectstack-ai/objectstack#26183 — one app, one address family. + * + * ADR-0048 option A keys `/apps/` on the package id, and every + * sidebar entry builds on it. The app NAME still resolves (old bookmarks and + * links), but a page rendered on it kept the name in every link it built, so a + * session that landed on `/apps/clm/…` opened contracts under the name and + * obligations and the approvals inbox under the package id. `AppContent` now + * moves a name-alias route onto the package id with `replace`, keeping path, + * search, hash and history state. + * + * Measured from a real `AppContent` mount (the `AppContent.recentOnPackageRoute + * -11818` harness); the router's location is the observation point. + * + * Direction, written before the run: on the pre-fix wiring the alias case is + * RED (the URL stays on the name); the package-id route, the name-equals- + * package-id app and the shared-package app are GREEN on both sides. + */ + +import '@testing-library/jest-dom/vitest'; +import { describe, it, expect, vi, afterEach } from 'vitest'; +import React, { useEffect } from 'react'; +import { render, cleanup, waitFor, screen } from '@testing-library/react'; +import { MemoryRouter, Routes, Route, useLocation, useNavigationType } from 'react-router-dom'; +import { createI18n, I18nProvider } from '@object-ui/i18n'; + +// The route's recent-items tracker runs; this file does not observe it. +vi.mock('../../context/RecentItemsProvider', async (importOriginal) => ({ + ...(await importOriginal>()), + useRecentItems: () => ({ recentItems: [], addRecentItem: () => {}, clearRecentItems: () => {} }), +})); + +vi.mock('@object-ui/plugin-form', async (importOriginal) => ({ + ...(await importOriginal>()), + ModalForm: () => null, +})); + +vi.mock('@object-ui/plugin-designer', async (importOriginal) => ({ + ...(await importOriginal()), + CreateAppPage: () =>
, + EditAppPage: () =>
, + DashboardDesignPage: () =>
, +})); + +vi.mock('../../layout/ConsoleLayout', () => ({ + ConsoleLayout: ({ children }: { children?: React.ReactNode }) => ( +
{children}
+ ), +})); +vi.mock('../../chrome/CommandPalette', () => ({ CommandPalette: () => null })); +vi.mock('../../chrome/KeyboardShortcutsDialog', () => ({ KeyboardShortcutsDialog: () => null })); +vi.mock('../../chrome/OnboardingWalkthrough', () => ({ OnboardingWalkthrough: () => null })); +vi.mock('../../views/ObjectView', () => ({ ObjectView: () =>
})); +vi.mock('../../views/RecordDetailView', () => ({ RecordDetailView: () => null })); + +vi.mock('@object-ui/auth', async (importOriginal) => ({ + ...(await importOriginal>()), + useAuth: () => ({ + user: { id: 'u1', name: 'Ada', email: 'ada@example.com', role: 'user' }, + getAuthConfig: async () => ({ features: {} }), + activeOrganization: null, + }), + useWorkspaceAdminStatus: () => ({ isAdmin: true, isResolved: true }), +})); + +const dataSourceStub = { + onConnectionStateChange: () => () => {}, + getConnectionState: () => 'connected', +}; +vi.mock('../../providers/AdapterProvider', async (importOriginal) => ({ + ...(await importOriginal>()), + useAdapter: () => dataSourceStub, +})); + +/** The showcase app as the catalog serves it: a name AND a package id. */ +const SHOWCASE = { + name: 'showcase_app', + label: 'Showcase', + _packageId: 'com.example.showcase', + isDefault: true, + navigation: [{ id: 'tasks', type: 'object', objectName: 'showcase_task', label: 'Tasks' }], +}; + +const OBJECTS = [{ name: 'showcase_task', label: 'Task', pluralLabel: 'Tasks', fields: { title: { type: 'text' } } }]; + +/** The catalog a test serves. One value per test: the route's effects key on its members. */ +const { metadataRef } = vi.hoisted(() => ({ metadataRef: { current: null as unknown } })); +function serve(apps: unknown[]): void { + metadataRef.current = { + apps, + objects: OBJECTS, + loading: false, + ensureType: undefined, + error: null, + refresh: async () => {}, + }; +} +vi.mock('../../providers/MetadataProvider', async (importOriginal) => ({ + ...(await importOriginal>()), + useMetadata: () => metadataRef.current, +})); + +const ACTION_RUNNER = { execute: () => {}, runner: { registerHandler: () => {}, getContext: () => ({}) } }; +vi.mock('@object-ui/react', async (importOriginal) => ({ + ...(await importOriginal>()), + useActionRunner: () => ACTION_RUNNER, + useGlobalUndo: () => {}, + useMutationInvalidationBridge: () => {}, +})); + +import { AppContent } from '../AppContent'; + +/** The router's location as the last render saw it. */ +const seen: { pathname?: string; search?: string; hash?: string; state?: unknown; type?: string } = {}; +function LocationProbe() { + const location = useLocation(); + const type = useNavigationType(); + useEffect(() => { + Object.assign(seen, { + pathname: location.pathname, + search: location.search, + hash: location.hash, + state: location.state, + type, + }); + }, [location, type]); + return null; +} + +afterEach(cleanup); + +/** Mount the console's app router at `entry`; the object list page renders. */ +type Entry = NonNullable['initialEntries']>[number]; +async function visit(entry: Entry): Promise { + const i18n = createI18n({ defaultLanguage: 'en', detectBrowserLanguage: false }); + render( + + + + } /> + + + + , + ); + await screen.findByTestId('object-view', undefined, { timeout: 5000 }); +} + +describe('AppContent moves a name-alias route onto the app\'s package id (objectstack-ai/objectstack#26183)', () => { + it('the name alias opens, and lands on the package id with search, hash and state kept', async () => { + serve([SHOWCASE]); + const state = { from: { pathname: '/apps/com.example.showcase', label: 'Home' } }; + await visit({ pathname: '/apps/showcase_app/showcase_task', search: '?view=all', hash: '#top', state }); + await waitFor(() => expect(seen.pathname).toBe('/apps/com.example.showcase/showcase_task')); + expect(seen.search).toBe('?view=all'); + expect(seen.hash).toBe('#top'); + expect(seen.state).toEqual(state); + // `replace`: Back never returns to the alias. + expect(seen.type).toBe('REPLACE'); + }); + + it('CONTROL: the package-id route is left where it is', async () => { + serve([SHOWCASE]); + await visit('/apps/com.example.showcase/showcase_task?view=all'); + expect(seen.pathname).toBe('/apps/com.example.showcase/showcase_task'); + expect(seen.search).toBe('?view=all'); + expect(seen.type).toBe('POP'); + }); + + it('CONTROL: an app whose name IS its package id does not loop', async () => { + serve([{ ...SHOWCASE, name: 'showcase', _packageId: 'showcase' }]); + await visit('/apps/showcase/showcase_task'); + expect(seen.pathname).toBe('/apps/showcase/showcase_task'); + expect(seen.type).toBe('POP'); + }); + + it('CONTROL: the second app of a shared package stays on its name — the package id opens the first', async () => { + serve([ + { ...SHOWCASE, name: 'first_app', isDefault: false }, + { ...SHOWCASE, name: 'showcase_app' }, + ]); + await visit('/apps/showcase_app/showcase_task'); + expect(seen.pathname).toBe('/apps/showcase_app/showcase_task'); + expect(seen.type).toBe('POP'); + }); +}); diff --git a/ui/packages/app-shell/src/console/__tests__/AppContent.pageRouteObjects-12980.test.tsx b/ui/packages/app-shell/src/console/__tests__/AppContent.pageRouteObjects-12980.test.tsx index c76540b8191..8e463ecfdc3 100644 --- a/ui/packages/app-shell/src/console/__tests__/AppContent.pageRouteObjects-12980.test.tsx +++ b/ui/packages/app-shell/src/console/__tests__/AppContent.pageRouteObjects-12980.test.tsx @@ -255,7 +255,7 @@ afterEach(() => { describe('a page route reads the objects its page binds, never the whole registry (objectui#12980)', () => { it('(a) asks the app package and, by name, only the bound object outside it', async () => { - boot('/apps/crm/page/case_board'); + boot(`/apps/${CRM}/page/case_board`); await waitFor(() => expect(screen.getByTestId('page-blocks')).toBeInTheDocument()); await waitFor(() => expect(screen.getByTestId('cloud-only-entry')).toHaveTextContent('false')); @@ -271,7 +271,7 @@ describe('a page route reads the objects its page binds, never the whole registr it('(a) a page that binds no object reads none, in an app whose package holds no objects of its own', async () => { server.apps = [{ name: 'crm', label: 'Control plane', _packageId: 'app.platform', navigation: [] }]; - boot('/apps/crm/page/pricing'); + boot('/apps/app.platform/page/pricing'); await waitFor(() => expect(screen.getByTestId('page-blocks')).toBeInTheDocument()); await waitFor(() => expect(objectCalls()).toContain('object/sys_user')); @@ -280,7 +280,7 @@ describe('a page route reads the objects its page binds, never the whole registr }); it('(b) CONTROL: an app route keeps its scoped read (objectui#12811)', async () => { - boot('/apps/crm/crm_account'); + boot(`/apps/${CRM}/crm_account`); await waitFor(() => expect(screen.getByTestId('object-view')).toHaveTextContent('sys_user')); await waitFor(() => expect(screen.getByTestId('cloud-only-entry')).toHaveTextContent('false')); @@ -289,7 +289,7 @@ describe('a page route reads the objects its page binds, never the whole registr it('(c) an object a block asks about outside the read set is read by name, and its link appears when it lands', async () => { server.holdByName = true; - boot('/apps/crm/page/case_board'); + boot(`/apps/${CRM}/page/case_board`); await waitFor(() => expect(server.calls).toContain('object/crm_case')); act(() => server.release()); await waitFor(() => expect(screen.getByTestId('page-blocks')).toBeInTheDocument()); @@ -298,7 +298,8 @@ describe('a page route reads the objects its page binds, never the whole registr act(() => server.release()); await waitFor(() => - expect(screen.getByTestId('page-blocks').getAttribute('data-lookup-href')).toBe('/apps/crm/sys_user/record/u1'), + // Booted on the package id, the app's one address (objectstack-ai/objectstack#26183). + expect(screen.getByTestId('page-blocks').getAttribute('data-lookup-href')).toBe('/apps/app.crm/sys_user/record/u1'), ); expect(objectCalls()).not.toContain('object'); }); diff --git a/ui/packages/app-shell/src/console/__tests__/AppContent.pageScopeArms-12980.test.tsx b/ui/packages/app-shell/src/console/__tests__/AppContent.pageScopeArms-12980.test.tsx index 1e56d4f1a60..2e7b1cba175 100644 --- a/ui/packages/app-shell/src/console/__tests__/AppContent.pageScopeArms-12980.test.tsx +++ b/ui/packages/app-shell/src/console/__tests__/AppContent.pageScopeArms-12980.test.tsx @@ -256,13 +256,15 @@ afterEach(() => { describe('a page bound through the missed object arms reads them in the route scope (objectui#12980)', () => { it('(a) every arm object is known on the page\'s first render, read by name beside the package list, never the whole registry', async () => { - boot('/apps/crm/page/service_board'); + boot(`/apps/${CRM}/page/service_board`); await waitFor(() => expect(screen.getByTestId('page-blocks')).toBeInTheDocument()); // Known on the first render: the route held the page until they answered, // so no block's ask had to start an on-demand read. expect(probe.first).toEqual( - Object.fromEntries(probe.armObjects.map((name) => [name, `/apps/crm/${name}/record/r1`])), + // Booted on the package id, the app's one address: a name-alias route + // moves there after its first render (objectstack-ai/objectstack#26183). + Object.fromEntries(probe.armObjects.map((name) => [name, `/apps/app.crm/${name}/record/r1`])), ); expect(objectCalls()).not.toContain('object'); expect(objectCalls()[0]).toBe(`object?package=${CRM}`); diff --git a/ui/packages/app-shell/src/console/__tests__/AppContent.recentOnPackageRoute-11818.test.tsx b/ui/packages/app-shell/src/console/__tests__/AppContent.recentOnPackageRoute-11818.test.tsx index eac35a83eef..bc48b0d37aa 100644 --- a/ui/packages/app-shell/src/console/__tests__/AppContent.recentOnPackageRoute-11818.test.tsx +++ b/ui/packages/app-shell/src/console/__tests__/AppContent.recentOnPackageRoute-11818.test.tsx @@ -157,9 +157,11 @@ describe('AppContent records a visit on the package-id route as a recent item (o expect(objectEntries()[0].href).toBe('/apps/com.example.showcase/showcase_task'); }); - it('CONTROL: a visit through the name alias is recorded on the alias it used', async () => { + it('CONTROL: a visit through the name alias is recorded once, on the package-id address it moves to', async () => { + // objectstack-ai/objectstack#26183 — the alias route is rewritten onto the + // package id, so the visit is never recorded on a second address. await visit('/apps/showcase_app/showcase_task'); await waitFor(() => expect(objectEntries()).toHaveLength(1)); - expect(objectEntries()[0].href).toBe('/apps/showcase_app/showcase_task'); + expect(objectEntries()[0].href).toBe('/apps/com.example.showcase/showcase_task'); }); }); diff --git a/ui/packages/app-shell/src/utils/__tests__/appRoute.test.ts b/ui/packages/app-shell/src/utils/__tests__/appRoute.test.ts index 6f1d641836b..a52f2e757ca 100644 --- a/ui/packages/app-shell/src/utils/__tests__/appRoute.test.ts +++ b/ui/packages/app-shell/src/utils/__tests__/appRoute.test.ts @@ -1,5 +1,5 @@ import { describe, it, expect } from 'vitest'; -import { appRouteSegment, matchAppBySegment, filterActiveApps, resolveHostAppSegment, appStudioDesignPath, appStudioSurfacePath, appStudioObjectPath, appStudioRoutePath } from '../appRoute'; +import { appRouteSegment, matchAppBySegment, appAddressSegment, canonicalAppPathname, filterActiveApps, resolveHostAppSegment, appStudioDesignPath, appStudioSurfacePath, appStudioObjectPath, appStudioRoutePath } from '../appRoute'; /** * ADR-0048 (option A) — package-id route segment. @@ -45,6 +45,73 @@ describe('matchAppBySegment', () => { * consume this are pinned end-to-end in `HomePage.inboxLinksTarget.test.tsx` and * `InboxPopover.viewAllTarget.test.tsx`; these cases pin the resolution itself. */ +/** + * objectstack-ai/objectstack#26183 — one app, one address. A route that reached + * its app through the name alias is rewritten onto the package id, so the + * page's own links and the sidebar's agree. + */ +describe('appAddressSegment', () => { + const CLM = { name: 'clm', _packageId: 'app.objectstack.hotclm' }; + + it('answers the package id when it resolves back to the same app', () => { + expect(appAddressSegment([CLM], CLM)).toBe('app.objectstack.hotclm'); + }); + + it('answers the name for a package-less (runtime/DB) app', () => { + const app = { name: 'scratch' }; + expect(appAddressSegment([app], app)).toBe('scratch'); + }); + + it('answers the name when the package id opens a DIFFERENT app of the same package', () => { + const crm = { name: 'crm', _packageId: 'com.acme.suite' }; + const ops = { name: 'ops', _packageId: 'com.acme.suite' }; + expect(appAddressSegment([crm, ops], crm)).toBe('com.acme.suite'); + expect(appAddressSegment([crm, ops], ops)).toBe('ops'); + }); + + it('answers undefined for no app', () => { + expect(appAddressSegment([CLM], undefined)).toBeUndefined(); + }); +}); + +describe('canonicalAppPathname', () => { + const CLM = { name: 'clm', _packageId: 'app.objectstack.hotclm' }; + + it('moves a name-alias route onto the package id, the rest of the path kept', () => { + expect(canonicalAppPathname([CLM], 'clm', '/apps/clm/clm_contract/view/my_contracts')).toBe( + '/apps/app.objectstack.hotclm/clm_contract/view/my_contracts', + ); + expect(canonicalAppPathname([CLM], 'clm', '/apps/clm')).toBe('/apps/app.objectstack.hotclm'); + expect(canonicalAppPathname([CLM], 'clm', '/apps/clm/')).toBe('/apps/app.objectstack.hotclm/'); + }); + + it('leaves the package-id route alone — the rewrite can never loop', () => { + const once = canonicalAppPathname([CLM], 'clm', '/apps/clm/clm_contract'); + expect(once).toBe('/apps/app.objectstack.hotclm/clm_contract'); + expect(canonicalAppPathname([CLM], 'app.objectstack.hotclm', once!)).toBeNull(); + }); + + it('leaves an app whose name IS its package id alone', () => { + const app = { name: 'crm', _packageId: 'crm' }; + expect(canonicalAppPathname([app], 'crm', '/apps/crm/account')).toBeNull(); + }); + + it('leaves a package-less app on its name, and the later app of a shared package on its name', () => { + expect(canonicalAppPathname([{ name: 'scratch' }], 'scratch', '/apps/scratch/x')).toBeNull(); + const crm = { name: 'crm', _packageId: 'com.acme.suite' }; + const ops = { name: 'ops', _packageId: 'com.acme.suite' }; + expect(canonicalAppPathname([crm, ops], 'ops', '/apps/ops/ticket')).toBeNull(); + expect(canonicalAppPathname([crm, ops], 'crm', '/apps/crm/account')).toBe('/apps/com.acme.suite/account'); + }); + + it('answers null for a segment no app answers to, or a pathname outside /apps', () => { + expect(canonicalAppPathname([CLM], 'nope', '/apps/nope/x')).toBeNull(); + expect(canonicalAppPathname([CLM], undefined, '/apps')).toBeNull(); + expect(canonicalAppPathname([CLM], 'clm', '/home')).toBeNull(); + expect(canonicalAppPathname(undefined, 'clm', '/apps/clm')).toBeNull(); + }); +}); + describe('filterActiveApps', () => { it('keeps apps that declare neither flag (the common case)', () => { expect(filterActiveApps([{ name: 'crm' }])).toEqual([{ name: 'crm' }]); diff --git a/ui/packages/app-shell/src/utils/__tests__/homePath.test.ts b/ui/packages/app-shell/src/utils/__tests__/homePath.test.ts index 37cc246d8c4..cd3b4de1465 100644 --- a/ui/packages/app-shell/src/utils/__tests__/homePath.test.ts +++ b/ui/packages/app-shell/src/utils/__tests__/homePath.test.ts @@ -79,6 +79,20 @@ describe('resolveDeclaredHomePath', () => { ).toBeNull(); }); + it('names the package id, the address the sidebar builds (objectstack-ai/objectstack#26183)', () => { + expect( + resolveDeclaredHomePath([{ name: 'clm', _packageId: 'app.objectstack.hotclm', isDefault: true }]), + ).toBe('/apps/app.objectstack.hotclm'); + // A package id the package's FIRST app answers to is not the second app's + // address: the name stays its only one. + expect( + resolveDeclaredHomePath([ + { name: 'crm', _packageId: 'com.acme.suite' }, + { name: 'ops', _packageId: 'com.acme.suite', isDefault: true }, + ]), + ).toBe('/apps/ops'); + }); + it('keeps the launcher path spelled once', () => { expect(HOME_LAUNCHER_PATH).toBe('/home'); }); diff --git a/ui/packages/app-shell/src/utils/appRoute.ts b/ui/packages/app-shell/src/utils/appRoute.ts index 63217d82d14..6a3287ffd8c 100644 --- a/ui/packages/app-shell/src/utils/appRoute.ts +++ b/ui/packages/app-shell/src/utils/appRoute.ts @@ -57,6 +57,62 @@ export function matchAppBySegment( return apps.find((a) => a?._packageId === seg) ?? apps.find((a) => a?.name === seg); } +/** + * The ONE segment that addresses `app` among `apps` — the address every + * `/apps/…` link to it should carry. + * + * That is its canonical {@link appRouteSegment} (the package id) whenever the + * segment resolves back to this very app. It does not when a package ships + * more than one app: they share the package id, `matchAppBySegment` answers the + * first, and the name stays the only address that reaches the others — so the + * name is the answer there, never a package id that opens a different app. + */ +export function appAddressSegment( + apps: readonly T[] | null | undefined, + app: T | null | undefined, +): string | undefined { + const canonical = appRouteSegment(app); + if (canonical === undefined || matchAppBySegment(apps, canonical) === app) return canonical; + return typeof app?.name === 'string' ? app.name : canonical; +} + +/** + * Where a route that reached its app through the name ALIAS belongs: the same + * pathname with the app segment swapped for {@link appAddressSegment}, or + * `null` when the route already carries it (or names no app here). + * + * The name alias keeps every older name-based URL and bookmark resolving, but + * a page rendered on it kept the alias in every link it built while the + * sidebar built on the package id, so one app answered under two address + * families (objectstack-ai/objectstack#26183). Rewriting the alias to the one + * address is what keeps both: an old URL still opens, and it opens on the + * address every link inside the app agrees on. + * + * Only the app segment changes; the caller carries search and hash across. The + * answer can never loop — it is a segment that resolves back to the same app, + * so the rewritten route answers `null`. + */ +export function canonicalAppPathname( + apps: readonly AppRouteLike[] | null | undefined, + segment: string | null | undefined, + pathname: string, +): string | null { + const app = matchAppBySegment(apps, segment); + const address = appAddressSegment(apps, app); + if (!address || address === segment) return null; + const parts = pathname.split('/'); + if (parts[1] !== 'apps' || parts[2] === undefined) return null; + let current: string; + try { + current = decodeURIComponent(parts[2]); + } catch { + return null; + } + if (current !== segment) return null; + parts[2] = encodeURIComponent(address); + return parts.join('/'); +} + /** * The console's own administration app. Only ever a LAST RESORT for the * resolution below — historically it was hardcoded as the FIRST choice, which diff --git a/ui/packages/app-shell/src/utils/homePath.ts b/ui/packages/app-shell/src/utils/homePath.ts index a10aacd2e0f..42a9d2dddb3 100644 --- a/ui/packages/app-shell/src/utils/homePath.ts +++ b/ui/packages/app-shell/src/utils/homePath.ts @@ -44,6 +44,8 @@ * @module */ +import { appAddressSegment } from './appRoute.js'; + /** * The multi-app workspace launcher — the environment layer's own home * (ObjectUI ADR-0075). The fallback whenever no App declares a landing, which is every @@ -51,12 +53,18 @@ */ export const HOME_LAUNCHER_PATH = '/home'; -/** Minimal shape this resolver needs off each App metadata record. */ -export interface DeclaredHomeApp { +/** + * Minimal shape this resolver needs off each App metadata record. A type alias, + * not an interface, so it carries the implicit index signature the route + * helpers in `./appRoute` read apps through. + */ +export type DeclaredHomeApp = { name?: string; /** ADR-0049: `isDefault` ROUTES (it used to be a display-only badge). */ isDefault?: boolean; -} + /** ADR-0048 owning package — the app's canonical route segment. */ + _packageId?: string; +}; /** * The path the product DECLARED as its landing, or `null` when it declared @@ -68,13 +76,16 @@ export interface DeclaredHomeApp { * app open" into a second place and drift the moment the app's nav is * re-ordered. * - * The segment is the app's `name` — byte-identical to what `/` has been - * redirecting to, so the logo and the post-login landing cannot disagree. - * (`matchAppBySegment()` accepts the name as a per-tenant alias.) + * The segment is the app's one address (`appAddressSegment()` — its package + * id, ADR-0048 option A), byte-identical to what `/` redirects to, so the logo + * and the post-login landing cannot disagree. It used to be the `name`, which + * `AppContent` now rewrites to the package id on arrival + * (objectstack-ai/objectstack#26183): naming the address directly saves that + * hop. */ export function resolveDeclaredHomePath( apps: readonly DeclaredHomeApp[] | null | undefined, ): string | null { const declared = (apps ?? []).find((a) => a?.isDefault === true && Boolean(a?.name)); - return declared ? `/apps/${declared.name}` : null; + return declared ? `/apps/${appAddressSegment(apps, declared)}` : null; }