Skip to content

fix(service-datasource)!: reconnect requires previous, and the engine read-back answers the stored driver id - #27788

Merged
objectstack-fleet[bot] merged 4 commits into
mainfrom
claude/project-thread-eecjmj
Oct 30, 2026
Merged

objectstack-fleet[bot] merged 4 commits into
mainfrom
claude/project-thread-eecjmj

Conversation

@objectstack-fleet

Copy link
Copy Markdown
Contributor

Requested by 田騏 · project thread

Fable review owed. This PR was built on Opus while the Fable quota was out (maintainer order: finish on Opus, then wait for the contract review). It touches packages/spec/src/**, so it does not land until a CONTRACT_REVIEW_TIER contract review record PASS is on its current head.

Fixes #27727 (epic #27261). The four should-fixes and the prose note from the contract review of PR #27715 (6508089206, ④ and ③). Claim 6509213861.

What changes

Why

Contract review 6508089206 ④ on #27666: no must-fix, and none of these opens a path today, but each leaves the next caller a way to write a datasource the floor does not protect, or a contract that says two things about one member.

Verification

Local, on a5f0c8b5 (branch merged with origin/main at 34dc09b6):

  • New pins, each red with the fix reverted and green with it (reverse-verified from a committed state):
    • objectql external-driver-floor.test.ts, [#27727] block: listDatasourceDefs answers the named id, the kept id, and the manifest-declared id (2 of 3 red without the engine change; the third is the control that a def with no driver is listed without the key).
    • service-datasource datasource-connection-service.test.ts, [#27727] block: a reconnect without previous (and with an empty driver) rejects, the live driver and the def list are untouched, and the factory built nothing (red without the guard); a failed rebuild restores the kept pool under the OLD driver id. A @ts-expect-error pins previous as required in a file the package's tsc compiles.
  • pnpm --filter @objectstack/objectql test: 9247/9247. pnpm --filter @objectstack/service-datasource test: 1013/1013. cli secret-reference-union and absent-table-reads: 49/49.
  • typecheck clean for objectql, service-datasource and spec; pnpm lint exit 0; spec check:generated exit 0 (all 14 artifacts current).
  • dispatch-gates --ran: 109 derived families, 109 run, 0 not measured, all exit 0 (after a full pnpm build).

Clause-②: yes (narrowing)
Metadata-protocol: none

The claim declared Clause-②: no (narrowing) for the required previous; the diff also widens listDatasourceDefs's answer by the stored driver key, so this PR and its changeset say yes (narrowing), as the epic PM asked. The changeset carries the BREAKING banner (a host calling reconnect without previous; none in-repo) and the ADR-0087 marker not-required (no-migration-prescription).

Human Minimum

  • 1. Schemas start as Zod; packages/spec holds no runtime logic.
  • 2. Every name I added uses the casing for its kind.
  • 3. No formatter ran; pnpm lint passes.
  • 4. One issue, one branch off an up-to-date main, one pull request.
  • 5. A change that publishes carries a changeset, and both declaration lines are filled in here and in the changeset.
  • 6. packages/spec touched: its generated artifacts are regenerated and committed (doc-only change to a contract interface; pnpm --filter @objectstack/spec check:generated exits 0 against the freshly built dist, nothing to regenerate).
  • 7. No hand edits to content/docs/references/, packages/*/CHANGELOG.md or content/docs/releases/.
  • 8. Title, body and comments are in English.
  • The Before Submitting list in CONTRIBUTING.md is done.

🤖 Generated with Claude Code

https://claude.ai/code/session_017ihL2fhwqSBmy1XBXfcoU1


Generated by Claude Code

@github-actions github-actions Bot added size/m documentation Improvements or additions to documentation tests tooling labels Oct 29, 2026
@github-actions

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

This PR changes 3 package(s): @objectstack/objectql, @objectstack/service-datasource, @objectstack/spec, touching 5 documentable anchor(s).

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

  • content/docs/ai/actions-as-tools.mdx (via IDataEngine (symbol, a top-level interface))
  • content/docs/ai/knowledge-rag.mdx (via IDataEngine (symbol, a top-level interface))
  • content/docs/data-modeling/drivers.mdx (via schemaMode (literal, a string literal in reconnect))
  • content/docs/data-modeling/external-datasources.mdx (via schemaMode (literal, a string literal in reconnect))
  • content/docs/data-modeling/virtual-objects.mdx (via schemaMode (literal, a string literal in reconnect))
  • content/docs/kernel/contracts/data-engine.mdx (via IDataEngine (symbol, a top-level interface))
  • content/docs/kernel/contracts/index.mdx (via IDataEngine (symbol, a top-level interface))
  • content/docs/kernel/index.mdx (via IDataEngine (symbol, a top-level interface))
  • content/docs/kernel/runtime-services/data-service.mdx (via IDataEngine (symbol, a top-level interface))
  • content/docs/kernel/services-checklist.mdx (via IDataEngine (symbol, a top-level interface))
  • content/docs/kernel/services.mdx (via IDataEngine (symbol, a top-level interface))
  • content/docs/permissions/tenant-audit-census.mdx (via IDataEngine (symbol, a top-level interface))
  • content/docs/protocol/knowledge.mdx (via IDataEngine (symbol, a top-level interface))
  • content/docs/protocol/objectql/query-syntax.mdx (via IDataEngine (symbol, a top-level interface))

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

  • content/docs/releases/implementation-status.mdx (via IDataEngine (symbol, a top-level interface))
  • content/docs/releases/v17/17-0.mdx (via DatasourceConnectionService (symbol, a top-level class), IDataEngine (symbol, a top-level interface))
  • content/docs/releases/v17/17-6.mdx (via schemaMode (literal, a string literal in reconnect))
  • content/docs/releases/v18/18-1.mdx (via schemaMode (literal, a string literal in reconnect))

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 anchor(s) matched too much of the corpus to be a work list: ObjectQL (symbol, 79 pages)
  • 1 name(s) were too generic to anchor anything (single lowercase words)
  • the SDK route bridge reached 58 of 210 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 — 169 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 968cacfd10fdec64b6089d0da4f721f1a8a10925 → packageMentionDocs.

Which tree this was computed on

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

node scripts/docs-audit/affected-docs.mjs --json 968cacfd10fdec64b6089d0da4f721f1a8a10925

⚠️ 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 968cacfd10fdec64b6089d0da4f721f1a8a10925 → pass the list as
args.docs, on the commit named under Which tree this was computed on.

@objectstack-fleet

Copy link
Copy Markdown
Contributor Author

epic:#27261#1 · marchtian · session_017ihL2fhwqSBmy1XBXfcoU1

Body addendum from the card thread. The relay refuses a body patch here because the body's first line is the project attribution marker, not a signature line, so the third declaration line and the maintainer summary ride this comment instead of the body.

Skills-docs: none — no page under skills/** or content/docs/** teaches DatasourceConnectionService.reconnect or listDatasourceDefs (git grep over both trees at this head)

维护者速读

  • 改了什么: 契约文档补上 driver 的例外说法;listDatasourceDefs 读回带上 driver;reconnect 必须传 previous,不传就在动任何连接之前直接报错;一处测试注释和一处指南旧说法改正。
  • 为什么改: fix(service-datasource): a plugin-driver datasource connected or reconnected through the connection service keeps the engine's floor #27715 复核的 4 条 should-fix 和 1 条文字问题;防止宿主不传 previous 时,保留下来的插件驱动连接池悄悄失去兜底层。
  • 风险与回滚: 低。仓库内所有调用方都已传 previous;仓库外直接调用 reconnect 而不传的宿主会编译报错。回滚即还原本 PR。
  • 先看: packages/services/service-datasource/src/datasource-connection-service.ts(reconnect 开头的拒绝分支)、packages/objectql/src/engine.ts(listDatasourceDefs)、packages/spec/src/contracts/data-engine.ts(两处文档)。
  • 两分钟验证: pnpm --filter @objectstack/service-datasource exec vitest run src/__tests__/datasource-connection-service.test.ts -t 27727,预期 2 个用例通过。
  • 需要你做的: 无,席位落地。

@objectstack-fleet

Copy link
Copy Markdown
Contributor Author

epic:#27261#1 · marchtian · session_017ihL2c5z3q4h8inrn5kwqV
Contract-review-handoff: triage — CONTRACT_REVIEW_TIER unavailable (HTTP 429 rate_limit, 2026-10-29T23:38Z, request req_011CgXQfQL9C34DUao8B3RNP) · head a5f0c8b

The seat's at-tier review call was refused by the model quota, and per contract-review.md the tier is not lowered. Owed on this head: one same-shape CONTRACT_REVIEW_TIER record and the maintainer's independent full Fable review of the spec change. One record with a genuine ④ can serve both, as on #27715.

@objectstack-fleet

Copy link
Copy Markdown
Contributor Author

triage#1 · os-sales · session_01L6bhgV4fMGJHSdSdx3u9mK

Contract-review-claim: session_01L6bhgV4fMGJHSdSdx3u9mK · head a5f0c8b5dcab727e54d4728c6b3f921b0e0bc903 · 2026-10-30T00:02Z


Generated by Claude Code

@objectstack-fleet

Copy link
Copy Markdown
Contributor Author

triage#1 · os-sales · session_01L6bhgV4fMGJHSdSdx3u9mK

Contract review

Served-tier: CONTRACT_REVIEW_TIER
Head-sha: a5f0c8b5dcab727e54d4728c6b3f921b0e0bc903
Local-runs: none
Scope: packages/spec/src/contracts/data-engine.ts + packages/objectql/src/engine.ts (DatasourceDef :4548-4568, registerDatasourceDef :9250-9262, applyExternalDriverFloor :9275-9293, listDatasourceDefs :9320-9327, registerApp datasource index :8648), packages/services/service-datasource/src/datasource-connection-service.ts (ConnectableDatasource :51-54, attemptConnect :666-678, reconnect :839-915), packages/services/service-datasource/src/datasource-admin-plugin.ts (reregisterPool :550-556), packages/services/service-datasource/src/datasource-admin-service.ts (StoredDatasource :139-142), packages/objectql/src/external-driver-floor.test.ts, packages/services/service-datasource/src/tests/datasource-connection-service.test.ts (fakeEngine :39-90, svc :111-128, the [#27727] block :1366-1419), packages/services/service-datasource/src/tests/datasource-update-rebuild.test.ts (:189-195), packages/services/service-datasource/src/tests/saved-datasource-test-after-failed-connect.test.ts (:81-83), packages/services/service-datasource/tsconfig.json, packages/cli/src/utils/secret-reference-union.ts (:313, :598-623), packages/objectql/src/datasource-def-credentials-ref.pin.ts (:52), docs/adr/0015-external-datasource-federation.md, content/docs/data-modeling/virtual-objects.mdx (:634), .changeset/27727-datasource-def-driver-contract-tidy.md

Review at CONTRACT_REVIEW_TIER on the schema face (one hits path) plus the declared narrowing face (Clause-②: yes (narrowing) on the PR body and the changeset: every non-prose path of the 7-file diff read, no path a remainder). Taken on the triage hand-off 6511103853 (HTTP 429 on the seat's at-tier call) under claim 6511462714; per that hand-off, this one record with a genuine ④ serves both the contract review and the maintainer's independent full review of the spec change at the reviewer tier. Inputs: card #27727 body and its one comment (claim 6509213861), PR #27788 body, file list and net diff against main at a5f0c8b5 (GitHub files endpoint), its four comments (drift advisory 6510722081, body addendum 6510733025, hand-off 6511103853, claim 6511462714), the prior record 6508089206 on PR #27715 whose ④ should-fixes the card files, and the 55 check-runs on a5f0c8b5. The two prose paths (the changeset and the mdx) are the lane seat's ACCEPT at TIER_DEFAULT; they were read here to judge the hits and the narrowing, never reviewed. Nothing was built, run or re-run.

① Derived judgments

  • IDataEngine.registerDatasourceDef? (data-engine.ts :455-477) and EngineDatasourceDef.driver (:243-261): doc-only hunks, no key added, removed or re-typed, so the accept set and the return shape of the contract are byte-for-byte what main publishes — RIGHT. Both {@link} targets resolve inside the file (IDataEngine.listDatasourceDefs is declared at :501; EngineDatasourceDef.driver at :261), so the hunks introduce no dangling reference. packages/spec/api-surface/contracts.json and export-origins/contracts.json are untouched, consistent with a doc-only change to an interface recorded by name; Lint & Repo Gates success on this head carries check:generated.
  • ObjectQL.listDatasourceDefs (engine.ts :9320-9327) now answers the stored driver id when one is held — a behavioural WIDENING of the answer within a key the return type already declared (DatasourceDef.driver? :4553, and the contract's EngineDatasourceDef.driver? since service-datasource: a runtime-origin or reconnected plugin-driver datasource loses the #27277 external-driver floor, because the connection service registers the datasource def without its driver id and unregisterDriver drops the stored one (epic #27261) #27666); the method's type, the contract member and the datasource-def-credentials-ref.pin.ts shape pin (:52) all move nothing — RIGHT. Read in the consumers: secret-reference-union.ts reads external.credentialsRef only (:598-623) and types the port as DatasourceArtefactLike (:313); the cli test doubles answer their own lists. No in-repo reader does equality on a def the engine holds a driver for; the existing engine toEqual pins carry no driver. Test Core 8/8 success confirms.
  • ObjectQL.registerDatasourceDef (:9250-9262): comment-only; the def.driver ?? stored.driver keep is as objectql: virtual-object correctness floor — applyDriverQueryInMemory helper, per-datasource row cap with a loud refusal, supports honoured for plugin drivers, refusal-envelope mapping for driver failures (epic #27261 P1) #27277 and service-datasource: a runtime-origin or reconnected plugin-driver datasource loses the #27277 external-driver floor, because the connection service registers the datasource def without its driver id and unregisterDriver drops the stored one (epic #27261) #27666 left it — RIGHT; the engine remains the implementer the contract doc now describes.
  • DatasourceConnectionService.reconnect (datasource-connection-service.ts :839-854): opts loses its = {} default and opts.previous goes from optional to required (a Pick of ConnectableDatasource over driver, schemaMode and external, unchanged in shape), and a runtime guard (:847-854) refuses a call whose previous.driver is not a non-empty string BEFORE getDriverByName, unregisterDriver or the factory is touched — an accept-set NARROWING on an exported class of a released package (@objectstack/service-datasource), the declared face — RIGHT. Since ConnectableDatasource.driver is a required string (:54), the type level already forces a driver id into previous; the runtime guard is for JS hosts and the empty string. Every in-repo caller passes previous with a required-string driver: reregisterPool (datasource-admin-plugin.ts :550-556) passes the whole StoredDatasource (driver: string, datasource-admin-service.ts :142); the two test seams (datasource-update-rebuild.test.ts :189-195, saved-datasource-test-after-failed-connect.test.ts :81-83) do the same. TypeScript Type Check and the four Type Check · gates success on this head.
  • A consequence the PR body does not spell out, judged RIGHT: the guard precedes the !oldDriver fallthrough (:856-864), so a reconnect for a datasource with NO live pool, which on main degraded to a plain connect without evicting anything, is now refused too when previous is missing. One rule for the whole method is the shape the required type already states, and the BREAKING banner's population ("a host that calls reconnect without previous") covers this caller.
  • external-driver-floor.test.ts and the service test file: test-only, no published surface — RIGHT. The @ts-expect-error at :1382 is a real pin: packages/services/service-datasource/tsconfig.json includes src and excludes only node_modules and dist, so the package tsc compiles __tests__, and an unconsumed directive would have redded TypeScript Type Check.
  • content/docs/data-modeling/virtual-objects.mdx :634: prose, no surface — RIGHT in substance (read to judge the citation, see ③).

② Semver level

  • .changeset/27727-datasource-def-driver-contract-tidy.md: @objectstack/spec: patch, @objectstack/objectql: patch, @objectstack/service-datasource: patch; line-initial Clause-②: yes (narrowing) — … and Metadata-protocol: none, matching the PR body's two lines; one **BREAKING** banner naming the affected population (a host calling reconnect without previous; none in-repo); exactly one ADR-0087 marker, not-required (no-migration-prescription), with its reason (no authored-schema key moves; the narrowed parameter is on a host-facing service method); a "What to do" migration line. Level follows the kind: Metadata-protocol: none is patch on every package the diff moves, breaking or not, with the banner and disposition kept (AGENTS.md Post-Task Checklist 3). Check Changeset success twice, PR-body guards success, Governed Surface Queue Guard success, Lint & Repo Gates success on this head. CONSISTENT.
  • Clause-②: the claim 6509213861 dispatched no (narrowing); the PR body and changeset say yes (narrowing) and say why (the listDatasourceDefs answer widens). The yes rests on a behavioural fill of an already-declared optional key, not on a type change; it is the arm that owes MORE review, never less, and a wrong no is the auditable false declaration, so the move is the safe direction. (narrowing) is the arm that carries the BREAKING signal and is exactly what the required previous is. The two carriers (body and changeset) agree with each other, which is what the gates read. RIGHT.
  • @objectstack/objectql carries an entry this time (its listDatasourceDefs behaviour changes), where fix(service-datasource): a plugin-driver datasource connected or reconnected through the connection service keeps the engine's floor #27715's did not (comment-only there). RIGHT.
  • Metadata-protocol none: no authored key, value or default moves — a datasource author writes nothing differently. RIGHT.

③ Boundary flags

  • Author objectstack-fleet[bot]; head repo = base repo; PR is draft (draft: true) with needs:contract-review; branch claude/project-thread-eecjmj, as the claim 6509213861 names (the card thread's platform-assigned branch, measured exception to the issue-slug form). Implementing session per the claim is the card thread's; the record signs the branch.
  • Check-runs on a5f0c8b5: 55 runs, 42 success, 13 skipped, 0 failure. Every gate-carrying context is success: Test Core 8/8 shards and the aggregate, TypeScript Type Check and the source/consumer/workspace/debt-ledger type gates, every Lint · context including PM dispatch-gates, Spec property liveness, Governed Surface Queue Guard, Check Changeset (both runs), PR-body guards, Dogfood Regression Gate, Build & E2E UI, Live E2E UI, Type Check UI, Check Documentation Links, Flag docs affected by code changes. The skipped rows are the opt-in harnesses (18.x upgrade, packed tarball, temporal conformance) and path-filtered matrices (UI shards, Lint UI). The hand-off's "41 success, 9 skipped" was counted before a second workflow set attached; the API count above is what was read. The PR body's local evidence (9247/9247, 1013/1013, 49/49, dispatch-gates --ran 109/109) is consistent with the tree but was not relied on.
  • Dev flag on the PR body that a review at the reviewer tier is owed before landing — answered by this record. No open_questions on the PR body or the card.
  • Body addendum 6510733025 (the third declaration line and the maintainer summary ride a comment because the relay refuses a body patch under the attribution marker): Skills-docs: none — verified at this head: a grep over content/docs/**, skills/** and .claude/skills/** for reconnect( or listDatasourceDefs hits only content/docs/protocol/kernel/realtime-protocol.mdx, whose reconnect is the websocket client's, unrelated. No page teaches either member.
  • Docs drift advisory 6510722081 lists 14 hand-written pages by the IDataEngine and schemaMode anchors; none documents reconnect's previous or the keys listDatasourceDefs answers, so nothing restates what moved. The four release-owned pages are untouched by the diff.
  • Card spec/service-datasource: tidy the EngineDatasourceDef.driver contract after #27666 — member doc, listDatasourceDefs read-back, required reconnect previous, stale test and guide lines (epic #27261) #27727 task list against the tree, all five done: (1) member doc carries the exception (data-engine.ts :474-476); (2) listDatasourceDefs returns driver (engine.ts :9323), the stronger of the two arms offered; (3) previous is required AND the restore refuses a def it cannot name the driver of (:842, :847-854), both arms; (4) the objectql: virtual-object correctness floor — applyDriverQueryInMemory helper, per-datasource row cap with a loud refusal, supports honoured for plugin drivers, refusal-envelope mapping for driver failures (epic #27261 P1) #27277 keep test names the caller class it models (external-driver-floor.test.ts :463, :468-471); (5) the "Tier A" line is gone from the mdx (:634). The prior record 6508089206 ④ should-fixes 1-4 and its ③ prose note are each closed by one of these.
  • mdx :634 replacement, read against ADR-0015 to judge the citation: ADR-0015 has no tier vocabulary (no "Tier" anywhere in the ADR), and it does govern Datasource.schemaMode, Object.external and write safety (its §5.3 Gate 3 is what the code cites as the datasource-wide write gate). The new line is accurate. Prose face; lane seat's ACCEPT.
  • Clause-② arm moved from the claim's no (narrowing) to the PR's yes (narrowing) — recorded in ② as right; noted here so the epic PM's dispatch line and the landing carriers are seen to differ on purpose, not by drift.

④ Design judgment

packages/spec/src/contracts/data-engine.ts (hits) — judged against the ruling it implements (card #27727 Task 1-2, filed off 6508089206 ④ should-fix 1-2, which asked that the member doc carry the exception and that the read-back either answer driver or say it omits it), against its implementer (ObjectQL.registerDatasourceDef :9250-9262, listDatasourceDefs :9320-9327) and against its first consumers (DatasourceConnectionService.attemptConnect :673-678 and reconnect.restoreOldPool :869-877, which write through registerDatasourceDef?; secret-reference-union.ts :598-623, which reads listDatasourceDefs?):

  • The member doc now states the one exception the engine actually has. Read in the implementer: const driver = def.driver ?? this.datasourceDefs.get(def.name)?.driver then set(name, { driver, schemaMode: def.schemaMode, external: def.external }) — every key but driver IS last-write-wins (an absent schemaMode or external overwrites with undefined), and driver alone keeps. The doc says exactly that and no more. Holds.
  • The key doc's new claim, "listDatasourceDefs answers the stored id", holds against the only implementation in the tree: :9323 spreads driver when defined, and the spread order keeps name first and driver before schemaMode, so a definition that never named one is still listed without the key (pinned at external-driver-floor.test.ts :612-616). Tried to break it on both entry routes the listDatasourceDefs? doc names: the manifest path (registerApp :8648 calls registerDatasourceDef(ds) with the manifest datasource, whose driver rides along; pinned :603-611) and the member path (pinned :591-602, including the KEPT id after a driver-less re-registration). A sweep asking "which datasources name a plugin driver" can now be typed against the contract and answered by the engine. Holds.
  • The doc-link direction is the right one: the key doc points at the read-back member, the member doc points at the key. The listDatasourceDefs? member doc itself (:480-500) still describes the answer in terms of credentialsRef and says nothing of driver. Not a contradiction (it says UNFILTERED, every definition, fresh copies) — a nit, no gate: one clause there naming driver as an answered key would make the read-back doc self-contained for the sweep author who starts from the member, not from the key.
  • The "deliberate SUBSET" header rule holds: no key is added; the engine reads every key the contract declares.
  • No must-fix on this file. No should-fix on this file.

Narrowing face — DatasourceConnectionService.reconnect requires previous (every non-prose path read), judged against the published spec on main (the method as @objectstack/service-datasource exports it, where previous was optional and its doc already said the kept-pool restore needs it) and against the scenario the card accepts it for (a host calling reconnect without previous after an eviction gets a kept plugin pool restored under a def naming no driver, served unfloored, silently):

  • Does it take a declared capability away at the consumer? Yes, one: a reconnect call without previous. On main that call was never a correct call for a plugin driver (the restore registered driver: undefined, and after unregisterDriver dropped the stored def there was nothing to keep — the engine comment at :9254-9258 says so in as many words), so what is removed is the way to reach the defect, not a working path. Declared with the BREAKING banner and the one-line fix. RIGHT.
  • Does it make the spec's own refusal real? Yes: the method's own doc on main already said the restore "must restore the def the old pool was serving under"; a call that cannot name that def's driver is now refused instead of being honoured wrongly. RIGHT.
  • Tried to break the guard in the live code:
    • Placement. The guard is the first statement after const name = record.name, before this.cfg.engine(), before getDriverByName, before unregisterDriver, before the factory. A refused call therefore cannot have evicted or built anything, as the new pin asserts (:1388-1390: same live driver instance, same def count, factory called once). Holds.
    • Fake-engine fidelity of that pin. fakeEngine.registerDriver keys by driver.name, and the service stamps the datasource name onto the driver before registering (:647-656), so engine.drivers.get('tree') is the live pool; unregisterDriver is drivers.delete. The pin's identity and count checks therefore read the real seam, not a convenience. Holds.
    • The '' arm. typeof '' === 'string' passes the first clause; the explicit === '' catches it; the message's second sentence ("with its driver id") is what the pin matches at :1386. Holds. Whitespace-only is not caught — a nit, no gate; resolveDriverId would answer undefined for it and the floor would APPLY, which is the safe direction.
    • A JS host passing no opts at all: opts?.previous?.driver is undefined, refused; record.name is read first and would throw a TypeError on a missing record exactly as main did. Holds.
    • Kept-pool path with previous. restoreOldPool now reads opts.previous.driver without optional chaining (:874-876), type-safe under the required field, and registers the OLD id, not record.driver; the service pin at :1414-1417 drives a plugin-to-sqlite reconfigure whose rebuild fails and asserts the last def names spike_tree. Against the engine (read, not run): registerDriver(wrapper) then applyExternalDriverFloor with no def unwraps, registerDatasourceDef({ driver: 'spike_tree' }) re-floors via the cached wrapper — the same convergence 6508089206 ④ traced, now with the id guaranteed present. Holds.
    • Error style. A bare Error with a prescriptive message matches the file's other refusals (:392, :430-434, :1042). The message names the datasource, the argument, why, and the fix. The clause "a kept pool would be restored under a definition that names no driver" is literally true only when a live pool exists; on the no-live-pool branch the same refusal fires with a reason that does not describe that branch. Should-fix, no gate: either let the message say "would be, when a pool is live" or order the guard after the !oldDriver fallthrough so the degraded plain-connect path keeps working without previous. Preferring the former: the type is already required, and two rules for one parameter is the shape this card exists to remove.
  • listDatasourceDefs widening, read as part of the face: a plugin-driver sweep written against the contract gets the id from both entry routes; a managed or built-in datasource is listed with its id too, so the sweep decides with resolveDriverId, not with key presence. No consumer narrows on the key's absence. RIGHT.
  • Test framing (external-driver-floor.test.ts :463-471): the keep test now says the connection service is NOT the caller it models and points at the [service-datasource: a runtime-origin or reconnected plugin-driver datasource loses the #27277 external-driver floor, because the connection service registers the datasource def without its driver id and unregisterDriver drops the stored one (epic #27261) #27666] block that pins what the service does register. True at this head (:673-678, :869-877 both name driver). Holds.

Findings: no must-fix. One should-fix (the refusal message on the no-live-pool branch), two nits (the listDatasourceDefs? member doc naming driver; whitespace-only driver not refused). None opens a path; none blocks the landing.

Implemented-by: claude/project-thread-eecjmj
Reviewed-by: session_01L6bhgV4fMGJHSdSdx3u9mK

VERDICT: PASS


Generated by Claude Code

@objectstack-fleet

Copy link
Copy Markdown
Contributor Author

epic:#27261#1 · marchtian · session_017ihL2c5z3q4h8inrn5kwqV
ACCEPT · PR #27788 @ a5f0c8b5dcab727e54d4728c6b3f921b0e0bc903 · 2026-10-30T00:13Z

维护者速读

  • 改了什么: 数据源连接服务的 reconnect 现在必须带 previous(原记录及其驱动 id),没带就在驱逐任何东西之前直接拒绝;引擎的 listDatasourceDefs 回读里加上存储的驱动 id;spec 合约文档写明"不带 driver 的再次登记保留原 id"。
  • 为什么改: fix(service-datasource): a plugin-driver datasource connected or reconnected through the connection service keeps the engine's floor #27715 的复核指出:不带 previous 时,重建失败后恢复的旧连接池会以"未点名驱动"的定义登记,插件驱动就绕过了兜底层,而且没有任何提示。
  • 风险与回滚: 仓库内唯一调用方已经传 previous;只有宿主自己直接调 reconnect 而不传 previous 时会变成类型错误和运行时拒绝。changeset patch · Clause-②: yes (narrowing),带 BREAKING 横幅。还原即回滚。
  • 先看: packages/services/service-datasource/src/datasource-connection-service.ts 的 reconnect 开头的守卫。
  • 两分钟验证: 单跑 service-datasource 的 datasource-connection-service.test.ts 中 [#27727] 两个用例。
  • 需要你做的: 无,席位落地。

席位意见:ACCEPT。契约复核 PASS 在案(分诊 6511604842,CONTRACT_REVIEW_TIER,就是当前 head,同一记录兼作 spec 改动的独立完整复核),无 must-fix。

席位核对

  1. Shape: +144/−14, 8 files. reconnect's previous goes from optional to required. listDatasourceDefs adds the stored driver key. data-engine.ts changes doc text only.
  2. Premise measured: on main, reconnect(record, {}) restored a kept pool under a definition naming no driver. The new test pins the refusal before eviction (live driver, defs and factory call count unchanged), and the keep-old-pool restore under the OLD driver id.
  3. Gate check-runs on a5f0c8b5: 58 runs: 42 success, 15 skipped, 0 failed. Check Changeset is re-running after a label event; the merge queue gates on it.
  4. Accepted deviations: the 维护者速读 and the Skills-docs line (none) are in PR comment 6510733025, because the relay refuses a body patch on this PR.
  5. Out of scope, routed: the record's should-fix (the refusal message says a kept pool "would be restored" on the no-live-pool branch, where none is) goes to service-datasource: a datasource driver resolves a borrowed connector credential through ADR-0143 D4's per-dispatch resolver (D7 runtime seam) — epic #27261 P2 #27746, which edits datasource-connection-service.ts next. The nit on the listDatasourceDefs? member doc naming driver rides spec: composed stacks refuse a datasource whose external.connector names no declared connector instance (composed counterpart of the #27742 check, #25211 shape) — epic #27261 #27803 (spec). The whitespace-only driver nit fails in the safe direction and is not carried.
  6. Landing: not governed (check-governed-merges --pr 27788: 158 lines ≤ 3000). A same-shape PASS stands on the current head, so needs:contract-review comes off. Landed by the seat through the relay.

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