Skip to content

Commit a35fb43

Browse files
os-steveclaude
andauthored
fix(service-datasource): rebuild the live pool when an update changes connectivity-bearing fields, and make active: false actually disable (#14196)
* fix(service-datasource): rebuild the live pool when an update changes 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 * chore: add changeset for #13804 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016ZC5rNQj3WEet5HAmmAkMs * fix(service-datasource): rule schemaMode into the connectivity-bearing 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 --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent 2e3e8c7 commit a35fb43

7 files changed

Lines changed: 858 additions & 2 deletions

File tree

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,45 @@
1+
---
2+
"@objectstack/service-datasource": minor
3+
---
4+
5+
Updating a datasource now rebuilds its live pool when the change actually bears on connectivity — and `active: false` actually takes it out of service
6+
7+
`updateDatasource` persisted the merged record and called `registerPool`, whose
8+
connect-path idempotency guard answered `already-registered` while the OLD
9+
driver held the name and returned before building anything. Nothing on the
10+
update path called `disconnect` first. So reconfiguring a datasource — new
11+
host, new credentials, new pool settings, `active: false` — changed the stored
12+
record and left the running connection untouched until process restart, while
13+
`toSummary` kept reporting the ORIGINAL connect's retained `connected`: a
14+
successful save describing a pool the record no longer declared. The
15+
`active: false` corner is security-adjacent — an explicitly disabled data
16+
plane kept serving.
17+
18+
What "Save" now means for the live pool, decided by what actually changed:
19+
20+
- **A connectivity-bearing field changed** (`driver`, `config`, `external`
21+
including `credentialsRef`, `pool`, `schemaMode`, `active` — the set verified
22+
against what `attemptConnect` reads into driver construction; a supplied
23+
secret counts too, since a rewrap-in-place changes the credential without
24+
changing the ref) → the pool is rebuilt in place via the new
25+
`DatasourceConnectionService.reconnect`: the old registration is evicted
26+
(the #13578 door), a new driver is built FROM THE NEW RECORD through the one
27+
shared connect path, and the replaced pool's connection is closed.
28+
- **The rebuild fails** → the OLD driver instance is restored (with the
29+
datasource def eviction removed alongside it), so the datasource is never
30+
left pool-less: the previous configuration keeps serving while the retained
31+
verdict is loudly degraded and says exactly that. Runtime-admin writes still
32+
never brick a running server over a UI action.
33+
- **`active: false`** → the pool is torn down and the registry stops answering
34+
the name — matching every other lifecycle door (`connectDeclared` and boot
35+
rehydration never build a pool for a disabled record). `createDatasource`
36+
gets the same guard: a datasource born disabled no longer comes up serving.
37+
- **Nothing connectivity-bearing changed** (a label edit) → the idempotent
38+
no-op path, exactly as before: same driver instance, no eviction, no
39+
connection churn.
40+
41+
Hosts wiring `DatasourceAdminServiceConfig` directly get the rebuild by
42+
supplying the new optional `reregisterPool` seam; without it the behaviour is
43+
unchanged (the idempotent register). The comparison itself is exported as
44+
`datasourceConnectivityChanged` so a custom seam can ask the same question the
45+
shipped update path asks.

0 commit comments

Comments
 (0)