diff --git a/.changeset/datasource-config-driver-contract.md b/.changeset/datasource-config-driver-contract.md index f912af836a..c62c57dbfe 100644 --- a/.changeset/datasource-config-driver-contract.md +++ b/.changeset/datasource-config-driver-contract.md @@ -13,7 +13,7 @@ package. So `config: { hostname: 'db.internal' }` (the key is `host`) was accepted in silence and the datasource connected to `localhost` while the parse, the save and the connection probe all reported success. -`DatasourceSchema` now parses `config` — and each `readReplicas` entry — against +`DatasourceSchema` now parses `config` against the contract for the declared driver, and `DatasourceAdminService` (create/update/test, the Setup wizard's path) applies the same check. Both read one registry in `@objectstack/spec/data`, which also projects each contract to diff --git a/.changeset/datasource-read-replicas-removed.md b/.changeset/datasource-read-replicas-removed.md new file mode 100644 index 0000000000..fa75e64a7b --- /dev/null +++ b/.changeset/datasource-read-replicas-removed.md @@ -0,0 +1,49 @@ +--- +'@objectstack/spec': major +--- + +`datasource.readReplicas` is removed (#4468, ADR-0049 enforce-or-remove) + +It described replica connections nothing ever opened. `ConnectableDatasource` +and `DatasourceConnectionSpec` carry no replicas field, the driver factory never +reads the key, and no query path distinguishes a read from a write — the +platform has no read/write splitting at all, so every statement always went to +the primary no matter what was declared here. + +**Migration.** + +| Wrote | Write instead | +| --- | --- | +| `readReplicas: [{ host: 'replica-a', … }]` | delete the key | +| `replicas: [ … ]` (the alias) | delete the key | + +There is no target to move to, because there is no read-replica routing to move +to. If you need replica reads today, front them behind a single endpoint — +pgpool, ProxySQL, an RDS reader endpoint — and point `config` at that endpoint. +That is the one read-scaling path that works, and it worked before this key was +removed too. + +Run `os migrate meta --from 16` to strip it from your sources; the +`datasource-read-replicas-removed` conversion emits one notice per datasource. +Authoring it now fails the parse with the same prescription. + +**Why this one is worth reading about.** #4410 closed the `datasource.config` +gap and, in passing, extended the new per-driver validation over each +`readReplicas` entry — reasonably, since replicas carry the same shape. The +result was a slot that had every marker of a working feature: declared with a +doc comment, `.strict()`-guarded against typos at the top level, and +field-by-field validated against the driver's contract underneath. A replica +block with a misspelt `hostname` was rejected by index, naming the canonical +key. + +None of that is evidence of a consumer, and all of it reads like one. That is +the specific trap ADR-0049 exists for: rigor is cheap to add to a dead slot and +expensive to distinguish from life. Two independent surfaces had drawn the +wrong conclusion — this validation, and objectui's datasource preview, which +rendered a "2 read replicas" pill confirming the config to the author while +nothing routed a single read. The preview goes with the key (objectui side, +same change); `packages/spec/liveness/README.md` has the standing rule it +violated ("an authoring/preview renderer is NOT a runtime consumer"). + +Read-replica routing remains unbuilt. It is tracked as a feature request rather +than left as a schema key that looks like one. diff --git a/.changeset/unknown-key-strictness-data-step.md b/.changeset/unknown-key-strictness-data-step.md index 7cec7ffe89..452894afd6 100644 --- a/.changeset/unknown-key-strictness-data-step.md +++ b/.changeset/unknown-key-strictness-data-step.md @@ -26,8 +26,12 @@ Deliberately still tolerant: runtime shape the engine hands a handler. Strictness there would turn an engine-internal enrichment (as `provenance` was in #3712) into a breaking change for anyone parsing a context they were given. -- `datasource.config` and `readReplicas` — per-driver by construction; the - driver's own `configSchema` validates them. +- `datasource.config` — per-driver by construction (a sqlite `filename` and a + postgres `host`/`port` share no shape). Left open here and closed one level + down instead: #4410 parses it against the contract for the declared driver. + This bullet used to say "the driver's own `configSchema` validates them", + which was not true when it was written — the field existed and nothing read + it. Errors are self-fixing: connection keys written one level too high (`host`, `port`, `filename`, `url`, …) are prescribed into `config`; a top-level diff --git a/AGENTS.md b/AGENTS.md index d47db92f67..0b132ddf61 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -106,8 +106,24 @@ this is mandatory, not a preference (Prime Directive #11), and a PreToolUse hook blocks edits made while on the shared `main` branch. Working in the shared `main` checkout is *not* a supported fallback: branches get switched and shared files — including ones you just wrote — get reset *under you* mid-task (a full session's -work was silently reverted twice before this rule was enforced). Even inside your -own worktree, operate defensively: +work was silently reverted twice before this rule was enforced). + +**Claim the issue BEFORE you write any code.** Assign it to yourself +(`gh issue edit --add-assignee @me`, or the `issue_write` MCP tool with +`assignees`) as the *first* action of the task — before the worktree, before the +first read. An unassigned issue reads as an open invitation, and several agents +work this repo at once: two that both start on it burn the same hours twice and +then race to land conflicting shapes for the same problem, which is worse than +either one alone. If it is already assigned to someone else it is taken — pick +another, or say so and ask; never reassign it to yourself. + +The claim is also what makes the *finding* rule (Prime Directive #10) safe to +follow. Once out-of-scope discoveries become issues, the issue list is a real +queue other agents read, and a claim is the only thing separating "someone is on +this" from "nobody has looked yet". File it unassigned when you are merely +recording a finding; assign it at the moment you actually start. + +Even inside your own worktree, operate defensively: 1. **Only touch the files your task needs.** Don't "fix" unrelated diffs, reverts, or other agents' in-flight edits, and don't try to manage the whole diff --git a/CLAUDE.md b/CLAUDE.md index aa8db33739..3e0603ef5c 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -1,8 +1,19 @@ # CLAUDE.md **[AGENTS.md](./AGENTS.md) is the source of truth for working in this repo — read it.** -Its Prime Directives are binding. Do not rely on this file alone; the one rule that must -never be missed is inlined here because missing it corrupts other agents' work. +Its Prime Directives are binding. Do not rely on this file alone; the two rules that must +never be missed are inlined here because missing either one wastes or corrupts other +agents' work. + +## ⛔ Claim the issue before you write any code + +Assign the issue to yourself (`gh issue edit --add-assignee @me`, or `issue_write` +with `assignees`) as the **first action of the task** — before the worktree, before the +first read. Several agents work this repo at once and an unassigned issue reads as an +open invitation: two that both start on it burn the same hours twice, then race to land +conflicting shapes for one problem. Already assigned to someone else? It is taken — pick +another or ask; never reassign it to yourself. File findings unassigned when you are only +recording them; assign at the moment you start. ## ⛔ Worktree-first — before your FIRST file edit (AGENTS.md Prime Directive #11) diff --git a/content/docs/references/data/datasource.mdx b/content/docs/references/data/datasource.mdx index 0bea2ed219..a1e82c7093 100644 --- a/content/docs/references/data/datasource.mdx +++ b/content/docs/references/data/datasource.mdx @@ -36,7 +36,6 @@ const result = Datasource.parse(data); | **driver** | `string` | ✅ | Underlying driver type | | **config** | `Record` | ✅ | Driver specific configuration | | **pool** | `{ min: number; max: number; idleTimeoutMillis: number; connectionTimeoutMillis: number }` | optional | Connection pool settings | -| **readReplicas** | `Record[]` | optional | Read-only replica configurations | | **capabilities** | `{ transactions: boolean; queryFilters: boolean; queryAggregations: boolean; querySorting: boolean; … }` | optional | Capability overrides | | **healthCheck** | `{ enabled: boolean; intervalMs: number; timeoutMs: number }` | optional | Datasource health check configuration | | **ssl** | `{ enabled: boolean; rejectUnauthorized: boolean; ca?: string; cert?: string; … }` | optional | SSL/TLS configuration for secure database connections | diff --git a/content/docs/releases/v17.mdx b/content/docs/releases/v17.mdx index 3c3ac3da59..6da1f7711d 100644 --- a/content/docs/releases/v17.mdx +++ b/content/docs/releases/v17.mdx @@ -412,12 +412,13 @@ schema to the two highest-risk authorable surfaces, per the triage in - **Datasources** — `DatasourceSchema` with its `pool` / `healthCheck` / `ssl` / `retryPolicy` blocks, the ADR-0015 `external` federation settings and their `validation` policy, `DatasourceCapabilities`, and `DriverDefinitionSchema`. - `config` and `readReplicas` stay **open** records: their shape is per-driver. - Nothing validates *inside* them — an earlier version of this note said the - driver's own `configSchema` did, which was wrong; the per-driver schemas exist - (`PostgresConfigSchema` and siblings) but nothing parses `config` against - them, tracked as #4410. So a misspelling one level *down* is still silent - today. That openness is why the + `config` stays an **open** record: its shape is per-driver. What it no longer + is, is unvalidated — #4410 wired the per-driver schemas + (`PostgresConfigSchema` and siblings) into `DatasourceSchema`'s refinement, so + a misspelling one level *down* is now rejected with the canonical key named. + An earlier version of this note said the driver's own `configSchema` did that, + which was wrong for two releases: the field existed, nothing read it. That + openness is why the top level had to close — a connection key written one level too high (`host` next to `driver` instead of inside `config`) was stripped, and the datasource then connected on driver defaults rather than failing. Those keys now @@ -970,6 +971,7 @@ import or the authored key. | `DEFAULT_DISPATCHER_ROUTES` | dead route table | | Aspirational config on Theme / Translation / Webhook | still-dead after #3494 | | `ChartInteraction.zoom` / `.clickAction` | never implemented (#3752) | +| `datasource.readReplicas` | replica connections nothing ever opened — no driver reads the key and no query path splits reads from writes, so every statement went to the primary. #4410 had just taught the schema to validate each entry against the declared driver's contract, which made a dead slot look rigorously alive (#4468) | The Console side follows: `@object-ui/types` drops its `ObjectStack`/`ObjectOS`/`ObjectQL`/`ObjectUI` Capabilities re-exports, which @@ -1979,7 +1981,13 @@ covers are folded into the list below rather than left to the changelog.) an object fails the gate even at zero violations. - **Datasources:** verify every declared datasource connects in every environment — a bound datasource that cannot connect now fails the boot - instead of failing every later query. + instead of failing every later query. Delete `readReplicas` (`os migrate meta` + does it). Nothing ever opened those connections, so read throughput is + unchanged by removing them; if you need replica reads, front them behind one + endpoint (pgpool, ProxySQL, an RDS reader endpoint) and point `config` there. + Also re-check what you wrote under `config`: it is parsed against the driver's + contract now, so a key that used to be ignored — and left the datasource on + driver defaults — is rejected by name. - **Sharing rules:** rewrite `sharedWith.type: 'group'` → `'team'`; drop `guest` and owner-type rules; expect `accessLevel: 'full'` to convert to `'edit'`. **A rule must state its criteria** — authoring one without is rejected, and a diff --git a/docs/audits/2026-07-unknown-key-strictness-ledger.md b/docs/audits/2026-07-unknown-key-strictness-ledger.md index 5e6549f7e8..e08d8f5558 100644 --- a/docs/audits/2026-07-unknown-key-strictness-ledger.md +++ b/docs/audits/2026-07-unknown-key-strictness-ledger.md @@ -184,8 +184,8 @@ tightening (the #4001 "sharing-rule lesson": candidates, not verdicts). | `field.zod.ts` | 11 | authorable | partially strict | | `filter.zod.ts` / `query.zod.ts` | 11+5 | open | query dialect — user data flows through; validated semantically elsewhere. `query.zod.ts` dropped one site in #4196: `FieldNodeSchema`'s nested-select object form was declared-but-inert and narrowed to `z.string()`, so the union's second member is gone. Four more left in #4286 with the `joins`/`windowFunctions` removals: `JoinNodeBaseSchema`, `WindowFunctionNodeSchema`, and `WindowSpecSchema`'s two blocks (outer + `frame`) were deleted with their clusters. Class unchanged | | `driver-nosql.zod.ts` / `driver.zod.ts` / `driver-sql.zod.ts` | 10+9+2 | wire | driver capability contracts | -| `datasource.zod.ts` | 9 | authorable | **strict as of #4001 data step** — all 9: `DatasourceSchema` (+ `pool` / `healthCheck` / `ssl` / `retryPolicy`), `ExternalDatasourceSettingsSchema` (+ `validation`), `DatasourceCapabilities`, `DriverDefinitionSchema`. `config` + `readReplicas` stay `z.record` **at this level** by construction (per-driver shapes), but are no longer unchecked: **#4410** made `DatasourceSchema`'s refinement parse both against the contract for the declared driver (`driver/config-registry.zod.ts`), so the openness here is a shape this level cannot express rather than the absence of one. This row used to add "the driver's own `configSchema` validates them", which was false until #4410 landed the parse site it names | -| `driver/memory.zod.ts` / `driver/mongo.zod.ts` / `driver/postgres.zod.ts` | 6+1+1 | authorable | The per-driver shapes for the `config` slot — what an author actually writes under `datasource.config` (`host`, `port`, `filename`). **Undeclared here until the coverage walk went recursive** (see below): a subdirectory was invisible to the gate, so these sites sat outside the map while the map reported full coverage. **Strict as of #4410**, which is also what unblocked them: this row previously read "strictness here would enforce nothing" because nothing parsed `datasource.config` against these schemas and both `*DriverSpec.configSchema` literals were `{}`. Now `DatasourceSchema` parses `config` (and each `readReplicas` entry) against them, and the same schemas project onto `configSchema` and onto the Studio connection form. `postgres.zod.ts` drops a site: its `ssl` was a `boolean | {ca, cert, key, …}` union, and the object arm is gone — certificates now live in the datasource-level `ssl` block (declared, strict, and until #4410 read by nobody), leaving `config.ssl` as the on/off shorthand. That narrowing is forced by the same projection: the Studio form renders anything that is not boolean/enum/number as a TEXT INPUT, so a union here would have produced a wizard whose every `ssl` value the new gate rejects. `memory.zod.ts` keeps 6 but loses two KEYS — `indexes` / `maxRecordsPerObject`, which `InMemoryDriverConfig` has no field for, removed under ADR-0049 rather than blessed by the new gate | +| `datasource.zod.ts` | 9 | authorable | **strict as of #4001 data step** — all 9: `DatasourceSchema` (+ `pool` / `healthCheck` / `ssl` / `retryPolicy`), `ExternalDatasourceSettingsSchema` (+ `validation`), `DatasourceCapabilities`, `DriverDefinitionSchema`. `config` stays `z.record` **at this level** by construction (per-driver shapes), but is no longer unchecked: **#4410** made `DatasourceSchema`'s refinement parse it against the contract for the declared driver (`driver/config-registry.zod.ts`), so the openness here is a shape this level cannot express rather than the absence of one. This row used to add "the driver's own `configSchema` validates them", which was false until #4410 landed the parse site it names. #4410 extended the same parse to each `readReplicas` entry; **#4468 retired that key** — no driver ever opened a replica connection and no query path splits reads from writes, so the entries were being checked against a contract nothing would apply. Strictness makes a dropped key loud; it cannot make a slot live, and a *precisely validated* dead slot is the more convincing lie | +| `driver/memory.zod.ts` / `driver/mongo.zod.ts` / `driver/postgres.zod.ts` | 6+1+1 | authorable | The per-driver shapes for the `config` slot — what an author actually writes under `datasource.config` (`host`, `port`, `filename`). **Undeclared here until the coverage walk went recursive** (see below): a subdirectory was invisible to the gate, so these sites sat outside the map while the map reported full coverage. **Strict as of #4410**, which is also what unblocked them: this row previously read "strictness here would enforce nothing" because nothing parsed `datasource.config` against these schemas and both `*DriverSpec.configSchema` literals were `{}`. Now `DatasourceSchema` parses `config` against them, and the same schemas project onto `configSchema` and onto the Studio connection form. (#4410 also ran the parse over each `readReplicas` entry; #4468 retired that key outright — see the row above.) `postgres.zod.ts` drops a site: its `ssl` was a `boolean | {ca, cert, key, …}` union, and the object arm is gone — certificates now live in the datasource-level `ssl` block (declared, strict, and until #4410 read by nobody), leaving `config.ssl` as the on/off shorthand. That narrowing is forced by the same projection: the Studio form renders anything that is not boolean/enum/number as a TEXT INPUT, so a union here would have produced a wizard whose every `ssl` value the new gate rejects. `memory.zod.ts` keeps 6 but loses two KEYS — `indexes` / `maxRecordsPerObject`, which `InMemoryDriverConfig` has no field for, removed under ADR-0049 rather than blessed by the new gate | | `driver/mysql.zod.ts` / `driver/sqlite.zod.ts` | 1+2 | authorable | The rest of the `config` contract, added by #4410. `mysql.zod.ts` and `sqlite.zod.ts` (sqlite + sqlite-wasm) are shapes that **never existed** — both driver ids were offered by the connection form and buildable by the shared factory, with no config contract anywhere, so `driver: 'sqlite'` + a misspelled `filename` was an ephemeral `:memory:` database reported as configured. All three sites strict, same error factory as the rest of the campaign. (Their sibling `driver/common.zod.ts` holds shared enums and prescription strings and has no `z.object(` site, so the coverage gate skips it) | | `analytics.zod.ts` | 8 | mixed (p) | | | `document.zod.ts` | 8 | wire (p) | | diff --git a/docs/protocol-upgrade-guide.md b/docs/protocol-upgrade-guide.md index c6b7cf8168..49ec3e58a3 100644 --- a/docs/protocol-upgrade-guide.md +++ b/docs/protocol-upgrade-guide.md @@ -156,6 +156,8 @@ The #4286 close-out settles the remaining three. `having` is ENFORCED, not remov The same kind of retirement covers `wait`'s timeout pair (#4158). `waitEventConfig.onTimeout` had ZERO readers — no path ever inspected it, so neither `fail` nor `continue` ever happened, while its `.default('fail')` stamped a decision nothing made onto every wait node. `waitEventConfig.timeoutMs` said "maximum wait time before timeout" and its only reader used it as the timer DURATION when `timerDuration` was absent: it did something, just not what it said. Together they declared a timeout `wait` does not have — the run resumes when its timer elapses or its signal arrives, never on a deadline. Rather than retrofit an implementation to fit two keys that happened to be declared, the pair is retired and real timeout semantics are left to be built to a requirement. `timeoutMs` converts to `timerDuration` (stringified — the target is `z.string()` and `parseIsoDuration` reads a bare numeric string as milliseconds, so the wait is unchanged); with `timerDuration` already set it is dropped, having been dead metadata. Like the other keys retired for MISDESCRIBING themselves rather than for being renamed, both leave the load path: absorbing them silently would let an author keep believing they configured a timeout. +Closing the same audit on the data side, `datasource.readReplicas` is removed (#4468). It described replica connections nothing ever opened: `ConnectableDatasource` and `DatasourceConnectionSpec` carry no replicas field, the driver factory never reads the key, and no query path distinguishes a read from a write — read/write splitting does not exist in the platform, so every statement always went to the primary. A lossless delete with no target to move to; front replicas behind one endpoint (pgpool, ProxySQL, an RDS reader endpoint) and point `config` at it. Notable as the case that shows how a key gets MORE convincing as it stays dead: #4410, closing the datasource-config gap, taught the schema to validate each replica entry against the declared driver's config contract, so sources written in between carry replica blocks that were genuinely checked — precise hosts, correct port types, typos rejected. Precision applied to an inert slot reads as evidence the slot is live, which is why ADR-0049 asks for a consumer rather than for rigor. Retired from the load path with the rest of the keys that misdescribed themselves. + ### Mechanical (applied for you) | Conversion | Surface | Change | Load window | @@ -183,6 +185,7 @@ The same kind of retirement covers `wait`'s timeout pair (#4158). `waitEventConf | `skill-trigger-phrases-removed` | `skill.triggerPhrases` | skill key 'triggerPhrases' removed (#3896 close-out — activation is triggerConditions + the agent's skills[] allowlist; phrases were a dead-end projection) | retired — `migrate meta` only | | `stack-api-require-auth-removed` | `stack.api.requireAuth` | stack key 'api.requireAuth' removed — anonymous access is always denied; publish public surfaces by declaration (#3963) | retired — `migrate meta` only | | `flow-node-wait-timeout-keys-removed` | `flow.node.waitEventConfig` | waitEventConfig keys 'timeoutMs' (→ 'timerDuration', stringified — its only reader used it as the duration) and 'onTimeout' (removed — zero readers, so no timeout ever fired) (#4158) | retired — `migrate meta` only | +| `datasource-read-replicas-removed` | `datasource.readReplicas` | datasource key 'readReplicas' removed (#4468 — no driver opened a replica connection and no query path splits reads from writes; front replicas behind one endpoint and point `config` at it) | retired — `migrate meta` only | ### Semantic (delegated to you, with acceptance criteria) diff --git a/packages/spec/authorable-surface.json b/packages/spec/authorable-surface.json index be9a921172..1309c98404 100644 --- a/packages/spec/authorable-surface.json +++ b/packages/spec/authorable-surface.json @@ -1,5 +1,5 @@ { - "description": "Ratchet of every AUTHORABLE key in the spec — what a metadata author may write, which for this platform IS the third-party API. Auto-updated on additions (commit the change). A key that disappears without a tombstone fails gen:schema, because these schemas are not .strict() and Zod would silently strip it. \"[RETIRED]\" marks a tombstoned key that still rejects with an upgrade prescription. See #3855, ADR-0059 §5.", + "description": "Ratchet of every AUTHORABLE key in the spec \u2014 what a metadata author may write, which for this platform IS the third-party API. Auto-updated on additions (commit the change). A key that disappears without a tombstone fails gen:schema, because these schemas are not .strict() and Zod would silently strip it. \"[RETIRED]\" marks a tombstoned key that still rejects with an upgrade prescription. See #3855, ADR-0059 \u00a75.", "keys": [ "ai/AIModelConfig:maxTokens", "ai/AIModelConfig:model", @@ -3313,7 +3313,6 @@ "data/Datasource:name", "data/Datasource:origin", "data/Datasource:pool", - "data/Datasource:readReplicas", "data/Datasource:retryPolicy", "data/Datasource:schemaMode", "data/Datasource:ssl", diff --git a/packages/spec/spec-changes.json b/packages/spec/spec-changes.json index 1116fc4573..af4642b007 100644 --- a/packages/spec/spec-changes.json +++ b/packages/spec/spec-changes.json @@ -205,6 +205,12 @@ "to": "waitEventConfig keys 'timeoutMs' (→ 'timerDuration', stringified — its only reader used it as the duration) and 'onTimeout' (removed — zero readers, so no timeout ever fired) (#4158)", "conversionId": "flow-node-wait-timeout-keys-removed", "toMajor": 17 + }, + { + "surface": "datasource.readReplicas", + "to": "datasource key 'readReplicas' removed (#4468 — no driver opened a replica connection and no query path splits reads from writes; front replicas behind one endpoint and point `config` at it)", + "conversionId": "datasource-read-replicas-removed", + "toMajor": 17 } ], "migrated": [ @@ -681,6 +687,12 @@ "to": "waitEventConfig keys 'timeoutMs' (→ 'timerDuration', stringified — its only reader used it as the duration) and 'onTimeout' (removed — zero readers, so no timeout ever fired) (#4158)", "conversionId": "flow-node-wait-timeout-keys-removed", "toMajor": 17 + }, + { + "surface": "datasource.readReplicas", + "to": "datasource key 'readReplicas' removed (#4468 — no driver opened a replica connection and no query path splits reads from writes; front replicas behind one endpoint and point `config` at it)", + "conversionId": "datasource-read-replicas-removed", + "toMajor": 17 } ], "migrated": [ diff --git a/packages/spec/src/conversions/registry.ts b/packages/spec/src/conversions/registry.ts index 70402f7e30..fb84fb359c 100644 --- a/packages/spec/src/conversions/registry.ts +++ b/packages/spec/src/conversions/registry.ts @@ -2228,6 +2228,63 @@ const flowNodeWaitTimeoutKeysRemoved: MetadataConversion = { }, }; +/** + * `datasource.readReplicas` — replica connections nothing ever opened (#4468). + * + * A lossless delete, and an unusually clear one: read/write splitting does not + * exist anywhere in the platform. `ConnectableDatasource` and + * `DatasourceConnectionSpec` carry no replicas field, the driver factory never + * reads the key, and no query path distinguishes a read from a write — so every + * statement always went to the primary regardless of what was declared here. + * + * Retired from the load path like every other key retired for *lying* rather + * than for being renamed. The distinction the registry draws (see + * `flow-node-wait-timeout-keys-removed`): a merely renamed key keeps a load + * window, because punishing an author for a spelling nobody warned them about + * is pointless. A key that misdescribed itself does not — silently absorbing it + * would let the author keep believing they had configured replica reads. + * + * Worth recording *why* this needed a conversion at all rather than passing + * unnoticed: #4410 had just taught the schema to validate each entry against the + * declared driver's config contract. Sources written between #4410 and here + * carry replica blocks that were *checked* — precise host names, correct port + * types, no typos — which is exactly the shape an author trusts most. The + * notice is what tells them the well-formed thing they wrote was never wired to + * anything. + */ +const datasourceReadReplicasRemoved: MetadataConversion = { + id: 'datasource-read-replicas-removed', + toMajor: 17, + retiredFromLoadPath: true, + surface: 'datasource.readReplicas', + summary: "datasource key 'readReplicas' removed (#4468 — no driver opened a replica connection and no query path splits reads from writes; front replicas behind one endpoint and point `config` at it)", + apply(stack, emit) { + return mapCollection(stack, 'datasources', (ds, path) => stripKeys(ds, ['readReplicas'], emit, path)); + }, + fixture: { + before: { + datasources: [{ + name: 'warehouse', + driver: 'postgres', + config: { host: 'primary.internal', port: 5432, database: 'analytics' }, + readReplicas: [ + { host: 'replica-a.internal', port: 5432, database: 'analytics' }, + { host: 'replica-b.internal', port: 5432, database: 'analytics' }, + ], + }], + }, + // One notice per datasource, not per replica: the key is what was removed. + after: { + datasources: [{ + name: 'warehouse', + driver: 'postgres', + config: { host: 'primary.internal', port: 5432, database: 'analytics' }, + }], + }, + expectedNotices: 1, + }, +}; + export const CONVERSIONS_BY_MAJOR: Readonly> = { 11: [flowNodeHttpRename, pageKindJsxToHtml, flowNodeFilterAlias, objectCompactLayoutRename], 13: [stackRolesToPositions, owdLegacyReadAliases, sharingRecipientRoleToPosition], @@ -2257,6 +2314,7 @@ export const CONVERSIONS_BY_MAJOR: Readonly z.object({ connectionTimeoutMillis: z.number().default(3000).describe('Connection establishment timeout'), }, { error: poolUnknownKeyError }).strict().optional().describe('Connection pool settings'), - /** - * Read Replicas - * Optional list of duplicate configurations for read-only operations. - * Useful for scaling read throughput. - */ - readReplicas: z.array(z.record(z.string(), z.unknown())).optional().describe('Read-only replica configurations'), + // `readReplicas` was removed here (#4468) — see RETIRED_READ_REPLICAS. It + // declared replica connections nothing opened; read/write splitting does not + // exist in the platform, so there was no consumer for it to reach. /** * Capability Overrides @@ -577,16 +603,14 @@ export const DatasourceSchema = lazySchema(() => z.object({ origin: z.enum(['code', 'runtime']).default('code') .describe('Datasource provenance (server-managed, read-only)'), }, { error: datasourceUnknownKeyError }).strict().superRefine((ds, ctx) => { - // The `config` gate (#4410). `config` and each `readReplicas` entry carry the - // same per-driver shape, so both are parsed against the contract for the + // The `config` gate (#4410). `config` is parsed against the contract for the // declared driver and every issue is re-pathed under the slot it came from — // the author sees `config.hostname`, not a detached message. + // + // #4410 ran this over each `readReplicas` entry too. #4468 removed that along + // with the key: validating entries for connections nothing opens spends the + // author's trust on a slot that cannot pay it back. reportDriverConfigIssues(ctx, ds.driver, ds.config, ['config']); - if (Array.isArray(ds.readReplicas)) { - ds.readReplicas.forEach((replica, index) => { - reportDriverConfigIssues(ctx, ds.driver, replica, ['readReplicas', index]); - }); - } if (ds.schemaMode !== 'managed' && !ds.external) { ctx.addIssue({ diff --git a/packages/spec/src/data/driver/config-registry.test.ts b/packages/spec/src/data/driver/config-registry.test.ts index a6c0700a20..25698350b3 100644 --- a/packages/spec/src/data/driver/config-registry.test.ts +++ b/packages/spec/src/data/driver/config-registry.test.ts @@ -112,23 +112,25 @@ describe('DatasourceSchema × driver config (#4410)', () => { }); /** - * `readReplicas` is the sibling escape hatch — same per-driver shape, same - * silence before #4410. Reported per index so the author is told WHICH - * replica is wrong. + * #4410 extended this gate over `readReplicas` too. #4468 retired the key — + * nothing ever opened a replica connection — so the parse must now REJECT the + * slot rather than check what goes in it. Pinned here, next to the config + * cases, because the two are easy to re-conflate: both are per-driver record + * shapes, and only one of them has a consumer. */ - it('validates each readReplicas entry and paths issues by index', () => { + it('rejects readReplicas outright, with the retirement prescription', () => { const result = DatasourceSchema.safeParse({ ...base, config: { host: 'db.internal', database: 'analytics' }, - readReplicas: [ - { host: 'replica-1.internal', database: 'analytics' }, - { hostname: 'replica-2.internal', database: 'analytics' }, - ], + readReplicas: [{ host: 'replica-1.internal', database: 'analytics' }], }); expect(result.success).toBe(false); - expect(result.error!.issues).toHaveLength(1); - expect(result.error!.issues[0]!.path).toEqual(['readReplicas', 1]); + // A well-formed replica block: the rejection is about the key existing at + // all, not about anything being wrong inside it. + expect(result.error!.issues[0]!.message).toMatch( + /`datasource\.readReplicas` was removed.*no query path separates reads from writes.*Delete the key/s, + ); }); it('rejects a sqlite datasource whose filename is misspelled', () => { diff --git a/packages/spec/src/data/driver/config-registry.zod.ts b/packages/spec/src/data/driver/config-registry.zod.ts index b0babf21ff..86f9ee3fbf 100644 --- a/packages/spec/src/data/driver/config-registry.zod.ts +++ b/packages/spec/src/data/driver/config-registry.zod.ts @@ -29,7 +29,14 @@ import { * * This registry closes that: it maps every driver id the platform can actually * BUILD onto the schema for that driver's config, and `DatasourceSchema` parses - * `config` (and each `readReplicas` entry) against it. + * `config` against it. + * + * #4410 ran the same parse over each `readReplicas` entry. #4468 retired that + * key: no driver ever opened a replica connection, so the entries were being + * checked against a contract nothing would ever apply them to. Worth keeping in + * view here — "we validate what we can construct" is a claim about drivers we + * build, and a slot the platform never connects is outside it in the other + * direction. * * ## Where the boundary is * diff --git a/packages/spec/src/migrations/registry.ts b/packages/spec/src/migrations/registry.ts index 20d57c4c5f..702e0ab806 100644 --- a/packages/spec/src/migrations/registry.ts +++ b/packages/spec/src/migrations/registry.ts @@ -528,7 +528,20 @@ const step17: MigrationStep = { + 'with `timerDuration` already set it is dropped, having been dead metadata. Like the other ' + 'keys retired for MISDESCRIBING themselves rather than for being renamed, both leave the ' + 'load path: absorbing them silently would let an author keep believing they configured a ' - + 'timeout.', + + 'timeout.\n\n' + + 'Closing the same audit on the data side, `datasource.readReplicas` is removed (#4468). ' + + 'It described replica connections nothing ever opened: `ConnectableDatasource` and ' + + '`DatasourceConnectionSpec` carry no replicas field, the driver factory never reads the ' + + 'key, and no query path distinguishes a read from a write — read/write splitting does not ' + + 'exist in the platform, so every statement always went to the primary. A lossless delete ' + + 'with no target to move to; front replicas behind one endpoint (pgpool, ProxySQL, an RDS ' + + 'reader endpoint) and point `config` at it. Notable as the case that shows how a key gets ' + + 'MORE convincing as it stays dead: #4410, closing the datasource-config gap, taught the ' + + 'schema to validate each replica entry against the declared driver\'s config contract, so ' + + 'sources written in between carry replica blocks that were genuinely checked — precise ' + + 'hosts, correct port types, typos rejected. Precision applied to an inert slot reads as ' + + 'evidence the slot is live, which is why ADR-0049 asks for a consumer rather than for ' + + 'rigor. Retired from the load path with the rest of the keys that misdescribed themselves.', conversionIds: [ 'action-execute-to-target', 'field-conditionalRequired-to-requiredWhen', @@ -553,6 +566,7 @@ const step17: MigrationStep = { 'skill-trigger-phrases-removed', 'stack-api-require-auth-removed', 'flow-node-wait-timeout-keys-removed', + 'datasource-read-replicas-removed', ], semantic: [ {