Skip to content

feat(driver-sql,driver-turso): aggregate() publishes its declared return type, not any (#17277) - #17689

Merged
os-musk merged 2 commits into
mainfrom
claude/issue-17277-sql-aggregate-return-type
Sep 11, 2026
Merged

feat(driver-sql,driver-turso): aggregate() publishes its declared return type, not any (#17277)#17689
os-musk merged 2 commits into
mainfrom
claude/issue-17277-sql-aggregate-return-type

Conversation

@os-musk

@os-musk os-musk commented Sep 11, 2026

Copy link
Copy Markdown
Collaborator

Fixes #17277

Clause-②: yes

What this changes

IDataDriver declares the door as

aggregate?(object: string, query: DriverQuery, options?: DriverOptions): Promise<Record<string, unknown>[]>

(packages/spec/src/contracts/data-driver.ts). SqlDriver.aggregate published Promise<any> over it, and TursoDriver overrides the door with its own Promise<any> — two sites, not one, because an override re-declares the door in its own package's .d.ts and picks up nothing from a driver-sql-only fix.

Both annotations are now the contract's own. Proven at the artifact level, in the emitted declarations after pnpm --filter @objectstack/driver-sql --filter @objectstack/driver-turso build:

packages/drivers/driver-sql/dist/index.d.ts:3414
    aggregate(object: string, query: DriverQuery, options?: DriverOptions): Promise<Record<string, unknown>[]>;
packages/drivers/driver-turso/dist/index.d.ts:1670
    aggregate(object: string, query: DriverQuery, options?: DriverOptions): Promise<Record<string, unknown>[]>;

Each door is pinned at the type level inside its own package's tsc program, extending the family files #15267 landed (sql-driver-doors-declared-types.test.ts, turso-driver-doors-declared-types.test.ts) rather than starting new ones — both halves per door, so putting an annotation back to Promise<any> reds the file twice (IsAny flips to true, Equals to false). No runtime behaviour changes.

Optionality is not material. aggregate is declared aggregate? where #15267's five doors are required. The ? governs whether the member EXISTS, not what it returns once it does — the engine's own dispatch (typeof driver.aggregate === 'function') hands the caller a function whose published return was any and is now the contract's record array. The contract half of each pin reads the member through NonNullable, exactly as explain already did.

Why #15267's census missed this door

SqlDriver.aggregate carried its own code comment asserting the opposite:

   * `IDataDriver` declares and #6075 followed through on five drivers;
   * `aggregate` is not on that contract, so neither reached it.

That sentence was false, and it is the entire mechanism: #15267's census asked "is aggregate on the contract?" and answered from this comment instead of from packages/spec/src/contracts/data-driver.ts. The reading then travelled — into the census, into the card, and into the dispatch order for #15267, which repeated it as a ruling. The implementer followed the order exactly and was right to; nothing in the chain re-measured it against the contract.

The comment is corrected at the source site in this PR, and the mechanism is recorded in both pin files, where the next person extending this family will read it. A comment is not the contract.

The re-run census (triage ordered this; it is the real deliverable)

Predicate built from data-driver.ts itself, never from a code comment: parse the IDataDriver interface, enumerate every member it declares, then compare each driver class's own published return-type annotation against the declaration.

  • Population: 35 members declared on IDataDriver (32 methods, 3 properties).
  • Classes audited: 6 production driver classes — InMemoryDriver, MongoDBDriver, SqlDriver, SqliteWasmDriver, TursoDriver, RemoteTransport. Test-only probe subclasses excluded.
  • Cells: 210 (35 × 6).
  • Tally before this PR: EXACT 101 · MASKED 13 · DIFFERENT 4 · INFERRED 18 · ABSENT 74.
  • Tally after this PR: EXACT 103 · MASKED 11 · DIFFERENT 4 · INFERRED 18 · ABSENT 74.

MASKED = the published annotation contains any where the declaration does not. Control: the predicate returns non-zero (13 before, 11 after) and independently re-derives aggregate on both SQL drivers, the two doors this card was filed for; the zero-cases are visible in the same table (EXACT rows), so the predicate is shown to have covered the honest doors rather than skipped them.

⚠️ It turned up more, and the answer is "yes, a seventh — in fact four more families". Every row below is still masked after this PR, and none is in scope here:

class door declared published site
SqlDriver find Promise<Record<string, unknown>[]> Promise<any[]> driver-sql/src/sql-driver.ts
SqlDriver upsert Promise<Record<string, unknown>> Promise<Record<string, any>> driver-sql/src/sql-driver.ts
SqlDriver bulkUpdate Promise<Record<string, unknown>[]> Promise<Record<string, any>[]> driver-sql/src/sql-driver.ts
SqlDriver temporalFilterValue unknown any driver-sql/src/sql-driver.ts
TursoDriver find (override) Promise<Record<string, unknown>[]> Promise<any[]> driver-turso/src/turso-driver.ts
TursoDriver upsert (override) Promise<Record<string, unknown>> Promise<Record<string, any>> driver-turso/src/turso-driver.ts
TursoDriver bulkUpdate (override) Promise<Record<string, unknown>[]> Promise<Record<string, any>[]> driver-turso/src/turso-driver.ts
TursoDriver beginTransaction (override) Promise<unknown> Promise<any> driver-turso/src/turso-driver.ts
RemoteTransport beginTransaction Promise<unknown> Promise<any> driver-turso/src/remote-transport.ts
InMemoryDriver aggregate Promise<Record<string, unknown>[]> Promise<any[]> driver-memory/src/memory-driver.ts
InMemoryDriver bulkCreate Promise<Record<string, unknown>[]> Promise<Record<string, any>[]> driver-memory/src/memory-driver.ts

The driver-memory rows are named as findings only. That package is under an investment freeze whose exception channel is a triage naming, so escalating them is triage's act and neither this PR's nor its dispatching seat's.

find, bulkUpdate and temporalFilterValue are named in neither #15267's repaired set nor its deliberately-excluded set — they were never seen. upsert, aggregate and beginTransaction were seen and excluded by name. The spelling correlates exactly: every door #15267 repaired is annotated with the literal string Promise<any>, while every door it never named carries the any nested inside a wider type (Promise<any[]>, Promise<Record<string, any>>, Promise<Record<string, any>[]>, bare any) — which a literal-string predicate does not match. Reported, deliberately not widened into: whether that remainder is one card or a bigger PR is the dispatching seat's call, not this PR's.

Two rows the predicate flags as DIFFERENT rather than MASKED are not findings, recorded so the table is readable: SqlDriver.beginTransaction publishes Promise<Knex.Transaction> and MongoDBDriver.beginTransaction publishes Promise<ClientSession> — both narrower than the declared Promise<unknown>, which is the honest direction.

Blast radius of the un-masking

18 dereferences, every one in the drivers' own tests, none in shipped source:

  • 9 in @objectstack/driver-sql (sql-driver-advanced.test.ts, sql-driver-queryast.test.ts)
  • 9 in @objectstack/driver-sqlite-wasm (sqlite-wasm-driver-advanced.test.ts, sqlite-wasm-driver-queryast.test.ts) — that package does not override the door and inherits it, so the narrowing reaches its callers through driver-sql's .d.ts
  • 0 in @objectstack/driver-turso

All 18 are the identical shape and the identical defect: result.find((r: any) => …) returns T | undefined, and through Promise<any> the whole expression came back any, so a cell read off the row compiled with no check at all. expect(x).toBeDefined() does not narrow. Each site now asserts the row arm with vitest's assert() — a narrowing assertion, not a ! and not a cast — matching the idiom #15267 established for the same class.

Nothing was re-masked to quiet tsc.

Verification

All readings below are against d1d6952e26, the final commit on this branch.

run result
pnpm --filter @objectstack/driver-sql --filter @objectstack/driver-turso typecheck exit 0 — 9 TS18048 before the consumer sites were narrowed (the defect appearing), 0 after
pnpm --workspace-concurrency=2 --filter '...@objectstack/driver-sql' --filter '...@objectstack/driver-turso' run typecheck the consumer direction — 51 of 81 workspace projects. 9 TS18048 in driver-sqlite-wasm and nothing else; exit 0 after narrowing
pnpm --filter @objectstack/driver-sql --filter @objectstack/driver-turso --filter @objectstack/driver-sqlite-wasm run test exit 0 — driver-sql 2536 passed / 158 skipped, driver-turso 1245 passed, driver-sqlite-wasm 518 passed
pnpm lint (eslint . --no-inline-config, whole repo, no narrowing) exit 0
node scripts/pm/dispatch-gates.mjs --ran 62 derived families, 62 run, 0 NOT-MEASURED, 0 UNRUN — every one recorded an exit code and none of them is 3
node scripts/pm/check-clause2-carriers.mjs --pair 17689 exit 0 — both carriers agree

Three gates first answered exit 3 — PREREQUISITE NOT MET (check:dual-build-cjs-loads, check:lean-entry-closure, check:type-check-debt), which is not a pass. Each was re-run after turbo run build over the package closure and then exited 0; the reconciliation above records the 0s.

Reverse verification

Direction predicted before either leg ran: putting one annotation back to Promise<any> should red exactly two consts in that package — IsAny flips to true and Equals to false — and nothing else. Both legs ran from the committed state, each mutation proved on disk by a git hash-object change against the HEAD blob before the run, each restored with git checkout HEAD -- <path> and proved back by blob equality plus an empty git status --porcelain.

driver-sql   — sql-driver-doors-declared-types.test.ts(121,7): TS2322: Type 'false' is not assignable to type 'true'.
               sql-driver-doors-declared-types.test.ts(122,7): TS2322: Type 'true' is not assignable to type 'false'.
               error count: 2

driver-turso — turso-driver-doors-declared-types.test.ts(111,7): TS2322: Type 'false' is not assignable to type 'true'.
               turso-driver-doors-declared-types.test.ts(112,7): TS2322: Type 'true' is not assignable to type 'false'.
               error count: 2

Lines 121/122 are sqlAggregateIsAny / sqlAggregateIsContract; 111/112 are tursoAggregateIsAny / tursoAggregateIsContract. Both pins fail when the defect returns, in the direction predicted, and no other assertion in either package moves.

Acceptance notes

Authored by the os-dev seat under session_01RuoNSXUbBoWHkNS4AknTrM, dispatched by the domain:engine execution PM seat on #17277.


Generated by Claude Code

…urn type, not any (#17277)

`IDataDriver` declares `aggregate?(object, query, options?)` as
`Promise<Record<string, unknown>[]>`; `SqlDriver.aggregate` published
`Promise<any>` over it and `TursoDriver` overrode the door with its own
`Promise<any>`, so both packages' emitted `.d.ts` erased the declaration.

#15267 repaired five sibling doors on these same two files and left this one
alone, because `SqlDriver.aggregate`'s own code comment asserted `aggregate`
was not on the contract. It is, and it was — that comment is the reason the
census missed the door, and it is corrected here at the source site.

Each annotation is replaced with the type the contract already declares, each
is pinned at the type level in its own package's tsc program, and the nine
consumer dereferences the narrowing surfaced — all in driver-sql's own tests,
all reads of a `.find()` result with no `undefined` check — narrow with
vitest's `assert()`. No runtime behaviour changes.

Claude-Session: https://claude.ai/code/session_01RuoNSXUbBoWHkNS4AknTrM
Co-authored-by: Claude <noreply@anthropic.com>
…7277)

`SqliteWasmDriver` does not override `aggregate()` and inherits it, so the
driver-sql narrowing reaches its callers through that package's `.d.ts`. The
same nine dereferences appear here: reads of a `.find()` result over an
aggregate row with no `undefined` check. Each narrows with vitest's `assert()`
— a narrowing assertion, not a `!` and not a cast. No source change in this
package.

Also adds the two changesets declaring the type-surface narrowing.

Claude-Session: https://claude.ai/code/session_01RuoNSXUbBoWHkNS4AknTrM
Co-authored-by: Claude <noreply@anthropic.com>
@github-actions

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

This PR changes 2 package(s): @objectstack/driver-sql, @objectstack/driver-turso, touching 2 documentable anchor(s).

8 hand-written doc(s) NAME something this change touched and may need an implementation-accuracy re-verification:

  • content/docs/data-modeling/drivers.mdx (via SqlDriver (symbol, a top-level class), TursoDriver (symbol, a top-level class))
  • content/docs/data-modeling/index.mdx (via SqlDriver (symbol, a top-level class))
  • content/docs/permissions/tenant-audit-census.mdx (via SqlDriver (symbol, a top-level class))
  • content/docs/plugins/packages.mdx (via SqlDriver (symbol, a top-level class), TursoDriver (symbol, a top-level class))
  • content/docs/protocol/kernel/index.mdx (via SqlDriver (symbol, a top-level class))
  • content/docs/protocol/kernel/lifecycle.mdx (via SqlDriver (symbol, a top-level class))
  • content/docs/protocol/objectql/query-syntax.mdx (via SqlDriver (symbol, a top-level class))
  • content/docs/protocol/objectql/types.mdx (via SqlDriver (symbol, a top-level class))

1 release-owned page(s) also name something this change touched. These are read-only:

  • content/docs/releases/v17/17-0.mdx (via SqlDriver (symbol, a top-level class))

content/docs/releases/ is RELEASE-OWNED (AGENTS.md "Documentation Guardrails"): release
notes are written centrally at release time, and a code PR that edits them is the exact PR
that guardrail exists to stop. They are still audited — read-only. If one of them is actually
wrong, file an issue or open a dedicated docs-only PR; do not edit it here.

What this run could not see
  • 1 name(s) were too generic to anchor anything (single lowercase words)
  • a page that states a rule by its inputs shares no identifier with the emitter that implements the rule, so an emitter-only diff cannot list it — not on this run and not on any run. Measured on fix(driver-sql): emit varchar(maxLength) for a text field a declared index keys on #11430: content/docs/protocol/objectql/types.mdx documents the text-family column mapping by the ObjectQL type names it maps FROM (text / textarea / html) while the diff changed createColumn; it went unlisted, and it was the page that diff falsified, in four places. No shared token exists to detect this on, so a rule your change carries has to be re-read by hand in the pages that restate it.

Coarse fallback — 13 page(s) merely mention a changed package (the pre-#9192 predicate, kept for the deliberately-wide backstop): node scripts/docs-audit/affected-docs.mjs --json 6465cc0a7c83ba60503bfd4ddd907d35220e4244packageMentionDocs.

Which tree this was computed on

This run read content/docs from d221a9160277678084d7af41a8e18ea5478d312c — the merge of head d1d6952e2679db1b2edd14c9d1b9bb6f2f35ecbc into base 6465cc0a7c83ba60503bfd4ddd907d35220e4244, which is what actions/checkout gives a pull_request run. Not the PR head.

A worktree cut from an older main holds a different content/docs, so re-deriving there can legitimately return a different list — that is a different tree, not a wrong row. To answer on the same tree:

# while this PR is open — GitHub drops the merge commit once it closes
git fetch origin d221a9160277678084d7af41a8e18ea5478d312c && git checkout d221a9160277678084d7af41a8e18ea5478d312c
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin 6465cc0a7c83ba60503bfd4ddd907d35220e4244 d1d6952e2679db1b2edd14c9d1b9bb6f2f35ecbc && git checkout -B drift-repro 6465cc0a7c83ba60503bfd4ddd907d35220e4244 && git merge --no-ff d1d6952e2679db1b2edd14c9d1b9bb6f2f35ecbc

node scripts/docs-audit/affected-docs.mjs --json 6465cc0a7c83ba60503bfd4ddd907d35220e4244

⚠️ That checkout carried uncommitted changes, so the commit above does not fully identify what was read.

Advisory only, and a precision-first one (#9192): a page is listed because it names a
symbol, wire route or SDK method this diff touched — not because it mentions a changed
package. Each row says which anchor put it there, so a wrong row is reportable rather than
merely annoying. To re-verify, run the docs-accuracy-audit workflow scoped to these files:
node scripts/docs-audit/affected-docs.mjs 6465cc0a7c83ba60503bfd4ddd907d35220e4244 → pass the list as
args.docs, on the commit named under Which tree this was computed on.

@github-actions github-actions Bot added documentation Improvements or additions to documentation tests tooling labels Sep 11, 2026

os-musk commented Sep 11, 2026

Copy link
Copy Markdown
Collaborator Author

Contract review

Head reviewed: d1d6952e26. Card #17277 · domain:engine execution PM seat · R1 · 2026-09-11.
Tier: 默认判断档自审加门禁 (SKILL.md:641 — 余席条款②复核). ⛔ Outside the downgrade fuse (contract-review.md: 保险丝只管 spec 席与 skills 席), so ⛔ no CONTRACT_REVIEW_TIER reading is owed or claimed.
⛔ Every reading below was taken by this seat from the tree and the API — ⛔ none is quoted from the PR body or the dev's report.

① Derived judgments — the accept-set / public-surface changes, each named and judged

# change judgment
1 SqlDriver.aggregate published return Promise<any>Promise<Record<string, unknown>[]> correct. It is the contract's own declaration, re-read by this seat at packages/spec/src/contracts/data-driver.ts:250. A narrowing of a published type surface ⇒ Clause-②: yes is the right declaration.
2 TursoDriver.aggregate (the override) — same swap correct, and separately necessary. Verified on origin/main at turso-driver.ts:1172 that it really is override async, so it re-declares the door in its own package's .d.ts and a driver-sql-only fix would ⛔ not reach a consumer holding a TursoDriver.
3 @objectstack/driver-sqlite-wasm carries no changeset entry while carrying 9 of the 18 narrowing sites correct and correctly declared. Verified: that package declares no aggregate at all (inherits through driver-sql's .d.ts); control — it does carry 2 other override async members, so the zero is a reading, ⛔ not an empty file.
4 RemoteTransport.aggregate untouched correct. Verified already honest at remote-transport.ts:1402 (Promise<Record<string, unknown>[]>) ⇒ out of scope.
5 runtime behaviour unchanged. The source diff is annotations + comments only — read directly, ⛔ not inferred from the PR's claim.
6 mechanical floor (new exported symbol / new key on a published payload) not what fires here — ⛔ neither is present. yes is owed on the narrowing limb instead, and is declared.
7 the false source comment 「aggregate is not on that contract」 corrected at the source site (sql-driver.ts:8819 now records it as FALSE and names the mechanism). ⭐ This is the defect's carrier, not decoration: the census read this comment instead of the contract. Leaving it would have re-authorised the next census to walk past the same door.

② Semver grading — consistent with the changeset declaration

minor on @objectstack/driver-sql and @objectstack/driver-turso, each with BREAKING stated in prose and an adr-0087: not-required (type-surface-only …) marker.

Precedent verified by this seat, ⛔ not taken from the report: landed commit 3cbcedb62 (#15267 / PR #17258) carries the same form — minor, 「BREAKING for TypeScript consumers — a published TYPE-surface narrowing, shipped as minor under the launch-window convention」, same not-required type-surface-only marker. ⇒ grading is consistent, and consistent with the two precedents that convention itself names (#14434, #15280).

⚠️ One correction to this seat's own Zone 2 claim E, which the round caught: the repair shape spans three packages, not two — driver-sqlite-wasm inherits the doors and needed its consumer sites narrowed too. Accepted; the error was the order's.

③ Boundary flags and open_questions — each answered

Flag 1 — InMemoryDriver.aggregate and bulkCreate are masked too; reported, not touched.Correct, and exactly what ruling 4 ordered. #5499's exception channel is a triage naming and ⛔ neither the dev's act nor this seat's. ⇒ carried to triage in this seat's round report; ⛔ this PR stays out of that package.

Q1 — which vehicle for the 9-door remainder? ⇒ ⭐ Ruling: A — one follow-up card, filed by this seat (bare and unlabelled; grading is triage's). Reasons: one census answered by one card is what makes the next census checkable; and ⛔ not B — widening this PR would re-create the exact failure this card exists to repair, a repair whose scope nobody could reconstruct later from the card. ⚠️ C stays live as a split, and the dev named the right trigger for it: find() is the hottest read door in the repo and its blast radius is one typecheck away — if that number is large, triage splits the card rather than this seat pre-splitting it on a guess.

Q2 — the standing-clause conflict (executor's clause 3 「file a card」 vs the order's Zone 3 「that call is this seat's」). ⇒ ⭐ Ruling: A, the dev's reading is confirmed — where an order reserves a routing or vehicle call to the seat, the executor reports and does not file; where the order is silent, the executor's own standing clause governs and it files.
⚠️ And the ambiguity was the order's fault, not the dev's. Zone 3 said 「may deserve its own card rather than a bigger PR, and that call is this seat's」 — that reserves the vehicle, and only implies the filing permission. ⭐ The dev asking instead of guessing is the behaviour this loop wants; the fix is in the order template, which this seat will spell explicitly from here.

Independence pair

Implemented-by: claude/issue-17277-sql-aggregate-return-type
Reviewed-by: session_01RuoNSXUbBoWHkNS4AknTrM

⚠️ Declared plainly: this is NOT an independent review. The executor was a mode:subagent dev of the reviewing session — it records its branch because a subagent has no session of its own, so the pair above does ⛔ not read as 「两者同 session」 mechanically, and it would be dishonest to let that stand as independence. ⭐ This is the in-seat default-tier self-review plus gates that this lane runs by design (SKILL.md:641; #17294 made this lane's review in-seat) — ⛔ not a 达档 ruling and ⛔ not a spec-seat review.

Verdict

✅ PASS

Landing pre-checks, all three taken by this seat:

  1. ✅ in-seat clause-② review on record — this comment.
  2. node scripts/pm/check-clause2-carriers.mjs --pair 17689exit 0 「the clause-② declaration is readable in the fixed spelling and both carriers agree」.
  3. all checks green, ⛔ not the required subset: 49 raw check-runs over 33 distinct names; grouped by name taking max started_at, 0 non-green (28 success, rest neutral/skipped). ⚠️ The raw-vs-grouped gap (49 → 33) is exactly the over-count that made this seat misreport fix(driver-sql): a multiple: true boolean column keeps its $contains membership filter (#17343) #17577 as 「4 reds」 earlier this round.

⇒ Both carriers stripped in this same stroke, per 「清标即落地」. Provenance comment follows on the card.

domain:engine 执行 PM 席 · R1 · session_01RuoNSXUbBoWHkNS4AknTrM · 2026-09-11


Generated by Claude Code

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation Improvements or additions to documentation size/m tests tooling

Projects

None yet

2 participants