Repository navigation
fix(app-shell,console): the chrome's Home affordances follow the declared landing - #7372
Merged
Merged
Conversation
…ared landing The top-bar logo, both sidebar Home rows and the app-switcher Home entry each hard-coded `/home` — the environment layer's launcher. On cloud's control plane the post-login landing is the app declared with `isDefault` (`cloud_control` → its Welcome page), so one click on the logo left the guided welcome for a screen 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 the control plane's own internal management apps. `resolveDeclaredHomePath` is now the one reader of `app.isDefault`; `/`'s resolver layers its emptiness heuristic on it, and `useHomePath()` gives the chrome the same answer. No hostname sniff and no product name in the shared bundle.
`/`'s routing tests replace `@object-ui/app-shell` wholesale with a chrome-free hand-rolled mock, so importing `resolveDeclaredHomePath` there put the fixture under test rather than the shipped policy. The resolver keeps its own reading of the declaration; a matrix comparing both shipped answers is what now forbids drift. Adds the changeset.
Contributor
✅ Console Performance Budget
The eager closure is every chunk the entry reaches through static imports — what the browser fetches and parses before the app renders. The entry chunk on its own is a small fraction of it. 📦 Bundle Size Report
Size Limits
|
hotlong
marked this pull request as ready for review
September 2, 2026 13:54
This was referenced Sep 2, 2026
finding(app-shell): app-missing / access-denied 恢复重定向仍写死 /home，应跟随声明的落地页（#7256 裁 B 的后续，先量夹具改动）
#7373
Closed
os-tesla
pushed a commit
that referenced
this pull request
Sep 19, 2026
…tui#7373) `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 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018HrVaotisyhgmot9o2MLRq
akarma-synetal
pushed a commit
to akarma-synetal/objectui
that referenced
this pull request
Sep 28, 2026
…tui#7373) (objectstack-ai#10041) Fixes objectstack-ai#7373 `app.isDefault` declares where a deployment's home is. objectui#7256 (PR objectstack-ai#7372) moved the console chrome's four Home affordances onto `useHomePath()`. The error-recovery exits kept naming the environment launcher literally (ADR-0075), so on cloud's control plane a customer refused an app — or bounced 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. Ruling B on the card ordered the work: **measure first, then change**. The classification below is the measurement, and it is the reviewable half of this change. ## 1. The measurement Population: every `/home` occurrence in `packages/app-shell/src` + `apps/console/src` at `030a675b0`, in every quoting. Re-derive it with ```bash git grep -n -E "['\"]/home['\"]" 030a675 -- packages/app-shell/src apps/console/src git grep -l isDefault -- packages/app-shell/src # lit control: the instrument is live ``` | | at `030a675b0` | |---|---:| | non-test occurrences, single-quoted (the card's own recipe) | 13 | | non-test occurrences, double-quoted | 5 | | **non-test, both quotings** | **18** (3 of them inside comments ⇒ 15 live) | | **test-side, both quotings** | **80**, across 34 files | Two readings worth having before the table: - ⭐ **The card's re-check recipe cannot see the two most important sites.** It greps `'/home'` — single quotes — and both `RedirectWithSplash to="/home"` redirects are double-quoted. One of them is `AppContent`'s no-app bounce, i.e. the first surface the card names. Anyone re-running that recipe and reading 12 or 13 as "the work" would ship the card's headline case unfixed. - The fixture half is not "about a dozen": it is 80 occurrences over 34 files at this denominator (57 by the dispatch's single-quote count). Most are route sinks and start URLs, not destination pins — which the table below is what separates. ## 2. Classification — every non-test occurrence, with its verdict ### MOVED — nine live sites now resolve through `useHomePath()` | file (at `030a675b0`) | what it is | why it moves | |---|---|---| | `console/AppContent.tsx` — the denied screen's Back-to-home button | recovery affordance on "you don't have access to this app" | the card's own case; the screen renders above the single `ConsoleLayout` mount, so this button is the whole way back | | `console/AppContent.tsx` — the no-app-to-enter bounce (`RedirectWithSplash`) | recovery redirect | the surface the card names first; sits below the readiness gate, so the list it resolves against has settled | | `console/ConsoleShell.tsx` — `RequireAiSurface`'s `redirectTo` default | recovery redirect for a runtime with no AI agent | only the DEFAULT moved; a host passing `redirectTo` still wins. No caller in this repo passes one | | `console/ai/AiChatPage.tsx` — `resolveCollapseToDockTarget`'s last rung | landing when `/ai` was the entry point | triage flagged this conditional for individual thought: rungs 1 and 2 (remembered origin, history back) are unchanged; only "nothing behind us" was a literal. It is now a **required parameter**, so a new call site cannot reintroduce the literal | | `console/ai/AiChatPage.tsx` — the no-agent screen's Home button | recovery affordance | same screen family as `AppContent`'s denied case | | `views/studio-design/StudioDesignSurface.tsx` — deleted-package eviction | recovery redirect: the package under the editor is gone and no sibling is left | destination was a literal; the sibling-package branch beside it is untouched | | `views/studio-design/StudioDesignSurface.tsx` — header Home button | Home affordance inside the Studio frame | named by the card | | `apps/console/.../StudioRoute.tsx` — `RequireStudioAccess`'s `redirectTo` default | recovery redirect: no `studio.access` | the "no access" half of the card, in the console app | | `apps/console/.../StudioRoute.tsx` — the Studio front door's wordmark | Home affordance one route from the previous row |⚠️ **declared in-place fix, flag it if you disagree.** It is the sibling screen of the header Home button above: leaving it would put two different homes one route apart inside one frame, which is the objectui#7256 defect reproduced by this very change | A **tenth** live site, `AcceptInvitationPage`, follows the same declaration by a different route — it lands on the console root so the resolution happens after the org switch. See its row below. ### MOVED BY A DIFFERENT ROUTE — one site follows the declaration without the hook | file | reading | |---|---| | `console/organizations/manage/AcceptInvitationPage.tsx` — after accepting an invitation | The app list in hand here belongs to the organization the user is **leaving**. `switchOrganization` has just resolved, `MetadataProvider` drops its cache on an org change (objectui#4486) and refetches, and this line runs before any of that lands — so a declared-landing answer read here names the PREVIOUS org's app, which is worse than the launcher. 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. This page was the THIRD site of that same transition and the only one not taking its shape, so it now lands on the console ROOT too (`window.location.href = resolveRootUrl()`) and `RootLandingRedirect` resolves the declaration for the org just JOINED. Seat ruling **A** on the fall-back stop (objectui#7373, 2026-09-19); the cache-timing reading is written at the call site, and the source scan keys this file on `resolveRootUrl` rather than on the hook, so a later edit that folds it onto `useHomePath()` goes red there. | ### NOT MOVED — with the reason, per the dispatch's requirement | file | why it stays | |---|---| | `utils/homePath.ts` — `HOME_LAUNCHER_PATH` | it IS the launcher, and the `??` fallback all nine moved sites resolve through. Changing it is option C, excluded by the ruling: it would strip the environment layer of its real launcher (ADR-0075) | | `console/ConsoleShell.tsx` — `RootRedirect` | not a recovery redirect: it is `/`'s landing. `/`'s policy is `resolveLandingPath`, which layers a single-visible-app emptiness heuristic (objectui#4048) and refuses to conclude from an unresolved list (objectui#4233). A third reading without those is a design question — raised below, not settled here | | `apps/console/.../RootLandingRedirect.tsx` — `return '/home'` | this IS rule 3 of the `/` policy — the documented answer when nothing is declared. Moving it is option C by another route | | `apps/console/src/App.tsx` — `Route path="/home"` | the route declaration. Move it and `/home` stops existing | | `apps/console/src/inbox-arrival-preview.tsx` | a `MemoryRouter initialEntries` preview fixture — a start URL, not a destination | | `layout/UnifiedSidebar.tsx` x2, `console/AppContent.tsx` x1 | prose comments about past defects. (The `AppContent` one was reworded because it quoted the line this change rewrote.) | ### Fixture discipline — what happened to the pins, and why each still fails for the right reason The existing `/home` assertions are **kept, not re-aimed**. Every one of them renders with an app list that declares nothing (or with no metadata provider at all), and for that population the resolved path *is* `/home` — so they now pin the undeclared-deployment case, and they still fail if a site navigates anywhere else.⚠️ On their own they are not enough, and that is the trap this card could have fallen into: an unchanged-and-still-green pin passes identically on the fixed and the unfixed implementation. So every moved site also gained a **declared-deployment** pin, which is what fails on the previous code. The one pin whose expected value genuinely changed is `resolveCollapseToDockTarget`'s, because the function took a new required argument; its launcher case is kept as a separate assertion so both answers stay pinned. `resolveCollapseToDockTarget` aside, no expectation was rewritten. ## 3. Evidence **Reverse verification** (mutate → prove it landed on disk → run → restore; `trap` on absolute paths, restoration proved by an empty `git diff HEAD` and a blob-hash comparison, not by an exit code): | leg | mutation | result | |---|---|---| | A — `AppContent` | `navigate(homePath)` 1 → 0, `navigate('/home')` 0 → 1; `to={homePath}` 1 → 0, `to="/home"` 0 → 1 | **3 failed, 31 passed** — exactly the two new behavioural pins and the source scan. Every pre-existing `/home` pin stayed green, which is the point: they cannot tell the two implementations apart | | B — `StudioRoute` | both `homePath` reads back to literals (2 literals on disk, verified by count) | **3 failed, 27 passed** — the gate pin, the wordmark pin and the scan | Restored byte-identically both times (`898a638de…` and `363d5b8f6…` match `HEAD`). **Suites** (repo-root invocation; all at `ad21d1a59`): | run | result | |---|---| | `vitest run packages/app-shell/src/console/ packages/app-shell/src/views/studio-design/` | 139 files, 887 tests passed | | `vitest run packages/app-shell/src/__tests__/ .../hooks/ .../utils/ .../chrome/ .../layout/` | 121 files, 1533 tests passed | | `pnpm --filter @object-ui/console test` | 101 files, 1161 tests passed | | `pnpm --filter @object-ui/app-shell type-check` | exit 0 | | `pnpm --filter @object-ui/console type-check` | exit 0 | | `pnpm exec eslint` over the 15 changed files | exit 0 — 0 errors, and **0 warnings on the lines this change wrote** (the fixtures are typed with `MetadataContextValue`, not `any`) | | `node scripts/check-changeset-presence.mjs` | exit 0 — 13 published source files, 1 changeset declared | | `node scripts/check-changeset-no-major.mjs` · `check-control-bytes.mjs` | exit 0 | Each exit code was captured before any pipe, from a redirected log. ## 4. One question raised, carried to its own card 1. **`RootRedirect` (`console/ConsoleShell.tsx`).** It is the `/` element published to consumers that do not mount `apps/console`'s `RootLandingRedirect`, and it ignores the declaration. Folding it into `useHomePath()` would give `/` a third answer — one without the emptiness heuristic and without the unresolved-list guard its twin has. Out of this card's subject (it is not a recovery exit), so it stays, documented at the site. Seat ruling **A** for this card; the follow-up is filed separately as **objectui#10042**. ## Acceptance notes Noted while in these files, not filed and not fixed here: - `apps/console/src/components/StudioRoute.tsx` ships a user-visible Chinese `title` attribute on the Studio wordmark, outside i18n, and `views/studio-design/StudioDesignSurface.tsx` carries Chinese source comments. AGENTS.md commandment #-1 is explicit that user-facing text and comments are English-only. Untouched: out of this card's scope, and the `title` is a behaviour change rather than a mechanical one. - `apps/console/src/components/StudioRoute.test.tsx`'s `@object-ui/app-shell` mock carries a comment explaining that the factory pulls one submodule "instead" of the whole barrel, while the factory does call `importOriginal()` on the barrel. The comment's own timing figures therefore describe a shape the code no longer has. --- _Generated by [Claude Code](https://claude.ai/code/session_018HrVaotisyhgmot9o2MLRq)_ --- _Generated by [Claude Code](https://claude.ai/code)_ --------- Co-authored-by: Claude <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #7256
What the card measured
cloud's control plane lands a signed-in customer on
/_console/apps/cloud_control/page/welcome— the app declared withisDefault: true, whose first navigation item is the Welcome page. The top bar's logo then said/_console/home: the ENVIRONMENT layer's launcher (ADR-0075), where "Build an app" and "Start from a template" act on an environment the control plane does not have, and "Your apps" lists the control plane's own internal management apps ("Cloud", "Account"). One product, two homes, two voices.Premise re-verified against
origin/main(63b4e0e71)Both producers the dispatch predicted are real, and are the two named below:
apps/console/src/components/RootLandingRedirect.tsx—resolveLandingPath()readsapp.isDefault, which cloud'sCLOUD_APPdeclares (packages/app-cloud/src/cloud.app.ts, withnav_home→ thewelcomepage first innavigation);packages/app-shell/src/layout/AppHeader.tsx:419— a literalLink to="/home".Three further affordances carried the same literal:
UnifiedSidebar's "Home" nav row and its mobile-sheet Home row, andAppSidebar's app-switcher "Home" entry. All four now readuseHomePath().The posture signal is the declaration, not a hostname or a product name
resolveDeclaredHomePath()readsapp.isDefaultand nothing else.cloud_controlappears nowhere in the shared bundle: re-introducing that name is precisely what retiringPREFERRED_APPS = ['cloud_control']removed, and hostname sniffing is ruled out by the card. A deployment that declares no landing is unaffected — every Home affordance still resolves to/home.The trade-off the card asked me to state: what becomes of the generic home
After this change the control-plane chrome offers no route into
/home: the logo and all three Home rows resolve to the declared landing. So the card's alternative half — hide "Cloud"/"Account" by role and drop the environment-side action cards — is not owed here; there is no control-plane entry point left to sanitise.What stays reachable, named rather than quietly omitted:
/_console/homeby hand;console/AppContent.tsxandconsole/ConsoleShell.tsx, plusAiChatPage's home button,StudioDesignSurface's close, andAcceptInvitationPage.Those are error-recovery paths, not Home affordances. Retargeting them moves a
/homeexpectation that ~a dozen existing fixtures pin, with a blast radius nobody has measured — deliberately left out of this card rather than folded in.Why
/'s resolver keeps its own reading of the declarationThe first version of this PR had
resolveLandingPathimportresolveDeclaredHomePath, so the declaration had exactly one reader. It turned 11 tests red:RootLandingRedirect.route.test.tsxandSetupRoute.test.tsxreplace@object-ui/app-shellwholesale with a chrome-free hand-rolled mock, and adding the symbol to that mock would have put the FIXTURE under test instead of the shipped policy.So the resolver is unchanged, and what forbids drift is a behavioural matrix comparing both shipped answers —
apps/console/src/components/landingHomeParity-7256.test.ts, 8 deployment shapes plus two drift canaries (which of several declarations wins; a nameless declaration). That is strictly stronger than a source scan of either file.Tests
Three new files, 20 + 8 assertions:
packages/app-shell/src/utils/__tests__/homePath.test.ts— the policy;packages/app-shell/src/hooks/__tests__/useHomePath.test.tsx— the hook's answer, including the in-flight app list;packages/app-shell/src/__tests__/homeAffordancesFollowDeclaration-7256.test.ts— a source scan pinning that no chrome affordance re-types the literal;apps/console/src/components/landingHomeParity-7256.test.ts— the parity matrix above.Reverse verification (fix committed first, then mutated): restoring
to={homePath}toto="/home"inAppHeader.tsxturned the scan red in the expected direction —Tests 2 failed | 6 passed (8), both failures on the "top-bar brand logo" cases. The mutation was confirmed on disk before the run (injected-literal count 0→1, removed-expression count 1→0, blob hash62364926vs HEAD1ea6f4dc); the restore leg was confirmed by hash equality with the HEAD blob and an emptygit diff HEAD.Gates run at
6147b3c86pnpm --filter @object-ui/app-shell type-check— exit 0 (its script istsc --noEmit && tsc -p tsconfig.test.json, so the new test files ARE covered)pnpm --filter @object-ui/console type-check— exit 0pnpm --filter @object-ui/app-shell build— exit 0,dist completeness: 1 package(s) complete (902 emitted files verified);@object-ui/consolebuild exit 0app-shelllayout / console / utils / hooks / tests +apps/console/src/components):Test Files 199 passed,Tests 2151 passedbesides the pre-existing red belowpnpm --filter @object-ui/app-shell --filter @object-ui/console lint— exit 0, 0 errors (216 pre-existing warnings)check:control-bytes,check:vi-mock-specifiers,check:vi-mock-inherit,check:self-import,check:phantom-deps,check:eager-closure,check:side-effects-array,check:esm-specifiers,check:readme-exports,check:entry-guard,check:i18n-keys,check:i18n-drift,check-changeset-no-major— all printed their own OK lineLint narrowing: run per PACKAGE (
eslint .from each package, i.e. eslint's own config-derived population for both packages this diff touches, not my guess at a file list); the repo's flat config enables no type-aware linting (noproject/projectService), so this diff cannot move the verdict on any file it does not contain.Pre-existing reds, not caused by this diff
packages/app-shell/src/console/__tests__/anonSeedScope-5746.enumeration.test.tsx— 7 red. Ablation: reverting all six of my modified source files to63b4e0e71(confirmed on disk,homePathreference count 0,0,0) leaves the same 7 failures. Already filed as objectui#7271 (Node 26 localStorage probe).packages/auth/src/__tests__/signOut-client-cache-purge-5198.test.tsx:5777— 1 red, same family.packages/authdeclares no dependency on@object-ui/app-shelland this diff contains no file in that package, so it cannot be reached from here.Changeset
.changeset/7256-chrome-home-follows-declared-landing.md,patchon@object-ui/app-shell+@object-ui/console.Generated by Claude Code