Repository navigation
fix(objectql,runtime): a hook whose handler names a function gets ctx.title in-process - #24423
Conversation
….title in-process bindHooksToEngine gains a wrapHandler option, applied to every handler a hook runs in-process after a string handler resolves; the IObjectQLEngine.bindHooks and ObjectQL.bindHooks opts carry it. bindAppArtifactHandlers passes withInProcessHookTitle through it instead of pre-wrapping inline handlers, so the inline and the named handler take one wrap. Claude-Session: https://claude.ai/code/session_01UxK7vXeLCgYoxSXaHuQA1Y Co-authored-by: Claude <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01UxK7vXeLCgYoxSXaHuQA1Y Co-authored-by: Claude <noreply@anthropic.com>
…okContext requires Claude-Session: https://claude.ai/code/session_01UxK7vXeLCgYoxSXaHuQA1Y Co-authored-by: Claude <noreply@anthropic.com>
📓 Docs Drift CheckThis PR changes 3 package(s): ⛔ 1 release-owned page(s) name something this change touched. These are read-only:
What this run could not see
Coarse fallback — 148 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 7d816eab584237726d2e342fbf2b17a24c00c12e && git checkout 7d816eab584237726d2e342fbf2b17a24c00c12e
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin e22defb13c171900e2c1c54d1d15c24ed85fe91b 5753938245d5000bbacd6287e086b4a5e2c01604 && git checkout -B drift-repro e22defb13c171900e2c1c54d1d15c24ed85fe91b && git merge --no-ff 5753938245d5000bbacd6287e086b4a5e2c01604
node scripts/docs-audit/affected-docs.mjs --json e22defb13c171900e2c1c54d1d15c24ed85fe91b
|
席位核对 (seat check) · PR #24423 ·
|
| item | reading |
|---|---|
| Shape | Draft, base main, assignee zhuangjianguo. Line 1 is Part of #24289, with no closing keyword (the brief's own route once P3 failed), and Clause-②: yes (widening) starts a line. 7 files, +297/−24. Not governed. The changeset is minor on @objectstack/spec and patch on @objectstack/objectql and @objectstack/runtime. |
| The option | BindHooksOptions.wrapHandler(handler, hook) is applied once per hook at bind, after a string handler resolves (resolveInProcessHandler), and never to a body's handler. ObjectQL.bindHooks and the spec contract carry it, typed with spec Hook / HookContext, with no any. bindAppArtifactHandlers passes withInProcessHookTitle through it. The inline-only pre-wrap is removed, so both handler kinds take one wrap, and a caller that passes no option binds as before. |
| Premises | P1 true: the new verify pin was red on ee882b845 on both doors (TypeError: ctx.title is not a function) and is green after. P2 true. P3 false: ql.registerHook(...) (documented in skills/objectstack-data/references/data-hooks.md) and a bare ObjectQL bindHooks without the runtime still run hooks with no ctx.title. So per the brief's prohibition, item 2 (HookContext.title) is not built, and the card stays open for the decision. |
| CI | On this head: 41 runs, 35 success, 6 skipped. |
| Accepted deviations | No @objectstack/verify changeset: only a test file changed there. body-runner.ts is untouched. Its withInProcessHookTitle "Why" docblock still speaks of the inline handler only; that is polish, carried by whichever PR next edits the file. |
| Item 2 | The seat puts the decision to the maintainer on #24289 (options A–D and the four-axis analysis there). The card moves to needs-user-decision when this PR lands. |
| Landing | After a contract-review PASS and a green head: pr_ready then automerge_enable. Part of, so #24289 stays open. |
Generated by Claude Code
Contract reviewServed-tier: PR #24423 ( ① Derived judgments
② Semver levelClause-②: yes (widening)
③ Boundary flags
Implemented-by: VERDICT: FAIL One item fails: ② Generated by Claude Code |
席位核对 (seat check) · PR #24423 ·
|
…Options.wrapHandler widening Claude-Session: https://claude.ai/code/session_01UxK7vXeLCgYoxSXaHuQA1Y Co-authored-by: Claude <noreply@anthropic.com>
Contract reviewServed-tier: Round 2 on PR #24423 ( ① Derived judgments
② Semver levelClause-②: yes (widening)
③ Boundary flags
Implemented-by: VERDICT: PASS The one FAIL item of round 1 is closed by the one-line changeset edit; everything else on the contract face is the round-1 reading on unchanged blobs, and the five required contexts are green on this head. Landing ( Generated by Claude Code |
|
Contract review adopted ·
Generated by Claude Code |
Part of #24289
Clause-②: yes (widening)
Item 1 of the card's release
6252025560: a hook whosehandlerNAMES a function now gets the samectx.title(field?)in-process that an inline handler and a sandboxedbodyget. Item 2 (declaringtitleonHookContext) is not built here. P3 below found two doors that run hooks in-process and still do not deliverctx.title, so the declaration would run ahead of the runtime. That half goes back to the PM asneeds_decision, and #24289 stays open for it. This PR carries no closing keyword for that reason.What changed
packages/objectql/src/hook-binder.ts:BindHooksOptionsgainswrapHandler(handler, hook).resolveHandlerapplies it once per hook, at bind, to every handler the hook runs in-process, after a stringhandlerresolves: an inline function, a name from the bind'sfunctions, or a name the same package registered on the engine (the newresolveInProcessHandleris the old resolution, moved unchanged). It is never applied to the handlerbodyRunnerbuilds from abody. The engine awaits a handler and discards its return value, and so does the adapter around the wrapped function. With nowrapHandler, every handler binds exactly as resolved.packages/spec/src/contracts/objectql-engine.ts:IObjectQLEngine.bindHooksopts gainwrapHandler?: (handler: (context: HookContext) => unknown, hook: Hook) => (context: HookContext) => unknown, typed with the spec's ownHookandHookContext(noany, no bareunknownfor a known signature;unknownis only the discarded return).packages/objectql/src/engine.ts:ObjectQL.bindHooksopts carry the same member (BindHooksOptions['wrapHandler']). No engine default is installed.packages/runtime/src/app-artifact-handlers.ts:bindAppArtifactHandlerspasseswrapHandler: (handler, hook) => withInProcessHookTitle(handler, ql, String(hook.name)). The pre-bind copy of inline-handler hooks (hookWithInProcessTitle, PR fix(runtime): an in-process hook handler is handed the same ctx.title a hook body gets #24356) is removed.withInProcessHookTitleandbuildTitleSurface(body-runner.ts) are unchanged.Why the wrap is a binder option (G1)
The alternative was to resolve names in the runtime before the bind. That would copy the binder's resolution rules (the bind's
functions, then a function the same package registered on the engine, never another package's, withINVALID_REFERENCEfor a miss) into a second resolver. The binder is documented as the one resolver ("Inline function handlers and string-named handlers share one resolver"). With the option, the binder resolves both kinds and applies the one wrap after resolution, so the inline handler and the named handler take the same wrap at the same point. That is also why the runtime's earlier inline-only pre-wrap is removed rather than kept beside it: keeping it would wrap an inline handler twice.Premises, each measured
P1 (true): a named handler ran in-process without
ctx.title, on both doors. The new pinpackages/verify/src/harness.named-hook-title-door.test.tswas run againstorigin/mainee882b845(built through turbo) before any fix:After the fix (head
043baeb3f), same command:Tests 4 passed (4).P2 (true): on
ee882b845,withInProcessHookTitlewrapped only an inlinehandlerinbindAppArtifactHandlers(hookWithInProcessTitle,typeof hook.handler !== 'function'returns the hook unchanged), beforeql.bindHooks. A stringhandlerreached the binder unwrapped. The P1 run above shows it: inline pins green, named pins red.P3 (false): not every in-process door delivers
ctx.titleafter item 1. Doors enumerated and measured at head043baeb3f:ctx.titleafter this PRbody(artifact door,objectstack dev)harness.hook-title-door.test.tsartifact casehandlerviabindAppArtifactHandlers(source boot,os serveover a config)harness.hook-title-door.test.tssource case andapp-artifact-handlers.hook-title.test.tshandler, source door (functions)handler, artifact door (runtimeModule)ObjectQLPlugin's metadata-service bind (ql.bindHooks(bindable, { packageId: 'metadata-service' }))metadata-servicemetadata-serviceql.registerHook(event, fn), documented as the plugin escape hatch (data-hooks.md, method 2)typeof ctx.title === 'undefined'ObjectQLused directly,ql.bindHooks([...])with no runtime (a host or a test harness)wrapHandler)typeof ctx.title === 'undefined'for an inline and a named handlerThe two measured
undefinedrows came from one scratch vitest run inpackages/objectqlagainst a barenew ObjectQL():{"registerHook":"undefined","bindInline":"undefined","bindNamed":"undefined"}. The scratch file was deleted right after; it was never committed. SoHookContextgains notitlehere. Declaring it, whether required or optional, would declare a member that the documentedregisterHookdoor does not deliver. The options go to the PM.Pins
packages/verify/src/harness.named-hook-title-door.test.ts(new, G2): on bothbootStackdoors, a named handler answersctx.title()=L1 · Widget(this record's formula title, one hop through a lookup),ctx.title('product')=Widget (W-1), andctx.title('spare')=null. These are the same answers PR fix(runtime): an in-process hook handler is handed the same ctx.title a hook body gets #24356's pin gets from abody.packages/objectql/src/hook-binder-wrap-handler.test.ts(new):functionsand the engine-registered name of the same package all take the wrap, once each, at bind;body's handler never takes it;wrapHandler, a named handler and an inline handler are called with the engine's own context object (identity), as before.Ablation (G2)
One run under trap restore, after the fix was committed (
39cf701a1):wrapHandler:line was deleted frompackages/runtime/src/app-artifact-handlers.tsthroughscripts/ablation-replace.mjs: anchor 1 → 0, blob1b658c88ace0→376387b42a8a.noUnusedLocalsflags the now-unusedwithInProcessHookTitleimport. That happens after the JS emit, so the marker count indist/index.jswent 1 → 0, andablation-dist-preflight.mjs runtime 'wrapHandler: (handler, hook)' --absentexited 0.3 failed | 1 passed: both named-handler doors and the inline source door failed withTypeError: ctx.title is not a function, and the artifactbodydoor stayed green. The inline door going red is expected: inline handlers now take the same one wrap.git checkout HEAD -- ABS_PATHgave blob1b658c88ace0, equal to the HEAD blob, andgit diff HEADwas empty. Runtime was rebuilt (exit 0, marker count back to 1) and the pins were4 passed (4).Local verification (head
043baeb3f)turbo run build --filter=@objectstack/verify...(49 tasks), then the 4 touched packages, then--filter='!@objectstack/docs'forcheck:dual-build-cjs-loads.dispatch-gates.mjs --commands --repo objectstack-ai/objectstack, no paths): 92 families.check-engine-split-ratio --days 90(exit 2) andcheck-plugin-teardown-shape --self-test(exit 3) on the shallow clone; both passed aftergit fetch --unshallow origin main.check:dual-build-cjs-loads(exit 3, no dist); passed after the full build: "111 published require entry point(s) across 69 package(s) load".pnpm --filter PKG typecheck, test layer included): spec, runtime and verify passed with exit 0. objectql first exited 1: the new pin's context literal lackedql, which gave TS2352 undertsconfig.test.json. That is fixed ina2263206e, and objectql then exited 0 ("test layer compiles").vitest run446 files, 8362 passed.vitest run --project local421 files, 6626 passed, 19 skipped.vitest run43 files, 371 passed.eslint --no-inline-configover the 6 changed.tsfiles: exit 0.a2263206echanges only that test literal.--commandsderives the same 92 families there. The test-facing gates (check:cross-package-test-inputs,check:test-source-alias,check:nul-bytes,check:tier-file-adoption) were re-run there with exit 0, and--ranreconciles 92 derived, 92 run, 0 NOT-MEASURED, 0 UNRUN.Acceptance notes
HookContext.title) is held for a decision; the reading and the options are in the os-dev-report on Hook ctx.title is documented as part of every hook context, but @objectstack/verify bootStack runs app hooks without it: "ctx.title is not a function" under the test harness, while objectstack dev serves it #24289.@objectstack/verifychanges only a test file (itsfiles[]shipsdistonly), so it takes no changeset entry.Dev session:
session_01UxK7vXeLCgYoxSXaHuQA1Y(mode:cloud), dispatched by thedomain:specseat 2 (session_019oFY6Hi35J7atvgVFnmoaK) for claim6257131238.🤖 Generated with Claude Code
https://claude.ai/code/session_01UxK7vXeLCgYoxSXaHuQA1Y
Generated by Claude Code