Uh oh!
There was an error while loading. Please reload this page.
fix(service-datasource): rebuild the live pool when an update changes connectivity-bearing fields, and make active: false actually disable - #14196
Conversation
… connectivity-bearing fields (#13804) updateDatasource persisted the merged record and called registerPool, whose connect-path idempotency guard answered already-registered while the old driver held the name — so the running pool never followed the record, and toSummary kept reporting the original connect's retained 'connected'. An explicitly disabled datasource (active: false) kept serving until restart. Ruled decision tree: rebuild only when driver/config/external(credentialsRef)/ pool/active actually changed; active: false tears the pool down; on rebuild failure the OLD pool is kept live under a loudly degraded verdict (never pool-less). Label-only edits keep the idempotent no-op path: same driver instance, no churn. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016ZC5rNQj3WEet5HAmmAkMs
…tasource-update-rebuild
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016ZC5rNQj3WEet5HAmmAkMs
📓 Docs Drift CheckThis PR changes 1 package(s): 2 hand-written doc(s) NAME something this change touched and may need an implementation-accuracy re-verification:
⛔ 1 release-owned page(s) also name something this change touched. These are read-only:
What this run could not see
Coarse fallback — 0 page(s) merely mention a changed package (the pre-#9192 predicate, kept for the deliberately-wide backstop): Which tree this was computed onThis run read A worktree cut from an older # while this PR is open — GitHub drops the merge commit once it closes
git fetch origin 1d366144835d182a880bb47ba76a04c585d730cc && git checkout 1d366144835d182a880bb47ba76a04c585d730cc
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin f394614486a47ca23c3b9fb5978f2eed39f4f8d4 84cb0b6e6174b244cf33b901f74ef404b88f26cf && git checkout -B drift-repro f394614486a47ca23c3b9fb5978f2eed39f4f8d4 && git merge --no-ff 84cb0b6e6174b244cf33b901f74ef404b88f26cf
node scripts/docs-audit/affected-docs.mjs --json f394614486a47ca23c3b9fb5978f2eed39f4f8d4
|
os-steve
commented
Sep 1, 2026
PM 复核 — 实现 ACCEPT;⛔ Clause-② 由 |
| 主张 | 复核 |
|---|---|
| 在 update 路径可 patch | ✅ datasource-admin-service.ts:401 — ...(patch.schemaMode !== undefined ? { schemaMode: patch.schemaMode } : {}) |
| 进 connect 策略闸 | ✅ datasource-connection-service.ts:269 — if (ds.schemaMode && ds.schemaMode !== 'managed') return true; |
进 toSpec → factory.create | ✅ :881 |
进 registerDatasourceDef | ✅ :639 |
⇒ 裁定:⛔ 不并入本 PR(裁决固定了六字段集合,扩集合不是实现者的裁量,你没自行加是对的),另立卡。已立 —— 见下方新卡编号,PR 无需改动。
⛔ 不采 C(在 update 里拒绝 patch schemaMode):那是移除一个已发布的编辑面,是本卡没有开启的契约变更 —— 你这条理由我采纳。
三、我复核过的其它几点
- 反向对照是 identity 而非 equivalence(
toBe(oldDriver)+factory.created计数为 1 +evicted为 0)⇒ ⭐ 这正是把「放着没动」与「重建了一个一样的池」区分开的唯一测法,被否掉的 always-swap 方案会在 equivalence 检查下蒙混过关。测试里那句注释把这一点写明了,保留。 - 失败路径三条断言齐全,且
restoreOldPool连 def 一起恢复 —— 因为unregisterDriver会把 def 与 driver 一并移除。这一条不是理论:fake engine 刻意镜像了这个语义,注释也说明了「这两个行为正是修复必须长成这样的原因」。⭐ 这是本 PR 最容易被做错而你做对了的地方。 - 消融方向先写后测,5 红 / 7 绿与预测逐条吻合;盘上用 marker count + blob(
08357aevs HEAD79bc33e)证实,恢复按状态证明。⛔ 声明的控制组保持绿 —— 你把它当控制组用、⛔ 没当消融证据,正确。 - bounded in-place fix(
createDatasource补active:false闸)四条件成立:同缺陷类、机械守卫且正确形状有既有证据(boot 两条路径)、同文件、同门族。⭐ 一个「生来就是 disabled 却带着活池起来」的数据源,与本卡的active:false肢是同一个洞的另一扇门 —— 接受。
四、待办
CI 仍在跑(29 条,多条 in_progress,⛔ 无 failure)。入队资格 = 每一条 check 出结论且全绿,⛔ 不是 required 子集 —— 全绿 且 契约复审 PASS 之后,本席转 ready + 挂 auto-merge。⛔ 你不必再动手,⛔ 也不要自行转 ready。
Generated by Claude Code
os-steve
commented
Sep 1, 2026
Fork card filed: #14198 ( (Completing the reference from my review above, which named the card before it existed. Generated by Claude Code |
os-steve
commented
Sep 1, 2026
⏸ 落地暂停 —— 卡在契约复审档位不可用,⛔ 不是卡在这个 PR
⛔ 本席不会降档代跑这次复审。 低于档位的复审是无效的 —— 这正是那道熔断存在的理由,绕过它等于给一个从未发生过的审查盖章。⇒ 宁可明说被挡住,也⛔ 不产出一份假的通过。 这个 PR 自身的状态(⛔ 与上面无关)
一条给复审者的现成线索(档位恢复后直接用)复审 agent 死前的最后一句是它已确认的进度:"Engine semantics all confirmed. Now ⇒ ⭐ 那正是本 PR 最该被审的一点:「停止服务」在这一缝的定义,取决于 档位恢复即跑复审;PASS 且 CI 全绿则转 ready + auto-merge。 Generated by Claude Code |
huangyiirene
commented
Sep 1, 2026
契约复审:有条件 PASS —— head |
os-steve
commented
Sep 1, 2026
执行席接令 —— 契约复审已收到,⛔ 更正我两条对外的假陈述
⛔ 先更正:我说过两次「本 PR 的契约复审从未运行」——假的复审早在 13:49:39Z 就贴出来了。而我在 ~14:20 读过本 PR,返回里带着 ⇒ 机制与本席刚记进座位贴的常设修正 28 完全同型:断言「某物不存在」,却不读手上已有的原始记录。
采纳的裁决要点(⛔ 不重述,只记落点)
待修两处(⛔ 只此两处,⛔ 不夹带)
排期(⛔ 如实说明,不含糊)⛔ 此刻不派 —— 维护者本轮指示并发上限 = 2,两个位都在飞(#14184、#14021)。⇒ 下一个位空出即派这两处修复;它们很小(一词 + 一行 + 一钉),⛔ 但不因为小就插队越限。 修毕推同支 ⇒ 交总监席复核增量后同笔清标。⛔ 本席不自行清 Generated by Claude Code |
…tasource-update-rebuild
…g set (#13804) Contract-review increment on PR #14196 (director-seat conditional PASS, comment 5494985273). Two changes, exactly as ruled: 1. The changeset for @objectstack/service-datasource moves patch -> minor. The diff adds public API — the `datasourceConnectivityChanged` and `ConnectivityBearingFields` exports, the public `DatasourceConnectionService.reconnect` method, and the published `DatasourceAdminServiceConfig.reregisterPool` member — which is additive widening = minor by repo convention (#13897 is the same shape). `patch` under-reported the surface movement. 2. `schemaMode` joins the connectivity-bearing field set. It was found during this card's premise verification and reported as a fork rather than added unilaterally; the review ruled it IN in the same stroke. It is really read at three sites on the connect path — the `canConnect` policy gate, `toSpec` -> `factory.create` (driver construction), and `registerDatasourceDef` (the write gate's def) — and it is patchable by `updateDatasource`, so omitting it left a schemaMode-only save persisting the new record while all three kept the OLD value until restart: a narrower instance of the stale-pool defect this card fixes. One comparator line, plus the exported field slice, plus one comparator pin. The module docblock now states the resolution instead of carrying the fork as an open question. The label-only reverse control (same driver instance, zero factory calls, zero evictions) stays green, which is what shows the set widened by exactly one member rather than into "rebuild on everything". Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016ZC5rNQj3WEet5HAmmAkMs
huangyiirene
commented
Sep 1, 2026
契约复审增量复核:PASS —— head |
Uh oh!
There was an error while loading. Please reload this page.
Increment record — head |
Fixes#13804
The defect
DatasourceAdminService.updateDatasourcepersisted the merged record, then calledtryRegisterPool(merged)→DatasourceConnectionService.connect→attemptConnect, whose idempotency guard answersalready-registeredand returns BEFORE building anything whileengine.getDriverByName(name)already answers. Nothing on the update path calleddisconnectfirst. So reconfiguring a datasource — new host, credentials, pool settings,active: false— changed the stored record and left the running connection untouched until process restart, whiletoSummaryreported the ORIGINAL connect's retainedconnectedverdict: a successful save describing a pool the record no longer declares.active: falsenot taking effect is the security-adjacent corner: an explicitly disabled data plane kept serving.What lands (the ruled decision tree, implemented exactly)
driver,config,externalincludingcredentialsRef,pool,schemaMode,active—schemaModejoined by the contract review's ruling, see the increment section at the end). The comparison is a new pure module,datasource-connectivity-change.ts, exported for hosts that wire their own seam. A supplied cleartext secret counts as a connectivity change the record diff cannot see: a rewrap-in-place keeps the ref string while changing what it dereferences to, and the pool reads the credential only at build time.DatasourceConnectionService.reconnect: evict the old registration through the existingIObjectQLEngine.unregisterDriverdoor (landed by the eviction card; this PR reinvents nothing — the teardown path's existing call sites indisconnectand the failed-start rollback were read first), rebuild from the NEW record through the one shared connect path, close the replaced pool (unless host-owned/adopted, ADR-0062 D5).unregisterDriverremoves the datasource def together with the driver, the def the old pool was serving under is restored from the previous record. The retained verdict stays loudly degraded (error, orblockedon a policy refusal) and its reason says the previous configuration's pool is still the one serving. Never pool-less; runtime-admin writes still never brick a running server over a UI action.active: falsetears the pool down via the existingdisconnect(close + evict + verdict cleared). The registry stops answering the name, so routing stops serving it — matching boot, whereconnectDeclaredskips disabled records and rehydration filters onactive ?? true. The summary then readsunvalidatedwithactive: false— the same reading a fresh boot gives a disabled datasource.toSummaryis now consistent with the real pool state on every path (new pool ⇒ok; kept-old-pool failure ⇒error+ reason naming the truth; disabled ⇒unvalidated+active: false) because the retained verdicts now follow the pool lifecycle instead of being frozen at first connect.Hosts that wire
DatasourceAdminServiceConfigdirectly get the rebuild via a new OPTIONALreregisterPool(previous, next)member; without it the update tail falls back to the old idempotent register (pinned by a test) — the safe direction, since a teardown-then-register fallback would honour the rebuild but lose the keep-old-pool-on-failure half of the ruling.Premise verification (dispatch section 2) — the field set, read backwards from
attemptConnectWorked backwards from what
attemptConnectactually READS into driver construction, at the merge base (packages/services/service-datasource/src/datasource-connection-service.ts):record.driver,record.pool→assertDatasourcePoolSupported(pool-support gate);record.driver,record.schemaMode,record.external→ the connect-policy gatecanConnect;record.external.credentialsRef→ the fail-closed secret resolution (ADR-0062 D3);toSpec(record)→factory.create, readingname,driver,config ?? {}(the WHOLE block — no sub-key is excluded on this path, so no config sub-key fork exists from this vantage),schemaMode,external,pool,ssl;record.schemaMode,record.external→registerDatasourceDef(the write gate's def);record.active— read by NOTHING inattemptConnect; it is read byconnectDeclared(boot skip) and the rehydration filter, i.e. it governs whether a pool may exist at all.Verdict on the ruled set:
driver✓,config✓,external✓ (andcredentialsRefis a sub-key ofexternal, so the deep compare covers the ruled sixth member; the supplied-secret trigger covers the rewrap-in-place case the record diff cannot see),pool✓,active✓ (via pool existence),labelcorrectly outside the set.schemaMode✓ as well — it was reported here as a fork rather than added unilaterally, and the contract review ruled it into the set in the same stroke; it is a member as of the increment below.⭐ Seventh field, RESOLVED:
schemaModeis a member of the set. It is patchable byupdateDatasource(the merge copiespatch.schemaMode) and it is read on the connect path at three places — thecanConnectpolicy gate,toSpec→factory.create(driver construction), andregisterDatasourceDef(the write gate's def). Without it, a schemaMode-only edit persisted the new record while all three kept the OLD value until restart: a narrower instance of the same stale-pool shape this card fixes. The first push of this PR reported the fork rather than adding it — an implementer does not widen a ruled set on its own — and the contract review then ruled it IN in the same stroke (comment 5494985273), directing that it be added here rather than on a separate card. It landed in the increment below;datasource-connectivity-change.tsnow states the resolution instead of carrying it as an open question. Two candidates were examined and are deliberately NOT members:sslis read bytoSpecbut is not a field ofStoredDatasourceorDatasourceDraft, so it cannot change through update;autoConnectis neither patchable nor read byattemptConnect.Bounded in-place fix (declared):
createDatasourcehonoursactive: falseCreate called
tryRegisterPool(record)unconditionally, so a datasource BORN disabled came up serving — the same defect class as this card'sactive: falselimb, one door over. Four-condition exemption: same defect class (active not honoured on the runtime-admin path); mechanical guard whose correct shape is pinned by existing evidence (boot rehydration filtersactive ?? true,connectDeclaredskips disabled records); same file as this card's claim; same gate family, no new verification surface. One line plus a pin test.Verification
src/__tests__/datasource-update-rebuild.test.ts(12 tests) wires the REAL admin + connection services together exactly as the plugin does, against a fake engine mirroring the real registry semantics (keep-incumbent on collision; def removed with driver on evict — both are why the fix needs its shape):factory.createinput carries the NEW config (the pin is on what the pool was built from, not on any eviction call), old pool closed, summaryok.active: falsepin: registry stops answering the name (the engine routes by consulting the registry first, so this is what stopping means at this seam), old pool closed, summaryunvalidated+active: false. Plus the re-enable direction.credentialsRef: undefinedonto the merge) ⇒ no rebuild.error/failed-degradedwhose reason names both the connect failure and that the previous configuration is still serving, datasource def restored — never pool-less.config ?? {},active ?? true, undefined-valued keys absent).dc7c99a2b: vitest 29 files / 613 tests passed,tsc --noEmitclean — andtsc --listFilesconfirms the new test file is inside the typecheck program (measured, not assumed).08357aediffering from HEAD blob79bc33e; restore by state — hash back to the HEAD blob andgit diff HEADempty. Predicted directions written before the run and observed exactly: 5 red (positive config-change,active: falseteardown, re-enable, failure-path verdict, secret rewrap) / 7 green (both reverse controls, the create guard, the fallback pin, 3 comparator unit tests). Declared controls stayed green while the pins went red — the mutation is what the pins measure.node scripts/pm/dispatch-gates.mjs --commandsat headdc7c99a2b, post-merge of origin/main): all 35 harvested commands run locally, every exit captured before any pipe — 34 green pluscheck:nul-bytesgreen;check-test-completenessexited 3 = its own printed "NOT MEASURED" verdict (it parses a saved turbo test log that only CI produces) — recorded as not measured, not as green or red. Key verdict lines: engine-double-contract "678 (file, verb) row(s) held by the RETAINED ledger"; type-check-debt "OK — 27 ledger entr(ies) re-measured … none above its recorded number"; nul-bytes "OK (scanned 7757 text file(s) …)".@objectstack/runtimeand@objectstack/restrebuilt green against the NEW dist; the full 70-packageturbo run buildover./packages/*and./packages/*/*passed 70/70 at this head.@objectstack/cliwas not compiled locally: its failures name only its own unbuilt siblings (service-storage,client) absent from this worktree — a declared narrowing; CI runs the full farm.Boundaries held
content/docs/releases/untouched; the changeset is the release-notes input.noon the card (path limb: nopackages/spec/srcedits; content limb: the accept/reject displacement is enforcement of the spec's own declaredactivesemantics — argued in full in the claim comment, not defaulted).Contract-review increment (head
84cb0b6e6) — the two ruled fixes, nothing elseApplied on top of the reviewed head
dc7c99a2bafter the director seat's conditional PASS (comment 5494985273) and the PM's acceptance (comment 5495602405).origin/mainhad moved, so it was merged in (504a4d04c, clean, no conflict, nothing in it touchesservice-datasource); the branch was never rebased and never force-pushed.@objectstack/service-datasource:patch→minor. This diff adds public API — thedatasourceConnectivityChangedandConnectivityBearingFieldsexports fromindex.ts, the publicDatasourceConnectionService.reconnectmethod, and the publishedDatasourceAdminServiceConfig.reregisterPoolmember — which is additive widening = minor by repo convention (feat(spec,lint): a layout section can reference a declared field group instead of copying its members #13897 is the same shape).patchunder-reported the surface movement. The changeset prose now namesschemaModein the set it describes.schemaModejoins the connectivity-bearing set — one comparator line (before.schemaMode !== after.schemaMode), the field added to the exportedConnectivityBearingFieldsslice, and one comparator pin. Strict comparison with no default applied, because the connect path applies none either:toSpecomits the key when the record has no value, and the policy gate andregisterDatasourceDefreceive it raw. The three read sites were re-located in this tree rather than taken on trust (this PR had moved lines in both files):datasource-admin-service.ts(the merge copyingpatch.schemaMode), anddatasource-connection-service.tsfor thecanConnectcall, theregisterDatasourceDefcall, andtoSpec. The module docblock and theupdateDatasourceset-listing comment now state the resolution.Incremental verification (the accepted record above was NOT re-run)
if (before.schemaMode !== after.schemaMode) return true;gives 1 red / 12 green — the red is the new pin, and every one of the 12 pins the review already passed stays green, including the label-only reverse control (same driver instance, zero factory calls, zero evictions). That is what shows the set widened by exactly one member rather than into "rebuild on everything". Direction predicted before the run and observed exactly. Resolution path: the test imports the comparator relatively fromsrc, vitest transformssrc, nodistis involved — hence no build leg. Both legs proven on disk: mutation by anchor count (1 → 0) and blob hash5c2181ddiffering from the HEAD bloba9044f2; restore by state — hash back toa9044f2andgit diff HEADempty.84cb0b6e6: vitest 29 files / 614 tests passed,tsc --noEmitclean, andtsc --listFilesconfirms both edited files are inside the typecheck program (measured, not assumed).node scripts/pm/dispatch-gates.mjs --repo objectstack-ai/objectstack --commands, both output sections read whole): 35 harvested commands, every exit captured before any pipe — 34 green, pluscheck:nul-bytesgreen ("OK (scanned 7782 text file(s) ... no raw ASCII control bytes)").check-test-completenessexited 3 = its own printed "PREREQUISITE NOT MET / NOT MEASURED" verdict (it grades a saved turbo test log only CI produces) — recorded as not measured, neither green nor red. Verdict lines: engine-double-contract "OK — 743 pinned, 134 in the DEBT ledger, 3 exempt"; type-check-coverage "OK — 67/78 workspace packages type-checked (plus the root), 11 in the DEBT ledger, 1 exempt"; type-check-debt (the ratchet half) "check-type-check-coverage --re-measure: OK — 27 ledger entr(ies) re-measured in 331.5s, 1217 raw tsc error(s) total, none above its recorded number"; query-options-erasure "ratchet holds: 67 unswept non-test site(s) in 17 file(s), none new". The workspace closure was rebuilt 70/70 first, as the debt ratchet requires.pnpm lint(eslint . --no-inline-config) run whole at this head: exit 0. No narrowing claimed.Increment authored in session
session_016ZC5rNQj3WEet5HAmmAkMs.Generated by Claude Code
Generated by Claude Code