Skip to content

Commit 69913f8

Browse files
fix(metadata-protocol)!: rollbackMetaItem and revertCommit ask the save door's refusal family before they write a history body back (#25312)
<!-- claude-projects-attribution: {"github_login":"os-bill"} --> _Requested by **Bill** · [project thread](https://claude.ai/code/project/chan_016iMXWSZDzqWRWvWzdNo8wg?thread=cmsg_016iMXWSZDzqWRWvWzdNo8wgULzXHHREefG78bonpN6vqt)_ Fixes #25278 Clause-②: no (narrowing) Metadata-protocol: narrowed > **BREAKING** (what a metadata restore writes narrows). FROM: `rollbackMetaItem` and `revertCommit`'s restore limb wrote back a stored version that the save door refuses today. TO: they refuse it with the save door's own code and status and store nothing. Fix: restore another version, or change the other half of the pair first, then restore. The two restore doors (`rollbackMetaItem`, `POST /api/v1/meta/TYPE/NAME/rollback`; and `revertCommit`'s restore limb, which `rollbackToPackageCommit` also uses) write a history body back as the active row through `repo.restoreVersion`. Before this PR they asked only the save door's `name` judge and permission-set owner judge (`restoredBodyWriter`). They asked none of the refusals `saveMetaItem` runs after its schema gate. A version saved before a refusal existed, or one half of a pair whose other half moved after the version was saved, came back live. ## Census (base `5801a4f2c`) The refusals `saveMetaItem` runs after its Zod gate (`protocol.ts:23986`), in its order, and whether each restore door asked them: | # | Save-door refusal | Call site on base · function | `rollbackMetaItem` before | `revertCommit` before | After this PR | |---|---|---|---|---|---| | 1 | hook with no `body` | `:24094` · `runtimeHookWithoutBodyRefusal` (`:1000`) | no | no | asked | | 2 | second default record page of an object | `:24107` · `defaultRecordPageSaveRefusal` (`:22429`) | no | no | asked (verb `restored`) | | 3 | `external` binding on a managed datasource, both halves | `:24118` · `externalBindingSaveRefusal` (`:22542`) | no | no | asked | | 4 | `runAs: 'system'` flow an anonymous endpoint names | `:24129` · `anonymousElevatedFlowRefusal` (`:22658`) | no | no | asked (a restore writes an active row) | | 5 | shared runtime authoring rules | `:24146` · `assertRuntimeAuthoringRules` (`:7372`) | no | no | asked: state `active`, the row's own package, the credential channel's held paths | | 6 | domain plugin authoring gate | `:24218` · `runAuthoringGate` (`:7309`) | no | no | asked, on the same channel condition as the save door | | 7 | ADR-0070 D1 read-only-base create | `:24348` · `SysMetadataRepository.readOnlyBaseCreateError` | no | no | **not asked**, reason below | | 8 | credential channel `store` refusal (no crypto provider) | `:24465` · `MetadataCredentialChannel.store` | no (R2 strip) | no (R2 strip) | **not asked**, reason below | Already asked by both restore doors before this PR, through `restoredBodyWriter` (`:7291`), and unchanged: `savedItemNameRefusal` (save door `:23813`) and `permissionSetOwnerBindingRefusal` (save door `:23824`). The Zod gate itself (`:23986`) is not in the family and is still not asked; reason below. ### Refusals a restore does not ask, with the reason (also written in the code) - **ADR-0070 D1.** D1 judges a package a caller *names* for a new item. A restore names none: it writes into the active row's own binding, which `put` keeps. `SysMetadataRepository.put`'s package-door note records why D1 is not extended to the doors that carry a row's own binding: it would refuse the repair of legacy package-bound rows. - **Credential channel `store`.** A restore stores no credential. The channel keeps its current one (R2, `restoredBodyDerivation`), so there is nothing for `store` to refuse. - **The schema gate.** A history body is a stored row. Whether today's parse may refuse a stored version is the ADR-0087 stored-shape question, which this PR does not decide (see Open question). Every judge above reads its body defensively, and the runtime gate reports a rule that throws instead of blocking on it. ## Fix - `refuseRestoredBodyTheSaveDoorRefuses` (new, `protocol.ts`) asks refusals 1–6 through the **same functions** the save door calls, in the save door's order, with their envelopes. ⛔ No rule is copied. - `restoredBodyWriter` calls it after the name and owner judges and before the R2 strip. It is now async and takes the restoring door's `source`. - `SysMetadataRepository.restoreVersion` awaits `deriveRestoredBody` (its type now allows a promise). It is still asked on the history body before the active row is read and before `put`, so a refused version writes nothing. - `rollbackMetaItem` rethrows the refusal. `revertCommit` reports it per item in `failed[]` with its code, as it does for its other per-item refusals, and reverts the commit's other items. - `defaultRecordPageSaveRefusal`'s verb union gains `'restored'`, so its sentence says "cannot be restored as the default record page". Three docblocks that listed the doors asking a refusal now name the restore doors. **The pair rule, both halves.** A datasource restored to managed (or with no `schemaMode`) while objects bind it `external` is refused at `schemaMode`, naming each object. An object restored with `external` on a datasource that is managed now is refused at `external`. Both come from `externalBindingSaveRefusal`, which reads the stored side as it stands when the restore runs. ## Measured `packages/metadata-protocol/src/protocol.restore-doors-save-refusal-family.test.ts` (new, 28 tests): refusals 1–6 (with both halves of #3), each through `rollbackMetaItem` and through `revertCommit`, plus a CONTROL per door that restores an acceptable version of the same item. A refusal pin asserts the code and status, one named subject (the holder page, the bound object, the `issues[].path`), and that rows, history and registry are byte-identical afterwards. | Run | Tree | Refusal pins (14) | Controls (14) | |---|---|---|---| | base | `d0cbd909` (test only, on `5801a4f2c`) | 14 red: rollback returned success (`expected null to be an instance of Error`), revert returned `success: true` | 14 green | | fix | `2cafc371` | 14 green | 14 green | | ablation | `2cafc371` with the `refuseRestoredBodyTheSaveDoorRefuses` call deleted (`scripts/ablation-replace.mjs --delete`: anchor 1 → 0, blob `3d4430ce` → `b2a9c352`, restored to the HEAD blob, `git diff HEAD` empty) | 14 red | 14 green | ## Tests and gates All on `2cafc371`; `e5daf940` adds only the engine-double ledger rows. The test file resolves `./protocol.js` from source, so the ablation needed no `dist/` leg. - `@objectstack/metadata-protocol`: `tsc --noEmit` exit 0. Full suite on `e5daf940`: 259 files passed (3 skipped), 20,943 tests passed, 19 skipped. That run includes one pin updated on purpose (next bullet); before that update it was the only failure. - **One existing pin moved, on purpose.** `protocol.credential-channel.test.ts` (R2 rollback) pinned the channel's calls during a rollback as `['strip']`. A rollback now first reads `heldPaths:active`, so the runtime authoring rules see a credential the channel holds as present (the draft promotion does the same). The pin now reads `['heldPaths:active', 'strip']`. What it guards is unchanged: the restore never calls `store`, the channel keeps its current secret, and nothing goes back at rest. - Downstream, after rebuilding `metadata-protocol`: `@objectstack/objectql` 465 files / 8,691 tests passed (it holds the real-repository rollback and revert suites). `@objectstack/rest` 321 files / 6,607 passed, 344 skipped. - Derived gates (`dispatch-gates.mjs --commands`): 77 families, every one exit 0. `--ran` reconciles 77 run, 0 NOT-MEASURED. 68 ran on `2cafc371` and the 9 the ledger file adds ran on `e5daf940`. Three were re-run on `e5daf940`: - `check:engine-double-contract` was red on the new test file's double. `--write` recorded 3 rows in `scripts/engine-double-contract.pinned.json`. - `check-plugin-teardown-shape --self-test` and `check:dual-build-cjs-loads` exited 3 on the shallow checkout and went green after deepening. The ledger file is outside the claim's declared surface; it is the gate's mechanical consequence of the new double. - Full dogfood (the title carries `!`): run on the final head after this PR opens; its VERDICT line is reported in the `os-dev-report` on #25278. ## Acceptance notes - **Refusals before the Zod gate that a restore does not ask** (outside this card's family, observations only, not filed): the protection-block refusal, the destructive-change diff, the view-container name collision, the ADR-0029 D9.9 package mismatch, the namespace-prefix rule and the tenant-authored flow stamp refusal. Each judges a request or a diff against the live row rather than a stored body's shape, and none was measured on a restore here. - **Batch order in `revertCommit`.** Each item is judged against the store as that item is reached, which is the save door's own judgement. Reverting a commit whose forward order was legal visits the same states in reverse, so a pair rule that held going forward holds going back. - **Stale REST comment.** `packages/rest/src/meta-item-history-routes.ts` (the rollback route) says rollback "runs no content gate at all". After this PR that sentence is false. The file is outside this claim's surface except for an envelope change, which was not needed (the route already forwards a thrown 4xx through `handleRouteError`), so it is left for the seat. - **Ledger row.** `scripts/engine-double-contract.pinned.json` gains the new test file's three engine-double rows (`check:engine-double-contract --write`). - **Advisories.** The runtime authoring gate's advisory half is not added to either restore receipt: neither declares a key for it, and adding one is a response-shape change. ## Open question Should a restore run the schema parse on the history body? Today a version that fails the current parse restores, and the save door would refuse it with `422 INVALID_METADATA`. This PR leaves that alone: it is the ADR-0087 stored-shape question (stored rows keep their conversion path) and needs a ruling, not a door edit. --- _Generated by [Claude Code](https://claude.ai/code/session_01HZVZ7xrs691S9potR9mSL1)_ --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent f5b6b3f commit 69913f8

6 files changed

Lines changed: 691 additions & 18 deletions
Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,29 @@
1+
---
2+
'@objectstack/metadata-protocol': minor
3+
---
4+
5+
fix(metadata-protocol)!: `rollbackMetaItem` and `revertCommit` refuse a stored version the save door refuses, instead of writing it back live
6+
7+
Clause-②: no (narrowing)
8+
9+
Metadata-protocol: narrowed
10+
11+
<!-- adr-0087: not-required (already-registered object-external-binding-managed-datasource-refused, page-default-record-page-one-per-object-refused, api-endpoint-anonymous-elevated-flow-refused) no rule is added or changed: the restore doors now ask the save door's existing refusals, and the ledger entries of the three that carry one already describe the refused shape and its fix -->
12+
13+
**BREAKING** (what a metadata restore writes narrows), shipped as `minor` under the 18.x convention for breaking changes.
14+
15+
The two restore doors write a stored history body back as the active row: `POST /api/v1/meta/TYPE/NAME/rollback` (`rollbackMetaItem`) and the restore limb of a commit revert (`revertCommit`, which `rollbackToPackageCommit` also uses). They asked only the save door's `name` and permission-set owner judges, and none of the refusals `saveMetaItem` runs after its schema gate. So a version saved before a refusal existed, or one half of a pair whose other half moved after the version was saved, came back live.
16+
17+
- **FROM:** a rollback or a commit revert restored a version that the save door would refuse today:
18+
- a hook with no `body`;
19+
- a record page marked `isDefault` for an object whose default another page now holds;
20+
- a datasource restored to `schemaMode: 'managed'` (or no `schemaMode`) while objects bind it `external`;
21+
- an object restored with `external` on a datasource that is managed now;
22+
- a `runAs: 'system'` flow that an anonymous (`authRequired: false`) endpoint names;
23+
- a body the shared runtime authoring rules refuse;
24+
- a body a domain plugin's authoring gate refuses (plugin-security's object posture gate, for example).
25+
- **TO:** each restore door asks those refusals through the save door's own functions, with the save door's code and status, and stores nothing on a refusal. The active row, the history and the registry stay as they were.
26+
- `rollbackMetaItem` answers the refusal: `400 VALIDATION_ERROR` for the hook, `422 DEFAULT_RECORD_PAGE_CONFLICT` for the page, `422 INVALID_METADATA` for the datasource pair, the flow and the authoring rules, and the plugin gate's own code.
27+
- `revertCommit` reports the refused item in `failed[]` with that code and reverts the commit's other items, as it does for its other per-item refusals.
28+
- **Unchanged:** a version the save door accepts restores as before. The restore does not run the schema parse on the history body, does not apply the read-only-base create refusal (that refusal judges a package a caller names for a new item; a restore names none and writes into the active row's own package binding), and keeps the write-only credential channel's current credential. The runtime authoring rules' advisories are not added to the restore response.
29+
- **The fix for an author:** a refused restore names the same problem the save door names. Restore a different version, or first change the other half of the pair (for example, remove `external` from the bound objects before restoring the datasource as managed), then restore again.

‎packages/metadata-protocol/src/protocol.credential-channel.test.ts‎

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -272,8 +272,11 @@ describe('[#20790] R2 — a rollback past the move keeps the channel\'s current
272272
expect(String(historyRows[historyRows.length - 1]!.metadata)).not.toContain(V1);
273273
// … the version written before the move keeps what it recorded (append-only, Q1 B) …
274274
expect(String(historyRows[0]!.metadata)).toContain(V1);
275-
// … and the channel keeps its current secret: a restore only strips.
276-
expect(calls).toEqual(['strip']);
275+
// … and the channel keeps its current secret: a restore never stores.
276+
// [#25278] It READS what the channel holds, for the save door's
277+
// runtime authoring rules it now asks of the restored body (as the
278+
// draft promotion does), then strips.
279+
expect(calls).toEqual(['heldPaths:active', 'strip']);
277280
expect(held.get('channel_intake|active')).toBe(V2);
278281
});
279282
});

0 commit comments

Comments
 (0)