diff --git a/.changeset/datasource-external-inert-keys-retired.md b/.changeset/datasource-external-inert-keys-retired.md new file mode 100644 index 0000000000..f397dcbcd5 --- /dev/null +++ b/.changeset/datasource-external-inert-keys-retired.md @@ -0,0 +1,27 @@ +--- +"@objectstack/spec": major +"@objectstack/example-showcase": patch +--- + +feat(spec)!: retire `external.label` and `external.requirePermission` (#4583 batch D) + +Two keys on the federation block, both read by nothing. + +**`external.label`** — nothing rendered the federation block's own label. Setup → +Datasources renders the datasource's **top-level** `label`, which every datasource already +has, so this was a second display name that never displayed. The showcase example declared +both; it now declares only the one that shows. + +**`external.requirePermission`** — no authorization check ever consulted it. A permission +named here gated nothing: access to a federated datasource's data is governed by the +ordinary object permission sets and RLS, exactly as for a managed datasource. Naming a +permission that is never required is the false-compliance shape ADR-0049 exists to remove +— it reads like an access control and is one only in the author's head. + +FROM → TO: delete `external.label` (use the top-level `label`); delete +`external.requirePermission` and grant or withhold the object permissions instead. +`os migrate meta --from 16` removes both automatically (conversion +`datasource-inert-blocks-removed`). + +With these, the `datasource` liveness ledger reaches **zero dead properties** — down from +the 20 it was seeded with in #4487, the highest dead ratio of any governed type. diff --git a/.changeset/datasource-health-check-retired.md b/.changeset/datasource-health-check-retired.md new file mode 100644 index 0000000000..bce48fe008 --- /dev/null +++ b/.changeset/datasource-health-check-retired.md @@ -0,0 +1,20 @@ +--- +"@objectstack/spec": major +--- + +feat(spec)!: retire `datasource.healthCheck` — no probe loop ever existed (#4583 batch C) + +Three keys — `enabled`, `intervalMs`, `timeoutMs` — declared, strict-guarded, read by +nothing. No health-check loop was ever scheduled, so `enabled: true` enabled nothing and +the two timeouts bounded nothing. + +Connection liveness is probed **on demand** through the driver handle's `ping()` / +`checkHealth()`, which the datasource admin service calls for "Test connection". That is +the mechanism — it needs no configuration here and never read this block. + +Note what it is NOT to be confused with: `external.validation.checkIntervalMs` is the one +recurring datasource timer, and it checks **schema drift** on a federated datasource, not +connection liveness. It is unaffected. + +FROM → TO: delete the block. `os migrate meta --from 16` removes it automatically +(conversion `datasource-inert-blocks-removed`). diff --git a/.changeset/datasource-retry-policy-retired.md b/.changeset/datasource-retry-policy-retired.md new file mode 100644 index 0000000000..1c99f2f845 --- /dev/null +++ b/.changeset/datasource-retry-policy-retired.md @@ -0,0 +1,21 @@ +--- +"@objectstack/spec": major +--- + +feat(spec)!: retire `datasource.retryPolicy` — nothing ever retried on it (#4583 batch B) + +Four keys — `maxRetries`, `baseDelayMs`, `maxDelayMs`, `backoffMultiplier` — declared, +strict-guarded, and read by no connect or query path. Connection failure is handled by +the boot policy in the datasource connection service (degraded boot, or `bootCritical` +fail-fast); nothing retries on a schedule, so setting `maxRetries: 5` changed nothing. + +**Do not "fix" this by renaming keys.** `hook.retryPolicy` and `job.retryPolicy` ARE +enforced — but they are a different key on a different type, and they spell the delay +`backoffMs`, not `baseDelayMs`. That very inconsistency is the evidence nothing read the +datasource one: no code in the repo reads both spellings. Moving these values onto a hook +or a job only makes sense if you actually want that hook or job retried. + +FROM → TO: delete the block. `os migrate meta --from 16` removes it automatically +(conversion `datasource-inert-blocks-removed`). `DatasourceSchema` is `.strict()`, so a +leftover `retryPolicy` is a loud rejection carrying this prescription — never a silent +strip. diff --git a/content/docs/references/data/datasource.mdx b/content/docs/references/data/datasource.mdx index 93dbc1eb24..58f2db5d5a 100644 --- a/content/docs/references/data/datasource.mdx +++ b/content/docs/references/data/datasource.mdx @@ -36,14 +36,12 @@ const result = DatasourceSchema.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 | -| **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 | -| **retryPolicy** | `{ maxRetries: number; baseDelayMs: number; maxDelayMs: number; backoffMultiplier: number }` | optional | Connection retry policy for transient failures | | **description** | `string` | optional | Internal description | | **active** | `boolean` | ✅ | Is datasource enabled | | **autoConnect** | `boolean` | ✅ | Force a live driver connection at boot even when managed + unrouted (ADR-0062 D2). | | **schemaMode** | `Enum<'managed' \| 'external' \| 'validate-only'>` | ✅ | Schema ownership mode | -| **external** | `{ label?: string; allowedSchemas?: string[]; allowWrites: boolean; validation: object; … }` | optional | External datasource federation settings (schemaMode != "managed") | +| **external** | `{ allowedSchemas?: string[]; allowWrites: boolean; validation: object; credentialsRef?: string; … }` | optional | External datasource federation settings (schemaMode != "managed") | | **origin** | `Enum<'code' \| 'runtime'>` | ✅ | Datasource provenance (server-managed, read-only) | | **_lock** | `Enum<'none' \| 'no-overlay' \| 'no-delete' \| 'full'>` | optional | Item-level lock — controls overlay & delete (ADR-0010). | | **_lockReason** | `string` | optional | Human-readable reason shown when a write is refused by _lock. | @@ -82,13 +80,11 @@ External datasource federation settings (schemaMode != "managed") | Property | Type | Required | Description | | :--- | :--- | :--- | :--- | -| **label** | `string` | optional | Display label, e.g. "Snowflake — ANALYTICS / PROD" | | **allowedSchemas** | `string[]` | optional | Whitelist of remote schemas/databases that may be exposed. | | **allowWrites** | `boolean` | ✅ | Global write gate. Individual objects must also opt in via object.external.writable. | | **validation** | `{ onMismatch: Enum<'fail' \| 'warn' \| 'ignore'>; checkOnBoot: boolean; checkIntervalMs?: number }` | ✅ | Boot/drift validation policy | | **credentialsRef** | `string` | optional | Reference into the secrets store; never inline credentials. | | **queryTimeoutMs** | `number` | ✅ | Hard cap on per-query execution time. | -| **requirePermission** | `string` | optional | Optional convenience: gate the entire datasource behind a single role. | --- diff --git a/docs/audits/2026-07-unknown-key-strictness-ledger.md b/docs/audits/2026-07-unknown-key-strictness-ledger.md index 8e8abf2a22..2277c1647e 100644 --- a/docs/audits/2026-07-unknown-key-strictness-ledger.md +++ b/docs/audits/2026-07-unknown-key-strictness-ledger.md @@ -454,7 +454,7 @@ tightening (the #4001 "sharing-rule lesson": candidates, not verdicts). | `notification.zod.ts` / `offline.zod.ts` / `report.zod.ts` | 3 ea | authorable (p) | | | `sharing.zod.ts` | 2 | authorable (p) | public-sharing config | -### `data/` — 164 sites +### `data/` — 162 sites | File | Sites | Class | Note | |---|---|---|---| @@ -465,7 +465,7 @@ 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` | 8 | authorable | **strict as of #4001 data step** — all 8: `DatasourceSchema` (+ `pool` / `healthCheck` / `ssl` / `retryPolicy`), `ExternalDatasourceSettingsSchema` (+ `validation`), `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 | **#4583 dropped the ninth site**: `DatasourceCapabilities` is gone — eleven flags no code read, on a block whose strictness was the clearest case of this row's own closing sentence. `readOnly` in particular was *precisely validated* and completely inert, and had been relocated twice (#4410, #4465) toward somewhere it might be enforced; the shipped CRM example called a datasource a read replica on the strength of it while writes went through. Class unchanged +| `datasource.zod.ts` | 6 | authorable | **strict as of #4001 data step** — all 6: `DatasourceSchema` (+ `pool` / `ssl`), `ExternalDatasourceSettingsSchema` (+ `validation`), `DriverDefinitionSchema`. **#4583 B/C dropped two more sites**: the `healthCheck` and `retryPolicy` blocks are gone — nothing scheduled a probe and nothing retried, so their strictness was validating a shape no code consumed. `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 | **#4583 dropped the ninth site**: `DatasourceCapabilities` is gone — eleven flags no code read, on a block whose strictness was the clearest case of this row's own closing sentence. `readOnly` in particular was *precisely validated* and completely inert, and had been relocated twice (#4410, #4465) toward somewhere it might be enforced; the shipped CRM example called a datasource a read replica on the strength of it while writes went through. Class unchanged | `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) | | diff --git a/docs/protocol-upgrade-guide.md b/docs/protocol-upgrade-guide.md index 284d3b3747..d53bbf56f2 100644 --- a/docs/protocol-upgrade-guide.md +++ b/docs/protocol-upgrade-guide.md @@ -189,6 +189,7 @@ The `script` flow node converges on its one real path (#4343). It had four ways | `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 | | `datasource-capabilities-removed` | `datasource.capabilities` | datasource key 'capabilities' removed (#4583 — eleven flags no code read; pushdown comes from the driver's own supports.*, and `readOnly` never made anything read-only) | retired — `migrate meta` only | +| `datasource-inert-blocks-removed` | `datasource.retryPolicy / datasource.healthCheck / datasource.external.label / datasource.external.requirePermission` | datasource keys 'retryPolicy'/'healthCheck' and external 'label'/'requirePermission' removed (#4583 — nothing retried, nothing probed on a schedule, and the federation label/permission were read by nobody) | retired — `migrate meta` only | | `flow-node-script-branch-keys-removed` | `flow.node.script.config.actionType / flow.node.script.config.template / flow.node.script.config.recipients / flow.node.script.config.variables / flow.node.script.config.script` | script flow-node config keys 'actionType' (→ 'function' when it was shorthand for one; otherwise removed — 'email'/'slack' were logger-backed stubs that delivered nothing), plus 'template' / 'recipients' / 'variables' (fed those stubs) and 'script' (inline JS the runtime never executed) (#4343) | retired — `migrate meta` only | ### Semantic (delegated to you, with acceptance criteria) diff --git a/examples/app-showcase/src/system/datasources/showcase-external.datasource.ts b/examples/app-showcase/src/system/datasources/showcase-external.datasource.ts index 71a8625fbb..0aa3cf73c2 100644 --- a/examples/app-showcase/src/system/datasources/showcase-external.datasource.ts +++ b/examples/app-showcase/src/system/datasources/showcase-external.datasource.ts @@ -34,7 +34,8 @@ export const ShowcaseExternalDatasource = defineDatasource({ // same place the fixture writes it. Sits next to the managed standalone.db. config: { filename: '.objectstack/data/showcase_external.db' }, external: { - label: 'External Analytics DB — read-only federation demo (ADR-0015)', + // `external.label` was removed in #4583 — nothing read the federation + // block's own label; the top-level `label` above is what Setup renders. allowWrites: false, validation: { onMismatch: 'warn', checkOnBoot: true }, }, diff --git a/packages/cli/src/utils/lint-liveness-properties.test.ts b/packages/cli/src/utils/lint-liveness-properties.test.ts index 6eb6af7211..4ba6aaa946 100644 --- a/packages/cli/src/utils/lint-liveness-properties.test.ts +++ b/packages/cli/src/utils/lint-liveness-properties.test.ts @@ -195,28 +195,27 @@ describe('lintLivenessProperties', () => { // the type that most needed it: 20 of its 43 props have no runtime consumer, // and until #4487 nothing told an author so. - it('warns on the dead datasource blocks that remain — healthCheck / retryPolicy (#4487)', () => { - // `capabilities` left this list in #4583: the block was REMOVED from the - // schema, so an author who writes it now gets a hard parse rejection with a - // prescription — a stronger signal than a lint warning, and the reason its - // ledger rows are gone rather than flipped. healthCheck / retryPolicy are - // still authorable and still dead (batches B and C of #4583). + it('no longer warns on ANY datasource block — the whole dead surface is gone (#4583)', () => { + // This assertion has now inverted twice, and the direction of travel is the + // point. It began (#4487) asserting warnings on capabilities/healthCheck/ + // retryPolicy; batch A removed `capabilities`, so it narrowed to the other + // two; batches B/C/D removed those as well. Every one of the twenty dead + // datasource properties is now a hard parse rejection carrying its own + // prescription — strictly stronger than an advisory lint warning, which is + // why their ledger rows are deleted rather than flipped. + // + // Kept (rather than deleted) as a REGRESSION GUARD: it runs against the + // real shipped ledger, so re-introducing a dead+authorWarn datasource + // property fails here rather than shipping quietly. const findings = lintLivenessProperties({ datasources: [{ name: 'warehouse', + label: 'Warehouse', driver: 'postgres', config: { host: 'db.internal', database: 'analytics' }, - healthCheck: { enabled: true, intervalMs: 30000 }, - retryPolicy: { maxRetries: 5, baseDelayMs: 1000 }, }], }); - const msgs = paths(findings); - expect(msgs.some((m) => m.includes('healthCheck.enabled'))).toBe(true); - expect(msgs.some((m) => m.includes('healthCheck.intervalMs'))).toBe(true); - expect(msgs.some((m) => m.includes('retryPolicy.maxRetries'))).toBe(true); - expect(msgs.some((m) => m.includes('retryPolicy.baseDelayMs'))).toBe(true); - // The removed block must no longer be reported by the lint at all. - expect(msgs.some((m) => m.includes('capabilities'))).toBe(false); + expect(findings).toEqual([]); }); // The entry the whole audit was worth doing for. `capabilities.readOnly` read diff --git a/packages/spec/authorable-surface.json b/packages/spec/authorable-surface.json index 4dda8f322a..98e36a4560 100644 --- a/packages/spec/authorable-surface.json +++ b/packages/spec/authorable-surface.json @@ -3238,12 +3238,10 @@ "data/Datasource:description", "data/Datasource:driver", "data/Datasource:external", - "data/Datasource:healthCheck", "data/Datasource:label", "data/Datasource:name", "data/Datasource:origin", "data/Datasource:pool", - "data/Datasource:retryPolicy", "data/Datasource:schemaMode", "data/Datasource:ssl", "data/Dimension:description", @@ -3392,9 +3390,7 @@ "data/ExternalDatasourceSettings:allowWrites", "data/ExternalDatasourceSettings:allowedSchemas", "data/ExternalDatasourceSettings:credentialsRef", - "data/ExternalDatasourceSettings:label", "data/ExternalDatasourceSettings:queryTimeoutMs", - "data/ExternalDatasourceSettings:requirePermission", "data/ExternalDatasourceSettings:validation", "data/ExternalFieldMapping:defaultValue", "data/ExternalFieldMapping:readonly", diff --git a/packages/spec/liveness/README.md b/packages/spec/liveness/README.md index edc084c42a..dddfa26a80 100644 --- a/packages/spec/liveness/README.md +++ b/packages/spec/liveness/README.md @@ -506,7 +506,7 @@ for t, v in r['types'].items(): | report | 21 | 0 | 0 | – | dataset-bound (ADR-0021); the aria/performance LEDGER entries were stale — the keys left the schema in the report-liveness close-out; deleted 2026-07-30 as hygiene. Audit-era `chart` DEAD superseded (framework#1890 / #3441) | | dashboard | 18 | 0 | 2 | – | ADR-0021 dataset widgets (#3251; DashboardWidgetSchema `.strict()`); `aria`/`performance` (and widget `performance` + PerformanceConfigSchema) REMOVED 2026-07-30 (#3896 close-out sweep — no renderer applied any of them); audit-era `globalFilters`/`dateRange` DEAD superseded (framework#2501) | | query | 16 | 1 | 4 | – | **not a metadata type** — the REQUEST surface (`QuerySchema`: client SDK QueryBuilder output; the `POST /data/:object/query` body), governed via `SPEC_ONLY_SCHEMAS` (#4286). The gate's one-level walk resolves 1 experimental; the 7 marker-experimental search affordances sit one level deeper, below the walk — resolved from `[EXPERIMENTAL — not enforced]` describe markers, not ledger entries (search `fuzzy`/`operator`/`boost`/`minScore`/`language`/`highlight` + `aggregations[].filter` — declared engine affordances no executor receives). The #4286 sweep closed out same-release: `having` ENFORCED 2026-07-31 (engine-side post-aggregation filter, both paths; was finding 1); dead 4 = the tombstoned removals `joins`/`windowFunctions`/`cursor`/`distinct` — REMOVED 2026-07-31 (retiredKey keeps each in the walked shape so the rows stay; protocol-17 semantic migrations; the JoinNode + WindowFunctionNode clusters and the `QueryBuilder.cursor()`/`.distinct()` producers deleted with their keys; `distinct`'s mis-wired REST count suppression deleted too — finding 2) | -| datasource | 30 | 0 | 9 | 0 | seeded 2026-08-01 (#4487) — the **highest dead ratio of any governed type** (20 of 43), and it was ungoverned until now, which is not a coincidence: #4410/#4465/#4481 found six inert keys here by hand, two security-shaped (`schemaMode` left an external DB constructible as `managed` with DDL ungated; `ssl` configured nothing while looking configured). Dead set = `capabilities.*` (all 11 — the engine gates pushdown on the runtime driver's `supports.*` object, a non-overlapping vocabulary), `healthCheck.*` (3 — nothing schedules a datasource probe; the 20 `healthCheck` hits in the repo all belong to the PLUGIN health monitor and other surfaces), `retryPolicy.*` (4 — `retryPolicy` IS enforced on `hook` and `job`, which is what makes this one read alive; the shapes differ), `external.label`, `external.requirePermission`. **`capabilities.readOnly` is the one to know**: it reads as a safety switch, gates nothing, and two shipped prescriptions pointed authors at it until #4487 — `external.allowWrites: false` is the enforced write gate. `config` is a `z.record`, so its per-driver keys sit outside the walk (recorded in the entry's note, not silently skipped) **批 A CLOSED 2026-08-02 (#4583)**: the `capabilities` block — 11 flags, every one dead and authorWarn'd — was REMOVED rather than bridged; pushdown comes from the runtime driver's own `supports.*`, so there was nothing to connect it to. Its rows are deleted (strict-removal route), which is why dead falls 20 → 9. `readOnly` was the reason the audit was worth doing: it read as a safety switch, gated nothing, and had already been MOVED twice toward somewhere it might be enforced (#4410, #4465) — the shipped CRM example called a datasource a read replica on the strength of it while the datasource took writes. Removing it does NOT hand the author a working alternative: `external.allowWrites` only gates FEDERATED datasources, so a managed one has no read-only gate at all (#4584). Remaining 9 = healthCheck ×3 + retryPolicy ×4 + external ×2, batches B/C/D of #4583 | +| datasource | 30 | 0 | 0 | 0 | seeded 2026-08-01 (#4487) — the **highest dead ratio of any governed type** (20 of 43), and it was ungoverned until now, which is not a coincidence: #4410/#4465/#4481 found six inert keys here by hand, two security-shaped (`schemaMode` left an external DB constructible as `managed` with DDL ungated; `ssl` configured nothing while looking configured). Dead set = `capabilities.*` (all 11 — the engine gates pushdown on the runtime driver's `supports.*` object, a non-overlapping vocabulary), `healthCheck.*` (3 — nothing schedules a datasource probe; the 20 `healthCheck` hits in the repo all belong to the PLUGIN health monitor and other surfaces), `retryPolicy.*` (4 — `retryPolicy` IS enforced on `hook` and `job`, which is what makes this one read alive; the shapes differ), `external.label`, `external.requirePermission`. **`capabilities.readOnly` is the one to know**: it reads as a safety switch, gates nothing, and two shipped prescriptions pointed authors at it until #4487 — `external.allowWrites: false` is the enforced write gate. `config` is a `z.record`, so its per-driver keys sit outside the walk (recorded in the entry's note, not silently skipped) **批 A CLOSED 2026-08-02 (#4583)**: the `capabilities` block — 11 flags, every one dead and authorWarn'd — was REMOVED rather than bridged; pushdown comes from the runtime driver's own `supports.*`, so there was nothing to connect it to. Its rows are deleted (strict-removal route), which is why dead falls 20 → 9. `readOnly` was the reason the audit was worth doing: it read as a safety switch, gated nothing, and had already been MOVED twice toward somewhere it might be enforced (#4410, #4465) — the shipped CRM example called a datasource a read replica on the strength of it while the datasource took writes. Removing it does NOT hand the author a working alternative: `external.allowWrites` only gates FEDERATED datasources, so a managed one has no read-only gate at all (#4584). Remaining 9 = healthCheck ×3 + retryPolicy ×4 + external ×2, batches B/C/D of #4583 **BATCHES B/C/D CLOSED 2026-08-02 — datasource now has ZERO dead properties**, down from the 20 it was seeded with (the highest dead ratio of any governed type). `retryPolicy` ×4 and `healthCheck` ×3 went as whole blocks, `external.label` / `external.requirePermission` as keys. None was bridgeable: each already had a different LIVE mechanism doing the job — the boot policy, the driver handle's on-demand `ping()`/`checkHealth()`, the top-level `label`, and ordinary permission sets + RLS. The `retryPolicy` rejection deliberately refuses to offer a rename: `hook`/`job` retryPolicy ARE enforced but spell the delay `backoffMs`, and that inconsistency is itself the evidence nothing read the datasource one (#4488's sharpest trap) | | webhook | 11 | 0 | 0 | – | **not a registered metadata type** — governed via the gate's spec-only schema override (`SPEC_ONLY_SCHEMAS`), not `getMetadataTypeSchema`; folding it onto the registry is the #3490 reassessment. This row once read 0/1/16 ("the ENTIRE authoring surface is dead", #3461) and both halves of that were CLOSED same-quarter: #3489 built the materializer bridge (authored `webhooks:` entries now land as `sys_webhook` dispatcher rows) and #3494 pruned the aspirational props outright — so the surviving surface is fully live. Kept in the table as the worked example that a dead verdict is a worklist entry, not a tombstone: enforce-or-remove resolved this one by ENFORCING | | app | 45 | – | 14 | – | seeded 2026-08-01 (#4488). Dead 14 = the seven #4142 `retiredKey` tombstones (version/aria/objects/apis/sharing/embed/mobileNavigation — rows stay while the tombstones hold the keys in the walked shape) + `homePageId` (the landing IS the first nav item; root landing follows `isDefault` routing) + the **fail-open area gates** `areas.visible` / `areas.requiredPermissions` (nothing evaluates them, while the per-ITEM siblings are enforced server- and client-side — the audit's most important app finding, both authorWarn'd) + `areas.order`/`description` + selector `includeAll` (deliberately ignored: selectors are mandatory-scope; an "All" would leak system metadata) and `placement`. Nav walk covers the union's `object` variant; other variants hand-verified live except the `actionDef` dispatch gap (renders, but no shipped shell passes `onAction`) — #4509 | | book | 13 | – | 2 | – | seeded 2026-08-01 (#4488). ADR-0046 §6 spine; `audience` is ENFORCED and fail-closed (tree 401/403 + per-doc effective-audience union on both list and tree). Dead 2 = BOTH inline `translations` maps (book-level and per-group): no resolver reads them and the bundle translator doesn't cover `book` — the trap is that `doc.translations` two files over works on every read path. Also recorded: the `include: { tag }` rule variant can never match (DocSchema declares no `tags`) | diff --git a/packages/spec/liveness/datasource.json b/packages/spec/liveness/datasource.json index 70f23db9a9..bd527b5ed2 100644 --- a/packages/spec/liveness/datasource.json +++ b/packages/spec/liveness/datasource.json @@ -1,6 +1,6 @@ { "type": "datasource", - "_note": "DatasourceSchema. Consumers: @objectstack/service-datasource (DatasourceConnectionService.toSpec → DatasourceConnectionSpec → createDefaultDatasourceDriverFactory), @objectstack/objectql (engine.ts federation write gate), @objectstack/runtime (external-validation-plugin). Seeded 2026-08-01 (#4487) after #4465/#4481 found six inert keys BY HAND on a type no gate governed. Method: the authoritative boundary is what crosses into `ConnectableDatasource` (datasource-connection-service.ts:45-74) and `DatasourceConnectionSpec` (contracts/datasource-driver-factory.ts:25-59) — a block on neither reaches no driver. objectui's DatasourcePreview renders `pool`/`ssl`/`retryPolicy`/`healthCheck` as SideBlocks and is NOT counted as evidence for any entry (see README, 'An authoring/preview renderer is NOT a runtime consumer' — the #4481 precedent was exactly this). Framework provenance/lock fields auto-live. RETIREMENT 2026-08-02 (#4583): the whole `capabilities` block (11 flags, every one dead and authorWarn'd) was REMOVED from the schema rather than bridged — pushdown is decided by the runtime driver's own `supports.*`, a different mechanism, so there was nothing to connect it to. Its rows are deleted rather than flipped, per the strict-removal route (the keys left the walked shape, so a kept row would read as an ORPHAN). `readOnly` is called out separately in the tombstone: deleting it does NOT hand the author a working alternative, because `external.allowWrites` only gates FEDERATED datasources — a managed datasource has no read-only gate at all, which is #4584 rather than something this removal invented.", + "_note": "DatasourceSchema. Consumers: @objectstack/service-datasource (DatasourceConnectionService.toSpec → DatasourceConnectionSpec → createDefaultDatasourceDriverFactory), @objectstack/objectql (engine.ts federation write gate), @objectstack/runtime (external-validation-plugin). Seeded 2026-08-01 (#4487) after #4465/#4481 found six inert keys BY HAND on a type no gate governed. Method: the authoritative boundary is what crosses into `ConnectableDatasource` (datasource-connection-service.ts:45-74) and `DatasourceConnectionSpec` (contracts/datasource-driver-factory.ts:25-59) — a block on neither reaches no driver. objectui's DatasourcePreview renders `pool`/`ssl`/`retryPolicy`/`healthCheck` as SideBlocks and is NOT counted as evidence for any entry (see README, 'An authoring/preview renderer is NOT a runtime consumer' — the #4481 precedent was exactly this). Framework provenance/lock fields auto-live. RETIREMENT 2026-08-02 (#4583): the whole `capabilities` block (11 flags, every one dead and authorWarn'd) was REMOVED from the schema rather than bridged — pushdown is decided by the runtime driver's own `supports.*`, a different mechanism, so there was nothing to connect it to. Its rows are deleted rather than flipped, per the strict-removal route (the keys left the walked shape, so a kept row would read as an ORPHAN). `readOnly` is called out separately in the tombstone: deleting it does NOT hand the author a working alternative, because `external.allowWrites` only gates FEDERATED datasources — a managed datasource has no read-only gate at all, which is #4584 rather than something this removal invented. BATCHES B/C/D CLOSED 2026-08-02 (#4583): `retryPolicy` (4) and `healthCheck` (3) were removed as whole blocks and `external.label` / `external.requirePermission` as individual keys — all nine dead, all authorWarn'd, none bridgeable because each already had a DIFFERENT live mechanism: connection failure is the boot policy (degraded boot / bootCritical), liveness is the driver handle's on-demand ping()/checkHealth(), the federation label is the top-level `label`, and federated access is governed by ordinary permission sets + RLS. Rows deleted rather than flipped (strict-removal route). The retryPolicy rejection deliberately does NOT offer a rename: hook/job retryPolicy ARE enforced but spell the delay `backoffMs`, and that very inconsistency is the evidence nothing read the datasource one. datasource now carries ZERO dead properties.", "props": { "name": { "status": "live", @@ -45,26 +45,6 @@ } } }, - "healthCheck": { - "children": { - "enabled": { - "status": "dead", - "authorWarn": true, - "authorHint": "Delete it. No health-check loop reads this block. Connection liveness is probed on demand via the driver handle's `ping()` / `checkHealth()` (contracts/datasource-driver-factory.ts:88-92), which the admin service calls for `testConnection` — not on any interval this could enable." - }, - "intervalMs": { - "status": "dead", - "authorWarn": true, - "authorHint": "Delete it. Nothing schedules a datasource health check, so there is no interval to set. The only recurring datasource timer is `external.validation.checkIntervalMs` (schema-drift checking, a different concern)." - }, - "timeoutMs": { - "status": "dead", - "authorWarn": true, - "authorHint": "Delete it. See `healthCheck.intervalMs` — there is no probe loop for this to bound." - } - }, - "note": "All 3 dead, verified 2026-08-01. `healthCheck` is absent from ConnectableDatasource and DatasourceConnectionSpec. Every `healthCheck` hit in the monorepo belongs to a DIFFERENT surface — the PLUGIN health monitor (core/src/health-monitor.ts, core/src/plugin-loader.ts:316), the AI model registry, the integration connector, `StartupOrchestratorOptions.healthCheck`. Name collision, not a consumer. Easy to mis-verify: a bare grep for 'healthCheck' returns 20 hits and none of them is this block." - }, "ssl": { "children": { "enabled": { @@ -91,31 +71,6 @@ }, "note": "Live only since #4465. Before that the whole block stopped at the record — nothing put it on the connection spec, so a TLS configuration with a CA certificate in it configured nothing while looking identical to one that worked. A security-shaped property that was silently inert; exactly the ADR-0078 class this ledger exists to catch, and it was found by hand rather than by a gate." }, - "retryPolicy": { - "children": { - "maxRetries": { - "status": "dead", - "authorWarn": true, - "authorHint": "Delete it. No connect or query path retries on this block. Connection failure handling is the boot policy in datasource-connection-service.ts (degraded boot / `bootCritical` fail-fast), which does not retry on a schedule. Do not confuse this with `hook.retryPolicy` (enforced, objectql/src/hook-wrappers.ts:105) or `job.retryPolicy` (enforced, runtime/src/app-plugin.ts:791) — same key name, different types, different shapes." - }, - "baseDelayMs": { - "status": "dead", - "authorWarn": true, - "authorHint": "Delete it — see `retryPolicy.maxRetries`. Note this key does not even exist on the two retryPolicy blocks that ARE enforced: `hook.retryPolicy` spells its delay `backoffMs`." - }, - "maxDelayMs": { - "status": "dead", - "authorWarn": true, - "authorHint": "Delete it — see `retryPolicy.maxRetries`." - }, - "backoffMultiplier": { - "status": "dead", - "authorWarn": true, - "authorHint": "Delete it — see `retryPolicy.maxRetries`." - } - }, - "note": "All 4 dead, verified 2026-08-01. Absent from ConnectableDatasource and DatasourceConnectionSpec. The trap here is the name: `retryPolicy` IS enforced on `hook` and on `job`, so a grep for the key looks alive and a reader who stops there concludes the datasource one works too. The shapes differ — hook uses `{maxRetries, backoffMs}`, this declares `{maxRetries, baseDelayMs, maxDelayMs, backoffMultiplier}` — which is itself the tell that nothing reads both." - }, "description": { "status": "live", "note": "internal documentation. No runtime consumer by design — ADR-0033 docs-shaped, deliberately kept, not authorWarn'd." @@ -137,11 +92,6 @@ }, "external": { "children": { - "label": { - "status": "dead", - "authorWarn": true, - "authorHint": "Delete it. Nothing reads the federation block's own label — use the datasource's top-level `label`, which the Setup list renders." - }, "allowedSchemas": { "status": "live", "evidence": "packages/services/service-datasource/src/external-datasource-service.ts:145", @@ -166,11 +116,6 @@ "status": "live", "evidence": "packages/services/service-datasource/src/datasource-admin-service.ts:220", "note": "carried into the external-datasource probe options as `timeoutMs`." - }, - "requirePermission": { - "status": "dead", - "authorWarn": true, - "authorHint": "Delete it. No authorization check consults it — a permission named here gates nothing, and access to a federated datasource's data is governed by the ordinary object permission sets and RLS. Naming a permission that is never required is the same false-compliance shape as the retired `tool.permissions` (#3896)." } }, "note": "5 of 7 live. The two dead ones are opposite in risk: `external.label` is cosmetic, `external.requirePermission` is security-shaped — it reads as an access gate and is not one." diff --git a/packages/spec/spec-changes.json b/packages/spec/spec-changes.json index 32530b25fd..5a0c00f6ab 100644 --- a/packages/spec/spec-changes.json +++ b/packages/spec/spec-changes.json @@ -218,6 +218,12 @@ "conversionId": "datasource-capabilities-removed", "toMajor": 17 }, + { + "surface": "datasource.retryPolicy / datasource.healthCheck / datasource.external.label / datasource.external.requirePermission", + "to": "datasource keys 'retryPolicy'/'healthCheck' and external 'label'/'requirePermission' removed (#4583 — nothing retried, nothing probed on a schedule, and the federation label/permission were read by nobody)", + "conversionId": "datasource-inert-blocks-removed", + "toMajor": 17 + }, { "surface": "flow.node.script.config.actionType / flow.node.script.config.template / flow.node.script.config.recipients / flow.node.script.config.variables / flow.node.script.config.script", "to": "script flow-node config keys 'actionType' (→ 'function' when it was shorthand for one; otherwise removed — 'email'/'slack' were logger-backed stubs that delivered nothing), plus 'template' / 'recipients' / 'variables' (fed those stubs) and 'script' (inline JS the runtime never executed) (#4343)", @@ -719,6 +725,12 @@ "conversionId": "datasource-capabilities-removed", "toMajor": 17 }, + { + "surface": "datasource.retryPolicy / datasource.healthCheck / datasource.external.label / datasource.external.requirePermission", + "to": "datasource keys 'retryPolicy'/'healthCheck' and external 'label'/'requirePermission' removed (#4583 — nothing retried, nothing probed on a schedule, and the federation label/permission were read by nobody)", + "conversionId": "datasource-inert-blocks-removed", + "toMajor": 17 + }, { "surface": "flow.node.script.config.actionType / flow.node.script.config.template / flow.node.script.config.recipients / flow.node.script.config.variables / flow.node.script.config.script", "to": "script flow-node config keys 'actionType' (→ 'function' when it was shorthand for one; otherwise removed — 'email'/'slack' were logger-backed stubs that delivered nothing), plus 'template' / 'recipients' / 'variables' (fed those stubs) and 'script' (inline JS the runtime never executed) (#4343)", diff --git a/packages/spec/src/conversions/registry.ts b/packages/spec/src/conversions/registry.ts index 22013cf159..18df3ff4db 100644 --- a/packages/spec/src/conversions/registry.ts +++ b/packages/spec/src/conversions/registry.ts @@ -2230,6 +2230,89 @@ const flowNodeWaitTimeoutKeysRemoved: MetadataConversion = { }, }; +/** + * The remaining inert datasource blocks removed (protocol 17, #4583 B/C/D). + * + * Three clusters, one finding each — declared, `.strict()`-guarded, read by no + * runtime path: + * + * - `retryPolicy` (4 keys): no connect or query path ever retried on it. + * Connection failure is the boot policy in the datasource connection service + * (degraded boot / `bootCritical` fail-fast), which does not retry on a + * schedule. + * - `healthCheck` (3 keys): nothing scheduled a probe, so `enabled` enabled + * nothing and the timeouts bounded nothing. Liveness is probed ON DEMAND via + * the driver handle's `ping()` / `checkHealth()`. + * - `external.label` / `external.requirePermission`: the federation block's own + * label was never read (the top-level `label` is what Setup renders), and no + * authorization check ever consulted the permission — naming one gated + * nothing, which is the false-compliance shape ADR-0049 removes. + * + * `retryPolicy` is the one with a booby trap, and it is a NAME collision rather + * than a behaviour question: `hook.retryPolicy` and `job.retryPolicy` ARE + * enforced. They are different keys on different types and spell the delay + * `backoffMs`, not `baseDelayMs` — which is itself the evidence nothing read + * the datasource one, since no code reads both spellings. The conversion + * therefore touches ONLY `datasources`, and the schema's rejection message + * spells the distinction out rather than offering a rename. + * + * `retiredFromLoadPath`: every affected shape is `.strict()` and rejects with + * its prescription (`RETIRED_DATASOURCE_BLOCKS`). + */ +const datasourceInertBlocksRemoved: MetadataConversion = { + id: 'datasource-inert-blocks-removed', + toMajor: 17, + retiredFromLoadPath: true, + surface: 'datasource.retryPolicy / datasource.healthCheck / datasource.external.label / datasource.external.requirePermission', + summary: "datasource keys 'retryPolicy'/'healthCheck' and external 'label'/'requirePermission' removed (#4583 — nothing retried, nothing probed on a schedule, and the federation label/permission were read by nobody)", + apply(stack, emit) { + return mapCollection(stack, 'datasources', (ds, path) => { + const next = stripKeys(ds, ['retryPolicy', 'healthCheck'], emit, path); + // `external.*` sits one level down, so stripKeys (top-level only) cannot + // reach it — drill in, and copy-on-write so an untouched datasource keeps + // its identity for the caller's change detection. + const external = next.external; + if (!external || typeof external !== 'object' || Array.isArray(external)) return next; + const strippedExternal = stripKeys( + external as Record, + ['label', 'requirePermission'], + emit, + `${path}.external`, + ); + if (strippedExternal === external) return next; + return { ...next, external: strippedExternal }; + }); + }, + fixture: { + before: { + datasources: [{ + name: 'warehouse', + driver: 'postgres', + config: { host: 'db.internal', database: 'analytics' }, + healthCheck: { enabled: true, intervalMs: 30000, timeoutMs: 5000 }, + retryPolicy: { maxRetries: 3, baseDelayMs: 1000, maxDelayMs: 30000, backoffMultiplier: 2 }, + schemaMode: 'external', + external: { + label: 'Warehouse — ANALYTICS / PROD', + allowWrites: false, + requirePermission: 'analytics_admin', + }, + }], + }, + // Four notices: one per removed key, counting the two nested ones. + after: { + datasources: [{ + name: 'warehouse', + driver: 'postgres', + config: { host: 'db.internal', database: 'analytics' }, + schemaMode: 'external', + external: { allowWrites: false }, + }], + }, + expectedNotices: 4, + }, +}; + /** * `datasource.capabilities` removed (protocol 17, #4583). * @@ -2534,6 +2617,7 @@ export const CONVERSIONS_BY_MAJOR: Readonly { driver: 'postgres', config: { url: 'postgres://user@warehouse.internal/analytics' }, schemaMode: 'external', - external: { label: 'Analytics Warehouse' }, + // An empty `external: {}` is enough — every key below is a default. It + // used to say `{ label: … }`, which #4583 removed: the federation block + // never had a display name of its own (the top-level `label` is what + // Setup renders), so the key was only ever making this literal look less + // bare. + external: {}, }); expect(ds.schemaMode).toBe('external'); // ExternalDatasourceSettingsSchema defaults @@ -535,16 +540,75 @@ describe('SchemaMode & External Federation (ADR-0015)', () => { it('should parse a fully-specified external settings block', () => { const settings = ExternalDatasourceSettingsSchema.parse({ - label: 'Snowflake — ANALYTICS / PROD', allowedSchemas: ['public', 'mart'], allowWrites: true, validation: { onMismatch: 'warn', checkOnBoot: false, checkIntervalMs: 60_000 }, credentialsRef: 'secret:warehouse/readonly', queryTimeoutMs: 15_000, - requirePermission: 'analytics_admin', }); expect(settings.allowedSchemas).toEqual(['public', 'mart']); expect(settings.validation.onMismatch).toBe('warn'); expect(settings.validation.checkIntervalMs).toBe(60_000); }); + + // ── #4583 batches B/C/D — the remaining inert datasource surface ────────── + // + // Same finding as `capabilities`: declared, strict-guarded, read by nothing. + // Each rejection has to name the mechanism that DOES decide the behaviour, + // which is why these assert the message and not merely the failure. + + it('rejects the `retryPolicy` block, and does NOT send the author to hook/job retryPolicy', () => { + const result = DatasourceSchema.safeParse({ + name: 'warehouse', + driver: 'sqlite', + config: { filename: ':memory:' }, + retryPolicy: { maxRetries: 5, baseDelayMs: 1000 }, + }); + + expect(result.success).toBe(false); + const msg = JSON.stringify(result.error?.issues ?? []); + // The trap this pins: `hook.retryPolicy` / `job.retryPolicy` ARE enforced + // and spell the delay `backoffMs`. An author who reads "retryPolicy is + // dead" and renames the key onto a datasource anyway, or moves these + // values to a hook expecting the datasource to retry, has not been helped. + expect(msg).toMatch(/backoffMs/); + expect(msg).toMatch(/hook|job/); + }); + + it('rejects the `healthCheck` block and points at the on-demand probe', () => { + const result = DatasourceSchema.safeParse({ + name: 'warehouse', + driver: 'sqlite', + config: { filename: ':memory:' }, + healthCheck: { enabled: true, intervalMs: 30_000 }, + }); + + expect(result.success).toBe(false); + const msg = JSON.stringify(result.error?.issues ?? []); + expect(msg).toMatch(/ping|checkHealth/); + // Must not be confused with the one recurring datasource timer, which + // checks schema drift rather than liveness. + expect(msg).toMatch(/checkIntervalMs/); + }); + + it('rejects `external.label` in favour of the top-level label', () => { + const result = ExternalDatasourceSettingsSchema.safeParse({ + label: 'Snowflake — ANALYTICS / PROD', + allowWrites: false, + }); + + expect(result.success).toBe(false); + expect(JSON.stringify(result.error?.issues ?? [])).toMatch(/top-level/i); + }); + + it('rejects `external.requirePermission` — it gated nothing', () => { + const result = ExternalDatasourceSettingsSchema.safeParse({ + requirePermission: 'analytics_admin', + allowWrites: false, + }); + + expect(result.success).toBe(false); + // The prescription must point at what really governs access. + expect(JSON.stringify(result.error?.issues ?? [])).toMatch(/permission set|RLS/i); + }); }); diff --git a/packages/spec/src/data/datasource.zod.ts b/packages/spec/src/data/datasource.zod.ts index 8e4ed38bd5..d3bbdf890b 100644 --- a/packages/spec/src/data/datasource.zod.ts +++ b/packages/spec/src/data/datasource.zod.ts @@ -45,8 +45,8 @@ const DRIVER_DEFINITION_KEYS = ['id', 'label', 'description', 'icon', 'configSch /** Keys {@link ExternalDatasourceSettingsSchema} declares (drift-guarded by datasource.test.ts). */ const EXTERNAL_SETTINGS_KEYS = [ - 'label', 'allowedSchemas', 'allowWrites', 'validation', - 'credentialsRef', 'queryTimeoutMs', 'requirePermission', + 'allowedSchemas', 'allowWrites', 'validation', + 'credentialsRef', 'queryTimeoutMs', ] as const; /** Keys the external `validation` block declares (drift-guarded by datasource.test.ts). */ @@ -55,22 +55,16 @@ const EXTERNAL_VALIDATION_KEYS = ['onMismatch', 'checkOnBoot', 'checkIntervalMs' /** Keys {@link DatasourceSchema} declares (drift-guarded by datasource.test.ts). */ const DATASOURCE_KEYS = [ 'name', 'label', 'driver', 'config', 'pool', - 'healthCheck', 'ssl', 'retryPolicy', 'description', 'active', 'autoConnect', + 'ssl', 'description', 'active', 'autoConnect', 'schemaMode', 'external', 'origin', ] as const; /** Keys the datasource `pool` block declares (drift-guarded by datasource.test.ts). */ const POOL_KEYS = ['min', 'max', 'idleTimeoutMillis', 'connectionTimeoutMillis'] as const; -/** Keys the datasource `healthCheck` block declares (drift-guarded by datasource.test.ts). */ -const HEALTH_CHECK_KEYS = ['enabled', 'intervalMs', 'timeoutMs'] as const; - /** Keys the datasource `ssl` block declares (drift-guarded by datasource.test.ts). */ const SSL_KEYS = ['enabled', 'rejectUnauthorized', 'ca', 'cert', 'key'] as const; -/** Keys the datasource `retryPolicy` block declares (drift-guarded by datasource.test.ts). */ -const DATASOURCE_RETRY_POLICY_KEYS = ['maxRetries', 'baseDelayMs', 'maxDelayMs', 'backoffMultiplier'] as const; - const CAPABILITIES_REMOVED_PREFIX = '`datasource.capabilities` was removed in @objectstack/spec 17.0.0 (#4583, ADR-0049) — ' + 'all eleven flags were declared, strict-guarded and read by nobody. '; @@ -100,6 +94,46 @@ const RETIRED_CAPABILITIES: Record = { + 'Tracked in #4584.', }; +/** + * Tombstones for the three inert blocks retired alongside `capabilities` + * (#4583 batches B/C/D). + * + * Same finding in each case: declared, `.strict()`-guarded, and read by no + * runtime path. What differs — and what each prescription has to name — is the + * mechanism that DOES decide the behaviour, because pointing an author at the + * wrong one is how the `readOnly` defect propagated for three releases. + */ +const RETIRED_DATASOURCE_BLOCKS: Record = { + retryPolicy: + '`datasource.retryPolicy` was removed in @objectstack/spec 17.0.0 (#4583, ADR-0049) — no ' + + 'connect or query path ever retried on it. Connection failure is handled by the boot ' + + 'policy in the datasource connection service (degraded boot, or `bootCritical` fail-fast), ' + + 'which does not retry on a schedule. Delete the block. ' + + 'CAREFUL — do NOT "fix" this by renaming keys: `hook.retryPolicy` and `job.retryPolicy` ARE ' + + 'enforced, but they are a DIFFERENT key on a different type and spell the delay `backoffMs`, ' + + 'not `baseDelayMs`. Moving these values onto a hook or a job only makes sense if you ' + + 'actually want that hook or job retried. Run `os migrate meta --from 16` to remove it.', + healthCheck: + '`datasource.healthCheck` was removed in @objectstack/spec 17.0.0 (#4583, ADR-0049) — no ' + + 'health-check loop ever read it, so `enabled: true` scheduled nothing and the two timeouts ' + + 'bounded nothing. Connection liveness is probed ON DEMAND through the driver handle ' + + '(`ping()` / `checkHealth()`), which the datasource admin service calls for "Test ' + + 'connection". The only recurring datasource timer is `external.validation.checkIntervalMs`, ' + + 'which checks SCHEMA DRIFT — a different concern, not a liveness probe. Delete the block. ' + + 'Run `os migrate meta --from 16` to remove it.', + externalLabel: + '`external.label` was removed in @objectstack/spec 17.0.0 (#4583, ADR-0049) — nothing read ' + + "the federation block's own label. Use the datasource's TOP-LEVEL `label`, which is what " + + 'Setup → Datasources actually renders. Run `os migrate meta --from 16` to remove it.', + externalRequirePermission: + '`external.requirePermission` was removed in @objectstack/spec 17.0.0 (#4583, ADR-0049) — no ' + + 'authorization check ever consulted it, so a permission named here gated nothing. Access to ' + + "a federated datasource's data is governed by the ordinary object permission sets and RLS, " + + 'exactly as for a managed datasource. Naming a permission that is never required is the ' + + 'false-compliance shape ADR-0049 exists to remove — grant or withhold the object ' + + 'permissions instead. Run `os migrate meta --from 16` to remove it.', +}; + /** * A connection detail written one level too high — it belongs inside `config`. * @@ -158,9 +192,11 @@ const externalSettingsUnknownKeyError = strictUnknownKeyError({ secretref: 'credentialsRef', timeoutms: 'queryTimeoutMs', querytimeout: 'queryTimeoutMs', - permission: 'requirePermission', }, guidance: { + label: RETIRED_DATASOURCE_BLOCKS.externalLabel, + requirePermission: RETIRED_DATASOURCE_BLOCKS.externalRequirePermission, + permission: RETIRED_DATASOURCE_BLOCKS.externalRequirePermission, password: '`password` must never be inlined. Put the secret in the secrets store and reference ' + 'it with `credentialsRef` (e.g. `credentialsRef: "secret:warehouse/password"`).', @@ -237,7 +273,6 @@ const datasourceUnknownKeyError = strictUnknownKeyError({ mode: 'schemaMode', schema_mode: 'schemaMode', federation: 'external', - retry: 'retryPolicy', tls: 'ssl', }, guidance: { @@ -257,6 +292,10 @@ const datasourceUnknownKeyError = strictUnknownKeyError({ replicas: RETIRED_READ_REPLICAS, capabilities: RETIRED_CAPABILITIES.capabilities, readOnly: RETIRED_CAPABILITIES.readOnly, + retryPolicy: RETIRED_DATASOURCE_BLOCKS.retryPolicy, + retry: RETIRED_DATASOURCE_BLOCKS.retryPolicy, + healthCheck: RETIRED_DATASOURCE_BLOCKS.healthCheck, + healthcheck: RETIRED_DATASOURCE_BLOCKS.healthCheck, }, history: 'Until #4001 these were dropped silently — a connection key written one level too high ' @@ -282,18 +321,6 @@ const poolUnknownKeyError = strictUnknownKeyError({ + 'no matter what was written. Note both timeouts end in `Millis`, not `Ms`.', }); -const healthCheckUnknownKeyError = strictUnknownKeyError({ - surface: "this datasource's healthCheck config", - knownKeys: HEALTH_CHECK_KEYS, - aliases: { - active: 'enabled', - interval: 'intervalMs', - intervalmillis: 'intervalMs', - timeout: 'timeoutMs', - timeoutmillis: 'timeoutMs', - }, - history: 'Until #4001 these were dropped silently — health checks ran on the defaults.', -}); const sslUnknownKeyError = strictUnknownKeyError({ surface: "this datasource's ssl config", @@ -319,23 +346,6 @@ const sslUnknownKeyError = strictUnknownKeyError({ + 'effect looked identical to one that did.', }); -const datasourceRetryPolicyUnknownKeyError = strictUnknownKeyError({ - surface: "this datasource's retryPolicy", - knownKeys: DATASOURCE_RETRY_POLICY_KEYS, - aliases: { - retries: 'maxRetries', - attempts: 'maxRetries', - backoffms: 'baseDelayMs', - basedelay: 'baseDelayMs', - delayms: 'baseDelayMs', - maxdelay: 'maxDelayMs', - multiplier: 'backoffMultiplier', - backoff: 'backoffMultiplier', - }, - history: - 'Until #4001 these were dropped silently — reconnects ran on the defaults. Note a hook ' - + 'retryPolicy spells its delay `backoffMs`; a datasource spells it `baseDelayMs`.', -}); @@ -400,8 +410,6 @@ export type SchemaMode = z.infer; * boot/drift validation behaviour, credentials reference, and query caps. */ export const ExternalDatasourceSettingsSchema = z.object({ - label: z.string().optional() - .describe('Display label, e.g. "Snowflake — ANALYTICS / PROD"'), allowedSchemas: z.array(z.string()).optional() .describe('Whitelist of remote schemas/databases that may be exposed.'), allowWrites: z.boolean().default(false) @@ -419,8 +427,6 @@ export const ExternalDatasourceSettingsSchema = z.object({ .describe('Reference into the secrets store; never inline credentials.'), queryTimeoutMs: z.number().default(30_000) .describe('Hard cap on per-query execution time.'), - requirePermission: z.string().optional() - .describe('Optional convenience: gate the entire datasource behind a single role.'), }, { error: externalSettingsUnknownKeyError }).strict() .describe('External datasource federation settings (schemaMode != "managed")'); @@ -491,12 +497,6 @@ export const DatasourceSchema = lazySchema(() => z.object({ * Manually override what the driver claims to support. */ - /** Health Check */ - healthCheck: z.object({ - enabled: z.boolean().default(true).describe('Enable health check endpoint'), - intervalMs: z.number().default(30000).describe('Health check interval in milliseconds'), - timeoutMs: z.number().default(5000).describe('Health check timeout in milliseconds'), - }, { error: healthCheckUnknownKeyError }).strict().optional().describe('Datasource health check configuration'), /** SSL/TLS Configuration */ ssl: z.object({ @@ -507,13 +507,6 @@ export const DatasourceSchema = lazySchema(() => z.object({ key: z.string().optional().describe('Client private key (PEM format or path to file)'), }, { error: sslUnknownKeyError }).strict().optional().describe('SSL/TLS configuration for secure database connections'), - /** Retry Policy */ - retryPolicy: z.object({ - maxRetries: z.number().default(3).describe('Maximum number of retry attempts'), - baseDelayMs: z.number().default(1000).describe('Base delay between retries in milliseconds'), - maxDelayMs: z.number().default(30000).describe('Maximum delay between retries in milliseconds'), - backoffMultiplier: z.number().default(2).describe('Exponential backoff multiplier'), - }, { error: datasourceRetryPolicyUnknownKeyError }).strict().optional().describe('Connection retry policy for transient failures'), /** Description */ description: z.string().optional().describe('Internal description'), diff --git a/packages/spec/src/migrations/registry.ts b/packages/spec/src/migrations/registry.ts index 4e31d8813c..6e138577f4 100644 --- a/packages/spec/src/migrations/registry.ts +++ b/packages/spec/src/migrations/registry.ts @@ -587,6 +587,7 @@ const step17: MigrationStep = { 'skill-trigger-phrases-removed', 'stack-api-require-auth-removed', 'datasource-capabilities-removed', + 'datasource-inert-blocks-removed', 'flow-node-wait-timeout-keys-removed', 'datasource-read-replicas-removed', 'flow-node-script-branch-keys-removed',