Skip to content

refactor(driver-sql): move the transaction doors out of sql-driver.ts into sql-driver-transactions.ts (#25521 cut 3) - #25655

Merged
objectstack-fleet[bot] merged 1 commit into
mainfrom
claude/issue-25521-sql-driver-cut-3
Oct 24, 2026
Merged

objectstack-fleet[bot] merged 1 commit into
mainfrom
claude/issue-25521-sql-driver-cut-3

Conversation

@objectstack-fleet

Copy link
Copy Markdown
Contributor

Part of #25521
Clause-②: no
Metadata-protocol: none
Ruling-ref: 6270097840
Ruling-ref: 6348891160

Cut 3 of the sql-driver.ts split (#24586 ruling A, recorded in 6270097840). The claim is 6384369018. Six SqlDriver method bodies move unedited into a new module, packages/drivers/driver-sql/src/sql-driver-transactions.ts: the public beginTransaction, commit and rollback, the deprecated aliases commitTransaction and rollbackTransaction, and the protected SQLite single-connection guard assertBareKnexSafe. releaseTransaction stays in the class whole.

It is move-only, and the card stays open for the later cuts. Branch claude/issue-25521-sql-driver-cut-3, cut from origin/main at 451bf61d0 (cut 2 merged as b694e56e9 before it).

Serial with the file's in-flight PRs. At claim the PM read no open PR touching sql-driver.ts, driver-turso/src or driver-sqlite-wasm/src. Re-measured at my start (18 open PRs) and again just before this PR was opened (16 open; #25632 had merged and #25619 had left the list), each PR's file list read in full. No PR touches packages/drivers/driver-sql/**, packages/drivers/driver-turso/** or packages/drivers/driver-sqlite-wasm/**, except the Version Packages PR #25640, which touches only those packages' CHANGELOG.md and package.json (its 183 files read page by page). origin/main moved one commit past the base meanwhile (#25632, now 49653f918). It touches only packages/spec and its tests, none of the three driver packages and no gate script this diff derives, so main is not merged in.

Seam table (fresh, at 451bf61d0)

sql-driver.ts is 21,651 lines. 30-day lines are git blame --line-porcelain lines whose committer time is on or after 2026-09-24T12:16:02Z, 30 days before the base commit. The clone's history floor is 2026-09-15, and scripts/pm/git-history.mjs count proves 69 commits on the file in that window (20 in the last 7 days). In all, 3,882 lines (17.9%) are from the last 30 days. The class SqlDriver spans lines 5487-21576 with 347 members: 251 methods (49 public), 32 statics. No public method has an inferred return type; the one inferred public member is the getter supports, in the class head. Sections are the class's own banner comments, cut by contiguous line range. "Public" counts public methods, plus public accessors where there are any:

Lines Section Lines 30-day lines Members (public)
1-5486 Module preamble: imports, types, top-level helpers (199 statements) 5,486 1,416 -
5487-7078 Class head: fields, statics, constructor, capability getters (supports) 1,592 290 88 (1 + 2 accessors)
7079-7510 Lifecycle 432 155 15 (3)
7511-10189 CRUD (IDataDriver core) 2,679 691 45 (7)
10190-10801 Bulk and batch operations 612 140 9 (6)
10802-10898 Raw execution (cut 2's three stubs and the runtimeCreatedIndexes field) 97 28 4 (1)
10899-10976 Transactions (this cut, less releaseTransaction) 78 0 7 (5)
10977-11508 Aggregation 532 61 5 (1)
11509-11796 Distinct 288 20 3 (1)
11797-11912 Window functions 116 4 1 (1)
11913-12011 Query plan analysis 99 1 2 (2)
12012-15766 Schema sync (syncSchema / init) 3,755 373 62 (14 + 3 accessors)
15767-15902 Schema introspection (listPhysicalColumns, introspectSchema) 136 65 2 (2)
15903-21576 Internal helpers, including cut 1's five introspection stubs 5,674 638 104 (5)
21577-21651 After the class: UniqueConstraintMember, singleColumnUniqueColumns 75 0 -

The limits per area are cut 1's (PR #25614), re-read on this tree:

  • getBuilder builders. Aggregation, Distinct, Window functions, Query plan analysis, CRUD and Bulk build through getBuilder, so a cut of any of them must first carry the new module into check:tenant-chokepoint's SCAN_FILES, or the gate goes silently narrower. None of them is cut here.
  • Protected statics. The hash-shadow family and the row-size explainers (Internal helpers) read protected statics, so those bodies stay.
  • this.FIELD.delete/insert/update(. Transactions has one, releaseTransaction (this.openTransactions.delete(key)), and Schema sync has three. Those bodies stay. The six bodies moved here have none: beginTransaction calls .add( on openTransactions, and the other five read no field method at all.
  • insertOnlyUpsertColumns stays in sql-driver.ts, because drivers(sql): upsert's empty-merge-set fallback is merge-ALL, which re-admits every insert-only column — currently unreachable, but it is the wrong shape for "nothing to merge" #8740's restart line names that symbol in that file. This cut does not touch it.

Proposed cut 4: Schema introspection (15767-15902, 136 lines, 65 from the last 30 days): the public listPhysicalColumns and introspectSchema, both with annotated return types. Neither builds through getBuilder (the word appears only in a docblock), neither reads a static, and neither has a this.FIELD.delete/insert/update( site. It is warmer than this cut, and its dev should price three things first: the two module-level helpers the bodies call (suggestFieldTypeForSqlType, postgresTableNameCollisionError), which may need cut 2's leading-export shape; TursoDriver's overrides of both members (the 'remote' and 'refused' rows of its door table); and whether check:tenant-chokepoint's raw-fragment read covers the knex.raw( calls in those bodies, since a move out of SCAN_FILES would narrow it. Every cold area left after that builds through getBuilder.

Why this area for cut 3

Cut 2 proposed it, and it re-measures clean: 78 lines, none from the last 30 days. All five public members (beginTransaction, commit, rollback, commitTransaction, rollbackTransaction) have annotated return types. The section reads no static and no getBuilder. Its one this.FIELD.delete( site is releaseTransaction, which stays. Between them, the six moved bodies read knex, openTransactions, activeTransactions, isSqlite, isProductionEnv, releaseTransaction, commit and rollback, and nothing else. They read no module-level declaration of sql-driver.ts, so unlike cut 2 no export was added.

Overrides and spies, measured before the move.

  • TursoDriver overrides beginTransaction, commit and rollback, and each override calls super after its remote-mode refusal. Its door table lists commitTransaction and rollbackTransaction as 'inherited', because they call this.commit / this.rollback, which refuse. So both aliases must keep reaching the subclass's commit / rollback, and they do: they reach them through late-bound closures.
  • SqliteWasmDriver overrides the getter isSqlite, which assertBareKnexSafe reads. The host reads it through a getter that reads the driver, so the override is the one the guard sees.
  • vi.spyOn(driver, 'beginTransaction') in service-storage and plugin-auth tests spies on the class method, which is still there.
  • A scratch probe against the built dist checked every moved member's late binding at base and at head: the counter and the open set after beginTransaction, an instance spy on isProductionEnv (reached by assertBareKnexSafe), on releaseTransaction (reached by commit), on commit and rollback (reached by the aliases), a subclass override of isSqlite, a subclass commit that refuses (reached by commitTransaction), and a rejecting commit that still releases. Its 14 readings are identical at base and head. The probe is not committed.

The shape (#23050 shape B, as cuts 1 and 2 apply it)

  • The class keeps each method with its name, signature, docblock and position, because tsup emits members with their JSDoc into the .d.ts. Each body is now one host literal plus fn.call(host, ...), marked [#25521 cut 3]. The five async methods drop async and return the moved function's promise. assertBareKnexSafe was never async.
  • TransactionHost is new. It types every member as the class member itself (an indexed access on SqlDriver). Each moved function types its this as the Pick of the host over exactly the members its body reads. The generator measures those reads from the AST (this.NAME property accesses) and asserts them, so the string this.knex. inside the guard's error text is not counted as a read. Each stub annotates its literal as the ThisParameterType of the moved function. There is no cast, and no member becomes public.
  • knex, openTransactions, activeTransactions and the getter isSqlite go as getters, so each read happens when the body makes it. commit, rollback, releaseTransaction and isProductionEnv go as late-bound closures.
  • One field is written. beginTransaction runs this.activeTransactions++, so its literal carries a setter beside the getter, which writes the field on the driver. The generator measures writes (++, -- and assignment on this.NAME) and asserts them, and the interface marks activeTransactions as the one non-readonly field. This is the accessor pair auth-manager.ts uses for jwtSigningFailureReported ([maintainer] refactor(plugin-auth): split src/auth-manager.ts (10,338 lines, +1,867 in 30 days) along its AuthManager seams, one move-only slice per PR #25224 cut 6). Both fields stay on the class: releaseTransaction, ensureSequencesTable's guard call and the tests read them.
  • this.knex.transaction() keeps its receiver: the getter hands back the class's knex, and .transaction is called on it.
  • The move was produced by a script from the source text, not retyped: each method's parameter list, return type and body lines, re-indented two columns. The guard's error message is five single-line template literals joined by +, so no line continues a template literal.

The published .d.ts does not change

Under the series bar (ruling 6348891160, letter A), measured on base 451bf61d0 and on head 59d63a8d1. Both were built against the same dependency tree: turbo run build --filter=@objectstack/driver-turso... --filter=@objectstack/driver-sqlite-wasm... at base, 9/9 tasks (driver-sql a cache miss). Then pnpm --filter @objectstack/driver-sql build at head.

  • This package emits dist/index.d.ts and dist/index.d.mts (there is no .d.cts). The two are identical to each other at both ends.
  • Raw sha256 of both files, base and head, all four the same: 93da45168e82136b7613944525e47402d5dc6f9f8daa9efa4bdceeaa11b52a89 (cuts 1 and 2's hash, unchanged). The raw diff is 0 lines.
  • Normalised with the ruling's one-liner verbatim (double-quoted unions), base = head = 93da45168e82136b7613944525e47402d5dc6f9f8daa9efa4bdceeaa11b52a89. This file has 0 double-quoted unions, so that normaliser rewrites nothing.
  • Normalised with the same one-liner, with the quote character swapped to a single quote (31 single-quoted unions match), base = head = 118e992ad8aad590044eb31509dca65e6b321e3410ce846e1e5f5dd34b956e95. With a raw diff of 0 lines, the proof rests on raw identity, and neither normaliser is relied on.
  • Positive control on a copy of the head dist/index.d.ts: line 4282, protected assertBareKnexSafe(op: string): void;, made public. That gives raw and double-quote-normalised f492eb1c19c6832b1e13aee2f02596041b34c0252971720a0472a266c44eb7cc and single-quote-normalised 8337de653b61ff24a59c29934ce1783051ba7c056523aa2e700107e9c69c6216, each different from the real hash.
  • Export tables: the ESM and CJS bundles each export the same 56 names at base and head, loaded at runtime (import() and require of the built bundles). The sorted list hashes to d61a9dd3a02661602b1bb2dbe18152ac43986148f36b2746f40c0884bd8ffdec on all four.
  • The head build carries the move: dist/index.mjs and dist/index.js each contain 14 host .call( sites (8 at base, plus these 6).

No changeset: skip-changeset, per ruling 6348891160 item 4.

Proof of move-only

  • Bodies, by AST token (recursive getChildren leaves). Each moved function's parameters (less the added this), return type and body have the same token sequence as the base method, and async matches on all six. 218 tokens compared, 0 differ.
  • Lines: 38 lines compared, from each method's name to its closing brace. Each one is the base line with two leading spaces removed (the first line, less the added this parameter, is byte-identical).
  • Negative control: changing one character inside a template literal of assertBareKnexSafe in a copy of the module makes the check report 1 token and 1 line different, and it exits PROOF FAILED, exit 1.
  • Class side: 347 members at base and head. The 341 members that did not move are byte-identical to base, leading trivia included. Each of the 6 stubs keeps its docblock and signature byte-identical, less async on the five that had it.
  • Top level of sql-driver.ts: all 201 base statements other than the class are byte-identical, in order. The one new statement is the import of the new module.
  • Size: +177 / -31 over 2 files, far under the 3,000-line manual-merge line. sql-driver.ts goes from 21,651 to 21,678 lines, and sql-driver-transactions.ts has 119. See the acceptance notes on why the file grows.

Readers of sql-driver.ts by path, checked against the moved text

None needed an edit. Each was read against the moved region (10899-10976) and run at both ends where it is a gate or test:

  • ADR anchors. scripts/adr-anchors/packages__drivers__driver-sql__src__sql-driver.ts.json carries ADR-0053 (formatOutput, presentReadValue) and ADR-0120 D4. Neither paragraph describes a moved body, so per card ask 3 the new module gets no entry of its own. The moved bodies cite no ADR. check:adr-anchors: 85 anchored files at base and head. Decision-letter citations on the anchored surface: 1,849 at base and 1,849 at head.
  • Symbol anchors. ADR-0119 anchors packages/drivers/driver-sql/src/sql-driver.ts#beginTransaction. The stub keeps that declaration in sql-driver.ts, so the anchor still resolves. No other anchor names a moved member. check:adr-symbol-anchors: 2,384 anchors resolve at both ends.
  • check:tenant-chokepoint reads sql-driver.ts for getBuilder bindings and for IS NULL arms in raw fragments. The moved text has neither, so the gate still covers the same population: 24 getBuilder() bindings across 3 files at both ends.
  • Logger receivers. The moved text reads no logger. check:logger-receiver-detach: 0 detaches at both ends (3,681 files walked at head, one more than base: the new module).
  • Other source scans, none of whose patterns occurs in the moved text: logger-receiver-detach.test.ts ③, sql-driver-compile-refusal-seam.test.ts, the five packages/cli generate-*.pin.test.ts files, check-keyed-text-bounds (varcharColumnChars), check-object-def-param-keys (the initObjects parameter keys).
  • Path lists and comment citations that name the file (turbo.json, scripts/cross-package-test-inputs.mjs, scripts/pm/check-half-states.mjs, scripts/check-changeset-no-major.mjs, packages/spec/liveness/*.json evidence, prose citations in objectql, spec, driver-turso). None names a moved member.
  • driver-turso and driver-sqlite-wasm: their readers of moved members reach the stubs unchanged. Both suites ran at both ends (below).
  • check:driver-conformance at base and head: 55 covered, 0 in the DEBT ledger, 0 exempt, both times.

Verification

  • tsc --noEmit (driver-sql) exit 0. eslint --no-inline-config on both source files exit 0.
  • packages/drivers/driver-sql suite: base 3,999 tests (3,787 passed, 0 failed, 212 skipped; 247 files), head the same. Compared per test name and status from vitest's JSON reporter: no test lost, none changed status, none added.
  • packages/drivers/driver-turso suite, run against the rebuilt head dist: base 2,526 tests (2,492 passed, 0 failed, 34 skipped; 95 files), head the same. Per test name and status there is no difference (2,513 distinct names; 13 names repeat).
  • packages/drivers/driver-sqlite-wasm suite, against the rebuilt head dist: base 704 tests (704 passed; 39 files), head the same, per test name no difference.
  • dispatch-gates --commands on this diff, at head 59d63a8d1: 55 derived commands, each run with its exit code. --ran reconciles 55 derived, 54 run, 1 NOT MEASURED, 0 unrun. The NOT MEASURED one is check:dual-build-cjs-loads (exit 3, PREREQUISITE NOT MET: it needs every package's dist), which CI runs. check:lean-entry-closure ran after objectql's closure was built.
  • No reverse verification or ablation: the diff changes no tested behaviour.

Acceptance notes

  • The file grows by 27 lines. The six bodies are short (the two deprecated aliases are one line each), and each stub's host literal is longer than the body it replaced. The cut moves the transaction logic into its own module rather than shrinking the file. Every remaining cut is larger than this one.
  • assertBareKnexSafe now builds its host literal before the production early return. Its docblock calls the guard zero-overhead on the hot path. Its one caller is ensureSequencesTable's lazy fallback (a write that reaches the sequences table before initObjects created it), and only when the runner is the bare knex, so this is not a hot path today. The docblock is unchanged, because this cut is move-only.
  • TursoDriver's three overrides still call super, which now returns the moved function's promise instead of an async method's promise. A caller awaits the same settlement.
  • ADR-0119 still says driver-sql implements beginTransaction at sql-driver.ts#beginTransaction. That declaration is still there, and the body is one module over. No docs/adr/** edit is made here.

Generated by Claude Code

… into sql-driver-transactions.ts (#25521 cut 3)

Move-only: the bodies of beginTransaction, assertBareKnexSafe, commit,
rollback, commitTransaction and rollbackTransaction move unedited into
sql-driver-transactions.ts and reach the class through one named host,
TransactionHost. The class keeps every method with its signature and
docblock; releaseTransaction stays in the class whole.

Claude-Session: https://claude.ai/code/session_01JVTd3PprStGa26aez5omLB
Co-authored-by: Claude <noreply@anthropic.com>
@objectstack-fleet objectstack-fleet Bot added the skip-changeset PR has no user-facing published change; bypasses the changeset gate label Oct 24, 2026
@github-actions

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

15 anchor(s) derived from 1 changed package(s); no hand-written page names any of them, so this run has nothing to list — not a clean bill of health. This check sees only pages that NAME a derived anchor: one that documents this change in prose, or enumerates it in an authoring dialect, names none and stays invisible to it on every run.

What this run could not see
  • 3 name(s) were too generic to anchor anything (single lowercase words)
  • the SDK route bridge reached 54 of 206 client-bound route-ledger rows — the other 152 have no registrar path: tail to select them, so pages documenting THEIR client methods cannot appear above, on this or any run. Of those 152: 0 are remediable by widening that discovery convention (an in-repo file declares the path; the convention did not scan it); 55 are structural — on a ledger where NOT ONE row is declared in-repo, so no discovery change reaches them at any price; 97 are undecided (no in-repo declaration, on a ledger that has other in-repo registrars — absence and an unreadable spelling are not distinguishable here). The rows themselves: node scripts/docs-audit/affected-docs.mjs --bridge-coverage
  • 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.
  • a key NAME is not a key, so the hand re-read the line above prescribes can land on the wrong schema. The same spelling is authorable on one governed type and a [REMOVED] tombstone on another for each of active, aria, joins, objects, template, tools and version (censused on [finding] tools is a key on BOTH AgentSchema (tombstoned, dead) and SkillSchema (live, cloud-attested), so a name-based search attributes skill examples to the agent key — it produced a false stop-the-line alarm on PR #19059 #19093 over the liveness ledger's governed types, top-level keys); nothing in a search result distinguishes the two, so a grep hit on a LIVE example reads as evidence about the DEAD key. Measured on fix(spec): the agent.tools liveness row says dead — it claimed live on a key the schema tombstoned #19059: content/docs/ai/agents.mdx was reported as contradicting the agent.tools tombstone over its tools: example at :161, which is inside the defineSkill({ block opened at :155 — the page was already correct. Settle ownership by PARSING the value against both schemas, never by the name: that literal PASSES SkillSchema, and as an AgentSchema it FAILS at tools with the tombstone prescription. ⛔ These names are not the whole class — a key retired through a .strict() guidance map leaves no tombstone in the walked shape and none of them here (tool.category, live as AIToolDefinition.category).

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 49653f91812815c72eb9194bf0a6cc451cf711f2 → packageMentionDocs.

Which tree this was computed on

This run read content/docs from d9255a48a1dc69252764bcc7ee7fc2dece021e7e — the merge of head 59d63a8d1bbd9fef22e594dc84f121eaaf25a4c2 into base 49653f91812815c72eb9194bf0a6cc451cf711f2, 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 d9255a48a1dc69252764bcc7ee7fc2dece021e7e && git checkout d9255a48a1dc69252764bcc7ee7fc2dece021e7e
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin 49653f91812815c72eb9194bf0a6cc451cf711f2 59d63a8d1bbd9fef22e594dc84f121eaaf25a4c2 && git checkout -B drift-repro 49653f91812815c72eb9194bf0a6cc451cf711f2 && git merge --no-ff 59d63a8d1bbd9fef22e594dc84f121eaaf25a4c2

node scripts/docs-audit/affected-docs.mjs --json 49653f91812815c72eb9194bf0a6cc451cf711f2

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

@objectstack-fleet

Copy link
Copy Markdown
Contributor Author

席位核对

ACCEPT · domain:server seat 2 · sunhaolin · session_01JVTd3PprStGa26aez5omLB · 2026-10-24T14:19Z · head 59d63a8d1b · claim 6384369018 · report 6384764296

  • Shape: draft, base main, author objectstack-fleet[bot], assignee sunhaolin, Part of #25521, no closing keyword, Clause-②: no, Metadata-protocol: none, skip-changeset (ruling 6348891160). 2 files in packages/drivers/driver-sql/src/; not governed.
  • Move-only, re-measured by the seat: six bodies at base (sql-driver.ts:10903 beginTransaction, :10936 assertBareKnexSafe, :10950 commit, :10959 rollback, :10968 commitTransaction, :10973 rollbackTransaction) against the new module (:72–:117). Whitespace-insensitive comparison: all six identical. releaseTransaction stays for its this.openTransactions.delete( site.
  • Overrides (A3), as measured by the dev: TursoDriver overrides beginTransaction, commit and rollback. The moved bodies reach commit / rollback / releaseTransaction / isProductionEnv as late-bound host closures, so the override still dispatches. The base and head probe readings agree.
  • Open question 1 (the activeTransactions setter), option A accepted: a get/set accessor pair over the class field, as on [maintainer] refactor(plugin-auth): split src/auth-manager.ts (10,338 lines, +1,867 in 30 days) along its AuthManager seams, one move-only slice per PR #25224 cut 6. The field stays the one source of truth on the class.
  • CI on head 59d63a8d1b: 34 success, 12 skipped, 0 red; every skip on main's roster.
  • Writes: mcp_calls 0.

One measured cost, and a rule for the next cuts: this cut moves 78 short lines, and the stubs with their host literals are longer than the bodies they replace. sql-driver.ts grows from 21,651 to 21,678 lines. The seam module is still worth having (transaction code now has its own file), so this cut is accepted. From cut 4 on, an area moves only when sql-driver.ts's net line count drops; areas whose bodies are shorter than their host literal stay in the class, as #25522 cut 2 already did. The dev's cut 4 proposal (Schema introspection, 136 lines) is priced against that rule at dispatch.

Landing: pr_ready + automerge_enable through the relay in this act (CI green, mergeable clean).


Generated by Claude Code

@objectstack-fleet
objectstack-fleet Bot marked this pull request as ready for review October 24, 2026 14:21
@objectstack-fleet
objectstack-fleet Bot enabled auto-merge October 24, 2026 14:21
@objectstack-fleet
objectstack-fleet Bot added this pull request to the merge queue Oct 24, 2026
Merged via the queue into main with commit 645ede5 Oct 24, 2026
47 checks passed
@objectstack-fleet
objectstack-fleet Bot deleted the claude/issue-25521-sql-driver-cut-3 branch October 24, 2026 15:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size/m skip-changeset PR has no user-facing published change; bypasses the changeset gate

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants