You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
Commit 2f7977c
Browse filesBrowse the repository at this point in the historyBrowse files
fix(app-shell): record pages and sealed managed apps get no navigation writes (#29294)
<!-- claude-projects-attribution: {"github_login":"zhuangjianguo"} -->
_Requested by **Steedos** · [project
thread](https://claude.ai/code/project/chan_018ETKaNwMZCNRYD7D8Q9Ubb?thread=cmsg_018ETKaNwMZCNRYD7D8Q9UbbM4E3JW8kr9W6U2A9dehyRU)_Fixes#29182
Clause-②: no
Metadata-protocol: none
## 维护者速读
- **改了什么**:新建页面首次发布时,只有「不需要记录就能打开」的页面才写进各应用导航;记录页(`type:
'record'`)不再写。由托管包提供、被服务端封存的应用(如 showcase 的
`showcase_app`)不再被写导航,也就不再弹红色「Failed to update navigation」。
- **为什么改**:管理员每建一个记录页,控制台就往每个应用的导航里加一个打不开的入口(没有记录 id),并且对封存应用的写入被 403
拒绝、每次都弹错误提示。
- **风险与回滚**:只改 `useNavigationSync.ts`
的两处判断;「全部发布」路径无页面正文时按名读一次页面类型,读失败则不写导航并提示。回滚 = revert 本 PR。
-
**先看**:`ui/packages/app-shell/src/hooks/useNavigationSync.ts`(`isNavigationSyncableApp`
的 `_provenance` 行、`useNavigationWrites().created`
的页面分支),`ui/packages/app-shell/src/hooks/__tests__/useNavigationSync.test.tsx`
末尾的新 describe。
- **两分钟验证**:showcase 上在 Studio 新建 `record` 类型页面并发布:网络面板里发布后没有任何 `PUT
/api/v1/meta/app/*`,也没有红色提示;改建 `home` 类型页面,只写运行时创建的应用。
- **需要你做的**:无,席位落地。
## What changes
`ui/packages/app-shell/src/hooks/useNavigationSync.ts`, per the seat's
rulings on #29182:
1. **A page that needs a record is never synced into navigation.**
`useNavigationWrites().created` now asks `isNavigablePage(body)` before
`syncPageCreatedAllApps`. The page type comes from the body the create
already holds (`ResourceEditPage` passes the published `lay.effective`).
A body that leaves `type` out counts as a record page, because
`PageSchema` declares `type` with the default `PAGE_TYPE_DEFAULT =
'record'`.
- **Publish all** (`usePublishAllDrafts`) holds only the `_drafts`
header rows, so no body reaches `created()`. For that caller only, the
door reads the page by name once (`client.meta.getItem('page', name)`,
one read per created page). If that read fails, the navigation write for
that page is **skipped, not written blind**, and the existing "Failed to
update navigation" toast says navigation was not updated. The seat
amended the claim to add the Publish-all test file to the surface for
this.
2. **A sealed app is not syncable.** `isNavigationSyncableApp` also
returns `false` for an app whose read carries `_provenance: 'package'`.
`app` has no per-org overlay channel (`allowEnvironmentOverlay: false`),
so the server refuses every package-less write onto an artifact-backed
app with `403 NOT_OVERRIDABLE` whatever its lock. The marker was
measured on the stock showcase (below). Because the read carries a
marker, the `403 NOT_OVERRIDABLE` fallback branch of the ruling is not
needed and not added. Any other app-write failure still shows today's
error toast.
3. Ruling A of #28680 stands: a navigable page is still written into
every app that can take it, at create time.
No metadata key, no server change, no new gate.
`isNavigationSyncableApp` is not exported from `@object-ui/app-shell`'s
entry, and its signature is unchanged.
### Non-navigable page types found
`PageTypeSchema` (`packages/spec/src/ui/page.zod.ts`, the enum at
`PageTypeSchema`) has five members: `record`, `home`, `app`, `utility`,
`list`. Only `record` cannot render without a record:
- `record`:
`ui/packages/app-shell/src/views/metadata-admin/inspectors/nav-target.ts`,
`isStaticPageOption` docblock: "Record-detail pages (`type: 'record'`)
require a specific record id to render, but a `type: 'page'` nav item
resolves to a static `/apps/{app}/page/{name}` URL with no mechanism to
pass one". Also `packages/spec/src/ui/page.zod.ts`,
`checkPageRecordBindingPlacement` docblock: "A record page already has
its row — the record it opens".
- A body with no `type`: `packages/spec/src/ui/default-record-page.ts`,
`PAGE_TYPE_DEFAULT` docblock: "a page authored without `type` IS a
record page, and an unparsed body … that leaves the key out is judged as
one".
- `home`, `list`, `utility` render without one (same
`checkPageRecordBindingPlacement` docblock: they are "not about a row at
all"). `app` resolves its own row from its optional `record` query
binding, so it does not need one from the route either.
### The sealed-app marker, measured
Stock showcase boot at this branch's head (`objectstack dev --seed-admin
--fresh`), `GET /api/v1/meta/app`, private keys per app:
| app | served keys | package-less `PUT` |
|:--|:--|:--|
| `showcase_app` | `_packageId: com.example.showcase`, `_provenance:
package` (no `_lock`) | `403 NOT_OVERRIDABLE` ("provided by a managed
package and is sealed") |
| `setup`, `account` | `_provenance: package`, `_lock: full` | (already
skipped by the lock) |
| `probe_app` (package-less, created by PUT) | none | 200 |
| `rthree_app` (app of a package created at runtime, `POST
/api/v1/packages`) | `_packageId: com.probe.rthree` only | 200 |
So `_provenance` is the discriminator. `_packageId` is not, because a
runtime package's app carries it and takes the write.
## Tests
- `useNavigationSync.test.tsx`: new `isNavigablePage` units, two new
`isNavigationSyncableApp` units, and a new `describe` on the door: a
record page (and a type-less body) writes no app and shows no toast;
`home` and `list` pages still write every syncable app; with no body
held the page is read by name and its type decides; a failed read writes
nothing and toasts once; a sealed app gets no write and no red toast on
create, dashboard create and delete; another app's write failure (500)
still toasts. Existing door cases now pass an `app` page body.
- `usePublishAllDrafts.navigationWrites-28680.test.tsx` (surface amended
by the seat): the adapter mock gains `getItem`. New case: Publish all
with a new record page and a new `home` page writes only the `home`
page, with exactly one by-name read per created page.
- `useNavigationSync.labelInherits-9868.test.tsx`: its page body now
states `type: 'app'`. A type-less body is a record page, so it no longer
writes. That is the behaviour this PR asks for.
<details><summary>Test runs (head 92a066d)</summary>
- Touched files: `cd ui && pnpm exec vitest run
useNavigationSync.test.tsx
usePublishAllDrafts.navigationWrites-28680.test.tsx`: Test Files 2
passed, Tests 30 passed.
- Every ui test naming `useNavigationSync`, `useNavigationWrites` or
`isNavigationSyncableApp` (9 files): Test Files 9 passed, Tests 77
passed.
- Every ui test naming `usePublishAllDrafts` or `publishAll`, plus every
`ResourceEditPage*.test.tsx` (76 files): Test Files 76 passed, Tests 419
passed.
- `turbo run type-check --filter=@object-ui/app-shell`: 65/65 tasks
successful.
- After merging `origin/main` (36 commits, none on the touched files),
on head `3c9ee893`: every ui test naming `useNavigationSync`,
`useNavigationWrites`, `isNavigationSyncableApp` or
`usePublishAllDrafts` (33 files): Test Files 33 passed, Tests 277
passed.
</details>
<details><summary>Derived gates (dispatch-gates --commands, 34
families)</summary>
All 34 exit 0 on head `92a066d9`, reconciled with `dispatch-gates
--ran`: "34 derived famil(ies) accounted for — 34 run, 0 NOT-MEASURED".
`pnpm --dir ui exec vitest run scripts/`: Test Files 169 passed, 2
skipped. Re-run on the merged head `3c9ee893`: `check-sdui-manifest`,
`check-changeset-no-major`, `check-closing-keyword-parity`,
`check-adr-0087-registration`, `check:nul-bytes`, all exit 0.
</details>
<details><summary>Ablations (each committed first, run through
scripts/ablation-replace.mjs, restored to the HEAD blob)</summary>
- Record-page guard `if (!isNavigablePage(body)) return;` made
always-pass: 3 cases red (record page writes no app; by-name read
decides; Publish all record page).
- Sealed-app line `if (a._provenance === 'package') return false;`
neutralised: 2 cases red (the unit; the door's sealed-app case).
- Failed-read branch `if (body === undefined) {` disabled: 1 case red
(the failed read toasts).
Each restore read back as the HEAD blob `06b76092f89a` with an empty
`git diff HEAD`.
</details>
## Acceptance notes
- A failed by-name page read (Publish all only) skips that page's
navigation write and shows "Failed to update navigation". It does not
write blind.
- Apps written by an earlier record-page create keep their stray
entries. This PR stops new ones and does not clean up old ones. Deleting
the page still removes its entries, as before.
## Verification
Card steps on a stock showcase (`examples/app-showcase`, `objectstack
dev --seed-admin --fresh`), Console from source (Vite, `@object-ui/*`
src-aliased), Playwright. Studio `/studio/~all/metadata/page/new`,
Object `showcase_task`, Save, then Publish. Apps present:
`showcase_app`, `setup`, `account`, and the two runtime apps above.
<details><summary>Before (useNavigationSync.ts restored from origin/main
14208f2), page type record</summary>
```text
PUT /api/v1/meta/page/before_record_probe?mode=draft -> 200
POST /api/v1/meta/page/before_record_probe/publish -> 200
PUT /api/v1/meta/app/showcase_app -> 403
PUT /api/v1/meta/app/probe_app -> 200
PUT /api/v1/meta/app/rthree_app -> 200
toasts: success "Navigation updated: added page before_record_probe" · error "Failed to update navigation"
```
</details>
<details><summary>After (this branch), page type record, run twice
(before and after the before-run's restore)</summary>
```text
PUT /api/v1/meta/page/after_record_probe_two?mode=draft -> 200
POST /api/v1/meta/page/after_record_probe_two/publish -> 200
toasts: none
```
</details>
<details><summary>After (this branch), page type home</summary>
```text
PUT /api/v1/meta/page/after_home_probe_two?mode=draft -> 200
POST /api/v1/meta/page/after_home_probe_two/publish -> 200
PUT /api/v1/meta/app/probe_app -> 200
PUT /api/v1/meta/app/rthree_app -> 200
toasts: success "Navigation updated: added page after_home_probe_two"
```
No write reaches `showcase_app`, `setup` or `account`, and no error
toast shows.
</details>
🤖 Generated with [Claude Code](https://claude.com/claude-code)
---
_Generated by [Claude
Code](https://claude.ai/code/session_01QsR2jUbgX5dtoeHsDmfa9Z)_
---------
Co-authored-by: Claude <noreply@anthropic.com>
fix(app-shell): creating a record page no longer adds it to every app's navigation, and no navigation write goes to a sealed managed app
6
+
7
+
Clause-②: no
8
+
Metadata-protocol: none
9
+
10
+
-**A page that needs a record joins no app.** When a page is published for the first time, the Console adds it to app navigation only if it renders without a record. A `record` page (and a page body that leaves `type` out, which `PageSchema` defaults to `record`) gets no navigation entry, because a page entry opens the page with no record id. `home`, `app`, `utility` and `list` pages are still added to every app that can take them. When the publishing screen holds no page body (Publish all), the page is read by name once to learn its type; if that read fails, the page is left out of navigation and the error toast says navigation was not updated.
11
+
-**A sealed managed app is not written.** An app a managed package ships (`_provenance: 'package'` on the app read, as the stock showcase serves `showcase_app`) is sealed: the server refuses a package-less write with `403 NOT_OVERRIDABLE`. Navigation writes now skip such an app, so publishing a page no longer shows a red "Failed to update navigation" toast for it. Apps stored at runtime, including apps of a package authored at runtime, are written as before. Any other failure of an app write still shows the error toast.
0 commit comments