Skip to content

Commit a59d00a

Browse files
fix(scripts,objectql): codemod-split-engine --members reads hand-written facet headers and inverts its import edits; move registerApp, registerDriver and registerHook (#30716)
<!-- claude-projects-attribution: {"github_login":"os-elon-musk"} --> _Requested by **Elon** · [project thread](https://claude.ai/code/project/chan_01WedDzV26nniwxv7jC88B9D?thread=cmsg_01WedDzV26nniwxv7jC88B9D6AnW6Yh9CRooCGz4c48JoP)_ domain:devx#3 · os-elon-musk · session_016WV4cwSsWgu5WEWLNtFrf3 Fixes #30572 Refs #30054 Clause-②: no Metadata-protocol: none ## 维护者速读 - 改了什么:`scripts/codemod-split-engine.mjs --members` 现在接受「模块头被手工续写过」的 facet 模块,并且在搬移改动了接收模块 import 块时,仍能逐字节证明撤销;随后用它把 `registerApp`、`registerDriver`、`registerHook` 三个方法体搬进各自的 facet,并删掉登记表里对应的三行例外。 - 为什么改:#30054 的 PR #30571 搬了五个方法体,这三个被 codemod 拒绝(`engine-registration.ts` 的模块头被手改过;`registerHook` 的撤销证明在 import 块处失败),只能留在类上当例外。 - 风险与回滚:纯搬移,运行时行为不变;`@objectstack/objectql` 构建出的三个 `.d.ts` 在搬移前后逐字节相同。撤销证明仍是逐字节比较,没有做任何空白或 import 归一化。回滚即 revert 本 PR。 - 先看:`scripts/codemod-split-engine.mjs` 的 `headerNamesOf` 与 `importEditsOf`;`packages/objectql/src/engine-hooks-and-functions.ts` 的 import 块。 - 两分钟验证:`node scripts/codemod-split-engine.mjs --members --verify --base origin/main`,预期最后一行写 "undoing the moves does rebuild the base engine.ts and its modules byte-for-byte",exit 0;`node scripts/codemod-split-engine.mjs --self-test` 预期 129 cases pass。 - 需要你做的:无,席位落地。 ## What changes - **`scripts/codemod-split-engine.mjs`, fourth pass (`--members`), two shapes it used to refuse:** - **A header a hand edit wrote on after.** `engine-registration.ts`'s header carries a sentence written by hand after the generated one (`isExternalWriteForbidden` was written there by hand). `headerNamesOf` now reads the words after the header sentence, in the same paragraph, as a `tail`, and accepts the header only when `moduleHeaderLines(title, names, tail)` rewraps to it byte for byte. The pass rewrites the sentence it owns (the list grows) and carries the tail over unedited; the undo rewraps the same tail. A tail that does not rewrap as the pass wraps it is still refused. - **An import edit the prune cannot take back.** `registerHook`'s parameter docblock links `HookEntry`, so the move adds `HookEntry` to the module's `import type` from `./engine-top-level.js`. The base module already carried an unresolved `{@link HookEntry.dispatchUnscopedMultiWrite}` in a staying docblock; once the binding exists, that link resolves, so "prune what nothing reads" keeps it and the undo stopped one line short of the base. `importEditsOf` now reads the edits the move made off the base and head import blocks (bindings added, and bindings widened from type-only to value) and the undo inverts exactly those, then prunes as before. Each inverted edit is checked: a moved member's own text must read the name, and a head that dropped or narrowed a base binding fails, since no undo could restore it. The lines left are rendered as the first pass writes them, and the result is still compared with the base byte for byte. - **The three moves**, written by the codemod on the latest `main`: `registerApp` and `registerDriver` into `engine-registration.ts`, `registerHook` into `engine-hooks-and-functions.ts`. The stubs, facet members, host interfaces, host factories and imports are the pass's own output. `engine.ts` goes from 11,309 to 11,086 lines. - **`scripts/facade-delegation-registry.json`**: the three `#30054` rows for these members are removed; `check:facade-delegation-only` is green with 53 named exceptions. - **`.changeset/30572-objectql-registration-bodies.md`**: `@objectstack/objectql` patch, following PR #30571's changeset. ## Tests The codemod's self-test (`node scripts/codemod-split-engine.mjs --self-test`): 129 cases in 9 batteries pass. The fourth-pass battery gains five cases, and its pinned floor goes from 18 to 23: 1. a header a hand edit wrote on after is accepted: the list grows, the hand's words follow it unedited, and the moves prove; 2. a hand tail that does not rewrap as the pass wraps it refuses; 3. one edited character in the hand's words at head fails; 4. a base docblock link that a moved body's import comes to resolve: the undo drops the binding the move added, and the moves prove byte for byte; 5. a binding the head added that only a staying docblock links, and no moved member reads, fails. <details><summary>Reverse verification (one-time, run on the committed fix, restored to the HEAD blob)</summary> - `importEditsOf` made to return no edits: case 4 goes red ("undoing the moves out of engine-lifecycle.ts does not rebuild its base (first difference at line 41)"), and `--members registerHook` on the real tree fails again at line 46 of `engine-hooks-and-functions.ts`, the card's refusal. - `headerNamesOf` made to accept no tail: the self-test stops at "engine-lifecycle.ts: its header is not as the first pass wrote it", and `--members registerApp` on the real tree refuses again with the card's message. - After each, `git checkout HEAD -- scripts/codemod-split-engine.mjs` and `git status --porcelain` empty. </details> ## Verification Final head is the merge of `origin/main` `a2437b56` (which edited `registerApp`'s body in #30656) into this branch; the moves were regenerated on that `main` rather than text-merged. - Before (`main` `f4d568ba`): `--members registerApp` / `registerDriver` refused with "engine-registration.ts: its header is not as the first pass wrote it" (exit 2); `--members registerHook` failed the undo proof at line 46 of `engine-hooks-and-functions.ts` (exit 1; the card recorded line 44 on an older `main`). - After: `node scripts/codemod-split-engine.mjs --members --verify --base a2437b5` exits 0: "3 moved method(s) (registerHook, registerApp, registerDriver); engine.ts 11309 -> 11086 lines; undoing the moves does rebuild the base engine.ts and its modules byte-for-byte once the 7 import binding(s) only moved bodies read are pruned from the base". - Public `.d.ts` for `@objectstack/objectql`: built at base `a2437b56` and at head; `dist/index.d.ts`, `dist/core.d.ts` and `dist/util-B0RqjNMv.d.ts` are byte-equal (sha256 `4d00ae43…`, `a0cdd71a…`, `69f1c293…` on both sides). Control: `RegistrationFacet.registerApp` occurs 0 times in the base `dist/index.js` and once in the head's, so each side's build is its own. - `pnpm --filter @objectstack/objectql typecheck`: exit 0 (including `check:test-typecheck`). `pnpm --filter @objectstack/objectql test`: 556 files, 11068 tests passed (lock VERDICT command-exit 0), at head `3e387e33`. - `node scripts/pm/dispatch-gates.mjs --commands`: 73 families derived at head `3e387e33`; 72 exit 0. `pnpm check:dual-build-cjs-loads` is NOT MEASURED locally (exit 3, PREREQUISITE NOT MET: it reads every package's `dist/`, and only the `@objectstack/objectql` closure was built here); CI runs it in `Lint & Repo Gates`. `--ran`: 73 derived, 72 run, 1 NOT-MEASURED, 0 UNRUN. The codemod's own self-test (above) is outside the derived set and was run separately. ## Acceptance notes - The undo is no longer computed from the head alone for the import block: it reads the base's binding names to know which bindings the move added. That is the information the head cannot carry (two different bases give the same head when a staying docblock links a name a moved body imports). The comparison itself is unchanged, byte for byte, and every edit it inverts must be one a moved member's text reads. - Sibling #30518 edits `scripts/codemod-split-protocol.mjs` and other rows of `scripts/facade-delegation-registry.json`; this PR does not touch that codemod. Whichever lands second merges `main`. ## Human Minimum - [x] 1. Schemas start as Zod; `packages/spec` holds no runtime logic. (Not touched.) - [x] 2. Every name I added uses the casing for its kind. - [x] 3. No formatter ran; `pnpm lint` is CI's run. - [x] 4. One issue, one branch off an up-to-date `main`, one pull request. - [x] 5. A change that publishes carries a changeset, and both declaration lines are filled in here and in the changeset. - [x] 6. `packages/spec` touched: its generated artifacts are regenerated and committed. (Not touched.) - [x] 7. No hand edits to `content/docs/references/`, `packages/*/CHANGELOG.md` or `content/docs/releases/`. - [x] 8. Title, body and comments are in English (the 维护者速读 block is the lane's maintainer summary). - [x] The *Before Submitting* list in CONTRIBUTING.md is done. --- _Generated by [Claude Code](https://claude.ai/code/session_016WV4cwSsWgu5WEWLNtFrf3)_ --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent 2038c56 commit a59d00a

7 files changed

Lines changed: 477 additions & 316 deletions

File tree

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
---
2+
'@objectstack/objectql': patch
3+
'@objectstack/spec': patch
4+
---
5+
6+
refactor(objectql): the bodies of `registerApp`, `registerDriver` and `registerHook` move into their facets
7+
8+
Clause-②: no
9+
Metadata-protocol: none
10+
11+
- The bodies of `ObjectQL#registerApp` and `ObjectQL#registerDriver` move out of `engine.ts` into `engine-registration.ts`, and the body of `ObjectQL#registerHook` into `engine-hooks-and-functions.ts`; the methods stay on the class as one-call stubs with their signatures and docblocks. The built declaration files are byte-equal before and after, and no behaviour changes; only the source maps move. Nothing to do on upgrade.
12+
- The `manifest` liveness ledger's seven rows that cited `engine.ts#registerApp` now cite `engine-registration.ts`, where the reads live (`registerApp`, and `registerPackageBody` for `objects`), because `engine.ts` keeps only a one-call stub that names no manifest key.

‎packages/objectql/src/engine-hooks-and-functions.ts‎

Lines changed: 83 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@
44
* [#29986 codemod] The
55
* "Class head: units, hooks, functions, body runners, hook dispatch" section of
66
* `ObjectQL`: the bodies of `recordResidualOwner`, `residualOwnerSource`,
7-
* `getStatus`, `unregisterHooksByPackage`, `registerFunction`,
7+
* `getStatus`, `registerHook`, `unregisterHooksByPackage`, `registerFunction`,
88
* `resolveFunction`, `resolveFunctionEntry`, `unregisterFunctionsByPackage`,
99
* `setDefaultBodyRunner`, `getDefaultBodyRunner`, `setDefaultActionRunner`,
1010
* `getDefaultActionRunner`, `triggerHooks`, `hasHooksFor`,
@@ -42,8 +42,11 @@ import { HookTargetRebindError } from './hook-target-rebind-errors.js';
4242
import { declaredHookCondition } from './hook-wrappers.js';
4343
import { CoreServiceName } from '@objectstack/spec/system';
4444
import { bindHookNotice } from './hook-notice.js';
45-
import { hookMatchesObject } from './engine-top-level.js';
46-
import type { FunctionEntry, FunctionRegistrationOptions, HookHandler } from './engine-top-level.js';
45+
import {
46+
DISPATCHABLE_HOOK_EVENTS, assertDispatchableHookEvent, assertHookScopeNotSelfCancelling,
47+
assertValidHookExcludeObjects, assertValidHookObject, assertValidUnscopedMultiWriteFlag, hookMatchesObject,
48+
} from './engine-top-level.js';
49+
import type { FunctionEntry, FunctionRegistrationOptions, HookEntry, HookHandler } from './engine-top-level.js';
4750
import type { ObjectQL } from './engine.js';
4851

4952
/**
@@ -95,6 +98,83 @@ export class HooksAndFunctionsFacet {
9598
};
9699
}
97100

101+
/** The body of `ObjectQL#registerHook`; the method's docblock there is its contract. */
102+
static registerHook(this: Pick<HooksAndFunctionsHost, 'hooks' | 'logger'>, event: string, handler: HookHandler, options?: {
103+
object?: string | string[];
104+
/**
105+
* [#5928] Object name(s) subtracted from what `object` admits — the way to
106+
* say "global, except these". See {@link HookEntry.excludeObjects} for why
107+
* an allow list cannot express it, and
108+
* {@link assertValidHookExcludeObjects} for the two refused shapes.
109+
*/
110+
excludeObjects?: string | string[];
111+
priority?: number;
112+
packageId?: string;
113+
/**
114+
* [#9719, widened by #9974] Opt-in whole-operation dispatch for an
115+
* UNSCOPED predicate write (`multi: true`, no `where`) — see
116+
* {@link HookEntry.dispatchUnscopedMultiWrite}. Valid on `beforeUpdate` /
117+
* `beforeDelete` registrations only.
118+
*/
119+
dispatchUnscopedMultiWrite?: boolean;
120+
/** Original metadata Hook definition (set by `bindHooksToEngine`). */
121+
meta?: any;
122+
/** Stable name from metadata (set by `bindHooksToEngine`). */
123+
hookName?: string;
124+
}): void {
125+
// Refuse a scope that is statically decidable as meaningless before
126+
// anything is registered or reported. Each face is checked on its own
127+
// first, so the combined check below can assume well-formed names.
128+
// [#5928] An exclusion face that subtracts nothing (`''`) or everything (`'*'`).
129+
assertValidHookExcludeObjects(options?.excludeObjects, event);
130+
// [commit 708431313] An allow face that names nothing (`''` → global, `[]`/`['']` → never fires).
131+
assertValidHookObject(options?.object, event);
132+
// [commit 708431313] Two well-formed faces that cancel out (`'account'` minus `'account'`).
133+
assertHookScopeNotSelfCancelling(options?.object, options?.excludeObjects, event);
134+
// [#9719/#9974] The unscoped-multi-write flag on an event whose dispatch never reads it.
135+
assertValidUnscopedMultiWriteFlag(options?.dispatchUnscopedMultiWrite, event);
136+
// [#17713] An event inside the engine's OWN lifecycle namespace that the
137+
// engine never dispatches — `beforeCount`, `beforeFindOne` and their four
138+
// siblings. #3195 warned and registered anyway; the handler then never ran,
139+
// which on a read filter is a guardrail the author believes they armed.
140+
// Refused at the door, with the per-seam repair named.
141+
assertDispatchableHookEvent(event);
142+
// [#3195] Guard against enum-vs-dispatch drift for every OTHER unknown
143+
// name: a hook on an event the engine never triggers would register
144+
// "successfully" and then silently never fire. Warn loudly rather than
145+
// swallow it. Still not a hard reject here — a custom driver/plugin may
146+
// dispatch its own events via `triggerHooks`, and outside the engine's own
147+
// vocabulary that is the likely reading (#17713 keeps it intact).
148+
if (!DISPATCHABLE_HOOK_EVENTS.has(event)) {
149+
this.logger.warn(
150+
`Hook registered for '${event}', which the engine never dispatches — it will never fire. ` +
151+
`Dispatchable events: ${[...DISPATCHABLE_HOOK_EVENTS].join(', ')}. ` +
152+
`(Read filtering → RLS/permissions; field masking → field metadata; delete guards → beforeDelete.)`,
153+
{ event, object: options?.object, hookName: options?.hookName },
154+
);
155+
}
156+
if (!this.hooks.has(event)) {
157+
this.hooks.set(event, []);
158+
}
159+
const entries = this.hooks.get(event)!;
160+
entries.push({
161+
handler,
162+
object: options?.object,
163+
excludeObjects: options?.excludeObjects,
164+
priority: options?.priority ?? 100,
165+
packageId: options?.packageId,
166+
dispatchUnscopedMultiWrite: options?.dispatchUnscopedMultiWrite,
167+
meta: options?.meta,
168+
hookName: options?.hookName,
169+
});
170+
// Sort by priority (lower runs first)
171+
entries.sort((a, b) => a.priority - b.priority);
172+
// The exclusion face is reported alongside the allow face: a registration's
173+
// scope is BOTH halves (#5928), and a log that printed only `object` would
174+
// describe a global hook for an entry that is global-minus-twenty-tables.
175+
this.logger.debug('Registered hook', { event, object: options?.object, excludeObjects: options?.excludeObjects, priority: options?.priority ?? 100, totalHandlers: entries.length });
176+
}
177+
98178
/** The body of `ObjectQL#unregisterHooksByPackage`; the method's docblock there is its contract. */
99179
static unregisterHooksByPackage(this: Pick<HooksAndFunctionsHost, 'hooks' | 'logger'>, packageId: string): number {
100180
if (!packageId) return 0;

0 commit comments

Comments
 (0)