From e4b0a30be96882c73b291dd80a13cf837c4c7253 Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 12 Sep 2026 02:16:56 +0000 Subject: [PATCH 1/3] fix(data-objectstack): apply the object-metadata write invariant at the doors, not the writers objectui#7714 ruled one client behaviour -- a half-filled relationship stays client-side and the PUT body never carries one without a non-empty `reference` -- and its PR implemented that ruling by ENUMERATING the writers it knew of. Two. objectui#8057 then reproduced the identical defect on a third writer in that card's own required dogfood, and a sweep found nine more. The half that outlives the count: the sweep the question is naturally asked in cannot see its own subject, because the call is `client.save(type, ...)` and a hand-rolled `fetch` PUT is not that spelling at all. So the invariant moves off the writers, which are an OPEN set nobody has to announce a member of, and onto the DOORS, which are a CLOSED set this repo owns. Three in-repo transports can PUT /meta/:type/:name; all three now apply the same assertion, so every writer is covered with no list existing anywhere. - `assertObjectMetadataWritable` in @object-ui/data-objectstack, applied by `MetadataClient.save` before the request, by `importObjectDraft`'s hand-rolled PUT, and by MetadataService's one SDK seam. - `scripts/check-object-metadata-write-doors.mjs` derives the door set on every run -- resolving each URL through templates, fields and helper returns, because the central door says nothing at its own call site -- and fails when an object-capable door does not reach the guard. - The relationship-type list, previously declared word-for-word in two writers, is now one declaration with a pin that DERIVES it from the installed @objectstack/spec. Part of #8676 Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01UzHd6hDYatoDn17BuwKxnZ --- .../8676-object-metadata-write-doors.md | 25 + .github/workflows/ci.yml | 20 + content/docs/guide/ci-cd-pipeline.md | 2 +- package.json | 1 + .../app-shell/src/services/MetadataService.ts | 75 ++- .../src/views/metadata-admin/external/api.ts | 6 + packages/data-objectstack/src/index.ts | 11 + .../metadata-client.objectWriteGuard.test.ts | 98 ++++ .../data-objectstack/src/metadata-client.ts | 9 + ...ct-metadata-write-guard.derivation.test.ts | 95 ++++ .../src/object-metadata-write-guard.test.ts | 152 ++++++ .../src/object-metadata-write-guard.ts | 168 ++++++ .../src/MetadataFieldsPage.tsx | 13 +- .../check-object-metadata-write-doors.test.ts | 280 ++++++++++ scripts/check-object-metadata-write-doors.mjs | 509 ++++++++++++++++++ 15 files changed, 1438 insertions(+), 26 deletions(-) create mode 100644 .changeset/8676-object-metadata-write-doors.md create mode 100644 packages/data-objectstack/src/metadata-client.objectWriteGuard.test.ts create mode 100644 packages/data-objectstack/src/object-metadata-write-guard.derivation.test.ts create mode 100644 packages/data-objectstack/src/object-metadata-write-guard.test.ts create mode 100644 packages/data-objectstack/src/object-metadata-write-guard.ts create mode 100644 scripts/__tests__/check-object-metadata-write-doors.test.ts create mode 100644 scripts/check-object-metadata-write-doors.mjs diff --git a/.changeset/8676-object-metadata-write-doors.md b/.changeset/8676-object-metadata-write-doors.md new file mode 100644 index 0000000000..e6ebb6e333 --- /dev/null +++ b/.changeset/8676-object-metadata-write-doors.md @@ -0,0 +1,25 @@ +--- +'@object-ui/data-objectstack': minor +'@object-ui/app-shell': minor +'@object-ui/plugin-designer': minor +--- + +Apply the object-metadata write invariant at the write DOORS instead of at the writers. + +objectui#7714 ruled that a half-filled relationship stays client-side and the PUT body never +carries one without a non-empty `reference`, and implemented that ruling by naming the two +writers it knew of. objectui#8057 reproduced the identical defect on a third; a sweep found +nine more. The doors — the three places in this repo that actually PUT `/meta/:type/:name` — +now apply the invariant themselves, so every writer is covered without any list of writers +existing anywhere, and a new door is caught by a gate that derives the door set from the +tree rather than restating it. + +Behaviour change for consumers: `MetadataClient.save('object', …)` now throws BEFORE issuing +the request when the body carries a relationship field with a missing, empty or whitespace-only +`reference`. The same document is refused by the server with a 422 on `fields.NAME.reference`, +so nothing that previously succeeded now fails — the refusal moves earlier, names the field, +and leaves the draft in the client instead of wedging every later save of that object. Writes +of every other metadata type are untouched. + +New export from `@object-ui/data-objectstack`: `assertObjectMetadataWritable`, +`RELATIONSHIP_TYPES_REQUIRING_REFERENCE` and `OBJECT_METADATA_TYPE`. diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 4ab8c3dbb2..8ad5867084 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -334,6 +334,26 @@ jobs: if: steps.relevant.outputs.should_run == 'true' run: pnpm check:handler-key-reads + # objectui#7714 ruled one client behaviour and its PR implemented that ruling + # by ENUMERATING the writers it knew of. Two. objectui#8057 hit the same + # defect on a third in that card's own required dogfood, and objectui#8676's + # sweep found nine more — and, the half that outlives the count, the sweep + # shape the question is naturally asked in (`client.save(`) CANNOT SEE them: + # it returns zero over the very file objectui#8057 is about, because the call + # is `client.save(type, ...)`. A hand list of writers is stale the next + # time somebody adds one, and nothing says so. + # + # So this step enumerates DOORS, not writers. The writer set is open and + # nobody has to announce a new member; the transport set is closed and this + # repo owns it. It derives every call that PUTs `/meta/:type/:name` — + # resolving the URL through templates, fields and helper returns, because + # the central door says nothing at its own call site — and requires each one + # that can carry an object document to reach the guard. Parses sources with + # `typescript`, so it needs the install and nothing built. + - name: Verify every object-metadata write door applies the write guard + if: steps.relevant.outputs.should_run == 'true' + run: pnpm check:metadata-write-doors + # A build tsconfig that excludes tooling by FILE NAME (`*.test.ts`) stops # the files that happen to be named that way and nothing else. The first # shared helper added to a `__tests__/` directory is then a program input, diff --git a/content/docs/guide/ci-cd-pipeline.md b/content/docs/guide/ci-cd-pipeline.md index bb1fa2b9d6..c551f91d89 100644 --- a/content/docs/guide/ci-cd-pipeline.md +++ b/content/docs/guide/ci-cd-pipeline.md @@ -217,7 +217,7 @@ it green — which is how two of `type-check`'s gates came to be missing from th | Job key | Appears as | What it runs | When | |---|---|---|---| | `changeset-check` | Changeset Fixed Group Check | `scripts/check-changeset-fixed.mjs` — every workspace package must be in the changeset `fixed` group or explicitly ignored. It checks group *membership*; it does **not** check whether the PR added a changeset. | Every run | -| `type-check` | Type Check | `scripts/check-type-check-coverage.mjs`, then `pnpm check:phantom-deps`, then `pnpm check:self-import`, then `pnpm check:unreferenced-sources`, then `pnpm check:doc-example-readers`, then `pnpm check:handler-key-reads`, then `pnpm check:published-tsconfig-exclude`, then `pnpm check:side-effects-array`, then `pnpm check:element-data-source-declaration`, then `pnpm check:esm-specifiers`, then `pnpm check:spec-symbols`, then `pnpm check:action-forward-parity`, then `pnpm check:designer-field-key-parity`, then `pnpm check:icon-record-names`, then `pnpm check:i18n-keys`, then `pnpm check:i18n-drift`, then `pnpm check:i18n-designer-parity`, then `pnpm type-check:scripts`, then `pnpm type-check:vitest-config`, then `pnpm type-check`, then `pnpm type-check:vitest-setup`. The coverage guard runs first because turbo silently skips packages that have no `type-check` script, so a package without one would otherwise read as passing (#2911). `pnpm check:phantom-deps` fails when a released package imports a bare specifier its own `package.json` does not declare — a *phantom dependency*, invisible locally because the workspace root's `devDependencies` sit on the upward resolution path from every package directory and on no consumer's, so `require.resolve('react', { paths: ['packages/core/src'] })` succeeds while `@object-ui/core` declares react in no field at all ([#4394](https://github.com/objectstack-ai/objectui/issues/4394)). `pnpm check:self-import` runs next because it reuses that gate's parser: it fails when a file inside a package names its OWN package, a specifier that resolves through the package's `exports` map to `dist/` while `type-check` waits on `^build` — the *dependencies'* builds, never the package's own — so on a cold cache the declarations do not exist yet and the file fails with `TS2307`. Locally it is always green, because every local workflow builds before it type-checks and leaves a `dist/` behind; PR #4789's first run was red on exactly one such line ([#4801](https://github.com/objectstack-ai/objectui/issues/4801)). `pnpm check:unreferenced-sources` runs next, reusing the same parser again: it fails when a covered package ships a source file that nothing reaches — not the package's declared entry, and not its build config. Until [#7515](https://github.com/objectstack-ai/objectui/issues/7515) no gate here could see one: `check-dist-completeness` asks whether `dist/` holds what `tsc` emits, `check-readme-exports` compares documented exports against shipped ones, and a file that is in the tarball while being reachable from nothing is outside both — so the detection mechanism was a human reading unrelated code, which is how both instances found in one week were found ([#7319](https://github.com/objectstack-ai/objectui/issues/7319), [#7397](https://github.com/objectstack-ai/objectui/issues/7397)). The hazard is not the bytes: the file #7319 removed carried the same export name as a live engine one package over and evaluated no predicate, so name-completion alone could have wired a silently wrong renderer into a published package. Reachability has TWO roots, and the second is the whole difficulty — `packages/components` reaches its two `use-sync-external-store` shims only through `vite.config.ts` `resolve.alias` entries whose importer is a bundled dependency no source file names, so a walk that skips that leg reports exactly those two live files as dead on its first run, and a gate that cries wolf gets switched off rather than fixed. Scope is DECLARED per package in `COVERED_PACKAGES` and the uncovered remainder is printed as a count derived from the workspace on every run, because the alias mechanisms differ per package and a gate that covers one package correctly beats one that covers forty with false positives. An alias expression it cannot evaluate is a FINDING rather than a skip, since skipping one would make it accuse whatever file that alias points at. `pnpm check:doc-example-readers` runs next, on the same parser again: it fails when an exported symbol's own JSDoc `@example` hand-spells a resolution that its REAL call sites obtain by calling a shared reader. A doc comment is what the next call site is copied from, so prose that outlives the ruling it encoded re-seeds every later copy — measured at two cards and three copied call sites ([#7627](https://github.com/objectstack-ai/objectui/issues/7627), [#7638](https://github.com/objectstack-ai/objectui/issues/7638)), both closed by pointing the prose at `resolveRecordSourceObjectName`. Nothing here could see either one, and `check-spec-symbol-derivation` was credited with the class twice — in #7638's card body and then in the dispatch that repeated it — while its rule 4 judges `@objectstack/spec` citations at member granularity and says nothing about prose prescribing a LOCAL spelling ([#7652](https://github.com/objectstack-ai/objectui/issues/7652)). It fires on four conditions at once — the example calls the symbol it documents, a real call site fills the same argument slot by calling an exported single-`return` reader, the example does not, and what the example writes there is that reader's own return expression or one of the rungs it resolves between — which is what keeps it off the literals and placeholders an example legitimately carries. It does NOT judge whether a prescribed spelling is correct: on the day either card was filed the prose and every copy of it agreed, and no gate reading only the tree can know a ruling. What it catches is the state right after, when the call sites move and the prose does not. `pnpm check:handler-key-reads` runs next, on the same parser again: it fails when an `on*` handler key that a REGISTERED renderer reads off the authored document is not a declared member of the zod arm for the type it is registered under. `BaseSchema` is `.passthrough()`, so an undeclared key is not refused — it stops being judged and the value is KEPT, then reaches the renderer that reads it; measured on the built dist, `{ type: 'kanban', columns: [], onCardClick: { action: 'toast' } }` went from REFUSED to ACCEPTED with the object surviving into the parsed output ([#7664](https://github.com/objectstack-ai/objectui/issues/7664)). Every gate stayed green, because the [#6124](https://github.com/objectstack-ai/objectui/issues/6124) ledger's population is two hand-written arrays of tuples and that change re-keyed the arm by SUBSTITUTION — so its length assertion held, and a count ratchet would have been green too, which is why [#7753](https://github.com/objectstack-ai/objectui/issues/7753) rejected that option on the instance itself. This gate derives BOTH populations: the arms from every `type: z.literal(…)` in `packages/types/src/zod`, and the read sites from every real `ComponentRegistry.register(…)` call — read off the AST, because one types file NAMES that call in prose eleven times and registers nothing. It follows the document one component at a time rather than every JSX child, because most children are handed a DIFFERENT document (a dashboard's widgets each get their own), and the chain it must reach is four hops long: `register('kanban', ObjectKanbanRenderer)` names a component, that component is an HOC, the document arrives at `ObjectKanban` through a render-prop parameter and at `KanbanRenderer` through an object spread. It says nothing about keys that reach a renderer only through a `{...props}` spread onto a Radix root or a DOM listener slot — there is no read site to derive from — nor about the ledger's `?: never` tombstones, which have no read site by construction; `KNOWN_UNDECLARED_READS` is an exemption list that only shrinks, each row naming the card that owns the fix, and a row whose read site the gate can no longer find fails it. It lives in `scripts/` because the read sites are spread across `@object-ui/plugin-*` and `packages/components`, which `@object-ui/types` may not import — `check:phantom-deps` rejects it and it would close a cycle. `pnpm check:published-tsconfig-exclude` follows, config reads only: it fails when a published package's build `tsconfig.json` excludes tooling by FILE NAME (`*.test.ts`) without also excluding the tooling DIRECTORIES (`**/__tests__/**` and its two siblings, derived from `TOOLING_FILE` rather than retyped). A name-only exclude stops the files that happen to be named that way and nothing else, so the first shared helper added to a `__tests__/` directory becomes a program input and an emitting program writes it into the published `dist` — three times so far, each found by a human and never by a gate ([#4006](https://github.com/objectstack-ai/objectui/issues/4006), [#4836](https://github.com/objectstack-ai/objectui/issues/4836), [#6943](https://github.com/objectstack-ai/objectui/issues/6943), the third in the same package as the first). [#7212](https://github.com/objectstack-ai/objectui/issues/7212) measured the standing exposure — 29 published packages carrying the name form with ZERO offending files, green because nobody had added such a helper yet — and the gate landed together with their conversion so `main` was green on merge. It reads `exclude` arrays and nothing else: no build, no artifact, no emit model, which is the narrower scope that keeps it clear of the modelling [#4846](https://github.com/objectstack-ai/objectui/issues/4846) declined for the artifact-level gate. Six published packages are named carve-outs, each re-proving its own reason on every run: `cli`, `create-plugin` and `data-objectstack` emit from a `tsup` entry graph, `plugin-charts` keeps its tooling exclude in the `dts()` options, and `console` and `runner` are Vite applications with `noEmit: true` and no `dts()` plugin. `pnpm check:side-effects-array` runs next, sources only and no build: it fails when a package's `sideEffects` ARRAY and its module bodies disagree in either direction — a module that registers something at load time and is not named (a bundler drops it, and the registration is gone from a *consumer's* app with no error, no warning and exit 0), or a name whose module no longer registers anything. `@object-ui/app-shell` declares such an array because both simpler answers are measurably wrong for it: omitting the field makes the whole package unshakeable, and `"sideEffects": false` silently drops three live SDUI widget registrations to zero chunks ([#6535](https://github.com/objectstack-ai/objectui/issues/6535), [#6683](https://github.com/objectstack-ai/objectui/issues/6683)). The enumeration is re-derived from the module bodies on every run rather than listed, so there is no second copy to rot. The artifact half of the same contract — do those registrations survive a real bundler — cannot run in this job at all: it needs a built console, so it lives in the SDUI registration pin step of `performance-budget.yml`. `pnpm check:element-data-source-declaration` runs next, sources only and no build: it fails when a source that consumes `ElementDataSourceGate` does not also pass through `elementDataSourceBlock()`, the seam that declares the `dataSource` key the gate reads. A block that wraps the gate off-seam publishes an authoring surface missing the one key its own runtime honours, and the html tier reports that key with the same `unknown-prop` warning it gives the spellings that do nothing ([#6678](https://github.com/objectstack-ai/objectui/issues/6678)). `pnpm check:esm-specifiers` follows it for the same reason — sources only, no build: it fails when a published package whose build preserves import specifiers (a bare emitting `tsc`, which never rewrites them) writes a relative specifier with no file extension. Node's ESM resolver does not extension-search relative specifiers, so such a specifier makes the published entry unloadable outside a bundler; `@object-ui/react`'s entry died with `ERR_MODULE_NOT_FOUND` while every bundler-based consumer, the whole test suite and CI stayed green ([#4538](https://github.com/objectstack-ai/objectui/issues/4538)). The half that actually *imports* each built entry needs a full build and runs in `node-esm-load-gate.yml`. `pnpm check:action-forward-parity` fails when an action renderer's forward whitelist drops a key the action runtime reads — the class that shipped six times one key at a time, each time green, because the key parses and publishes while the payload is dropped one hop before the runner ([#4050](https://github.com/objectstack-ai/objectui/issues/4050)). `pnpm check:designer-field-key-parity` fails when one of the field designers' statically declared payload shapes (`FieldMetadataPayload`, `ServerFieldSchema`, `DesignerFieldDefinition`) declares a key the installed `@objectstack/spec` `FieldSchema` refuses by NAME. Such a key makes `PUT /api/v1/meta/object/:name` return a hard 422 `INVALID_METADATA` that blocks *every subsequent save* of that object, and the author cannot tell from the designer UI which key did it — the class had been filed three times, each closed with a per-key tombstone written after the instance was found in production, with nothing detecting the next one ([#4644](https://github.com/objectstack-ai/objectui/issues/4644) `indexed`, [#4687](https://github.com/objectstack-ai/objectui/issues/4687) `distance_metric`, [#4676](https://github.com/objectstack-ai/objectui/issues/4676) `placeholder`, gated by [#5761](https://github.com/objectstack-ai/objectui/issues/5761)). It reads the accept set off the schema itself rather than from a list, and it covers a deliberately documented *subset* of the write path: a key that reaches the payload only through a `patchDef` spread or an index signature is outside its reach, and the boundary is stated in the script's own docblock. Its draft-I/O half — the `readFields`/`writeFields` round-trip, which has no declared shape to read — runs in the test suite as `object-fields-io.spec-keys.test.ts`. Same placement rationale as the gates around it: it parses the sources with `typescript` and imports the installed spec, so it needs the install and nothing built. `pnpm check:icon-record-names` fails when an authored icon NAME that reaches a resolver reading lucide's runtime `icons` record is not a live key of that record. lucide retires a spelling by dropping it from that record while keeping it as a deprecated named export, so the retired name still imports, still type-checks and still renders wherever it is used as a *component* — `Edit === SquarePen` is true — and resolves to nothing wherever it is used as a *string*: nothing goes red in either direction, which is why the class was repaired twice in two packages before anyone gated it ([#5586](https://github.com/objectstack-ai/objectui/issues/5586), [#5622](https://github.com/objectstack-ai/objectui/issues/5622), [#5633](https://github.com/objectstack-ai/objectui/issues/5633)). It carries no list of retired spellings — the record itself is the judgement — and it re-discovers the resolver population from source on every run, which is how its first pass found four record-reading resolvers nobody had catalogued. It sits here because it parses the sources with `typescript` and reads the installed lucide: the install, and nothing built. The three locale gates sit in the middle because all of them parse the sources with `typescript`: they need the install and nothing built. `pnpm check:i18n-keys` fails when a `t()` call site asks for a key the `en` pack does not define ([#3530](https://github.com/objectstack-ai/objectui/issues/3530)); `pnpm check:i18n-drift` fails when a change to an `en` string is not accompanied by the nine translation packs ([#3650](https://github.com/objectstack-ai/objectui/issues/3650)), and it is why this job's checkout sets `fetch-depth: 0` — it diffs against the merge base, which a depth-1 clone cannot resolve. `pnpm check:i18n-designer-parity` fails when the metadata-admin designer's own module-local string tables come apart — an `en` row with no `zh` row, or a shared row whose two values carry different `{placeholders}` ([#8834](https://github.com/objectstack-ai/objectui/issues/8834)). The two gates before it are blind to that file by construction — the first classifies the module `module-local table` by declaration and skips it, the second read only the ten locale packs and this table is not one of them — so the same card also gave `pnpm check:i18n-drift` that table as a SECOND population, which is the half that catches a changed `en` value whose `zh` row did not follow. `pnpm type-check:scripts` (`tsconfig.scripts.json`) covers `scripts/**/*.ts`, which `pnpm type-check` cannot reach at all — `scripts/` has no package.json, so turbo never walks it, and the coverage guard decides coverage per *package*. Until [#3494](https://github.com/objectstack-ai/objectui/issues/3494) that left the pin tests in `scripts/__tests__/` — including the one pinning this very page — compiled by nothing. `pnpm type-check:vitest-config` runs `apps/console/tsconfig.node.json` directly — the program that already lists `../../vitest.config.mts` and `apps/console/vitest.config.ts` ([#3476](https://github.com/objectstack-ai/objectui/issues/3476)) — because until [#7328](https://github.com/objectstack-ai/objectui/issues/7328) the only thing that ran it was the console's own `type-check` script, reached through the task runner, whose `type-check` task waits on `^build`. (Named in prose rather than as a code span on purpose: the pin below reads this cell as this job's gate list, so spelling that invocation out would credit the job with a command it does not run.) The cheapest compiler that reads the root Vitest config was therefore reachable only through the most expensive job here, and PR #7291 paid for it: a conditionally spread `dist` project whose literal `extends: true` widened to `boolean` degraded the whole `projects` array to `never[]`, every gate its author ran was green, and CI reported three errors, two of them at `../../vitest.config.mts`. It sits in the cheap half beside `pnpm type-check:scripts` for the same measured reason — nothing in its program imports an `@object-ui/*` package, so it needs the install and nothing built. `pnpm type-check:vitest-setup` (`tsconfig.vitest-setup.json`) closes the same gap for the four repo-root `vitest.setup.*` files, uncovered until [#3515](https://github.com/objectstack-ai/objectui/issues/3515); it runs *last*, after `pnpm type-check`, because `vitest.setup.dom.tsx` side-effect-imports four `@object-ui/*` packages and resolves them through the declarations that turbo's `^build` produces. | Every run; on a PR the steps short-circuit when only ignored paths changed | +| `type-check` | Type Check | `scripts/check-type-check-coverage.mjs`, then `pnpm check:phantom-deps`, then `pnpm check:self-import`, then `pnpm check:unreferenced-sources`, then `pnpm check:doc-example-readers`, then `pnpm check:handler-key-reads`, then `pnpm check:metadata-write-doors`, then `pnpm check:published-tsconfig-exclude`, then `pnpm check:side-effects-array`, then `pnpm check:element-data-source-declaration`, then `pnpm check:esm-specifiers`, then `pnpm check:spec-symbols`, then `pnpm check:action-forward-parity`, then `pnpm check:designer-field-key-parity`, then `pnpm check:icon-record-names`, then `pnpm check:i18n-keys`, then `pnpm check:i18n-drift`, then `pnpm check:i18n-designer-parity`, then `pnpm type-check:scripts`, then `pnpm type-check:vitest-config`, then `pnpm type-check`, then `pnpm type-check:vitest-setup`. The coverage guard runs first because turbo silently skips packages that have no `type-check` script, so a package without one would otherwise read as passing (#2911). `pnpm check:phantom-deps` fails when a released package imports a bare specifier its own `package.json` does not declare — a *phantom dependency*, invisible locally because the workspace root's `devDependencies` sit on the upward resolution path from every package directory and on no consumer's, so `require.resolve('react', { paths: ['packages/core/src'] })` succeeds while `@object-ui/core` declares react in no field at all ([#4394](https://github.com/objectstack-ai/objectui/issues/4394)). `pnpm check:self-import` runs next because it reuses that gate's parser: it fails when a file inside a package names its OWN package, a specifier that resolves through the package's `exports` map to `dist/` while `type-check` waits on `^build` — the *dependencies'* builds, never the package's own — so on a cold cache the declarations do not exist yet and the file fails with `TS2307`. Locally it is always green, because every local workflow builds before it type-checks and leaves a `dist/` behind; PR #4789's first run was red on exactly one such line ([#4801](https://github.com/objectstack-ai/objectui/issues/4801)). `pnpm check:unreferenced-sources` runs next, reusing the same parser again: it fails when a covered package ships a source file that nothing reaches — not the package's declared entry, and not its build config. Until [#7515](https://github.com/objectstack-ai/objectui/issues/7515) no gate here could see one: `check-dist-completeness` asks whether `dist/` holds what `tsc` emits, `check-readme-exports` compares documented exports against shipped ones, and a file that is in the tarball while being reachable from nothing is outside both — so the detection mechanism was a human reading unrelated code, which is how both instances found in one week were found ([#7319](https://github.com/objectstack-ai/objectui/issues/7319), [#7397](https://github.com/objectstack-ai/objectui/issues/7397)). The hazard is not the bytes: the file #7319 removed carried the same export name as a live engine one package over and evaluated no predicate, so name-completion alone could have wired a silently wrong renderer into a published package. Reachability has TWO roots, and the second is the whole difficulty — `packages/components` reaches its two `use-sync-external-store` shims only through `vite.config.ts` `resolve.alias` entries whose importer is a bundled dependency no source file names, so a walk that skips that leg reports exactly those two live files as dead on its first run, and a gate that cries wolf gets switched off rather than fixed. Scope is DECLARED per package in `COVERED_PACKAGES` and the uncovered remainder is printed as a count derived from the workspace on every run, because the alias mechanisms differ per package and a gate that covers one package correctly beats one that covers forty with false positives. An alias expression it cannot evaluate is a FINDING rather than a skip, since skipping one would make it accuse whatever file that alias points at. `pnpm check:doc-example-readers` runs next, on the same parser again: it fails when an exported symbol's own JSDoc `@example` hand-spells a resolution that its REAL call sites obtain by calling a shared reader. A doc comment is what the next call site is copied from, so prose that outlives the ruling it encoded re-seeds every later copy — measured at two cards and three copied call sites ([#7627](https://github.com/objectstack-ai/objectui/issues/7627), [#7638](https://github.com/objectstack-ai/objectui/issues/7638)), both closed by pointing the prose at `resolveRecordSourceObjectName`. Nothing here could see either one, and `check-spec-symbol-derivation` was credited with the class twice — in #7638's card body and then in the dispatch that repeated it — while its rule 4 judges `@objectstack/spec` citations at member granularity and says nothing about prose prescribing a LOCAL spelling ([#7652](https://github.com/objectstack-ai/objectui/issues/7652)). It fires on four conditions at once — the example calls the symbol it documents, a real call site fills the same argument slot by calling an exported single-`return` reader, the example does not, and what the example writes there is that reader's own return expression or one of the rungs it resolves between — which is what keeps it off the literals and placeholders an example legitimately carries. It does NOT judge whether a prescribed spelling is correct: on the day either card was filed the prose and every copy of it agreed, and no gate reading only the tree can know a ruling. What it catches is the state right after, when the call sites move and the prose does not. `pnpm check:handler-key-reads` runs next, on the same parser again: it fails when an `on*` handler key that a REGISTERED renderer reads off the authored document is not a declared member of the zod arm for the type it is registered under. `BaseSchema` is `.passthrough()`, so an undeclared key is not refused — it stops being judged and the value is KEPT, then reaches the renderer that reads it; measured on the built dist, `{ type: 'kanban', columns: [], onCardClick: { action: 'toast' } }` went from REFUSED to ACCEPTED with the object surviving into the parsed output ([#7664](https://github.com/objectstack-ai/objectui/issues/7664)). Every gate stayed green, because the [#6124](https://github.com/objectstack-ai/objectui/issues/6124) ledger's population is two hand-written arrays of tuples and that change re-keyed the arm by SUBSTITUTION — so its length assertion held, and a count ratchet would have been green too, which is why [#7753](https://github.com/objectstack-ai/objectui/issues/7753) rejected that option on the instance itself. This gate derives BOTH populations: the arms from every `type: z.literal(…)` in `packages/types/src/zod`, and the read sites from every real `ComponentRegistry.register(…)` call — read off the AST, because one types file NAMES that call in prose eleven times and registers nothing. It follows the document one component at a time rather than every JSX child, because most children are handed a DIFFERENT document (a dashboard's widgets each get their own), and the chain it must reach is four hops long: `register('kanban', ObjectKanbanRenderer)` names a component, that component is an HOC, the document arrives at `ObjectKanban` through a render-prop parameter and at `KanbanRenderer` through an object spread. It says nothing about keys that reach a renderer only through a `{...props}` spread onto a Radix root or a DOM listener slot — there is no read site to derive from — nor about the ledger's `?: never` tombstones, which have no read site by construction; `KNOWN_UNDECLARED_READS` is an exemption list that only shrinks, each row naming the card that owns the fix, and a row whose read site the gate can no longer find fails it. It lives in `scripts/` because the read sites are spread across `@object-ui/plugin-*` and `packages/components`, which `@object-ui/types` may not import — `check:phantom-deps` rejects it and it would close a cycle. `pnpm check:metadata-write-doors` runs next, on the same parser again: it fails when an in-repo DOOR that can PUT an object-metadata document does not apply the object-metadata write guard. [#7714](https://github.com/objectstack-ai/objectui/issues/7714) ruled one client behaviour — a half-filled relationship is held client-side and the PUT body never carries one without a non-empty `reference` — and its PR implemented that ruling by ENUMERATING the writers it knew of, which were two. [#8057](https://github.com/objectstack-ai/objectui/issues/8057) then reproduced the identical defect on a THIRD writer neither guard covered, in that card's own required dogfood, and [#8676](https://github.com/objectstack-ai/objectui/issues/8676) swept and found nine more. The half that outlives the count is that the sweep the question is naturally asked in cannot see its own subject: `git grep 'client\.save('` returns ZERO over the file #8057 is entirely about, because the call is `client.save(type, …)` and the generic argument sits between the name and the paren — and a hand-rolled `fetch` PUT to `/api/v1/meta/object/:name` is not that spelling at all. So this gate enumerates DOORS, never writers: the writer set is OPEN and nobody has to announce a new member, while the transport set is CLOSED and this repository owns it, so guarding the doors covers every writer past and future without a list anywhere. It derives every call carrying a `method: 'PUT'` literal whose URL RESOLVES to a `/meta` path — resolved through templates, fields and helper return values rather than read, because the repo's central door spells its URL three hops from the call and says nothing at the call site — plus every call to the SDK's `meta.saveItem`, which lives in a package this repo does not own and so cannot be guarded from the inside. A door whose type is a string literal other than `object` is exempt; a door whose type is a runtime value is judged CAPABLE, which is the fail-closed direction. It answers COVERAGE only — whether a guarded door's body is correct is the guard's own pins — and it refuses to report OK unless it found at least one door of each kind and at least one guarded door, so a renamed transport turns it red rather than green. Same placement rationale as the gates around it: it parses the sources with `typescript`, so it needs the install and nothing built. `pnpm check:published-tsconfig-exclude` follows, config reads only: it fails when a published package's build `tsconfig.json` excludes tooling by FILE NAME (`*.test.ts`) without also excluding the tooling DIRECTORIES (`**/__tests__/**` and its two siblings, derived from `TOOLING_FILE` rather than retyped). A name-only exclude stops the files that happen to be named that way and nothing else, so the first shared helper added to a `__tests__/` directory becomes a program input and an emitting program writes it into the published `dist` — three times so far, each found by a human and never by a gate ([#4006](https://github.com/objectstack-ai/objectui/issues/4006), [#4836](https://github.com/objectstack-ai/objectui/issues/4836), [#6943](https://github.com/objectstack-ai/objectui/issues/6943), the third in the same package as the first). [#7212](https://github.com/objectstack-ai/objectui/issues/7212) measured the standing exposure — 29 published packages carrying the name form with ZERO offending files, green because nobody had added such a helper yet — and the gate landed together with their conversion so `main` was green on merge. It reads `exclude` arrays and nothing else: no build, no artifact, no emit model, which is the narrower scope that keeps it clear of the modelling [#4846](https://github.com/objectstack-ai/objectui/issues/4846) declined for the artifact-level gate. Six published packages are named carve-outs, each re-proving its own reason on every run: `cli`, `create-plugin` and `data-objectstack` emit from a `tsup` entry graph, `plugin-charts` keeps its tooling exclude in the `dts()` options, and `console` and `runner` are Vite applications with `noEmit: true` and no `dts()` plugin. `pnpm check:side-effects-array` runs next, sources only and no build: it fails when a package's `sideEffects` ARRAY and its module bodies disagree in either direction — a module that registers something at load time and is not named (a bundler drops it, and the registration is gone from a *consumer's* app with no error, no warning and exit 0), or a name whose module no longer registers anything. `@object-ui/app-shell` declares such an array because both simpler answers are measurably wrong for it: omitting the field makes the whole package unshakeable, and `"sideEffects": false` silently drops three live SDUI widget registrations to zero chunks ([#6535](https://github.com/objectstack-ai/objectui/issues/6535), [#6683](https://github.com/objectstack-ai/objectui/issues/6683)). The enumeration is re-derived from the module bodies on every run rather than listed, so there is no second copy to rot. The artifact half of the same contract — do those registrations survive a real bundler — cannot run in this job at all: it needs a built console, so it lives in the SDUI registration pin step of `performance-budget.yml`. `pnpm check:element-data-source-declaration` runs next, sources only and no build: it fails when a source that consumes `ElementDataSourceGate` does not also pass through `elementDataSourceBlock()`, the seam that declares the `dataSource` key the gate reads. A block that wraps the gate off-seam publishes an authoring surface missing the one key its own runtime honours, and the html tier reports that key with the same `unknown-prop` warning it gives the spellings that do nothing ([#6678](https://github.com/objectstack-ai/objectui/issues/6678)). `pnpm check:esm-specifiers` follows it for the same reason — sources only, no build: it fails when a published package whose build preserves import specifiers (a bare emitting `tsc`, which never rewrites them) writes a relative specifier with no file extension. Node's ESM resolver does not extension-search relative specifiers, so such a specifier makes the published entry unloadable outside a bundler; `@object-ui/react`'s entry died with `ERR_MODULE_NOT_FOUND` while every bundler-based consumer, the whole test suite and CI stayed green ([#4538](https://github.com/objectstack-ai/objectui/issues/4538)). The half that actually *imports* each built entry needs a full build and runs in `node-esm-load-gate.yml`. `pnpm check:action-forward-parity` fails when an action renderer's forward whitelist drops a key the action runtime reads — the class that shipped six times one key at a time, each time green, because the key parses and publishes while the payload is dropped one hop before the runner ([#4050](https://github.com/objectstack-ai/objectui/issues/4050)). `pnpm check:designer-field-key-parity` fails when one of the field designers' statically declared payload shapes (`FieldMetadataPayload`, `ServerFieldSchema`, `DesignerFieldDefinition`) declares a key the installed `@objectstack/spec` `FieldSchema` refuses by NAME. Such a key makes `PUT /api/v1/meta/object/:name` return a hard 422 `INVALID_METADATA` that blocks *every subsequent save* of that object, and the author cannot tell from the designer UI which key did it — the class had been filed three times, each closed with a per-key tombstone written after the instance was found in production, with nothing detecting the next one ([#4644](https://github.com/objectstack-ai/objectui/issues/4644) `indexed`, [#4687](https://github.com/objectstack-ai/objectui/issues/4687) `distance_metric`, [#4676](https://github.com/objectstack-ai/objectui/issues/4676) `placeholder`, gated by [#5761](https://github.com/objectstack-ai/objectui/issues/5761)). It reads the accept set off the schema itself rather than from a list, and it covers a deliberately documented *subset* of the write path: a key that reaches the payload only through a `patchDef` spread or an index signature is outside its reach, and the boundary is stated in the script's own docblock. Its draft-I/O half — the `readFields`/`writeFields` round-trip, which has no declared shape to read — runs in the test suite as `object-fields-io.spec-keys.test.ts`. Same placement rationale as the gates around it: it parses the sources with `typescript` and imports the installed spec, so it needs the install and nothing built. `pnpm check:icon-record-names` fails when an authored icon NAME that reaches a resolver reading lucide's runtime `icons` record is not a live key of that record. lucide retires a spelling by dropping it from that record while keeping it as a deprecated named export, so the retired name still imports, still type-checks and still renders wherever it is used as a *component* — `Edit === SquarePen` is true — and resolves to nothing wherever it is used as a *string*: nothing goes red in either direction, which is why the class was repaired twice in two packages before anyone gated it ([#5586](https://github.com/objectstack-ai/objectui/issues/5586), [#5622](https://github.com/objectstack-ai/objectui/issues/5622), [#5633](https://github.com/objectstack-ai/objectui/issues/5633)). It carries no list of retired spellings — the record itself is the judgement — and it re-discovers the resolver population from source on every run, which is how its first pass found four record-reading resolvers nobody had catalogued. It sits here because it parses the sources with `typescript` and reads the installed lucide: the install, and nothing built. The three locale gates sit in the middle because all of them parse the sources with `typescript`: they need the install and nothing built. `pnpm check:i18n-keys` fails when a `t()` call site asks for a key the `en` pack does not define ([#3530](https://github.com/objectstack-ai/objectui/issues/3530)); `pnpm check:i18n-drift` fails when a change to an `en` string is not accompanied by the nine translation packs ([#3650](https://github.com/objectstack-ai/objectui/issues/3650)), and it is why this job's checkout sets `fetch-depth: 0` — it diffs against the merge base, which a depth-1 clone cannot resolve. `pnpm check:i18n-designer-parity` fails when the metadata-admin designer's own module-local string tables come apart — an `en` row with no `zh` row, or a shared row whose two values carry different `{placeholders}` ([#8834](https://github.com/objectstack-ai/objectui/issues/8834)). The two gates before it are blind to that file by construction — the first classifies the module `module-local table` by declaration and skips it, the second read only the ten locale packs and this table is not one of them — so the same card also gave `pnpm check:i18n-drift` that table as a SECOND population, which is the half that catches a changed `en` value whose `zh` row did not follow. `pnpm type-check:scripts` (`tsconfig.scripts.json`) covers `scripts/**/*.ts`, which `pnpm type-check` cannot reach at all — `scripts/` has no package.json, so turbo never walks it, and the coverage guard decides coverage per *package*. Until [#3494](https://github.com/objectstack-ai/objectui/issues/3494) that left the pin tests in `scripts/__tests__/` — including the one pinning this very page — compiled by nothing. `pnpm type-check:vitest-config` runs `apps/console/tsconfig.node.json` directly — the program that already lists `../../vitest.config.mts` and `apps/console/vitest.config.ts` ([#3476](https://github.com/objectstack-ai/objectui/issues/3476)) — because until [#7328](https://github.com/objectstack-ai/objectui/issues/7328) the only thing that ran it was the console's own `type-check` script, reached through the task runner, whose `type-check` task waits on `^build`. (Named in prose rather than as a code span on purpose: the pin below reads this cell as this job's gate list, so spelling that invocation out would credit the job with a command it does not run.) The cheapest compiler that reads the root Vitest config was therefore reachable only through the most expensive job here, and PR #7291 paid for it: a conditionally spread `dist` project whose literal `extends: true` widened to `boolean` degraded the whole `projects` array to `never[]`, every gate its author ran was green, and CI reported three errors, two of them at `../../vitest.config.mts`. It sits in the cheap half beside `pnpm type-check:scripts` for the same measured reason — nothing in its program imports an `@object-ui/*` package, so it needs the install and nothing built. `pnpm type-check:vitest-setup` (`tsconfig.vitest-setup.json`) closes the same gap for the four repo-root `vitest.setup.*` files, uncovered until [#3515](https://github.com/objectstack-ai/objectui/issues/3515); it runs *last*, after `pnpm type-check`, because `vitest.setup.dom.tsx` side-effect-imports four `@object-ui/*` packages and resolves them through the declarations that turbo's `^build` produces. | Every run; on a PR the steps short-circuit when only ignored paths changed | | `test` | Test (shard N/4) | When a pull request changed nothing outside the exclusion list, the decision step runs `scripts/markdown-test-inputs.mjs` before anything else, and the job runs in full when the answer is yes ([#8861](https://github.com/objectstack-ai/objectui/issues/8861)). The exclusions drop every markdown path, and a markdown document can be a TEST'S INPUT: [#8857](https://github.com/objectstack-ai/objectui/issues/8857) changed one package README, this job reported success in ten seconds having run nothing, and the merge-queue build then failed the same shard in 907 seconds and dequeued it. That script carries the derived class — which documents a test reads, and which test reads each — and audits itself against the tree, so the widening stays narrow: a markdown-only change nothing reads still skips. It is this job's stage only, because this is the job that runs those tests. Then `pnpm test --shard=N/4` across a 4-runner matrix with `fail-fast: false`, so every shard reports its own failures. No coverage instrumentation — v8 adds 40–100% overhead. Then, **on shard 1 only**, `pnpm test:dist` — the built-artifact lane ([#7183](https://github.com/objectstack-ai/objectui/issues/7183)). It delegates to a turbo task scoped to the one package that holds built-artifact pins; that task depends on the package's OWN build (`dependsOn: ["build"]`, not `^build`), so the bundle exists before the pins read it, and then runs the `dist` vitest project, whose pins import a package's BUILT bundle instead of its `src` — a claim the source-aliased suite above is structurally unable to make, since the root config aliases every workspace package to `src`. It is deliberately not sharded and not repeated on the other three runners: the lane is a handful of files, and running it on all four would pay for the same build four times. | Pull requests and merge-queue builds (everything but `push`); steps short-circuit on a PR that changed only ignored paths | | `test-coverage` | Test (coverage shard N/4) | `pnpm test:coverage --reporter=blob --shard=N/4` across a 4-runner matrix with `fail-fast: false`. Each shard writes `.vitest-reports/blob-N-4.json` — raw coverage and test results in one file — and uploads it as an artifact even when the shard is red, which is what makes a failing coverage run diagnosable at all (vitest deletes `coverage/` on a red run unless `coverage.reportOnFailure` is set, [#5402](https://github.com/objectstack-ai/objectui/issues/5402)). The configured coverage thresholds are neutralised on the shard legs, because a quarter of the suite judged against a whole-suite threshold is not a defect signal; they are enforced once, on the merged report, by the job below ([#5403](https://github.com/objectstack-ai/objectui/issues/5403)). | **Push only** | | `coverage-report` | Test (coverage) | Downloads the four blob reports, refuses to continue unless all four arrived, merges them with `pnpm test:coverage --merge-reports` into one complete report — which is where the configured coverage thresholds are enforced, over the whole merged map, the shard legs having overridden them to zero — and publishes that report as the `coverage-report` artifact (kept 7 days, the same as the blobs it is derived from). Its last step runs on every path and states the outcome: the job is **red, with an error annotation**, whenever the gate did not run for the commit — before [#5403](https://github.com/objectstack-ai/objectui/issues/5403) the final step carried the implicit `success()` and was silently skipped by 311 of 373 coverage jobs, which is how four days of a 100%-failing coverage job went unnoticed. A breach of the thresholds is reported *separately* from a lane that never delivered, because the two call for opposite actions. ⛔ It never merges a report from fewer than four shards: a wrong coverage number is worse than a missing one. The Codecov upload this job used to carry was retired by [#5436](https://github.com/objectstack-ai/objectui/issues/5436) — `CODECOV_TOKEN` was never set, so it failed on every push; the trend dashboard and PR coverage comments are gone with it, the gate is not. | **Push only** | diff --git a/package.json b/package.json index a1ad28128d..cdc05fa084 100644 --- a/package.json +++ b/package.json @@ -99,6 +99,7 @@ "check:unreferenced-sources": "node scripts/check-unreferenced-sources.mjs", "check:doc-example-readers": "node scripts/check-doc-example-shared-reader.mjs", "check:handler-key-reads": "node scripts/check-handler-key-read-sites.mjs", + "check:metadata-write-doors": "node scripts/check-object-metadata-write-doors.mjs", "check:changeset-claims": "node scripts/check-changeset-claims.mjs", "cli": "node packages/cli/dist/cli.js", "objectui": "node packages/cli/dist/cli.js", diff --git a/packages/app-shell/src/services/MetadataService.ts b/packages/app-shell/src/services/MetadataService.ts index 189d553db2..ebbafee167 100644 --- a/packages/app-shell/src/services/MetadataService.ts +++ b/packages/app-shell/src/services/MetadataService.ts @@ -17,7 +17,12 @@ */ import { stripReadDecorations } from '@objectstack/spec/kernel'; -import { viewItemObjectName, type ObjectStackAdapter } from '@object-ui/data-objectstack'; +import { + assertObjectMetadataWritable, + RELATIONSHIP_TYPES_REQUIRING_REFERENCE, + viewItemObjectName, + type ObjectStackAdapter, +} from '@object-ui/data-objectstack'; import type { ObjectDefinition, DesignerFieldDefinition } from '@object-ui/types'; // The retired-field-key tombstone registry lives at a dedicated internal // subpath, not the main barrel — objectui#6527 option B (maintainer ruling, @@ -187,24 +192,21 @@ function toObjectPayload(obj: ObjectDefinition, fields?: FieldMetadataPayload[]) }; } -/** - * Field types whose `reference` — the target object a relationship links to — - * `@objectstack/spec` requires to be present and non-empty. - * - * Re-measured for objectui#7714 against the 17.3.0 artifact by parsing - * `{ type, label: 'L' }` for every one of `FieldType`'s 49 declared members: - * exactly two are refused at path `reference`, both with code `custom`, and - * the other 47 are not refused at all on that minimal document. - * - * ⛔ Deliberately NOT "parse every field through `FieldSchema` before the PUT". - * That would refuse plugin-registered keys the SERVER accepts — measured on the - * installed 17.2.0, `x_plugin_thing` is `unrecognized_keys` to the schema while - * the server that sent it takes it back — which is the same reason - * {@link RETIRED_FIELD_KEYS} is a named list rather than a schema filter. This - * guard states one invariant; it is not a client-side revalidation of the - * document. +/* + * `RELATIONSHIP_TYPES_REQUIRING_REFERENCE` — the field types whose `reference` + * `@objectstack/spec` requires to be present and non-empty — is imported above + * and no longer declared here. + * + * objectui#8676: it used to be declared here AND word-for-word again in + * `plugin-designer`'s `MetadataFieldsPage`. Two remembered copies of one + * contract fact is the same hazard as a remembered list of writers, so both + * writers now read the single declaration in `@object-ui/data-objectstack`, + * beside the write doors, where a pin DERIVES the set from the installed spec + * on every run instead of restating a measurement. The reasoning that used to + * sit here — including ⛔ why this is one invariant and not a client-side + * revalidation of the document through `FieldSchema` — moved with it, to + * `object-metadata-write-guard.ts`. */ -const RELATIONSHIP_TYPES_REQUIRING_REFERENCE = ['lookup', 'master_detail']; /** * Why THIS value cannot be a target, and what the contract does about it — @@ -627,6 +629,35 @@ export class MetadataService { return []; } + /** + * The ONE place this class puts metadata on the wire (objectui#8676). + * + * `@objectstack/client`'s `meta.saveItem` is the third of the three in-repo + * transports that can PUT `/meta/:type/:name`, and the only one that lives in + * a package this repo does not own — so the invariant cannot be pushed down + * into it the way it is pushed into `MetadataClient.save`. This method is the + * compensating seam: the SDK door is reached through it and through nothing + * else in this class, so the guard runs once rather than three times, and + * `scripts/check-object-metadata-write-doors.mjs` has one site to judge. + * + * ⚠ The guard here is a BACKSTOP, not the primary refusal for the two + * object-shaped callers. `saveObject` and `saveFields` both build their + * `fields` through {@link toFieldsMap}, which refuses the same half-filled + * relationship EARLIER and with the designer-facing four-state wording those + * writers' pins assert. Nothing here replaces that; this covers the callers + * that do NOT pass through a conversion — `saveMetadataItem`, whose `category` + * is a runtime value and can be `'object'`, and whoever calls it next. + */ + private async putMetadataItem( + category: string, + name: string, + data: Record, + ): Promise { + assertObjectMetadataWritable(category, data, 'MetadataService'); + const client = this.adapter.getClient(); + await client.meta.saveItem(category, name, data); + } + /** * Persist a metadata item (upsert) for any category. * @@ -649,8 +680,7 @@ export class MetadataService { * private copy of "which object is this?". */ async saveMetadataItem(category: string, name: string, data: Record): Promise { - const client = this.adapter.getClient(); - await client.meta.saveItem(category, name, data); + await this.putMetadataItem(category, name, data); this.adapter.invalidateCache(`${category}:${name}`); if (category === 'view') { const objectName = viewItemObjectName(data); @@ -750,9 +780,8 @@ export class MetadataService { * its own terms (ADR-0049 shape). */ async saveObject(obj: ObjectDefinition, existingFields: FieldMetadataPayload[]): Promise { - const client = this.adapter.getClient(); const payload = toObjectPayload(obj, existingFields); - await client.meta.saveItem('object', obj.name, payload); + await this.putMetadataItem('object', obj.name, payload as unknown as Record); this.adapter.invalidateCache(`object:${obj.name}`); } @@ -889,7 +918,7 @@ export class MetadataService { fields: toFieldsMap(fields.map((field) => toFieldPayload(field, previousFieldEntry(previousFields, field.name)))), }) as Record; - await client.meta.saveItem('object', objectName, updatedObject); + await this.putMetadataItem('object', objectName, updatedObject); this.adapter.invalidateCache(`object:${objectName}`); } diff --git a/packages/app-shell/src/views/metadata-admin/external/api.ts b/packages/app-shell/src/views/metadata-admin/external/api.ts index 8196d1c3bf..ab5a2c450b 100644 --- a/packages/app-shell/src/views/metadata-admin/external/api.ts +++ b/packages/app-shell/src/views/metadata-admin/external/api.ts @@ -21,6 +21,11 @@ */ import { createAuthenticatedFetch } from '@object-ui/auth'; +// objectui#8676 - this module is the SECOND of the three in-repo doors that PUT +// `/meta/:type/:name`, and the one no `client.save(` or `.saveItem(` sweep can +// see: it is a hand-rolled fetch. It writes `object` metadata, so it applies the +// same invariant `MetadataClient.save` applies, from the same module. +import { assertObjectMetadataWritable } from '@object-ui/data-objectstack'; import type { GenerateDraftOpts, ObjectDraft, @@ -187,6 +192,7 @@ export async function validateDatasource( * draft's `definition` is the parseable ObjectSchema body. */ export async function importObjectDraft(draft: ObjectDraft): Promise { + assertObjectMetadataWritable('object', draft.definition, 'importObjectDraft'); const res = await authFetch( `${serverBase()}/api/v1/meta/object/${encodeURIComponent(draft.name)}`, { diff --git a/packages/data-objectstack/src/index.ts b/packages/data-objectstack/src/index.ts index 5f66265ed2..df7912500c 100644 --- a/packages/data-objectstack/src/index.ts +++ b/packages/data-objectstack/src/index.ts @@ -6629,6 +6629,17 @@ export { MetadataClient, readSaveAdvisories } from './metadata-client'; // `getDraft` that produces the envelope, because the unwrap-and-strip is part // of that method's contract rather than a detail of any one view. export { extractDraftBody } from './draft-envelope'; +// objectui#8676 - the object-metadata write invariant, exported so the two DOORS +// that do not run through `MetadataClient.save` can apply the same one. It is +// exported for DOORS, not for writers: a writer that calls it by hand is a +// writer that can forget to, which is the enumeration failure this closes. +// `scripts/check-object-metadata-write-doors.mjs` derives the door set and +// fails when a door does not reach this function. +export { + assertObjectMetadataWritable, + RELATIONSHIP_TYPES_REQUIRING_REFERENCE, + OBJECT_METADATA_TYPE, +} from './object-metadata-write-guard'; export type { RuntimeAuthoringIssue, MetadataSaveAdvisoryEvent, diff --git a/packages/data-objectstack/src/metadata-client.objectWriteGuard.test.ts b/packages/data-objectstack/src/metadata-client.objectWriteGuard.test.ts new file mode 100644 index 0000000000..a30bcabd08 --- /dev/null +++ b/packages/data-objectstack/src/metadata-client.objectWriteGuard.test.ts @@ -0,0 +1,98 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * objectui#8676 — `MetadataClient.save` is a DOOR, and it applies the + * object-metadata write invariant. + * + * ## What this pins that the guard's own suite cannot + * + * The guard's suite proves the function refuses. This proves the DOOR REACHES + * IT — which is the half objectui#7714 lost. That ruling's invariant was + * implemented, correct, and pinned, and it still did not hold for twelve of + * fifteen write call sites, because nothing connected the two facts. So the + * load-bearing assertion here is not "it throws": it is ⭐ **no request was + * issued**. A guard that fires after the bytes leave has not held the draft + * client-side, which is the behaviour the ruling actually names. + * + * ⚠ Every refusal below is measured against a LIT CONTROL on the same harness — + * the same call with a usable target, observed to reach `fetch` and resolve. A + * "no request was issued" assertion is worthless beside a harness that never + * issues one. + */ + +import { describe, expect, it, vi } from 'vitest'; +import { MetadataClient } from './metadata-client'; + +function okResponse(): Response { + return new Response(JSON.stringify({ success: true, version: 'v1' }), { + status: 200, + headers: { 'content-type': 'application/json' }, + }); +} + +function harness() { + const fetchImpl = vi.fn(async () => okResponse()); + const client = new MetadataClient({ + baseUrl: 'http://test.local', + fetch: fetchImpl as unknown as typeof fetch, + }); + return { client, fetchImpl }; +} + +const HALF_FILLED = { + name: 'account', + label: 'Account', + fields: { + title: { type: 'text', label: 'Title' }, + owner: { type: 'lookup', label: 'Owner' }, + }, +}; + +const COMPLETE = { + name: 'account', + label: 'Account', + fields: { + title: { type: 'text', label: 'Title' }, + owner: { type: 'lookup', label: 'Owner', reference: 'contact' }, + }, +}; + +describe('MetadataClient.save — the door applies the object-metadata write guard', () => { + it('refuses a half-filled relationship AND ISSUES NO REQUEST', async () => { + const { client, fetchImpl } = harness(); + await expect(client.save('object', 'account', HALF_FILLED, { mode: 'draft' })) + .rejects.toThrow(/`owner`/); + // ⭐ The discriminating assertion. A guard that ran after the request would + // satisfy the rejection above and fail this line. + expect(fetchImpl).not.toHaveBeenCalled(); + }); + + it('CONTROL — the same harness DOES issue the PUT when the target is usable', async () => { + const { client, fetchImpl } = harness(); + await client.save('object', 'account', COMPLETE, { mode: 'draft' }); + expect(fetchImpl).toHaveBeenCalledTimes(1); + const [url, init] = fetchImpl.mock.calls[0] as unknown as [string, RequestInit]; + expect(url).toContain('/meta/object/account'); + expect(init.method).toBe('PUT'); + }); + + it('CONTROL — a non-object type with the same field shape still reaches the wire', async () => { + // The door serves every metadata type. The guard must not leak into them. + const { client, fetchImpl } = harness(); + await client.save('view', 'account_list', HALF_FILLED); + expect(fetchImpl).toHaveBeenCalledTimes(1); + }); + + it('refuses the ARRAY `fields` shape at the door too, and still issues nothing', async () => { + const { client, fetchImpl } = harness(); + const body = { name: 'account', fields: [{ name: 'owner', type: 'lookup', label: 'Owner' }] }; + await expect(client.save('object', 'account', body)).rejects.toThrow(/`owner`/); + expect(fetchImpl).not.toHaveBeenCalled(); + }); + + it('the message names the door, so an author sees where the write stopped', async () => { + const { client } = harness(); + await expect(client.save('object', 'account', HALF_FILLED)) + .rejects.toThrow(/^MetadataClient\.save refused/); + }); +}); diff --git a/packages/data-objectstack/src/metadata-client.ts b/packages/data-objectstack/src/metadata-client.ts index 6812ff3d0f..3972243cf2 100644 --- a/packages/data-objectstack/src/metadata-client.ts +++ b/packages/data-objectstack/src/metadata-client.ts @@ -33,6 +33,12 @@ */ import { GetMetaItemLayeredResponseSchema } from '@objectstack/spec/api'; +// objectui#8676 - the object-metadata write invariant, applied HERE because this +// is a DOOR and not a writer. Every `client.save('object', ...)` call site in the +// repo passes through this one method, so guarding it covers a writer set that +// nothing has to enumerate. See that module's docblock for why the writers are +// deliberately not listed anywhere. +import { assertObjectMetadataWritable } from './object-metadata-write-guard'; import type { GetMetaItemLayeredResponse, RuntimeAuthoringIssue, @@ -944,6 +950,9 @@ export class MetadataClient { ' The PUT /meta/:type/:name route requires a name segment.', ); } + // objectui#8676 - before the request, so a refused body issues no PUT and the + // half-filled draft stays in the client (objectui#7714's ruled behaviour). + assertObjectMetadataWritable(type, item, 'MetadataClient.save'); const params: string[] = []; if (options.force) params.push('force=true'); if (options.mode === 'draft') params.push('mode=draft'); diff --git a/packages/data-objectstack/src/object-metadata-write-guard.derivation.test.ts b/packages/data-objectstack/src/object-metadata-write-guard.derivation.test.ts new file mode 100644 index 0000000000..bf1acd6493 --- /dev/null +++ b/packages/data-objectstack/src/object-metadata-write-guard.derivation.test.ts @@ -0,0 +1,95 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * objectui#8676 — the guard's ONE list is derived from the installed contract on + * every run, never recalled. + * + * ## Why this file is the point rather than a formality + * + * The card this guard closes is about an enumeration that went stale while + * nothing said so. `RELATIONSHIP_TYPES_REQUIRING_REFERENCE` is the only + * enumeration the fix itself keeps, so it does not get to be exempt from the + * card's own lesson. This re-derives it from `@objectstack/spec` — parsing a + * minimal `{ type, label }` document for EVERY member of `FieldType` and keeping + * the types the contract refuses at path `reference` — and asserts the guard's + * array equals that. A spec release that makes a third field type require a + * target turns this red, which is precisely what nothing did for the writers. + * + * ⚠ The derivation carries its own controls. A probe that refuses everything, or + * accepts everything, would also "agree" with some array, so the run asserts + * that the population is non-trivial and that a type OUTSIDE the derived set + * parses green on the same minimal document. + * + * ⛔ Deliberately a TEST and not a runtime probe. Parsing 50 documents on the way + * to every save pays a hot-path cost to re-learn something that changes at most + * once per spec release, and a runtime probe that throws must choose between + * blocking writes and failing open. A pin has neither problem: it fails at CI + * time with nothing at stake. + */ + +import { describe, expect, it } from 'vitest'; +import { FieldSchema, FieldType, ObjectSchema } from '@objectstack/spec/data'; +import { RELATIONSHIP_TYPES_REQUIRING_REFERENCE } from './object-metadata-write-guard'; + +/** Every field type the installed contract refuses for want of a `reference`. */ +function deriveTypesRequiringReference(): string[] { + const derived: string[] = []; + for (const type of FieldType.options) { + const result = FieldSchema.safeParse({ type, label: 'L' }); + if (result.success) continue; + if (result.error.issues.some((issue) => issue.path.join('.') === 'reference')) derived.push(type); + } + return derived; +} + +describe('RELATIONSHIP_TYPES_REQUIRING_REFERENCE — derived from the installed spec', () => { + it('is exactly the set the contract refuses at `reference`', () => { + const derived = deriveTypesRequiringReference(); + expect([...RELATIONSHIP_TYPES_REQUIRING_REFERENCE].sort()).toEqual([...derived].sort()); + }); + + it('CONTROL — the probe is neither refusing nor accepting everything', () => { + const derived = deriveTypesRequiringReference(); + // Non-trivial on both sides: some types are in, most are not. Without this, + // a probe that collapsed to "all" or "none" could still agree with an array. + expect(derived.length).toBeGreaterThan(0); + expect(derived.length).toBeLessThan(FieldType.options.length); + expect(FieldType.options.length).toBeGreaterThan(10); + }); + + it('CONTROL — a type outside the derived set parses green on the same document', () => { + const outside = FieldType.options.filter((type) => !RELATIONSHIP_TYPES_REQUIRING_REFERENCE.includes(type)); + expect(outside.length).toBeGreaterThan(0); + expect(FieldSchema.safeParse({ type: 'text', label: 'L' }).success).toBe(true); + }); +}); + +describe('the four target states, measured against the contract rather than asserted', () => { + // The guard's message distinguishes four states. This is where the claim that + // all four are REFUSED BY THE SERVER is re-measured, so the guard's "it + // forecloses nothing the server would have taken" argument stays checkable. + const cases: Array<[string, Record]> = [ + ['absent', { type: 'lookup', label: 'L' }], + ['null', { type: 'lookup', label: 'L', reference: null }], + ['empty string', { type: 'lookup', label: 'L', reference: '' }], + ['whitespace only', { type: 'lookup', label: 'L', reference: ' ' }], + ]; + + for (const [label, field] of cases) { + it(`the contract refuses a lookup whose reference is ${label}`, () => { + const result = ObjectSchema.safeParse({ name: 'account', label: 'A', fields: { rel: field } }); + expect(result.success).toBe(false); + if (result.success) return; + expect(result.error.issues.map((issue) => issue.path.join('.'))).toContain('fields.rel.reference'); + }); + } + + it('CONTROL — a usable target is ACCEPTED by the same schema on the same document', () => { + const result = ObjectSchema.safeParse({ + name: 'account', + label: 'A', + fields: { rel: { type: 'lookup', label: 'L', reference: 'contact' } }, + }); + expect(result.success).toBe(true); + }); +}); diff --git a/packages/data-objectstack/src/object-metadata-write-guard.test.ts b/packages/data-objectstack/src/object-metadata-write-guard.test.ts new file mode 100644 index 0000000000..6e870a6a19 --- /dev/null +++ b/packages/data-objectstack/src/object-metadata-write-guard.test.ts @@ -0,0 +1,152 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * objectui#8676 — the object-metadata write invariant, pinned at the DOOR. + * + * The companion halves, each pinning something this file cannot: + * + * - `object-metadata-write-guard.derivation.test.ts` DERIVES the relationship + * type set from the installed `@objectstack/spec` and asserts it equals the + * array the guard keeps, so the one list here cannot go stale silently. + * - `metadata-client.objectWriteGuard.test.ts` pins the DOOR: that + * `MetadataClient.save` reaches this function, and that a refused body issues + * NO REQUEST. A guard nothing calls is the defect objectui#8676 is about, so + * "the function refuses" and "the door calls it" are pinned apart on purpose. + * - `scripts/check-object-metadata-write-doors.mjs` answers COVERAGE — whether + * every door reaches it at all. + * + * ⚠ What every negative case below is measured against: a LIT CONTROL that runs + * the same call with a usable target and observes it return. An assertion never + * observed to pass on the accepting side would be satisfied by a guard that + * refuses everything, which is the caricature this invariant must not become. + */ + +import { describe, expect, it } from 'vitest'; +import { + assertObjectMetadataWritable, + OBJECT_METADATA_TYPE, + RELATIONSHIP_TYPES_REQUIRING_REFERENCE, +} from './object-metadata-write-guard'; + +const objectWith = (fields: unknown) => ({ name: 'account', label: 'Account', fields }); + +describe('assertObjectMetadataWritable — the four states of an unusable target', () => { + // The four states the contract distinguishes, each measured against + // `ObjectSchema` by the derivation pin. The guard must refuse all four, + // because all four reach the server as the same 422. + const unusable: Array<[string, unknown]> = [ + ['absent', undefined], + ['null', null], + ['empty string', ''], + ['whitespace only', ' '], + ]; + + for (const [label, reference] of unusable) { + it(`refuses a lookup whose reference is ${label}`, () => { + const body = objectWith({ + title: { type: 'text', label: 'Title' }, + owner: reference === undefined + ? { type: 'lookup', label: 'Owner' } + : { type: 'lookup', label: 'Owner', reference }, + }); + expect(() => assertObjectMetadataWritable('object', body, 'TEST')).toThrow(/`owner`/); + }); + } + + it('CONTROL — a usable target is written, so the guard is not simply always refusing', () => { + const body = objectWith({ + title: { type: 'text', label: 'Title' }, + owner: { type: 'lookup', label: 'Owner', reference: 'account' }, + }); + expect(() => assertObjectMetadataWritable('object', body, 'TEST')).not.toThrow(); + }); + + it('names WHICH of the four states it saw, so the message is actionable on screen', () => { + const absent = objectWith({ owner: { type: 'lookup', label: 'Owner' } }); + const blank = objectWith({ owner: { type: 'lookup', label: 'Owner', reference: ' ' } }); + expect(() => assertObjectMetadataWritable('object', absent, 'TEST')) + .toThrow(/no `reference` key at all/); + expect(() => assertObjectMetadataWritable('object', blank, 'TEST')) + .toThrow(/whitespace-only `reference`/); + }); + + it('names the door that refused, so a thrown message says where the write stopped', () => { + const body = objectWith({ owner: { type: 'lookup', label: 'Owner' } }); + expect(() => assertObjectMetadataWritable('object', body, 'MetadataClient.save')) + .toThrow(/^MetadataClient\.save refused/); + }); +}); + +describe('assertObjectMetadataWritable — every relationship type, and only those', () => { + it('covers each member of the derived set rather than `lookup` alone', () => { + // The set is not spelled here: this reads whatever the guard declares, so a + // type added to it is covered by this pin the moment it is added. + expect(RELATIONSHIP_TYPES_REQUIRING_REFERENCE.length).toBeGreaterThan(0); + for (const type of RELATIONSHIP_TYPES_REQUIRING_REFERENCE) { + const body = objectWith({ rel: { type, label: 'R' } }); + expect(() => assertObjectMetadataWritable('object', body, 'TEST')).toThrow(/`rel`/); + const ok = objectWith({ rel: { type, label: 'R', reference: 'account' } }); + expect(() => assertObjectMetadataWritable('object', ok, 'TEST')).not.toThrow(); + } + }); + + it('says nothing about a NON-relationship field with no reference', () => { + // ⛔ The counter-case to a guard that drifted into revalidating the document: + // a `text` field has no target and must sail straight through. + const body = objectWith({ title: { type: 'text', label: 'Title' } }); + expect(() => assertObjectMetadataWritable('object', body, 'TEST')).not.toThrow(); + }); +}); + +describe('assertObjectMetadataWritable — both `fields` shapes a writer can hand the door', () => { + it('reads the ARRAY shape, which one whole designer surface PUTs verbatim', () => { + const body = objectWith([ + { name: 'title', type: 'text', label: 'Title' }, + { name: 'owner', type: 'lookup', label: 'Owner' }, + ]); + expect(() => assertObjectMetadataWritable('object', body, 'TEST')).toThrow(/`owner`/); + }); + + it('CONTROL — the same array with a usable target passes', () => { + const body = objectWith([ + { name: 'title', type: 'text', label: 'Title' }, + { name: 'owner', type: 'lookup', label: 'Owner', reference: 'account' }, + ]); + expect(() => assertObjectMetadataWritable('object', body, 'TEST')).not.toThrow(); + }); + + it('names an array entry by its POSITION when it carries no name', () => { + const body = objectWith([{ type: 'lookup', label: 'Owner' }]); + expect(() => assertObjectMetadataWritable('object', body, 'TEST')).toThrow(/\[0\]/); + }); +}); + +describe('assertObjectMetadataWritable — everything it deliberately does not judge', () => { + it('is a no-op for every metadata type other than `object`', () => { + // This door serves `view`, `app`, `flow`, `hook`, `permission` and + // `dashboard` writes too. A `lookup`-shaped key inside one of those is not + // this invariant's business, and refusing it would be the door overreaching. + const body = objectWith({ owner: { type: 'lookup', label: 'Owner' } }); + for (const type of ['view', 'app', 'flow', 'permission', 'hook', 'dashboard']) { + expect(() => assertObjectMetadataWritable(type, body, 'TEST')).not.toThrow(); + } + // ...and the constant naming the one type it does judge is the same one. + expect(() => assertObjectMetadataWritable(OBJECT_METADATA_TYPE, body, 'TEST')).toThrow(); + }); + + it('is a no-op for a body that carries no readable `fields`', () => { + for (const body of [undefined, null, 'not an object', 42, {}, { fields: null }, { fields: 7 }]) { + expect(() => assertObjectMetadataWritable('object', body, 'TEST')).not.toThrow(); + } + }); + + it('⛔ does NOT strip the offending field and report success', () => { + // objectstack#4001's silent-drop shape, ruled out for this family twice. + // The body must come back unchanged; the only outcome is a throw. + const fields = { owner: { type: 'lookup', label: 'Owner' } }; + const body = objectWith(fields); + expect(() => assertObjectMetadataWritable('object', body, 'TEST')).toThrow(); + expect(body.fields).toBe(fields); + expect(Object.keys(fields)).toEqual(['owner']); + }); +}); diff --git a/packages/data-objectstack/src/object-metadata-write-guard.ts b/packages/data-objectstack/src/object-metadata-write-guard.ts new file mode 100644 index 0000000000..bde42f839d --- /dev/null +++ b/packages/data-objectstack/src/object-metadata-write-guard.ts @@ -0,0 +1,168 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * The object-metadata write invariant, placed at the DOOR rather than at the + * writers (objectui#8676). + * + * ## Why this module exists — the enumeration, not the count + * + * objectui#7714 ruled one client behaviour: *a half-filled relationship stays + * client-side and the PUT body never carries a `lookup` without a non-empty + * `reference`*. Its PR implemented that ruling by naming the writers it knew + * about — two of them — and putting the assertion inside each one's array-to-map + * conversion (`toFieldsMap`). objectui#8057 then reproduced the very failure the + * ruling was written against, on a THIRD writer neither conversion covers, in + * that card's own required dogfood. objectui#8676 swept and found nine more. + * + * The durable half of that card is a sentence about method, not a number: + * + * > a ruling that enumerates writers is only as good as the enumeration. + * + * A hand list of writers is stale the next time somebody adds one, and nothing + * says so. So this invariant is not attached to writers at all. It is attached + * to the DOORS — the places that put bytes on the wire — because the doors are a + * CLOSED set that this repository owns, while the writers are an OPEN set it + * does not. Three in-repo transports can PUT `/meta/:type/:name`, and + * `scripts/check-object-metadata-write-doors.mjs` derives that set on every run + * rather than restating it: + * + * 1. `MetadataClient.save` `packages/data-objectstack/src/metadata-client.ts` + * 2. `importObjectDraft` `packages/app-shell/src/views/metadata-admin/external/api.ts` + * 3. the `@objectstack/client` SDK's `meta.saveItem`, reached in this repo + * through `MetadataService`'s one seam + * + * Every writer in the repo reaches the server through one of those three. Guard + * them and the writer count stops mattering; add a fourth transport and the gate + * turns red naming it. That is the whole design: ⛔ do not re-open this by + * adding a writer list anywhere. + * + * ## What it asserts, and the two things it deliberately does NOT + * + * It asserts objectui#7714's invariant and nothing else. + * + * ⛔ **NOT a client-side revalidation of the document.** Parsing the whole body + * through `ObjectSchema` before the PUT would refuse plugin-registered keys the + * SERVER accepts, and it would promote a client PREDICTION into a block — which + * is exactly what objectui#4306 / objectui#6980 ruled against for the designer's + * live Zod pass, on the stated ground that a schema issue on a draft the server + * ACCEPTS would dead-bolt Save with no on-screen editor able to clear it. This + * refusal cannot dead-bolt anything the server would have taken: the same body + * is refused one layer down with a 422 on `fields.NAME.reference`, so the guard + * forecloses nothing and only moves an identical refusal earlier, where it can + * name the field while it is still on screen. + * + * ⛔ **NOT strip-and-report-saved.** Dropping the half-filled field and + * reporting success would trade a visible refusal for an invisible deletion — + * objectstack#4001's shape, ruled out for this family in objectui#7714 and again + * in objectui#8057. It throws; the caller surfaces the message. + * + * ## The type list is pinned to the contract, not to memory + * + * {@link RELATIONSHIP_TYPES_REQUIRING_REFERENCE} is the one list this module + * keeps, and it is the same class of hazard the module exists to close, so it + * does not get to be a remembered list either. + * `object-metadata-write-guard.derivation.test.ts` DERIVES the set from the + * installed `@objectstack/spec` — every member of `FieldType`, parsed as + * `{ type, label }` through `FieldSchema`, keeping those refused at path + * `reference` — and asserts it equals this array. A spec release that makes a + * third field type require a target turns that pin red instead of leaving a + * guard that quietly stopped covering the contract. + * + * The derivation is a TEST and not a runtime probe on purpose: parsing 50 + * field-type documents on the way to every save is a cost paid on the hot path + * to re-learn something that changes at most once per spec release, and a probe + * that throws at runtime has to choose between blocking writes and failing open. + * A pin has neither problem — it fails at CI time, loudly, with nothing at stake. + */ + +/** + * Field types whose `reference` — the target object a relationship links to — + * `@objectstack/spec` requires to be present and non-empty. + * + * Derived from the installed artifact by the pin named above, never recalled. + */ +export const RELATIONSHIP_TYPES_REQUIRING_REFERENCE: readonly string[] = ['lookup', 'master_detail']; + +/** The metadata type whose documents carry the `fields` map this guard reads. */ +export const OBJECT_METADATA_TYPE = 'object'; + +/** + * Whether a `reference` value names an object the server could resolve. + * + * The trim is not cosmetic: `ObjectSchema.fields`' own key grammar + * (`/^[a-z_][a-z0-9_]*$/`) admits no whitespace-bearing name, so a + * whitespace-only target names nothing at either end. Measured on the installed + * spec by the derivation pin, which asserts the contract refuses `' '` exactly + * as it refuses `''` and an absent key. + */ +function isUsableTarget(reference: unknown): boolean { + return typeof reference === 'string' && reference.trim() !== ''; +} + +/** Name the state of an unusable target, so the message says which of them it is. */ +function describeTarget(reference: unknown): string { + if (reference === undefined) return 'no `reference` key at all'; + if (reference === null) return 'a `null` `reference`'; + if (typeof reference !== 'string') return `a \`reference\` of type \`${typeof reference}\``; + if (reference === '') return 'an empty `reference`'; + return 'a whitespace-only `reference`'; +} + +/** + * The `fields` member of an object document, in either shape a writer may hand + * the door. + * + * Both are real. The spec's stored shape is a RECORD keyed by field name, and + * that is what `MetadataService.toFieldsMap` and `MetadataFieldsPage` emit. The + * ARRAY shape is what `readFields`/`writeFields` round-trip for documents that + * arrived that way, and `StudioDesignSurface` PUTs the result verbatim — so a + * door that read only the record shape would be silently blind to a whole + * surface, which is this card's failure mode wearing a different hat. + */ +function fieldEntries(fields: unknown): Array<{ name: string; def: Record }> { + if (Array.isArray(fields)) { + return fields.flatMap((raw, index) => { + if (!raw || typeof raw !== 'object') return []; + const record = raw as Record; + const name = typeof record.name === 'string' && record.name ? record.name : `[${index}]`; + return [{ name, def: record }]; + }); + } + if (fields && typeof fields === 'object') { + return Object.entries(fields as Record).flatMap(([name, def]) => + def && typeof def === 'object' ? [{ name, def: def as Record }] : [], + ); + } + return []; +} + +/** + * Refuse an object-metadata write whose body carries a relationship field with + * no usable target. + * + * A no-op for every other metadata type and for any body with no readable + * `fields` member — this door serves `view`, `app`, `flow`, `permission`, + * `hook` and `dashboard` writes too, and has no opinion about them. + * + * @param type the metadata type segment of the write (`'object'`, `'view'`, …) + * @param item the body about to be serialised onto the wire + * @param writer a short label for the door, so the message names where it fired + * @throws Error naming the offending field, before any request is issued + */ +export function assertObjectMetadataWritable(type: unknown, item: unknown, writer: string): void { + if (String(type) !== OBJECT_METADATA_TYPE) return; + if (!item || typeof item !== 'object') return; + const fields = (item as Record).fields; + for (const { name, def } of fieldEntries(fields)) { + if (!RELATIONSHIP_TYPES_REQUIRING_REFERENCE.includes(String(def.type))) continue; + if (isUsableTarget(def.reference)) continue; + throw new Error( + `${writer} refused this object metadata write: the field \`${name}\` is a ` + + `\`${String(def.type)}\` and carries ${describeTarget(def.reference)}, so it names no object ` + + 'to link to. `@objectstack/spec` refuses the same document at the server with a 422 on ' + + `\`fields.${name}.reference\`, and that refusal blocks every later save of this object for ` + + 'as long as the half-filled field rides along in the draft (objectui#7714, objectui#8057). ' + + 'Pick the target object, or change the field to a non-relationship type.', + ); + } +} diff --git a/packages/plugin-designer/src/MetadataFieldsPage.tsx b/packages/plugin-designer/src/MetadataFieldsPage.tsx index 5c42c6dcfe..d271ce2d87 100644 --- a/packages/plugin-designer/src/MetadataFieldsPage.tsx +++ b/packages/plugin-designer/src/MetadataFieldsPage.tsx @@ -46,7 +46,11 @@ import type { DesignerFieldDefinition, DesignerFieldType } from '@object-ui/type // 2026-08-28): a barrel import eagerly evaluates every other barrel member, // which widened an unrelated consumer's module graph under the prior shape. import { retiredFieldKeysFor } from '@object-ui/types/internal/retired-field-keys'; -import { MetadataClient, type MetadataClientConfig } from '@object-ui/data-objectstack'; +import { + MetadataClient, + RELATIONSHIP_TYPES_REQUIRING_REFERENCE, + type MetadataClientConfig, +} from '@object-ui/data-objectstack'; import { FieldDesigner } from './FieldDesigner'; /** Subset of the framework FieldSchema shape we render. */ @@ -497,7 +501,12 @@ function fromDesignerField( * `saveObject` — so the two writers now state ONE invariant and both exercise * it. */ -const RELATIONSHIP_TYPES_REQUIRING_REFERENCE = ['lookup', 'master_detail']; +// objectui#8676 - imported, not declared. The sibling copy this file used to +// carry lived word-for-word in `MetadataService.ts`, and a pin existed only to +// notice when the two drifted. One declaration cannot drift: it lives in +// `@object-ui/data-objectstack` beside the write doors, and a pin there DERIVES +// it from the installed spec so a contract change reddens CI rather than +// leaving both writers quietly short of the contract. /** * Why THIS value cannot be a target, and what the contract does about it — diff --git a/scripts/__tests__/check-object-metadata-write-doors.test.ts b/scripts/__tests__/check-object-metadata-write-doors.test.ts new file mode 100644 index 0000000000..76feeff2a5 --- /dev/null +++ b/scripts/__tests__/check-object-metadata-write-doors.test.ts @@ -0,0 +1,280 @@ +import { afterAll, describe, expect, it } from 'vitest'; +import fs from 'node:fs'; +import os from 'node:os'; +import path from 'node:path'; +import { fileURLToPath } from 'node:url'; + +import { analyze } from '../check-object-metadata-write-doors.mjs'; + +/** + * objectui#8676 — every in-repo DOOR that can PUT an object-metadata document + * must apply the object-metadata write invariant. + * + * objectui#7714 ruled one client behaviour and its PR implemented that ruling by + * ENUMERATING the writers it knew of. Two. objectui#8057 hit the same defect on a + * third in that card's own required dogfood; objectui#8676's sweep found nine + * more, and — the half that outlives the count — showed that the sweep shape the + * question is naturally asked in cannot see them: `client.save(` returns ZERO + * over the file objectui#8057 is entirely about. + * + * So the gate enumerates DOORS. What this file pins, in the order the gate can + * go wrong: + * + * 1. **The lit control, and a control ON that control.** An unguarded + * object-capable door must go RED and be named; the SAME door with the guard + * must stay GREEN. Without the second leg, "the plant reddens it" only + * proves the gate reacts to edits. + * 2. **The resolution hop.** The repo's central door spells its URL through a + * template, a field and a helper's return value, and says NOTHING at its own + * call site. A census that reads the call rather than resolving it reports a + * clean run with that door missing — a confident zero over a population it + * never searched, which is this card's own subject. Pinned on a fixture + * rebuilt in that shape. + * 3. **As-written beats resolved.** Substitution is textual, so a name that + * also occurs in the path (`app`, in `/meta/app/`) must not rewrite a door + * that names its own type into one that does not. + * 4. **Fail-closed on an unknown type.** A door whose type is a runtime value + * is CAPABLE, never exempt. + * 5. **A green is never "the walk found nothing."** The census must collapse + * loudly when a transport stops matching. + * 6. **This repository is green**, with every counter non-zero. + * 7. **The gate is wired** where the sibling parse-based gates run. + */ +const repoRoot = path.resolve(path.dirname(fileURLToPath(import.meta.url)), '../..'); + +const fixtures: string[] = []; +afterAll(() => { + for (const dir of fixtures) fs.rmSync(dir, { recursive: true, force: true }); +}); + +/** A throwaway tree in the shape the gate walks: `packages//src/`. */ +function tree(files: Record): string { + const root = fs.mkdtempSync(path.join(os.tmpdir(), 'write-doors-')); + fixtures.push(root); + for (const [rel, source] of Object.entries(files)) { + const full = path.join(root, rel); + fs.mkdirSync(path.dirname(full), { recursive: true }); + fs.writeFileSync(full, source, 'utf8'); + } + return root; +} + +/** An SDK door with a non-object literal type — present so the census never collapses. */ +const EXEMPT_SDK = ` +export async function saveView(client: any, name: string, body: unknown) { + await client.meta.saveItem('view', name, body); +} +`; + +describe('the lit control, and a control on that control', () => { + const rawDoor = (guardCall: string) => ` +import { assertObjectMetadataWritable } from '@object-ui/data-objectstack'; + +export async function importObjectDraft(draft: any) { + ${guardCall} + await fetch(\`/api/v1/meta/object/\${draft.name}\`, { + method: 'PUT', + body: JSON.stringify(draft.definition), + }); +} +`; + + it('RED — an object-capable door that does not reach the guard is found and named', () => { + const root = tree({ + 'packages/app-shell/src/importer.ts': rawDoor(''), + 'packages/app-shell/src/views.ts': EXEMPT_SDK, + }); + const { findings, counters } = analyze(root); + expect(findings).toHaveLength(1); + expect(findings[0].file).toBe(path.join('packages', 'app-shell', 'src', 'importer.ts')); + expect(findings[0].kind).toBe('raw'); + expect(findings[0].type).toBe('object'); + expect(counters.raw).toBe(1); + }); + + it('GREEN — the SAME door with the guard call is clean', () => { + const root = tree({ + 'packages/app-shell/src/importer.ts': rawDoor("assertObjectMetadataWritable('object', draft.definition, 'importObjectDraft');"), + 'packages/app-shell/src/views.ts': EXEMPT_SDK, + }); + const { findings, counters, collapsed } = analyze(root); + expect(findings).toHaveLength(0); + expect(counters.guarded).toBe(1); + expect(collapsed).toBe(false); + }); +}); + +describe('the resolution hop — the door that says nothing at its own call site', () => { + // Rebuilt in the shape of `MetadataClient.save`: the URL is a local `const` + // over a field, the field is set from a helper, and the helper returns a + // template over a module constant. Read AT THE CALL, that URL is the single + // identifier `url`. + const indirect = ` +const API_PREFIX = '/api/v1'; +const META_PREFIX = '/meta'; + +function buildBase(config: { baseUrl: string }): string { + return \`\${config.baseUrl}\${API_PREFIX}\${META_PREFIX}\`; +} + +export class Client { + private readonly base: string; + constructor(config: { baseUrl: string }) { + this.base = buildBase(config); + } + async save(type: string, name: string, item: unknown) { + const url = \`\${this.base}/\${type}/\${name}\`; + await fetch(url, { method: 'PUT', body: JSON.stringify(item) }); + } +} +`; + + it('finds it, and judges it CAPABLE because its type is a runtime value', () => { + const root = tree({ + 'packages/data/src/client.ts': indirect, + 'packages/data/src/views.ts': EXEMPT_SDK, + }); + const { doors, findings } = analyze(root); + const raw = doors.filter((door) => door.kind === 'raw'); + expect(raw).toHaveLength(1); + expect(raw[0].type).toBeNull(); + expect(findings.map((finding) => finding.file)).toContain(path.join('packages', 'data', 'src', 'client.ts')); + }); + + it('CONTROL — the same file with the `/meta` segment removed is not a door at all', () => { + const root = tree({ + 'packages/data/src/client.ts': indirect.replace("const META_PREFIX = '/meta';", "const META_PREFIX = '/records';"), + 'packages/data/src/views.ts': EXEMPT_SDK, + }); + const { doors } = analyze(root); + expect(doors.filter((door) => door.kind === 'raw')).toHaveLength(0); + }); +}); + +describe('as-written beats resolved', () => { + it('keeps the literal type when a local name collides with a path segment', () => { + // `app` is both the path segment and a local const. Substituting it would + // turn an exempt `app` door into a CAPABLE one and demand a guard that has + // no business there. + const root = tree({ + 'packages/app-shell/src/publish.ts': ` +export async function publish(app: Record, routeApp: string) { + await fetch(\`/api/v1/meta/app/\${routeApp}\`, { + method: 'PUT', + body: JSON.stringify({ ...app, _unpublished: false }), + }); +} +`, + 'packages/app-shell/src/views.ts': EXEMPT_SDK, + }); + const { doors, findings } = analyze(root); + expect(doors.find((door) => door.kind === 'raw')?.type).toBe('app'); + expect(findings).toHaveLength(0); + }); +}); + +describe('the SDK door, and the fail-closed direction', () => { + it('exempts a non-object literal type and judges a runtime type CAPABLE', () => { + const root = tree({ + 'packages/app-shell/src/service.ts': ` +export class Service { + async saveAnything(category: string, name: string, data: Record) { + await this.client.meta.saveItem(category, name, data); + } + async saveApp(name: string, data: Record) { + await this.client.meta.saveItem('app', name, data); + } + client: any; +} +`, + 'packages/app-shell/src/importer.ts': ` +import { assertObjectMetadataWritable } from '@object-ui/data-objectstack'; +export async function put(type: string, body: unknown) { + assertObjectMetadataWritable(type, body, 'put'); + await fetch(\`/api/v1/meta/\${type}/x\`, { method: 'PUT', body: JSON.stringify(body) }); +} +`, + }); + const { doors, findings } = analyze(root); + const sdk = doors.filter((door) => door.kind === 'sdk'); + expect(sdk.map((door) => door.type).sort()).toEqual(['app', null].sort()); + // Only the runtime-typed one is a finding; the `'app'` literal is exempt. + expect(findings).toHaveLength(1); + expect(findings[0].kind).toBe('sdk'); + expect(findings[0].type).toBeNull(); + }); + + it('ignores a `saveItem` that is not the metadata door by arity', () => { + const root = tree({ + 'packages/app-shell/src/cart.ts': 'export const put = (store: any, item: unknown) => store.saveItem(item);\n', + 'packages/app-shell/src/views.ts': EXEMPT_SDK, + }); + const { doors } = analyze(root); + expect(doors.filter((door) => door.file.endsWith('cart.ts'))).toHaveLength(0); + }); +}); + +describe('a green is never "the walk found nothing"', () => { + it('reports a COLLAPSED census when a transport kind has gone missing', () => { + const root = tree({ 'packages/app-shell/src/views.ts': EXEMPT_SDK }); + const { collapsed, counters, findings } = analyze(root); + // SDK doors exist, raw doors do not, and nothing is guarded. Findings are + // empty — which without the collapse flag would read as a clean run. + expect(findings).toHaveLength(0); + expect(counters.raw).toBe(0); + expect(collapsed).toBe(true); + }); + + it('does not collapse on a tree that has all three', () => { + const root = tree({ + 'packages/app-shell/src/views.ts': EXEMPT_SDK, + 'packages/app-shell/src/importer.ts': ` +import { assertObjectMetadataWritable } from '@object-ui/data-objectstack'; +export async function put(body: unknown) { + assertObjectMetadataWritable('object', body, 'put'); + await fetch('/api/v1/meta/object/x', { method: 'PUT', body: JSON.stringify(body) }); +} +`, + }); + expect(analyze(root).collapsed).toBe(false); + }); +}); + +describe('this repository', () => { + it('is green, and every counter that makes a green mean something is non-zero', () => { + const { findings, counters, collapsed } = analyze(repoRoot); + expect(collapsed).toBe(false); + expect(counters.raw).toBeGreaterThan(0); + expect(counters.sdk).toBeGreaterThan(0); + expect(counters.capable).toBeGreaterThan(0); + expect(counters.guarded).toBe(counters.capable); + expect(findings).toEqual([]); + }); + + it('finds the three doors this card measured, by transport rather than by list', () => { + const { doors, capable } = analyze(repoRoot); + // Named here as an ASSERTION ABOUT COVERAGE, not as the gate's input: the + // gate derives these. If a door moves or is renamed, this row is what says + // the derivation stopped reaching it. + const capableFiles = capable.map((door) => door.file.split(path.sep).join('/')); + expect(capableFiles).toContain('packages/data-objectstack/src/metadata-client.ts'); + expect(capableFiles).toContain('packages/app-shell/src/views/metadata-admin/external/api.ts'); + expect(capableFiles).toContain('packages/app-shell/src/services/MetadataService.ts'); + // ...and the exempt remainder is real, so "capable" is a narrowing rather + // than the whole census wearing a different name. + expect(doors.length).toBeGreaterThan(capable.length); + }); +}); + +describe('wiring', () => { + it('is reachable through the `pnpm check:*` alias the workflow invokes', () => { + const manifest = JSON.parse(fs.readFileSync(path.join(repoRoot, 'package.json'), 'utf8')); + expect(manifest.scripts['check:metadata-write-doors']) + .toBe('node scripts/check-object-metadata-write-doors.mjs'); + }); + + it('runs in CI, beside the sibling gates that parse sources with `typescript`', () => { + const workflow = fs.readFileSync(path.join(repoRoot, '.github/workflows/ci.yml'), 'utf8'); + expect(workflow).toContain('run: pnpm check:metadata-write-doors'); + }); +}); diff --git a/scripts/check-object-metadata-write-doors.mjs b/scripts/check-object-metadata-write-doors.mjs new file mode 100644 index 0000000000..2f81cdc141 --- /dev/null +++ b/scripts/check-object-metadata-write-doors.mjs @@ -0,0 +1,509 @@ +#!/usr/bin/env node +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * Every in-repo DOOR that can PUT an object-metadata document must apply the + * object-metadata write invariant. + * + * Run: node scripts/check-object-metadata-write-doors.mjs (also `pnpm check:metadata-write-doors`) + * node scripts/check-object-metadata-write-doors.mjs --list (the whole derived door census) + * Exit: 0 = every object-capable door reaches `assertObjectMetadataWritable`, + * 1 = at least one does not, or the census collapsed. + * + * ## The defect class this closes (objectui#8676, from objectui#7714 / objectui#8057) + * + * objectui#7714 ruled a client behaviour — a half-filled relationship is held + * client-side and the PUT body never carries one without a non-empty + * `reference` — and its PR implemented that ruling by ENUMERATING the writers it + * knew of. Two of them. objectui#8057 then hit the same defect on a third writer + * in that card's own required dogfood, and objectui#8676's sweep found nine + * more. The sentence that outlives all three cards: + * + * > a ruling that enumerates writers is only as good as the enumeration. + * + * ⭐ And the half that outlives even that: the sweep the question is naturally + * asked in CANNOT SEE its own subject. `git grep 'client\.save('` returns ZERO + * over `ResourceEditPage.tsx`, the file objectui#8057 is entirely about, because + * the call is `client.save(type, …)` and the generic argument sits between + * the name and the paren. A second writer hides identically. A third method + * (`client.meta.saveItem(`) is not that spelling at all. And the door this gate + * found that no `.save`-shaped sweep of any spelling can see is + * `importObjectDraft`, a hand-rolled `fetch` PUT to `/api/v1/meta/object/:name`. + * + * ⇒ So this gate does not enumerate writers, and ⛔ no fix for this class ever + * should. It enumerates DOORS. + * + * ## Why doors, and why that is not the same trick with a shorter list + * + * The writer set is OPEN — any component may decide to save an object, and + * nothing in the repo has to tell anyone when one is added. The door set is + * CLOSED and this repository owns it: bytes reach `PUT /meta/:type/:name` only + * through code that is in this tree. So the census below is derived from the + * TRANSPORT — what actually issues the request — and every writer, named or + * not, past or future, arrives through one of the doors it finds. A writer + * added tomorrow needs no entry anywhere; a DOOR added tomorrow reddens this + * gate by construction, because the census is re-derived on every run. + * + * That is the difference between this and a list. A list is a claim about the + * world; this is a measurement of the tree. + * + * ## The census, stated as a rule + * + * DOORS over `packages//src/**` and `apps//src/**`, excluding + * tests: + * (A) RAW — a call carrying a `method: 'PUT'` object literal at + * any argument position, one of whose other arguments RESOLVES + * to a URL on the `/meta` path. ⭐ Resolved, not read: the + * repo's central door spells its URL through a template, a + * field, and a helper's return value, and says nothing at the + * call site. See {@link resolveUrl}. + * (B) SDK — any call to a member named `saveItem` with three or + * more arguments. That is `@objectstack/client`'s metadata + * write door, which lives in a package this repo does not own + * and therefore cannot be guarded from the inside. + * TYPE for (A) the literal path segment following `/meta/`, when the URL + * spells one; for (B) the first argument, when it is a string + * literal. Anything else is UNKNOWN. + * CAPABLE TYPE is `object`, or TYPE is UNKNOWN. A door whose type is a + * literal other than `object` cannot carry an object document and + * is exempt — `view`, `app`, `flow`, `dashboard` writes are not this + * invariant's business. + * GUARDED the nearest enclosing function body contains a call to + * `assertObjectMetadataWritable`. + * FINDING a CAPABLE door that is not GUARDED. + * + * ⭐ `MetadataClient.save` call sites are deliberately NOT in the census, and + * that absence is the whole point rather than a gap: `MetadataClient.save` IS + * door (A) at `packages/data-objectstack/src/metadata-client.ts`, so every one + * of its callers is covered by guarding that single door. Enumerating them + * would re-introduce exactly the list this gate exists to abolish. + * + * ## What it deliberately does NOT answer + * + * Each is a boundary, not an oversight: + * + * 1. **Writes that are not PUT.** `publish`, `reset` and `rollback` POST to + * `/meta/:type/:name/` with no body of fields; they promote or + * discard a document the door already judged. A future POST route that + * accepts a fields-bearing body would be outside this census. + * 2. **A door built by indirection.** A `fetch` whose method string arrives in + * a variable, or whose URL is assembled far from the call, is not matched. + * The census is syntactic; it reads what is written at the call. + * 3. **Whether a guarded door's body is CORRECT.** That is + * `object-metadata-write-guard.*.test.ts`'s job. This gate answers + * coverage, and coverage only — the same split objectui#8676 was graded on. + * 4. **Metadata written by a consumer of the published packages.** A host app + * that builds its own client reaches the server without passing through + * this tree at all. + * + * ## Population self-proof + * + * A census that finds nothing passes while asserting nothing — the failure mode + * objectui#8676 is a card about. So the gate refuses to report OK unless it + * found at least one door of EACH kind and at least one guarded door. A refactor + * that renames the transport turns this red rather than green. + */ + +import { readFileSync, readdirSync, statSync } from 'node:fs'; +import { join, relative, resolve, sep } from 'node:path'; +import { fileURLToPath } from 'node:url'; +import ts from 'typescript'; + +const HERE = fileURLToPath(new URL('.', import.meta.url)); +const REPO_ROOT = join(HERE, '..'); + +/** The guard every object-capable door must reach. */ +const GUARD_SYMBOL = 'assertObjectMetadataWritable'; + +/** The metadata type whose documents this invariant judges. */ +const OBJECT_TYPE = 'object'; + +const SOURCE_EXTENSIONS = ['.ts', '.tsx']; + +function isTestPath(path) { + return ( + /\.(test|spec)\.[cm]?tsx?$/.test(path) + || path.split(sep).includes('__tests__') + || path.split(sep).includes('__mocks__') + ); +} + +function collectSources(root) { + const out = []; + let entries; + try { + entries = readdirSync(root, { withFileTypes: true }); + } catch { + return out; + } + for (const entry of entries) { + const full = join(root, entry.name); + if (entry.isDirectory()) { + if (entry.name === 'node_modules' || entry.name === 'dist') continue; + out.push(...collectSources(full)); + continue; + } + if (!SOURCE_EXTENSIONS.some((ext) => entry.name.endsWith(ext))) continue; + if (isTestPath(full)) continue; + out.push(full); + } + return out; +} + +/** Every `//src` that exists under `root`. */ +export function sourceRoots(root = REPO_ROOT) { + const roots = []; + for (const workspace of ['packages', 'apps']) { + const base = join(root, workspace); + let entries; + try { + entries = readdirSync(base, { withFileTypes: true }); + } catch { + continue; + } + for (const entry of entries) { + if (!entry.isDirectory()) continue; + const src = join(base, entry.name, 'src'); + try { + if (statSync(src).isDirectory()) roots.push(src); + } catch { + /* a package without a `src` directory is not a source root */ + } + } + } + return roots; +} + +export function parse(file) { + const text = readFileSync(file, 'utf8'); + return ts.createSourceFile(file, text, ts.ScriptTarget.Latest, true, ts.ScriptKind.TSX); +} + +/** The source text of a node, with template substitutions left as written. */ +function textOf(node) { + return node?.getText?.() ?? ''; +} + +const EXPANSION_ROUNDS = 6; +const EXPANSION_LIMIT = 40_000; + +/** + * Everything in one file that a URL expression could be spelled through: + * `const` initialisers, `this.X = …` assignments, and the return expressions of + * same-file functions. + * + * ⭐ This exists because the MOST IMPORTANT door in the repo is invisible + * without it. `MetadataClient.save` issues `this.fetchImpl(url, { method: 'PUT' })`, + * and `url` is a template over `this.base`, which the constructor sets from + * `buildBase(config)`, which returns a template over `META_PREFIX`. Read at the + * call, that URL says nothing at all. A gate that only read the call would have + * reported a clean census with the repo's central metadata door missing from it + * -- a confident zero over a population it never searched, which is the exact + * failure objectui#8676 is about. So the URL is RESOLVED rather than read. + */ +export function spellingsIn(source) { + const idents = new Map(); + const returns = new Map(); + const visit = (node) => { + if (ts.isVariableDeclaration(node) && ts.isIdentifier(node.name) && node.initializer) { + if (!idents.has(node.name.text)) idents.set(node.name.text, textOf(node.initializer)); + } + if ( + ts.isBinaryExpression(node) + && node.operatorToken.kind === ts.SyntaxKind.EqualsToken + && ts.isPropertyAccessExpression(node.left) + && node.left.expression.kind === ts.SyntaxKind.ThisKeyword + ) { + const key = `this.${node.left.name.text}`; + if (!idents.has(key)) idents.set(key, textOf(node.right)); + } + if (ts.isFunctionDeclaration(node) && node.name && node.body) { + const collected = []; + const walk = (inner) => { + if (ts.isReturnStatement(inner) && inner.expression) collected.push(textOf(inner.expression)); + ts.forEachChild(inner, walk); + }; + ts.forEachChild(node.body, walk); + if (collected.length && !returns.has(node.name.text)) returns.set(node.name.text, collected.join(' ')); + } + ts.forEachChild(node, visit); + }; + ts.forEachChild(source, visit); + return { idents, returns }; +} + +/** Replace `NAME(...)` (balanced) with `replacement`, every occurrence. */ +function replaceCall(text, name, replacement) { + let out = ''; + let index = 0; + const needle = new RegExp(`\\b${name}\\s*\\(`, 'g'); + let match; + while ((match = needle.exec(text)) !== null) { + if (match.index < index) continue; + let depth = 1; + let cursor = match.index + match[0].length; + while (cursor < text.length && depth > 0) { + if (text[cursor] === '(') depth += 1; + else if (text[cursor] === ')') depth -= 1; + cursor += 1; + } + if (depth !== 0) break; + out += text.slice(index, match.index) + `(${replacement})`; + index = cursor; + needle.lastIndex = cursor; + } + return out + text.slice(index); +} + +/** + * Expand a URL expression until it either spells a `/meta` path or stops + * growing. Each name is substituted at most once, so a self-referential + * spelling terminates instead of looping. + */ +export function resolveUrl(node, spellings) { + let text = textOf(node); + const used = new Set(); + for (let round = 0; round < EXPANSION_ROUNDS; round += 1) { + const before = text; + for (const [name, value] of spellings.returns) { + if (used.has(`fn:${name}`) || !new RegExp(`\\b${name}\\s*\\(`).test(text)) continue; + used.add(`fn:${name}`); + text = replaceCall(text, name, value); + } + for (const [name, value] of spellings.idents) { + if (used.has(name)) continue; + const pattern = name.startsWith('this.') + ? new RegExp(`\\bthis\\.${name.slice(5)}\\b`, 'g') + : new RegExp(`\\b${name}\\b`, 'g'); + if (!pattern.test(text)) continue; + used.add(name); + text = text.replace(pattern, `(${value})`); + } + if (text === before || text.length > EXPANSION_LIMIT) break; + } + return text; +} + +/** `'object'` for a string literal argument, `null` for anything else. */ +function literalString(node) { + if (!node) return null; + if (ts.isStringLiteral(node) || ts.isNoSubstitutionTemplateLiteral(node)) return node.text; + return null; +} + +/** Does this object literal carry `method: 'PUT'`? */ +function isPutInit(node) { + if (!node || !ts.isObjectLiteralExpression(node)) return false; + return node.properties.some((prop) => { + if (!ts.isPropertyAssignment(prop)) return false; + const key = ts.isIdentifier(prop.name) || ts.isStringLiteral(prop.name) ? prop.name.text : null; + return key === 'method' && literalString(prop.initializer) === 'PUT'; + }); +} + +/** + * The metadata type a URL expression spells, or `null` when it does not spell + * one literally. Reads the segment that follows `/meta/` in the written text, + * so both `'/api/v1/meta/object/' + x` and a template literal are covered. + */ +function typeFromUrl(resolved) { + const match = /\/meta\/([A-Za-z0-9_-]+)(\/|$)/.exec(resolved); + return match ? match[1] : null; +} + +/** `/metadata` is a different path; the negative lookahead keeps it out. */ +function urlTouchesMeta(resolved) { + return /\/meta(?![A-Za-z0-9_-])/.test(resolved); +} + +/** The nearest enclosing function-like node, or the source file. */ +function enclosingFunction(node) { + let current = node.parent; + while (current) { + if ( + ts.isFunctionDeclaration(current) + || ts.isFunctionExpression(current) + || ts.isArrowFunction(current) + || ts.isMethodDeclaration(current) + || ts.isConstructorDeclaration(current) + ) { + return current; + } + current = current.parent; + } + return node.getSourceFile(); +} + +function callsGuard(scope) { + let found = false; + const visit = (node) => { + if (found) return; + if (ts.isCallExpression(node)) { + const callee = node.expression; + const name = ts.isIdentifier(callee) + ? callee.text + : ts.isPropertyAccessExpression(callee) + ? callee.name.text + : null; + if (name === GUARD_SYMBOL) { + found = true; + return; + } + } + ts.forEachChild(node, visit); + }; + ts.forEachChild(scope, visit); + return found; +} + +function lineOf(source, node) { + return source.getLineAndCharacterOfPosition(node.getStart(source)).line + 1; +} + +export function censusDoors(root = REPO_ROOT) { + const doors = []; + for (const sourceRoot of sourceRoots(root)) { + for (const file of collectSources(sourceRoot)) { + const source = parse(file); + const rel = relative(root, file); + const spellings = spellingsIn(source); + const visit = (node) => { + if (ts.isCallExpression(node)) { + const callee = node.expression; + const memberName = ts.isPropertyAccessExpression(callee) ? callee.name.text : null; + + // (A) RAW — a `PUT` whose URL RESOLVES to a `/meta` path. The init may + // sit at any argument position (a `fetch`-alike wrapper takes it last), + // so every argument is offered to both tests rather than positions 0 + // and 1 being assumed. + if (node.arguments.some((argument) => isPutInit(argument))) { + const candidates = node.arguments.filter((argument) => !isPutInit(argument)); + // As WRITTEN first, RESOLVED only as a fallback. Substitution is + // textual, so a name that also occurs inside the path (`app`, in + // `/meta/app/`) would be rewritten and turn a door that names its + // type into one that does not. Reading the literal first keeps the + // precise answer precise; resolution is for the doors that say + // nothing at the call at all. + const metaUrl = candidates.map((argument) => textOf(argument)).find((written) => urlTouchesMeta(written)) + ?? candidates.map((argument) => resolveUrl(argument, spellings)).find((resolved) => urlTouchesMeta(resolved)); + if (metaUrl !== undefined) { + doors.push({ + kind: 'raw', + file: rel, + line: lineOf(source, node), + type: typeFromUrl(metaUrl), + guarded: callsGuard(enclosingFunction(node)), + written: textOf(callee), + }); + } + } + + // (B) SDK — `@objectstack/client`'s metadata write door. + if (memberName === 'saveItem' && node.arguments.length >= 3) { + doors.push({ + kind: 'sdk', + file: rel, + line: lineOf(source, node), + type: literalString(node.arguments[0]), + guarded: callsGuard(enclosingFunction(node)), + written: textOf(callee), + }); + } + } + ts.forEachChild(node, visit); + }; + ts.forEachChild(source, visit); + } + } + return doors.sort((a, b) => (a.file === b.file ? a.line - b.line : a.file.localeCompare(b.file))); +} + +/** + * The whole verdict for one tree, as data. The CLI below is a thin printer over + * it, and the gate's own suite drives it against fixture trees. + */ +export function analyze(root = REPO_ROOT) { + const doors = censusDoors(root); + const capable = doors.filter((door) => door.type === null || door.type === OBJECT_TYPE); + const findings = capable.filter((door) => !door.guarded); + const counters = { + doors: doors.length, + raw: doors.filter((door) => door.kind === 'raw').length, + sdk: doors.filter((door) => door.kind === 'sdk').length, + capable: capable.length, + guarded: capable.filter((door) => door.guarded).length, + exempt: doors.length - capable.length, + }; + const collapsed = counters.raw === 0 || counters.sdk === 0 || counters.guarded === 0; + return { doors, capable, findings, counters, collapsed }; +} + +/** `true` when this module was started as a program rather than imported. */ +const RUN_AS_CLI = process.argv[1] !== undefined + && fileURLToPath(import.meta.url) === resolve(process.argv[1]); + +if (!RUN_AS_CLI) { + // Imported by the suite that tests it; the CLI below must not run or exit. +} else { + const { doors, capable, findings, counters } = analyze(); + const rawCount = counters.raw; + const sdkCount = counters.sdk; + const guardedCount = counters.guarded; + + if (process.argv.includes('--list')) { + for (const door of doors) { + const verdict = door.type !== null && door.type !== OBJECT_TYPE + ? `exempt (type '${door.type}')` + : door.guarded + ? 'guarded' + : 'UNGUARDED'; + console.log( + `${door.kind.toUpperCase().padEnd(3)} ${door.file}:${door.line} ` + + `type=${door.type ?? ''} ${verdict} [${door.written}]`, + ); + } + } + + if (rawCount === 0 || sdkCount === 0 || guardedCount === 0) { + console.error( + 'x the door census collapsed and this gate is asserting nothing:\n' + + ` raw PUT doors found : ${rawCount}\n` + + ` SDK saveItem doors : ${sdkCount}\n` + + ` guarded doors : ${guardedCount}\n\n` + + 'Every one of those must be non-zero for a green run to mean anything. A zero here is a\n' + + 'renamed transport, a moved source root, or a matcher that stopped matching -- not a repo\n' + + 'with no metadata writes. Fix the census before reading any verdict off it (objectui#8676).', + ); + process.exit(1); + } + + if (!findings.length) { + console.log( + `OK ${doors.length} metadata write door(s) derived (${rawCount} raw PUT, ${sdkCount} SDK), ` + + `${capable.length} can carry an object document, ${guardedCount} reach ${GUARD_SYMBOL}, ` + + `${doors.length - capable.length} exempt by a non-object literal type -- no writer list anywhere.`, + ); + process.exit(0); + } + + console.error( + `x ${findings.length} metadata write door(s) can PUT an object document without the invariant:\n`, + ); + for (const finding of findings) { + console.error( + ` ${finding.file}:${finding.line} (${finding.kind}) ` + + `type=${finding.type ?? ''}\n` + + ` ${finding.written}(...)`, + ); + } + console.error( + `\nEach door above puts bytes on the wire without calling ${GUARD_SYMBOL}, so objectui#7714's ruled\n` + + 'invariant -- a PUT body never carries a relationship field without a non-empty `reference` --\n' + + 'does not hold for anything that writes through it. That ruling was enumerated in prose twice and\n' + + 'falsified twice (objectui#8057, objectui#8676).\n\n' + + 'Fix it AT THE DOOR: call assertObjectMetadataWritable(type, body, ) from\n' + + '@object-ui/data-objectstack before the request. ⛔ Do not fix it by guarding the writers that\n' + + 'call the door -- that is the enumeration this gate exists to make unnecessary.', + ); + process.exit(1); +} From 2981c4ade5f37f21c1671941cf76d65a9d95d4ff Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 12 Sep 2026 02:45:36 +0000 Subject: [PATCH 2/3] docs(data-objectstack): document the write guard, and re-count the spec-pin ledger row The ledger row for `MetadataService.ts` said three `@objectstack/spec` 17.2.0 citations; the tree now has two. The third sat on the relationship-type list's docblock, which moved to `object-metadata-write-guard.ts` and deliberately did NOT take the version stamp with it -- the claim it stamped is re-measured on every run by that module's derivation pin instead of recalled at a version. Part of #8676 Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01UzHd6hDYatoDn17BuwKxnZ --- packages/data-objectstack/README.md | 41 +++++++++++++++++++++ scripts/check-installed-spec-pin-claims.mjs | 4 +- 2 files changed, 43 insertions(+), 2 deletions(-) diff --git a/packages/data-objectstack/README.md b/packages/data-objectstack/README.md index 4e009cc944..5a57a959c9 100644 --- a/packages/data-objectstack/README.md +++ b/packages/data-objectstack/README.md @@ -708,6 +708,47 @@ still succeeds, but non-atomically via the fallback above. Treat the advertised capability as the floor for the atomicity guarantee, not as a connection prerequisite. +## Object-Metadata Write Guard + +`MetadataClient.save` refuses an `object` document whose `fields` carry a +relationship field (`lookup`, `master_detail`) with a missing, empty or +whitespace-only `reference`, **before** issuing the request: + +```ts +import { MetadataClient } from '@object-ui/data-objectstack'; + +const client = new MetadataClient({ baseUrl: '/api/v1' }); + +await client.save('object', 'account', { + name: 'account', + fields: { owner: { type: 'lookup', label: 'Owner' } }, +}); +// throws: MetadataClient.save refused this object metadata write: the field +// `owner` is a `lookup` and carries no `reference` key at all ... +``` + +Nothing that previously succeeded now fails. `@objectstack/spec` refuses the same +document at the server with a 422 on `fields.owner.reference`, and that refusal +blocks every *later* save of the object for as long as the half-filled field +rides along in the draft. The guard moves the identical refusal earlier, names +the field while it is still on screen, and leaves the draft in the client. Writes +of every other metadata type are untouched, and the guard never strips the +offending field — a dropped field reported as saved would be a silent deletion. + +Hosts that write object metadata through their own transport can apply the same +invariant at their own door: + +```ts +import { assertObjectMetadataWritable } from '@object-ui/data-objectstack'; + +assertObjectMetadataWritable('object', body, 'myUploader'); +``` + +`RELATIONSHIP_TYPES_REQUIRING_REFERENCE` and `OBJECT_METADATA_TYPE` are exported +beside it. The relationship-type set is derived from the installed +`@objectstack/spec` by this package's own pin, so it follows the contract rather +than a remembered list. + ## User-Scoped State Adapter In addition to the main `DataSource` adapter, this package ships diff --git a/scripts/check-installed-spec-pin-claims.mjs b/scripts/check-installed-spec-pin-claims.mjs index a3cb186a8a..ee3b67e8cb 100644 --- a/scripts/check-installed-spec-pin-claims.mjs +++ b/scripts/check-installed-spec-pin-claims.mjs @@ -506,9 +506,9 @@ export const LEDGER = [ file: "packages/app-shell/src/services/MetadataService.ts", package: "@objectstack/spec", version: "17.2.0", - sites: 3, + sites: 2, class: "stale", - why: "Three sites in one docblock family, all stamping `unrecognized_keys` behaviour \"measured against the installed @objectstack/spec 17.2.0 (ESM build)\".", + why: "Two sites in one docblock family, both stamping `unrecognized_keys` behaviour \"measured against the installed @objectstack/spec 17.2.0 (ESM build)\". Was three until objectui#8676: the third sat on the relationship-type list's docblock, which moved to `@object-ui/data-objectstack`'s `object-metadata-write-guard.ts` \u2014 and did NOT take the version stamp with it. The claim it stamped is now re-measured on every run by that module's derivation pin instead of recalled at a version, which is what this ledger wants of a citation rather than one more row.", }, { file: "packages/app-shell/src/views/RecordDetailView.relatedListFilter-4664.test.tsx", From 4e03a42fc1e09dffaa38e85bc0fe96a026356daf Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 12 Sep 2026 03:19:35 +0000 Subject: [PATCH 3/3] fix(scripts): route the write-door gate through the shared entry predicate, and make the README snippet compile MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two CI reds, both in code this PR adds, and both in gates I had not run in the form CI runs them. 1. `check:entry-guard` — the new gate hand-typed its own `process.argv[1]` comparison. Node resolves symlinks for the module graph but leaves `process.argv[1]` as the caller typed it, so a hand-typed guard reached through a symlink answers false and the gate does NOTHING: exit 0, no output, indistinguishable from a pass to a wrapper holding only `result.status`. Now `isEntrypoint(import.meta.url)` from `scripts/invoked-as.mjs`, the one predicate, like its 92 neighbours. The now-unused `resolve` import is dropped with it. 2. `Doc Snippet Type Check` — the README's second example called `assertObjectMetadataWritable('object', body, ...)` with no `body` in scope (TS2304). It is now a complete function that also shows the shape the example is about: guard first, then the PUT. ⛔ Neither gate was weakened or exempted, and no ledger row was added: `KNOWN_HAND_TYPED_GUARDS` and `UNGATED_EXAMPLES` are both shrink-only. Part of #8676 Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01UzHd6hDYatoDn17BuwKxnZ --- packages/data-objectstack/README.md | 9 ++++++++- scripts/check-object-metadata-write-doors.mjs | 14 ++++++++++---- 2 files changed, 18 insertions(+), 5 deletions(-) diff --git a/packages/data-objectstack/README.md b/packages/data-objectstack/README.md index 5a57a959c9..f74328406c 100644 --- a/packages/data-objectstack/README.md +++ b/packages/data-objectstack/README.md @@ -741,7 +741,14 @@ invariant at their own door: ```ts import { assertObjectMetadataWritable } from '@object-ui/data-objectstack'; -assertObjectMetadataWritable('object', body, 'myUploader'); +async function uploadObject(name: string, body: unknown) { + assertObjectMetadataWritable('object', body, 'uploadObject'); + await fetch(`/api/v1/meta/object/${encodeURIComponent(name)}`, { + method: 'PUT', + headers: { 'Content-Type': 'application/json' }, + body: JSON.stringify(body), + }); +} ``` `RELATIONSHIP_TYPES_REQUIRING_REFERENCE` and `OBJECT_METADATA_TYPE` are exported diff --git a/scripts/check-object-metadata-write-doors.mjs b/scripts/check-object-metadata-write-doors.mjs index 2f81cdc141..f1b38c6c6b 100644 --- a/scripts/check-object-metadata-write-doors.mjs +++ b/scripts/check-object-metadata-write-doors.mjs @@ -105,9 +105,16 @@ */ import { readFileSync, readdirSync, statSync } from 'node:fs'; -import { join, relative, resolve, sep } from 'node:path'; +import { join, relative, sep } from 'node:path'; import { fileURLToPath } from 'node:url'; import ts from 'typescript'; +// ⛔ NOT a hand-typed `process.argv[1]` comparison. Node resolves symlinks for +// the module graph but leaves `process.argv[1]` as the caller typed it, so a +// hand-typed guard reached through a symlink answers false and the gate does +// NOTHING -- exit 0, no output, which a wrapper holding only `result.status` +// cannot tell apart from a pass. `scripts/invoked-as.mjs` is the one predicate, +// and `check:entry-guard` enforces that every `scripts/` entry goes through it. +import { isEntrypoint } from './invoked-as.mjs'; const HERE = fileURLToPath(new URL('.', import.meta.url)); const REPO_ROOT = join(HERE, '..'); @@ -439,10 +446,9 @@ export function analyze(root = REPO_ROOT) { } /** `true` when this module was started as a program rather than imported. */ -const RUN_AS_CLI = process.argv[1] !== undefined - && fileURLToPath(import.meta.url) === resolve(process.argv[1]); +const invokedDirectly = isEntrypoint(import.meta.url); -if (!RUN_AS_CLI) { +if (!invokedDirectly) { // Imported by the suite that tests it; the CLI below must not run or exit. } else { const { doors, capable, findings, counters } = analyze();