Repository navigation
fix(app-shell): the app bar's Edit app pencil hides for system apps and read-only packages (objectui#12615) - #12652
Conversation
…he package's posture (WIP) The app bar's Edit app pencil, the app switcher's Studio row it feeds and the sidebar menu's Design entry now read one gate, useStudioDesignGate: a workspace admin holding manage_metadata, on a package Studio's package list serves as writable. Kernel packages (Setup, Studio, Account) are not in that list, so they fall out with no app-id list. Claude-Session: https://claude.ai/code/session_01Vk15bz8xHtKGUfXfCm16h4 Co-authored-by: Claude <noreply@anthropic.com>
…t now wait on the posture read The shared feeds key on the adapter's identity, so a stub that hands out a fresh adapter per render re-attaches them on every commit: a case that waits with the bar mounted never settles (measured: tens of thousands of commits in a minute, no new request). The right-click case waits for the verdict before it reads the menu. Claude-Session: https://claude.ai/code/session_01Vk15bz8xHtKGUfXfCm16h4 Co-authored-by: Claude <noreply@anthropic.com>
…it-app-pencil-posture Claude-Session: https://claude.ai/code/session_01Vk15bz8xHtKGUfXfCm16h4 Co-authored-by: Claude <noreply@anthropic.com>
…it-app-pencil-posture objectui#12601 (the search entry on /home) landed in AppHeader.tsx; the merge is clean and keeps both: its search entry, this branch's Edit app gate. Claude-Session: https://claude.ai/code/session_01Vk15bz8xHtKGUfXfCm16h4 Co-authored-by: Claude <noreply@anthropic.com>
…touched Claude-Session: https://claude.ai/code/session_01Vk15bz8xHtKGUfXfCm16h4 Co-authored-by: Claude <noreply@anthropic.com>
✅ 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
|
…it-app-pencil-posture Claude-Session: https://claude.ai/code/session_01Vk15bz8xHtKGUfXfCm16h4 Co-authored-by: Claude <noreply@anthropic.com>
…posture, as the app bar's does PageView's pencil now reads useStudioDesignGate, the gate the app bar's Edit app, the switcher's Studio row and the sidebar's Design read: a workspace admin holding manage_metadata, on a package Studio lists as writable. A read-only package's page and a system app's page draw no pencil; it is absent while the list is read and after a failed read. The gate is asked before the not-found return, and a page that does not resolve asks nothing. Claude-Session: https://claude.ai/code/session_01Vk15bz8xHtKGUfXfCm16h4 Co-authored-by: Claude <noreply@anthropic.com>
✅ 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
|
…it-app-pencil-posture objectui#12605 (the app bar's home mark and app menu) landed in AppHeader.tsx and AppHeader.appIdentity-12201.test.tsx; the merge is clean and keeps both: its left-side changes, this branch's Edit app gate and harness. Claude-Session: https://claude.ai/code/session_01Vk15bz8xHtKGUfXfCm16h4 Co-authored-by: Claude <noreply@anthropic.com>
|
Generated by Claude Code |
✅ 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
|
席位核对 — ACCEPT ·
|
| surface | PR head | origin/main |
|---|---|---|
Setup: app bar pencil · switcher Studio row · sidebar ⋯ Design · page pencil on /page/marketplace_installed |
all absent (DOM count 0) | all present |
CRM (writable: false): the same four, page crm_welcome |
all absent | all present |
| Account: bar · switcher row · page pencil | all absent | all present |
Verify App (writable: true): bar pencil ("Edit app"), switcher row, sidebar Design, page pencil |
all present; the pencil opens Studio editable (Design canvas, Add block, Publish) | present |
| a non-admin member on Verify App | all absent | all absent |
- Switching apps: the pencil never shows under the CRM identity. It lingers about 180–280 ms while the bar still reads the previous app, and returns about 150–170 ms after the bar reads the writable app.
- Browser console: no errors on either build.
3. Gates (check-runs pinned to this head). 32 success and 3 skipped (Test (coverage), its matrix row, dependabot). Among them: Lint, Type Check, Repo Gates, Test (shard 1/16) … (16/16), Test (dist pins), Build & E2E, Build Docs, Bundle Analysis (budget PASS, 6214634752), the changeset checks, Line Citation Gate and Governed Surface Queue Guard.
- One red:
Live E2E (informational). It fails atStart ObjectStack backend (published packages), the same as every PR built onf62b6ff. That red is anchor objectui#12649's (domain:devxp1), not this diff's (comment6214603865). - The local subset is in the dev's reports on the card (110 files / 822 tests; PageView ablation 5 red / 1 control green; gate ablation 7 red).
4. Accepted deviations.
- Each gated surface costs one
GET /api/v1/packagesper app or page navigation, for principals who pass the capability check. It is shared in flight between the bar and the sidebar rows. No provider caches the package list today; the dev measured that, and both cost cases are pinned. - The narrowed local suite: CI ran the full app-shell suite on all 16 shards.
- Prose checked sentence by sentence against the diff: the changeset's "for a metadata author, on an app whose package Studio lists as writable", "no longer show it, and neither does the app switcher's 'Edit app in Studio' row", "a page's Edit in studio pencil follow the same rule", and "While the package list is loading, or if it cannot be read, these entries stay hidden". Each matches
useStudioDesignGateand its four call sites.
5. Out of scope (where each went). An AppHeader test-harness trap: a fresh useAdapter object per render loops once a case awaits with the bar mounted. This PR fixes the harnesses it touched, and the others stay dormant (synchronous reads). Noted, not filed. The one-step target change of the switcher row (/data first, then /interfaces?surface=page:…) predates this PR: it is identical on main.
6. Landing. Not governed, under 3000 lines, no contract face. Every check is green except Live E2E (informational). That red is neither this PR's nor "red by design" (its source does not declare it), so the queue-entry rule holds the PR until objectui#12649 lands. ⇒ The PR stays draft. Card objectui#12615 goes pm:blocked on objectui#12649 with Unlock-action: re-check PR #12652. When the anchor lands, the seat merges main if needed, re-reads this head's checks, and lands it through the relay (pr_ready + automerge_enable). On MERGED, Fixes closes objectui#12615.
Generated by Claude Code
❌ 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. Which half objected:
📦 Bundle Size Report
Size Limits
|
✅ 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
|
Fixes #12615
What changed
One gate,
useStudioDesignGate(new,packages/app-shell/src/hooks/useStudioDesignGate.ts, not exported from the package), now decides every app → Studio "design this" entry the card names:AppHeader), and with it the app switcher's Edit APP in Studio row, which takes the pencil's own target;SidebarItemMenu), for a navigation entry and for a favorite.PageView,page-edit-in-studio-button, objectui#12540), added by the patch round on claim6214038429:useStudioDesignGate(page ? appStudioRoutePath(activeApp, true, { type: 'page', name: pageName }) : null). It is asked before the page's not-found return, and a page that does not resolve asks nothing.An entry is drawn only when both halves hold:
manage_metadata(useCanAuthorMetadata, the check objectui#5521's home cover cards use);fetchPackages/PkgEntry.writable(the server'swritableverdict, ADR-0070 D2). No second derivation of "writable" was written.Otherwise the entry is not rendered (never disabled). That covers a read-only package's app (HotCRM in the card's walk) and the console's own Setup, Studio and Account apps.
The hook takes the Studio path the entry would open (
appStudioRoutePath's answer) and reads the package off its first segment (studioPathPackageId). Both callers therefore ask the same question: may this principal design what this link opens?SidebarItemMenuis only handed adesignPath, so that path is the one input the two callers share. No host (UnifiedSidebar) had to change.The PM's hypotheses, measured on
origin/main79c1996AppHeadergated the pencil withisWorkspaceAdmin && canAuthorMetadataand nothing else.appStudioRoutePath's own doc says "Writability of the target package is NOT checked here". That sentence is still true of that function, which this PR does not touch.packages-io.tsis the posture read Studio's own read-only posture uses (StudioDesignSurfacesetsreadOnlyfromlist.find(...)?.writable). It is reused unchanged.scope: 'system'(objectstackpackages/apps/{setup,studio,account}/src/index.ts). The server's package-writability predicate returnsfalsefor those scopes (READ_ONLY_PACKAGE_SCOPES), andparsePackagesleavessystem/cloudpackages out of the list. The rule is "listed AND writable", so a kernel package falls out either way. If its row is ever served in the registry's flat shape, the scope sits at top level, the row is not hidden, and itswritable: falseanswers instead. Both shapes are pinned.fetchPackagesis called only by the Studio landing, the Studio design surface and the command palette'sstudioscope. Nothing at shell level holds the list, andpackages-iodocuments that it is deliberately not a response cache. The gate costs oneGET /api/v1/packagesper package asked about, and only for a principal who passes the capability half. It reads when the asked package changes (an app switch), never per render. The bar and the sidebar rows mounted with it share that one request throughfetchPackages' in-flight read (objectui#11797). Pinned: "the bar and the sidebar rows mounted with it share one read" counts one request, and "a principal the capability half refuses never reads the package list" counts zero.No verdict, no entry. While the list is being read, and when the read fails, the entry is absent. This is the opposite of
useCanAuthorMetadata's fail-open, on purpose; the hook's header says why. An unreported capability set comes from a deployment that never reports one, so failing closed there would remove the CTAs for good. This read answers or fails on each visit. A missing shortcut costs one click through Studio's landing. A drawn one on a read-only package is the dead end this card removes.Tests
New:
AppHeader.editAppFollowsPackagePosture-12615.test.tsxcovers:com.objectstack.setup;writable: false);SidebarItemMenu.designFollowsPackagePosture-12615.test.tsxcovers:studioPathPackageIdround-tripping every route shapeappStudioRoutePathbuilds.Updated harnesses (the entries now wait on a verdict):
AppHeader.designInStudioNeedsAuthoring-10899,AppHeader.appIdentity-12201,AppHeader.appBarLeftGroup-12461,AppHeader.centeredSearch-12472,AppHeader.pageRouteNoAuthoringHandles-12198,SidebarItemMenu-12427,SidebarItemMenu.designOpensStudio-12520andUnifiedSidebar.favoritesMenu-12427:SidebarItemMenu.designOpensStudio-12520andUnifiedSidebar.favoritesMenu-12427:app.objectstack.hotcrmbecamecom.acme.crm. HotCRM's package is read-only, and the new tests pin it as such.adapterKey), so a stub that hands out a fresh adapter on every render re-attaches them on every commit;AppHeader.pageRouteNoAuthoringHandles-12198notes ("with both halves mounted in one tree, this harness's render never finished"). That is an observation; it was not re-measured here.Results
The table's readings are round 1's, on
f599de6. The patch round's readings on the final headfe2aebafollow the table. Every heavy run went through the shared verify lock, so wall-clock figures come from a shared box.pnpm exec vitest run --maxWorkers=2over the 108 test files that renderAppHeader,ConsoleLayout,UnifiedSidebar,SidebarItemMenuorPageView, or live inconsole/home/__tests__(all oflayout/__tests__they name, theconsole/__tests__AppContent suites, theapps/consoleshells)Test Files 108 passed (108),Tests 802 passed (802), lock verdictcommand-exit 0pnpm --workspace-concurrency=2 --filter '@object-ui/app-shell^...' build(the dependency closure, so the type-check reads freshdist)command-exit 0pnpm --filter @object-ui/app-shell type-check(tsc --noEmit && tsc -p tsconfig.test.json; the second project'sincludeissrc/**/*.test.ts(x))command-exit 0pnpm exec eslinton the 13 changed.ts/.tsxfilesAppHeader.tsx,SidebarItemMenu.tsxand the 10899 test's mock)node scripts/check-changeset-presence.mjspnpm check:new-line-citations0 new citation(s), exit 0Patch round, on
fe2aeba:pnpm exec vitest run --maxWorkers=2over 110 files (now including the four PageView Edit-in-studio files andmain'sAppHeader.homeMark-12605) →Test Files 110 passed (110),Tests 822 passed (822).app-shelltype-check 0; eslint on 17 files: 0 errors (35 warnings, all on pre-existing lines); changeset presence 0; line citations 0.1b4e67f(byte-identical atfe2aeba):const studioPath = useStudioDesignGate(was replaced withconst studioPath = ((p: string | null) => p)((blobd7ebfcfdaf1e→edb39c825ba6). Result:Tests 5 failed | 1 passed (6), with the writable CONTROL green. Restore proven.origin/mainwas merged three times in all. objectui#12601 and objectui#12605 landed inAppHeader.tsx(Console app bar: the product mark stays top-left as the home button; the app menu gains Home, All apps and Setup #12605 also inAppHeader.appIdentity-12201.test.tsx); both merged clean and keep both intents.Live E2E (informational)is red at its backend-start step, as on every PR built onf62b6ff. That red is objectui#12649's, not this diff's.Ablation, at
690835c(the ablated line is unchanged atfe2aeba; the hook changed only in doc comments). The run usednode ../objectstack/scripts/ablation-replace.mjs, which replacedreturn packages.some((pkg) => pkg.id === packageId && pkg.writable);inuseStudioDesignGate.tswithreturn true;, so the posture half was gone. The mutation landed (anchor x1 to x0, blob0f8d83a07af5to06d12c72b15b). The two new files then wentTests 7 failed | 9 passed (16). Exactly the seven posture cases went red: Setup, the read-only package, the unlisted package and the flat-shape row in the bar; Setup, the read-only package and the read-only favorite in the sidebar. The controls, the pending and failed reads, the cost pins and the principal case stayed green. Restore was proven by blob equal to HEAD andgit diff HEADempty. The predicted direction was red, and red is what was observed.The rest of
@object-ui/app-shell's suite (the package holds well over a thousand test files) was left to CI. That narrowing was a choice, and the list above is what it kept. The list comes from a source grep for those component names, so it cannot see a test that reaches them through another module.Acceptance notes
objectstackcheckout in this container has no install and no build (nonode_modules, nopackages/*/dist). Booting a backend for the console takes a full install plus a build of the CLI's closure on a box shared by parallel agents. The seat runs the browser check at review.PageView's Edit in studio pencil, now follows the same gate (patch round, claim6214038429). It is pinned byPageView.editInStudioFollowsPackagePosture-12615:/studio/com.example.my_base/interfaces?surface=page:welcome.PageView.editInStudioNeedsAuthoring-10899,PageView.editInStudioOpensStudio-12540andAppHeader.pageRouteNoAuthoringHandles-12198serve their fixture package as writable and settle before they read.@object-ui/app-shell: the hook is imported by relative path only, and neither the package index norhooks/index.tsre-exports it.Generated by Claude Code