Repository navigation
Commit a36a691
fix(rest): the draft read serves an app whole to whoever may save it, and pruned to everyone else (#20337)
Fixes #20290
Clause-②: no
This executes triage's grade on the card (comment 5859504238). Triage
decided the carrier is the server, under maintainer ruling 5856774816 on
#20156 (letter B). Triage's words, verbatim:
> **Carrier, decided here: the server.** Ruling B's own words decide it:
「Read-to-display is pruned per user; read-to-edit is whole for the
editor」, and 「whoever can save it must see it whole, or a save drops
entries silently」. A draft is a stored version, not a rendered one. So
the plain read's `?state=draft` branch takes the same author exemption
PR #20284 gave the stored-version doors: whole for whoever may save the
app, pruned for everyone else.
## What changed
**The transport** (`packages/rest/src/rest-server.ts`)
- The plain read `GET /meta/:type/:name` now chooses its gate policy by
what it serves. Its `?state=draft` branch serves the pending draft row,
which is a stored version, so it runs
`RestServer.STORED_VERSION_DOOR_POLICY` (`{ arms: 'per-caller', app:
'author-exempt' }`). Every other read on that route keeps `{ arms:
'all', app: 'gate' }`, including the `?preview=draft` render.
- There is no second predicate and no route test in the gate.
`metaItemReadGate` already attaches `MetaReadGateCaller.mayWriteItem`
whenever the policy is `author-exempt`, from
`RestServer.metaSaveVerdict`: the admission of `PUT /meta/:type/:name`,
spelled once by PR #20284. The draft read inherits that.
- **For an app:**
- a caller the app's save door admits reads the stored draft whole;
- every other caller who may open the app reads it pruned per caller,
exactly as before;
- an app the plain read refuses whole (an app-level
`requiredPermissions` the caller lacks, or an unpublished app to a
non-builder) is still refused, to an author too.
- **Per-deployment gates:** the ADR-0057 D10 `requiresService` arms
(app, nav entry, dashboard widget) and the nav servability gate no
longer run on the draft read, for any caller. See Acceptance notes item
4 for the measurement and the reasons.
- Unchanged: `NO_DRAFT` (404) when nothing is pending, the docs audience
on `doc` and `book`, and the object mask.
**The shared gate** (`packages/rest/src/meta-item-read-gate.ts`):
docblock only. Three sentences listed the doors each policy member
serves and named "the plain read" as a rendered door without an
exception. They now name the draft branch. No code changed.
**Tests**
- The census `meta-alternate-door-read-gates.test.ts` gains the draft
read as a door, `?state=draft`, of kind `stored`. Its cells sit beside
`/layers`, `?layers=true` and `/diff` for every subject and caller (36
new cells).
- `AUTHOR_EXEMPTION` names four doors now, with `draftCarrier:
'5859504238'`.
- The scope pins hold the exemption to exactly the author's partial
`crm` cells: 4 changed cells and 8 whole refusals kept.
- Each authenticated draft cell asserts that the protocol was asked for
`state: 'draft'`.
- New: `meta-draft-read-author-exemption.test.ts` runs the real stack:
better-sqlite3 `:memory:`, the real `sys_metadata*` objects, a real
`ObjectStackProtocolImplementation` and the real routes. The only stubs
are the auth boundary and the `tenancy` service probe.
- An author holding `manage_metadata` but not `finance.access` reads the
draft whole: the withheld entries, a draft-only entry and one whose
service is off.
- A member reads it pruned per caller (the control).
- **A draft save by that author keeps `nav_finance_ledger`.** The test
runs the editors' round trip: `/layers` effective, merged with the
stripped draft, saved back through `PUT ?mode=draft`. It then reads the
persisted `sys_metadata` draft row.
- It widens nothing: the author's draft answer equals what `/diff`
already serves them for the same history version.
- The plain read and `?preview=draft` still prune for the author.
- A whole refusal (`payroll`) stays `403` on the draft read.
**Docs and release notes**
- `content/docs/ui/apps.mdx` names `?state=draft` among the doors that
answer by who is asking, and states that those doors apply no
per-deployment gate. Its gate-table row now says "the rendered `/meta`
body".
- The two pending release notes are corrected in place (see the section
below). This PR's own note is
`.changeset/20290-draft-read-author-exemption.md`, `@objectstack/rest`
`patch`.
## Verification
**Tests**, at head `bd1361ee6`. `git diff bd1361e 8055937 --
packages/` is empty: the only change since is the one `apps.mdx` table
row.
- Census plus the new real-stack file: 303/303.
- `@objectstack/rest`, `vitest run --project local`: 203 files, 3704
passed, 1 skipped, 0 failed.
- `test:repo`: 8/8.
- `@objectstack/rest` `typecheck` (`tsc --noEmit` plus the test layer
under `tsconfig.test.json`): exit 0. `tsc -p tsconfig.test.json
--listFiles` includes both test files (204 test files in the program).
- `@objectstack/runtime` is not touched and no rest export changed, so
no runtime test is owed. The dispatcher does not serve `?state=draft`;
see Acceptance notes item 5.
**Ablations**, committed fix first, at head `bd1361ee6`.
- Each mutation ran through `scripts/ablation-replace.mjs` in wrap mode,
inside a script with its own EXIT, INT and TERM trap that restores
`packages/rest/src/rest-server.ts` from `HEAD` by absolute path.
- Each restore was proven twice: the blob equals `HEAD` (`d5ccd775bb3b`)
and `git diff HEAD` is empty.
- The subject is imported by relative path (`./rest-server.js`), so the
mutations act on source and no `dist` is involved.
- Each prediction was written down before its run.
| mutation | landed (anchor, blob) | predicted | result |
|:--|:--|:--|:--|
| A: the exemption off on the draft read (`{ arms: 'per-caller', app:
'gate' }`) | 1 → 0, `d5ccd775bb3b` → `4b8f055b9a0e` | 4 red | **4 red**:
the census `?state=draft app/crm × author` cell; real stack: author
reads whole, the draft-save round trip, "widens nothing" |
| B: the exemption for everyone (`mayWriteItem: true`) | 1 → 0,
`d5ccd775bb3b` → `f45c0e2167e9` | 7 red | **7 red**: `app/crm ×
non-reader` on all four stored doors (`?state=draft` included), the
fault-path `/layers` edge, the org-presentation edge, the real-stack
member control |
| C: per-deployment arms back on the draft read (`{ arms: 'all', app:
'author-exempt' }`) | 1 → 0, `d5ccd775bb3b` → `ad2e5f485f20` | 4 red |
**4 red**: `?state=draft dashboard/ops` × reader, non-reader and author;
the real-stack member control (`nav_org_directory`) |
**Gates**, at head `8055937f5`.
- `node scripts/pm/dispatch-gates.mjs --repo objectstack-ai/objectstack
--commands` derived 91 families from merge base `10ea9eb2e`. Every
command ran with its exit code captured before any pipe.
- `--ran` reconciled the run: 91 derived, 91 run, 0 NOT-MEASURED, 0
UNRUN.
- 90 exit 0. `node scripts/check-empty-changeset.mjs --base origin/main`
exits 1 **by design** (next section).
- `check:skill-examples`, `check:type-check-debt` and
`check:dual-build-cjs-loads` first refused with exit 3, because the
merge moved `packages/spec/src` and the client SDK was unbuilt. All
three exit 0 once their named prerequisites were built:
- `check:skill-examples`: 259 examples;
- `check:type-check-debt`: 4 entries re-measured, none above its record;
- `check:dual-build-cjs-loads`: 104 entry points across 66 packages.
- Added by this lane:
- `pnpm lint` (`eslint . --no-inline-config`, the whole repo): exit 0.
It is a full run, not a narrowed one.
- `pnpm --filter @objectstack/spec run check:liveness`: exit 0.
- The board check `node scripts/check-issue-citations.mjs`, after
merging `origin/main` at `10ea9eb2e`: exit 0 at the merge head, with 6
citations judged and all resolving. At the final head, `--base
10ea9eb`: exit 0, 5 of 5 resolve. `origin/main` then moved to
`a78f731ad`, and `--base origin/main` against that unmerged tip exits 2.
It counts 99 citations that #20326 removed on `main` as if this change
had added them. They sit in 21 files, none of them in this diff: a
moving-ref reading, not a finding.
## A pending release note is corrected in place, so `Check Changeset`
stays red
`check-empty-changeset` names both notes: "present on the merge base and
CHANGED by this PR". This is the **DELIBERATE CORRECTION** class. Its
remedy text reads 「do NOT restore it -- say so on the PR and get it
confirmed」 and 「this gate stays red either way」. Both notes are pending,
not yet consumed by a Version Packages PR. Each said the plain read
prunes for every caller, authors included, which this PR makes false for
`?state=draft`:
- `.changeset/20156-alternate-door-read-gates.md`, in the `/layers` /
`?layers=true` / `/diff` bullet.
- Before: "The plain read and `/published` prune for every caller,
authors included."
- After: "The plain read and `/published` prune for every caller,
authors included, except the plain read's `?state=draft`: it serves the
pending draft, a stored version, and answers as these three doors do."
- `.changeset/20156-app-author-exemption.md`, in the "Unchanged" bullet.
- Before: "Unchanged: the plain read and `/published` still prune for
every caller, authors included."
- After: "Unchanged: the plain read (its `?preview=draft` included) and
`/published` still prune for every caller, authors included. The plain
read's `?state=draft` is the exception: it serves the pending draft, a
stored version, and answers as these three doors do."
This PR's own note is `.changeset/20290-draft-read-author-exemption.md`.
`skip-changeset` is not applied, because this PR publishes.
## Acceptance notes
All readings below were taken on the real stack described above. The
pre-fix readings are a one-shot probe at `de091b50e`, since removed. Its
caller "author" holds `manage_metadata` only; "member" holds nothing;
"finance author" holds `manage_metadata` and `finance.access`. `tenancy`
is off. The app is published with `nav_leads`, `nav_finance_ledger`
(`finance.access`) and `nav_org_directory` (`requiresService:
'tenancy'`). The finance author saved a draft that adds
`nav_finance_forecast` (`finance.access`).
**Item 1: the site, and the red re-measured through REST.**
- The site is the uncached arm of `GET /meta/:type/:name`: `stateParam`
sets `state: 'draft'` on `getMetaItem`, then the gate call ran `{ arms:
'all', app: 'gate' }`.
- Before the fix, the author's `?state=draft` answered 200 with the
draft's label and navigation `[nav_leads]`. `/layers` answered
`[nav_leads, nav_finance_ledger, nav_org_directory]` on all three
layers.
**Item 2: what the draft read is.**
- At `.objectui-sha` `f8a9d0fb` (a read-only clone),
`MetadataClient.getDraft` sends `GET /meta/:type/:name?state=draft`,
adding `&package=` when scoped, and maps 404 to `null`.
- Both editors use it for their baseline, never `?preview=draft`:
- `StudioDesignSurface.tsx`: about :1759-:1767 for the app (`{ ...eff,
...appDraftBody }`), and about :1885-:1893 for a nav leaf, whose type
can be `dashboard`, `page`, `object`, `report` or `action`;
- `ResourceEditPage.tsx`: about :1015-:1051 on load, :1509-:1518 after a
draft save and :1686-:1699 after a publish.
- With no draft pending, `?state=draft` answers `404` `{ error: "No
pending draft exists for app/NAME.", code: "NO_DRAFT" }` (measured). It
does not fall back to the active version: the protocol stops before the
registry for a draft read.
**Item 3: the premise, which holds on the stop condition as written.**
- `/layers` does **not** serve the draft. `getMetaItemLayered` looks its
overlay up with `state: 'active'` only, and the draft-only
`nav_finance_forecast` was absent from every layer for every caller.
- `/diff` **does**:
- a draft save goes through `SysMetadataRepository.put` with `state:
'draft'`, which appends a full-body `sys_metadata_history` row
(measured: version 1 is the active app, version 2 the draft with 4
entries);
- before this PR, `/diff?from=0&to=2` served the author `[nav_leads,
nav_finance_ledger, nav_org_directory, nav_finance_forecast]`, the
co-author's draft whole, under ruling B's exemption on `/diff`.
- So the exemption on the draft read discloses to an author nothing a
ruled stored-version door does not already serve them whole. The test
"it widens nothing" pins that permanently.
- For a non-author the permission axis is unchanged: `nav_finance_*` is
withheld before and after. Their only change is item 4's, from
`[nav_leads]` to `[nav_leads, nav_org_directory]`, which equals what
`/layers` and `/diff` already served the member before this PR.
**Item 4: the arms.**
- Before the fix, the draft read dropped `nav_org_directory` for
**every** caller, the finance author who holds every entry's permission
included (`[nav_leads, nav_finance_ledger, nav_finance_forecast]`). That
is the same data-loss class: a save of the merged baseline deletes the
entry.
- The census shows the dashboard twin: `dashboard/ops` has its widget
`w_org_kpi` bound to `tenancy`. The design surface loads dashboards
through the same door.
- Hence `STORED_VERSION_DOOR_POLICY`. The per-deployment arms need not
stay for non-authors:
- they withhold nothing from the caller (the `MetaReadGatePolicy`
docblock);
- the draft read is not a render door, since `?preview=draft` is, and it
keeps `arms: 'all'`;
- non-authors already read the per-caller-only answer on `/layers` and
`/diff`.
- Ablation C pins the choice.
**Item 5: the dispatcher.** `packages/runtime/src/domains/meta.ts`'s
item read passes `packageId`, `organizationId` and `previewDrafts` to
`getMetaItem`, and never reads `state`. So on a host that serves only
the dispatcher, `?state=draft` answers the active item under the
rendered policy: never the draft, and never `NO_DRAFT`. It does not
follow this rule because it does not serve this door. Left alone, as
ordered, and reported for #20320's family. This is a source reading at
`bd1361ee6`, not measured through `dispatch()`.
**Item 6:** see the section above. `content/docs/ui/apps.mdx` is
corrected the same way.
**Triage note 3: another read-to-display baseline saved back in
objectui**, for the objectui seat. This is a source reading at
`f8a9d0fb`, not measured at runtime.
- `useMetadata().apps` is the list read, pruned per caller.
- Two console paths publish an app built from that list, without `mode:
'draft'`:
- `app-shell/src/hooks/useNavigationSync.ts` `saveApp`, via
`NavigationSyncEffect` when a page or dashboard is created or deleted:
`client.meta.saveItem('app', appName, { ...app, navigation: updated })`;
- `apps/console/src/pages/system/AppManagementPage.tsx`
`handleToggleActive` and `handleSetDefault`: `meta.saveItem('app',
app.name, { ...app, ... })`.
- A saving author who is withheld an entry, or an entry whose service is
off here, would publish the app without it.
- A server change here cannot reach these paths: the list read is
read-to-display by ruling B, so the client must read what it saves from
a stored-version door.
**Deviation from the claim's file surface.** The claim admits
`meta-item-read-gate.ts` "only if the policy type needs a member for the
draft read". This PR changes no member there. It edits three docblock
sentences that this change made false, and no code. The change is
declared here for confirmation.
**Observed, not in scope.** The draft reads carry no builder gate: a
member who may open an app reads its pending draft (pruned per caller)
through `?state=draft` and `?preview=draft`. ADR-0037's risk table plans
「confirm/add a builder/admin role gate on the dispatcher reads」. This PR
does not change who may read a draft: the non-author answer is unchanged
on the permission axis. It is reported to the seat.
## Not addressed here
- #20139 remains open: the bare-number query reads in `rest-server.ts`.
- #20320 remains open: the dispatcher's divergences. Item 5 above is a
candidate row for it.
- The objectui paths above remain for the objectui seat.
---
_Generated by [Claude
Code](https://claude.ai/code/session_01UYBdGBzWSrAMzpW8ah3GbP)_
---------
Co-authored-by: Claude <noreply@anthropic.com>1 parent 329ea2e commit a36a691
8 files changed
Lines changed: 406 additions & 64 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
13 | 13 | | |
14 | 14 | | |
15 | 15 | | |
16 | | - | |
| 16 | + | |
17 | 17 | | |
18 | 18 | | |
19 | 19 | | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
6 | 6 | | |
7 | 7 | | |
8 | 8 | | |
9 | | - | |
| 9 | + | |
10 | 10 | | |
11 | 11 | | |
12 | 12 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
| 1 | + | |
| 2 | + | |
| 3 | + | |
| 4 | + | |
| 5 | + | |
| 6 | + | |
| 7 | + | |
| 8 | + | |
| 9 | + | |
| 10 | + | |
| 11 | + | |
| 12 | + | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
344 | 344 | | |
345 | 345 | | |
346 | 346 | | |
347 | | - | |
| 347 | + | |
348 | 348 | | |
349 | 349 | | |
350 | 350 | | |
| |||
356 | 356 | | |
357 | 357 | | |
358 | 358 | | |
359 | | - | |
360 | | - | |
361 | | - | |
362 | | - | |
363 | | - | |
364 | | - | |
365 | | - | |
366 | | - | |
367 | | - | |
368 | | - | |
369 | | - | |
370 | | - | |
371 | | - | |
372 | | - | |
373 | | - | |
| 359 | + | |
| 360 | + | |
| 361 | + | |
| 362 | + | |
| 363 | + | |
| 364 | + | |
| 365 | + | |
| 366 | + | |
| 367 | + | |
| 368 | + | |
| 369 | + | |
| 370 | + | |
| 371 | + | |
| 372 | + | |
| 373 | + | |
| 374 | + | |
| 375 | + | |
| 376 | + | |
| 377 | + | |
374 | 378 | | |
375 | 379 | | |
376 | 380 | | |
| |||
0 commit comments