Skip to content

Commit cf60dbc

Browse files
fix(metadata-protocol)!: the ADR-0010 _lock gate reads the row the read serves for the request's organization (#21716) (#21737)
Fixes #21716 Clause-②: no (narrowing) ## What was wrong The two item reads (`getMetaItem`, and `getMetaItemLayered`, which serves `GET /api/v1/meta/:type/:name/layers`) resolve a stored row by precedence: the organization's own row, else the env-wide row (ADR-0005). The ADR-0010 `_lock` gate's overlay limb (`getEffectiveLock`) asked for one row only: `organization_id` equal to the request's organization. So when an organization had no row of its own and the env-wide row declared `_lock: 'full'`, the reads served the env-wide row and published `lock: full`, `editable: false`, `deletable: false`, while that organization's save, publish, rollback and delete were admitted. ADR-0010 §3.3 says `full` means "Overlay writes rejected". This is the third member of the family, on the organization axis. The first was the package door (PR #21693) and the second the topology axis (PR #21715). ## H1: the two resolutions, measured at `c43a8ae612` Measured on the real `ObjectStackProtocolImplementation` over an engine double, for a `view`. The table was identical on an environment kernel and a host-config kernel. "Door" is the overlay limb's own `sys_metadata` query. | stored rows | request | both reads serve | door read at base | door verdict | |---|---|---|---|---| | env-wide `full` | no organization | env-wide row, `full` | `organization_id` null: env-wide row | `full`: agrees | | env-wide `full` | `org_a` | env-wide row, `full` | `organization_id = org_a`: nothing | `none`: **split** | | org `full` | no organization | nothing, `none` | null: nothing | `none`: agrees | | org `full` | `org_a` | org row, `full` | org_a: org row | `full`: agrees | | env `full` + org `none` | no organization | env-wide row, `full` | env-wide row | agrees | | env `full` + org `none` | `org_a` | org row, `none` | org row | agrees | | env `none` + org `full` | no organization | env-wide row, `none` | env-wide row | agrees | | env `none` + org `full` | `org_a` | org row, `full` | org row | agrees | | neither | either | nothing, `none` | nothing | agrees | There is one split cell per kernel, and it is the card's. ## What changed (H2) Only `packages/metadata-protocol/src/protocol.ts` changes at runtime. - **One resolution.** A new private method, `findServedOverlayRow`, holds the served-row resolution. It applies the org-scoped row first and the env-wide row as the fallback, with ADR-0048 prefer-local inside each scope, and returns the row and its scope. Three callers now use it, and the three hand-written copies are gone: - `getMetaItem`'s row read and its draft-preview arm; - `getMetaItemLayered`'s overlay layer; - `getEffectiveLock`'s overlay limb. - **The same organization gate.** The gate passes the organization through `organizationIdForMetaRead`, the predicate both reads apply. So on a type with no per-org channel, the door ignores an org-scoped row exactly as the reads do. Pin 4 holds this. - **One declared difference, `otherSpelling`.** The reads keep their at-rest tolerance: as a last resort in each scope, they also read a row stored under the type's other spelling. The gate passes `false` and stays on the canonical spelling. This was measured, not assumed: - Full reuse turned `packages/objectql/src/protocol-meta-type-canonicalization.test.ts` red ("a plural-spelled write addresses the canonical namespace and no other"). On a create, the gate queried `type: 'actions'` after the canonical row missed. - That test pins #4432's rule that a write addresses only the canonical namespace. Changing it would loosen it, so the gate keeps the rule and the test is untouched. - The difference is stated on `findServedOverlayRow` and in the gate's header. - **A stale docblock fixed.** The `getEffectiveLock` header still told callers to gate on `environmentId`, which has not been true since #21694. It now says the method answers alike on every topology. - **The ledger.** `scripts/engine-double-contract.pinned.json` gains one row for the new pin's `findOne` double. It was written by the gate's own `--write`: 1 added, 0 lost. No `packages/spec/src/**` file is touched, so no contract review is owed on that ground. No governed surface is touched. ## H3: precedence when both rows exist The reads serve the organization's own row to that organization. That precedence is the ruling recorded in `getMetaItem` (ADR-0005: precedence, never a merge). The gate now binds the lock of that same row, whatever the env-wide row declares, and pin 3 covers it. ADR-0010 §3.3 states its lock table per item and records no cross-scope cascade. I found no text under which the reads' precedence is wrong, so the precedence is unchanged here. ## H4: every caller moves together `getEffectiveLock` has two callers. Both read the new limb, because the change is inside it: - `lockWriteRefusal` is reached through: - `assertLockAllowsWrite`, from `saveMetaItem` and `rollbackMetaItem`; - `promoteDraftForPublish`, from `publishMetaItem` and `publishPackageDrafts`. - `assertLockAllowsDelete` is reached from `deleteMetaItem`, which `deletePackage` and `discardPackageDrafts` also call. Pin 2 drives save, delete, publish and rollback. ## What moves (H5) - **Tests: none re-aimed.** No existing test changed in the final shape. - The one red seen during the work was the canonicalization case above. It was caused by a first shape of this fix, not by the split, and it is green again under the final shape. - `metadata-protocol`: 212 files and 3675 tests pass (3589 before, plus 86 new). - `objectql`: 372 files and 7458 tests pass. - **Examples.** `git grep` finds no `_lock` and no `protection:` under `examples/**`. - **Shipped locks.** `platform-objects` declares `protection.lock` only on `app` items (`setup`, `studio`, `account`) and `object` items (`sys_*`). Both types are `allowOrgOverride: false`, so the gate never carries an organization for them, and their artifact limb is unchanged. - **Writers that reach the gate with an organization:** - over the wire, REST `PUT`, `DELETE`, publish and rollback, and the runtime dispatcher's save. They carry `organizationIdForMetaWrite`, which is non-empty only for `view`, `dashboard`, `report`, `translation` and `email_template`; - in process, `migrateStoredMetadata` (the row's own organization, so its own row is served and nothing changes), `deletePackage` and `discardPackageDrafts` (the row's organization), and `duplicatePackage` (a new name). - **Newly refused.** An org-scoped write of one of those five types is now refused `ITEM_LOCKED` / 403 when that organization has no row of its own and the env-wide row's lock refuses the operation. On a type with no per-org channel, an in-process removal of a pre-#6190 org-scoped row is now judged by the env-wide row's lock, the row both reads serve. - **The dev server's boot and seed replay.** Seed rows are type `seed`, which has no per-org channel, and they name no organization. The boot path reads; it does not write through these doors. This was measured by `git grep` of every non-test caller of the four write verbs. **NOT MEASURED: a live server boot** (no dev server was started for this card). - **Cost.** With an organization, the gate makes 2 `findOne` reads on a miss (the org row, then the env-wide row) where it made 1. Without an organization nothing changes. ## Pins `packages/metadata-protocol/src/protocol.lock-org-axis-agree.test.ts` has 86 cases. Each one drives the real doors and the real reads on one protocol instance, and every refusal asserts `code` and `status` (ADR-0112). 1. **The family's enumeration pin (triage's acceptance).** One table covers kernel topology (environment, host-config), row scope (env-wide, org-scoped), request scope (no organization, `org_a`), every `MetadataLockSchema` level (read off the schema itself) and operation (save, delete). That is 64 rows, each its own named case. - In every row the door admits exactly when the envelope says `editable` (save) or `deletable` (delete). - Both reads must agree with each other first. - A span check holds the table size to the product of the axes, and a lit control proves the org axis reaches the env-wide row. 2. **The measured defect.** An env-wide `_lock: 'full'` row is tested on both kernels. - The org-scoped read says `editable: false`. - Save and delete are refused `ITEM_LOCKED` / 403 with `lock: 'full'`. The denial rows are written to `sys_metadata_audit` under `org_a`. - Publish and rollback are refused the same way. 3. **Precedence (H3).** Three lock pairs ((full, none), (none, full), (no-delete, no-overlay)) are tested against both request scopes on both kernels. The served row is named in every case, and both doors follow it. 4. **The organization gate.** A `page` (no per-org channel) is tested with an env-wide row and an org-scoped residue row whose locks differ. The read serves the env-wide row, and the door binds that row's lock. ## Reverse verification Both ablations ran from the committed fix (HEAD `7b37480d8c`), through `scripts/ablation-replace.mjs` in wrap mode, inside a script whose `EXIT INT TERM` trap restores from `HEAD` by absolute path. The pin imports `./protocol.js`, which resolves to source, so no rebuild is in the path. The expected direction was declared before each run. 1. **The exact-`organization_id` limb restored** (one `findOne` on `organization_id: organizationId ?? null`). - The anchor went from 1 to 0, and 1 marker was on disk. The blob went from `e6a207612cc4` to `c598f7de3b47`. - Predicted: 16 red, 70 green. Measured: **16 failed, 70 passed**. - Red: the 8 org-axis cells of pin 1 (an env-wide row, an `org_a` request, with `no-overlay`/`full` on save and `no-delete`/`full` on delete, on both kernels), all 4 cases of pin 2, and all 4 of pin 4. - Green: the other 56 rows of pin 1, the span check and the lit control, and all of pin 3. 2. **The organization gate removed from the door only.** - The anchor went from 1 to 0, and 1 marker was on disk. - Predicted: 4 red. Measured: **4 failed, 82 passed**, all of them pin 4. **Restore.** After each leg, the blob equals the `HEAD` blob `e6a207612cc4`, `git diff HEAD` is empty, and `git status --porcelain` is empty. Both the tool and the trap proved it. ## Tests On head `87e350bbaf`, after merging `origin/main` at `316be321ef` (which touched `runtime` and `scripts/` only): - `@objectstack/metadata-protocol`: - `vitest run`: 212 files passed and 3 skipped; 3675 tests passed and 19 skipped. - `typecheck` (`tsc --noEmit`) is green. `--listFiles` compiles 215 test files, including the new pin. - `@objectstack/objectql`, against the rebuilt `metadata-protocol` dist: - `vitest run --project local`: 372 files and 7458 tests passed. - `--project repo`: 1 file and 5 tests passed. - **Lint, narrowed and proved.** `eslint --no-inline-config --format json` over the two changed TypeScript files gives 2 file results, with 0 errors and 0 warnings. - The population comes from ESLint's own config: `isPathIgnored` is false for both files, and each computes a 5-rule config. - Invariance: neither computed config has `parserOptions.project` or `projectService`, and `eslint.config.mjs` states it never enables type-aware linting. So this diff cannot move any untouched file's verdict. NOT MEASURED, owned by CI or out of reach here: - HTTP: the reach is on the real protocol over doubles. - a live server boot. - the `rest` and `runtime` suites. They are consumers, and no export, spec contract or wire shape changes. - the Dogfood Regression Gate. - Temporal Conformance. - the whole-workspace type-check lanes. - the full `pnpm lint`. ## Gates All of these ran on head `87e350bbaf`, after the final commit. Each exit code was captured before any pipe. - **Derived set.** `node scripts/pm/dispatch-gates.mjs --commands` (no paths) derives 72 families. All 72 exit 0. The `--ran` reconciliation reads "72 derived, 72 run, 0 NOT-MEASURED, 0 UNRUN". - Among them: `check:adr-0087-registration` (1 breaking changeset, carrying its disposition), `check:changeset-no-major`, `check:empty-changeset`, `check:engine-double-contract` (855 rows held), `check:nul-bytes`, `check:doc-authoring`, `check:cross-package-test-inputs`, `check:test-source-alias`, `check:published-files`, `check:dts-closure`, `check:dual-build-cjs-loads` and `check:lean-entry-closure`. - The last two first answered PREREQUISITE NOT MET (exit 3). They were re-run after a full `turbo run build` and exit 0. - **Artifact-roster block.** All 54 roster commands were run; 51 exit 0. - `check-closing-target-claim`, `check-partof-closing-keyword` and `check-single-claim-paths` exit 2 NOT WIRED, because they read a pull request. - **The four symbol-anchor sweeps.** All four exit 0: - `check:adr-symbol-anchors`: 2167 anchors across 140 records resolve. - `check:scripts-symbol-anchors`: 3760 anchors across 282 scripts resolve. - `check:spec-docblock-symbol-anchors`: 4950 anchors across 1868 spec sources resolve. - `check:adr-anchors`: OK. ## Acceptance notes Three members of the same family sit outside triage's five axes. Each was measured on the real reads and the gate over doubles, at `c43a8ae612`, and none is fixed here. They are reported to the seat for its call. - **The artifact axis: an explicit `none`.** A packaged view declares `_lock: 'none'`, and its stored row declares `full`. - Both reads say `editable: true`, because `mergeArtifactProtection` lets the artifact's explicit value win. - The gate refuses, because its artifact limb skips `none` and its overlay limb finds `full`. - The door is stricter than the read. - **The layered read: a packaged item's stored lock.** A packaged view declares no lock, and its stored row declares `full`. - `getMetaItem` says `full` / `editable: false`, which agrees with the door. - `getMetaItemLayered` says `none` / `editable: true`, because its lock source is `code ?? overlay`. - The two reads disagree with each other. - **The other spelling.** This is the declared difference above. A pre-#4432 row stored under the plural spelling is served by the reads when no canonical row exists in that scope. The gate does not read it. No live write mints such a row. Two smaller notes: - **The package axis.** The gate asks without a `packageId`, while a read that names one prefers that package's row (ADR-0048). The two can split only when one (type, name, scope) holds rows from two packages. Not measured. - **One pathological layered case.** `getMetaItemLayered` used to fall back to the env-wide row when an org row's stored body was JSON `null`. It now reports the org row, as `getMetaItem` always did. --- _Generated by [Claude Code](https://claude.ai/code/session_017ErfyP2Rx7XWHJA27QjyUi)_ --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent 72b6333 commit cf60dbc

4 files changed

Lines changed: 488 additions & 116 deletions

File tree

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
---
2+
'@objectstack/metadata-protocol': minor
3+
---
4+
5+
fix(metadata-protocol)!: the ADR-0010 `_lock` gate reads the row the read serves for the request's organization, so an env-wide row's lock binds an organization with no row of its own (#21716)
6+
7+
Clause-②: no (narrowing)
8+
9+
<!-- adr-0087: not-required (no-migration-prescription) a write-door verdict that now reads one more stored row: the per-item _lock gate (save, publish, rollback, delete) used to read only the stored row whose organization equals the request's, and now reads the row both metadata reads serve for that organization, which is the env-wide row when the organization has none of its own. No authorable key, spelling, export or stored shape is retired or renamed: MetadataLock keeps its four states, every stored row keeps parsing and is neither read differently by the reads nor rewritten by an upgrade, and which items an operator meant to keep writable for an organization is not something a ledger entry can rewrite. The other categories are closed on facts: the package publishes (not unpublished); no ADR-0087 id covers a lock verdict (not already-registered); and the change is a door verdict, not a TypeScript declaration (not runtime-interface-only or type-surface-only). -->
10+
11+
**BREAKING**: an organization-scoped `/meta` write that used to be accepted can now be refused. The item reads (`getMetaItem`, `getMetaItemLayered`) serve an organization its own stored row, and the env-wide row when it has none (ADR-0005). The item-level `_lock` gate read only the organization's own row. So when the env-wide row declared a lock, an organization with no row of its own read `lock: "full"` and `editable: false`, while its `saveMetaItem`, `publishMetaItem`, `rollbackMetaItem` and `deleteMetaItem` were admitted. The gate now reads the row the reads serve, through the same resolution, and answers `403 ITEM_LOCKED` where the envelope says the write is not allowed. It ships as `minor` under the launch-window convention for accept-set narrowings. No export is added or removed.
12+
13+
**What is refused now.** On every kernel topology, a save, publish or rollback with an organization of an item whose env-wide stored row declares `_lock: "no-overlay"` or `"full"`, and a delete with an organization of one whose env-wide row declares `"no-delete"` or `"full"`, when that organization has no stored row of its own for the item. Each refusal writes its `denied` row to `sys_metadata_audit` under the requesting organization. Over the wire this reaches the five per-org overridable types (`view`, `dashboard`, `report`, `translation`, `email_template`): the REST and dispatcher write doors already send no organization for any other type. For those other types the reads never serve an organization-scoped row, and now neither does the gate: an in-process removal of a pre-#6190 organization-scoped row of such a type (`deletePackage`, `discardPackageDrafts`) is judged by the env-wide row's lock, the row both reads serve. If a deployment relied on writing such an item per organization: change or remove the env-wide row's lock (a `no-delete` row can still be saved, a `no-overlay` row deleted), or keep the lock and author the organization's variant under a new name.
14+
15+
**Unchanged.** When the organization has a stored row of its own, that row is the one both reads serve, and its `_lock` decides, whatever the env-wide row declares (ADR-0005 precedence, never a merge). A request with no organization. The packaged artifact's lock, which still wins when it declares one. Both reads, apart from one case in `getMetaItemLayered`: it now serves an organization's own stored row whose body is JSON `null`, as `getMetaItem` already did, instead of falling back to the env-wide row, because the two reads now share one row resolution. Only residue can reach it: no live writer stores a `null` body (measured: `SysMetadataRepository.put`, the writer behind every `/meta` save, stores `{}` for an absent body; `saveMetaItem` refuses a `null` item with `400 INVALID_REQUEST`; and the only other `sys_metadata` writer, the datasource admin plugin, stores an object env-wide). The gate addresses the canonical type spelling only; the reads' last-resort read of a row stored under the type's other spelling is not extended to the write path.

0 commit comments

Comments
 (0)