From 3a8aaa0f8cb5a921cb2d0ed4be0a47131b25c0d9 Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 1 Aug 2026 09:00:43 +0000 Subject: [PATCH 1/2] docs(agents): claim the issue before writing any code MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Several agents work this repo at once and an unassigned issue reads as an open invitation. Two agents starting the same issue burn the same hours twice and then race to land conflicting shapes for one problem. Makes assignment the first action of a task — before the worktree, before the first read — and states the corollary for Prime Directive #10: file a finding unassigned, assign it at the moment you actually start, so the issue list works as a queue other agents can trust. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01WsgTqRF58HsQYKLsrZ5pQY --- AGENTS.md | 20 ++++++++++++++++++-- CLAUDE.md | 15 +++++++++++++-- 2 files changed, 31 insertions(+), 4 deletions(-) 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) From ed1b90ebd785b27c0077f71cba9cb8291f6c8d7e Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 1 Aug 2026 09:23:30 +0000 Subject: [PATCH 2/2] feat(spec)!: retire datasource.readReplicas (#4468) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit BREAKING CHANGE: `datasource.readReplicas` is removed. 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 regardless of what was declared. There is no target to move to, because there is no read-replica routing to move to. Front replicas behind one endpoint (pgpool, ProxySQL, an RDS reader endpoint) and point `config` there; `os migrate meta --from 16` strips the key. Worth recording why this one survived so long: #4410 closed the `datasource.config` gap and, reasonably, extended the new per-driver validation over each `readReplicas` entry. The result was a slot with every marker of a working feature — declared, `.strict()`-guarded, and validated field-by-field against the driver's contract, rejecting a misspelt replica host by index. None of that is evidence of a consumer and all of it reads like one. Rigor is cheap to add to a dead slot and expensive to tell apart from life, which is why ADR-0049 asks for a consumer rather than for rigor. The retirement kit: - DatasourceSchema: key deleted (strict route), `readReplicas`/`replicas` added to the guidance map so the rejection carries the prescription; the #4410 validation loop removed with it - `datasource-read-replicas-removed` D2 conversion + step-17 chain wiring, retired from the load path like the other keys retired for misdescribing themselves - authorable-surface baseline line deleted deliberately (gate (a)'s strict- removal trip wire); spec-changes, upgrade guide, reference docs regenerated - pin test flipped from "validates each entry" to "rejects the slot" - release notes, strictness ledger, and the two pending changesets that still described the key corrected Follow-ups filed: #4479 (read-replica routing as a real feature request, starting from the read/write decision point) and #4480 (the same feature declared a second time as `DatabaseConnector.readReplicaConfig`, also unread). Closes #4468 Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01WsgTqRF58HsQYKLsrZ5pQY --- .../datasource-config-driver-contract.md | 2 +- .../datasource-read-replicas-removed.md | 49 +++++++++++++++ .../unknown-key-strictness-data-step.md | 8 ++- content/docs/references/data/datasource.mdx | 1 - content/docs/releases/v17.mdx | 22 ++++--- .../2026-07-unknown-key-strictness-ledger.md | 4 +- docs/protocol-upgrade-guide.md | 3 + packages/spec/authorable-surface.json | 3 +- packages/spec/spec-changes.json | 12 ++++ packages/spec/src/conversions/registry.ts | 58 +++++++++++++++++ packages/spec/src/data/datasource.zod.ts | 62 +++++++++++++------ .../src/data/driver/config-registry.test.ts | 22 ++++--- .../src/data/driver/config-registry.zod.ts | 9 ++- packages/spec/src/migrations/registry.ts | 16 ++++- 14 files changed, 225 insertions(+), 46 deletions(-) create mode 100644 .changeset/datasource-read-replicas-removed.md 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/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: [ {