Skip to content

Commit 4adc890

Browse files
refactor(objectql): type the six #4918 erasure-pinned ObjectQL methods and move their bodies out of engine.ts with the codemod's fourth pass (#30463)
domain:server#3 · os-bill · session_01Utp6CXX2qjqm2uXbJA8Jcv <!-- claude-projects-attribution: {"github_login":"os-bill"} --> _Requested by **Bill** · [project thread](https://claude.ai/code/project/chan_016iMXWSZDzqWRWvWzdNo8wg?thread=cmsg_016iMXWSZDzqWRWvWzdNo8wgErpL5pVngyYYPX5sGx2GhS)_ Fixes #30055 Clause-②: no Metadata-protocol: none ## 维护者速读 - 改了什么:`engine.ts` 里被 #4918 擦除钉住的六个 `ObjectQL` 方法,先把查询选项补上类型(`engine.ts` 的擦除点 6 → 0,退出 erasure baseline),再用 codemod 新加的第四遍 `--members` 把方法体搬进三个已有的 facet 模块。`engine.ts` 11,771 → 11,316 行。PIN 表和 facade 例外表各退六行,`referenceExists` 的 read-invention 台账行跟着改指向。 - 为什么改:维护者在 #30012 上说「我觉得文件还是太长,还有进一步的拆分优化空间吗」「可以后续立卡处理」「合并之后再继续」。第一遍 codemod 只能在未拆分的文件上跑,所以加了第四遍,它在已拆分的文件上按第一遍的规则搬指定方法。 - 风险与回滚:行为不变。四处直接删掉 `as any`,另外两处保留具名断言(见 Acceptance notes)。搬移由撤销证明逐字节还原 base;`.d.ts` 声明不变,只少了随方法体搬走的私有 docblock;objectql 测试 base 550 文件 / 10,979 = head。回滚就是 revert。 - 先看:① `engine.ts` 里六个擦除点的补类型提交 `c358b417`、`162cb326`;② `scripts/codemod-split-engine.mjs` 新的第四遍一节(`MEMBERS_CARD`);③ `engine-references-and-master-detail.ts` 新增的四个成员。 - 两分钟验证:`node scripts/codemod-split-engine.mjs --members --verify --base baf1a53^`,应看到 `6 moved method(s)` 和 `does rebuild the base engine.ts and its modules byte-for-byte`,退出 0。 - 需要你做的:无,席位落地。 ## What changes 1. **The six #4918 sites are typed** (commits `c358b417`, `162cb326`, no behaviour change). `referenceExists`, `resolveMasterDetailParent`, `resolveMasterDetailParents` and `probeReferencingRows` drop the `as any` outright: their literals (`where` / `fields` / `context`) already satisfy `EngineQueryOptions`, so `tsc` now checks them. Two sites keep a named assertion instead of an erasure: - `seedAutonumber`: the keyset walk's page query types its `where` as `unknown` (it is this scan's filter AND-ed with the seek predicate), so the spread names that one slot as `EngineQueryOptions['where']` (an `Omit` of `where` from `KeysetPageQuery`, intersected with that slot). The runtime object is the same spread. - `inspectDanglingReferences`: the published `DanglingReferenceAuditPort.find` spells its options as a string-keyed record of unknowns, so the hand-over is `opts as EngineQueryOptions`. Changing the port would change the public `.d.ts`, which this card rules out. The rule's sites in `engine.ts` go from 6 to 0, so `engine.ts` leaves `scripts/query-options-erasure-baseline.json` and the lint rule now covers it like any other file. 2. **The codemod gains a fourth pass, `--members`** (`scripts/codemod-split-engine.mjs`). The first pass refuses an `engine.ts` that already carries its stubs, and `main` has been split since #29986, so "re-run the codemod" needs a pass that works on the split file. `--members A,B` moves the named methods exactly as the first pass would: same stay rules (any reason refuses), same stub, same facet member, same docblock rule, into the facet module of the method's section that the first pass wrote, in class order. That module's header list, host interface and import lines grow by what the moved bodies read, and so do the class's statics route and that facet's host factory. The pass never starts a facet, reroutes a module or widens a generated export list; each of those refuses with exit 2. `--members --verify --base REF` undoes the moves from the head alone and requires the base `engine.ts` and modules byte for byte, every other generated module unchanged, and each moved member equal to its base method's text. Pass 1's renderers are shared, not copied: five pure helpers were extracted from it (`classStaticsOf`, `specifierImportLines`, `moduleHeaderLines`, `facetMethodText`, `hostInstallLines`), and its 105 self-test cases still pass unchanged. 3. **The six bodies move** (commit `baf1a536`, generated, not hand-edited): `node scripts/codemod-split-engine.mjs --members seedAutonumber,referenceExists,resolveMasterDetailParent,resolveMasterDetailParents,inspectDanglingReferences,probeReferencingRows --write`. - `engine-defaults-and-autonumbers.ts` takes `seedAutonumber`; `engine-references-and-master-detail.ts` takes `referenceExists`, `resolveMasterDetailParent`, `resolveMasterDetailParents` and `inspectDanglingReferences`; `engine-cascade-planning.ts` takes `probeReferencingRows`. - The private five move with their docblocks; `inspectDanglingReferences` is public, so its docblock stays on the class and the module carries the placeholder. - `ObjectQLStatics` gains `referenceCheckContext`. The references facet's host gains `find`, `findOne`, `materializeParentHeader`, `resolveEnginePosture`. Nine import bindings only the moved bodies read leave `engine.ts`. 4. **The pin and allow-list rows retire**: the six `PATH_READER_PINS` rows (the table is now empty), the six #30019 no-body exceptions in `scripts/facade-delegation-registry.json`, and the `referenceExists` entry in `scripts/durability-read-invention.baseline.json` now names `engine-references-and-master-detail.ts`, where its body lives. Size: 9 files, +1,479 / −609 = 2,088 changed lines (under 3,000). ## Tests - `node scripts/codemod-split-engine.mjs --self-test`: 124 cases in 9 batteries pass. The new battery `named methods onto the split class (fourth pass)` has 18 cases: the first pass keeps two pinned fixture methods; the fourth pass moves both into the right module in class order; the proof holds and two runs write identical trees; the stub is the first pass's with `async` dropped; a private docblock moves and a protected one stays; the header, host interface and factory, statics route and imports each grow by exactly what the bodies read; engine.ts prunes what only the bodies read and keeps a type a stub's signature still names; unknown, already-moved, stay-rule and still-pinned names refuse; and the proof fails on an edited body character, a host member nothing reads, an unread import, a stub that kept `async`, an unrelated edit in `engine.ts` or another module, and a head that moved nothing. - `pnpm --filter @objectstack/objectql test`: base (`f7afab86` sources) 550 files / 10,979 tests passed; head 550 / 10,979 passed. - `pnpm --filter @objectstack/objectql typecheck`: exit 0 (tsc and `check:test-typecheck`). ## Acceptance notes - Two named assertions stay (the two sites in What changes 1). They name the contract the value is read under, which is what the #4918 rule asks for when the value crosses a boundary typed looser than the engine. Neither is counted by the rule. - `homeAmountReadsFor` (added by hand in #30280) sits in `engine-references-and-master-detail.ts` out of class order and missing from the module header's list. The fourth pass reads the header's own list and adds its names to it, leaving that drift as it stands. Not a defect; noted for whoever next regenerates the module. - `.d.ts`: the private methods' docblocks leave the bundled `.d.ts` with their bodies (211 lines in each of `util-HASH.d.ts` and `util-HASH.d.mts`). Ruling 6624345766 item 1 says a private docblock moves with its body. This is the same reading #30341 / #30415 gave. ## Verification <details><summary>Undo proof on the real tree</summary> `node scripts/codemod-split-engine.mjs --members --verify --base baf1a53^` exit 0: ```text undo proof: 6 moved method(s) (seedAutonumber, referenceExists, resolveMasterDetailParent, resolveMasterDetailParents, inspectDanglingReferences, probeReferencingRows); engine.ts 11776 -> 11316 lines; undoing the moves does rebuild the base engine.ts and its modules byte-for-byte once the 9 import binding(s) only moved bodies read are pruned from the base (AUTONUMBER_SEED_PAGE_SIZE, AuditableObject, KeysetPageQuery, declaredMultiValued, isMissingTableError, isStoredMetadataBodyObject, keysetWalk, materializeDeclaredFields, readStoredAutonumberCounter) ``` Against `main` (`f7afab86`) the same command refuses all six (`the facet member is not its base method's text`), because the typing commits changed those bodies. The move is proved against the commit just before it; the typing is the reviewed diff. </details> <details><summary>Declarations, base vs head</summary> `pnpm --filter @objectstack/objectql build` at `f7afab86` sources and at head. The chunk name hash (`util-HASH`) was normalised. `core.d.ts`, `core.d.mts`, `index.d.ts` and `index.d.mts` are byte-identical once the hash is normalised. The two `util-HASH` files differ by 422 lines, every one a docblock line. With docblocks stripped, all six declaration files are byte-identical. </details> <details><summary>Ablation</summary> Re-erasing one moved site (`as any` back on `referenceExists`'s options in `engine-references-and-master-detail.ts`, through `scripts/ablation-replace.mjs`): `pnpm check:query-options-erasure` exits 1 with `engine-references-and-master-detail.ts: NEW engine query-options erasure (1 site(s))`. Restored to HEAD; the hash matches and `git diff HEAD` is empty. </details> <details><summary>Gates</summary> `node scripts/pm/dispatch-gates.mjs --commands --repo objectstack-ai/objectstack` derived 74 families, 80 commands. Among them are the fourth-pass verify, the codemod self-test, eslint on the touched files, and `pnpm --filter @objectstack/spec check:liveness`. All 80 recorded `exit 0`, and `--ran` reconciles: `74 derived famil(ies) accounted for — 74 run, 0 NOT-MEASURED`. Two first ran `exit 3` (prerequisite, not measured) and were rerun green: `check-plugin-teardown-shape --self-test` needed its pinned fixture commit fetched into the shallow clone, and `check:dual-build-cjs-loads` needed `pnpm build` (119 of 119 tasks; then 116 entry points across 71 packages load). Named here because the card asks: `check-facade-delegation-only` passes with `5 facade class(es) in 3 file(s) hold delegations only, 62 named exception(s)` (six fewer); `check:query-options-erasure` passes with `30 unswept … 13 file(s), none new`; `check:liveness` passes. </details> --- 🤖 Generated with [Claude Code](https://claude.com/claude-code) https://claude.ai/code/session_01Utp6CXX2qjqm2uXbJA8Jcv --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent 8b0fbb8 commit 4adc890

9 files changed

Lines changed: 1479 additions & 609 deletions
Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
---
2+
'@objectstack/objectql': patch
3+
---
4+
5+
refactor(objectql): the six `ObjectQL` methods the #4918 query-options erasure pin kept in `engine.ts` are typed and move into their facet modules; no behaviour or public declaration changes
6+
7+
Clause-②: no
8+
Metadata-protocol: none
9+
10+
- The six internal reads in `seedAutonumber`, `referenceExists`, `resolveMasterDetailParent`, `resolveMasterDetailParents`, `inspectDanglingReferences` and `probeReferencingRows` no longer erase their query options to `any`. Four are plain literals `tsc` now checks against `EngineQueryOptions`; the keyset walk's `where` and the dangling-reference audit's port options are named as `EngineQueryOptions` slots instead. `engine.ts` leaves the erasure baseline with zero sites.
11+
- A new fourth pass of the `engine.ts` codemod, `--members`, moves the six bodies the way the first pass moves a body, into the facet modules it already wrote, and proves the move by undoing it. `engine.ts` goes from 11,771 to 11,316 lines.
12+
- Nothing to do on upgrade: the exported names, the public `.d.ts` declarations and what every method does are the same.

‎packages/objectql/src/engine-cascade-planning.ts‎

Lines changed: 113 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -6,9 +6,9 @@
66
* `storedReferenceIncludes`, `remainderAfterMemberRemoval`,
77
* `referenceCheckContext`, `dependentCountIsDisclosable`,
88
* `cascadeDeleteRelations`, `cascadeRelationBehavior`,
9-
* `fileReferenceCheckElevation`, `referenceProbeFilterOfSet`,
10-
* `probeReferencingRowsOfSet`, `walkReferencingRelations`,
11-
* `isTenantColumnSetNull`, `outlivesOrganization`,
9+
* `fileReferenceCheckElevation`, `probeReferencingRows`,
10+
* `referenceProbeFilterOfSet`, `probeReferencingRowsOfSet`,
11+
* `walkReferencingRelations`, `isTenantColumnSetNull`, `outlivesOrganization`,
1212
* `keepsOrganizationReference`, `countReferencingRowsOutside`,
1313
* `organizationCountContext`, `organizationCountIsDisclosable`,
1414
* `refuseOrganizationDeleteWhileOwned`, `organizationOwnedRowCounts` moved out
@@ -658,6 +658,116 @@ export class CascadePlanningFacet {
658658
this.recordReferenceCheckElevation(object, id, childName, fieldName, context);
659659
}
660660

661+
/**
662+
* The dependents probe of {@link cascadeDeleteRelations}: the rows of
663+
* `childName` whose `fieldName` references `id`, read under SYSTEM identity
664+
* and exactly narrowed. [#22305] Both phases read through it, so the set the
665+
* collection walk gathers and the rows the deleting walk acts on are one
666+
* reading of the same relation.
667+
*/
668+
static async probeReferencingRows(this: Pick<CascadePlanningHost, 'find'>, childName: string, fieldName: string, fdef: any, id: string | number | ReadonlyArray<string | number>, probeWhere: Record<string, unknown>, context: ExecutionContext | undefined): Promise<any[]> {
669+
let dependents: any[];
670+
try {
671+
dependents = await this.find(
672+
childName,
673+
// [#12166] (maintainer ruling 2026-08-26, option A) SYSTEM identity,
674+
// unconditionally. This probe is the platform's own referential-
675+
// integrity read, not a query the caller asked for, and running it
676+
// as the caller made "delete permission" silently mean "delete +
677+
// read on EVERY referencing table": a caller with full delete rights
678+
// on `object` but no read grant on `childName` got a blanket 403
679+
// from the security middleware — whether or not a reference existed,
680+
// and with `childName` EMPTY. Measured on a real deployment across
681+
// 17 role×object pairs, with the A/B control that granting read-only
682+
// on the referencing object (touching NOTHING about delete rights)
683+
// turned the identical delete into a 200.
684+
//
685+
// Referential-integrity actions are engine responsibility executed
686+
// under system identity on every mainstream platform — the RDBMS FK
687+
// baseline, Salesforce (lookup clearing / cascade delete documented
688+
// as bypassing sharing), Dataverse, ServiceNow, Odoo. Caller
689+
// identity here was the outlier.
690+
//
691+
// The elevation is `sudo()`-SHAPED (`{ ...context, isSystem: true }`),
692+
// never a bare `{ isSystem: true }` — the same posture
693+
// `recomputeSummaries` holds one axis over. Three reasons, and each
694+
// one is a defect if dropped:
695+
// - the open transaction handle, `tenantId` and `timezone` must
696+
// survive, or this probe leaves the caller's transaction and
697+
// stops being TENANT-scoped — a bare system context would widen
698+
// the probe across the tenant wall, which is the opposite of
699+
// what this card relaxes;
700+
// - `userId` survives, and that IS the audit ledger's
701+
// "triggered-by" half (ruling constraint 3): the record carries
702+
// triggered-by = the deleting operator and executed-as = system.
703+
// `read-audit.ts` states the same property of `sudo()`;
704+
// - it is a NARROW elevation: nothing else about the delete path
705+
// changes identity (ruling constraint 1). The `set_null` UPDATE
706+
// and the `cascade` DELETE below still run as the caller, byte
707+
// for byte, so this relaxes the reference CHECK and not the
708+
// caller's own authority over the dependent rows.
709+
//
710+
// [Constraint 4] If the spec later declares per-relationship
711+
// on-delete behaviour, BEHAVIOUR follows the declaration; the
712+
// identity of this probe stays system, unconditionally. Do not make
713+
// this line conditional on `behavior`.
714+
{ where: probeWhere, context: ObjectQL.referenceCheckContext(context) },
715+
);
716+
} catch (error) {
717+
// [#8895] Discriminate by error TYPE — this probe IS the referential
718+
// guard, so an empty answer is only truthful for the one failure that
719+
// really means "no dependents".
720+
//
721+
// The bare `catch { continue }` this replaces reached `continue` on
722+
// ANY probe failure, and every consequence of that is silent: the
723+
// walk's `restrict` branch never fires, so a delete the integrity
724+
// rules say must be REFUSED is allowed through; `set_null`/`cascade`
725+
// never run, so child rows that should have been nulled or removed
726+
// are left orphaned; nothing is logged and the caller is told the
727+
// delete succeeded. Fail-OPEN on an integrity guard — the read did
728+
// not happen and the answer "there are none" was invented for it
729+
// (ADR-0110 D3: "the probe found nothing" and "the probe could not
730+
// run" are different facts with opposite meanings here).
731+
//
732+
// Benign: the child object is registered but its TABLE was never
733+
// provisioned (schema sync not run yet). It cannot hold a row that
734+
// references anything, so zero dependents IS the truth and skipping
735+
// the relation is correct. Asked through the shared
736+
// `isMissingTableError` predicate (`@objectstack/metadata/errors`,
737+
// #4825) — the same call `seedAutonumber` and `resolveFileReferences`
738+
// make — never a hand-rolled code test.
739+
//
740+
// Everything else (connection drop, timeout, permission denial, a
741+
// query error, a missing COLUMN on a provisioned table) means the
742+
// dependents may well exist and simply were not seen. It propagates:
743+
// the delete fails loudly and nothing is written, which is the same
744+
// disposition the maintainer's 2026-08-15 ruling gives this family —
745+
// unprovisioned is truthful emptiness, everything else must surface.
746+
// A guard that could not be EVALUATED must not silently pass.
747+
//
748+
// No new response field and no new error code: the caller receives
749+
// the probe's own failure, envelope intact.
750+
if (isMissingTableError(error, childName)) return [];
751+
throw error;
752+
}
753+
// [#9362] The multi-value pushdown in the probe filter can be a SUPERSET
754+
// (the substring reading, on a backend without the declaration), so the
755+
// exact answer is taken here, on the rows themselves. Everything the walk
756+
// does with them — the `restrict` count in the 409 envelope, the `cascade`
757+
// recursion, the `set_null` write — and the set the collection walk
758+
// gathers read this answer, so narrowing anywhere later would leave one of
759+
// them acting on a row that never referenced this record.
760+
// [#27004] Over a set of ids (`probeReferencingRowsOfSet`), a row is kept
761+
// when it holds any of them.
762+
if (declaredMultiValued(fdef) && dependents) {
763+
const wanted: ReadonlyArray<string | number> = Array.isArray(id) ? id : [id as string | number];
764+
dependents = dependents.filter((row) =>
765+
wanted.some((w) => ObjectQL.storedReferenceIncludes(row?.[fieldName], w)),
766+
);
767+
}
768+
return dependents ?? [];
769+
}
770+
661771
/**
662772
* [#27004] {@link referenceProbeFilter} for a SET of records: the rows that
663773
* reference any of `ids`. A single-valued reference asks `$in`; a

0 commit comments

Comments
 (0)