Skip to content

Commit beb09fb

Browse files
refactor(objectql): move count and rejectCredentialAggregation out of engine.ts into engine-count-aggregate.ts (#25522 cut 18) (#26932)
<!-- claude-projects-attribution: {"github_login":"os-project-manager"} --> _Requested by **Leehom** · [project thread](https://claude.ai/code/project/chan_01TenzsRopZj4jMbvjX8uz1X?thread=cmsg_01TenzsRopZj4jMbvjX8uz1XJNnGo3BD78wE9eBrmiBx5g)_ Part of #25522 Clause-②: no Metadata-protocol: none Ruling-ref: 6348891160 + 6441032442 ## 维护者速读 - **改了什么**:把 `engine.ts` 里「count / aggregate」区的两个方法体(`count` 计数门,和 `aggregate()` 的凭据字段聚合守卫 `rejectCredentialAggregation`)原样搬进新文件 `engine-count-aggregate.ts`;类上的方法签名、注释、位置不变,方法体换成转交。`count` 与留下的 `find` / `findOne` / `update` / `delete` / `aggregate` 共用的 7 个顶层 helper 原地加 `export`、由新文件导入(不复制)。`engine.ts` 21,835 → 21,762 行。 - **为什么改**:#25522 按缝拆分 `engine.ts` 的第 18 刀。重测的 seam 表里价最高的是删除级联区(−248),但派单把它和相邻两区排除(#26864 刚落地、#26871 正在那里开发);其次是本区,定价 −71,高于 −30,且没有任何 open PR 触及。`aggregate` 本身留在原文件,因为 `packages/spec/liveness/query.json` 以 `engine.ts#aggregate` 登记它为生产者。 - **风险与回滚**:纯搬移,无行为变化;公开 `.d.ts` 六个文件与基线逐字节相同(含阳性对照);运行期导出名单相同;构建产物只多出新文件一个区段,外部包导入顺序一条不差(新文件紧接 `./secret-fields.js` 导入);一次性晚绑定探针在新旧两版构建产物上逐行一致。回滚 = revert 本 PR。 - **先看**:`packages/objectql/src/engine-count-aggregate.ts`(两个函数与 `CountAggregateHost`)、`packages/objectql/src/engine.ts` 里两个转交桩、新导入行与 7 处原地 `export`。 - **两分钟验证**:`pnpm --filter @objectstack/objectql exec vitest run --project local --maxWorkers=2 src/engine-aggregate-filter.test.ts src/engine-middleware-operation-vocabulary.test.ts`,应见两个文件全部通过(聚合与计数门各自走到的测试)。 - **需要你做的**:无,席位落地。 Cut 18 of the `packages/objectql/src/engine.ts` split (#24586 ruling A, recorded in `6270097840`), under the claim `6453372157`. Two bodies of the "count / aggregate" section move unedited into a new module, `packages/objectql/src/engine-count-aggregate.ts`, behind `CountAggregateHost`: `count` (the counting door: alias fold, option refusal, filter-array lowering, search expansion, the operation context, token resolution, the middleware chain, the readable-ids boundary and the `find()` fallback) and `rejectCredentialAggregation` (`aggregate()`'s fail-closed guard against grouping or aggregating a `secret` / `password` / `internal: true` field). No behaviour changes. Every published declaration file is byte-identical to the base. #25522 stays open for the later cuts. Branch `claude/project-thread-axdp1m` (the card thread's harness-pinned branch), cut from `origin/main` at `1708115e2f`, which contains cut 17's merge (`cea0871ff2`, #26881) and #26864. ## What changes ### Why this area Re-measured on `1708115e2f` with cut 11's measure script and cut 14's price step (`price3.mjs`), both unchanged. `engine.ts` is 21,835 lines, 6,991 of them from the last 30 days (`git blame --since=2026-09-27T21:00Z`, clone deepened to 2026-09-21 first); `class ObjectQL` holds 326 members, 261 methods and accessors. Query-options erasure sites: 8 at base (lines 8408, 9648, 9894, 9958, 10290, 13570, 13869, 18499), none in this area. | price (net lines) | section | taken? | |---|---|---| | -248 | Delete cascade and reference walking | no: the order excludes it (#26864 landed there; #26871, branch `claude/project-thread-vdrn59`, is developing the predicate-delete walk across it) | | **-71 priced, -73 landed** | **count / aggregate** | **yes** (`count` 58 body lines, `rejectCredentialAggregation` 38; `aggregate` stays, named `engine.ts#aggregate` in `packages/spec/liveness/query.json`) | | -40 | Query lowering | runner-up: pays less | | -27 | Hook dispatch internals; `resolveFormulaRelated` | pays less; `resolveFormulaRelated` is a later cut, as the order says | | -21 or less | find / findOne, the rest | pays less | 36 of the 96 moved body lines are from the last 30 days. The two other excluded sections ("Dangling-reference and cascade inspection", "Referential integrity and master-detail resolution") price below -30 anyway. `registerMetadataCollections` was not considered (the order excludes it). <details><summary>Seam table on <code>1708115e2f</code> (40 sections of <code>class ObjectQL</code>; U = untyped receivers)</summary> | range | section | first … last member | lines | 30-day lines | methods | inferred public | U | private statics | |---|---|---|---|---|---|---|---|---| | 4974-4996 | Status and registry accessor | `getStatus` … `registry` | 23 | 0 | 2 (2 public) | getStatus | 0 | - | | 4997-5340 | Hook and function registry | `registerHook` … `triggerHooks` | 344 | 6 | 16 (16 public) | registerHook, triggerHooks | 1 | - | | 5341-5696 | Hook dispatch internals | `hasHooksFor` … `assertBulkPerRowHookBudget` | 356 | 61 | 7 (0 public) | - | 0 | - | | 5697-5859 | Action registry and activation | `registerAction` … `setActionActive` | 163 | 24 | 10 (10 public) | - | 1 | - | | 5860-6141 | Registered resolvers and write-gate probes | `_heldFileResolver` … `registerInsertRecomputeListener` | 282 | 116 | 6 (6 public) | - | 0 | - | | 6142-6783 | Readable-ids security and permission/formula resolution | `readableIdsSecurityReported` … `resolveDefaultPermissions` | 642 | 509 | 16 (2 public) | - | 0 | - | | 6784-6890 | Authz invalidation and middleware registration | `writeEpoch` … `hasObjectMiddleware` | 107 | 18 | 4 (4 public) | - | 0 | - | | 6891-7068 | Execution plumbing (middleware, session, provenance, users) | `executeWithMiddleware` … `buildUser` | 178 | 45 | 7 (0 public) | - | 0 | - | | 7069-7277 | Driver options and engine posture | `buildDriverOptions` … `resolveEnginePosture` | 209 | 150 | 6 (0 public) | - | 0 | - | | 7278-7487 | Organization probe and system-insert organization | `probeInstallOrganizations` … `transactionCoversDriverFor` | 210 | 45 | 4 (0 public) | - | 0 | - | | 7488-7853 | Field defaults | `resolveNowDefault` … `defaultFieldEntries` | 366 | 4 | 6 (0 public) | - | 0 | - | | 7854-8466 | Autonumber | `applyAutonumbers` … `seedAutonumber` | 613 | 80 | 6 (0 public) | - | 1 | - | | 8467-9053 | Package, app, plugin and driver registration | `registrationConversionWarned` … `registerDriver` | 587 | 157 | 6 (3 public) | registerApp, registerDriver | 0 | - | | 9054-9262 | Datasource registry | `supplyFileColumnsMovedResolver` … `listUnavailableDatasources` | 209 | 1 | 8 (7 public) | - | 3 | - | | 9263-9414 | Write guard and realtime publish | `assertWriteAllowed` … `publishBulkDataEvent` | 152 | 18 | 4 (1 public) | - | 0 | - | | 9415-9522 | Validation-message i18n | `setI18nService` … `objectDisplayLabel` | 108 | 24 | 4 (1 public) | - | 0 | - | | 9523-10231 | Referential integrity and master-detail resolution | `assertReferencesResolve` … `masterDeclaredFields` | 709 | 308 | 12 (0 public) | - | 0 | - | | 10232-10347 | Dangling-reference and cascade inspection | `inspectDanglingReferences` … `inspectCascadeDelete` | 116 | 48 | 2 (2 public) | - | 0 | - | | 10348-10442 | Crypto and tenancy providers | `setCryptoProvider` … `setTenancyPostureProvider` | 95 | 20 | 4 (4 public) | - | 1 | - | | 10443-10862 | Secret and internal-field handling | `encryptSecretFields` … `resolveInternalField` | 420 | 61 | 9 (3 public) | - | 0 | - | | 10863-10931 | Schema lookup and filter judging | `getSchema` … `judgeFilter` | 69 | 25 | 3 (2 public) | - | 0 | - | | 10932-11197 | Datasource routing | `LIFECYCLE_DATASOURCE` … `setDatasourceMapping` | 266 | 20 | 8 (3 public) | setDatasourceMapping | 0 | - | | 11198-11365 | Lifecycle init and health | `init` … `resolvePrimaryDatasource` | 168 | 16 | 3 (3 public) | init | 0 | - | | 11366-11439 | Media and value-shape presence | `objectHasMediaField` … `valueShapeStrictFor` | 74 | 13 | 4 (0 public) | - | 0 | mediaFieldPresence, coveredValueFieldPresence | | 11440-12029 | Migration flags and attestations | `isFileReferencesMigrationVerified` … `wasDatastoreCreatedFromEmpty` | 590 | 96 | 17 (8 public) | - | 0 | - | | 12030-12052 | destroy | `destroy` … `destroy` | 23 | 0 | 1 (1 public) | destroy | 0 | - | | 12053-12071 | Expand/cascade limits (statics) | `MAX_EXPAND_DEPTH` … `MAX_BULK_PER_ROW_HOOK_ROWS` | 19 | 0 | 0 (0 public) | - | 0 | MAX_EXPAND_DEPTH, MAX_CASCADE_DEPTH | | 12072-12522 | Roll-up summaries | `autonumberCounters` … `recomputeSummaries` | 451 | 0 | 7 (1 public) | - | 0 | - | | 12523-12776 | Related-record expansion | `expandRelatedRecords` … `expandRelatedRecords` | 254 | 10 | 1 (0 public) | - | 0 | - | | 12777-12974 | File-reference hydration | `resolveFileReferences` … `readParentDerivedFiles` | 198 | 183 | 5 (0 public) | - | 0 | - | | 12975-13453 | Query lowering | `resolveWhereTokens` … `requireFindOnePredicate` | 479 | 271 | 9 (0 public) | - | 0 | - | | 13454-14010 | find / findOne | `find` … `staticReadonlyCreateStrip` | 557 | 142 | 4 (2 public) | - | 0 | - | | 14011-14322 | validate | `validate` … `assignment` | 312 | 224 | 4 (1 public) | - | 0 | - | | 14323-15692 | insert / insertMany | `readUpdatePriorRow` … `insertMany` | 1370 | 462 | 3 (2 public) | - | 1 | - | | 15693-17640 | update | `update` … `update` | 1948 | 290 | 1 (1 public) | - | 0 | - | | 17641-19325 | Delete cascade and reference walking | `planCascadeAtomicity` … `organizationOwnedRowCounts` | 1685 | 955 | 24 (1 public) | - | 0 | storedReferenceIncludes, remainderAfterMemberRemoval, referenceCheckContext, outlivesOrganization, keepsOrganizationReference, organizationCountContext | | 19326-19895 | delete | `delete` … `delete` | 570 | 47 | 1 (1 public) | - | 0 | - | | 19896-20515 | count / aggregate | `count` … `aggregate` | 620 | 192 | 3 (2 public) | - | 0 | - | | 20516-20824 | execute and transactions | `execute` … `enforceTransactionOrigin` | 309 | 74 | 8 (2 public) | - | 0 | - | | 20825-21117 | Compatibility API | `registerObject` … `create` | 293 | 52 | 16 (16 public) | - | 0 | - | The per-member screen and the full price output are in the project files, `pm-orders/server/25522-cut18-seam-table.md`. </details> ### The shape - The class keeps both methods with name, modifiers, signature, docblock and position. A body becomes one host literal of exactly the `CountAggregateHost` members it reads, plus a `.call`, marked `[#25522 cut 18]`, as cuts 8 and 10 to 17 did. Fields go as getters over a `readX` closure (`_registry`, `relatedSchemaOf`); methods go as late-bound closures (`resolveObjectName`, `expandSearchOnOptions`, `getDriver`, `resolveWhereTokens`, `executeWithMiddleware`, `buildDriverOptions`, `composeReadableIdsAtBoundary`, `find`), so a spy or double set on the engine is still the one a call reaches. No body assigns a field, so there is no setter (the interface docblock says only "getters" this time, not the cut-17 wording the seat flagged). - **Series limits.** `count` is public with a declared return type (`Promise<number>`); `rejectCredentialAggregation` is private, so its `.d.ts` line is `private rejectCredentialAggregation;` either way. No moved body has a `this.FIELD.delete/insert/update(` site, a private-static read, a self-call to `insert` / `update` / `delete`, an `engine.ts#NAME` pointer or an ADR anchor paragraph of its own. `count` calls `composeReadableIdsAtBoundary` and `executeWithMiddleware`, which ADR-0139 names by path; they stay in the class and are reached through the host. - **Helpers, under the cut-14 seat ruling.** `count` reads seven top-level helpers, all also read by bodies that stay: `foldEngineOptionAliases`, `rejectUnknownEngineOptions`, `lowerWhereFilterArray`, `declaredDatetimeLowering` (`find`, `findOne`, `update`, `delete`, `aggregate`, and `judgeWhereAdmission` for the last two), `ENGINE_WHERE_SLOTS` (`update`, `delete`, `aggregate`), `ENGINE_COUNT_OPTION_KEYS` (`ENGINE_OPTION_KEY_SETS`) and `mergeReadContext` (`expandSearchOnOptions`, `find`, `findOne`, `aggregate`). Each gains `export` in place, byte for byte otherwise (leading docblocks included), and the module imports them as values from `./engine.js`; nothing is copied. `foldEngineOptionAliases` is also named by `packages/spec/liveness/query.json` as `engine.ts#foldEngineOptionAliases`, which still holds: it stays in `engine.ts`. Measured alone before the move (`dts-export-variant-cut18.sh`, base `engine.ts` plus the `export`, no move): each of the seven, and all seven together, leave all published declaration files byte-identical. `index.ts` and `core.ts` name their exports singly, so nothing new is published. - **The run-time cycle and the load order (measured).** The value import makes `engine.ts` and the module import each other. The module runs nothing at evaluation (declarations only). `rejectCredentialAggregation` was `engine.ts`'s only reader of `StandardErrorCode`, so that import line (`@objectstack/spec/api`, with its `[#22646]` comment) moves to the module verbatim. Two positions were built and compared (`cut18-place-builds.sh`, `bundlecmp.py`): importing the module where the `StandardErrorCode` line was (line 31) splits an extra `engine.ts` region and prints the `secret-fields.ts` region ahead of `engine-secret-internal-fields.ts` and the regions between, so the module evaluation order changes; importing it right after `./secret-fields.js` (the one other module it imports), as this PR does, leaves the order intact. Measured on the final head's bundles: with the `// src/engine.ts` markers set aside (10 at base and head), the `// src/...` region order equals the base's with one line added, `// src/engine-count-aggregate.ts` (`index.mjs` 99 to 100 regions, `core.mjs` 88 to 89, `index.js` 100 to 101, `core.js` 89 to 90). The external modules load in the same first-occurrence order, with the same statement counts: 113 and 96 `import` statements in the ESM bundles, 148 and 130 `require(` calls in the CJS bundles. The late-binding probe answers the same on base and head (Tests). - **Imports.** The module imports `EngineAggregateOptions`, `EngineCountOptions`, `QueryAST` (types, `@objectstack/spec/data`), `StandardErrorCode` (`@objectstack/spec/api`), `collectInternalReadFields` and `collectCredentialFields` (`./secret-fields.js`), the seven helpers (values) and `EngineReadOptions`, `ObjectQL`, `OperationContext` (types) from `./engine.js`. `engine.ts` loses the `StandardErrorCode` line; its `./secret-fields.js` import stays (`collectInternalReadFields` is still read there, and `collectCredentialFields` by the staying docblock's `{@link}`). - The module header names what moved, how the bodies reach the class, why `aggregate` stays, and how the helpers and the import position were handled. ### Net lines (the seat's rule) `engine.ts` goes from 21,835 to 21,762 lines, **net -73** (33 added, 106 removed). The module is 202 lines. The diff against `origin/main` is 341 changed lines over 2 files (+235 / -106). ### Readers of the moved lines - No census, ADR anchor, docs row or test key names a moved member by `engine.ts` path. Searched: `engine.ts#count`, `engine.ts#rejectCredentialAggregation` and `engine.ts … count` spellings, plus every `scripts/**` file that reads `engine.ts` (durability baselines, engine-double contract, logger-receiver detach, single-authz resolver, startup-registry verdict, widget-option census, type-check coverage, erasure baseline, tenant-audit and isSystem censuses): none names them. The prose mentions found (`CHANGELOG.md` entries, a `metadata-protocol/src/protocol.ts` docblock, `service-analytics/src/api-exposure-door.ts`) name `rejectCredentialAggregation` by behaviour, not by path. - `packages/spec/liveness/query.json` names `engine.ts#aggregate`, `engine.ts#foldEngineOptionAliases` and `engine.ts#ENGINE_QUERY_SLOTS`: all three stay in `engine.ts`, so nothing under `packages/spec` changes. - `engine-middleware-operation-vocabulary.test.ts` reads the `OperationContext['operation']` union out of `engine.ts`, where it stays; the test passes. The bodies call no `canReadObject(` (core's `row-serving-door-exposure.pin.test.ts` is unchanged), no `referenceTargetOf(`, no `isFederatedObject` / `resolveTenantFieldName`, and read no `isSystem` (`system-context.mdx` is unchanged; its census is green). They hold no `query-options/no-any-erasure` site and no tenant-audit untyped receiver. - **Read-seam census (the cut-21 trap of #25467).** `check-durability-degradation-log-level --list` at base and head: identical once line numbers are masked (164 lines: 48 durability seams and 88 read seams); `count`'s driver read sits outside any catch, so it is no seam. - **No ADR anchor paragraph describes a moved body** (the `engine.ts` entry is the transaction-seam paragraph), so the module needs no anchor entry of its own. - **Decision-letter count:** `check-adr-anchors` at base `1708115e2f` (clean detached worktree): **1,897** decision-letter citations on the anchored surface, 60,009 resolving citations across 8,416 files. At head: **1,896**, 60,009 across 8,417. The one letter that leaves the anchored surface is `[ADR-0139 D2]` in `count`'s body (the executor-boundary comment), which moved verbatim into the unanchored module; cut 15 disclosed the same kind of drop (`[ADR-0139 D6]`, 1,881 to 1,880). No anchor paragraph was written for it: none of the `engine.ts` entry's text describes `count`, and `docs/adr/**` is outside this claim. ## Why #25522 splits `engine.ts` one seam at a time so that a change to one area reads, reviews and conflicts as that area alone. After cut 17, the highest-paying section no open PR touches is "count / aggregate". Nothing about what the engine does changes. ## Tests No test is added and none is edited: the move is proved by identity, not by new behaviour, and no test reads a moved line by path. <details><summary>Public <code>.d.ts</code>: byte-identical (ruling <code>6348891160</code> item 1), with the <code>6441032442</code> report block and a positive control</summary> Built `@objectstack/objectql` at the base (`1708115e2f`: `engine.ts` blob `1878bcbf6512`, no `engine-count-aggregate.ts`) and at head (`engine.ts` blob `c16b33857e02`), each from a clean `src/` (`dts-cut18.sh`: the base leg restores all of `src/` from the base and deletes the module; both legs check the blob). The builds are the moved code, not a stale cache: `count.call(host` occurs 0 times in the base `dist/index.mjs` and once in the head's. Every declaration file is **raw byte-identical** base to head, so `6348891160` item 1 holds without any normalisation, and no chunk is renamed (`util-BaJGwxSm` at both). The `6441032442` block, for the record (normaliser `pm-orders/server/evidence/25522/dtsnorm.mjs`, sha256 `25226e0677bdad24726e9eff76a7e247bf7319440d2430d28cbaab0db6add94b`, run as `node dtsnorm.mjs --diff BASE_FILE HEAD_FILE`; output in `cut18-dtsnorm.txt`): | file | raw sha256 (base = head) | raw-diff lines | statements base / head | normalised sha256 (base = head) | declared-surface digest (base = head) | import / export specifier sets | |---|---|---|---|---|---|---| | `core.d.ts` | `9f22dc90973142623116d4f119dcfbbe37289c83a4b28734425014727212376d` | 0 | 14 / 14 | `40e2fde86719d3ccd1cf492314c426c977ab836a5ab54580800cd1f41352247a` | `4450aaf478092516…` (1 decl) EQUAL | 0 / 84, EQUAL | | `core.d.mts` | `e9a0b5acd001130a4a231c60aa44fbd3b59bcecd0c7d7b032e57e9d4d45d2e51` | 0 | 14 / 14 | `171eab8743fd8dfabc014a162c40c3e034be29f262b26a2bcc24cb3e06be0459` | `4450aaf478092516…` EQUAL | 0 / 84, EQUAL | | `index.d.ts` | `6a16c278b5ac5e3ccafa7eff4f71e5f848bd77431442b455f87bb7fbfe455621` | 0 | 122 / 122 | `2bb6a21a929b41c8dbb85c72c600f87ed3a3af12ca731d1cace61f5d4aa3d57e` | `9136189ce4d0aba3…` (103 decls) EQUAL | 18 / 284, EQUAL | | `index.d.mts` | `284396b272347adb8ee7226e5798f152bf76aa4878c821e3628541de7060b0c8` | 0 | 122 / 122 | `c7d7bab765f3b0fb35241fa03bd0b716a93139c98e5638d209379dad96528f98` | `9136189ce4d0aba3…` EQUAL | 18 / 284, EQUAL | | `util-BaJGwxSm.d.ts` | `cb1fddd4136122b5a0cb983b05eca8e06233614eda7e33156ccc3437c199c90c` | 0 | 185 / 185 | `c43cb8b103cc68a1077b69fcbf78fb71ee5de0ace6651f351cb8e90c7a89cc6f` | `b264bade9e7d8157…` (173 decls) EQUAL | 55 / 147, EQUAL | | `util-BaJGwxSm.d.mts` | `cb1fddd4136122b5a0cb983b05eca8e06233614eda7e33156ccc3437c199c90c` | 0 | 185 / 185 | `c43cb8b103cc68a1077b69fcbf78fb71ee5de0ace6651f351cb8e90c7a89cc6f` | `b264bade9e7d8157…` EQUAL | 55 / 147, EQUAL | - (a) Every file: statements in the same order, `statements only in base: 0; only in head: 0`, declared-surface digest EQUAL. (b) Every normalised sha256 EQUAL. (c) Specifier sets EQUAL; the run-time export lists are too (`index` 182 names and `core` 54, ESM and CJS, the same sorted list at base and head). (d) Raw diff: 0 lines in every file. (e) After the head build, the full `turbo run build` (116 of 116 tasks, the dependents' DTS builds included) and `pnpm --filter @objectstack/core typecheck` ran on the head `dist/`: exit 0 each. - Premises re-checked at this head: (P2) `packages/objectql/package.json` `exports` names `./dist/index.*` and `./dist/core.*` only; (P3) `git grep -n -E 'objectql/dist|driver-sql/dist' -- scripts package.json`: 0 hits. - **Each in-place export alone, before the move** (`dts-export-variant-cut18.sh`, base `engine.ts` with only `export` added): `none`, each of the seven alone, and all seven together print the same `util-BaJGwxSm.d.ts`, `index.d.ts` `6a16c278b5ac…` and `core.d.ts` `9f22dc909731…`. **Positive control.** On this head, one `sed` dropped `private` from ` private rejectCredentialAggregation(` (a moved member's stub; anchor count 1 to 0, replacement count 0 to 1; `engine.ts` blob `c16b33857e02` to `b97293c87022`) and rebuilt (`posctl18.sh`, build exit 0). The chunk is renamed `util-CKT4sa10` and now publishes `rejectCredentialAggregation(object: string, query: EngineAggregateOptions): void;` (line 11266, where the head prints `private rejectCredentialAggregation;`); its normalised sha256 is `32d9098f1fd316a6b8ffb7e5a40a0da99017775007bf862aed09cf9ed0c83ae5` ≠ `c43cb8b1…`, its declared-surface digest `4c6ca61214efff63…` ≠ `b264bade9e7d8157…` (the `declare class ObjectQL` statement named as the one changed), and the raw `core.d.ts` (`6548856dfe13…`) and `index.d.ts` (`7bff7db9b23e…`) change with the chunk name. `index.d.ts` normalises EQUAL under the control (an entry only re-exports), which is why every declaration file is read. The restore leg put the blob back to `c16b33857e02` (`git diff HEAD` empty), and the full build that followed produced an `@objectstack/objectql` `dist/` whose six declaration files and two ESM bundles compare equal to the head build. </details> <details><summary>AST compare (move-only)</summary> `astcmp.mjs` (TypeScript compiler API, comments and whitespace ignored, tokens compared) on base `engine.ts` against head `engine.ts` plus the module: - The two moved bodies are **SAME** against base `1708115e2f` (parameters, return type, `async`, body tokens, and every body line equal after one level of indent): `count` 296 tokens / 58 lines, `rejectCredentialAggregation` 296 / 38. - `class ObjectQL`: 326 of 326 members kept, in the same order; 324 byte-identical, and the 2 stubs keep docblock, modifiers and signature identical. - Top level: 227 of 236 base statements byte-identical. The other 9 (`exportcmp.mjs`): the `@objectstack/spec/api` `StandardErrorCode` import, moved verbatim to the module; the seven helpers, exported in place with statement and leading docblock otherwise identical; and the class. The head adds one statement: the module import. </details> <details><summary>Late-binding probe (base and head <code>dist</code>)</summary> `zz-probe-cut18.mjs` (a one-off, not committed; in the project files) loads the built `ObjectQL` and drives both moved bodies with doubles set on the instance after construction. `count`: with every host member replaced (`_registry`, `relatedSchemaOf`, and the eight methods), recording each call and its arguments in order; the `filter` alias folding into `where`; a malformed filter array refused (`INVALID_FILTER` / 400) and a well-formed one lowered; a `search` expanded through the host; no query; an unknown option refused; a token resolved by `resolveWhereTokens` and the caller's `where` captured before the middleware scopes it, on a driver without `count` (the `find()` fallback, 3 rows); `where` and `filter` given together (refused); and a real engine with a registered driver and a registered object (the driver's `count` reached twice, the second through a filter array). `rejectCredentialAggregation`: group by a `secret`; `min` over a `password` with an `internal: true` object `groupBy` (both named, envelope `INVALID_FIELD` / 400 / `field` / `fields` / `object`); safe fields; an object with no protected field; `_registry` replaced twice after construction; `aggregate()` reaching the stub; and `aggregate()` reaching a guard double set on the instance. The 35 output lines are identical on base `1708115e2f` and head (the 17 construction log lines, which carry timestamps, set aside); stderr is identical (empty) too. </details> <details><summary>Suites</summary> Under `scripts/pm/os-verify-lock.sh` on head `b0142c4a2`: - Full `turbo run build` (the root `build` filters): 116 of 116 tasks, exit 0. - `typecheck` of `@objectstack/objectql` and `@objectstack/core`: exit 0 each. - `@objectstack/objectql` vitest `--project local`: 475 files, 8,849 tests passed, exit 0 (`engine-aggregate-filter.test.ts` and `engine-middleware-operation-vocabulary.test.ts`, which drive these bodies, among them). - Every `packages/*/vitest.repo-tests.json` project: core 6 files / 83 tests, objectql 1 / 5, rest 5 / 280 passed and 18 skipped, runtime 3 / 948, spec 80 / 1,295, types 1 / 11. Exit 0 each. </details> ## Verification Head `b0142c4a27f4b95b789f22f24e7b198a8a449980`. The gate list is `node scripts/pm/dispatch-gates.mjs --commands --repo objectstack-ai/objectstack` on this diff at that head (48 commands, merge base `1708115e2`), each run with its exit code captured before any pipe, then reconciled with `--ran`. - **Derived gates: 48 of 48 run, exit 0 each; `--ran`: 48 run, 0 NOT-MEASURED, 0 UNRUN.** `check:dual-build-cjs-loads` and `check:dts-closure` ran after the full build; `check-plugin-teardown-shape --self-test` after fetching `621a4876` and deepening the clone. The exit codes are in `pm-orders/server/evidence/25522/cut18-ran-final.txt`. - **Hand gates, exit 0 each:** `node scripts/check-adr-anchors.mjs` (counts above), `node scripts/check-tenant-audit-census.mjs`, `node scripts/check-adr-symbol-anchors.mjs`, `node scripts/check-platform-checklist.mjs --changed-from origin/main`, `node scripts/check-commit-card-trailers.mjs --range origin/main..HEAD`, `node scripts/check-system-context-census.mjs`, `node scripts/check-durability-degradation-log-level.mjs --list` (base and head identical, masked), and eslint as `pnpm lint` runs it (`--no-inline-config`) on `packages/objectql`. ## Acceptance notes - **Serial order.** Nothing was edited before the claim `6453372157` named this branch. The branch is `origin/main` at `1708115e2f` plus one commit. - **Overlap.** Open PRs and the one named branch touching `packages/objectql/**`, listed at start (each head fetched, `git diff --name-only origin/main...pr/N`) and again just before opening this: at start, #26871's branch `claude/project-thread-vdrn59` (now draft #26930) edits `engine.ts` in the delete section (hunks at lines 18240 and 19857, the docblock of `cascadeDeleteRelations` and `delete()`'s predicate arm), and #26831 edits `engine-execution-plumbing.ts`; the rest touch test files or `CHANGELOG.md` only. Before opening: #26930 (head `27b890d0d`, unchanged), #26920 (15 `protocol-*.test.ts` files), #26831 and the release PR #26517 (`CHANGELOG.md`, `package.json`). None touches this area, the seven helpers or the import lines, and `git merge-tree` of this head with #26930's head and with `origin/main` `8074feb64` exits 0 each. `origin/main` moved after the cut (test files under `packages/objectql/src` only, none of this diff's paths); the derived gate list is unchanged. Whichever lands second merges `main`. - **`aggregate` stays.** `packages/spec/liveness/query.json` cites `engine.ts#aggregate` three times as a producer; moving it would mean re-keying those rows, a `packages/spec` edit this claim forbids. Its guard is what moves, and the class's `aggregate` reaches it through the stub, as before. - **Receivers.** The host literal carries `_registry` and `relatedSchemaOf` as getters, so a body reads whatever the instance holds when it reads it: the probe replaces both after construction and sees the moved bodies use the replacements. No body assigns a field, so there is no setter. - **Left as is.** The `{@link collectCredentialFields}` / `{@link collectInternalReadFields}` in the staying `rejectCredentialAggregation` docblock keep resolving through `engine.ts`'s `./secret-fields.js` import, which stays. The moved `count` body keeps its five-space indentation minus one level (three spaces), as moved, not re-formatted. ## 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 (`CountAggregateHost`, `engine-count-aggregate.ts`). - [x] 3. No formatter ran; `pnpm lint` passes on the touched package. - [x] 4. One issue, one branch off an up-to-date `main`, one pull request. - [x] 5. No publish-visible change, so `skip-changeset`; both declaration lines are filled in above. - [x] 6. `packages/spec` 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, apart from the maintainer quick-read block the project asks for. - [x] The *Before Submitting* list in CONTRIBUTING.md is done. `skip-changeset`: move-only inside `@objectstack/objectql`, with every published declaration file byte-identical and no behaviour change, so there is nothing for a release note to say. 🤖 Generated with [Claude Code](https://claude.com/claude-code) https://claude.ai/code/session_01FEkQ5n1LHPdPmHoXe1BGDa Co-authored-by: Claude <noreply@anthropic.com>
1 parent 5ec398d commit beb09fb

2 files changed

Lines changed: 235 additions & 106 deletions

File tree

Lines changed: 202 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,202 @@
1+
// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
2+
3+
/**
4+
* [#25522 cut 18] Two bodies of the `ObjectQL` "count / aggregate" section:
5+
* `count` (the counting door, `find`'s sequence up to the driver's `count`)
6+
* and `rejectCredentialAggregation` (`aggregate()`'s fail-closed guard against
7+
* grouping or aggregating a field withheld on the generic read path), moved
8+
* out of `engine.ts`.
9+
*
10+
* A MOVE, not a redesign. Each exported function below is the body of the
11+
* `ObjectQL` method of the same name as it stood, re-indented one level and
12+
* otherwise unedited. Each method's docblock stays on the method in
13+
* `engine.ts`, which is still the contract.
14+
*
15+
* ## How the moved bodies reach the class
16+
*
17+
* The shape #23050 set for `rest-server.ts`, applied as
18+
* `engine-driver-options.ts` applies it. The class keeps each method with
19+
* name, modifiers, signature, docblock and position; its body becomes one
20+
* literal of exactly the {@link CountAggregateHost} members it reads, plus a
21+
* `.call`. The fields read (`_registry`, `relatedSchemaOf`) go as GETTERS, so
22+
* each read happens when the body makes it, as it did; no body assigns a
23+
* field. The methods called go as late-bound closures over the instance, so a
24+
* double or spy set on the engine is still the one a call reaches.
25+
* `aggregate` stays in the class: `packages/spec/liveness/query.json` names
26+
* `engine.ts#aggregate` as a producer.
27+
*
28+
* ## Module helpers
29+
*
30+
* `count` reads seven top-level helpers that `find`, `findOne`, `update`,
31+
* `delete` or `aggregate` read too: `foldEngineOptionAliases`,
32+
* `ENGINE_WHERE_SLOTS`, `rejectUnknownEngineOptions`,
33+
* `ENGINE_COUNT_OPTION_KEYS`, `lowerWhereFilterArray`, `mergeReadContext` and
34+
* `declaredDatetimeLowering`. One home each: they stay declared in
35+
* `engine.ts`, exported in place, and are imported here, never copied.
36+
* Nothing re-exports them from the package entry, so the published surface
37+
* does not move.
38+
*
39+
* That import is a run-time module cycle (`engine.ts` imports this module,
40+
* this module imports `engine.ts`). Nothing here runs at module evaluation,
41+
* only declarations, and the helpers are read only when a body runs, after
42+
* both modules have evaluated. `engine.ts` imports this module right after
43+
* `./secret-fields.js`, the one other module this one imports, so the
44+
* evaluation order of every module is what it was; the
45+
* `@objectstack/spec/api` import that left `engine.ts` with
46+
* `rejectCredentialAggregation` was not that package's first load. The
47+
* published build bundles both files into one, where the cycle does not
48+
* exist.
49+
*/
50+
51+
import type { EngineAggregateOptions, EngineCountOptions, QueryAST } from '@objectstack/spec/data';
52+
// [#22646] The ADR-0112 catalog — `rejectCredentialAggregation`'s refusal
53+
// envelope carries `INVALID_FIELD` from it rather than a bare `Error`.
54+
import { StandardErrorCode } from '@objectstack/spec/api';
55+
import {
56+
collectInternalReadFields,
57+
collectCredentialFields,
58+
} from './secret-fields.js';
59+
import {
60+
ENGINE_COUNT_OPTION_KEYS,
61+
ENGINE_WHERE_SLOTS,
62+
declaredDatetimeLowering,
63+
foldEngineOptionAliases,
64+
lowerWhereFilterArray,
65+
mergeReadContext,
66+
rejectUnknownEngineOptions,
67+
} from './engine.js';
68+
import type { EngineReadOptions, ObjectQL, OperationContext } from './engine.js';
69+
70+
/**
71+
* The `ObjectQL` members the functions below read — every one of them, and no
72+
* other; each function's `this` is the `Pick` of the members IT reads. Each is
73+
* typed AS the class member (an indexed access on the class), so a change to a
74+
* member's type on the class is a change here too, and naming a member the
75+
* class does not have is a compile error. See the module header for which
76+
* members are getters.
77+
*/
78+
export interface CountAggregateHost {
79+
readonly _registry: ObjectQL['_registry'];
80+
readonly relatedSchemaOf: ObjectQL['relatedSchemaOf'];
81+
resolveObjectName: ObjectQL['resolveObjectName'];
82+
expandSearchOnOptions: ObjectQL['expandSearchOnOptions'];
83+
getDriver: ObjectQL['getDriver'];
84+
resolveWhereTokens: ObjectQL['resolveWhereTokens'];
85+
executeWithMiddleware: ObjectQL['executeWithMiddleware'];
86+
buildDriverOptions: ObjectQL['buildDriverOptions'];
87+
composeReadableIdsAtBoundary: ObjectQL['composeReadableIdsAtBoundary'];
88+
find: ObjectQL['find'];
89+
}
90+
91+
/** The body of `ObjectQL#count`; the method's docblock there is its contract. */
92+
export async function count(
93+
this: Pick<CountAggregateHost, 'resolveObjectName' | '_registry' | 'relatedSchemaOf' | 'expandSearchOnOptions' | 'getDriver' | 'resolveWhereTokens' | 'executeWithMiddleware' | 'buildDriverOptions' | 'composeReadableIdsAtBoundary' | 'find'>,
94+
object: string,
95+
query?: EngineCountOptions,
96+
options?: EngineReadOptions,
97+
): Promise<number> {
98+
object = this.resolveObjectName(object);
99+
// Fold the `filter` alias into `where` (#4346) — the AST below reads
100+
// `query.where` only, so an unfolded `{ filter }` counted the whole table.
101+
query = foldEngineOptionAliases(object, 'count', query, ENGINE_WHERE_SLOTS);
102+
rejectUnknownEngineOptions(object, 'count', query, ENGINE_COUNT_OPTION_KEYS);
103+
query = lowerWhereFilterArray(object, 'count', query, this._registry.getObject(object), this.relatedSchemaOf);
104+
// [#22790] ADR-0061 `search` → the rows are searched before they are
105+
// counted, by the one expander `find` runs, at the same point in the
106+
// sequence — so the count is the number of rows `find` returns for the bag.
107+
if (query) query = await this.expandSearchOnOptions(object, query, options);
108+
const driver = this.getDriver(object);
109+
110+
// The AST must ride on the opCtx so the security/sharing middlewares can
111+
// inject their read filters (RLS, OWD/sharing scope) into `ast.where` —
112+
// exactly like find(). Building it locally inside the executor (#2737)
113+
// discarded every injected filter: `total` counted the RAW table while
114+
// `records` were scoped, leaking invisible-row counts and breaking
115+
// pagination.
116+
const opCtx: OperationContext = {
117+
object,
118+
operation: 'count',
119+
ast: { object, where: query?.where },
120+
options: query,
121+
context: mergeReadContext(query?.context, options?.context),
122+
};
123+
// [ADR-0053 D-D1, amended — #5930] Resolve, then lower. [#20802] …with
124+
// the nested-relation step between the two.
125+
await this.resolveWhereTokens(
126+
opCtx.ast as QueryAST,
127+
opCtx.context,
128+
declaredDatetimeLowering(this._registry.getObject(object)),
129+
'where',
130+
'count',
131+
);
132+
// The caller's own `where`, placeholders expanded — captured BEFORE the
133+
// middleware chain scopes `opCtx.ast.where`, so the find() fallback below
134+
// still passes the unscoped filter (find() applies the read filters itself).
135+
const callerWhere = (opCtx.ast as QueryAST).where;
136+
137+
await this.executeWithMiddleware(opCtx, async () => {
138+
const countOpts = this.buildDriverOptions(object, opCtx.context, undefined, { recordedVerdict: opCtx.tenantLayer0Verdict });
139+
if (driver.count) {
140+
// [ADR-0139 D2] The executor boundary, as in `find` (`count` has no
141+
// before-hook): the AST is final once the chain has run. The
142+
// `find()` fallback below composes at its own boundary.
143+
return driver.count(
144+
object,
145+
await this.composeReadableIdsAtBoundary(object, 'count', opCtx.ast as QueryAST, opCtx.context),
146+
countOpts,
147+
);
148+
}
149+
// Fallback to find().length — find() applies the read filters itself,
150+
// so pass the caller's original where, not the already-scoped ast.
151+
const res = await this.find(object, { where: callerWhere, fields: ['id'], context: opCtx.context });
152+
return res.length;
153+
});
154+
155+
return opCtx.result as number;
156+
}
157+
158+
/** The body of `ObjectQL#rejectCredentialAggregation`; the method's docblock there is its contract. */
159+
export function rejectCredentialAggregation(
160+
this: Pick<CountAggregateHost, '_registry'>,
161+
object: string,
162+
query: EngineAggregateOptions,
163+
): void {
164+
const schema = this._registry.getObject(object);
165+
// Deduped: one field can be reachable through both collectors (a `secret`
166+
// column that is also flagged `internal`), and it must be named once.
167+
const protectedFields = [
168+
...new Set([...collectCredentialFields(schema), ...collectInternalReadFields(schema)]),
169+
];
170+
if (protectedFields.length === 0) return;
171+
172+
const referenced = new Set<string>();
173+
for (const agg of query?.aggregations ?? []) {
174+
const field = (agg as { field?: string })?.field;
175+
if (field && field !== '*') referenced.add(field);
176+
}
177+
for (const g of query?.groupBy ?? []) {
178+
const field = typeof g === 'string' ? g : g?.field;
179+
if (field) referenced.add(field);
180+
}
181+
182+
const hit = protectedFields.filter((f) => referenced.has(f));
183+
if (hit.length > 0) {
184+
const err = new Error(
185+
`Cannot aggregate credential field(s) ${hit.map((f) => `"${object}.${f}"`).join(', ')}: `
186+
+ 'secret/password fields are masked on read and `internal: true` fields are omitted '
187+
+ 'outright, so the value never leaves the engine on the generic data path; aggregating '
188+
+ 'them (group-by, min/max, array_agg, …) would surface it. '
189+
+ 'Refusing (fail-closed) — see ADR-0100.',
190+
) as Error & { code?: string; status?: number; httpStatus?: number; field?: string; fields?: string[]; object?: string };
191+
// ADR-0112 envelope, the same number under D5's two spellings — `status`
192+
// for the HTTP doors, `httpStatus` for a consumer holding the thrown
193+
// error (the CLI `--json` envelope).
194+
err.code = StandardErrorCode.enum.INVALID_FIELD;
195+
err.status = 400;
196+
err.httpStatus = 400;
197+
err.field = hit[0];
198+
err.fields = hit;
199+
err.object = object;
200+
throw err;
201+
}
202+
}

0 commit comments

Comments
 (0)