Skip to content

fix(rest): app nav prunes doc entries the caller may not read (ADR-0046 §6.7) - #20128

Merged
objectstack-fleet[bot] merged 3 commits into
mainfrom
claude/issue-19790-nav-doc-audience-prune
Sep 25, 2026
Merged

objectstack-fleet[bot] merged 3 commits into
mainfrom
claude/issue-19790-nav-doc-audience-prune

Conversation

@objectstack-fleet

Copy link
Copy Markdown
Contributor

Fixes #19790
Clause-②: no

The server's app-nav filter (filterAppForUserWithReason, packages/rest/src/rest-server.ts) gains the audience arm the maintainer ruled (ruling 5793362670, letter A). A type: 'doc' entry naming a doc the caller may not read is pruned. A book entry with no page the caller may read is pruned too. Both callers apply it: the list route GET /meta/app and the by-name route GET /meta/app/:name. It covers the top-level navigation, children and areas[].navigation. The rule is the one DocNavItemSchema already declares. Before this change only the (unshipped) objectui renderer could honour it, so every member received the entry: its label, plus the gated book or doc name.

One resolution, no second resolver

A private resolveDocsAudience(environmentId, req, books) now builds a DocsAudience: the caller, the fast path, a book lookup (declared book, else the implicit per-package book), the book's own audience gate, a per-doc reader, the readable tree, and the readable pages. Every verdict is asked of the @objectstack/spec/system helpers (audienceAllows, resolveDocAudiences, docAudienceAllows, resolveBookTree, resolveBookClaimedDocs, deriveImplicitPackageBook). Five doors are built on it: /meta/book, /meta/book/:name, /meta/doc, /meta/doc/:name and /meta/book/:name/tree, which used to hold three hand-written copies of this logic, plus the new nav arm.

  • The reads' responses are byte-identical. A one-off probe (not committed) ran BASE fa00ebf4's rest-server.ts against this branch's over 1584 requests. The matrix crossed 4 callers (anonymous, non-holder, holder, unresolvable holdings) × 3 book sets (set-gated, org and public only, none) × 3 failure modes (none, book read throws, doc read throws) × the book and doc list, item and tree routes, in both spellings, with and without ?package= and ?include=content. It found 0 diffs across statuses 200:752 401:168 403:28 404:240 500:396, with 55 distinct responses. A control (non-holder 403 vs holder 200 on the same doc) proves the probe distinguishes callers. The existing suites (rest.test.ts ADR-0046 §6.7 block, meta-audience-plural, meta-public-book-grant) stay green.

Entry semantics, measured against the resolver

  • doc alone: served iff docAudienceAllows(resolveDocAudiences(...).get(doc), caller). This is the answer /meta/doc/:name gives.
  • book alone: the named book (else the implicit per-package book) must admit the caller by its own audience. That is the tree read's 401/403. And at least one of its pages must be readable. A book's pages are the docs it claims, per resolveBookClaimedDocs, the membership resolveDocAudiences itself uses. Measured: resolveBookTree appends every doc the book does not claim as a synthetic Uncategorized group. Over an env-wide corpus, nearly every book's tree therefore holds some readable doc, and counting tree entries would never prune. The spec calls those orphans "not an authored membership claim". A pin shows the implicit ops book's tree serving crm_intro under Uncategorized while the entry is still pruned.
  • book + doc: pruned when either the book's own audience denies the caller or the doc is unreadable. A readable doc inside an unreadable book is possible, because a doc's effective audience is the union over every book claiming it (docAudienceAllows, book.zod.ts). But the entry opens the page in that book's context, whose tree read is refused (the admitsBook check in the tree route), and the entry itself names the gated book. So it is pruned.
  • Existence is left to docs/nav-target. A doc naming a doc absent from the corpus is served, because the resolver's own default for a doc it has no entry for is org. A book naming nothing resolves to an implicit book with no page, and is pruned: "no readable page" is the rule's own wording. Both are pinned.
  • Anonymous: GET /meta/app answers 401 before any read (pinned). The umbrella gate admits anonymous callers only to the book and doc reads, so there is no anonymous app nav for the arm to cover.
  • Entry-level arms carry no reason. withheld is app-level only, so the arm is a bare continue, like its neighbours. An app it empties is still served.

Fails closed

  • No gate means no doc entries. Both callers must pass a gate, or the arm drops every doc entry.
  • A thrown book read, or a thrown doc corpus read, also drops every doc entry of that response and logs a warn. The rest of the navigation is served.
  • Unresolvable holdings deny set-gated entries, through resolveAudienceCaller (ADR-0049).
  • Why the nav gate does not copy the reads' degradation. Mechanism assumption: "degrades exactly as the reads do". Measured falsified for safety: the reads swallow a thrown book read to [] (fetchAudienceBooks), which reads as "no gated book anywhere". Copying that would serve set-gated entries to every member. The nav gate reads the same data through readAudienceBooks / readDocCorpus, which report the fault, and prunes. The reads keep their answers unchanged. That fail-open is reported below as a finding.

Cost per /meta/app request (ruling point 4)

  • No doc entry in the app list: zero added reads, only a walk of the nav trees (pinned: no book read, no doc read, no holdings).
  • Otherwise, once per request whatever the app count (pinned with two apps: 1 book read, 1 doc read, 1 holdings resolution):
    • one book list read;
    • one permission-set resolution, only if some book is set-gated. The execution context is already memoised per request.
    • one doc list read, only off the fast path or when an entry names a book alone.
  • Book traversal. Off the fast path, resolveDocAudiences runs one resolveBookTree per book over the whole corpus, shared by every entry. Per entry: a doc is one map lookup. A book alone is one resolveBookClaimedDocs, which is one resolveBookTree of that book: every doc is visited once per group rule, plus one lookup per claimed page.
  • No cache. Nothing outlives the request. The fast path (authenticated, no set-gated book) is the doc list's own.

Tests

packages/rest/src/meta-app-nav-doc-audience.test.ts has 19 cases. They drive the real list, by-name and tree routes:

  • a non-reader receives no such entries, and the wire carries none of the names or labels;
  • a reader receives them;
  • a book with exactly one readable page stays, and agrees with the tree read;
  • a book whose own audience admits the member but whose every page is gated is pruned;
  • list and by-name parity, for a non-holder and a holder;
  • the requiredPermissions control;
  • the four fail-closed rows;
  • fast path and existence;
  • cost;
  • anonymous.

Evidence:

  • pnpm --filter @objectstack/rest exec vitest run --project local --maxWorkers=2 src/meta-app-nav-doc-audience.test.ts: Tests 19 passed (19).
  • @objectstack/rest full local project: Test Files 195 passed (195), Tests 3284 passed | 1 skipped (3285).
  • pnpm --filter @objectstack/rest run typecheck: exit 0, test layer 0 file(s) / 0 error(s).
  • Measured at cb800e24. The later commit f2bc219b touches only the docs page and the changeset.
  • Ablation. Committed first. The arm line was deleted through scripts/ablation-replace.mjs in wrap mode (anchor 1 to 0, blob 5c7dbf8f1015 to f0b3c3cdd872). Result: Tests 12 failed | 7 passed (19). The 12 red are every non-reader, fail-closed, parity-non-holder, fast-path-existence and two-app-cost row. The 7 green are the holder, one-readable-page, control, no-doc-entry-cost, no-book-entry and anonymous rows. The restore was proven: blob equals HEAD, and git diff HEAD is empty. A re-run is green (19/19).
  • Gates. node scripts/pm/dispatch-gates.mjs --commands derived 90 commands at f2bc219b. All exit 0. Reconciled with --ran: 90 derived, 90 run, 0 NOT-MEASURED (a derived zero, since every row recorded its exit code). Two gates first refused with PREREQUISITE NOT MET and were re-run green after building what they read: check:skill-examples after building client and client-react, and check:dual-build-cjs-loads after the workspace build.
  • Lint. pnpm lint (full union, eslint . --no-inline-config) exited 0 in 107s at f2bc219b.

Docs and changeset

  • content/docs/ui/apps.mdx: the Audience bullet now says the server applies the rule to the entry itself, including the book + doc rule, and which pages count.
  • .changeset/19790-nav-doc-audience-prune.md: @objectstack/rest patch.

Acceptance notes

  • Conflict in the dispatch. The dispatch asked the gate both to "degrade exactly as the reads do" and to "not fail open". The reads fail open on a thrown book read, so the gate follows the second and prunes (see "Fails closed").
  • Observation, not filed. Without ?package=, /meta/book/:name/tree resolves over the env-wide corpus. Every book's tree therefore lists every doc it does not claim under Uncategorized. That is the resolver's documented "nothing is ever dropped" rule, and it is why the nav arm counts claimed pages rather than tree entries.
  • Out of scope, reported for the seat, not fixed here (evidence from one-off probes, not committed):
    • (a) The docs reads fail open when the book list read throws. fetchAudienceBooks swallows the fault to [], and /meta/doc/:name swallows a thrown corpus read the same way. Probe: a non-holder's GET /meta/doc/crm_admin_runbook goes from 403 to 200 with the body, and GET /meta/doc lists it, once the book read rejects. The shipped getMetaItems rethrows every sys_metadata read failure except "unprovisioned".
    • (b) GET /meta/app/:name/layers, the deprecated ?layers=true, and GET /meta/app/:name/published serve the unfiltered app document. Probe: an authenticated member holding neither the audience nor the requiredPermissions receives both kinds of entry. /history and /diff serve stored app versions the same way; that was read from code, not driven. packages/runtime/src/domains/meta.ts's dispatcher list and item reads apply no app-nav filter where that door answers. The MCP list_metadata / describe_metadata implementations are not in this tree, so they are NOT MEASURED here.
  • [Decision] admins author Markdown docs in the console and put them on the app menu — runtime doc/book authoring surface and a doc navigation item (maintainer's stated need) #19482 (the doc variant) is the parent card and is not addressed here. The objectui renderer pruning (objectui#10188) stays as defence in depth, per the ruling.

Generated by Claude Code

@github-actions github-actions Bot added size/l documentation Improvements or additions to documentation tests tooling labels Sep 25, 2026
@github-actions

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

This PR changes 1 package(s): @objectstack/rest, touching 26 documentable anchor(s).

2 hand-written doc(s) NAME something this change touched and may need an implementation-accuracy re-verification:

  • content/docs/api/client-sdk.mdx (via getBookTree (sdk, the bare tail of client method meta.getBookTree, bound to GET /api/v1/meta/book/:name/tree), meta.getBookTree (sdk, the route ledger binds it to GET /api/v1/meta/book/:name/tree, selected by route anchor /meta/book/:name/tree))
  • content/docs/ui/apps.mdx (via filterAppForUser (symbol, a method of class RestServer), /book/:name/tree (route, bridged from symbol admitsBook — its route source's handler names it; bridged from symbol audienceBooksOf — its route source's handler names it; bridged from symbol bookNamed — its route source's handler names it; bridged from symbol readableTree — its route source's handler names it; bridged from symbol resolveDocsAudience — its route source's handler names it), /meta/book/:name/tree (route, a path literal in a comment in DocsAudience; a path literal in a comment in RestServer; a path literal in a comment in filterAppForUserWithReason; a path literal in a comment on a changed line))

⛔ 5 release-owned page(s) also name something this change touched. These are read-only:

  • content/docs/releases/implementation-status.mdx (via RestServer (symbol, a top-level class))
  • content/docs/releases/v12.mdx (via RestServer (symbol, a top-level class))
  • content/docs/releases/v14.mdx (via /book/:name/tree (route, bridged from symbol admitsBook — its route source's handler names it; bridged from symbol audienceBooksOf — its route source's handler names it; bridged from symbol bookNamed — its route source's handler names it; bridged from symbol readableTree — its route source's handler names it; bridged from symbol resolveDocsAudience — its route source's handler names it), /meta/book/:name/tree (route, a path literal in a comment in DocsAudience; a path literal in a comment in RestServer; a path literal in a comment in filterAppForUserWithReason; a path literal in a comment on a changed line))
  • content/docs/releases/v16.mdx (via RestServer (symbol, a top-level class))
  • content/docs/releases/v17/17-3.mdx (via RestServer (symbol, a top-level class))

content/docs/releases/ is RELEASE-OWNED (AGENTS.md "Documentation Guardrails"): release
notes are written centrally at release time, and a code PR that edits them is the exact PR
that guardrail exists to stop. They are still audited — read-only. If one of them is actually
wrong, file an issue or open a dedicated docs-only PR; do not edit it here.

What this run could not see
  • 1 name(s) were too generic to anchor anything (single lowercase words)
  • the SDK route bridge reached 60 of 215 client-bound route-ledger rows — the other 155 have no registrar path: tail to select them, so pages documenting THEIR client methods cannot appear above, on this or any run. Of those 155: 0 are remediable by widening that discovery convention (an in-repo file declares the path; the convention did not scan it); 55 are structural — on a ledger where NOT ONE row is declared in-repo, so no discovery change reaches them at any price; 100 are undecided (no in-repo declaration, on a ledger that has other in-repo registrars — absence and an unreadable spelling are not distinguishable here). The rows themselves: node scripts/docs-audit/affected-docs.mjs --bridge-coverage
  • a page that states a rule by its inputs shares no identifier with the emitter that implements the rule, so an emitter-only diff cannot list it — not on this run and not on any run. Measured on fix(driver-sql): emit varchar(maxLength) for a text field a declared index keys on #11430: content/docs/protocol/objectql/types.mdx documents the text-family column mapping by the ObjectQL type names it maps FROM (text / textarea / html) while the diff changed createColumn; it went unlisted, and it was the page that diff falsified, in four places. No shared token exists to detect this on, so a rule your change carries has to be re-read by hand in the pages that restate it.
  • a key NAME is not a key, so the hand re-read the line above prescribes can land on the wrong schema. The same spelling is authorable on one governed type and a [REMOVED] tombstone on another for each of active, aria, joins, objects, template, tools and version (censused on [finding] tools is a key on BOTH AgentSchema (tombstoned, dead) and SkillSchema (live, cloud-attested), so a name-based search attributes skill examples to the agent key — it produced a false stop-the-line alarm on PR #19059 #19093 over the liveness ledger's governed types, top-level keys); nothing in a search result distinguishes the two, so a grep hit on a LIVE example reads as evidence about the DEAD key. Measured on fix(spec): the agent.tools liveness row says dead — it claimed live on a key the schema tombstoned #19059: content/docs/ai/agents.mdx was reported as contradicting the agent.tools tombstone over its tools: example at :161, which is inside the defineSkill({ block opened at :155 — the page was already correct. Settle ownership by PARSING the value against both schemas, never by the name: that literal PASSES SkillSchema, and as an AgentSchema it FAILS at tools with the tombstone prescription. ⛔ These names are not the whole class — a key retired through a .strict() guidance map leaves no tombstone in the walked shape and none of them here (tool.category, live as AIToolDefinition.category).

Coarse fallback — 15 page(s) merely mention a changed package (the pre-#9192 predicate, kept for the deliberately-wide backstop): node scripts/docs-audit/affected-docs.mjs --json 226e00c038527234472ef89e7bbd6ee11da33dfd → packageMentionDocs.

Which tree this was computed on

This run read content/docs from 7ee373edfe1da10fb99433a006430370c7ff43ab — the merge of head f2bc219be0f104aaac1d4ba84d2b84c69bf6c0fc into base 226e00c038527234472ef89e7bbd6ee11da33dfd, which is what actions/checkout gives a pull_request run. Not the PR head.

A worktree cut from an older main holds a different content/docs, so re-deriving there can legitimately return a different list — that is a different tree, not a wrong row. To answer on the same tree:

# while this PR is open — GitHub drops the merge commit once it closes
git fetch origin 7ee373edfe1da10fb99433a006430370c7ff43ab && git checkout 7ee373edfe1da10fb99433a006430370c7ff43ab
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin 226e00c038527234472ef89e7bbd6ee11da33dfd f2bc219be0f104aaac1d4ba84d2b84c69bf6c0fc && git checkout -B drift-repro 226e00c038527234472ef89e7bbd6ee11da33dfd && git merge --no-ff f2bc219be0f104aaac1d4ba84d2b84c69bf6c0fc

node scripts/docs-audit/affected-docs.mjs --json 226e00c038527234472ef89e7bbd6ee11da33dfd

⚠️ That checkout carried uncommitted changes, so the commit above does not fully identify what was read.

Advisory only, and a precision-first one (#9192): a page is listed because it names a
symbol, wire route or SDK method this diff touched — not because it mentions a changed
package. Each row says which anchor put it there, so a wrong row is reportable rather than
merely annoying. To re-verify, run the docs-accuracy-audit workflow scoped to these files:
node scripts/docs-audit/affected-docs.mjs 226e00c038527234472ef89e7bbd6ee11da33dfd → pass the list as
args.docs, on the commit named under Which tree this was computed on.

@objectstack-fleet
objectstack-fleet Bot marked this pull request as ready for review September 25, 2026 07:08
@objectstack-fleet
objectstack-fleet Bot added this pull request to the merge queue Sep 25, 2026
Merged via the queue into main with commit bc80e16 Sep 25, 2026
37 checks passed
@objectstack-fleet
objectstack-fleet Bot deleted the claude/issue-19790-nav-doc-audience-prune branch September 25, 2026 07:24
os-steve pushed a commit that referenced this pull request Sep 25, 2026
Brings in PR #20128 (app nav doc-audience prune), which landed on
rest-server.ts after this branch was cut. No conflict.

Claude-Session: https://claude.ai/code/session_01TnPAC1UsTGfHPXVUCL6iLn
Co-authored-by: Claude <noreply@anthropic.com>
akarma-synetal pushed a commit to akarma-synetal/framework that referenced this pull request Sep 28, 2026
…rt, meta history and search (objectstack-ai#20061, objectstack-ai#20062) (objectstack-ai#20137)

Fixes objectstack-ai#20061
Fixes objectstack-ai#20062
Clause-②: no (narrowing)

## What

Four published REST doors read `?limit=` with a bare `Number()`. They
substituted, clamped or dropped the result and answered `200`:
- `GET /data/import/jobs`
- `GET /data/:object/export`
- `GET /meta/:type/:name/history`
- `GET /search`

Each door now reads `limit` against its own declaration and refuses a
value outside it. `GET /data/import/jobs` does the same for `offset`,
because its declaration bounds `offset` too. The refusal is `400` with
the data surface's existing `VALIDATION_FAILED` + `fields[]` envelope,
and the service is never called. Nothing a conforming caller sends
changes its answer.

The change is one private reader in `packages/rest/src/rest-server.ts`,
`readDeclaredQueryNumber`, with no new package export:
- **import jobs** read through `ListImportJobsRequestSchema.shape.limit`
/ `.offset`;
- **history** reads through `HistoryMetaItemRequestSchema.shape.limit`;
- **export** and **search** declare no request schema, so they read
through a module-private `z.number().int().optional()`: a whole number,
nothing about range.

A non-blank numeric string is coerced with `Number()` and handed to the
schema. Anything else (`abc`, a blank string, a structured value) goes
to the schema as it came, so the declaration refuses it by type.
`Number()` never gets to invent a `0` or a `NaN` for it.

## Before and after, measured on the real `RestServer` routes (spy
protocol)

Every row answered `200` before this PR.

| door | request | answered before | answers now |
|:--|:--|:--|:--|
| import jobs | `?limit=0` / `?limit=-3` | limit 50 / 1 | `400`,
`fields[0]` = `limit` / `min_value` |
| import jobs | `?limit=201` / `?limit=500` | limit 200 | `400`,
`max_value` |
| import jobs | `?limit=abc` / `1.5` / `Infinity` / blank | limit 50 /
1.5 / 200 / 50 | `400`, `invalid_type` |
| import jobs | `?offset=-1` / `abc` / `1.5` | offset 0 / 0 / 1.5 |
`400`, `min_value` / `invalid_type` |
| export | `?limit=abc` / empty / blank | `X-Export-Limit: 1` (a one-row
export) | `400`, `invalid_type` |
| export | `?limit=1.5` / `Infinity` | `X-Export-Limit: 1.5` / `50000` |
`400`, `invalid_type` |
| meta history | `?limit=abc` / `Infinity` | limit dropped: the whole
change log | `400`, `invalid_type` |
| meta history | `?limit=` (empty) / blank | limit 0: zero events |
`400`, `invalid_type` |
| search | `?limit=abc` | `limit: NaN` handed to `searchAll`: no overall
cap | `400`, `invalid_type` |
| search | `?limit=1.5` / `Infinity` / blank | 1.5 / clamped to 100 / 0
| `400`, `invalid_type` |

Unchanged and pinned by the lit controls:
- **Import jobs:** absent or empty `limit` → 50, absent or empty
`offset` → 0. Conforming values reach `findData` unchanged.
- **Export:** absent → 10000. `?limit=25` → 25. The door's own range
handling is untouched: `?limit=0` → 1 and `?limit=60000` → 50000.
- **History:** absent → no `limit` member. `5`, `0` and `1.5` are
forwarded unchanged, because the declaration is `z.number()` with no
`int()` and no bounds.
- **Search:** absent or empty → `undefined`. `20`, `0` and `500` reach
`searchAll` unchanged, and its `[1, 100]` clamp is its own business.

The refusal body carries:
- `code: 'VALIDATION_FAILED'`;
- `fields: [{ field: 'limit', code, message }]`;
- an `error` sentence naming the parameter;
- `object`, on the export door only. The handler throws
`validationFailure` (`@objectstack/types`), and each door's existing
catch (`handleRouteError` / `mapDataError`) writes the 400. So
`rest-server.ts` gains no response write site, and
`check:route-envelope`'s `siblingCode` count on it is unchanged (green).

## PM mechanism assumptions, measured

1. **The `parseIntegerParam` cycle: confirmed.**
`packages/runtime/package.json` depends on `@objectstack/rest`. rest's
dependencies are `core`, `metadata-core`, `observability`,
`platform-objects`, `service-package`, `spec`, `types` (plus `exceljs`,
`zod`). Nothing was moved; `packages/runtime/**` and `packages/spec/**`
are untouched.
2. **The declared-schema idiom holds, with one measured difference in
`fields[].code`.** zod 4.6.1 against the declared schemas, mapped
through `zodIssuesToFields`:
- `NaN`, a string, `Infinity`, and `1.5` against `int()` all give
`invalid_type`;
   - below `min` gives `min_value`; above `max` gives `max_value`.
These are all ADR-0114 catalog members, never zod's own codes. Runtime's
`parseIntegerParam` answers `invalid_number` for the same `abc` / `1.5`;
see the open question in the report.
3. **The doors match the card, with two corrections.**
- Export serves no declared request schema.
`CreateExportJobRequestSchema` is the contract of `POST
/api/v1/data/:object/export`, which rest does not mount.
`ExportRequestSchema` (`contract.zod.ts`) is bound to no route.
- The search door holds: the only production caller of `searchAll` is
the REST door (`git grep`, tests excluded). So
`packages/metadata-protocol/src/protocol.ts` is not in this diff.
4. **The empty-string premise is partly falsified.** Only import jobs
and search answered `?limit=` with their default. Export answered a
one-row export (`Number('') || 0` → `Math.max(1, 0)`), and history
answered zero events (`Number('')` = 0, forwarded). The rule applied is
the one `parseEnumParam` (runtime `query-param.ts`) states for the same
spelling:
- Where the old answer already was the absent answer, empty stays
absent.
   - Where the old answer was an invented `0`, empty is refused.
Reading it as absent on export and history would have turned those two
into a 10000-row export and the whole change log. That grows the window,
the widening this claim stops on, so the reader carries an explicit
per-door `emptyIsAbsent`.

No door accepts anything it refused before: none refused any
single-valued `limit` before this PR. No conforming value's answer
changes either, so the declaration stays `no (narrowing)`.

## Tests

The new file is
`packages/rest/src/rest-server-limit-param-parsing.test.ts` (43 cases).
Each refusal pins `status` 400, `code` `VALIDATION_FAILED`,
`fields[0].field` and `fields[0].code`, and that the service spy was
never called. Each lit control pins the argument the service received.
- Against unfixed `6780e34a`: 24 failed (every refusal row answered
200), 19 passed (every lit control).
- Against the fix: 43 passed.

One existing fixture was corrected in
`rest-server-closed-query-params.test.ts`. Its search closed-set loop
sent `limit: 'x'` and expected 200; it now takes a valid value per name,
as its export twin in the same file already does.

Runs at head `74898ed5`, after merging `origin/main`:
- **Rest suite:** `pnpm --filter @objectstack/rest exec vitest run
--project local --maxWorkers=2` → `Test Files 196 passed (196)`, `Tests
3327 passed | 1 skipped (3328)`.
- **Typecheck:** `pnpm --filter @objectstack/rest run typecheck` → exit
0. `tsc --noEmit` passes, and `check:test-typecheck: OK — ... 0 file(s)
/ 0 error(s)`.

Ablations went through `scripts/ablation-replace.mjs` in WRAP mode, run
after the fix was committed. Vitest reads `src/` through the relative
import, so no dist was involved.
- **Import jobs:** restoring `Math.min(200, Math.max(1, Number(q.limit)
|| 50))` landed on disk (anchor 1 → 0, blob `02ceccd1` → `cab07300`).
Result: `Tests 8 failed | 35 passed`, exactly the 8 import-jobs `limit`
refusal rows.
- **Search:** restoring `req.query?.limit ? Number(req.query.limit) :
undefined` landed (blob `02ceccd1` → `9b589c00`). Result: `Tests 4
failed | 39 passed`, exactly the 4 search refusal rows.
- Both restores were proven: blob == HEAD `02ceccd1`, `git diff HEAD`
empty.

Public surface: rest's `dist/index.d.ts` and `dist/index.d.cts`, built
from merge-base `bc80e162`'s `rest-server.ts` and from HEAD's, are
byte-identical (`cmp`). The restore was proven by blob hash. So no
importing package's types can move, and only rest's own tests are owed.

## Gates

`node scripts/pm/dispatch-gates.mjs --commands --repo
objectstack-ai/objectstack` on the real diff gives 61 commands,
identical to the PM's list. All ran at `74898ed5` with exit codes
captured before any pipe. `--ran` reconciliation: `61 derived, 59 run, 2
NOT-MEASURED, 0 UNRUN`.
- **59 exit 0.** This includes `check:adr-0087-registration --base
origin/main` (`not-required (no-migration-prescription)` accepted),
`check:changeset-no-major`, `check:empty-changeset`,
`check:route-envelope`, `check:nul-bytes`, `check:doc-authoring`,
`check:published-files`, `check:issue-citations` and
`check:undeclared-dep-imports`.
- **NOT MEASURED: `check:dual-build-cjs-loads`**, exit 3 `PREREQUISITE
NOT MET`: 43 workspace packages have no `dist/`. Declared narrowing:
`require('./dist/index.cjs')` and `import('@objectstack/rest')` both
load and expose `RestServer`.
- **NOT MEASURED: `check:type-check-debt`**, exit 3 `PREREQUISITE NOT
MET`: the full closure is not built. See the `.d.ts` identity above; CI
builds the closure before this step.
- **`pnpm lint`** (the lane addition) ran as the full union, `eslint .
--no-inline-config`, at `74898ed5`: exit 0.

## First-party callers, measured

- `@objectstack/client` sends `limit` as written: `data.listImportJobs`,
`data.export`, `meta.getHistory`, `search`.
- objectui at its pin `f8a9d0fb`: `ImportWizard` sends `limit: 50`,
`ResourceHistoryPage` sends `limit: 100`, and the search and export
emitters send `limit` only when it is a number greater than 0. None
sends a value this PR refuses.

## Acceptance notes

- **Same-family reads left out of this diff.** Triage's note 3 keeps
them out, and the file is also held by another claim (PR objectstack-ai#20125).
  - Measured on the real routes with a spy protocol, each answering 200:
- `GET /meta/:type/:name/history?sinceSeq=abc` drops `sinceSeq` and
reads the log from the start (`rest-server.ts:8139`);
- `GET /meta/:type/:name/audit?limit=abc` drops `limit`, so the
producer's default 100 is served (`:8318`; declared `z.number()`, which
refuses `NaN`);
- `GET /search?perObject=abc` hands `perObject: NaN` to `searchAll`
(`:10709`).
- Read only, not measured: `GET /data/approvals/requests` `limit` /
`offset` drop a non-numeric value and serve the unpaged list (`:13601`).
Export `?page=` falls back to the 500-row chunk (`:10297`), which
changes chunking only, not the rows returned.
- **History's declared `limit` admits `1.5`, `0` and negatives.** The
door forwards them as before. Whether
`HistoryMetaItemRequestSchema.limit` should declare `int()` / `min()` is
the spec seat's question, and this PR takes no position.
- **Export and search range stay as they were:** the export floor of 1
and cap of 50000, and search's `[1, 100]` clamp. Both cards take no
position on bounds.
- **`GET /data/import/jobs?status=` is not checked against the declared
`ImportJobStatus`.** Read only, not measured, out of scope.
- **Merge:** `origin/main` was merged (no rebase) after PR objectstack-ai#20128 landed
on `rest-server.ts`, with no conflict. PR objectstack-ai#20125's hunks do not overlap
these doors.
- **CI-owned, not run locally:** the full `Test Core` shards, `Dogfood`,
`Build Core`, `Temporal Conformance`, and the workspace type-check
lanes.

---
_Generated by [Claude
Code](https://claude.ai/code/session_01TnPAC1UsTGfHPXVUCL6iLn)_

---------

Co-authored-by: Claude <noreply@anthropic.com>
@objectstack-fleet

Copy link
Copy Markdown
Contributor Author

Face review (post-hoc, released)

Served-tier: CONTRACT_REVIEW_TIER
Merge-sha: bc80e16260597d52b062b7e1ee810c7b9a99aed2
Local-runs: none

Scope: the changeset (and named docs) of PR #20128, released; this review cannot block anything and exists to find false released prose.

Sentence verdicts

Changeset .changeset/19790-nav-doc-audience-prune.md at the merge:

  1. "DocNavItemSchema declares that a doc entry the member may not read is not rendered, and that a book entry is not rendered for a member with no readable page in it." TRUE. packages/spec/src/ui/app.zod.ts:~660-685 (Audience paragraph of the docblock).
  2. "Until now only a renderer could honour that. The server's app-nav filter pruned on requiredPermissions, requiresService and object servability, so every member of the app got the entry." TRUE. The pre-change filterAppForUserWithReason arms (packages/rest/src/rest-server.ts:3476-3478, servability arm below) had no audience arm; [Decision] should the server-side app-nav filter also prune a doc menu entry by the docs audience, so a member who cannot read the doc never receives the entry — or is renderer-side pruning (objectui#10188) the whole answer? #19790's body measured the same.
  3. "The filter now applies the docs audience (ADR-0046 §6.7) on both the list route (GET /meta/app) and the by-name route (GET /meta/app/:name), in the top-level navigation, inside children and inside areas[].navigation." TRUE. Arm at rest-server.ts:~3493; filterNav recurses into children (:~3560-3567) and filterAreas walks areas[].navigation (:~3571-3583); list route :~6344-6349, by-name route :~7364-7369; pinned by meta-app-nav-doc-audience.test.ts:211 (area included) and :246-260 (by-name equals list).
  4. "doc: the entry is dropped when the doc's effective audience does not admit the caller. This is the answer GET /meta/doc/:name gives." TRUE. resolveNavDocAudience returns canRead(docName) built from the same DocsAudience.docReader the doc reads use (:~2861-2905, :~3890); docAudienceAllows treats an unresolved doc as org (packages/spec/src/system/book.zod.ts:496), same as the by-name read.
  5. "book: the entry is dropped when the book's own audience does not admit the caller, or when none of its pages is readable. A book's pages are the docs its groups claim. The Uncategorized group that the book tree adds does not count." TRUE. admitsBook then readablePages over resolveBookClaimedDocs (:~3886-3888, :~2898-2901); pinned at test :222-244.
  6. "book + doc: the entry is dropped when either of those checks fails." OVERSTATED in one corner. For a book+doc entry the code asks the book's own audience and the doc's readability (:~3884-3890); the book's readable-page count is not asked. The two coincide whenever the named doc is a page of the named book; they diverge only for an entry naming a readable doc the book does not claim, which the code serves and this sentence says is dropped. The docs page states the rule precisely (apps.mdx sentence 2 below), and the test pins the served-when-book-gated case (:193). Precise wording: "dropped when the book's own audience does not admit the caller or the doc is not readable".
  7. "An app emptied by the prune is still served ... An emptied group or area collapses, as with every other gate." TRUE. :~3562 (empty group dropped), :~3579 (area with emptied navigation dropped); an app is withheld only on _unpublished, app-level requiredPermissions or requiresService (:3455-3467); test :204.
  8. "The verdicts come from the same resolution that /meta/doc, /meta/doc/:name, /meta/book and /meta/book/:name/tree now share. What those reads return is unchanged." TRUE. All five callers go through resolveDocsAudience (:~6482, :~6499, :~6852, :~7339, :~7348); each refactored branch computes the same predicate it did before (fast path authenticated && no set-gated book, per-doc union, implicit book by name).
  9. "Fails closed. If the book or doc list read throws while /meta/app is being answered, every doc entry is left out of that one response and a warning is logged. The rest of the navigation is still served. If the caller's permission-set holdings cannot be resolved, set-gated entries are dropped, as set-gated content already is." TRUE. readMetaList reports { fault } (:~2810-2818); failClosed logs and returns () => false (:~3860-3869); resolveAudienceCaller returns no holdings on a failed resolution and audienceAllows then denies set-gated audiences (:2566-2596); tests :276-306.
  10. "Cost. An app list with no doc entry performs no extra read. Otherwise each request adds one book list read, one permission-set resolution when some book is set-gated, and one doc list read when a set-gated book exists or an entry names only a book. Each of these happens once per request, however many apps are listed. Nothing is cached across requests." TRUE. :~3856-3883 (early return on zero entries; one books read; holdings only when gated; corpus only when !allReadable || some bookAlone); gate built once per request at both routes; tests :342-363.

Docs content/docs/ui/apps.mdx:223-235 at the merge:

  1. "The server applies this rule to the entry itself: /meta/app leaves such an entry out of the app it returns, so its label and its book or doc name never reach a member who cannot open it." TRUE (arm at :~3493; test :184 reads the wire for gated names and labels).
  2. "An item with both book and doc is also left out when the member may not open that book, even if they may read the page." TRUE (test :193).
  3. "A book's pages are the docs its groups claim; the Uncategorized group the book tree adds does not count." TRUE (readablePages over resolveBookClaimedDocs).
  4. Retained sentences ("The page content itself is gated on the server by the /meta/doc and /meta/book/:name/tree reads. visible and requiredPermissions still apply and can only narrow further."). TRUE, unchanged.

Still true on origin/main?

Yes, with one relocation worth naming. The gate moved out of rest-server.ts into packages/rest/src/meta-item-read-gate.ts (:385 type, :795 arm, :1168 resolveNavDocAudience, :1374 and :1545 the two call sites) by 2bcd5cfeb7 (#20236) and cc40033ed4 (#20319), so the dispatcher's /meta reads share it; behaviour on the two routes the changeset names is intact. A later author-exempt policy (meta-item-read-gate.ts:275-300, ruling 5856774816, #20156 / #20290) serves the stored app unpruned to a caller who may write it, but only on the stored-version doors (/layers, ?layers=, /diff, ?state=draft), not on GET /meta/app or the plain GET /meta/app/:name, so nothing in the changeset or the docs page became false. content/docs/ui/apps.mdx:224-236 on origin/main carries the reviewed sentences unchanged. packages/rest/CHANGELOG.md:1450 carries the reviewed changeset verbatim under bc80e16.

Finding

NONE at the correction bar. The one overstatement (changeset sentence 6, the book + doc corner where the named doc is readable but not a page of the named book) promises a stricter prune than shipped for an entry shape docs/nav-target does not produce in practice; the docs site already states the precise rule. If the seat wants the CHANGELOG exact, the one-line wording is given under sentence 6.

Reviewed-by: session_01VvcEokUG1tvVxkceYfR5XB

VERDICT: CLEAN


Generated by Claude Code

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation Improvements or additions to documentation size/l tests tooling

Projects

None yet

2 participants