Skip to content

feat(service-datasource): external-datasource driver return contract fails compile-time on mis-shaped drivers (#11123 option C) - #11492

Merged
os-steve merged 1 commit into
mainfrom
claude/issue-11381-driver-return-contract
Aug 24, 2026
Merged

feat(service-datasource): external-datasource driver return contract fails compile-time on mis-shaped drivers (#11123 option C)#11492
os-steve merged 1 commit into
mainfrom
claude/issue-11381-driver-return-contract

Conversation

@os-steve

Copy link
Copy Markdown
Collaborator

Fixes#11381

Implements option C of the #11123 maintainer ruling (recorded 2026-08-23, 「10950 不考虑存量,其他接受你的建议」= A now + C as its own card): the custom external-datasource driver seam gets a declared return-shape contract, so a mis-written driver fails to compile, naming the wrong field, instead of being absorbed silently.

This PR stays DRAFT — Clause-② contract-review tier. The card carries needs:contract-review; the landing decision is the PM review's, not this seat's.

Seam map (measured at merge base b6d9432d1a, not assumed)

The dispatch expected the seam "around packages/spec and/or the objectql driver interface"; measurement places it where #11123's recorded analysis names it:

surfacerolestate before this PR
packages/services/service-datasource/src/contracts/datasource-driver-factory.tsDatasourceDriverHandle.introspectSchemaTHE seam — the only typed crossing every host-built driver makes (the framework deliberately ships no driver-by-id registry)Promise<unknown> — the compiler channel the #11124 retirement named as its migration path provably never fired here
packages/spec/src/contracts/schema-diff-service.tsIntrospectedSchema/IntrospectedTable/IntrospectedColumnthe one introspection contract (per-column primaryKey, dialect, introspectedAt)already truthful after #11270 (indexes optional, defaultValue: unknown); not edited
packages/services/service-datasource/src/external-datasource-service.tsprimaryKeyReaderthe PR #11001 compatibility shim (three-signal union read)code untouched — docblock truth update only, see below
packages/objectql/src/engine.tsintrospectDatasource(): Promise<unknown>; packages/spec/src/contracts/data-driver.tsIDataDriver (declares no introspectSchema)adjacent consumer-side facade / the engine-registration pathdeliberately not widened into — see "Cascade boundary"

In-tree fallout of the tightening: zero implementers move. Neither in-tree factory (default-datasource-driver-factory, prebuilt-driver-factory) nor the runtime Turso factory puts introspectSchema on its handle (they use the driver escape hatch + health probes); the consumer sites (plugin.ts, datasource-admin-plugin.ts probe) either already type the result as the spec IntrospectedSchema or discard it. SqlDriver-family drivers declare return types that extend the spec contract (post-#11124/#11270), so they satisfy the tightened member by covariance.

What tightened

  • introspectSchema?(): Promise<unknown>introspectSchema?(): Promise<IntrospectedSchema> (spec contract, imported from @objectstack/spec/contracts), with a docblock recording the why, the named-field failure mode, the legality of richer extending types, and the shim's continued ownership of runtime.
  • The DatasourceDriverHandle interface docblock now says what "fully optional" means: optional in presence, contractual in shape.

Shim untouched — declaration

The PR #11001primaryKeyReader union read is byte-identical: all three arms (col.primaryKey / col.isPrimary / table.primaryKeys) intact, its tests untouched and green (the staged-disagreement pair included). Per the ruling's ordering constraint, the tightening and the shim coexist: new/updated TS drivers are told by the compiler; already-built, plain-JS, or cast drivers are still absorbed at runtime. Option B (shim removal) remains a later, separate, gated step — this diff neither performs nor licenses it.

Two docblock bullets inside the shim's expiry-condition record are updated, both named here because that naming is the condition for taking them in place:

  1. Falsified by this diff: the bullet stating the handle "types the handle as introspectSchema?(): Promise<unknown>" and that the compiler channel "never fires". Now records the tightened type and the exact population the compiler still cannot reach (stale builds, plain JS, casts) — the belt's remaining constituency.
  2. Falsified by the release record (same defect class, mechanical, evidence pinned): "BOTH still unconsumed changesets at 17.1.0" — the fix(service-datasource): read the introspected primary key at the isPrimary/primaryKey seam #11001 and fix(driver-sql): introspectSchema() emits the spec introspection contract — primaryKey, dialect, introspectedAt (#10676, #10998) #11124 changesets were consumed into 17.2.0 (version-packages commit e7d2cc67fd, 2026-08-23; driver-sql/CHANGELOG.md carries the retirement entry). The bullet now says the clock HAS started and that option B's "retirement published" gate is a release-record question (npm), not a tree measurement. ⚠️ PM note: with 17.2.0 versioned, B's second gate condition may already be met once the publish is confirmed — that re-judgment is the ruling's to make, flagged here only so it is not lost.

Compile-fail demonstrations (real tsc output, measured on b6583cd03e)

New type-level suite src/__tests__/datasource-driver-handle-contract.test.ts, in the sibling-contract idiom (data-driver.test.ts): pins resolved by tsc --noEmit (the package's typecheck, which compiles __tests__ — no test exclusion, no test-typecheck-debt entry, so zero errors is the measured baseline), @ts-expect-error directives with the expected text, expectTypeOf identity pin on the member itself.

With the directives stripped (mutation proven on disk: 7→0 occurrences; restore proven byte-identical by git hash-object), tsc names the field in every mis-shape the card/#11123 name:

Reverse verification (both legs from the committed state; predicted direction written before running, observed exactly): with the contract reverted to the merge-base Promise<unknown> (mutation proven on disk: Promise<unknown> 1 hit / tightened 0) and the new tests kept, typecheck reds with 3× TS2578 unused-directive on the three handle-level pins — i.e. the mis-shaped drivers compile pre-patch, which is the defect — plus 5 consumer-side errors (expectTypeOf mismatches, property access on unknown). The two field-level pins stay consumed on both legs, as designed: they pin the spec contract itself, which #11270 already ships. Restore proven byte-identical (git hash-object = committed blob) both times.

Cascade boundary (stopped, not widened)

Tightening this one member forces no public-surface change elsewhere — the diff stays inside @objectstack/service-datasource. Two adjacent unknown seams were found and deliberately left: IDataEngine.introspectDatasource(): Promise<unknown> (objectql consumer facade) and the handle.driver: unknown escape-hatch path into registerDriver(), where IDataDriver declares no introspectSchema at all — a driver entering the engine that way is still unreached by any compiler. Filed as a follow-up finding (see issue comment / report) rather than smuggled into this diff.

Changeset level — reasoning

minor for @objectstack/service-datasource, with an explicit BREAKING declaration in the body. This is the repo's documented, gate-enforced convention, not a downgrade: scripts/check-changeset-no-major.mjs's own header — "During the launch window we ship breaking changes as minor" — and the direct precedent, the #11124 retirement changeset ("BREAKING … shipped as minor under the repo's launch-window convention"). The dispatch's major-vs-gate conflict therefore does not arise: the convention itself dictates minor. ADR-0087 disposition: not-required (runtime-interface-only …#DatasourceDriverHandle) — verified mechanically by check-adr-0087-registration.mjs (green, notice emitted): a published runtime TS interface, no Zod schema, no stored representation, nothing for objectstack migrate meta to rewrite; the channel that reaches every affected consumer is the compiler.

Verification summary — all on final commit b6583cd03e, clean tree; exit codes captured before any pipe; verdicts are the gates' own printed lines

  • Build: deps closure pnpm --workspace-concurrency=2 --filter '@objectstack/service-datasource^...' buildVERDICT command-exit 0; full workspace closure turbo run build --filter='./packages/*' --filter='./packages/*/*' --concurrency=2Tasks: 70 successful, 70 total.
  • pnpm --filter @objectstack/service-datasource typecheck → exit 0 (script name echoed — not a zero-match no-op); … test -- --maxWorkers=2Test Files 27 passed (27) · Tests 585 passed (585).
  • Gate union derived on the final commit with no hand-supplied paths: node scripts/pm/dispatch-gates.mjs --repo objectstack-ai/objectstack (repo assertion held; 4 paths vs merge base b6d9432d1; 12 path-matched + 6 convention-triggered). All 18 ran locally, all green, no declared narrowing: check:changeset-gate-self-tests (116 assertions), check:objectui-changeset, check-adr-0087-registration (exemption notice emitted), check-changeset-no-major (✓ This diff introduces no major bump.), check-empty-changeset (1 declaring changeset), check:published-files (69 publishable), check:slot-lookup (107 unswept … none new), check:test-source-alias (72 packages), check:type-source-resolution (77 packages), check-ci-filter-parity (89 globs covered), check-plugin-teardown-shape (63 plugins), check-affected-docs (exit 0), check:query-options-erasure (67 unswept … none new), check:type-check-coverage (65/78), check:type-check-debt --re-measure (33 ledger entr(ies) re-measured … none above its recorded number), check:engine-double-contract (390 pinned), check:cross-package-test-inputs (14 declared), check:where-matcher (288 matcher(s) … correctly). Plus check:nul-bytes (6443 text file(s) … no raw ASCII control bytes).
  • No runtime behaviour change is claimed, so no runtime ablation beyond the type-level legs above: the executable diff is one type annotation; the mutation whose removal reddens the suite IS the reverse-verification leg, and it was run in both directions.

Generated by Claude Code

…lares the spec introspection contract (#11381, #11123 option C)
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01T9cDbY2NBiVJWYx3BpWfH2
@github-actions

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

4 anchor(s) derived from 1 changed package(s); no hand-written page names any of them. ✅

What this run could not see
  • 1 changed file(s) yielded no anchor (packages/services/service-datasource/src/external-datasource-service.ts) — pages documenting those are invisible to this run
  • the SDK route bridge reached 45 of 222 client-bound route-ledger rows — the other 177 have no registrar path: tail to select them, so pages documenting THEIR client methods cannot appear above, on this or any run: node scripts/docs-audit/affected-docs.mjs --bridge-coverage

Coarse fallback — 0 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 81462d06b5a15b17ee30146994b1b67b56a166d7packageMentionDocs.

Which tree this was computed on

This run read content/docs from 3f9c5b40f43e29f1bcb48e953f5b9e388c37418d — the merge of head b6583cd03e62006a955b341f872a29f017c131b8 into base 81462d06b5a15b17ee30146994b1b67b56a166d7, 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 3f9c5b40f43e29f1bcb48e953f5b9e388c37418d && git checkout 3f9c5b40f43e29f1bcb48e953f5b9e388c37418d
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin 81462d06b5a15b17ee30146994b1b67b56a166d7 b6583cd03e62006a955b341f872a29f017c131b8 && git checkout -B drift-repro 81462d06b5a15b17ee30146994b1b67b56a166d7 && git merge --no-ff b6583cd03e62006a955b341f872a29f017c131b8
node scripts/docs-audit/affected-docs.mjs --json 81462d06b5a15b17ee30146994b1b67b56a166d7

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

@github-actionsgithub-actionsBot added documentation Improvements or additions to documentation tests tooling labels Aug 23, 2026
@os-steve
os-steve marked this pull request as ready for review August 23, 2026 23:43
@os-steve
os-steve added this pull request to the merge queueAug 24, 2026
Merged via the queue into main with commit 3d79144Aug 24, 2026
38 checks passed
@os-steve
os-steve deleted the claude/issue-11381-driver-return-contract branch August 24, 2026 00:45
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentationImprovements or additions to documentationsize/mteststooling

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Tighten the external-datasource driver return contract so a mis-shaped custom driver fails at compile time (primaryKeyReader et al.)

2 participants

@os-steve@claude