From df94c71c457300c5f492065b6d92970abd1f16e5 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 5 Nov 2026 05:22:43 +0000 Subject: [PATCH 1/7] refactor(objectql): move 36 hand-series private docblocks out of engine.ts with the codemod's third pass (slice 1 of 2) scripts/codemod-split-engine.mjs gains --hand-docs: a private hand-series stub's docblock moves onto the function in its hand module (replacing the placeholder, one level out), the stub's marker says so, the module header gains one note; public and protected docblocks stay on the class. The proof undoes every move and rebuilds the base engine.ts and hand modules byte for byte. This slice runs it on 8 hand modules (929 docblock lines). Co-Authored-By: Claude Claude-Session: https://claude.ai/code/session_01Utp6CYHBS7P5hkUQrNHkHZ --- .../30053-hand-series-private-docblocks.md | 12 + .../objectql/src/engine-datasource-routing.ts | 33 +- .../objectql/src/engine-delete-cascade.ts | 179 ++- .../objectql/src/engine-execution-plumbing.ts | 65 +- .../src/engine-master-detail-related.ts | 193 +++- .../objectql/src/engine-migration-flags.ts | 40 +- .../src/engine-package-registration.ts | 23 +- .../engine-readable-ids-permission-formula.ts | 183 ++- .../src/engine-secret-internal-fields.ts | 289 ++++- packages/objectql/src/engine.ts | 1001 +---------------- scripts/codemod-split-engine.mjs | 466 ++++++++ 11 files changed, 1483 insertions(+), 1001 deletions(-) create mode 100644 .changeset/30053-hand-series-private-docblocks.md diff --git a/.changeset/30053-hand-series-private-docblocks.md b/.changeset/30053-hand-series-private-docblocks.md new file mode 100644 index 00000000000..a2c1be480bd --- /dev/null +++ b/.changeset/30053-hand-series-private-docblocks.md @@ -0,0 +1,12 @@ +--- +'@objectstack/objectql': patch +--- + +refactor(objectql): the private docblocks of 36 hand-series stubs in `engine.ts` move into their hand modules, onto the functions that hold their bodies; no behaviour or public declaration changes + +Clause-②: no +Metadata-protocol: none + +- A new third pass of the `engine.ts` codemod, `--hand-docs`, moves a private hand-series stub's docblock onto the function its stub calls, as the first two passes already do for the bodies they move. A public or protected method keeps its docblock on the class. +- This is the first of two slices: 8 hand modules here, the other 9 next. `engine.ts` goes from 13,544 to 12,615 lines. +- Nothing to do on upgrade: the exported names, the public `.d.ts` surface and what every method does are the same. diff --git a/packages/objectql/src/engine-datasource-routing.ts b/packages/objectql/src/engine-datasource-routing.ts index f6252c183b1..ab98564fc58 100644 --- a/packages/objectql/src/engine-datasource-routing.ts +++ b/packages/objectql/src/engine-datasource-routing.ts @@ -45,6 +45,11 @@ * This module imports the engine only as a TYPE, so there is no module cycle * at run time, and nothing re-exports it, so the published surface does not * move. + * + * [#30053 codemod] A PRIVATE method's docblock has since moved here, + * onto its function (`--hand-docs` of `scripts/codemod-split-engine.mjs`), + * and the class keeps none for it; a public or protected method's docblock + * stays on the class, the contract, and its function keeps the placeholder. */ import type { IDataDriver } from '@objectstack/core'; @@ -68,7 +73,21 @@ export interface DatasourceRoutingHost { matchPattern: ObjectQL['matchPattern']; } -/** The body of `ObjectQL#getDriver`; the method's docblock there is its contract. */ +/** + * Helper to get the target driver + * + * Resolution priority (first match wins): + * 1. Object's explicit `datasource` field (if not 'default') + * 2. DatasourceMapping rules (namespace/package/pattern matching) + * 3. Lifecycle-class separation (ADR-0057 §3.6): telemetry/event/audit + * objects route to the dedicated 'telemetry' datasource when registered + * 4. Package's `defaultDatasource` from manifest + * 5. Global default driver + * + * The order itself lives in {@link resolveDatasourceBinding} — this method + * turns the name it decides on into a driver, and diagnoses the cases where + * that driver is missing. + */ export function getDriver( this: Pick, objectName: string, @@ -131,7 +150,12 @@ export function getDriver( throw new Error(`[ObjectQL] No driver available for object '${objectName}'`); } -/** The body of `ObjectQL#resolveDatasourceFromMapping`; the method's docblock there is its contract. */ +/** + * Resolve datasource from mapping rules + * + * Rules are evaluated in order (or by priority if specified). + * First matching rule wins. + */ export function resolveDatasourceFromMapping( this: Pick, objectName: string, @@ -173,7 +197,10 @@ export function resolveDatasourceFromMapping( return null; } -/** The body of `ObjectQL#matchPattern`; the method's docblock there is its contract. */ +/** + * Simple glob pattern matching + * Supports * (any chars) and ? (single char) + */ export function matchPattern(objectName: string, pattern: string): boolean { const regexPattern = pattern .replace(/[.+^${}()|[\]\\]/g, '\\$&') // Escape regex special chars diff --git a/packages/objectql/src/engine-delete-cascade.ts b/packages/objectql/src/engine-delete-cascade.ts index 45c2de0d292..9c963e57986 100644 --- a/packages/objectql/src/engine-delete-cascade.ts +++ b/packages/objectql/src/engine-delete-cascade.ts @@ -69,6 +69,11 @@ * other module this one imports, so the evaluation order of every module is * what it was. The published build bundles both files into one, where the * cycle does not exist. + * + * [#30053 codemod] A PRIVATE method's docblock has since moved here, + * onto its function (`--hand-docs` of `scripts/codemod-split-engine.mjs`), + * and the class keeps none for it; a public or protected method's docblock + * stays on the class, the contract, and its function keeps the placeholder. */ import type { ServiceObject } from '@objectstack/spec/data'; @@ -120,7 +125,65 @@ function cascadeReachOf(set: CascadeDeleteSet, object: string, id: unknown): Map return reach; } -/** The body of `ObjectQL#referenceProbeFilter`; the method's docblock there is its contract. */ +/** + * [#9362] The dependents probe's `where`, spelled for the KIND of reference + * field it is aimed at. + * + * A single-valued `lookup` / `master_detail` stores a scalar foreign key, and + * bare equality is the right question about it. A MULTI-VALUED reference + * field stores an ARRAY, and every SQL backend in this repo puts that array + * in a JSON TEXT column. [#18408] Which of the two a field is, is + * {@link declaredMultiValued} — the one predicate the storage side asks + * (#17469) — ⛔ never a raw `field.multiple`: `master_detail` and `tree` are + * outside `MULTI_CAPABLE_TYPES`, so the flag on one of those buys a JSON + * column from nobody, and aiming `$contains` at the scalar column the driver + * really built is the mirror image of the defect below. + * + * Aiming bare equality at a multi-valued column compares + * the whole serialization (`["a","b"]`) against one id, which can never hold; + * `driver-sql` refuses the spelling outright (`INVALID_FILTER` / 400, #7398), + * and that refusal is correct and stays. Until this method existed the probe + * built bare equality for both kinds, so every object pointed at by any + * registered `multiple: true` lookup had its delete refused with a 400 — on + * the stock showcase, `showcase_account`, with an EMPTY dependent table: + * the fault is schema-driven, not data-driven, because the probe runs per + * DECLARED relation. + * + * The multi-value spelling is `$contains`, which is what the refusal itself + * prescribes and the one membership spelling every driver here answers. On a + * column the driver holds declared multi-valued it asks MEMBERSHIP, as + * `FILTER_OPERATORS`' `$contains` docblock (`@objectstack/spec`) declares for + * a JSON-stored column: `driver-sql` (and `driver-sqlite-wasm` and the local + * `driver-turso`, which extend it) compiles a per-dialect membership + * construct, `driver-memory` a mingo `$elemMatch`, and `driver-mongodb` the + * same `$elemMatch`. No public filter surface is widened: `$contains` is + * already declared, and this is the only construction site that changes. + * + * Membership answers EXACTLY on a well-formed slot, but a backend that does + * not hold the declaration (a table it was never told about) answers the + * SUBSTRING reading instead, which is a SUPERSET: with ids `acc_1` and + * `acc_10`, a row holding `acc_10` also matches a probe for `acc_1`. That is + * why the caller still narrows the rows exactly through + * {@link ObjectQL.storedReferenceIncludes} — over-matching here would make + * `cascade` DELETE and `set_null` clear rows that never referenced this + * record, which is worse than the 400 being fixed. The pushdown's only job is + * to keep the probe from reading the whole table. + * + * ⚠️ Membership is array-only on every backend that answers it, so a slot + * holding an off-shape bare scalar (the write door never stores one, it wraps + * a scalar; out-of-band data can) is not matched by the pushdown, and the + * scalar arm of {@link ObjectQL.storedReferenceIncludes} never sees that row. + * + * The `$or` limb covers the other direction — a FALSE NEGATIVE, which on an + * integrity guard is the fail-OPEN that #8895 ruled out. An id needing JSON + * escaping (a quote, a backslash) appears in the serialized text in its + * ESCAPED form, so a backend answering the substring reading over that text + * would miss the row for the raw id; a membership test, and a document or + * in-memory backend, compares the element itself and needs the RAW form. Both + * are asked whenever they differ, and the exact narrowing discards whatever + * the extra limb over-matched. Identical for an ordinary id, which is every + * id this engine generates. + */ export function referenceProbeFilter( fieldName: string, fdef: { type?: unknown; multiple?: unknown }, @@ -141,7 +204,48 @@ export function referenceProbeFilter( }; } -/** The body of `ObjectQL#recordReferenceCheckElevation`; the method's docblock there is its contract. */ +/** + * [#12166, ruling constraint 3] File the reference-check elevation, BOTH + * halves: `triggeredBy` = the operator who asked for the delete, + * `executedAs: 'system'` = the identity the check actually ran under. + * + * ## Why a record exists at all + * + * The elevation is the whole of what this card changed, and an elevation + * nobody can see afterwards is the thing a security reviewer has no way to + * audit. The ruling names the Salesforce/Dataverse ledger shape for exactly + * this reason: a platform-executed integrity action is attributable to the + * human who triggered it, not laundered into "system did something". + * + * ## Why the LOGGER and not a `sys_audit_log` row + * + * Stated plainly, because a narrow audit edge has to be visible to be honest + * (`read-audit.ts` takes the same posture about its own narrowing): + * + * The elevated operation is a READ, and `plugin-audit`'s read writer declares + * — and pins — that a system-elevated read produces NO row, deliberately: + * those are the platform reading for its own bookkeeping, and recording them + * would bury the human views that ledger exists to make findable. Minting a + * row here would either contradict that declared boundary or require the + * engine to reach into a plugin it does not depend on. The engine's logger is + * the channel it owns, and it is where the reporting deployment read the + * defect in the first place — the issue's evidence is a server log line. + * + * So: this is a LOG record, at `info`, and the honest limit of it is that a + * deployment which does not retain engine logs retains no trace. A durable + * `sys_audit_log` row for the elevation is a follow-up that belongs to + * plugin-audit, which owns that row shape. + * + * `debug` was not an option: a record that is off by default in production is + * not an audit record. + * + * ⛔ The record names the referenced OBJECT and the relation FIELD — declared + * metadata — and never a row id, a field value, or a count. Constraint 2 + * governs what the CALLER is told; this is the server-side ledger and could + * defensibly say more, but saying more here is how a count reaches a support + * transcript, and the two surfaces are kept on one rule so neither has to be + * re-audited alone. + */ export function recordReferenceCheckElevation( this: Pick, object: string, @@ -247,7 +351,35 @@ export function collectCascadeDeleteObjects( return named; } -/** The body of `ObjectQL#collectCascadeDeleteSet`; the method's docblock there is its contract. */ +/** + * [#22305] Phase 1 of {@link cascadeDeleteRelations}: every record a by-id + * delete of `object`/`id` removes, the root included, collected READ-ONLY. + * + * Breadth-first across every relation whose resolved behaviour is + * `cascade`, read through the walk's own two readings — which relations + * point at an object ({@link cascadeRelationBehavior}) and which rows + * reference a record ({@link probeReferencingRows}: system identity, the + * same filter, the same missing-table and multi-value handling) — so the + * two phases cannot disagree about which rows a cascade reaches. A record + * already in the set is not expanded again, so a cycle in the data (a + * self-reference, two objects cascading into each other) terminates. + * + * The cost is one extra probe per CASCADING relation per member, since the + * walk probes those relations again as it deletes. `restrict` and + * `set_null` relations are not read here. Every probe here is an elevated + * read, so it files its #12166 record before it runs, through the + * once-per-record ledger the walk shares ({@link fileReferenceCheckElevation}). + * + * [#24964] …except for {@link inspectCascadeDelete}, which passes + * `fileElevations: false`: that record says "read as system FOR THE DELETE + * of" the record, and an inspection deletes nothing, so filing it there would + * put a delete that never happened in the log. The reads are the same. + * + * [#27004] From a root SET, level by level: the records found at one depth + * are probed together, per object and per relation, through + * {@link probeReferencingRowsOfSet}. The set and its edges are the ones the + * record-by-record walk collected. + */ export async function collectCascadeDeleteSet( this: Pick, object: string, @@ -312,7 +444,38 @@ export async function collectCascadeDeleteSet( return set; } -/** The body of `ObjectQL#refuseRestrictsInsideOwnCascade`; the method's docblock there is its contract. */ +/** + * [#26718] Before the cascade writes anything: refuse the delete when an + * AUTHORED `restrict` is held by a row the cascade would remove only because + * the record that restrict protects is removed. + * + * The deleting walk judges a `restrict` against rows OUTSIDE the cascade set + * alone (#22305), and a row inside it was exempt whatever put it there. That + * exemption is right for a row the cascade reaches through ANOTHER record (a + * contract and the contact it restricts, both details of the account being + * deleted: the account's delete removes both, and neither is left dangling). + * It was wrong for a row reached THROUGH the very record it protects: a child + * holding a `master_detail` and a `restrict` lookup to the same parent is in + * that parent's set by its own master_detail, so the restrict was never + * asked and the cascade deleted the child — the record the author declared + * undeletable. A grandchild under that parent holding the restrict is the + * same case one level down. + * + * So for every member, every relation whose RESOLVED behaviour is `restrict` + * ({@link cascadeRelationBehavior}, an authored `restrict` on a lookup, + * tree or master_detail) is probed here when its object has a member in the + * reach of that member ({@link cascadeReachOf}); a referencing row in the + * reach refuses with the walk's own `DELETE_RESTRICTED` + * ({@link deleteRestrictedError}). Every restrict is judged before the + * first cascade, so the outcome no longer depends on the order the fields or + * objects are declared in, and nothing has been written when it refuses. + * + * ⛔ The required-lookup escalation (`set_null` on a required field read as + * `restrict`) is not judged here: it restricts because a required reference + * cannot be CLEARED, and a row the cascade deletes is never cleared. It keeps + * the #22305 rule. Neither does this replace the walk's judgement of rows + * outside the set, which is unchanged. + */ export async function refuseRestrictsInsideOwnCascade( this: Pick, cascadeSet: CascadeDeleteSet, @@ -396,7 +559,13 @@ export async function refuseRestrictsInsideOwnCascade( } } -/** The body of `ObjectQL#deleteRestrictedError`; the method's docblock there is its contract. */ +/** + * The `DELETE_RESTRICTED` refusal of deleting `object`/`id` because + * `dependents` of `childName` reference it via `fieldName`. One builder for + * the deleting walk ({@link walkReferencingRelations}) and the judgement + * before it ({@link refuseRestrictsInsideOwnCascade}), so the two answer one + * envelope. + */ export async function deleteRestrictedError( this: Pick, args: { diff --git a/packages/objectql/src/engine-execution-plumbing.ts b/packages/objectql/src/engine-execution-plumbing.ts index 4487bae3b7d..22307c86aab 100644 --- a/packages/objectql/src/engine-execution-plumbing.ts +++ b/packages/objectql/src/engine-execution-plumbing.ts @@ -32,12 +32,29 @@ * This module imports nothing from the engine, so there is no module cycle at * run time, and nothing re-exports it, so the published surface does not * move. + * + * [#30053 codemod] A PRIVATE method's docblock has since moved here, + * onto its function (`--hand-docs` of `scripts/codemod-split-engine.mjs`), + * and the class keeps none for it; a public or protected method's docblock + * stays on the class, the contract, and its function keeps the placeholder. */ import type { HookContext } from '@objectstack/spec/data'; import type { ExecutionContext } from '@objectstack/spec/kernel'; -/** The body of `ObjectQL#buildSession`; the method's docblock there is its contract. */ +/** + * Build a HookContext.session from ExecutionContext — WHO is calling. + * + * Returns `undefined` when the context yields nothing session-worthy, which + * keeps `!ctx.session` meaning exactly one thing for hooks: "no identity + * envelope was supplied" — the bare-kernel / programmatic call that + * caller-gating hooks skip for. A context carrying only write PROVENANCE + * (`{ flowRunId }`, all an identity-less flow run has — #3712) is such a + * case: it says what produced the write, not who is calling, and surfaces + * through {@link buildProvenance} instead. Every real transport resolves + * `positions` into the context, so an anonymous HTTP request still yields a + * session and stays gated. + */ export function buildSession(execCtx?: ExecutionContext): HookContext['session'] { if (!execCtx) return undefined; const session = { @@ -90,7 +107,32 @@ export function buildSession(execCtx?: ExecutionContext): HookContext['session'] return session; } -/** The body of `ObjectQL#buildProvenance`; the method's docblock there is its contract. */ +/** + * Build the HookContext.provenance envelope — WHERE this write came from. + * + * Deliberately separate from {@link buildSession}: provenance is server- + * stamped, evaluated by no security middleware, and can exist with no + * identity beside it. A schedule-triggered flow run resolves no principal + * yet still owns its writes, and that is the case the approvals record lock + * needs to recognize (#3456 / #3712). + * + * `attributedUserId` rides the SAME envelope for the same reason (#4586): + * a better-auth-originated write authorizes as the system, so the human who + * triggered it must reach the audit writer WITHOUT appearing in `session` — + * where every caller-gating hook would read them as the caller. Attribution + * here, authorization in `session`/`isSystem`, never the two mixed. + * + * `performedByClientId` rides it for the THIRD time on the same reasoning + * (#17022, ADR-0090 D10 rule 4): an MCP OAuth agent's write authorizes as + * the human it acts for, so the acting client can only reach the audit + * writer through a channel that is not `session` — putting it there would + * make every caller-gating hook read the client as the caller, which is the + * opposite of what the delegation means. This copy list is the whole reason + * the field is not inert: `HookContext` is built from a CLOSED literal whose + * `provenance` value is exactly what this function returns, so a key + * declared on `hook.zod.ts` and missing here is declared and never + * populated (ADR-0049). + */ export function buildProvenance(execCtx?: ExecutionContext): HookContext['provenance'] { const flowRunId = (execCtx as any)?.flowRunId; const attributedUserId = (execCtx as any)?.attributedUserId; @@ -103,7 +145,12 @@ export function buildProvenance(execCtx?: ExecutionContext): HookContext['proven }; } -/** The body of `ObjectQL#buildEvalUser`; the method's docblock there is its contract. */ +/** + * Build the acting-user object (ADR-0068 EvalUser shape) surfaced to + * validation-time predicates as `current_user` — notably per-option + * `visibleWhen` authorization gating (objectui#2284). Returns undefined for + * system / unauthenticated writes, where membership predicates then fail-open. + */ export function buildEvalUser( execCtx?: ExecutionContext, ): { id: string; positions: string[]; organizationId: string | null } | undefined { @@ -115,7 +162,17 @@ export function buildEvalUser( }; } -/** The body of `ObjectQL#buildUser`; the method's docblock there is its contract. */ +/** + * Build the `HookContext.user` shortcut — the ergonomic "current user" + * object surfaced to JS hooks. Carries `organizationId` (the blessed name + * for the caller's active org, identical to `session.organizationId` and + * `current_user.organizationId`) so a hook author who needs "the current org + * to filter by" writes `ctx.user.organizationId` with zero relearning (#3280). + * + * Returns undefined for system / unauthenticated writes (no acting user) — + * hooks that need an org regardless of a resolved user read + * `ctx.session.organizationId`, which is populated whenever a session is. + */ export function buildUser(execCtx?: ExecutionContext): HookContext['user'] { if (!execCtx || execCtx.userId == null) return undefined; return { diff --git a/packages/objectql/src/engine-master-detail-related.ts b/packages/objectql/src/engine-master-detail-related.ts index fe11231af0a..21490a3e228 100644 --- a/packages/objectql/src/engine-master-detail-related.ts +++ b/packages/objectql/src/engine-master-detail-related.ts @@ -49,6 +49,11 @@ * this one imports, so the engine's evaluation order is what it was. The * published build bundles both files into one, where the cycle does not * exist. + * + * [#30053 codemod] A PRIVATE method's docblock has since moved here, + * onto its function (`--hand-docs` of `scripts/codemod-split-engine.mjs`), + * and the class keeps none for it; a public or protected method's docblock + * stays on the class, the contract, and its function keeps the placeholder. */ import { referenceTargetOf } from '@objectstack/spec/data'; @@ -133,7 +138,64 @@ function staticReadonlyStripTakes( return !(key in staticReadonlyStoredView(schema, data, supplied, strip)); } -/** The body of `ObjectQL#settleMasterDetailLanding`; the method's docblock there is its contract. */ +/** + * [#19853] Settle whether an UPDATE lands the master-detail FK its payload + * names, BEFORE any `parent`-scoped lock is judged — so `parent` is the + * header the write STORES, not merely the one it NAMES. Shared by the by-id + * and bulk paths, which differ only in how a header is resolved and how the + * FK's own lock is judged (one row, or "locked in ≥1 matched row"). + * + * #4889's rule — a repoint is judged against the master it lands on — is + * right only when the repoint LANDS. Two strips can take the FK back out + * after `parent` was resolved from it, and the row then keeps the header it + * had while every other parent-scoped lock was judged against one it never + * reached. Measured on the card's shape: `update(l1, { amount: 999, invoice: + * 'inv_b' })` with `invoice` statically `readonly` and `l1` under a PAID + * `inv_a` committed `{ amount: 999, invoice: 'inv_a' }` — the frozen line + * rewritten, still under the paid invoice. The strips, in the order they run + * after this point: + * + * ① the FK's OWN `readonlyWhen` lock. Judged here first, against the + * header the FK names — #4889's rule for the FK itself, unmoved. + * [#19911] Judged WITH the other caller-supplied locks, not alone, and + * settled with them (`settleReadonlyWhenDrops`), so a value one of them + * drops can no longer unlock it — but only the FK is TAKEN here (`only`). + * That moves the FK's verdict in BOTH directions: a repoint its lock + * used to let through can now stay home, and a repoint it used to hold + * can now LAND, when the value its `record` lock reads is itself locked + * under the header the FK names — that value is dropped, the FK's lock + * reads the stored one, and the rest are then judged under the header + * the row lands on. When that verdict keeps the FK off the landing it is final: + * the returned `supplied` no longer holds the FK, so the strip that + * judges the other fields never re-asks it against the header the row + * keeps, where it could flip and land the FK after the rest were judged + * against the header it left. When the FK lands, that strip re-judges it + * on the same landing and reaches the same verdict. + * ② the static `readonly` strip (`staticReadonlyStripTakes`), which runs + * after the conditional one and takes a non-system caller's forged FK. + * + * Nothing else on either path moves the FK after this point, and what moves + * it before this point (a `beforeUpdate` hook, the post-hook payload) is + * already in `data`. Field-level security refuses a forbidden FK outright + * rather than stripping it, so it never leaves a payload that names one + * header and stores another. + * + * Returns: + * - `subject` — the payload the `readonlyWhen` strip judges next: `data`, + * or `data` without the FK when ①'s verdict locked it; + * - `supplied` — the entry snapshot that strip judges against: without the + * FK when the FK does not land (its verdict is in), whole when it does; + * - `view` — the payload the header id is read from: `data` when the FK + * lands, else `data` without it, so `masterIdOf` falls through to the + * prior row's FK; + * - `header` — what `resolve(view)` answers. ① needs the NAMED header only + * when judging the FK's lock reads `parent` — its own predicate, or + * [#19911] a `record`-reading one whose view another payload key's + * `parent`-scoped lock shapes (`readonlyWhenFkJudgementReadsParent`); + * when the FK then lands that is the same header and is reused, so the + * write still reads one header — two only when ① read the named header + * and the FK does not land. + */ export async function settleMasterDetailLanding( args: { schema: unknown; @@ -173,7 +235,46 @@ export async function settleMasterDetailLanding( return { subject, supplied: rest, view, header: await resolve(view) }; } -/** The body of `ObjectQL#resolvePreviewParents`; the method's docblock there is its contract. */ +/** + * [#22474] The master-detail headers a `validate()` preview binds, per row: + * `parents[i]` is the header row `i` is judged under (`parent`), and + * `previousParents[i]` the header its stored row hangs off when the row + * REPOINTS it (`previousParent`, for the ADR-0113 pre-check), else + * `undefined`, which the evaluator reads as "the same header", as the + * write's own `undefined` is read. + * + * The header ids are read the way the write reads them ({@link masterIdOf}): + * `views[i]` is the judged view, the row on an insert and the stored row + * merged with the patch on an update, so the patch's FK wins over the stored + * one; `storedRows[i]` names the header a repoint leaves. + * + * ## Read under the caller's access, not elevated + * + * A header read elevated here would answer a predicate over a row the caller + * may not read. Two reads, each answering one question, as the preview's + * stored-row read does — and, since #22519, as the write's own header read + * does ({@link resolveMasterDetailParent}): + * + * 1. ⛔ MAY THIS CALLER SEE THE HEADER — the READ DOOR, under the caller's + * own context, one `find` for the whole batch. A header it does not + * return binds as a missing header binds: `null`, so a predicate that + * reads `parent` refuses as unevaluable, whatever the header holds. A + * failed read binds every header as missing, as the write's own failed + * header read does. + * 2. WHAT THE WRITE WOULD JUDGE — the write's own header read + * ({@link resolveMasterDetailParents}, whose own read door returns the + * same headers), for the headers read 1 returned only. + * + * ⛔ The column rule is the preview's own; the write judges a header the + * caller can read on its stored values. A header column is kept only where + * the read door served this caller + * that very value ({@link servedAsStored}); every other column, hidden or + * served transformed (a partial mask keeps the key and replaces the value), + * is judged as empty: the header is materialised over the master's declared + * fields after the columns are kept, so such a column reads `null`. So the + * verdict never depends on a header value the caller could not have read in + * full. + */ export async function resolvePreviewParents( this: Pick, schema: unknown, @@ -237,7 +338,62 @@ export async function resolvePreviewParents( }; } -/** The body of `ObjectQL#resolvePredicateRelated`; the method's docblock there is its contract. */ +/** + * [#18682] Resolve the related rows this object's PREDICATE rules read one + * hop through a reference field, for a whole batch of rows at once. + * + * Returns a per-row lookup the validation seams hand to + * `evaluateValidationRules` as `related`. When no rule traverses anything the + * schema says so up front and this costs NOTHING — no query, no closure work + * — which is what bounds the N+1: one hop, only the fields a rule names, one + * batched read per reference field per write (two for a caller that is not + * SYSTEM where a read door bounds the rows, below), and only when a rule asks. + * + * ## Which related rows a caller's rule may read, and how their values are read + * + * The VALUES are read as SYSTEM: a validation rule's output is a pass/fail the + * SYSTEM enforces, not data handed to the caller — categorically unlike an + * access-control rule, which is why RLS predicates are excluded from this + * capability altogether. Reading the columns as the acting user instead made + * the rule unauthorable for exactly the persona it exists to constrain. What + * bounds that elevation is the PROJECTION: only the columns the predicate + * names, intersected with the related object's declared fields. ⛔ Never the + * whole row. + * + * Which ROWS a caller that is not SYSTEM may have read is a different + * question, answered before any value is read: + * + * - ⛔ Through a `master_detail` FK the related row is the detail's HEADER, + * and ruling A on #22519 holds here as on the write's own header read + * ({@link resolveMasterDetailParent}): an unreadable header is a header + * that exists nowhere (#22892). The caller's read door first, under the + * caller's own context ({@link masterDetailHeadersCallerReads}); a header + * it does not return is `unresolved`, exactly as a header that exists + * nowhere is, so the rule's answer never depends on what an unreadable + * header holds. The elevated, tenant-kept read follows for the headers the + * door returned only, and a header the caller CAN read is judged on its + * stored values, as the write judges it. + * - On a related object no organization wall scopes, the own read of the + * caller bounds the rows, and any other id is `unreadable`, stored or not. + * - ⛔ A plain `lookup` to a walled object keeps the tenant-kept elevated read + * alone: no ruling extends the header rule to it. + * + * A SYSTEM caller (seed replay, boot) asks no read door: the elevated read + * alone, as before. + * + * ## An unresolved row is left UNAVAILABLE, and it says which kind + * + * No reference stored, the related record not found, the related object + * declares no such column, or the row could not be read: each is its own reason, and + * {@link checkPredicate} turns it into a refusal naming the related object and + * column. The write is REJECTED rather than judged on a rule that produced no + * verdict. ⛔ Never silently true, and never silently false. + * + * The projection always names `id` alongside the fields the rules read: + * the map below is keyed on `row.id`, and a projection that omitted it would + * build an EMPTY map and leave every row unbound (#7537's shape, one seam + * over). + */ export async function resolvePredicateRelated( this: Pick, schema: any, @@ -316,7 +472,36 @@ export async function resolvePredicateRelated( }; } -/** The body of `ObjectQL#resolveOneHopRelated`; the method's docblock there is its contract. */ +/** + * [#24197] Resolve the related rows a page of `rows` reads ONE hop through a + * reference field — the one loader behind both seams that serve a hop: + * validation predicates ({@link resolvePredicateRelated}, `authority: + * 'predicate'`) and formula fields ({@link resolveFormulaRelated}, + * `authority: 'caller'`). + * + * Everything but the AUTHORITY of the value read is shared, so the two seams + * cannot disagree about it: which reference fields and related fields to + * read ({@link collectReferenceTraversals}), the declared-field intersection + * and its `undeclared-field` report, the id set (one batched read per + * reference field per call, never one per row; a multi-value or already + * expanded slot names no single row), the `null` materialisation of a + * declared column the driver omitted, and the per-row + * {@link RelatedFieldBinding} with its reason when there is no row. + * + * ## The authority, and why the two differ + * + * - `predicate`: a rule's output is a pass/fail the SYSTEM enforces, so the + * values are read as SYSTEM behind the caller's read doors — see + * {@link resolvePredicateRelated} for the whole argument. + * - `caller`: a formula's output is DATA handed to the reader. Reading the + * hop as SYSTEM would hand any reader of the junction row a related + * column it could not read directly, so the hop is read under the + * caller's OWN context, through every enforcement layer `find` applies + * (row-level security, field-level security, the organization wall). A + * related row that read does not return is `unresolved` — whether it + * exists nowhere or the reader may not see it, the formula answers the + * same, so the answer discloses neither. + */ export async function resolveOneHopRelated( this: Pick, schema: any, diff --git a/packages/objectql/src/engine-migration-flags.ts b/packages/objectql/src/engine-migration-flags.ts index 2f875977c1f..9af9ed2f40d 100644 --- a/packages/objectql/src/engine-migration-flags.ts +++ b/packages/objectql/src/engine-migration-flags.ts @@ -43,6 +43,11 @@ * This module imports the engine only as a TYPE, so there is no module cycle * at run time, and nothing re-exports it, so the published surface does not * move. + * + * [#30053 codemod] A PRIVATE method's docblock has since moved here, + * onto its function (`--hand-docs` of `scripts/codemod-split-engine.mjs`), + * and the class keeps none for it; a public or protected method's docblock + * stays on the class, the contract, and its function keeps the placeholder. */ import { @@ -87,7 +92,16 @@ export interface MigrationFlagsHost { readMigrationFlagVerified: ObjectQL['readMigrationFlagVerified']; } -/** The body of `ObjectQL#supplyTenantKeyCertifiedResolver`; the method's docblock there is its contract. */ +/** + * [ADR-0131 D1] Hand a freshly-registered driver the question "is this + * database certified", the same closure-not-value seam as + * {@link supplyFileColumnsMovedResolver} and for the same reason: the answer + * is a `sys_migration` row on a store this driver has not connected to yet. + * A SQL driver asks it only when a table carries an index an earlier release + * built with the NULL-safe `COALESCE(, …)` key part, to decide + * whether that index already is the plain key new DDL emits. A driver with + * no such seam is left alone. + */ export function supplyTenantKeyCertifiedResolver( this: Pick, driver: IDataDriver, @@ -135,7 +149,13 @@ export async function isOrganizationOwnershipCertified( } } -/** The body of `ObjectQL#readFileReferencesFlagRow`; the method's docblock there is its contract. */ +/** + * The one memoized read both file-flag questions come off (#15989). + * + * Written once so the two public readers cannot drift apart in the ARGUMENTS + * they pass — the slot and the migration id decide which row is read, and a + * second copy of them is a second way to read a different row. + */ export function readFileReferencesFlagRow( this: Pick, ): Promise { @@ -160,7 +180,21 @@ export async function isValueShapesMigrationVerified( ); } -/** The body of `ObjectQL#readMigrationFlagVerified`; the method's docblock there is its contract. */ +/** + * Read one deployment migration flag and answer whether it authorises its + * consumers. Shared by both flags so the "every way of not knowing answers + * false" rule is written once: no `sys_migration` object registered, no row, + * an unreadable table, a malformed row — all `false`. Enforcement derives + * from evidence, and absent evidence is not permission. + * + * `conclusive` says whether the ledger was actually consulted, so the caller + * can tell "asked, and the answer is no" from "could not ask" — see + * {@link readMigrationFlagMemoized}. It never changes the verdict, only + * whether that verdict is worth remembering. + * + * Costs nothing on a kernel without the platform objects: the registry + * lookup short-circuits before any query. + */ export async function readMigrationFlagVerified( this: Pick, migrationId: string, diff --git a/packages/objectql/src/engine-package-registration.ts b/packages/objectql/src/engine-package-registration.ts index baef35f8f21..6dba7f9c0c3 100644 --- a/packages/objectql/src/engine-package-registration.ts +++ b/packages/objectql/src/engine-package-registration.ts @@ -49,6 +49,11 @@ * this one imports, so the engine's evaluation order is what it was. The * published build bundles both files into one, where the cycle does not * exist. + * + * [#30053 codemod] A PRIVATE method's docblock has since moved here, + * onto its function (`--hand-docs` of `scripts/codemod-split-engine.mjs`), + * and the class keeps none for it; a public or protected method's docblock + * stays on the class, the contract, and its function keeps the placeholder. */ import type { ServiceObject } from '@objectstack/spec/data'; @@ -112,7 +117,11 @@ export function convertPackageBodyForward( return result.definition; } -/** The body of `ObjectQL#resolveNavObjectNames`; the method's docblock there is its contract. */ +/** + * Deep-clone an app definition, resolving objectName references in navigation + * items via the registry. Object names are canonical identifiers — no FQN + * expansion is applied. + */ export function resolveNavObjectNames( app: any, namespace: string, @@ -134,7 +143,17 @@ export function resolveNavObjectNames( return { ...app, navigation: resolveItems(app.navigation) }; } -/** The body of `ObjectQL#registerPlugin`; the method's docblock there is its contract. */ +/** + * Register a nested plugin's metadata (objects, actions, views, etc.) + * + * Unlike registerApp(), this does NOT call SchemaRegistry.installPackage() + * because plugins are not formal manifests — they are lightweight config + * bundles with objects, actions, triggers, and navigation. + * + * @param plugin - The plugin config object + * @param parentId - The parent package ID (for ownership tracking) + * @param parentNamespace - The parent package's namespace (for FQN resolution) + */ export function registerPlugin( this: Pick, plugin: any, diff --git a/packages/objectql/src/engine-readable-ids-permission-formula.ts b/packages/objectql/src/engine-readable-ids-permission-formula.ts index a9e688aa27c..5d67025ab7b 100644 --- a/packages/objectql/src/engine-readable-ids-permission-formula.ts +++ b/packages/objectql/src/engine-readable-ids-permission-formula.ts @@ -57,6 +57,11 @@ * declarations, and `engine.ts` imports this module after every module this one * imports, so the engine's evaluation order is what it was. The published build * bundles both files into one, where the cycle does not exist. + * + * [#30053 codemod] A PRIVATE method's docblock has since moved here, + * onto its function (`--hand-docs` of `scripts/codemod-split-engine.mjs`), + * and the class keeps none for it; a public or protected method's docblock + * stays on the class, the contract, and its function keeps the placeholder. */ import { ReadableIds, RLS_DENY_FILTER, isControlledByParentMasterField, markFilterSubtreeProvenance } from '@objectstack/spec/data'; @@ -111,7 +116,43 @@ export interface ReadableIdsPermissionFormulaHost { */ export const READABLE_IDS_MAX_CHAIN_DEPTH = 8; -/** The body of `ObjectQL#composeReadableIdsChain`; the method's docblock there is its contract. */ +/** + * [ADR-0139 D2, D6 as amended] Compose the leaves at `sites` in `node` — + * the read's `where` at the top level, a parent's composed scope below it — + * and, first, every leaf a scope answered for one of their parents carries, + * recursively. Returns `node` rewritten copy-on-write, every leaf replaced + * by its composed form or `[]`. + * + * `ancestors` are the parents whose scopes enclose `node`, outermost first: + * empty for the read's own `where`, `[P]` inside P's scope, `[P, G]` inside + * the scope G answered within it. Per distinct parent of `sites`, in walk + * order: + * + * 1. D5's two classes are refused by name, exactly as at the top level + * ({@link refuseUncomposableReadableIdsClasses}): a nested parent an + * object-scoped middleware narrows, or one bound to another datasource + * than the read's object, is refused — the whole chain compiles into the + * read's one statement, on the read's datasource. + * 2. A parent already answered on this read — at any level — is not asked + * again: its composed scope is reused by reference. + * 3. A parent already on `ancestors` is a cycle (its own scope, or a scope + * inside it, reaches back to it); a parent whose scope would be the + * chain's scope number {@link READABLE_IDS_MAX_CHAIN_DEPTH} + 1 is past + * the bound. Both are refused by name, `INTERNAL_ERROR` / 500, before + * the service is asked about that parent: a chain the engine cannot + * compose whole is not composed partway, and ⛔ never answered "no + * restriction". + * 4. The parent is asked both halves ({@link readableIdsScopeOf}) with the + * read's context as it stands; a scope that carries leaves is composed + * through this method first, under `[...ancestors, parent]`, and the + * rewritten scope is what the parent's leaf is composed with. The + * provenance mark the scope carries (`'policy'`) survives the rewrite. + * + * A nested leaf outside a `$in` comparand, or a composed leaf the engine did + * not compose, inside an answered scope is refused by the same walk that + * refuses it in a caller's `where` ({@link collectComposableReadableIds}): + * the scope is the `where` of a read of its parent and is judged as one. + */ export async function composeReadableIdsChain( this: Pick, chain: ReadableIdsChain, @@ -185,7 +226,11 @@ export function findUnservableControlledByParentMasters( return found; } -/** The body of `ObjectQL#readableIdsScopeOf`; the method's docblock there is its contract. */ +/** + * [ADR-0139 D2 steps 1-2] The scope a leaf over `parent` is composed with, + * or `null` for `$in: []`. Fails closed on every answer that is not a + * positive one, and on every throw. + */ export async function readableIdsScopeOf( this: Pick, object: string, @@ -225,7 +270,35 @@ export async function readableIdsScopeOf( return { scope: markFilterSubtreeProvenance(scope as FilterCondition, 'policy') }; } -/** The body of `ObjectQL#resolveOptionPermissions`; the method's docblock there is its contract. */ +/** + * [#18783] The permission map one write's option gates are evaluated with — + * resolved at most ONCE for the whole write (a batch insert, a by-id update, + * an N-row bulk update, a `validate()` preview), never per row and never per + * predicate, and never kept past the call: the contract member is + * request-scoped, and a map held across writes would serve a grant that may + * since have been revoked. + * + * Asked ONLY when the write can need it — a resolver is registered, the write + * has an acting user (`current_user` is otherwise unbound and `can` asks + * about nobody), and some payload PICKS an option whose `visibleWhen` calls + * `can` ({@link optionVisibilityReadsPermissions}, the evaluator's own + * picker). Every other write pays nothing, and cannot be refused by a + * resolution it never depended on. + * + * The answer goes through `toEvalPermissions`, formula's one door into + * `EvalContext.permissions`, so a map that is not the published shape is + * refused here rather than answering `can()` confidently wrong. + * + * Returns a per-payload accessor, the shape {@link resolvePredicateRelated} + * hands back: + * + * - a payload that picks no `can`-gated option gets `undefined`; + * - one that does gets the map; + * - when the resolution THREW (or the map was refused), one that does gets + * the throw, re-raised untouched: the write fails CLOSED, with the + * resolution's own error. ⛔ It is never read as "no grants", which would + * publish a failure as a measured denial of everything. + */ export async function resolveOptionPermissions( schema: unknown, payloads: ReadonlyArray | null | undefined>, @@ -252,7 +325,30 @@ export async function resolveOptionPermissions( return (payload) => (needs(payload) ? permissions : undefined); } -/** The body of `ObjectQL#permissionResolution`; the method's docblock there is its contract. */ +/** + * [#20082] ONE operation's effective-permission resolution — the single + * resolver ask every consumer inside that operation shares. On a write those + * are its CEL `defaultValue`s ({@link resolveDefaultPermissions}), its option + * gates ({@link resolveOptionPermissions}) and the formula fields on its + * response ({@link resolveFormulaPermissions}); on a read, its formula fields. + * The contract member's "resolve it ONCE per request" is kept per engine + * operation: a write that carries a `can` default, a `can` option gate and a + * `can` formula asks the resolver once, not three times. + * + * `undefined` when there is nothing to ask — no resolver registered, or no + * acting user (`current_user` is then unbound, and `can` asks about nobody). + * Every consumer then passes NO permission data: ⛔ never an empty map, which + * is a real answer ("holds nothing") and would deny confidently. + * + * Lazy and memoised for the operation: the ask happens when the first consumer + * NEEDS the map, never earlier, and a later consumer awaits the same promise — + * its rejection included, so one failed resolution is the same failure + * everywhere in the operation rather than a second, possibly different, ask. + * The answer goes through `toEvalPermissions`, formula's one door into + * `EvalContext.permissions`, so an off-shape map fails here like a throw. + * ⛔ Never kept past the operation: each operation mints its own, because a map + * held across operations would serve a grant that may since have been revoked. + */ export function permissionResolution( this: Pick, context: ExecutionContext | undefined, @@ -270,7 +366,33 @@ export function permissionResolution( }; } -/** The body of `ObjectQL#resolveFormulaPermissions`; the method's docblock there is its contract. */ +/** + * [#20082] The permission map a batch of `formula` fields is evaluated with — + * a read's (`find`, `findOne`) or a write response's + * ({@link hydrateWriteFormulas}). + * + * Asked only when it can change an answer: the context binds an acting user + * (the way {@link applyFormulaPlan} binds `current_user` — a truthy `userId`), + * some planned formula CALLS `can` ({@link readsPermissionPredicate}, the same + * reading the option gate uses), and there is a record to evaluate. Every + * other read pays nothing, and a whole `find` asks at most once, never per row. + * + * A read cannot refuse a row because one computed field lacks data, so when + * the map cannot be had the formula keeps the rule every formula that does not + * evaluate follows on this path — it reads `null` — and this method says so, + * once per operation, at `warn`, naming the object, the fields and the reason: + * + * - `no-permission-source`: no resolver is registered. NO map is passed (the + * contract member's rule for an absent method), so `can()` keeps its own + * loud refusal and the field reads `null`. + * - `permission-resolution-failed`: the resolution threw, or answered a map + * that is not the published shape. Fail CLOSED: ⛔ never read as "no + * grants" (an empty map would publish a confident `false`) and ⛔ never a + * grant — the field reads `null`, with the resolution's error in the line. + * + * `warn`, not `error`: a formula is computed on read and nothing is persisted + * from it, so the degradation is visible to whoever reads the field. + */ export async function resolveFormulaPermissions( this: Pick, object: string, @@ -306,7 +428,33 @@ export async function resolveFormulaPermissions( } } -/** The body of `ObjectQL#reportFormulaFault`; the method's docblock there is its contract. */ +/** + * [#22019] A formula field that did not evaluate, reported ONCE per + * (object, field) per engine instance. + * + * ## What it closes + * + * {@link applyFormulaPlan} answers `null` for a formula it cannot evaluate, + * and said nothing. A formula calling an unregistered function + * (`sqrt(record.amount)`) therefore read `null` on every row of every read + * and every write response, with no line anywhere to say why — the silent + * swallow ADR-0032 forbids at a call site. The answer stays `null` (what a + * read returns is protocol); the fault is now said. + * + * ## Level and rate + * + * `warn`, on AGENTS.md's one question: a formula is computed on read and + * nothing is persisted from it, so the degradation is a field reading `null` + * — visible to whoever reads it — never a write that claims to land and does + * not. That is the reading {@link resolveFormulaPermissions} makes for the + * same `null`. + * + * Once per pair per engine instance, as {@link warnCascadeNotAtomic} says its + * piece once per object: a list read evaluates a faulting formula once per + * row, and a line per row is the volume that gets a log skimmed. A later + * fault on the same pair is not re-reported, even with a different error — + * the first line already names the field to fix. + */ export function reportFormulaFault( this: Pick, object: string, @@ -329,7 +477,28 @@ export function reportFormulaFault( ); } -/** The body of `ObjectQL#resolveDefaultPermissions`; the method's docblock there is its contract. */ +/** + * [#20082] The permission map a write's CEL `defaultValue`s are evaluated with + * — a per-row accessor, the shape {@link resolveOptionPermissions} hands back, + * over the write's ONE {@link permissionResolution}. + * + * Asked only when a row will really be defaulted by an expression that calls + * `can`: the write has an acting user (the way {@link applyFieldDefaults} + * binds `current_user` — a truthy `userId`), and the row leaves such a field + * without a value (`applyFieldDefaults`' own `!= null` test). A row that + * supplies the field, and an object whose defaults never call `can`, pay + * nothing and cannot be refused by a resolution they never depended on. + * + * - no resolution (no resolver registered): every row gets `undefined` — NO + * map. The default then does not evaluate and is left unset with + * `applyFieldDefaults`' existing warn, which carries formula's own "carries + * no permission data" refusal; a `required` field so defaulted is refused + * by required-validation, exactly as before this seam existed. + * - the resolution threw (or its map was refused): a row that needs it gets + * the throw, re-raised untouched — the write fails CLOSED with the + * resolution's own error, as an option gate's does. ⛔ Never read as "no + * grants", which would STORE `false` as though it had been measured. + */ export async function resolveDefaultPermissions( this: Pick, object: string, diff --git a/packages/objectql/src/engine-secret-internal-fields.ts b/packages/objectql/src/engine-secret-internal-fields.ts index 8796f407712..fe8ad2a06ca 100644 --- a/packages/objectql/src/engine-secret-internal-fields.ts +++ b/packages/objectql/src/engine-secret-internal-fields.ts @@ -51,6 +51,11 @@ * This module imports the engine only as a TYPE, so there is no module cycle * at run time, and nothing re-exports it, so the published surface does not * move. + * + * [#30053 codemod] A PRIVATE method's docblock has since moved here, + * onto its function (`--hand-docs` of `scripts/codemod-split-engine.mjs`), + * and the class keeps none for it; a public or protected method's docblock + * stays on the class, the contract, and its function keeps the placeholder. */ import type { CryptoHandle } from '@objectstack/spec/contracts'; @@ -159,7 +164,52 @@ export class SecretRefWriteError extends Error { } } -/** The body of `ObjectQL#encryptSecretFields`; the method's docblock there is its contract. */ +/** + * Normalize credential fields on `row` in place before it reaches the driver. + * + * Two channels share the read mask ({@link SECRET_MASK}) but differ on write + * (see ADR-0100): + * + * - **`secret`** — encrypted: each plaintext is wrapped by the ICryptoProvider + * and replaced on `row` by an opaque ref. Cleartext never reaches the + * business table. [#27495] The `sys_secret` row is SEALED here and handed + * back, not written: the caller writes it with {@link persistSecretRows} + * immediately before the record's own driver write, so a write refused + * after this pass (validation, the post-image seam) leaves no row behind. + * - **`password`** (generic, non-`better-auth`) — plaintext at rest: stored + * verbatim, no encryption and no `sys_secret` row. Only the echoed-mask drop + * below applies to it. + * + * Rules: + * - Any masked field (secret or password) whose value equals the read mask ⇒ + * the key is dropped, so a form round-trip that echoes the mask does not + * overwrite the stored value. + * - No secret fields on the object ⇒ no further work (fast path, no crypto). + * - `null`/`undefined` secret value ⇒ left as-is (clears the secret). + * - [#8559] `""` secret value ⇒ REFUSED with {@link EmptyCredentialWriteError} + * (ADR-0112 `VALIDATION_ERROR`/400, message names `null` as the clear + * spelling). Encrypting it would mint a `sys_secret` row that decrypts to + * nothing behind a perfectly valid ref, so every read reports "a secret is + * set" while the dereference returns nothing — the contradictory state + * #8542/#8558 defend consumers against. Refused, not folded into the + * `null` branch: silently reinterpreting the write was explicitly rejected + * (maintainer ruling 2026-08-13 on #8559). The `password` half of the same + * ruling lands at its own seam — {@link refuseEmptyPasswordFields} — since + * the two types share the read mask but not this encryption path. + * - Secret value already a ref ⇒ [#27582] a non-system write may only seal + * a new value or keep its own record's ref. From the system principal + * (`context.isSystem === true`) it is left as-is, as it always was. From + * any other caller it is kept only when every row the write lands on + * already holds exactly that ref in that column — read from `storedRows`, + * which each call site supplies (none for an insert, the pre-image for a + * by-id update, the matched rows for a predicate update) — and then the + * key is dropped, as the echoed mask is. Any other ref is REFUSED with + * {@link SecretRefWriteError} (ADR-0112 `VALIDATION_ERROR`/400), one + * answer whatever the ref names. + * - **Fail-closed:** any other secret value with no CryptoProvider registered, + * or no reachable `sys_secret` store, THROWS — never persists cleartext. + * (A `password` field needs no CryptoProvider — it is stored as-is.) + */ export async function encryptSecretFields( this: Pick, object: string, @@ -323,7 +373,14 @@ export function deletedSecretSlots( return retiredSecretSlots(schema, deletedRows, clearEvery); } -/** The body of `ObjectQL#persistSecretRows`; the method's docblock there is its contract. */ +/** + * [#27495] Write the `sys_secret` rows {@link encryptSecretFields} sealed for + * one record write. Called immediately before that record's driver write and + * after every pass that can refuse it, so the credential row is written only + * with the record that names it. Each row lands on `written` as it is + * written, which is what the write's failure path hands to + * {@link releaseSecretHandles} when the record write does not land. + */ export async function persistSecretRows( this: Pick, pending: readonly PendingSecretRow[], @@ -335,7 +392,40 @@ export async function persistSecretRows( } } -/** The body of `ObjectQL#releaseSecretHandles`; the method's docblock there is its contract. */ +/** + * [#27495] Remove each `sys_secret` row in `slots` that the column it was + * minted for no longer names — best-effort, and never a throw. + * + * Two occasions reach it. `'refused'`: a write that sealed and wrote a row + * did not complete, so the row may name nothing. `'retired'`: a committed + * update cleared the field (`null`) or replaced its value, so the handle the + * column named before may name nothing; [#27581] a committed delete reaches + * it the same way, for every handle the removed rows held. The same three + * checks decide both, + * and every one that cannot pass KEEPS the row, the recoverable direction + * (deleting a handle a row still names destroys that credential, and nothing + * can name what was lost): + * + * - **Verified, never inferred.** No row of `object` may still name the + * handle in that column: a predicate write seals ONE handle for every row + * it matches, so clearing one of them leaves the handle in force for the + * rest; a write that did land after all (a partially applied batch) still + * names its row. + * - **This column's own handle.** The `sys_secret` row's `namespace`/`key` + * must be this object and field; a ref a system writer copied onto another + * column is not released through that column. + * - **Inside the record's unit.** The count and the delete ride the open + * transaction where it covers their drivers, so they see the write's own + * state and roll back with it; a record write in a unit that does not + * cover the `sys_secret` store keeps the row, since the delete could + * outlive a rollback of the record. + * + * A row kept for a failure (not for being in use) is reported once at + * `error`, naming the column and the remedy: the caller's answer does not + * change, and the stored ciphertext is exactly what the operator believes + * gone. Legacy orphans written before this existed are not swept here; the + * orphan report lists them. + */ export async function releaseSecretHandles( this: Pick, object: string, @@ -403,7 +493,29 @@ export async function releaseSecretHandles( } } -/** The body of `ObjectQL#refuseEmptyPasswordFields`; the method's docblock there is its contract. */ +/** + * [#8559] Refuse `""` on a `password`-typed field before it reaches the + * driver — the `password` half of the same maintainer ruling (2026-08-13) + * that makes {@link encryptSecretFields} refuse `""` on a `secret` field. + * + * Its own seam, deliberately NOT routed through the encryption path: the two + * types share the read mask and the echoed-mask drop but a `password` field + * is plaintext at rest (no CryptoProvider, no `sys_secret` row — ADR-0100), + * so the refusal binds where its write actually flows. Storing `""` verbatim + * would produce the same contradiction as the secret arm without the cipher + * row: a non-null stored value that every read masks as "a password is set" + * while the credential has no content. `null` (clear) and the echoed mask + * (drop, handled in {@link encryptSecretFields}) keep their meanings. + * + * Scope is {@link collectMaskedPasswordFields}: `managedBy: 'better-auth'` + * objects are exempt — their password column is not masked on read, so the + * "reads as set, holds nothing" contradiction cannot arise there, and its + * write path belongs to the auth subsystem. + * + * Called at every call site of {@link encryptSecretFields} (single/bulk + * insert, single-id update, multi update) so the two halves of the ruling + * guard the same doors. + */ export function refuseEmptyPasswordFields( this: Pick, object: string, @@ -418,7 +530,19 @@ export function refuseEmptyPasswordFields( } } -/** The body of `ObjectQL#maskSecretFields`; the method's docblock there is its contract. */ +/** + * Mask credential fields on read so plaintext never leaves the engine through + * the normal query path. Covers `secret` fields (always) and `password` fields + * on generic, non-`better-auth` objects (see {@link collectMaskedReadFields} + * and ADR-0100). A set value becomes {@link SECRET_MASK}; an unset one stays + * `null`. Privileged callers that genuinely need a secret's plaintext use + * {@link resolveSecret} against the stored ref; a `password` field is stored + * as plaintext at rest, so its cleartext is only ever reachable off this path. + * + * [#7728] Second collector branch, same choke point: a field declared + * `internal: true` is OMITTED rather than masked — see + * {@link omitInternalFields} for why the two dispositions differ. + */ export function maskSecretFields( this: Pick, object: string, @@ -443,7 +567,61 @@ export function maskSecretFields( this.omitInternalFields(object, rows); } -/** The body of `ObjectQL#omitInternalFields`; the method's docblock there is its contract. */ +/** + * [#7728] Drop every field declared `internal: true` from the rows the engine + * hands back — "the declared value is never returned on the generic data + * path". This is the read protection for ADR-0100's third credential channel: + * auth-subsystem one-way hashes stored in `text` columns, which the two + * type-keyed credential collectors structurally cannot reach. + * + * **OMIT, not mask** (maintainer ruling 2026-08-12 on #7728). The credential + * mask exists to signal "a value is set" without leaking it. The column this + * was minted for — `sys_api_key.key` — is `required: true`, so it is ALWAYS + * set: the signal carries zero bits, while still shipping a value under a + * field whose own description says it is "never exposed to clients". Omission + * also leaves that description string untouched, so the four generated + * translation bundles that mirror it do not churn. + * + * **`?select=` is covered by construction, and that is load-bearing.** The + * strip acts on the RESULT ROWS, not on the projection, so a client that + * spells the column out (`?select=id,key`) gets a 200 without it rather than + * a bypass. `select` only gates on whether a field is KNOWN + * (`assertProjectionFieldsExist`) and a flagged column is known, so a + * projection-aware strip would have shipped looking complete and still leaked + * to any caller who named the column — measured on the sibling column in + * #7823, and reproduced here on `sys_api_key.key` before the fix. + * + * **No system carve-out**, and this is where the shape deliberately diverges + * from its sibling {@link stripSearchCompanionFromRead}. That one keeps the + * `__search` companion for a system caller who names it by projection, + * because it has exactly one such reader whose backfill comparison would + * otherwise rewrite every row on every run. This flag has no such reader: the + * API-key verifier uses the column as a `where` FILTER and never reads it off + * the result (`resolveApiKeyPrincipal` takes `expires_at` / `user_id` / + * `organization_id` / `scopes`), and the mint path returns the plaintext it + * generated, not the row it inserted. An escape hatch nobody needs is a hole + * in a non-exposure guarantee, so there isn't one — if a legitimate system + * reader ever appears, it reads the column through a purpose-built privileged + * accessor, the way {@link resolveSecret} does for `secret`. + * + * Nothing below storage is touched. The strip runs on rows the driver has + * already produced, so the predicate has been evaluated and the index used + * before this method sees anything — which is precisely why authentication + * keeps working. + * + * **READ path only** (#7823, maintainer ruling 2026-08-13 / A-prime). This + * used to run at the two write-response sites as well (the 201 create body + * and the by-id-update 200 body), which conflated "never returned on the + * generic data path" with "never returned to the engine-level caller that + * performed the write" — and broke authentication for `sys_session.token`, + * whose minted value better-auth reads back off the insert result. The + * write-response half of the guarantee now lives at the generic-data-path + * ingress (`omitInternalFieldsFromWriteResponse` in + * `@objectstack/metadata-protocol`, tripwire-enforced across every `*Data` + * write face); engine write results keep the stored row whole. Lifecycle + * consumers that need a flagged value back off a READ go through + * {@link resolveInternalField} — never a carve-out here. + */ export function omitInternalFields( this: Pick, object: string, @@ -460,7 +638,70 @@ export function omitInternalFields( } } -/** The body of `ObjectQL#stripSearchCompanionFromRead`; the method's docblock there is its contract. */ +/** + * [#7642] Strip the hidden `__search` companion column from what a read + * hands back, unless a SYSTEM caller named it in its projection. + * + * The column is declared client-invisible (`hidden` + `readonly` + `system` + * + `searchable: false`) and the enforcement that exists is real: it is kept + * out of auto-views, out of the `$search` auto-default, and a `$searchFields` + * override naming it is refused with a 400 ("is hidden"). What was missing is + * the PROJECTION half — a query that names no `fields` reaches the driver + * with `ast.fields` undefined, every driver answers that with `SELECT *`, and + * the companion rode back in every record body: list results, GET by id, + * `/search` hits (which are `engine.find` rows verbatim) and the 201 create + * body. The rule is applied HERE, at the engine, because the engine is the + * PRODUCER those four surfaces share; fixing them one consumer at a time is + * how three of the four would stay broken. + * + * Two carve-outs, both measured rather than defensive: + * + * - **A system caller that asks for it by name keeps it.** The companion has + * exactly one such reader: `plugin-pinyin-search`'s backfill/reconcile + * walk, which projects `['id', ...sources, '__search']` under + * `{ isSystem: true }` and compares the stored blob against the recomputed + * one. Strip it unconditionally and that comparison reads `undefined` + * every pass — the backfill would rewrite every row of every object on + * every run, which is worse than the disclosure it was fixing. + * - **A non-system caller does NOT keep it, even by name.** `select` only + * gates on whether a field is KNOWN (`assertProjectionFieldsExist`), and + * the companion is known once provisioned — so `?select=__search` would + * otherwise be an open door straight through this strip, and a + * client-invisibility rule with a documented spelling that bypasses it is + * not one. `isSystem` is server-derived (never client input), the same + * trust the read-only strips on the write path already place in it. + * + * ## Why this door is SILENT where the `$searchFields` door returns a 400 + * + * Asked on #7876 and ruled there on 2026-08-12 (direction C): the divergence + * is intended, and it is not reopenable on symmetry alone. The two doors are + * two KINDS of surface. + * + * - `$searchFields` is AUTHORING input — it tells the server how to RUN the + * query. A value the server will not honour has to be said out loud, or + * the caller gets a WIDER answer than the one they narrowed to, in a + * response with nothing to distinguish it from a satisfied one. That is + * why `assertSearchFieldsAreSearchable` refuses the name with a 400 + * (#4254) — the refusal is protecting the ROW SET. + * - `select` is a READ PROJECTION — it names what the caller would like + * back. Dropping a column the caller may not see leaves the answer + * correct: the rows are still the rows that were asked for, one key + * lighter. {@link omitInternalFields} directly above answers + * `?select=id,key` exactly this way, for exactly this reason (#7728), so + * silence here is the platform's existing read-path rule, not an + * exception to it. + * + * ⛔ Do not add a refusal here to make the two doors agree. That was option B + * on #7876, weighed and declined: it turns a request that answers 200 + * today into a failure, for tidiness, on a spelling no non-system caller in + * this tree uses — the companion's only deliberate reader is the backfill + * carved out above. If a REAL caller is ever burned by the silent drop, that + * measurement reopens it; the asymmetry by itself does not. + * + * ⚠️ `requestedFields` must be the CALLER's `fields`, captured before + * `planFormulaProjection` — that pass rewrites the projection to every stored + * column when a formula is in play, companion included. + */ export function stripSearchCompanionFromRead( rows: unknown, requestedFields: readonly string[] | undefined, @@ -470,7 +711,39 @@ export function stripSearchCompanionFromRead( stripSearchCompanion(rows); } -/** The body of `ObjectQL#privilegedReadDriverOptions`; the method's docblock there is its contract. */ +/** + * Driver options for a PRIVILEGED, driver-level read so it JOINS the open + * ambient transaction instead of asking the pool for a second connection. + * + * The three privileged read verbs — {@link resolveSecret}, + * {@link resolveSecretField}, {@link resolveInternalField} — deliberately + * read at DRIVER level, the only layer where the masked/omitted value still + * exists. That bypasses hooks, field-level security and sharing by design; + * what it must NOT bypass is the connection the surrounding transaction is + * holding. {@link buildDriverOptions} threads the ambient handle onto every + * ordinary read for that reason (ADR-0034); these three passed NO options at + * all, so their read went to a FRESH pooled connection. + * + * On a roomy pool that is invisible — the pool simply hands out a second + * connection. On a **single-connection pool it is a deadlock**: SQLite's knex + * pool is `max: 1` (`driver-sqlite-wasm` and `driver-sql`/better-sqlite3 + * both), so the open transaction holds the one connection and the privileged + * read waits for a connection that cannot be freed until the transaction + * that is waiting on the read commits. Measured on the erasure path + * (`runSubjectErasureAtomically` → better-auth `/admin/remove-user` → + * `reattachInternalFieldsOnRead` → `resolveInternalField`): the read blocked + * until knex's own acquire timeout fired ("Timeout acquiring a connection. + * The pool is probably full", from `Transaction_Sqlite.acquireConnection`), + * and the vendor route degraded that into an authentication refusal — a + * signed-in caller answered `401` after ~120 s, on a route reachable without + * credentials. Postgres/MySQL (`max >= 10`) never exhibited it. + * + * Reads only, and only the transaction: the same-origin gate (#5351) still + * decides whether the handle is this object's driver's to use, so a + * privileged read that resolves to a DIFFERENT datasource keeps its own + * connection rather than executing on the wrong one. Returns `undefined` + * when there is no ambient transaction, which is the pre-existing shape. + */ export function privilegedReadDriverOptions( this: Pick, object: string, diff --git a/packages/objectql/src/engine.ts b/packages/objectql/src/engine.ts index da869d82d92..902a5b43b29 100644 --- a/packages/objectql/src/engine.ts +++ b/packages/objectql/src/engine.ts @@ -1813,50 +1813,13 @@ export class ObjectQL implements IObjectQLEngine { return (SecurityResolversFacet.composeReadableIdsAtBoundary).call(securityResolversHost.of(this), object, operation, ast, context); } - /** - * [ADR-0139 D2, D6 as amended] Compose the leaves at `sites` in `node` — - * the read's `where` at the top level, a parent's composed scope below it — - * and, first, every leaf a scope answered for one of their parents carries, - * recursively. Returns `node` rewritten copy-on-write, every leaf replaced - * by its composed form or `[]`. - * - * `ancestors` are the parents whose scopes enclose `node`, outermost first: - * empty for the read's own `where`, `[P]` inside P's scope, `[P, G]` inside - * the scope G answered within it. Per distinct parent of `sites`, in walk - * order: - * - * 1. D5's two classes are refused by name, exactly as at the top level - * ({@link refuseUncomposableReadableIdsClasses}): a nested parent an - * object-scoped middleware narrows, or one bound to another datasource - * than the read's object, is refused — the whole chain compiles into the - * read's one statement, on the read's datasource. - * 2. A parent already answered on this read — at any level — is not asked - * again: its composed scope is reused by reference. - * 3. A parent already on `ancestors` is a cycle (its own scope, or a scope - * inside it, reaches back to it); a parent whose scope would be the - * chain's scope number {@link READABLE_IDS_MAX_CHAIN_DEPTH} + 1 is past - * the bound. Both are refused by name, `INTERNAL_ERROR` / 500, before - * the service is asked about that parent: a chain the engine cannot - * compose whole is not composed partway, and ⛔ never answered "no - * restriction". - * 4. The parent is asked both halves ({@link readableIdsScopeOf}) with the - * read's context as it stands; a scope that carries leaves is composed - * through this method first, under `[...ancestors, parent]`, and the - * rewritten scope is what the parent's leaf is composed with. The - * provenance mark the scope carries (`'policy'`) survives the rewrite. - * - * A nested leaf outside a `$in` comparand, or a composed leaf the engine did - * not compose, inside an answered scope is refused by the same walk that - * refuses it in a caller's `where` ({@link collectComposableReadableIds}): - * the scope is the `where` of a read of its parent and is judged as one. - */ private async composeReadableIdsChain( chain: ReadableIdsChain, node: N, sites: readonly ComposableReadableIdsSite[], ancestors: readonly string[], ): Promise { - // [#25522 cut 15] The body moved to `engine-readable-ids-permission-formula.ts` (see `ReadableIdsPermissionFormulaHost`). + // [#25522 cut 15] The body moved to `engine-readable-ids-permission-formula.ts`, its docblock with it (see `ReadableIdsPermissionFormulaHost`). const host: ThisParameterType = { refuseUncomposableReadableIdsClasses: (...a) => this.refuseUncomposableReadableIdsClasses(...a), readableIdsScopeOf: (...a) => this.readableIdsScopeOf(...a), @@ -1908,11 +1871,6 @@ export class ObjectQL implements IObjectQLEngine { return findUnservableControlledByParentMasters.call(host); } - /** - * [ADR-0139 D2 steps 1-2] The scope a leaf over `parent` is composed with, - * or `null` for `$in: []`. Fails closed on every answer that is not a - * positive one, and on every throw. - */ private async readableIdsScopeOf( object: string, operation: string, @@ -1920,7 +1878,7 @@ export class ObjectQL implements IObjectQLEngine { svc: ReadableIdsSecurity, context: ExecutionContext | undefined, ): Promise<{ scope: FilterCondition | undefined } | null> { - // [#25522 cut 15] The body moved to `engine-readable-ids-permission-formula.ts` (see `ReadableIdsPermissionFormulaHost`). + // [#25522 cut 15] The body moved to `engine-readable-ids-permission-formula.ts`, its docblock with it (see `ReadableIdsPermissionFormulaHost`). const readLogger = () => this.logger; const host: ThisParameterType = { get logger() { return readLogger(); }, @@ -1928,70 +1886,17 @@ export class ObjectQL implements IObjectQLEngine { return readableIdsScopeOf.call(host, object, operation, parent, svc, context); } - /** - * [#18783] The permission map one write's option gates are evaluated with — - * resolved at most ONCE for the whole write (a batch insert, a by-id update, - * an N-row bulk update, a `validate()` preview), never per row and never per - * predicate, and never kept past the call: the contract member is - * request-scoped, and a map held across writes would serve a grant that may - * since have been revoked. - * - * Asked ONLY when the write can need it — a resolver is registered, the write - * has an acting user (`current_user` is otherwise unbound and `can` asks - * about nobody), and some payload PICKS an option whose `visibleWhen` calls - * `can` ({@link optionVisibilityReadsPermissions}, the evaluator's own - * picker). Every other write pays nothing, and cannot be refused by a - * resolution it never depended on. - * - * The answer goes through `toEvalPermissions`, formula's one door into - * `EvalContext.permissions`, so a map that is not the published shape is - * refused here rather than answering `can()` confidently wrong. - * - * Returns a per-payload accessor, the shape {@link resolvePredicateRelated} - * hands back: - * - * - a payload that picks no `can`-gated option gets `undefined`; - * - one that does gets the map; - * - when the resolution THREW (or the map was refused), one that does gets - * the throw, re-raised untouched: the write fails CLOSED, with the - * resolution's own error. ⛔ It is never read as "no grants", which would - * publish a failure as a measured denial of everything. - */ private async resolveOptionPermissions( schema: unknown, payloads: ReadonlyArray | null | undefined>, resolution: PermissionResolution | undefined, ): Promise<(payload: Record | null | undefined) => EvalPermissions | undefined> { - // [#25522 cut 15] The body moved to `engine-readable-ids-permission-formula.ts`; it reads no member of the class. + // [#25522 cut 15] The body moved to `engine-readable-ids-permission-formula.ts`, its docblock with it; it reads no member of the class. return resolveOptionPermissions(schema, payloads, resolution); } - /** - * [#20082] ONE operation's effective-permission resolution — the single - * resolver ask every consumer inside that operation shares. On a write those - * are its CEL `defaultValue`s ({@link resolveDefaultPermissions}), its option - * gates ({@link resolveOptionPermissions}) and the formula fields on its - * response ({@link resolveFormulaPermissions}); on a read, its formula fields. - * The contract member's "resolve it ONCE per request" is kept per engine - * operation: a write that carries a `can` default, a `can` option gate and a - * `can` formula asks the resolver once, not three times. - * - * `undefined` when there is nothing to ask — no resolver registered, or no - * acting user (`current_user` is then unbound, and `can` asks about nobody). - * Every consumer then passes NO permission data: ⛔ never an empty map, which - * is a real answer ("holds nothing") and would deny confidently. - * - * Lazy and memoised for the operation: the ask happens when the first consumer - * NEEDS the map, never earlier, and a later consumer awaits the same promise — - * its rejection included, so one failed resolution is the same failure - * everywhere in the operation rather than a second, possibly different, ask. - * The answer goes through `toEvalPermissions`, formula's one door into - * `EvalContext.permissions`, so an off-shape map fails here like a throw. - * ⛔ Never kept past the operation: each operation mints its own, because a map - * held across operations would serve a grant that may since have been revoked. - */ private permissionResolution(context: ExecutionContext | undefined): PermissionResolution | undefined { - // [#25522 cut 15] The body moved to `engine-readable-ids-permission-formula.ts` (see `ReadableIdsPermissionFormulaHost`). + // [#25522 cut 15] The body moved to `engine-readable-ids-permission-formula.ts`, its docblock with it (see `ReadableIdsPermissionFormulaHost`). const readEffectiveObjectPermissionsResolver = () => this._effectiveObjectPermissionsResolver; const host: ThisParameterType = { get _effectiveObjectPermissionsResolver() { return readEffectiveObjectPermissionsResolver(); }, @@ -2000,33 +1905,6 @@ export class ObjectQL implements IObjectQLEngine { return permissionResolution.call(host, context); } - /** - * [#20082] The permission map a batch of `formula` fields is evaluated with — - * a read's (`find`, `findOne`) or a write response's - * ({@link hydrateWriteFormulas}). - * - * Asked only when it can change an answer: the context binds an acting user - * (the way {@link applyFormulaPlan} binds `current_user` — a truthy `userId`), - * some planned formula CALLS `can` ({@link readsPermissionPredicate}, the same - * reading the option gate uses), and there is a record to evaluate. Every - * other read pays nothing, and a whole `find` asks at most once, never per row. - * - * A read cannot refuse a row because one computed field lacks data, so when - * the map cannot be had the formula keeps the rule every formula that does not - * evaluate follows on this path — it reads `null` — and this method says so, - * once per operation, at `warn`, naming the object, the fields and the reason: - * - * - `no-permission-source`: no resolver is registered. NO map is passed (the - * contract member's rule for an absent method), so `can()` keeps its own - * loud refusal and the field reads `null`. - * - `permission-resolution-failed`: the resolution threw, or answered a map - * that is not the published shape. Fail CLOSED: ⛔ never read as "no - * grants" (an empty map would publish a confident `false`) and ⛔ never a - * grant — the field reads `null`, with the resolution's error in the line. - * - * `warn`, not `error`: a formula is computed on read and nothing is persisted - * from it, so the degradation is visible to whoever reads the field. - */ private async resolveFormulaPermissions( object: string, plan: readonly FormulaPlanEntry[], @@ -2034,7 +1912,7 @@ export class ObjectQL implements IObjectQLEngine { context: ExecutionContext | undefined, resolution: PermissionResolution | undefined, ): Promise { - // [#25522 cut 15] The body moved to `engine-readable-ids-permission-formula.ts` (see `ReadableIdsPermissionFormulaHost`). + // [#25522 cut 15] The body moved to `engine-readable-ids-permission-formula.ts`, its docblock with it (see `ReadableIdsPermissionFormulaHost`). const readLogger = () => this.logger; const host: ThisParameterType = { get logger() { return readLogger(); }, @@ -2149,35 +2027,8 @@ export class ObjectQL implements IObjectQLEngine { return SecurityResolversFacet.formulaFaultSink.call(securityResolversHost.of(this), object); } - /** - * [#22019] A formula field that did not evaluate, reported ONCE per - * (object, field) per engine instance. - * - * ## What it closes - * - * {@link applyFormulaPlan} answers `null` for a formula it cannot evaluate, - * and said nothing. A formula calling an unregistered function - * (`sqrt(record.amount)`) therefore read `null` on every row of every read - * and every write response, with no line anywhere to say why — the silent - * swallow ADR-0032 forbids at a call site. The answer stays `null` (what a - * read returns is protocol); the fault is now said. - * - * ## Level and rate - * - * `warn`, on AGENTS.md's one question: a formula is computed on read and - * nothing is persisted from it, so the degradation is a field reading `null` - * — visible to whoever reads it — never a write that claims to land and does - * not. That is the reading {@link resolveFormulaPermissions} makes for the - * same `null`. - * - * Once per pair per engine instance, as {@link warnCascadeNotAtomic} says its - * piece once per object: a list read evaluates a faulting formula once per - * row, and a line per row is the volume that gets a log skimmed. A later - * fault on the same pair is not re-reported, even with a different error — - * the first line already names the field to fix. - */ private reportFormulaFault(object: string, field: string, error: EvalError): void { - // [#25522 cut 15] The body moved to `engine-readable-ids-permission-formula.ts` (see `ReadableIdsPermissionFormulaHost`). + // [#25522 cut 15] The body moved to `engine-readable-ids-permission-formula.ts`, its docblock with it (see `ReadableIdsPermissionFormulaHost`). const readFormulaFaultReported = () => this.formulaFaultReported; const readLogger = () => this.logger; const host: ThisParameterType = { @@ -2187,28 +2038,6 @@ export class ObjectQL implements IObjectQLEngine { return reportFormulaFault.call(host, object, field, error); } - /** - * [#20082] The permission map a write's CEL `defaultValue`s are evaluated with - * — a per-row accessor, the shape {@link resolveOptionPermissions} hands back, - * over the write's ONE {@link permissionResolution}. - * - * Asked only when a row will really be defaulted by an expression that calls - * `can`: the write has an acting user (the way {@link applyFieldDefaults} - * binds `current_user` — a truthy `userId`), and the row leaves such a field - * without a value (`applyFieldDefaults`' own `!= null` test). A row that - * supplies the field, and an object whose defaults never call `can`, pay - * nothing and cannot be refused by a resolution they never depended on. - * - * - no resolution (no resolver registered): every row gets `undefined` — NO - * map. The default then does not evaluate and is left unset with - * `applyFieldDefaults`' existing warn, which carries formula's own "carries - * no permission data" refusal; a `required` field so defaulted is refused - * by required-validation, exactly as before this seam existed. - * - the resolution threw (or its map was refused): a row that needs it gets - * the throw, re-raised untouched — the write fails CLOSED with the - * resolution's own error, as an option gate's does. ⛔ Never read as "no - * grants", which would STORE `false` as though it had been measured. - */ private async resolveDefaultPermissions( object: string, rows: ReadonlyArray, @@ -2217,7 +2046,7 @@ export class ObjectQL implements IObjectQLEngine { /** Only these fields' defaults count — the insert's re-default pass keeps no others. */ onlyFields?: readonly string[], ): Promise<(row: unknown) => EvalPermissions | undefined> { - // [#25522 cut 15] The body moved to `engine-readable-ids-permission-formula.ts` (see `ReadableIdsPermissionFormulaHost`). + // [#25522 cut 15] The body moved to `engine-readable-ids-permission-formula.ts`, its docblock with it (see `ReadableIdsPermissionFormulaHost`). const host: ThisParameterType = { defaultFieldEntries: (...a) => this.defaultFieldEntries(...a), }; @@ -2320,52 +2149,13 @@ export class ObjectQL implements IObjectQLEngine { return MiddlewareAndContextFacet.executeWithMiddleware.call(middlewareAndContextHost.of(this), ctx, executor); } - /** - * Build a HookContext.session from ExecutionContext — WHO is calling. - * - * Returns `undefined` when the context yields nothing session-worthy, which - * keeps `!ctx.session` meaning exactly one thing for hooks: "no identity - * envelope was supplied" — the bare-kernel / programmatic call that - * caller-gating hooks skip for. A context carrying only write PROVENANCE - * (`{ flowRunId }`, all an identity-less flow run has — #3712) is such a - * case: it says what produced the write, not who is calling, and surfaces - * through {@link buildProvenance} instead. Every real transport resolves - * `positions` into the context, so an anonymous HTTP request still yields a - * session and stays gated. - */ private buildSession(execCtx?: ExecutionContext): HookContext['session'] { - // [#25522 cut 9] The body moved to `engine-execution-plumbing.ts`; it reads no member. + // [#25522 cut 9] The body moved to `engine-execution-plumbing.ts`, its docblock with it; it reads no member. return buildSession(execCtx); } - /** - * Build the HookContext.provenance envelope — WHERE this write came from. - * - * Deliberately separate from {@link buildSession}: provenance is server- - * stamped, evaluated by no security middleware, and can exist with no - * identity beside it. A schedule-triggered flow run resolves no principal - * yet still owns its writes, and that is the case the approvals record lock - * needs to recognize (#3456 / #3712). - * - * `attributedUserId` rides the SAME envelope for the same reason (#4586): - * a better-auth-originated write authorizes as the system, so the human who - * triggered it must reach the audit writer WITHOUT appearing in `session` — - * where every caller-gating hook would read them as the caller. Attribution - * here, authorization in `session`/`isSystem`, never the two mixed. - * - * `performedByClientId` rides it for the THIRD time on the same reasoning - * (#17022, ADR-0090 D10 rule 4): an MCP OAuth agent's write authorizes as - * the human it acts for, so the acting client can only reach the audit - * writer through a channel that is not `session` — putting it there would - * make every caller-gating hook read the client as the caller, which is the - * opposite of what the delegation means. This copy list is the whole reason - * the field is not inert: `HookContext` is built from a CLOSED literal whose - * `provenance` value is exactly what this function returns, so a key - * declared on `hook.zod.ts` and missing here is declared and never - * populated (ADR-0049). - */ private buildProvenance(execCtx?: ExecutionContext): HookContext['provenance'] { - // [#25522 cut 9] The body moved to `engine-execution-plumbing.ts`; it reads no member. + // [#25522 cut 9] The body moved to `engine-execution-plumbing.ts`, its docblock with it; it reads no member. return buildProvenance(execCtx); } @@ -2379,32 +2169,15 @@ export class ObjectQL implements IObjectQLEngine { return MiddlewareAndContextFacet.claimReferentialCascadeDelete.call(middlewareAndContextHost.of(this), object, id); } - /** - * Build the acting-user object (ADR-0068 EvalUser shape) surfaced to - * validation-time predicates as `current_user` — notably per-option - * `visibleWhen` authorization gating (objectui#2284). Returns undefined for - * system / unauthenticated writes, where membership predicates then fail-open. - */ private buildEvalUser( execCtx?: ExecutionContext, ): { id: string; positions: string[]; organizationId: string | null } | undefined { - // [#25522 cut 9] The body moved to `engine-execution-plumbing.ts`; it reads no member. + // [#25522 cut 9] The body moved to `engine-execution-plumbing.ts`, its docblock with it; it reads no member. return buildEvalUser(execCtx); } - /** - * Build the `HookContext.user` shortcut — the ergonomic "current user" - * object surfaced to JS hooks. Carries `organizationId` (the blessed name - * for the caller's active org, identical to `session.organizationId` and - * `current_user.organizationId`) so a hook author who needs "the current org - * to filter by" writes `ctx.user.organizationId` with zero relearning (#3280). - * - * Returns undefined for system / unauthenticated writes (no acting user) — - * hooks that need an org regardless of a resolved user read - * `ctx.session.organizationId`, which is populated whenever a session is. - */ private buildUser(execCtx?: ExecutionContext): HookContext['user'] { - // [#25522 cut 9] The body moved to `engine-execution-plumbing.ts`; it reads no member. + // [#25522 cut 9] The body moved to `engine-execution-plumbing.ts`, its docblock with it; it reads no member. return buildUser(execCtx); } @@ -3119,29 +2892,13 @@ export class ObjectQL implements IObjectQLEngine { return RegistrationFacet.registerPackageBody.call(registrationHost.of(this), manifest, id, namespace, version, source, scope); } - /** - * Deep-clone an app definition, resolving objectName references in navigation - * items via the registry. Object names are canonical identifiers — no FQN - * expansion is applied. - */ private resolveNavObjectNames(app: any, namespace: string): any { - // [#25522 cut 17] The body moved to `engine-package-registration.ts`; it reads no member of the class. + // [#25522 cut 17] The body moved to `engine-package-registration.ts`, its docblock with it; it reads no member of the class. return resolveNavObjectNames(app, namespace); } - /** - * Register a nested plugin's metadata (objects, actions, views, etc.) - * - * Unlike registerApp(), this does NOT call SchemaRegistry.installPackage() - * because plugins are not formal manifests — they are lightweight config - * bundles with objects, actions, triggers, and navigation. - * - * @param plugin - The plugin config object - * @param parentId - The parent package ID (for ownership tracking) - * @param parentNamespace - The parent package's namespace (for FQN resolution) - */ private registerPlugin(plugin: any, parentId: string, parentNamespace?: string, parentVersion?: string) { - // [#25522 cut 17] The body moved to `engine-package-registration.ts` (see `PackageRegistrationHost`). + // [#25522 cut 17] The body moved to `engine-package-registration.ts`, its docblock with it (see `PackageRegistrationHost`). const readLogger = () => this.logger; const readRegistry = () => this._registry; const host: ThisParameterType = { @@ -3956,64 +3713,6 @@ export class ObjectQL implements IObjectQLEngine { return masterDetailHeadersCallerReads.call(host, master, ids, context); } - /** - * [#19853] Settle whether an UPDATE lands the master-detail FK its payload - * names, BEFORE any `parent`-scoped lock is judged — so `parent` is the - * header the write STORES, not merely the one it NAMES. Shared by the by-id - * and bulk paths, which differ only in how a header is resolved and how the - * FK's own lock is judged (one row, or "locked in ≥1 matched row"). - * - * #4889's rule — a repoint is judged against the master it lands on — is - * right only when the repoint LANDS. Two strips can take the FK back out - * after `parent` was resolved from it, and the row then keeps the header it - * had while every other parent-scoped lock was judged against one it never - * reached. Measured on the card's shape: `update(l1, { amount: 999, invoice: - * 'inv_b' })` with `invoice` statically `readonly` and `l1` under a PAID - * `inv_a` committed `{ amount: 999, invoice: 'inv_a' }` — the frozen line - * rewritten, still under the paid invoice. The strips, in the order they run - * after this point: - * - * ① the FK's OWN `readonlyWhen` lock. Judged here first, against the - * header the FK names — #4889's rule for the FK itself, unmoved. - * [#19911] Judged WITH the other caller-supplied locks, not alone, and - * settled with them (`settleReadonlyWhenDrops`), so a value one of them - * drops can no longer unlock it — but only the FK is TAKEN here (`only`). - * That moves the FK's verdict in BOTH directions: a repoint its lock - * used to let through can now stay home, and a repoint it used to hold - * can now LAND, when the value its `record` lock reads is itself locked - * under the header the FK names — that value is dropped, the FK's lock - * reads the stored one, and the rest are then judged under the header - * the row lands on. When that verdict keeps the FK off the landing it is final: - * the returned `supplied` no longer holds the FK, so the strip that - * judges the other fields never re-asks it against the header the row - * keeps, where it could flip and land the FK after the rest were judged - * against the header it left. When the FK lands, that strip re-judges it - * on the same landing and reaches the same verdict. - * ② the static `readonly` strip (`staticReadonlyStripTakes`), which runs - * after the conditional one and takes a non-system caller's forged FK. - * - * Nothing else on either path moves the FK after this point, and what moves - * it before this point (a `beforeUpdate` hook, the post-hook payload) is - * already in `data`. Field-level security refuses a forbidden FK outright - * rather than stripping it, so it never leaves a payload that names one - * header and stores another. - * - * Returns: - * - `subject` — the payload the `readonlyWhen` strip judges next: `data`, - * or `data` without the FK when ①'s verdict locked it; - * - `supplied` — the entry snapshot that strip judges against: without the - * FK when the FK does not land (its verdict is in), whole when it does; - * - `view` — the payload the header id is read from: `data` when the FK - * lands, else `data` without it, so `masterIdOf` falls through to the - * prior row's FK; - * - `header` — what `resolve(view)` answers. ① needs the NAMED header only - * when judging the FK's lock reads `parent` — its own predicate, or - * [#19911] a `record`-reading one whose view another payload key's - * `parent`-scoped lock shapes (`readonlyWhenFkJudgementReadsParent`); - * when the FK then lands that is the same header and is reused, so the - * write still reads one header — two only when ① read the named header - * and the FK does not land. - */ private async settleMasterDetailLanding(args: { schema: unknown; data: Record; @@ -4027,7 +3726,7 @@ export class ObjectQL implements IObjectQLEngine { view: Record; header: H; }> { - // [#25522 cut 14] The body moved to `engine-master-detail-related.ts`; it reads no member of the class. + // [#25522 cut 14] The body moved to `engine-master-detail-related.ts`, its docblock with it; it reads no member of the class. return settleMasterDetailLanding(args); } @@ -4218,46 +3917,6 @@ export class ObjectQL implements IObjectQLEngine { }; } - /** - * [#22474] The master-detail headers a `validate()` preview binds, per row: - * `parents[i]` is the header row `i` is judged under (`parent`), and - * `previousParents[i]` the header its stored row hangs off when the row - * REPOINTS it (`previousParent`, for the ADR-0113 pre-check), else - * `undefined`, which the evaluator reads as "the same header", as the - * write's own `undefined` is read. - * - * The header ids are read the way the write reads them ({@link masterIdOf}): - * `views[i]` is the judged view, the row on an insert and the stored row - * merged with the patch on an update, so the patch's FK wins over the stored - * one; `storedRows[i]` names the header a repoint leaves. - * - * ## Read under the caller's access, not elevated - * - * A header read elevated here would answer a predicate over a row the caller - * may not read. Two reads, each answering one question, as the preview's - * stored-row read does — and, since #22519, as the write's own header read - * does ({@link resolveMasterDetailParent}): - * - * 1. ⛔ MAY THIS CALLER SEE THE HEADER — the READ DOOR, under the caller's - * own context, one `find` for the whole batch. A header it does not - * return binds as a missing header binds: `null`, so a predicate that - * reads `parent` refuses as unevaluable, whatever the header holds. A - * failed read binds every header as missing, as the write's own failed - * header read does. - * 2. WHAT THE WRITE WOULD JUDGE — the write's own header read - * ({@link resolveMasterDetailParents}, whose own read door returns the - * same headers), for the headers read 1 returned only. - * - * ⛔ The column rule is the preview's own; the write judges a header the - * caller can read on its stored values. A header column is kept only where - * the read door served this caller - * that very value ({@link servedAsStored}); every other column, hidden or - * served transformed (a partial mask keeps the key and replaces the value), - * is judged as empty: the header is materialised over the master's declared - * fields after the columns are kept, so such a column reads `null`. So the - * verdict never depends on a header value the caller could not have read in - * full. - */ private async resolvePreviewParents( schema: unknown, views: ReadonlyArray>, @@ -4267,7 +3926,7 @@ export class ObjectQL implements IObjectQLEngine { parents: Array | null>; previousParents: Array | null | undefined>; }> { - // [#25522 cut 14] The body moved to `engine-master-detail-related.ts` (see `MasterDetailRelatedHost`). + // [#25522 cut 14] The body moved to `engine-master-detail-related.ts`, its docblock with it (see `MasterDetailRelatedHost`). const readLogger = () => this.logger; const host: ThisParameterType = { find: (...a) => this.find(...a), @@ -4278,68 +3937,12 @@ export class ObjectQL implements IObjectQLEngine { return resolvePreviewParents.call(host, schema, views, storedRows, context); } - /** - * [#18682] Resolve the related rows this object's PREDICATE rules read one - * hop through a reference field, for a whole batch of rows at once. - * - * Returns a per-row lookup the validation seams hand to - * `evaluateValidationRules` as `related`. When no rule traverses anything the - * schema says so up front and this costs NOTHING — no query, no closure work - * — which is what bounds the N+1: one hop, only the fields a rule names, one - * batched read per reference field per write (two for a caller that is not - * SYSTEM where a read door bounds the rows, below), and only when a rule asks. - * - * ## Which related rows a caller's rule may read, and how their values are read - * - * The VALUES are read as SYSTEM: a validation rule's output is a pass/fail the - * SYSTEM enforces, not data handed to the caller — categorically unlike an - * access-control rule, which is why RLS predicates are excluded from this - * capability altogether. Reading the columns as the acting user instead made - * the rule unauthorable for exactly the persona it exists to constrain. What - * bounds that elevation is the PROJECTION: only the columns the predicate - * names, intersected with the related object's declared fields. ⛔ Never the - * whole row. - * - * Which ROWS a caller that is not SYSTEM may have read is a different - * question, answered before any value is read: - * - * - ⛔ Through a `master_detail` FK the related row is the detail's HEADER, - * and ruling A on #22519 holds here as on the write's own header read - * ({@link resolveMasterDetailParent}): an unreadable header is a header - * that exists nowhere (#22892). The caller's read door first, under the - * caller's own context ({@link masterDetailHeadersCallerReads}); a header - * it does not return is `unresolved`, exactly as a header that exists - * nowhere is, so the rule's answer never depends on what an unreadable - * header holds. The elevated, tenant-kept read follows for the headers the - * door returned only, and a header the caller CAN read is judged on its - * stored values, as the write judges it. - * - On a related object no organization wall scopes, the own read of the - * caller bounds the rows, and any other id is `unreadable`, stored or not. - * - ⛔ A plain `lookup` to a walled object keeps the tenant-kept elevated read - * alone: no ruling extends the header rule to it. - * - * A SYSTEM caller (seed replay, boot) asks no read door: the elevated read - * alone, as before. - * - * ## An unresolved row is left UNAVAILABLE, and it says which kind - * - * No reference stored, the related record not found, the related object - * declares no such column, or the row could not be read: each is its own reason, and - * {@link checkPredicate} turns it into a refusal naming the related object and - * column. The write is REJECTED rather than judged on a rule that produced no - * verdict. ⛔ Never silently true, and never silently false. - * - * The projection always names `id` alongside the fields the rules read: - * the map below is keyed on `row.id`, and a projection that omitted it would - * build an EMPTY map and leave every row unbound (#7537's shape, one seam - * over). - */ private async resolvePredicateRelated( schema: any, rows: ReadonlyArray | undefined | null>, context: unknown, ): Promise<((row: Record | undefined | null) => RelatedRecordBinding | undefined)> { - // [#25522 cut 14] The body moved to `engine-master-detail-related.ts` (see `MasterDetailRelatedHost`). + // [#25522 cut 14] The body moved to `engine-master-detail-related.ts`, its docblock with it (see `MasterDetailRelatedHost`). const readRegistry = () => this._registry; const host: ThisParameterType = { buildReferentialFieldClear: (...a) => this.buildReferentialFieldClear(...a), @@ -4349,36 +3952,6 @@ export class ObjectQL implements IObjectQLEngine { return resolvePredicateRelated.call(host, schema, rows, context); } - /** - * [#24197] Resolve the related rows a page of `rows` reads ONE hop through a - * reference field — the one loader behind both seams that serve a hop: - * validation predicates ({@link resolvePredicateRelated}, `authority: - * 'predicate'`) and formula fields ({@link resolveFormulaRelated}, - * `authority: 'caller'`). - * - * Everything but the AUTHORITY of the value read is shared, so the two seams - * cannot disagree about it: which reference fields and related fields to - * read ({@link collectReferenceTraversals}), the declared-field intersection - * and its `undeclared-field` report, the id set (one batched read per - * reference field per call, never one per row; a multi-value or already - * expanded slot names no single row), the `null` materialisation of a - * declared column the driver omitted, and the per-row - * {@link RelatedFieldBinding} with its reason when there is no row. - * - * ## The authority, and why the two differ - * - * - `predicate`: a rule's output is a pass/fail the SYSTEM enforces, so the - * values are read as SYSTEM behind the caller's read doors — see - * {@link resolvePredicateRelated} for the whole argument. - * - `caller`: a formula's output is DATA handed to the reader. Reading the - * hop as SYSTEM would hand any reader of the junction row a related - * column it could not read directly, so the hop is read under the - * caller's OWN context, through every enforcement layer `find` applies - * (row-level security, field-level security, the organization wall). A - * related row that read does not return is `unresolved` — whether it - * exists nowhere or the reader may not see it, the formula answers the - * same, so the answer discloses neither. - */ private async resolveOneHopRelated( schema: any, wanted: ReadonlyMap>, @@ -4386,7 +3959,7 @@ export class ObjectQL implements IObjectQLEngine { context: unknown, authority: 'predicate' | 'caller', ): Promise<((row: Record | undefined | null) => RelatedRecordBinding | undefined)> { - // [#25522 cut 14] The body moved to `engine-master-detail-related.ts` (see `MasterDetailRelatedHost`). + // [#25522 cut 14] The body moved to `engine-master-detail-related.ts`, its docblock with it (see `MasterDetailRelatedHost`). const readRegistry = () => this._registry; const readFormulaHopReads = () => this.formulaHopReads; const readLogger = () => this.logger; @@ -4631,52 +4204,6 @@ export class ObjectQL implements IObjectQLEngine { return SecretsAndRoutingFacet.setTenancyPostureProvider.call(secretsAndRoutingHost.of(this), provider); } - /** - * Normalize credential fields on `row` in place before it reaches the driver. - * - * Two channels share the read mask ({@link SECRET_MASK}) but differ on write - * (see ADR-0100): - * - * - **`secret`** — encrypted: each plaintext is wrapped by the ICryptoProvider - * and replaced on `row` by an opaque ref. Cleartext never reaches the - * business table. [#27495] The `sys_secret` row is SEALED here and handed - * back, not written: the caller writes it with {@link persistSecretRows} - * immediately before the record's own driver write, so a write refused - * after this pass (validation, the post-image seam) leaves no row behind. - * - **`password`** (generic, non-`better-auth`) — plaintext at rest: stored - * verbatim, no encryption and no `sys_secret` row. Only the echoed-mask drop - * below applies to it. - * - * Rules: - * - Any masked field (secret or password) whose value equals the read mask ⇒ - * the key is dropped, so a form round-trip that echoes the mask does not - * overwrite the stored value. - * - No secret fields on the object ⇒ no further work (fast path, no crypto). - * - `null`/`undefined` secret value ⇒ left as-is (clears the secret). - * - [#8559] `""` secret value ⇒ REFUSED with {@link EmptyCredentialWriteError} - * (ADR-0112 `VALIDATION_ERROR`/400, message names `null` as the clear - * spelling). Encrypting it would mint a `sys_secret` row that decrypts to - * nothing behind a perfectly valid ref, so every read reports "a secret is - * set" while the dereference returns nothing — the contradictory state - * #8542/#8558 defend consumers against. Refused, not folded into the - * `null` branch: silently reinterpreting the write was explicitly rejected - * (maintainer ruling 2026-08-13 on #8559). The `password` half of the same - * ruling lands at its own seam — {@link refuseEmptyPasswordFields} — since - * the two types share the read mask but not this encryption path. - * - Secret value already a ref ⇒ [#27582] a non-system write may only seal - * a new value or keep its own record's ref. From the system principal - * (`context.isSystem === true`) it is left as-is, as it always was. From - * any other caller it is kept only when every row the write lands on - * already holds exactly that ref in that column — read from `storedRows`, - * which each call site supplies (none for an insert, the pre-image for a - * by-id update, the matched rows for a predicate update) — and then the - * key is dropped, as the echoed mask is. Any other ref is REFUSED with - * {@link SecretRefWriteError} (ADR-0112 `VALIDATION_ERROR`/400), one - * answer whatever the ref names. - * - **Fail-closed:** any other secret value with no CryptoProvider registered, - * or no reachable `sys_secret` store, THROWS — never persists cleartext. - * (A `password` field needs no CryptoProvider — it is stored as-is.) - */ private async encryptSecretFields( object: string, row: Record, @@ -4684,7 +4211,7 @@ export class ObjectQL implements IObjectQLEngine { driverOptions: unknown, storedRows: StoredRowsReader, ): Promise { - // [#25522 cut 5] The body moved to `engine-secret-internal-fields.ts` (see `SecretInternalFieldsHost`). + // [#25522 cut 5] The body moved to `engine-secret-internal-fields.ts`, its docblock with it (see `SecretInternalFieldsHost`). const readRegistry = () => this._registry; const readCryptoProvider = () => this.cryptoProvider; const host: ThisParameterType = { @@ -4695,63 +4222,21 @@ export class ObjectQL implements IObjectQLEngine { return encryptSecretFields.call(host, object, row, context, driverOptions, storedRows); } - /** - * [#27495] Write the `sys_secret` rows {@link encryptSecretFields} sealed for - * one record write. Called immediately before that record's driver write and - * after every pass that can refuse it, so the credential row is written only - * with the record that names it. Each row lands on `written` as it is - * written, which is what the write's failure path hands to - * {@link releaseSecretHandles} when the record write does not land. - */ private async persistSecretRows(pending: readonly PendingSecretRow[], written: PendingSecretRow[]): Promise { - // [#27495] The body lives in `engine-secret-internal-fields.ts` (see `SecretInternalFieldsHost`). + // [#27495] The body lives in `engine-secret-internal-fields.ts`, its docblock with it (see `SecretInternalFieldsHost`). const host: ThisParameterType = { getDriver: (...a) => this.getDriver(...a), }; return persistSecretRows.call(host, pending, written); } - /** - * [#27495] Remove each `sys_secret` row in `slots` that the column it was - * minted for no longer names — best-effort, and never a throw. - * - * Two occasions reach it. `'refused'`: a write that sealed and wrote a row - * did not complete, so the row may name nothing. `'retired'`: a committed - * update cleared the field (`null`) or replaced its value, so the handle the - * column named before may name nothing; [#27581] a committed delete reaches - * it the same way, for every handle the removed rows held. The same three - * checks decide both, - * and every one that cannot pass KEEPS the row, the recoverable direction - * (deleting a handle a row still names destroys that credential, and nothing - * can name what was lost): - * - * - **Verified, never inferred.** No row of `object` may still name the - * handle in that column: a predicate write seals ONE handle for every row - * it matches, so clearing one of them leaves the handle in force for the - * rest; a write that did land after all (a partially applied batch) still - * names its row. - * - **This column's own handle.** The `sys_secret` row's `namespace`/`key` - * must be this object and field; a ref a system writer copied onto another - * column is not released through that column. - * - **Inside the record's unit.** The count and the delete ride the open - * transaction where it covers their drivers, so they see the write's own - * state and roll back with it; a record write in a unit that does not - * cover the `sys_secret` store keeps the row, since the delete could - * outlive a rollback of the record. - * - * A row kept for a failure (not for being in use) is reported once at - * `error`, naming the column and the remedy: the caller's answer does not - * change, and the stored ciphertext is exactly what the operator believes - * gone. Legacy orphans written before this existed are not swept here; the - * orphan report lists them. - */ private async releaseSecretHandles( object: string, slots: readonly SecretHandleSlot[], context: ExecutionContext | undefined, occasion: 'refused' | 'retired', ): Promise { - // [#27495] The body lives in `engine-secret-internal-fields.ts` (see `SecretInternalFieldsHost`). + // [#27495] The body lives in `engine-secret-internal-fields.ts`, its docblock with it (see `SecretInternalFieldsHost`). const readTxStore = () => this.txStore; const readLogger = () => this.logger; const host: ThisParameterType = { @@ -4764,31 +4249,8 @@ export class ObjectQL implements IObjectQLEngine { return releaseSecretHandles.call(host, object, slots, context, occasion); } - /** - * [#8559] Refuse `""` on a `password`-typed field before it reaches the - * driver — the `password` half of the same maintainer ruling (2026-08-13) - * that makes {@link encryptSecretFields} refuse `""` on a `secret` field. - * - * Its own seam, deliberately NOT routed through the encryption path: the two - * types share the read mask and the echoed-mask drop but a `password` field - * is plaintext at rest (no CryptoProvider, no `sys_secret` row — ADR-0100), - * so the refusal binds where its write actually flows. Storing `""` verbatim - * would produce the same contradiction as the secret arm without the cipher - * row: a non-null stored value that every read masks as "a password is set" - * while the credential has no content. `null` (clear) and the echoed mask - * (drop, handled in {@link encryptSecretFields}) keep their meanings. - * - * Scope is {@link collectMaskedPasswordFields}: `managedBy: 'better-auth'` - * objects are exempt — their password column is not masked on read, so the - * "reads as set, holds nothing" contradiction cannot arise there, and its - * write path belongs to the auth subsystem. - * - * Called at every call site of {@link encryptSecretFields} (single/bulk - * insert, single-id update, multi update) so the two halves of the ruling - * guard the same doors. - */ private refuseEmptyPasswordFields(object: string, row: Record): void { - // [#25522 cut 5] The body moved to `engine-secret-internal-fields.ts` (see `SecretInternalFieldsHost`). + // [#25522 cut 5] The body moved to `engine-secret-internal-fields.ts`, its docblock with it (see `SecretInternalFieldsHost`). const readRegistry = () => this._registry; const host: ThisParameterType = { get _registry() { return readRegistry(); }, @@ -4796,21 +4258,8 @@ export class ObjectQL implements IObjectQLEngine { return refuseEmptyPasswordFields.call(host, object, row); } - /** - * Mask credential fields on read so plaintext never leaves the engine through - * the normal query path. Covers `secret` fields (always) and `password` fields - * on generic, non-`better-auth` objects (see {@link collectMaskedReadFields} - * and ADR-0100). A set value becomes {@link SECRET_MASK}; an unset one stays - * `null`. Privileged callers that genuinely need a secret's plaintext use - * {@link resolveSecret} against the stored ref; a `password` field is stored - * as plaintext at rest, so its cleartext is only ever reachable off this path. - * - * [#7728] Second collector branch, same choke point: a field declared - * `internal: true` is OMITTED rather than masked — see - * {@link omitInternalFields} for why the two dispositions differ. - */ private maskSecretFields(object: string, rows: any): void { - // [#25522 cut 5] The body moved to `engine-secret-internal-fields.ts` (see `SecretInternalFieldsHost`). + // [#25522 cut 5] The body moved to `engine-secret-internal-fields.ts`, its docblock with it (see `SecretInternalFieldsHost`). const readRegistry = () => this._registry; const host: ThisParameterType = { get _registry() { return readRegistry(); }, @@ -4819,63 +4268,8 @@ export class ObjectQL implements IObjectQLEngine { return maskSecretFields.call(host, object, rows); } - /** - * [#7728] Drop every field declared `internal: true` from the rows the engine - * hands back — "the declared value is never returned on the generic data - * path". This is the read protection for ADR-0100's third credential channel: - * auth-subsystem one-way hashes stored in `text` columns, which the two - * type-keyed credential collectors structurally cannot reach. - * - * **OMIT, not mask** (maintainer ruling 2026-08-12 on #7728). The credential - * mask exists to signal "a value is set" without leaking it. The column this - * was minted for — `sys_api_key.key` — is `required: true`, so it is ALWAYS - * set: the signal carries zero bits, while still shipping a value under a - * field whose own description says it is "never exposed to clients". Omission - * also leaves that description string untouched, so the four generated - * translation bundles that mirror it do not churn. - * - * **`?select=` is covered by construction, and that is load-bearing.** The - * strip acts on the RESULT ROWS, not on the projection, so a client that - * spells the column out (`?select=id,key`) gets a 200 without it rather than - * a bypass. `select` only gates on whether a field is KNOWN - * (`assertProjectionFieldsExist`) and a flagged column is known, so a - * projection-aware strip would have shipped looking complete and still leaked - * to any caller who named the column — measured on the sibling column in - * #7823, and reproduced here on `sys_api_key.key` before the fix. - * - * **No system carve-out**, and this is where the shape deliberately diverges - * from its sibling {@link stripSearchCompanionFromRead}. That one keeps the - * `__search` companion for a system caller who names it by projection, - * because it has exactly one such reader whose backfill comparison would - * otherwise rewrite every row on every run. This flag has no such reader: the - * API-key verifier uses the column as a `where` FILTER and never reads it off - * the result (`resolveApiKeyPrincipal` takes `expires_at` / `user_id` / - * `organization_id` / `scopes`), and the mint path returns the plaintext it - * generated, not the row it inserted. An escape hatch nobody needs is a hole - * in a non-exposure guarantee, so there isn't one — if a legitimate system - * reader ever appears, it reads the column through a purpose-built privileged - * accessor, the way {@link resolveSecret} does for `secret`. - * - * Nothing below storage is touched. The strip runs on rows the driver has - * already produced, so the predicate has been evaluated and the index used - * before this method sees anything — which is precisely why authentication - * keeps working. - * - * **READ path only** (#7823, maintainer ruling 2026-08-13 / A-prime). This - * used to run at the two write-response sites as well (the 201 create body - * and the by-id-update 200 body), which conflated "never returned on the - * generic data path" with "never returned to the engine-level caller that - * performed the write" — and broke authentication for `sys_session.token`, - * whose minted value better-auth reads back off the insert result. The - * write-response half of the guarantee now lives at the generic-data-path - * ingress (`omitInternalFieldsFromWriteResponse` in - * `@objectstack/metadata-protocol`, tripwire-enforced across every `*Data` - * write face); engine write results keep the stored row whole. Lifecycle - * consumers that need a flagged value back off a READ go through - * {@link resolveInternalField} — never a carve-out here. - */ private omitInternalFields(object: string, rows: any): void { - // [#25522 cut 5] The body moved to `engine-secret-internal-fields.ts` (see `SecretInternalFieldsHost`). + // [#25522 cut 5] The body moved to `engine-secret-internal-fields.ts`, its docblock with it (see `SecretInternalFieldsHost`). const readRegistry = () => this._registry; const host: ThisParameterType = { get _registry() { return readRegistry(); }, @@ -4883,114 +4277,17 @@ export class ObjectQL implements IObjectQLEngine { return omitInternalFields.call(host, object, rows); } - /** - * [#7642] Strip the hidden `__search` companion column from what a read - * hands back, unless a SYSTEM caller named it in its projection. - * - * The column is declared client-invisible (`hidden` + `readonly` + `system` - * + `searchable: false`) and the enforcement that exists is real: it is kept - * out of auto-views, out of the `$search` auto-default, and a `$searchFields` - * override naming it is refused with a 400 ("is hidden"). What was missing is - * the PROJECTION half — a query that names no `fields` reaches the driver - * with `ast.fields` undefined, every driver answers that with `SELECT *`, and - * the companion rode back in every record body: list results, GET by id, - * `/search` hits (which are `engine.find` rows verbatim) and the 201 create - * body. The rule is applied HERE, at the engine, because the engine is the - * PRODUCER those four surfaces share; fixing them one consumer at a time is - * how three of the four would stay broken. - * - * Two carve-outs, both measured rather than defensive: - * - * - **A system caller that asks for it by name keeps it.** The companion has - * exactly one such reader: `plugin-pinyin-search`'s backfill/reconcile - * walk, which projects `['id', ...sources, '__search']` under - * `{ isSystem: true }` and compares the stored blob against the recomputed - * one. Strip it unconditionally and that comparison reads `undefined` - * every pass — the backfill would rewrite every row of every object on - * every run, which is worse than the disclosure it was fixing. - * - **A non-system caller does NOT keep it, even by name.** `select` only - * gates on whether a field is KNOWN (`assertProjectionFieldsExist`), and - * the companion is known once provisioned — so `?select=__search` would - * otherwise be an open door straight through this strip, and a - * client-invisibility rule with a documented spelling that bypasses it is - * not one. `isSystem` is server-derived (never client input), the same - * trust the read-only strips on the write path already place in it. - * - * ## Why this door is SILENT where the `$searchFields` door returns a 400 - * - * Asked on #7876 and ruled there on 2026-08-12 (direction C): the divergence - * is intended, and it is not reopenable on symmetry alone. The two doors are - * two KINDS of surface. - * - * - `$searchFields` is AUTHORING input — it tells the server how to RUN the - * query. A value the server will not honour has to be said out loud, or - * the caller gets a WIDER answer than the one they narrowed to, in a - * response with nothing to distinguish it from a satisfied one. That is - * why `assertSearchFieldsAreSearchable` refuses the name with a 400 - * (#4254) — the refusal is protecting the ROW SET. - * - `select` is a READ PROJECTION — it names what the caller would like - * back. Dropping a column the caller may not see leaves the answer - * correct: the rows are still the rows that were asked for, one key - * lighter. {@link omitInternalFields} directly above answers - * `?select=id,key` exactly this way, for exactly this reason (#7728), so - * silence here is the platform's existing read-path rule, not an - * exception to it. - * - * ⛔ Do not add a refusal here to make the two doors agree. That was option B - * on #7876, weighed and declined: it turns a request that answers 200 - * today into a failure, for tidiness, on a spelling no non-system caller in - * this tree uses — the companion's only deliberate reader is the backfill - * carved out above. If a REAL caller is ever burned by the silent drop, that - * measurement reopens it; the asymmetry by itself does not. - * - * ⚠️ `requestedFields` must be the CALLER's `fields`, captured before - * `planFormulaProjection` — that pass rewrites the projection to every stored - * column when a formula is in play, companion included. - */ private stripSearchCompanionFromRead( rows: unknown, requestedFields: readonly string[] | undefined, context: ExecutionContext | undefined, ): void { - // [#25522 cut 5] The body moved to `engine-secret-internal-fields.ts`; it reads no member. + // [#25522 cut 5] The body moved to `engine-secret-internal-fields.ts`, its docblock with it; it reads no member. stripSearchCompanionFromRead(rows, requestedFields, context); } - /** - * Driver options for a PRIVILEGED, driver-level read so it JOINS the open - * ambient transaction instead of asking the pool for a second connection. - * - * The three privileged read verbs — {@link resolveSecret}, - * {@link resolveSecretField}, {@link resolveInternalField} — deliberately - * read at DRIVER level, the only layer where the masked/omitted value still - * exists. That bypasses hooks, field-level security and sharing by design; - * what it must NOT bypass is the connection the surrounding transaction is - * holding. {@link buildDriverOptions} threads the ambient handle onto every - * ordinary read for that reason (ADR-0034); these three passed NO options at - * all, so their read went to a FRESH pooled connection. - * - * On a roomy pool that is invisible — the pool simply hands out a second - * connection. On a **single-connection pool it is a deadlock**: SQLite's knex - * pool is `max: 1` (`driver-sqlite-wasm` and `driver-sql`/better-sqlite3 - * both), so the open transaction holds the one connection and the privileged - * read waits for a connection that cannot be freed until the transaction - * that is waiting on the read commits. Measured on the erasure path - * (`runSubjectErasureAtomically` → better-auth `/admin/remove-user` → - * `reattachInternalFieldsOnRead` → `resolveInternalField`): the read blocked - * until knex's own acquire timeout fired ("Timeout acquiring a connection. - * The pool is probably full", from `Transaction_Sqlite.acquireConnection`), - * and the vendor route degraded that into an authentication refusal — a - * signed-in caller answered `401` after ~120 s, on a route reachable without - * credentials. Postgres/MySQL (`max >= 10`) never exhibited it. - * - * Reads only, and only the transaction: the same-origin gate (#5351) still - * decides whether the handle is this object's driver's to use, so a - * privileged read that resolves to a DIFFERENT datasource keeps its own - * connection rather than executing on the wrong one. Returns `undefined` - * when there is no ambient transaction, which is the pre-existing shape. - */ private privilegedReadDriverOptions(object: string): { transaction: unknown } | undefined { - // [#25522 cut 5] The body moved to `engine-secret-internal-fields.ts` (see `SecretInternalFieldsHost`). + // [#25522 cut 5] The body moved to `engine-secret-internal-fields.ts`, its docblock with it (see `SecretInternalFieldsHost`). const readTxStore = () => this.txStore; const host: ThisParameterType = { get txStore() { return readTxStore(); }, @@ -5211,23 +4508,8 @@ export class ObjectQL implements IObjectQLEngine { return SecretsAndRoutingFacet.isSystemLedgerObject.call(secretsAndRoutingHost.of(this), objectName); } - /** - * Helper to get the target driver - * - * Resolution priority (first match wins): - * 1. Object's explicit `datasource` field (if not 'default') - * 2. DatasourceMapping rules (namespace/package/pattern matching) - * 3. Lifecycle-class separation (ADR-0057 §3.6): telemetry/event/audit - * objects route to the dedicated 'telemetry' datasource when registered - * 4. Package's `defaultDatasource` from manifest - * 5. Global default driver - * - * The order itself lives in {@link resolveDatasourceBinding} — this method - * turns the name it decides on into a driver, and diagnoses the cases where - * that driver is missing. - */ private getDriver(objectName: string): IDataDriver { - // [#25522 cut 2] The body moved to `engine-datasource-routing.ts` (see `DatasourceRoutingHost`). + // [#25522 cut 2] The body moved to `engine-datasource-routing.ts`, its docblock with it (see `DatasourceRoutingHost`). const readDrivers = () => this.drivers; const readLogger = () => this.logger; const readUnavailableDatasources = () => this.unavailableDatasources; @@ -5304,17 +4586,11 @@ export class ObjectQL implements IObjectQLEngine { return SecretsAndRoutingFacet.resolveMappedDatasource.call(secretsAndRoutingHost.of(this), objectName); } - /** - * Resolve datasource from mapping rules - * - * Rules are evaluated in order (or by priority if specified). - * First matching rule wins. - */ private resolveDatasourceFromMapping( objectName: string, object?: any ): string | null { - // [#25522 cut 2] The body moved to `engine-datasource-routing.ts` (see `DatasourceRoutingHost`). + // [#25522 cut 2] The body moved to `engine-datasource-routing.ts`, its docblock with it (see `DatasourceRoutingHost`). const readDatasourceMapping = () => this.datasourceMapping; const host: ThisParameterType = { get datasourceMapping() { return readDatasourceMapping(); }, @@ -5323,12 +4599,8 @@ export class ObjectQL implements IObjectQLEngine { return resolveDatasourceFromMapping.call(host, objectName, object); } - /** - * Simple glob pattern matching - * Supports * (any chars) and ? (single char) - */ private matchPattern(objectName: string, pattern: string): boolean { - // [#25522 cut 2] The body moved to `engine-datasource-routing.ts`; it reads no member. + // [#25522 cut 2] The body moved to `engine-datasource-routing.ts`, its docblock with it; it reads no member. return matchPattern(objectName, pattern); } @@ -5565,18 +4837,8 @@ export class ObjectQL implements IObjectQLEngine { return MigrationStateFacet.isFileReferencesMigrationVerified.call(migrationStateHost.of(this)); } - /** - * [ADR-0131 D1] Hand a freshly-registered driver the question "is this - * database certified", the same closure-not-value seam as - * {@link supplyFileColumnsMovedResolver} and for the same reason: the answer - * is a `sys_migration` row on a store this driver has not connected to yet. - * A SQL driver asks it only when a table carries an index an earlier release - * built with the NULL-safe `COALESCE(, …)` key part, to decide - * whether that index already is the plain key new DDL emits. A driver with - * no such seam is left alone. - */ private supplyTenantKeyCertifiedResolver(driver: IDataDriver): void { - // [#25522 cut 13] The body moved to `engine-migration-flags.ts` (see `MigrationFlagsHost`). + // [#25522 cut 13] The body moved to `engine-migration-flags.ts`, its docblock with it (see `MigrationFlagsHost`). const readLogger = () => this.logger; const host: ThisParameterType = { isOrganizationOwnershipCertified: (...a) => this.isOrganizationOwnershipCertified(...a), @@ -5605,15 +4867,8 @@ export class ObjectQL implements IObjectQLEngine { return isOrganizationOwnershipCertified.call(host); } - /** - * The one memoized read both file-flag questions come off (#15989). - * - * Written once so the two public readers cannot drift apart in the ARGUMENTS - * they pass — the slot and the migration id decide which row is read, and a - * second copy of them is a second way to read a different row. - */ private readFileReferencesFlagRow(): Promise { - // [#25522 cut 13] The body moved to `engine-migration-flags.ts` (see `MigrationFlagsHost`). + // [#25522 cut 13] The body moved to `engine-migration-flags.ts`, its docblock with it (see `MigrationFlagsHost`). const host: ThisParameterType = { readMigrationFlagRowMemoized: (...a) => this.readMigrationFlagRowMemoized(...a), }; @@ -5705,26 +4960,11 @@ export class ObjectQL implements IObjectQLEngine { return result; } - /** - * Read one deployment migration flag and answer whether it authorises its - * consumers. Shared by both flags so the "every way of not knowing answers - * false" rule is written once: no `sys_migration` object registered, no row, - * an unreadable table, a malformed row — all `false`. Enforcement derives - * from evidence, and absent evidence is not permission. - * - * `conclusive` says whether the ledger was actually consulted, so the caller - * can tell "asked, and the answer is no" from "could not ask" — see - * {@link readMigrationFlagMemoized}. It never changes the verdict, only - * whether that verdict is worth remembering. - * - * Costs nothing on a kernel without the platform objects: the registry - * lookup short-circuits before any query. - */ private async readMigrationFlagVerified( migrationId: string, verifiedLog?: string, ): Promise { - // [#25522 cut 13] The body moved to `engine-migration-flags.ts` (see `MigrationFlagsHost`). + // [#25522 cut 13] The body moved to `engine-migration-flags.ts`, its docblock with it (see `MigrationFlagsHost`). const readRegistry = () => this._registry; const readLogger = () => this.logger; const host: ThisParameterType = { @@ -10817,71 +10057,12 @@ export class ObjectQL implements IObjectQLEngine { return CascadePlanningFacet.warnCascadeNotAtomic.call(cascadePlanningHost.of(this), object); } - /** - * [#9362] The dependents probe's `where`, spelled for the KIND of reference - * field it is aimed at. - * - * A single-valued `lookup` / `master_detail` stores a scalar foreign key, and - * bare equality is the right question about it. A MULTI-VALUED reference - * field stores an ARRAY, and every SQL backend in this repo puts that array - * in a JSON TEXT column. [#18408] Which of the two a field is, is - * {@link declaredMultiValued} — the one predicate the storage side asks - * (#17469) — ⛔ never a raw `field.multiple`: `master_detail` and `tree` are - * outside `MULTI_CAPABLE_TYPES`, so the flag on one of those buys a JSON - * column from nobody, and aiming `$contains` at the scalar column the driver - * really built is the mirror image of the defect below. - * - * Aiming bare equality at a multi-valued column compares - * the whole serialization (`["a","b"]`) against one id, which can never hold; - * `driver-sql` refuses the spelling outright (`INVALID_FILTER` / 400, #7398), - * and that refusal is correct and stays. Until this method existed the probe - * built bare equality for both kinds, so every object pointed at by any - * registered `multiple: true` lookup had its delete refused with a 400 — on - * the stock showcase, `showcase_account`, with an EMPTY dependent table: - * the fault is schema-driven, not data-driven, because the probe runs per - * DECLARED relation. - * - * The multi-value spelling is `$contains`, which is what the refusal itself - * prescribes and the one membership spelling every driver here answers. On a - * column the driver holds declared multi-valued it asks MEMBERSHIP, as - * `FILTER_OPERATORS`' `$contains` docblock (`@objectstack/spec`) declares for - * a JSON-stored column: `driver-sql` (and `driver-sqlite-wasm` and the local - * `driver-turso`, which extend it) compiles a per-dialect membership - * construct, `driver-memory` a mingo `$elemMatch`, and `driver-mongodb` the - * same `$elemMatch`. No public filter surface is widened: `$contains` is - * already declared, and this is the only construction site that changes. - * - * Membership answers EXACTLY on a well-formed slot, but a backend that does - * not hold the declaration (a table it was never told about) answers the - * SUBSTRING reading instead, which is a SUPERSET: with ids `acc_1` and - * `acc_10`, a row holding `acc_10` also matches a probe for `acc_1`. That is - * why the caller still narrows the rows exactly through - * {@link ObjectQL.storedReferenceIncludes} — over-matching here would make - * `cascade` DELETE and `set_null` clear rows that never referenced this - * record, which is worse than the 400 being fixed. The pushdown's only job is - * to keep the probe from reading the whole table. - * - * ⚠️ Membership is array-only on every backend that answers it, so a slot - * holding an off-shape bare scalar (the write door never stores one, it wraps - * a scalar; out-of-band data can) is not matched by the pushdown, and the - * scalar arm of {@link ObjectQL.storedReferenceIncludes} never sees that row. - * - * The `$or` limb covers the other direction — a FALSE NEGATIVE, which on an - * integrity guard is the fail-OPEN that #8895 ruled out. An id needing JSON - * escaping (a quote, a backslash) appears in the serialized text in its - * ESCAPED form, so a backend answering the substring reading over that text - * would miss the row for the raw id; a membership test, and a document or - * in-memory backend, compares the element itself and needs the RAW form. Both - * are asked whenever they differ, and the exact narrowing discards whatever - * the extra limb over-matched. Identical for an ordinary id, which is every - * id this engine generates. - */ private referenceProbeFilter( fieldName: string, fdef: { type?: unknown; multiple?: unknown }, id: string | number, ): Record { - // [#25522 cut 19] The body moved to `engine-delete-cascade.ts`. + // [#25522 cut 19] The body moved to `engine-delete-cascade.ts`, its docblock with it. return referenceProbeFilter(fieldName, fdef, id); } @@ -10904,48 +10085,6 @@ export class ObjectQL implements IObjectQLEngine { return CascadePlanningFacet.referenceCheckContext(context); } - /** - * [#12166, ruling constraint 3] File the reference-check elevation, BOTH - * halves: `triggeredBy` = the operator who asked for the delete, - * `executedAs: 'system'` = the identity the check actually ran under. - * - * ## Why a record exists at all - * - * The elevation is the whole of what this card changed, and an elevation - * nobody can see afterwards is the thing a security reviewer has no way to - * audit. The ruling names the Salesforce/Dataverse ledger shape for exactly - * this reason: a platform-executed integrity action is attributable to the - * human who triggered it, not laundered into "system did something". - * - * ## Why the LOGGER and not a `sys_audit_log` row - * - * Stated plainly, because a narrow audit edge has to be visible to be honest - * (`read-audit.ts` takes the same posture about its own narrowing): - * - * The elevated operation is a READ, and `plugin-audit`'s read writer declares - * — and pins — that a system-elevated read produces NO row, deliberately: - * those are the platform reading for its own bookkeeping, and recording them - * would bury the human views that ledger exists to make findable. Minting a - * row here would either contradict that declared boundary or require the - * engine to reach into a plugin it does not depend on. The engine's logger is - * the channel it owns, and it is where the reporting deployment read the - * defect in the first place — the issue's evidence is a server log line. - * - * So: this is a LOG record, at `info`, and the honest limit of it is that a - * deployment which does not retain engine logs retains no trace. A durable - * `sys_audit_log` row for the elevation is a follow-up that belongs to - * plugin-audit, which owns that row shape. - * - * `debug` was not an option: a record that is off by default in production is - * not an audit record. - * - * ⛔ The record names the referenced OBJECT and the relation FIELD — declared - * metadata — and never a row id, a field value, or a count. Constraint 2 - * governs what the CALLER is told; this is the server-side ledger and could - * defensibly say more, but saying more here is how a count reaches a support - * transcript, and the two surfaces are kept on one rule so neither has to be - * re-audited alone. - */ private recordReferenceCheckElevation( object: string, id: string | number, @@ -10953,7 +10092,7 @@ export class ObjectQL implements IObjectQLEngine { fieldName: string, context?: ExecutionContext, ): void { - // [#25522 cut 19] The body moved to `engine-delete-cascade.ts` (see `DeleteCascadeHost`). + // [#25522 cut 19] The body moved to `engine-delete-cascade.ts`, its docblock with it (see `DeleteCascadeHost`). const readLogger = () => this.logger; const host: ThisParameterType = { get logger() { return readLogger(); }, @@ -10991,35 +10130,6 @@ export class ObjectQL implements IObjectQLEngine { */ private static readonly REFERENCE_PROBE_BATCH = 500; - /** - * [#22305] Phase 1 of {@link cascadeDeleteRelations}: every record a by-id - * delete of `object`/`id` removes, the root included, collected READ-ONLY. - * - * Breadth-first across every relation whose resolved behaviour is - * `cascade`, read through the walk's own two readings — which relations - * point at an object ({@link cascadeRelationBehavior}) and which rows - * reference a record ({@link probeReferencingRows}: system identity, the - * same filter, the same missing-table and multi-value handling) — so the - * two phases cannot disagree about which rows a cascade reaches. A record - * already in the set is not expanded again, so a cycle in the data (a - * self-reference, two objects cascading into each other) terminates. - * - * The cost is one extra probe per CASCADING relation per member, since the - * walk probes those relations again as it deletes. `restrict` and - * `set_null` relations are not read here. Every probe here is an elevated - * read, so it files its #12166 record before it runs, through the - * once-per-record ledger the walk shares ({@link fileReferenceCheckElevation}). - * - * [#24964] …except for {@link inspectCascadeDelete}, which passes - * `fileElevations: false`: that record says "read as system FOR THE DELETE - * of" the record, and an inspection deletes nothing, so filing it there would - * put a delete that never happened in the log. The reads are the same. - * - * [#27004] From a root SET, level by level: the records found at one depth - * are probed together, per object and per relation, through - * {@link probeReferencingRowsOfSet}. The set and its edges are the ones the - * record-by-record walk collected. - */ private async collectCascadeDeleteSet( object: string, ids: ReadonlyArray, @@ -11027,7 +10137,7 @@ export class ObjectQL implements IObjectQLEngine { objects: ServiceObject[], opts: { fileElevations: boolean } = { fileElevations: true }, ): Promise { - // [#25522 cut 19] The body moved to `engine-delete-cascade.ts` (see `DeleteCascadeHost`). + // [#25522 cut 19] The body moved to `engine-delete-cascade.ts`, its docblock with it (see `DeleteCascadeHost`). const host: ThisParameterType = { cascadeRelationBehavior: (...a) => this.cascadeRelationBehavior(...a), fileReferenceCheckElevation: (...a) => this.fileReferenceCheckElevation(...a), @@ -11206,45 +10316,13 @@ export class ObjectQL implements IObjectQLEngine { return CascadePlanningFacet.walkReferencingRelations.call(cascadePlanningHost.of(this), object, ids, context, objects, cascadeSet); } - /** - * [#26718] Before the cascade writes anything: refuse the delete when an - * AUTHORED `restrict` is held by a row the cascade would remove only because - * the record that restrict protects is removed. - * - * The deleting walk judges a `restrict` against rows OUTSIDE the cascade set - * alone (#22305), and a row inside it was exempt whatever put it there. That - * exemption is right for a row the cascade reaches through ANOTHER record (a - * contract and the contact it restricts, both details of the account being - * deleted: the account's delete removes both, and neither is left dangling). - * It was wrong for a row reached THROUGH the very record it protects: a child - * holding a `master_detail` and a `restrict` lookup to the same parent is in - * that parent's set by its own master_detail, so the restrict was never - * asked and the cascade deleted the child — the record the author declared - * undeletable. A grandchild under that parent holding the restrict is the - * same case one level down. - * - * So for every member, every relation whose RESOLVED behaviour is `restrict` - * ({@link cascadeRelationBehavior}, an authored `restrict` on a lookup, - * tree or master_detail) is probed here when its object has a member in the - * reach of that member ({@link cascadeReachOf}); a referencing row in the - * reach refuses with the walk's own `DELETE_RESTRICTED` - * ({@link deleteRestrictedError}). Every restrict is judged before the - * first cascade, so the outcome no longer depends on the order the fields or - * objects are declared in, and nothing has been written when it refuses. - * - * ⛔ The required-lookup escalation (`set_null` on a required field read as - * `restrict`) is not judged here: it restricts because a required reference - * cannot be CLEARED, and a row the cascade deletes is never cleared. It keeps - * the #22305 rule. Neither does this replace the walk's judgement of rows - * outside the set, which is unchanged. - */ private async refuseRestrictsInsideOwnCascade( cascadeSet: CascadeDeleteSet, roots: ReadonlyArray<{ object: string; id: string | number }>, context: ExecutionContext | undefined, objects: ServiceObject[], ): Promise { - // [#25522 cut 19] The body moved to `engine-delete-cascade.ts` (see `DeleteCascadeHost`). + // [#25522 cut 19] The body moved to `engine-delete-cascade.ts`, its docblock with it (see `DeleteCascadeHost`). const host: ThisParameterType = { cascadeRelationBehavior: (...a) => this.cascadeRelationBehavior(...a), fileReferenceCheckElevation: (...a) => this.fileReferenceCheckElevation(...a), @@ -11255,13 +10333,6 @@ export class ObjectQL implements IObjectQLEngine { return refuseRestrictsInsideOwnCascade.call(host, cascadeSet, roots, context, objects); } - /** - * The `DELETE_RESTRICTED` refusal of deleting `object`/`id` because - * `dependents` of `childName` reference it via `fieldName`. One builder for - * the deleting walk ({@link walkReferencingRelations}) and the judgement - * before it ({@link refuseRestrictsInsideOwnCascade}), so the two answer one - * envelope. - */ private async deleteRestrictedError(args: { object: string; id: string | number; @@ -11276,7 +10347,7 @@ export class ObjectQL implements IObjectQLEngine { objects: ServiceObject[]; context: ExecutionContext | undefined; }): Promise { - // [#25522 cut 19] The body moved to `engine-delete-cascade.ts` (see `DeleteCascadeHost`). + // [#25522 cut 19] The body moved to `engine-delete-cascade.ts`, its docblock with it (see `DeleteCascadeHost`). const host: ThisParameterType = { validationMessageContext: (...a) => this.validationMessageContext(...a), dependentCountIsDisclosable: (...a) => this.dependentCountIsDisclosable(...a), diff --git a/scripts/codemod-split-engine.mjs b/scripts/codemod-split-engine.mjs index bccc3efadbb..6589dc98064 100644 --- a/scripts/codemod-split-engine.mjs +++ b/scripts/codemod-split-engine.mjs @@ -15,6 +15,10 @@ * # the second pass (#30052): plan (or apply) ObjectRepository and ScopedContext * node scripts/codemod-split-engine.mjs --scoped --verify --base * # re-prove the second pass on disk against engine.ts at + * node scripts/codemod-split-engine.mjs --hand-docs [--modules ] [--write] + * # the third pass (#30053): plan (or apply) the hand series' private docblocks + * node scripts/codemod-split-engine.mjs --hand-docs --verify --base + * # re-prove the third pass on disk against engine.ts and its hand modules at * node scripts/codemod-split-engine.mjs --self-test * node scripts/codemod-split-engine.mjs --help # this usage, on any tree (split or not) * @@ -136,6 +140,14 @@ * the class body may, a module function may not) and one that reads a * non-public static (`private-static`: the statics route is `ObjectQL`'s). * + * ## The third pass: the hand series' private docblocks (#30053) + * + * `--hand-docs` moves a private hand-series stub's docblock into its hand + * module, onto the function the stub calls, as the first two passes move a + * private method's docblock with its body; a public or protected one stays on + * the class. Its section below (`HAND_STUB_MARK`) states the rule, the slices + * (`--modules`) and the proof, the undo of every move. + * * ts-morph is a root devDependency, resolved from the root manifest, as the * pilot takes it. * @@ -2677,6 +2689,315 @@ async function mainScoped(tm, argv) { return 0; } +// ── The third pass: the hand series' private docblocks (#30053) ────────────── + +/** + * The hand series (#25522, and the two later hand cuts #27495 and #29460) + * moved its bodies out of `engine.ts` before this codemod existed, and kept + * EVERY method's docblock on the class: each function in a hand module + * carries the placeholder `placeholderDoc` writes instead. This codemod's own + * cut pattern moves a PRIVATE method's docblock with its body (a private + * member is no part of the published surface, so nothing outside the class + * reads it there) and keeps a public or protected one on the class, the + * contract. `--hand-docs` applies that rule to the hand series, and touches + * nothing else: + * + * - A hand-series stub is a method whose body opens with a hand marker + * (`HAND_STUB_MARK`), naming its hand module. + * - A private stub's docblock leaves the class and replaces its function's + * placeholder, de-indented one level (the hand series re-indented every + * body it moved, so the function stands at column 0); the stub's marker + * gains `HAND_DOC_MOVED`; the module's header docblock gains + * `HAND_DOCS_NOTE`, once, because its header says each docblock stays on + * the class. + * - A public or protected stub, a private stub with no docblock, and a stub + * whose leading text is anything but one docblock stay as they are. + * - `--modules a.ts,b.ts` moves only the stubs of the named hand modules, so + * the pass can land in slices; a stub already moved is skipped, so a later + * slice runs on an earlier one's output. + * + * The proof is the undo: putting every moved docblock back on its stub + * (re-indented), the placeholder back on its function, and taking the marker + * phrase and the header note out rebuilds the base `engine.ts` and every base + * hand module byte-for-byte. A one-character edit in a moved docblock, a block + * dropped on either side, a public docblock moved, or any other byte changed + * in those files fails it. The public `.d.ts` bar is a report reading on two + * built dists, as for the first two passes. + */ +const HAND_DOCS_CARD = '#30053'; + +/** The marker a hand-series stub's body opens with; group 1 is the module, group 2 the phrase once moved. */ +const HAND_STUB_MARK = /^ *\/\/ \[(?:#25522 cut \d+|#27495|#29460)\] The body (?:moved to|lives in) `(engine-[a-z0-9-]+\.ts)`(, its docblock with it)?/; + +/** The phrase a stub's marker gains after its module's backticked name. */ +const HAND_DOC_MOVED = `, ${DOC_MOVED}`; + +/** The paragraph a hand module's header docblock gains, above its closing line, when a docblock moves into it. */ +const HAND_DOCS_NOTE = [ + ' *', + ` * [${HAND_DOCS_CARD} codemod] A PRIVATE method's docblock has since moved here,`, + ' * onto its function (`--hand-docs` of `scripts/codemod-split-engine.mjs`),', + ' * and the class keeps none for it; a public or protected method\'s docblock', + ' * stays on the class, the contract, and its function keeps the placeholder.', +].join('\n'); + +/** One docblock and the indentation before the member, or null: the only leading text that moves. */ +function soleDocblock(lead) { + const m = /^(\/\*\*(?:(?!\*\/)[\s\S])*\*\/)\n( *)$/.exec(lead); + return m ? { doc: m[1], indent: m[2] } : null; +} + +/** The docblock one level out (two spaces), or null when a line is not indented that far. */ +function deindentDoc(doc) { + const lines = doc.split('\n'); + const out = [lines[0]]; + for (const line of lines.slice(1)) { + if (line === '') out.push(line); + else if (line.startsWith(' ')) out.push(line.slice(2)); + else return null; + } + return out.join('\n'); +} + +const reindentDoc = (doc) => doc.split('\n').map((line, i) => (i === 0 || line === '' ? line : ` ${line}`)).join('\n'); + +/** The header docblock's closing line offset in a module, or -1: the first `/**` before any import or export. */ +function headerCloseAt(text) { + const open = text.indexOf('/**'); + const firstStatement = text.search(/^(?:import|export) /m); + if (open < 0 || (firstStatement >= 0 && open > firstStatement)) return -1; + const close = text.indexOf('\n */\n', open); + return close < 0 || (firstStatement >= 0 && close > firstStatement) ? -1 : close; +} + +/** Every hand-series stub of the class, in source order, with what moving its docblock needs. */ +function handStubs(tm, engineText, className = CLASS_NAME) { + const sf = parse(tm, HOST_BASENAME, engineText); + const cls = sf.getClass(className); + if (!cls) refuse(`${HOST_BASENAME} declares no class ${className}`); + const stubs = []; + for (const m of cls.getMethods()) { + const body = m.getBody(); + if (!body) continue; + const line1Start = engineText.indexOf('\n', body.getStart()) + 1; + const line1 = engineText.slice(line1Start, engineText.indexOf('\n', line1Start)); + const hit = HAND_STUB_MARK.exec(line1); + if (!hit) continue; + stubs.push({ + node: m, name: m.getName(), scope: m.getScope(), module: hit[1], moved: Boolean(hit[2]), + lead: engineText.slice(m.getStart(true), m.getStart()), leadStart: m.getStart(true), memberStart: m.getStart(), + // The offset right after the module's closing backtick, where the phrase goes. + phraseAt: line1Start + hit.index + hit[0].length - (hit[2] ? hit[2].length : 0), + }); + } + return stubs; +} + +/** The exported function of that name in a hand module: its leading text and offsets, or null. */ +function handFunction(tm, file, text, name) { + const sf = parse(tm, file, text); + const fn = sf.getFunction(name); + if (!fn || !fn.isExported()) return null; + return { lead: text.slice(fn.getStart(true), fn.getStart()), leadStart: fn.getStart(true), start: fn.getStart() }; +} + +/** Text edits applied end to start; each is `{ start, end, text }`. */ +function applyEdits(text, edits) { + let out = text; + for (const e of [...edits].sort((a, b) => b.start - a.start)) out = out.slice(0, e.start) + e.text + out.slice(e.end); + return out; +} + +/** + * The plan: each hand stub's verdict. `modules` is the slice (null: every + * hand module); `moduleTexts` maps a hand module file to its text. + */ +function planHandDocs(tm, engineText, moduleTexts, { modules = null, className = CLASS_NAME } = {}) { + const stubs = handStubs(tm, engineText, className); + const known = new Set(stubs.map((s) => s.module)); + for (const file of modules ?? []) { + if (!known.has(file)) refuse(`--modules names ${file}, which no hand-series stub of ${className} delegates to (known: ${[...known].sort().join(', ')})`); + } + const records = []; + for (const s of stubs) { + const why = (() => { + if (s.moved) return 'already-moved'; + if (s.scope !== 'private') return `${s.scope} (its docblock is the contract and stays on the class)`; + if (s.lead === '') return 'no-docblock'; + if (modules && !modules.includes(s.module)) return 'not-in-slice'; + const sole = soleDocblock(s.lead); + if (!sole) return 'leading-text (anything but one docblock stays)'; + if (deindentDoc(sole.doc) === null) return 'indent (a docblock line is not indented one level)'; + return ''; + })(); + let fn = null; + let reason = why; + if (why === '') { + const text = moduleTexts.get(s.module); + if (text == null) refuse(`${s.module}: the hand module ${s.name} delegates to is not on disk`); + fn = handFunction(tm, s.module, text, s.name); + if (!fn) refuse(`${s.module}: exports no function ${s.name}, the body ${className}#${s.name}'s marker names`); + if (headerCloseAt(text) < 0) refuse(`${s.module}: no header docblock closing in a bare \` */\` line before its first import or export`); + // A function whose docblock says more than the placeholder keeps it, + // and the method keeps its own: merging two docblocks is an edit, not a move. + if (fn.lead !== `${placeholderDoc(s.name, className)}\n`) reason = 'function-docblock (its function carries more than the placeholder)'; + } + records.push({ ...s, reason, fn }); + } + return { records, movers: records.filter((r) => r.reason === '') }; +} + +/** Apply the plan: the new `engine.ts` and every hand module it changed. */ +function generateHandDocs(tm, engineText, moduleTexts, planned) { + const hostEdits = []; + const perModule = new Map(); + for (const r of planned.movers) { + hostEdits.push({ start: r.leadStart, end: r.memberStart, text: '' }); + hostEdits.push({ start: r.phraseAt, end: r.phraseAt, text: HAND_DOC_MOVED }); + const edits = perModule.get(r.module) ?? []; + edits.push({ start: r.fn.leadStart, end: r.fn.start, text: `${deindentDoc(soleDocblock(r.lead).doc)}\n` }); + perModule.set(r.module, edits); + } + const modules = new Map(); + for (const [file, edits] of [...perModule].sort(([a], [b]) => (a < b ? -1 : 1))) { + const text = moduleTexts.get(file); + if (!text.includes(HAND_DOCS_NOTE)) { + const at = headerCloseAt(text); + edits.push({ start: at, end: at, text: `\n${HAND_DOCS_NOTE}` }); + } + modules.set(file, applyEdits(text, edits)); + } + return { host: applyEdits(engineText, hostEdits), modules }; +} + +/** + * The proof. Undo every move the head's markers record and require the base + * `engine.ts` and the base hand modules back, byte-for-byte. `baseModules` and + * `headModules` map a hand module file to its text (a file absent from one + * side fails). + */ +function verifyHandDocs(tm, baseText, headText, baseModules, headModules, { className = CLASS_NAME } = {}) { + const failures = []; + const moved = []; + const hostEdits = []; + const perModule = new Map(); + for (const s of handStubs(tm, headText, className)) { + if (!s.moved) continue; + if (s.scope !== 'private') failures.push(`${s.name}: a ${s.scope} method's docblock moved off the class (only a private one may)`); + if (s.lead !== '') failures.push(`${s.name}: its stub says the docblock moved, but the class still carries leading text`); + const text = headModules.get(s.module); + const fn = text == null ? null : handFunction(tm, s.module, text, s.name); + if (!fn) { failures.push(`${s.name}: ${s.module} exports no function of that name`); continue; } + const sole = soleDocblock(fn.lead); + if (!sole || sole.indent !== '' || fn.lead === `${placeholderDoc(s.name, className)}\n`) { + failures.push(`${s.name}: its stub says the docblock moved, but its function in ${s.module} carries no moved docblock`); + continue; + } + const indent = /^ */.exec(headText.slice(headText.lastIndexOf('\n', s.memberStart) + 1, s.memberStart))[0]; + hostEdits.push({ start: s.memberStart, end: s.memberStart, text: `${reindentDoc(sole.doc)}\n${indent}` }); + hostEdits.push({ start: s.phraseAt, end: s.phraseAt + HAND_DOC_MOVED.length, text: '' }); + const edits = perModule.get(s.module) ?? []; + edits.push({ start: fn.leadStart, end: fn.start, text: `${placeholderDoc(s.name, className)}\n` }); + perModule.set(s.module, edits); + moved.push({ name: s.name, module: s.module, lines: sole.doc.split('\n').length }); + } + const firstDiff = (a, b) => { + const x = a.split('\n'); const y = b.split('\n'); + let i = 0; while (i < Math.max(x.length, y.length) && x[i] === y[i]) i += 1; + return `line ${i + 1}: base ${JSON.stringify(x[i] ?? null)} | undone head ${JSON.stringify(y[i] ?? null)}`; + }; + const rebuilt = applyEdits(headText, hostEdits); + if (rebuilt !== baseText) failures.push(`undoing the moves does NOT rebuild the base ${HOST_BASENAME} (${firstDiff(baseText, rebuilt)})`); + const files = [...new Set([...baseModules.keys(), ...headModules.keys()])].sort(); + for (const file of files) { + const base = baseModules.get(file); const head = headModules.get(file); + if (base == null || head == null) { failures.push(`${file}: only in the ${base == null ? 'head' : 'base'}`); continue; } + const edits = perModule.get(file) ?? []; + if (edits.length > 0 || head.includes(HAND_DOCS_NOTE)) { + const at = head.indexOf(`\n${HAND_DOCS_NOTE}\n */\n`); + if (at < 0) failures.push(`${file}: a docblock moved into it, but its header carries no note saying so before its closing line`); + else edits.push({ start: at, end: at + HAND_DOCS_NOTE.length + 1, text: '' }); + } + const undone = applyEdits(head, edits); + if (undone !== base) failures.push(`undoing the moves does NOT rebuild the base ${file} (${firstDiff(base, undone)})`); + } + return { failures, moved, modules: files.length, linesBefore: lineCount(baseText), linesAfter: lineCount(headText) }; +} + +function planReportHandDocs(planned) { + const out = [`# codemod-split-engine --hand-docs (${HAND_DOCS_CARD}): the hand series' private docblocks`, '']; + const byModule = new Map(); + for (const r of planned.movers) byModule.set(r.module, [...(byModule.get(r.module) ?? []), r]); + let total = 0; + for (const [file, rs] of [...byModule].sort(([a], [b]) => (a < b ? -1 : 1))) { + const lines = rs.reduce((n, r) => n + soleDocblock(r.lead).doc.split('\n').length, 0); + total += lines; + out.push(`- ${file}: ${rs.length} docblock(s), ${lines} line(s): ${rs.map((r) => r.name).join(', ')}`); + } + out.push('', `moves: ${planned.movers.length}/${planned.records.length} hand-series stubs, ${total} docblock line(s)`); + const stays = new Map(); + for (const r of planned.records) if (r.reason) stays.set(r.reason, [...(stays.get(r.reason) ?? []), r.name]); + for (const [reason, names] of stays) out.push(`stays (${reason}): ${names.length}: ${names.join(', ')}`); + return out.join('\n'); +} + +function printVerifyHandDocs(v) { + for (const f of v.failures) console.error(` ✗ ${f}`); + console.log(`hand docblocks: ${v.moved.length} moved (${v.moved.reduce((n, m) => n + m.lines, 0)} lines) across ${new Set(v.moved.map((m) => m.module)).size} module(s); ` + + `${HOST_BASENAME} ${v.linesBefore} -> ${v.linesAfter} lines; undoing the moves ${v.failures.some((f) => f.startsWith('undoing')) ? 'does NOT' : 'does'} rebuild the base ` + + `${HOST_BASENAME} and its ${v.modules} hand module(s) byte-for-byte`); +} + +/** The hand modules `engine.ts`'s hand stubs name, read through `read(file)` (null: absent). */ +function handModuleTexts(tm, engineText, read) { + const out = new Map(); + for (const file of [...new Set(handStubs(tm, engineText).map((s) => s.module))].sort()) { + const text = read(file); + if (text != null) out.set(file, text); + } + return out; +} + +/** `--hand-docs`: plan, write or re-prove the third pass. */ +async function mainHandDocs(tm, argv) { + if (argv.includes('--verify')) { + const i = argv.indexOf('--base'); + const ref = i >= 0 ? argv[i + 1] : null; + if (!ref) refuse('--hand-docs --verify needs --base '); + const baseText = gitShow(ref, `${SRC_REL}/${HOST_BASENAME}`); + const headText = readFileSync(HOST_PATH, 'utf8'); + const baseModules = handModuleTexts(tm, baseText, (f) => { try { return gitShow(ref, `${SRC_REL}/${f}`); } catch { return null; } }); + const headModules = handModuleTexts(tm, baseText, readSibling); + const v = verifyHandDocs(tm, baseText, headText, baseModules, headModules); + printVerifyHandDocs(v); + return v.failures.length === 0 ? 0 : 1; + } + const j = argv.indexOf('--modules'); + const modules = j >= 0 ? (argv[j + 1] ?? '').split(',').map((s) => s.trim()).filter(Boolean) : null; + if (j >= 0 && modules.length === 0) refuse('--modules needs a comma-separated list of hand module files'); + const input = readFileSync(HOST_PATH, 'utf8'); + const moduleTexts = handModuleTexts(tm, input, readSibling); + const planned = planHandDocs(tm, input, moduleTexts, { modules }); + console.log(planReportHandDocs(planned)); + const out = generateHandDocs(tm, input, moduleTexts, planned); + const files = new Map([[HOST_BASENAME, out.host], ...moduleTexts, ...out.modules]); + console.log(`output tree sha256: ${treeHash(files)} (input sha256 ${sha256(input)})`); + const v = verifyHandDocs(tm, input, out.host, moduleTexts, new Map([...moduleTexts, ...out.modules])); + printVerifyHandDocs(v); + if (v.failures.length > 0) { + console.error('✗ the generated moves fail the undo proof: nothing written. Report the fork; never hand-edit the moves.'); + return 1; + } + if (!argv.includes('--write')) { + console.log('plan only: nothing written (pass --write to apply).'); + return 0; + } + writeFileSync(HOST_PATH, out.host); + for (const [file, text] of out.modules) writeFileSync(join(SRC_DIR, file), text); + console.log(`✓ wrote ${HOST_BASENAME} and ${out.modules.size} hand module(s) under ${SRC_REL}`); + return 0; +} + // ── Entry ──────────────────────────────────────────────────────────────────── const USAGE = [ @@ -2684,6 +3005,8 @@ const USAGE = [ ' node scripts/codemod-split-engine.mjs --verify --base REF re-prove a split on disk against engine.ts at REF', ' node scripts/codemod-split-engine.mjs --scoped [--write] plan (or apply) the second pass: ObjectRepository and ScopedContext', ' node scripts/codemod-split-engine.mjs --scoped --verify --base REF re-prove the second pass on disk against engine.ts at REF', + ' node scripts/codemod-split-engine.mjs --hand-docs [--modules A.ts,B.ts] [--write] plan (or apply) the third pass: the hand series\' private docblocks', + ' node scripts/codemod-split-engine.mjs --hand-docs --verify --base REF re-prove the third pass on disk against engine.ts and its hand modules at REF', ' node scripts/codemod-split-engine.mjs --self-test', ' node scripts/codemod-split-engine.mjs --help', ].join('\n'); @@ -2708,6 +3031,7 @@ async function main(argv) { } const tm = await loadTsMorph(); if (argv.includes('--scoped')) return mainScoped(tm, argv); + if (argv.includes('--hand-docs')) return mainHandDocs(tm, argv); if (argv.includes('--verify')) { const i = argv.indexOf('--base'); const ref = i >= 0 ? argv[i + 1] : null; @@ -2780,6 +3104,7 @@ const SELF_TEST_BATTERIES = Object.freeze({ 'import prune': 4, 'refusals': 6, 'scoped classes (second pass)': 16, + 'hand-series docblocks (third pass)': 15, }); const SELF_TEST_BATTERY_FLOOR = 5; @@ -3031,6 +3356,100 @@ export class ScopedContext { `; } +function handFixtureEngine() { + return `import { privateDoc, publicDoc, noDoc, richer } from './engine-alpha.js'; +import { betaPrivate } from './engine-beta.js'; + +export class ObjectQL { + private n = 1; + + /** + * A private docblock: it moves. + * + * Second paragraph. + */ + private privateDoc(n: number): number { + // [#25522 cut 1] The body moved to \`engine-alpha.ts\` (see \`AlphaHost\`). + return privateDoc.call({ n: this.n }, n); + } + + /** A public docblock: it stays. */ + publicDoc(): number { + // [#25522 cut 1] The body moved to \`engine-alpha.ts\` (see \`AlphaHost\`). PUBLIC + return publicDoc.call({ n: this.n }); + } + + private noDoc(): number { + // [#25522 cut 1] The body moved to \`engine-alpha.ts\`; it reads no member. + return noDoc(); + } + + /** Its function says more than the placeholder. */ + private richer(): number { + // [#25522 cut 1] The body moved to \`engine-alpha.ts\`; it reads no member. + return richer(); + } + + /** A beta docblock. */ + private betaPrivate(): number { + // [#29460] The body lives in \`engine-beta.ts\`; it reads no member. + return betaPrivate(); + } + + /** Not a stub: no hand marker. */ + private notAStub(): number { + return this.n; + } +} +`; +} + +function handFixtureModules() { + const head = (what) => `// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * [#25522 cut 1] ${what}. + */ + +import type { ObjectQL } from './engine.js'; +`; + return new Map([ + ['engine-alpha.ts', `${head('The alpha bodies')} +export interface AlphaHost { readonly n: ObjectQL['n'] } + +${placeholderDoc('privateDoc')} +export function privateDoc(this: AlphaHost, n: number): number { + return this.n + n; +} + +${placeholderDoc('publicDoc')} +export function publicDoc(this: AlphaHost): number { + return this.n; +} + +${placeholderDoc('noDoc')} +export function noDoc(): number { + return 0; +} + +/** + * ${placeholderDoc('richer').slice(4, -3)} + * + * And a note of its own. + */ +export function richer(): number { + return 1; +} +`], + ['engine-beta.ts', `${head('The beta body')} +${placeholderDoc('betaPrivate')} +export function betaPrivate(): number { + return 2; +} +`], + ]); +} + async function selfTest() { const tm = await loadTsMorph(); const batterySeen = new Map(); @@ -3258,6 +3677,53 @@ async function selfTest() { refused(() => planScoped(tm, input, { classes: [['Gone', 'engine-gone.ts', 'Gone']] }))); } + open('hand-series docblocks (third pass)'); + { + const input = handFixtureEngine(); + const mods = handFixtureModules(); + const planned = planHandDocs(tm, input, mods); + const why = (name) => planned.records.find((r) => r.name === name)?.reason ?? ''; + check('a private stub with one docblock moves; public, undocumented and richer-function stubs stay', + why('privateDoc') === '' && why('betaPrivate') === '' && why('publicDoc').startsWith('public') && + why('noDoc') === 'no-docblock' && why('richer').startsWith('function-docblock') && why('notAStub') === '', + planned.records.map((r) => `${r.name}=${r.reason}`).join(' | ')); + const out = generateHandDocs(tm, input, mods, planned); + const again = generateHandDocs(tm, input, mods, planHandDocs(tm, input, mods)); + const alpha = out.modules.get('engine-alpha.ts') ?? ''; + check('the moved docblock replaces the placeholder, one level out, and the class keeps none', + alpha.includes('/**\n * A private docblock: it moves.\n *\n * Second paragraph.\n */\nexport function privateDoc(') && + !out.host.includes('A private docblock') && out.host.includes(' private privateDoc(n: number): number {\n')); + check('the stub\'s marker gains the phrase after its module', + out.host.includes(`// [#25522 cut 1] The body moved to \`engine-alpha.ts\`${HAND_DOC_MOVED} (see \`AlphaHost\`).`)); + check('a public docblock stays on the class and its function keeps the placeholder', + out.host.includes(' /** A public docblock: it stays. */\n publicDoc(') && alpha.includes(`${placeholderDoc('publicDoc')}\nexport function publicDoc(`)); + check('the module header gains the note once, above its closing line', + alpha.split(HAND_DOCS_NOTE).length === 2 && alpha.includes(`${HAND_DOCS_NOTE}\n */\n`)); + const v = verifyHandDocs(tm, input, out.host, mods, new Map([...mods, ...out.modules])); + check('the generated moves prove against their base', v.failures.length === 0 && v.moved.length === 2, v.failures.join(' | ')); + check('two runs on one input write byte-identical trees', + treeHash(new Map([['h', out.host], ...out.modules])) === treeHash(new Map([['h', again.host], ...again.modules]))); + const slice = generateHandDocs(tm, input, mods, planHandDocs(tm, input, mods, { modules: ['engine-alpha.ts'] })); + const sliceMods = new Map([...mods, ...slice.modules]); + const rest = generateHandDocs(tm, slice.host, sliceMods, planHandDocs(tm, slice.host, sliceMods)); + check('a slice moves only its modules, and the next slice on its output lands where one run lands', + !slice.modules.has('engine-beta.ts') && slice.host.includes('A beta docblock') && + rest.host === out.host && (rest.modules.get('engine-beta.ts') ?? '') === out.modules.get('engine-beta.ts') && + planHandDocs(tm, slice.host, sliceMods).records.find((r) => r.name === 'privateDoc')?.reason === 'already-moved'); + const fails = (host, edited) => verifyHandDocs(tm, input, host, mods, new Map([...mods, ...out.modules, ...edited])).failures.length > 0; + check('one edited character in a moved docblock fails', fails(out.host, [['engine-alpha.ts', alpha.replace('Second paragraph.', 'Second paragraph!')]])); + check('a moved docblock dropped from its function fails', fails(out.host, [['engine-alpha.ts', alpha.replace(/\/\*\*\n \* A private docblock[\s\S]*?\*\/\n/, '')]])); + check('a staying docblock dropped from the class fails', fails(out.host.replace(' /** A public docblock: it stays. */\n', ''), [])); + check('a public docblock moved off the class fails', fails( + out.host.replace(' /** A public docblock: it stays. */\n', '').replace('`engine-alpha.ts` (see `AlphaHost`). PUBLIC', `\`engine-alpha.ts\`${HAND_DOC_MOVED} (see \`AlphaHost\`). PUBLIC`), + [['engine-alpha.ts', alpha.replace(`${placeholderDoc('publicDoc')}\n`, '/** A public docblock: it stays. */\n')]])); + check('a marker that says moved over a docblock still on the class fails', fails( + out.host.replace('`engine-alpha.ts` (see `AlphaHost`). PUBLIC', `\`engine-alpha.ts\`${HAND_DOC_MOVED} (see \`AlphaHost\`). PUBLIC`), [])); + check('a module missing its header note fails', fails(out.host, [['engine-alpha.ts', alpha.replace(`\n${HAND_DOCS_NOTE}`, '')]])); + check('--modules naming a module no stub delegates to refuses', + refused(() => planHandDocs(tm, input, mods, { modules: ['engine-gone.ts'] }))); + } + const declared = Object.keys(SELF_TEST_BATTERIES); if (declared.length < SELF_TEST_BATTERY_FLOOR) { failed += 1; From 89308f35d68a020ef95cfb4b3c2bf1ffa0924f01 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 5 Nov 2026 05:24:19 +0000 Subject: [PATCH 2/7] fix(codemod-split-engine): prove a --hand-docs slice against a base an earlier slice landed in The proof now undoes every move in the head and in the base and compares the two, so the second slice's in-run proof holds on the first slice's output and against the unsliced base alike. One self-test case pins it. Co-Authored-By: Claude Claude-Session: https://claude.ai/code/session_01Utp6CYHBS7P5hkUQrNHkHZ --- scripts/codemod-split-engine.mjs | 95 +++++++++++++++++++++----------- 1 file changed, 64 insertions(+), 31 deletions(-) diff --git a/scripts/codemod-split-engine.mjs b/scripts/codemod-split-engine.mjs index 6589dc98064..045e047a56c 100644 --- a/scripts/codemod-split-engine.mjs +++ b/scripts/codemod-split-engine.mjs @@ -2719,10 +2719,11 @@ async function mainScoped(tm, argv) { * The proof is the undo: putting every moved docblock back on its stub * (re-indented), the placeholder back on its function, and taking the marker * phrase and the header note out rebuilds the base `engine.ts` and every base - * hand module byte-for-byte. A one-character edit in a moved docblock, a block - * dropped on either side, a public docblock moved, or any other byte changed - * in those files fails it. The public `.d.ts` bar is a report reading on two - * built dists, as for the first two passes. + * hand module byte-for-byte (a base an earlier slice landed in is undone the + * same way first, so a slice is proved on its own). A one-character edit in a + * moved docblock, a block dropped on either side, a public docblock moved, or + * any other byte changed in those files fails it. The public `.d.ts` bar is a + * report reading on two built dists, as for the first two passes. */ const HAND_DOCS_CARD = '#30053'; @@ -2871,21 +2872,23 @@ function generateHandDocs(tm, engineText, moduleTexts, planned) { } /** - * The proof. Undo every move the head's markers record and require the base - * `engine.ts` and the base hand modules back, byte-for-byte. `baseModules` and - * `headModules` map a hand module file to its text (a file absent from one - * side fails). + * Undo every move a tree's markers record: each moved docblock back on its + * stub (re-indented), the placeholder back on its function, the marker phrase + * and the header note out. `moduleTexts` maps a hand module file to its text. + * Returns the undone texts, the moves it undid, and every way the tree breaks + * the rule (a public docblock moved, a marker over a docblock still on the + * class, a function with no moved docblock, a header with no note). */ -function verifyHandDocs(tm, baseText, headText, baseModules, headModules, { className = CLASS_NAME } = {}) { +function undoHandDocs(tm, engineText, moduleTexts, className = CLASS_NAME) { const failures = []; const moved = []; const hostEdits = []; const perModule = new Map(); - for (const s of handStubs(tm, headText, className)) { + for (const s of handStubs(tm, engineText, className)) { if (!s.moved) continue; if (s.scope !== 'private') failures.push(`${s.name}: a ${s.scope} method's docblock moved off the class (only a private one may)`); if (s.lead !== '') failures.push(`${s.name}: its stub says the docblock moved, but the class still carries leading text`); - const text = headModules.get(s.module); + const text = moduleTexts.get(s.module); const fn = text == null ? null : handFunction(tm, s.module, text, s.name); if (!fn) { failures.push(`${s.name}: ${s.module} exports no function of that name`); continue; } const sole = soleDocblock(fn.lead); @@ -2893,7 +2896,7 @@ function verifyHandDocs(tm, baseText, headText, baseModules, headModules, { clas failures.push(`${s.name}: its stub says the docblock moved, but its function in ${s.module} carries no moved docblock`); continue; } - const indent = /^ */.exec(headText.slice(headText.lastIndexOf('\n', s.memberStart) + 1, s.memberStart))[0]; + const indent = /^ */.exec(engineText.slice(engineText.lastIndexOf('\n', s.memberStart) + 1, s.memberStart))[0]; hostEdits.push({ start: s.memberStart, end: s.memberStart, text: `${reindentDoc(sole.doc)}\n${indent}` }); hostEdits.push({ start: s.phraseAt, end: s.phraseAt + HAND_DOC_MOVED.length, text: '' }); const edits = perModule.get(s.module) ?? []; @@ -2901,27 +2904,49 @@ function verifyHandDocs(tm, baseText, headText, baseModules, headModules, { clas perModule.set(s.module, edits); moved.push({ name: s.name, module: s.module, lines: sole.doc.split('\n').length }); } + const modules = new Map(); + for (const [file, text] of moduleTexts) { + const edits = perModule.get(file) ?? []; + if (edits.length > 0 || text.includes(HAND_DOCS_NOTE)) { + const at = text.indexOf(`\n${HAND_DOCS_NOTE}\n */\n`); + if (at < 0) failures.push(`${file}: a docblock moved into it, but its header carries no note saying so above its closing line`); + else edits.push({ start: at, end: at + HAND_DOCS_NOTE.length + 1, text: '' }); + } + modules.set(file, applyEdits(text, edits)); + } + return { host: applyEdits(engineText, hostEdits), modules, moved, failures }; +} + +/** + * The proof: undoing every move in the head lands on exactly what undoing + * every move in the base lands on, `engine.ts` and each hand module byte for + * byte. A base no slice has touched undoes to itself, so against it this is + * "the undo rebuilds the base"; against a base an earlier slice landed in, it + * proves this slice without re-proving that one's text. `baseModules` and + * `headModules` map a hand module file to its text (a file absent from one + * side fails). + */ +function verifyHandDocs(tm, baseText, headText, baseModules, headModules, { className = CLASS_NAME } = {}) { + const head = undoHandDocs(tm, headText, headModules, className); + const base = undoHandDocs(tm, baseText, baseModules, className); + const failures = [...head.failures, ...base.failures.map((f) => `base: ${f}`)]; const firstDiff = (a, b) => { const x = a.split('\n'); const y = b.split('\n'); let i = 0; while (i < Math.max(x.length, y.length) && x[i] === y[i]) i += 1; - return `line ${i + 1}: base ${JSON.stringify(x[i] ?? null)} | undone head ${JSON.stringify(y[i] ?? null)}`; + return `line ${i + 1}: undone base ${JSON.stringify(x[i] ?? null)} | undone head ${JSON.stringify(y[i] ?? null)}`; }; - const rebuilt = applyEdits(headText, hostEdits); - if (rebuilt !== baseText) failures.push(`undoing the moves does NOT rebuild the base ${HOST_BASENAME} (${firstDiff(baseText, rebuilt)})`); + if (head.host !== base.host) failures.push(`undoing the moves does NOT rebuild the base ${HOST_BASENAME} (${firstDiff(base.host, head.host)})`); const files = [...new Set([...baseModules.keys(), ...headModules.keys()])].sort(); for (const file of files) { - const base = baseModules.get(file); const head = headModules.get(file); - if (base == null || head == null) { failures.push(`${file}: only in the ${base == null ? 'head' : 'base'}`); continue; } - const edits = perModule.get(file) ?? []; - if (edits.length > 0 || head.includes(HAND_DOCS_NOTE)) { - const at = head.indexOf(`\n${HAND_DOCS_NOTE}\n */\n`); - if (at < 0) failures.push(`${file}: a docblock moved into it, but its header carries no note saying so before its closing line`); - else edits.push({ start: at, end: at + HAND_DOCS_NOTE.length + 1, text: '' }); - } - const undone = applyEdits(head, edits); - if (undone !== base) failures.push(`undoing the moves does NOT rebuild the base ${file} (${firstDiff(base, undone)})`); - } - return { failures, moved, modules: files.length, linesBefore: lineCount(baseText), linesAfter: lineCount(headText) }; + const b = base.modules.get(file); const h = head.modules.get(file); + if (b == null || h == null) { failures.push(`${file}: only in the ${b == null ? 'head' : 'base'}`); continue; } + if (h !== b) failures.push(`undoing the moves does NOT rebuild the base ${file} (${firstDiff(b, h)})`); + } + const baseMoved = new Set(base.moved.map((m) => m.name)); + return { + failures, moved: head.moved.filter((m) => !baseMoved.has(m.name)), baseMoved: base.moved.length, + modules: files.length, linesBefore: lineCount(baseText), linesAfter: lineCount(headText), + }; } function planReportHandDocs(planned) { @@ -2943,9 +2968,9 @@ function planReportHandDocs(planned) { function printVerifyHandDocs(v) { for (const f of v.failures) console.error(` ✗ ${f}`); - console.log(`hand docblocks: ${v.moved.length} moved (${v.moved.reduce((n, m) => n + m.lines, 0)} lines) across ${new Set(v.moved.map((m) => m.module)).size} module(s); ` + - `${HOST_BASENAME} ${v.linesBefore} -> ${v.linesAfter} lines; undoing the moves ${v.failures.some((f) => f.startsWith('undoing')) ? 'does NOT' : 'does'} rebuild the base ` + - `${HOST_BASENAME} and its ${v.modules} hand module(s) byte-for-byte`); + console.log(`hand docblocks: ${v.moved.length} moved (${v.moved.reduce((n, m) => n + m.lines, 0)} lines) across ${new Set(v.moved.map((m) => m.module)).size} module(s), ${v.baseMoved} already moved at base; ` + + `${HOST_BASENAME} ${v.linesBefore} -> ${v.linesAfter} lines; undoing every move ${v.failures.some((f) => f.startsWith('undoing')) ? 'does NOT rebuild' : 'rebuilds'} the base, ` + + `undone the same way, for ${HOST_BASENAME} and its ${v.modules} hand module(s) byte-for-byte`); } /** The hand modules `engine.ts`'s hand stubs name, read through `read(file)` (null: absent). */ @@ -3104,7 +3129,7 @@ const SELF_TEST_BATTERIES = Object.freeze({ 'import prune': 4, 'refusals': 6, 'scoped classes (second pass)': 16, - 'hand-series docblocks (third pass)': 15, + 'hand-series docblocks (third pass)': 16, }); const SELF_TEST_BATTERY_FLOOR = 5; @@ -3710,6 +3735,14 @@ async function selfTest() { !slice.modules.has('engine-beta.ts') && slice.host.includes('A beta docblock') && rest.host === out.host && (rest.modules.get('engine-beta.ts') ?? '') === out.modules.get('engine-beta.ts') && planHandDocs(tm, slice.host, sliceMods).records.find((r) => r.name === 'privateDoc')?.reason === 'already-moved'); + { + const restMods = new Map([...sliceMods, ...rest.modules]); + const onSlice = verifyHandDocs(tm, slice.host, rest.host, sliceMods, restMods); + const onBase = verifyHandDocs(tm, input, rest.host, mods, restMods); + check('the next slice proves against the earlier slice\'s output, counting only its own moves, and against the unsliced base', + onSlice.failures.length === 0 && onSlice.moved.length === 1 && onSlice.baseMoved === 1 && onBase.failures.length === 0 && onBase.moved.length === 2, + [...onSlice.failures, ...onBase.failures].join(' | ')); + } const fails = (host, edited) => verifyHandDocs(tm, input, host, mods, new Map([...mods, ...out.modules, ...edited])).failures.length > 0; check('one edited character in a moved docblock fails', fails(out.host, [['engine-alpha.ts', alpha.replace('Second paragraph.', 'Second paragraph!')]])); check('a moved docblock dropped from its function fails', fails(out.host, [['engine-alpha.ts', alpha.replace(/\/\*\*\n \* A private docblock[\s\S]*?\*\/\n/, '')]])); From 9f387b197233e46015050f6f05d9dd279ab58133 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 5 Nov 2026 05:30:48 +0000 Subject: [PATCH 3/7] fix(codemod-split-engine): prune an import only a moved docblock linked, and prove the prune noUnusedLocals counts a JSDoc {@link X} as a read, so five engine.ts import bindings went unused once their docblocks moved. --hand-docs now prunes them (and the // lines directly above a declaration that goes whole); the proof recomputes the prune from the head and requires every pruned name to be linked by a moved docblock. Re-run on main a26abbc6: the tree equals a fresh --write there. Co-Authored-By: Claude Claude-Session: https://claude.ai/code/session_01Utp6CYHBS7P5hkUQrNHkHZ --- packages/objectql/src/engine.ts | 7 +--- scripts/codemod-split-engine.mjs | 70 +++++++++++++++++++++++++------- 2 files changed, 57 insertions(+), 20 deletions(-) diff --git a/packages/objectql/src/engine.ts b/packages/objectql/src/engine.ts index 1a868d5635c..5c6a86fb071 100644 --- a/packages/objectql/src/engine.ts +++ b/packages/objectql/src/engine.ts @@ -281,9 +281,9 @@ import { declaredOptionLabels } from './formula-option-labels.js'; import { bindHooksToEngine, type BindHooksOptions } from './hook-binder.js'; import { validateRecord, validateRecordInScope, normalizeMultiValueFields, normalizeBlankTypedValues, normalizeNumericStringValues, coerceBooleanFields, ValidationError } from './validation/record-validator.js'; import type { AdmittedValueShapeViolation, AdmittedValueShapeViolationSink, ValidationMessageContext } from './validation/record-validator.js'; -import type { RelatedFieldBinding, RelatedRecordBinding } from './validation/rule-validator.js'; +import type { RelatedRecordBinding } from './validation/rule-validator.js'; import { createBatchSummaryAccumulator } from './summary-batch-accumulation.js'; -import { collectReferenceTraversals, evaluateValidationRules, emitValidationAdvisories, optionVisibilityReadsPermissions, readsPermissionPredicate, needsPriorRecord, stripReadonlyWhenFields, stripReadonlyWhenFieldsMulti, hasReadonlyWhenInPayload, hasParentScopedReadonlyWhenInPayload, hasParentScopedRequiredWhen, stripReadonlyFields, stripRuntimeOwnedFields, staticReadonlyInsertSubject, preserveAuditIgnoredOnInsertWarning } from './validation/rule-validator.js'; +import { evaluateValidationRules, emitValidationAdvisories, needsPriorRecord, stripReadonlyWhenFields, stripReadonlyWhenFieldsMulti, hasReadonlyWhenInPayload, hasParentScopedReadonlyWhenInPayload, hasParentScopedRequiredWhen, stripReadonlyFields, stripRuntimeOwnedFields, staticReadonlyInsertSubject, preserveAuditIgnoredOnInsertWarning } from './validation/rule-validator.js'; // [#14088] The before-phase write recorder — the provenance channel the static // `readonly` strip needs to tell a hook's write from a caller's echo of the // SAME value. Armed and sealed in `update()`; the module owns the argument for @@ -333,9 +333,6 @@ import { resolveOneHopRelated, resolvePredicateRelated, resolvePreviewParents, s // (`ReadableIdsPermissionFormulaHost`). Imported after every module those bodies import, so // the engine's module evaluation order is what it was. import { composeReadableIdsChain, findUnservableControlledByParentMasters, permissionResolution, readableIdsScopeOf, reportFormulaFault, resolveDefaultPermissions, resolveFormulaPermissions, resolveOptionPermissions } from './engine-readable-ids-permission-formula.js'; -// [#25522 cut 16] Type-only, for the `{@link}`s on `composeReadableIdsAtBoundary`'s and -// `composeReadableIdsChain`'s docblocks: the constant moved with its reader in cut 15. -import type { READABLE_IDS_MAX_CHAIN_DEPTH } from './engine-readable-ids-permission-formula.js'; // [#25522 cut 19] The delete cascade's collection walk, the in-cascade // `restrict` judge, the `DELETE_RESTRICTED` refusal, the reference-probe // filter and the reference-check elevation record; the methods of the same diff --git a/scripts/codemod-split-engine.mjs b/scripts/codemod-split-engine.mjs index 045e047a56c..16b69bf4bdb 100644 --- a/scripts/codemod-split-engine.mjs +++ b/scripts/codemod-split-engine.mjs @@ -1641,10 +1641,12 @@ function usedImportNames(tm, text) { /** * Drop every import binding `used` lacks. A declaration left with no binding * goes whole (its own text and the line break after it; the comments above it - * stay); one left with some keeps them, a one-line `{ … }` on one line, a - * multi-line one as one binding per line. Pure. Returns `{ text, removed }`. + * stay, unless `dropAttachedComments`: then the `//` lines directly above it, + * with no blank line between, go with it); one left with some keeps them, a + * one-line `{ … }` on one line, a multi-line one as one binding per line. + * Pure. Returns `{ text, removed }`. */ -function pruneImports(tm, text, used) { +function pruneImports(tm, text, used, { dropAttachedComments = false } = {}) { const ts = tm.ts; const sf = parseSourceFile(HOST_PATH, text); const edits = []; @@ -1666,7 +1668,18 @@ function pruneImports(tm, text, used) { const keepNamed = named.filter((e) => used.has(e.name.text)); if (!keepDefault && !keepNs && keepNamed.length === 0) { const end = text[st.getEnd()] === '\n' ? st.getEnd() + 1 : st.getEnd(); - edits.push({ start: st.getStart(sf), end, text: '' }); + let start = st.getStart(sf); + if (dropAttachedComments) { + // Walk up the `//` lines that touch the declaration (no blank line between). + for (;;) { + const prevEnd = start - 1; + if (prevEnd <= 0 || text[prevEnd] !== '\n') break; + const lineStart = text.lastIndexOf('\n', prevEnd - 1) + 1; + if (!/^\s*\/\/.*$/.test(text.slice(lineStart, prevEnd))) break; + start = lineStart; + } + } + edits.push({ start, end, text: '' }); continue; } if (nb && ts.isNamedImports(nb) && dropNamed.length > 0) { @@ -2720,10 +2733,14 @@ async function mainScoped(tm, argv) { * (re-indented), the placeholder back on its function, and taking the marker * phrase and the header note out rebuilds the base `engine.ts` and every base * hand module byte-for-byte (a base an earlier slice landed in is undone the - * same way first, so a slice is proved on its own). A one-character edit in a - * moved docblock, a block dropped on either side, a public docblock moved, or - * any other byte changed in those files fails it. The public `.d.ts` bar is a - * report reading on two built dists, as for the first two passes. + * same way first, so a slice is proved on its own), once the import bindings + * only a moved docblock's `{@link}` read (`noUnusedLocals` counts a link) are + * pruned from the base with the `//` lines directly above a declaration that + * goes whole; every pruned name must be one a moved docblock links. A + * one-character edit in a moved docblock, a block dropped on either side, a + * public docblock moved, or any other byte changed in those files fails it. + * The public `.d.ts` bar is a report reading on two built dists, as for the + * first two passes. */ const HAND_DOCS_CARD = '#30053'; @@ -2868,7 +2885,16 @@ function generateHandDocs(tm, engineText, moduleTexts, planned) { } modules.set(file, applyEdits(text, edits)); } - return { host: applyEdits(engineText, hostEdits), modules }; + const moved = applyEdits(engineText, hostEdits); + // A JSDoc `{@link X}` is a read `noUnusedLocals` counts, so an import binding + // only a moved docblock linked is dropped, with the `//` lines that explain it. + const { text: host, removed: pruned } = pruneImports(tm, moved, usedImportNames(tm, moved), { dropAttachedComments: true }); + return { host, modules, pruned }; +} + +/** The names a docblock links (`{@link X}`, `{@linkcode X}`, `@see X`; the leftmost name of a dotted one). */ +function linkedNames(doc) { + return new Set([...doc.matchAll(/\{@link(?:code|plain)?\s+([A-Za-z_$][\w$]*)|@see\s+([A-Za-z_$][\w$]*)/g)].map((m) => m[1] ?? m[2])); } /** @@ -2902,7 +2928,7 @@ function undoHandDocs(tm, engineText, moduleTexts, className = CLASS_NAME) { const edits = perModule.get(s.module) ?? []; edits.push({ start: fn.leadStart, end: fn.start, text: `${placeholderDoc(s.name, className)}\n` }); perModule.set(s.module, edits); - moved.push({ name: s.name, module: s.module, lines: sole.doc.split('\n').length }); + moved.push({ name: s.name, module: s.module, lines: sole.doc.split('\n').length, doc: sole.doc }); } const modules = new Map(); for (const [file, text] of moduleTexts) { @@ -2935,7 +2961,13 @@ function verifyHandDocs(tm, baseText, headText, baseModules, headModules, { clas let i = 0; while (i < Math.max(x.length, y.length) && x[i] === y[i]) i += 1; return `line ${i + 1}: undone base ${JSON.stringify(x[i] ?? null)} | undone head ${JSON.stringify(y[i] ?? null)}`; }; - if (head.host !== base.host) failures.push(`undoing the moves does NOT rebuild the base ${HOST_BASENAME} (${firstDiff(base.host, head.host)})`); + // The one edit the undo does not read back from the head is the import + // prune: recomputed from the head and applied to the base, and every binding + // it drops must be a name a moved docblock links. + const { text: prunedBase, removed: pruned } = pruneImports(tm, base.host, usedImportNames(tm, headText), { dropAttachedComments: true }); + const links = new Set(head.moved.flatMap((m) => [...linkedNames(m.doc)])); + for (const name of pruned) if (!links.has(name)) failures.push(`${name}: the head no longer imports it, and no moved docblock links it`); + if (head.host !== prunedBase) failures.push(`undoing the moves does NOT rebuild the base ${HOST_BASENAME} (${firstDiff(prunedBase, head.host)})`); const files = [...new Set([...baseModules.keys(), ...headModules.keys()])].sort(); for (const file of files) { const b = base.modules.get(file); const h = head.modules.get(file); @@ -2944,7 +2976,7 @@ function verifyHandDocs(tm, baseText, headText, baseModules, headModules, { clas } const baseMoved = new Set(base.moved.map((m) => m.name)); return { - failures, moved: head.moved.filter((m) => !baseMoved.has(m.name)), baseMoved: base.moved.length, + failures, moved: head.moved.filter((m) => !baseMoved.has(m.name)), baseMoved: base.moved.length, pruned, modules: files.length, linesBefore: lineCount(baseText), linesAfter: lineCount(headText), }; } @@ -2970,7 +3002,8 @@ function printVerifyHandDocs(v) { for (const f of v.failures) console.error(` ✗ ${f}`); console.log(`hand docblocks: ${v.moved.length} moved (${v.moved.reduce((n, m) => n + m.lines, 0)} lines) across ${new Set(v.moved.map((m) => m.module)).size} module(s), ${v.baseMoved} already moved at base; ` + `${HOST_BASENAME} ${v.linesBefore} -> ${v.linesAfter} lines; undoing every move ${v.failures.some((f) => f.startsWith('undoing')) ? 'does NOT rebuild' : 'rebuilds'} the base, ` + - `undone the same way, for ${HOST_BASENAME} and its ${v.modules} hand module(s) byte-for-byte`); + `undone the same way, for ${HOST_BASENAME} and its ${v.modules} hand module(s) byte-for-byte, once the ${v.pruned.length} import binding(s) ` + + `only a moved docblock linked are pruned from the base (${v.pruned.join(', ') || 'none'})`); } /** The hand modules `engine.ts`'s hand stubs name, read through `read(file)` (null: absent). */ @@ -3129,7 +3162,7 @@ const SELF_TEST_BATTERIES = Object.freeze({ 'import prune': 4, 'refusals': 6, 'scoped classes (second pass)': 16, - 'hand-series docblocks (third pass)': 16, + 'hand-series docblocks (third pass)': 18, }); const SELF_TEST_BATTERY_FLOOR = 5; @@ -3384,6 +3417,8 @@ export class ScopedContext { function handFixtureEngine() { return `import { privateDoc, publicDoc, noDoc, richer } from './engine-alpha.js'; import { betaPrivate } from './engine-beta.js'; +// Type-only, for the link on betaPrivate's docblock. +import type { LinkedOnly } from './engine-beta.js'; export class ObjectQL { private n = 1; @@ -3415,7 +3450,7 @@ export class ObjectQL { return richer(); } - /** A beta docblock. */ + /** A beta docblock, {@link LinkedOnly}. */ private betaPrivate(): number { // [#29460] The body lives in \`engine-beta.ts\`; it reads no member. return betaPrivate(); @@ -3753,6 +3788,11 @@ async function selfTest() { check('a marker that says moved over a docblock still on the class fails', fails( out.host.replace('`engine-alpha.ts` (see `AlphaHost`). PUBLIC', `\`engine-alpha.ts\`${HAND_DOC_MOVED} (see \`AlphaHost\`). PUBLIC`), [])); check('a module missing its header note fails', fails(out.host, [['engine-alpha.ts', alpha.replace(`\n${HAND_DOCS_NOTE}`, '')]])); + check('an import only a moved docblock links is pruned with the // lines above it (not before its docblock moves), and the proof names it', + out.pruned.join() === 'LinkedOnly' && v.pruned.join() === 'LinkedOnly' && !out.host.includes('LinkedOnly }') && + !out.host.includes('for the link on') && slice.host.includes('for the link on'), `${out.pruned} | ${v.pruned}`); + check('a binding the head dropped that no moved docblock links fails', + fails(out.host.replace("import { betaPrivate } from './engine-beta.js';\n", ''), [])); check('--modules naming a module no stub delegates to refuses', refused(() => planHandDocs(tm, input, mods, { modules: ['engine-gone.ts'] }))); } From 0afee7a99749a5bb948f27946b954cff1b1acceb Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 5 Nov 2026 05:36:25 +0000 Subject: [PATCH 4/7] test(codemod-split-engine): pin the hand-module half of the --hand-docs proof An ablation of the per-module comparison stayed green: every case so far edited a moved docblock, which the engine.ts half catches. One case now edits a body byte in a hand module. Co-Authored-By: Claude Claude-Session: https://claude.ai/code/session_01Utp6CYHBS7P5hkUQrNHkHZ --- scripts/codemod-split-engine.mjs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/scripts/codemod-split-engine.mjs b/scripts/codemod-split-engine.mjs index 16b69bf4bdb..1b19f34e328 100644 --- a/scripts/codemod-split-engine.mjs +++ b/scripts/codemod-split-engine.mjs @@ -3162,7 +3162,7 @@ const SELF_TEST_BATTERIES = Object.freeze({ 'import prune': 4, 'refusals': 6, 'scoped classes (second pass)': 16, - 'hand-series docblocks (third pass)': 18, + 'hand-series docblocks (third pass)': 19, }); const SELF_TEST_BATTERY_FLOOR = 5; @@ -3780,6 +3780,7 @@ async function selfTest() { } const fails = (host, edited) => verifyHandDocs(tm, input, host, mods, new Map([...mods, ...out.modules, ...edited])).failures.length > 0; check('one edited character in a moved docblock fails', fails(out.host, [['engine-alpha.ts', alpha.replace('Second paragraph.', 'Second paragraph!')]])); + check('one edited character elsewhere in a hand module fails', fails(out.host, [['engine-alpha.ts', alpha.replace('return this.n + n;', 'return this.n - n;')]])); check('a moved docblock dropped from its function fails', fails(out.host, [['engine-alpha.ts', alpha.replace(/\/\*\*\n \* A private docblock[\s\S]*?\*\/\n/, '')]])); check('a staying docblock dropped from the class fails', fails(out.host.replace(' /** A public docblock: it stays. */\n', ''), [])); check('a public docblock moved off the class fails', fails( From a7fe548832bd6aded1d392abd84a1ddc5c17a2a8 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 5 Nov 2026 05:39:33 +0000 Subject: [PATCH 5/7] docs(changeset): the slice's engine.ts line counts on main a26abbc6 Co-Authored-By: Claude Claude-Session: https://claude.ai/code/session_01Utp6CYHBS7P5hkUQrNHkHZ --- .changeset/30053-hand-series-private-docblocks.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.changeset/30053-hand-series-private-docblocks.md b/.changeset/30053-hand-series-private-docblocks.md index a2c1be480bd..d0f7e39b43f 100644 --- a/.changeset/30053-hand-series-private-docblocks.md +++ b/.changeset/30053-hand-series-private-docblocks.md @@ -8,5 +8,5 @@ Clause-②: no Metadata-protocol: none - A new third pass of the `engine.ts` codemod, `--hand-docs`, moves a private hand-series stub's docblock onto the function its stub calls, as the first two passes already do for the bodies they move. A public or protected method keeps its docblock on the class. -- This is the first of two slices: 8 hand modules here, the other 9 next. `engine.ts` goes from 13,544 to 12,615 lines. +- This is the first of two slices: 8 hand modules here, the other 9 next. `engine.ts` goes from 13,550 to 12,618 lines. - Nothing to do on upgrade: the exported names, the public `.d.ts` surface and what every method does are the same. From b09479c5ddb37e675eeb246ea187ed0adfe8fb25 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 5 Nov 2026 05:40:39 +0000 Subject: [PATCH 6/7] fix(codemod-split-engine): --hand-docs reads serveReferenceNames' untagged marker as a hand stub Its marker is 'The body lives in' with no tag, so the pass saw 85 of the 86 hand stubs. The fixture's beta stub now carries the untagged form. Co-Authored-By: Claude Claude-Session: https://claude.ai/code/session_01Utp6CYHBS7P5hkUQrNHkHZ --- scripts/codemod-split-engine.mjs | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/scripts/codemod-split-engine.mjs b/scripts/codemod-split-engine.mjs index 1b19f34e328..7e1e722cba8 100644 --- a/scripts/codemod-split-engine.mjs +++ b/scripts/codemod-split-engine.mjs @@ -2744,8 +2744,12 @@ async function mainScoped(tm, argv) { */ const HAND_DOCS_CARD = '#30053'; -/** The marker a hand-series stub's body opens with; group 1 is the module, group 2 the phrase once moved. */ -const HAND_STUB_MARK = /^ *\/\/ \[(?:#25522 cut \d+|#27495|#29460)\] The body (?:moved to|lives in) `(engine-[a-z0-9-]+\.ts)`(, its docblock with it)?/; +/** + * The marker a hand-series stub's body opens with: `[#25522 cut N]` or + * `[#27495]` then "The body moved to", or #29460's untagged "The body lives + * in". Group 1 is the module, group 2 the phrase once moved. + */ +const HAND_STUB_MARK = /^ *\/\/ (?:\[(?:#25522 cut \d+|#27495)\] )?The body (?:moved to|lives in) `(engine-[a-z0-9-]+\.ts)`(, its docblock with it)?/; /** The phrase a stub's marker gains after its module's backticked name. */ const HAND_DOC_MOVED = `, ${DOC_MOVED}`; @@ -3452,7 +3456,7 @@ export class ObjectQL { /** A beta docblock, {@link LinkedOnly}. */ private betaPrivate(): number { - // [#29460] The body lives in \`engine-beta.ts\`; it reads no member. + // The body lives in \`engine-beta.ts\`; it reads no member. return betaPrivate(); } From 078b4d5cff570c3aed420c2d1d92be51b55f0028 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 5 Nov 2026 07:55:58 +0000 Subject: [PATCH 7/7] fix(spec): repoint the query.searchFields liveness row at engine-find.ts#expandSearchOnAst The cited engine.ts no longer names searchFields: the private docblock that did moved into its hand module with the first hand-docs slice. The body has lived in engine-find.ts since the codemod split. Co-Authored-By: Claude Claude-Session: https://claude.ai/code/session_01Utp6CYHBS7P5hkUQrNHkHZ --- .changeset/30053-liveness-search-fields-repoint.md | 11 +++++++++++ packages/spec/liveness/query.json | 4 ++-- 2 files changed, 13 insertions(+), 2 deletions(-) create mode 100644 .changeset/30053-liveness-search-fields-repoint.md diff --git a/.changeset/30053-liveness-search-fields-repoint.md b/.changeset/30053-liveness-search-fields-repoint.md new file mode 100644 index 00000000000..5b1898bf5b9 --- /dev/null +++ b/.changeset/30053-liveness-search-fields-repoint.md @@ -0,0 +1,11 @@ +--- +'@objectstack/spec': patch +--- + +fix(spec): the liveness ledger's `query.searchFields` row cites `engine-find.ts#expandSearchOnAst`, where the hand-off lives, so `check:liveness` is green again + +Clause-②: no +Metadata-protocol: none + +- The row cited `engine.ts#expandSearchOnAst`. The body has lived in `engine-find.ts` since the `engine.ts` codemod split, and the last mention of `searchFields` in `engine.ts`, a private docblock, moved out with the first slice of the hand-series docblock move, so the check found no mention of the key in the cited file. +- Nothing to do on upgrade: only the shipped `liveness/query.json` evidence text changes. diff --git a/packages/spec/liveness/query.json b/packages/spec/liveness/query.json index 031403a1fae..24f3f3c0ad1 100644 --- a/packages/spec/liveness/query.json +++ b/packages/spec/liveness/query.json @@ -29,8 +29,8 @@ "searchFields": { "status": "live", "verifiedAt": "2026-08-28", - "note": "Formally declared by #3899 (ADR-0061 P1, 'formalize $searchFields'): the per-query narrowing override the executor has served since the ADR landed — server-intersected with the object's allowed searchable set, can only narrow, never widen. The schema-side declaration caught up with the executor when #3899's entry validation started rejecting bodies the schema did not describe. 2026-08-28: RE-ANCHORED (commit 8cb96ec41) and REPOINTED — `engine.ts:2893-2898` had rotted onto `const row = rows[index];`, and the `search-filter.ts` leg was PATH-ONLY. Both halves of a two-hop claim (`requestedFields hand-off` → `resolveSearchFields`) were unfalsifiable in different ways: one bounded to a line that had stopped meaning anything, the other bounded to nothing. Re-closed by hand against 93ea19bca.", - "evidence": "packages/objectql/src/engine.ts#expandSearchOnAst (the hand-off — the requested fields ride into the expansion); packages/objectql/src/search-filter.ts#resolveSearchFields (the intersection with the object's allowed searchable set: narrows only, never widens); proof showcase-search.dogfood.test.ts + search-conformance.ledger.ts row search-fields-override" + "note": "Formally declared by #3899 (ADR-0061 P1, 'formalize $searchFields'): the per-query narrowing override the executor has served since the ADR landed — server-intersected with the object's allowed searchable set, can only narrow, never widen. The schema-side declaration caught up with the executor when #3899's entry validation started rejecting bodies the schema did not describe. 2026-08-28: RE-ANCHORED (commit 8cb96ec41) and REPOINTED — `engine.ts:2893-2898` had rotted onto `const row = rows[index];`, and the `search-filter.ts` leg was PATH-ONLY. Both halves of a two-hop claim (`requestedFields hand-off` → `resolveSearchFields`) were unfalsifiable in different ways: one bounded to a line that had stopped meaning anything, the other bounded to nothing. Re-closed by hand against 93ea19bca. 2026-11-05: REPOINTED to `engine-find.ts#expandSearchOnAst`, where the body has lived since the engine.ts codemod split: the first slice of #30053 moved the private docblock that named `$searchFields` out of engine.ts, which no longer names the key.", + "evidence": "packages/objectql/src/engine-find.ts#expandSearchOnAst (the hand-off — the requested fields ride into the expansion); packages/objectql/src/search-filter.ts#resolveSearchFields (the intersection with the object's allowed searchable set: narrows only, never widens); proof showcase-search.dogfood.test.ts + search-conformance.ledger.ts row search-fields-override" }, "orderBy": { "status": "live",