Repository navigation
Commit ced217c
Fixes #21658
Clause-②: no (narrowing)
This carries out triage's ruling on #21658 (comment 5975986454, unlocked
in 5976053780). The ruling inherits from the maintainer's ruling on
#21604 (comment 5974477722, letter B) and from the install-local door
precedent (#21585, PR #21615). **The metadata save door refuses a
body-less `handler` hook with a named error and the prescription "give
it a `body`".** `HookSchema` is untouched.
## What changes
`saveMetaItem` in `packages/metadata-protocol/src/protocol.ts` now
refuses a `hook` whose `handler` is a non-empty string and that carries
no `body` object. Both `PUT /api/v1/meta/hook/:name` and the
dispatcher's metadata save call this door.
- **Envelope:** `VALIDATION_ERROR` / 400. This is the envelope of the
name check the same door runs on every body (`savedItemNameRefusal`). No
new code is added, and the ledger is not edited.
- **Message:** it names the hook and the function and gives the
prescription before the explanation. It stays under the 500-character
REST message bound when each name is shorter than about 65 characters.
The measured text reads: "Invalid hook: 'scope_authored_cross' names the
function 'x_stamp' in its `handler` and carries no `body`, so it can
never run. Give it a `body` (sandboxed JS, `{ language: 'js', source }`,
or an expression), which is stored with the hook. A hook saved through
the metadata API ships with no code package, so it holds no functions,
and a `handler` name resolves only inside the hook's own package."
- **When:** in draft mode and in publish mode, before anything is stored
or bound.
- **Where in the door:** right after the type schema accepts the body,
and before the runtime authoring gate and every write. H1 below explains
the placement.
The diff adds one module-level helper with its TSDoc,
`runtimeHookWithoutBodyRefusal`, and one call site.
## Why such a hook can never bind (measured)
- `ObjectQLPlugin`'s authored-hook re-sync binds every stored hook under
the synthetic owner `metadata-service`, with no `functions` map. Both
bind sites in `packages/objectql/src/plugin.ts` do this.
- Since PR #21653, the binder looks a name up in two places only: the
bind's own `functions`, and an engine function whose owner is the bind's
package. Nothing registers a function under `metadata-service`.
- So the name has nothing to bind to. Before this change, the door
answered 200 with `Saved hook 'scope_authored_cross' (env-wide,
state=active)`. The binder then refused the stored hook three times
(`INVALID_REFERENCE` / 400, logged at `error`). The ablation run below
reproduces exactly this.
## The PM's mechanism hypotheses, measured
| # | Hypothesis | Reading |
|:--|:--|:--|
| H1 | The check sits beside the view checks. | **Falsified in part, by
choice.** The check sits one step later, right after the type-schema
parse. Beside the view checks, a hook with a malformed `body` (a string,
say) would be told "give it a `body`", which misdescribes a hook that
has one. After the parse, `body` is either absent or a declared hook
body, so the binder's body-first test is exact. The check still runs
before the authoring gate and before every write. A pin covers this: a
malformed `body` beside a `handler` gets the schema's `422
INVALID_METADATA` located at `body`. |
| H2 | The predicate. A hook with both a `body` and a `handler` stays
allowed. | **Holds.** `HookSchema` declares both keys optional and does
not make them exclusive, and the binder runs `body` first. Pinned at the
unit level and at the composed door: the hook with both binds and runs
its body, and `x_stamp` never runs. |
| H3 | `VALIDATION_ERROR` / 400. | **Holds.** Install-local answers
`VALIDATION_ERROR` / 422 on its own door. This door's name refusal and
its view-container refusals answer `VALIDATION_ERROR` / 400, so 400
keeps one dialect per door. No new code is needed. |
| H4 | The re-savers record a failure, and stored rows keep their bytes.
| **Holds.** Measured with a one-off harness that is not committed.
`duplicatePackage` on a package holding a handler-only hook row and a
body hook row answered `{ success: false, copiedCount: 1, failedCount: 1
}`. This refusal was in `failed[0].error`, and the source row's bytes
were unchanged. `migrateStoredMetadata({ apply: true })` on such a row
answered `{ scanned: 1, canonical: 1, rewritten: 0, failed: 0 }`, with
the bytes unchanged. No conversion is pending for such a row, so it is
never re-saved. |
| H5 | No artifact or install path calls `saveMetaItem` for a hook. |
**Holds.** Every call site at `e9162b1180` falls in one of two groups.
The callers that forward an author's or a stored row's type are the REST
`PUT /meta/:type/:name` and its compound twin, the dispatcher's metadata
save, `migrateStoredMetadata` and `duplicatePackage`. The fixed-type
callers are `automation.ts` and `flow-credential-migration.ts` (flow),
`packages.ts` (app) and `permission-set-projection.ts` (permission).
`AppPlugin`, `loadArtifactBundle`, the install-local door and the boot
path make zero `saveMetaItem` calls. |
## Scope: only the `handler` form
A hook with neither a `body` nor a `handler` never runs either. Measured
at the composed door: `PUT` answered 200, and the binder warned
`skipping hook with unresolved handler`. This PR still refuses only the
`handler` form, for two reasons:
- The ruling and the claim name only the `handler` form.
- The bare shape is the schema-valid probe body in at least five
existing suites: `protocol.code-only-types`,
`protocol.meta-types-mint-door-agreement` and
`protocol.unrecognised-meta-type` in metadata-protocol, and
`overlay-precedence` and `protocol-meta` in objectql.
Widening the predicate is a separate call. It goes to the seat as a
finding and is not folded in here.
## Pins (ADR-0112: each refusal asserts `code` and `status`)
| Pin (triage 5975986454) | Where |
|:--|:--|
| 1. The measured `PUT` is refused with the named error, and nothing is
stored or bound. | **Composed kernel**,
`packages/runtime/src/hook-handler-package-scope.pin.test.ts`. Case ②
asserts 400, the body `{ error, code: 'VALIDATION_ERROR' }`, the names
of the hook and the function, the `body` prescription, and a 404 on the
by-name GET. Case "② nothing bound" asserts that the binder recorded no
refusal of the hook after the re-sync ran. **Unit**, section 7 of
`protocol.invalid-metadata-422-face-inventory.test.ts`: publish and
draft mode each assert `code`, `status` and an empty store. |
| 2. A body hook saves and binds. | **Composed** case ②b: a body hook
and a body-plus-handler hook both bind and run, and `x_stamp` never
runs. **Unit**: the CONTROL case and the body-beside-handler case. |
| 3. A built artifact's `handler` hook is unchanged on its own door. |
**Composed** controls. App X's hook names its own `functions` entry and
binds and runs. App Z's hook names a function that its `--artifact`
runtime module exports (loaded with `loadArtifactBundle`), and it binds
and runs. |
Before this PR, the composed case ② recorded the door's 200 and asserted
the refusal at bind. It now asserts the refusal at the door. The
binder's refusal for the `metadata-service` owner is still pinned in
objectql's `hook-binder-package-scope.test.ts`, which is green below.
## Reverse verification (the fix committed first, at `7d9d4b4221`)
**Mutation.** `node scripts/ablation-replace.mjs` replaced `if
(hookRefusal) throw hookRefusal;` with a marker log. Anchor count 1 → 0;
blob `3496aca9fec3` → `03aa7af3511c`. `@objectstack/metadata-protocol`
was then rebuilt, and `node scripts/ablation-dist-preflight.mjs
@objectstack/metadata-protocol ABLATED_21658_HOOK_REFUSAL` found the
marker in `dist/index.js` and `dist/index.cjs`.
**Prediction:** pin 1 red, pins 2 and 3 green. **Observed:**
- **Unit (src):** publish ✗ and draft ✗. CONTROL ✓, body beside handler
✓, malformed body ✓. 2 failed, 21 passed.
- **Composed (dist):**
- ② ✗: `expected { status: 200, … }`, with the body `Saved hook
'scope_authored_cross' … state=active`.
- "② nothing bound" ✗: the binder recorded 3 refusals.
- ②b ✓, ① ✓, X control ✓, Z control ✓.
- 2 failed, 4 passed.
**Restore.**
- `ablation-replace` restored the path: blob == HEAD (`3496aca9fec3`)
and `git diff HEAD` is empty. A shell trap also ran `git checkout HEAD
-- …`.
- Whole-tree `git status --porcelain` is empty.
- After a rebuild, the `--absent` preflight found the marker in none of
the 24 built files, and the tree was clean.
- The reruns are green: 23/23 and 6/6.
## Tests (at `e9162b1180`, after merging `origin/main` `7d0781482d`)
- `pnpm --filter @objectstack/metadata-protocol exec vitest run
--maxWorkers=2`: 209 files passed and 3 skipped; 3468 tests passed and
19 skipped.
- Typecheck, exit 0 for both packages:
- metadata-protocol `typecheck`. Its tsc program includes the edited
test file (`--listFiles` count: 1).
- runtime `typecheck`: tsc plus `check:test-typecheck`, OK, debt ledger
held.
- Runtime `hook-handler-package-scope.pin.test.ts` and
`stored-metadata-body-boundary.pin.test.ts`: 13/13.
- objectql `protocol-meta`, `overlay-precedence`,
`plugin-authored-hooks` and `hook-binder-package-scope`: 139/139.
- Dependency closure: `pnpm turbo run build
--filter='@objectstack/runtime^...' --concurrency=2`, 29/29.
- The `packages/runtime` tests outside these files are declared to CI.
## Gates (at `e9162b1180`)
**Derived.** `node scripts/pm/dispatch-gates.mjs --commands` (no paths)
derives 64 families, and all 64 ran.
- 63 exited 0.
- `check:dual-build-cjs-loads` exited 3: PREREQUISITE NOT MET. It needs
a full `pnpm build`, and more than 30 packages outside this closure have
no `dist/`. NOT MEASURED. Targeted reading instead:
`require('./packages/metadata-protocol/dist/index.cjs')` loads with 83
exports.
- The `--ran` reconciliation: 64 accounted for, 63 run, 1 NOT-MEASURED,
0 UNRUN.
**Artifact-roster block** (54 families, outside the derived total). All
54 ran.
- 51 exited 0. These include `check:error-status-conformance`,
`check:error-code-casing`, `check:authz-resolver`,
`check:route-ledger-census`, `check-changeset-fixed` and
`check:engine-double-contract`.
- 3 exited 2 and are NOT WIRED without PR context:
`check-closing-target-claim`, `check-partof-closing-keyword` and
`check-single-claim-paths`. They are rerun with this PR's context, and
the results go in the os-dev report.
**Lint.** CI owns `pnpm lint`. This PR records a proven narrowing
instead:
- **Population:** `eslint.config.mjs` lints `files:
['**/*.{ts,tsx,mts,cts,js,jsx,mjs,cjs}']` minus `NEVER_LINTED`.
- **Count:** `eslint --no-inline-config --format json` over the 3
changed TS files reports 3 files, 0 errors and 0 warnings.
- **Invariance:** the config enables no type-aware linting (no
`parserOptions.project`), so this diff cannot move the verdict of any
untouched file.
## Changeset
`.changeset/21658-hook-handler-without-body-save-door.md`: `minor` for
`@objectstack/metadata-protocol`, `Clause-②: no (narrowing)`, the
BREAKING banner, and the ADR-0087 marker `not-required
(no-migration-prescription)` with the census.
`check-adr-0087-registration --base origin/main` accepts it.
## Landing point
As the claim predicted: `packages/metadata-protocol/src/protocol.ts`,
`saveMetaItem`, type `hook`. No producer elsewhere needs a change.
## Acceptance notes
- **The draft-promotion and restore doors do not re-ask this rule.**
`publishMetaItem`, `rollbackMetaItem` and `revertCommit` can still make
a draft or a history version stored before this change into an active
handler-only row. The runtime then refuses that row at bind, as before.
The rule covers the save door only, as the same door's view-container
refusal does. Carrier: none.
- **Kernels with no `environmentId`.** A save there under the name of an
artifact-shipped hook writes a row the re-sync skips
(`isArtifactShippedHook`). So a GET-then-PUT round trip of an artifact
hook's served `handler` body is now refused on such a kernel. Before, it
stored an inert row that was never bound. Environment-scoped kernels
already refuse that write (`refusePackagedBaseOverride`). Carrier: none.
- **One finding goes to the seat in the os-dev report:** a hook with
neither a `body` nor a `handler` (see Scope).
---
_Generated by [Claude
Code](https://claude.ai/code/session_017ErfyP2Rx7XWHJA27QjyUi)_
---------
Co-authored-by: Claude <noreply@anthropic.com>
1 parent 83e2fee commit ced217c
4 files changed
Lines changed: 232 additions & 26 deletions
File tree
- .changeset
- packages
- metadata-protocol/src
- runtime/src
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
| 1 | + | |
| 2 | + | |
| 3 | + | |
| 4 | + | |
| 5 | + | |
| 6 | + | |
| 7 | + | |
| 8 | + | |
| 9 | + | |
| 10 | + | |
| 11 | + | |
| 12 | + | |
| 13 | + | |
| 14 | + | |
| 15 | + | |
| 16 | + | |
| 17 | + | |
| 18 | + | |
| 19 | + | |
| 20 | + | |
| 21 | + | |
| 22 | + | |
| 23 | + | |
| 24 | + | |
| 25 | + | |
| 26 | + | |
Lines changed: 78 additions & 0 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
492 | 492 | | |
493 | 493 | | |
494 | 494 | | |
| 495 | + | |
| 496 | + | |
| 497 | + | |
| 498 | + | |
| 499 | + | |
| 500 | + | |
| 501 | + | |
| 502 | + | |
| 503 | + | |
| 504 | + | |
| 505 | + | |
| 506 | + | |
| 507 | + | |
| 508 | + | |
| 509 | + | |
| 510 | + | |
| 511 | + | |
| 512 | + | |
| 513 | + | |
| 514 | + | |
| 515 | + | |
| 516 | + | |
| 517 | + | |
| 518 | + | |
| 519 | + | |
| 520 | + | |
| 521 | + | |
| 522 | + | |
| 523 | + | |
| 524 | + | |
| 525 | + | |
| 526 | + | |
| 527 | + | |
| 528 | + | |
| 529 | + | |
| 530 | + | |
| 531 | + | |
| 532 | + | |
| 533 | + | |
| 534 | + | |
| 535 | + | |
| 536 | + | |
| 537 | + | |
| 538 | + | |
| 539 | + | |
| 540 | + | |
| 541 | + | |
| 542 | + | |
| 543 | + | |
| 544 | + | |
| 545 | + | |
| 546 | + | |
| 547 | + | |
| 548 | + | |
| 549 | + | |
| 550 | + | |
| 551 | + | |
| 552 | + | |
| 553 | + | |
| 554 | + | |
| 555 | + | |
| 556 | + | |
| 557 | + | |
| 558 | + | |
| 559 | + | |
| 560 | + | |
| 561 | + | |
| 562 | + | |
| 563 | + | |
| 564 | + | |
| 565 | + | |
| 566 | + | |
| 567 | + | |
| 568 | + | |
| 569 | + | |
| 570 | + | |
| 571 | + | |
| 572 | + | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
779 | 779 | | |
780 | 780 | | |
781 | 781 | | |
| 782 | + | |
| 783 | + | |
| 784 | + | |
| 785 | + | |
| 786 | + | |
| 787 | + | |
| 788 | + | |
| 789 | + | |
| 790 | + | |
| 791 | + | |
| 792 | + | |
| 793 | + | |
| 794 | + | |
| 795 | + | |
| 796 | + | |
| 797 | + | |
| 798 | + | |
| 799 | + | |
| 800 | + | |
| 801 | + | |
| 802 | + | |
| 803 | + | |
| 804 | + | |
| 805 | + | |
| 806 | + | |
| 807 | + | |
| 808 | + | |
| 809 | + | |
| 810 | + | |
| 811 | + | |
| 812 | + | |
| 813 | + | |
| 814 | + | |
| 815 | + | |
| 816 | + | |
| 817 | + | |
| 818 | + | |
| 819 | + | |
| 820 | + | |
| 821 | + | |
| 822 | + | |
| 823 | + | |
| 824 | + | |
| 825 | + | |
| 826 | + | |
| 827 | + | |
| 828 | + | |
| 829 | + | |
| 830 | + | |
| 831 | + | |
| 832 | + | |
| 833 | + | |
| 834 | + | |
| 835 | + | |
| 836 | + | |
| 837 | + | |
| 838 | + | |
| 839 | + | |
| 840 | + | |
| 841 | + | |
| 842 | + | |
| 843 | + | |
| 844 | + | |
| 845 | + | |
782 | 846 | | |
783 | 847 | | |
784 | 848 | | |
| |||
18815 | 18879 | | |
18816 | 18880 | | |
18817 | 18881 | | |
| 18882 | + | |
| 18883 | + | |
| 18884 | + | |
| 18885 | + | |
| 18886 | + | |
| 18887 | + | |
| 18888 | + | |
| 18889 | + | |
| 18890 | + | |
| 18891 | + | |
| 18892 | + | |
18818 | 18893 | | |
18819 | 18894 | | |
18820 | 18895 | | |
| |||
0 commit comments