fix(metadata-protocol)!: the save door refuses a hook that names a function in handler and carries no body (#21658) - #21686
Conversation
…nction in handler and carries no body A hook stored through the metadata door ships with no code package, and a handler name resolves only inside the hook's own package, so such a hook can never bind. saveMetaItem now refuses it with VALIDATION_ERROR / 400, naming the hook and its handler and prescribing a body, before anything is stored, in draft and in publish mode. A hook carrying a body beside its handler still saves. Claude-Session: https://claude.ai/code/session_017ErfyP2Rx7XWHJA27QjyUi Co-authored-by: Claude <noreply@anthropic.com>
…bound; pin the door's error body Claude-Session: https://claude.ai/code/session_017ErfyP2Rx7XWHJA27QjyUi Co-authored-by: Claude <noreply@anthropic.com>
…d not-bound cases Claude-Session: https://claude.ai/code/session_017ErfyP2Rx7XWHJA27QjyUi Co-authored-by: Claude <noreply@anthropic.com>
…er refusal Claude-Session: https://claude.ai/code/session_017ErfyP2Rx7XWHJA27QjyUi Co-authored-by: Claude <noreply@anthropic.com>
…ta-hook-handler-save
📓 Docs Drift CheckThis PR changes 1 package(s): 15 hand-written doc(s) NAME something this change touched and may need an implementation-accuracy re-verification:
⛔ 3 release-owned page(s) also name something this change touched. These are read-only:
What this run could not see
Coarse fallback — 11 page(s) merely mention a changed package (the pre-#9192 predicate, kept for the deliberately-wide backstop): Which tree this was computed onThis run read A worktree cut from an older # while this PR is open — GitHub drops the merge commit once it closes
git fetch origin 6e5585e1ab9e51f374d5cd3b27158bb621a25bab && git checkout 6e5585e1ab9e51f374d5cd3b27158bb621a25bab
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin 251a7dd4b491d1f216e8a470d0efd0dc7e8ac5e8 e9162b11801e7ca6f9ddbfc77693b4ab6b500b46 && git checkout -B drift-repro 251a7dd4b491d1f216e8a470d0efd0dc7e8ac5e8 && git merge --no-ff e9162b11801e7ca6f9ddbfc77693b4ab6b500b46
node scripts/docs-audit/affected-docs.mjs --json 251a7dd4b491d1f216e8a470d0efd0dc7e8ac5e8
|
ACCEPT — PR #21686 at head
|
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
handlerhook with a named error and the prescription "give it abody".HookSchemais untouched.What changes
saveMetaIteminpackages/metadata-protocol/src/protocol.tsnow refuses ahookwhosehandleris a non-empty string and that carries nobodyobject. BothPUT /api/v1/meta/hook/:nameand the dispatcher's metadata save call this door.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.handlerand carries nobody, so it can never run. Give it abody(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 ahandlername resolves only inside the hook's own package."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 ownermetadata-service, with nofunctionsmap. Both bind sites inpackages/objectql/src/plugin.tsdo this.functions, and an engine function whose owner is the bind's package. Nothing registers a function undermetadata-service.Saved hook 'scope_authored_cross' (env-wide, state=active). The binder then refused the stored hook three times (INVALID_REFERENCE/ 400, logged aterror). The ablation run below reproduces exactly this.The PM's mechanism hypotheses, measured
body(a string, say) would be told "give it abody", which misdescribes a hook that has one. After the parse,bodyis 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 malformedbodybeside ahandlergets the schema's422 INVALID_METADATAlocated atbody.bodyand ahandlerstays allowed.HookSchemadeclares both keys optional and does not make them exclusive, and the binder runsbodyfirst. Pinned at the unit level and at the composed door: the hook with both binds and runs its body, andx_stampnever runs.VALIDATION_ERROR/ 400.VALIDATION_ERROR/ 422 on its own door. This door's name refusal and its view-container refusals answerVALIDATION_ERROR/ 400, so 400 keeps one dialect per door. No new code is needed.duplicatePackageon a package holding a handler-only hook row and a body hook row answered{ success: false, copiedCount: 1, failedCount: 1 }. This refusal was infailed[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.saveMetaItemfor a hook.e9162b1180falls in one of two groups. The callers that forward an author's or a stored row's type are the RESTPUT /meta/:type/:nameand its compound twin, the dispatcher's metadata save,migrateStoredMetadataandduplicatePackage. The fixed-type callers areautomation.tsandflow-credential-migration.ts(flow),packages.ts(app) andpermission-set-projection.ts(permission).AppPlugin,loadArtifactBundle, the install-local door and the boot path make zerosaveMetaItemcalls.Scope: only the
handlerformA hook with neither a
bodynor ahandlernever runs either. Measured at the composed door:PUTanswered 200, and the binder warnedskipping hook with unresolved handler. This PR still refuses only thehandlerform, for two reasons:handlerform.protocol.code-only-types,protocol.meta-types-mint-door-agreementandprotocol.unrecognised-meta-typein metadata-protocol, andoverlay-precedenceandprotocol-metain 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
codeandstatus)PUTis refused with the named error, and nothing is stored or bound.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, thebodyprescription, 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 ofprotocol.invalid-metadata-422-face-inventory.test.ts: publish and draft mode each assertcode,statusand an empty store.x_stampnever runs. Unit: the CONTROL case and the body-beside-handler case.handlerhook is unchanged on its own door.functionsentry and binds and runs. App Z's hook names a function that its--artifactruntime module exports (loaded withloadArtifactBundle), 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-serviceowner is still pinned in objectql'shook-binder-package-scope.test.ts, which is green below.Reverse verification (the fix committed first, at
7d9d4b4221)Mutation.
node scripts/ablation-replace.mjsreplacedif (hookRefusal) throw hookRefusal;with a marker log. Anchor count 1 → 0; blob3496aca9fec3→03aa7af3511c.@objectstack/metadata-protocolwas then rebuilt, andnode scripts/ablation-dist-preflight.mjs @objectstack/metadata-protocol ABLATED_21658_HOOK_REFUSALfound the marker indist/index.jsanddist/index.cjs.Prediction: pin 1 red, pins 2 and 3 green. Observed:
expected { status: 200, … }, with the bodySaved hook 'scope_authored_cross' … state=active.Restore.
ablation-replacerestored the path: blob == HEAD (3496aca9fec3) andgit diff HEADis empty. A shell trap also rangit checkout HEAD -- ….git status --porcelainis empty.--absentpreflight found the marker in none of the 24 built files, and the tree was clean.Tests (at
e9162b1180, after mergingorigin/main7d0781482d)pnpm --filter @objectstack/metadata-protocol exec vitest run --maxWorkers=2: 209 files passed and 3 skipped; 3468 tests passed and 19 skipped.typecheck. Its tsc program includes the edited test file (--listFilescount: 1).typecheck: tsc pluscheck:test-typecheck, OK, debt ledger held.hook-handler-package-scope.pin.test.tsandstored-metadata-body-boundary.pin.test.ts: 13/13.protocol-meta,overlay-precedence,plugin-authored-hooksandhook-binder-package-scope: 139/139.pnpm turbo run build --filter='@objectstack/runtime^...' --concurrency=2, 29/29.packages/runtimetests 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.check:dual-build-cjs-loadsexited 3: PREREQUISITE NOT MET. It needs a fullpnpm build, and more than 30 packages outside this closure have nodist/. NOT MEASURED. Targeted reading instead:require('./packages/metadata-protocol/dist/index.cjs')loads with 83 exports.--ranreconciliation: 64 accounted for, 63 run, 1 NOT-MEASURED, 0 UNRUN.Artifact-roster block (54 families, outside the derived total). All 54 ran.
check:error-status-conformance,check:error-code-casing,check:authz-resolver,check:route-ledger-census,check-changeset-fixedandcheck:engine-double-contract.check-closing-target-claim,check-partof-closing-keywordandcheck-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:eslint.config.mjslintsfiles: ['**/*.{ts,tsx,mts,cts,js,jsx,mjs,cjs}']minusNEVER_LINTED.eslint --no-inline-config --format jsonover the 3 changed TS files reports 3 files, 0 errors and 0 warnings.parserOptions.project), so this diff cannot move the verdict of any untouched file.Changeset
.changeset/21658-hook-handler-without-body-save-door.md:minorfor@objectstack/metadata-protocol,Clause-②: no (narrowing), the BREAKING banner, and the ADR-0087 markernot-required (no-migration-prescription)with the census.check-adr-0087-registration --base origin/mainaccepts it.Landing point
As the claim predicted:
packages/metadata-protocol/src/protocol.ts,saveMetaItem, typehook. No producer elsewhere needs a change.Acceptance notes
publishMetaItem,rollbackMetaItemandrevertCommitcan 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.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 servedhandlerbody 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.bodynor ahandler(see Scope).Generated by Claude Code