diff --git a/.changeset/datasource-config-key-alias-conversion.md b/.changeset/datasource-config-key-alias-conversion.md new file mode 100644 index 0000000000..e05d4f24ae --- /dev/null +++ b/.changeset/datasource-config-key-alias-conversion.md @@ -0,0 +1,38 @@ +--- +"@objectstack/spec": minor +"@objectstack/service-datasource": minor +--- + +feat(spec,service-datasource): graduate the driver factory's four legacy `datasource.config` `??` fallbacks into an ADR-0087 conversion (#4456) + +`createDefaultDatasourceDriverFactory` still carried four undeclared read-side +`??` fallbacks that predate the #4410 config gate: sqlite `file`/`database` +(canonical `filename`), postgres/mysql `connectionString` (canonical `url`), +postgres/mysql/mongo `user` (canonical `username`), and mongo `uri` (canonical +`url`). They were never part of the contract — no schema, form, doc or example +ever named them — and they kept working only because the reader was lenient +(AGENTS.md Prime Directive #12 debt). + +**FROM → TO, applied automatically at load** by the new conversion entry +`datasource-config-driver-key-aliases` (retired-from-load-path; replayed over +stored `sys_metadata` rows by `applyConversionsToStoredItem` and by +`os migrate meta`): + +- sqlite / sqlite-wasm: `config.file` / `config.database` → `config.filename` +- postgres / mysql: `config.connectionString` → `config.url`, `config.user` → `config.username` +- mongo: `config.uri` → `config.url`, `config.user` → `config.username` + +The mapping is driver-aware — `database` renames only under sqlite, where it +aliased the file path; for postgres/mysql/mongo it is a canonical key and is +untouched. A canonical key already present wins; the legacy alias is left +shadowed (the factory's `??` precedence, preserved). + +**Behaviour change (the deletion):** the factory now reads exactly one spelling +per key. A `DatasourceConnectionSpec` handed to the factory *directly* with a +legacy spelling is no longer honoured — authored metadata was already rejected +by the per-driver zod gate with a rename hint (#4410), and stored runtime +datasource rows are canonicalized at every rehydration seam (including the +`sys_metadata` restore path in `DatasourceAdminServicePlugin`, which now +replays the full conversion chain), so no supported path still produces the +legacy shape. One-line fix for hand-built specs: use the canonical key from +the table above. diff --git a/content/docs/references/data/driver-sqlite.mdx b/content/docs/references/data/driver-sqlite.mdx index c3fed739ab..4b6c2eeeb1 100644 --- a/content/docs/references/data/driver-sqlite.mdx +++ b/content/docs/references/data/driver-sqlite.mdx @@ -19,17 +19,15 @@ fell back to `:memory:`, and their data vanished on restart with every signal saying the datasource was configured. -`file` and `database` are a different case — the factory reads them as +`file` and `database` once also worked, purely because the factory read them -undeclared `??` fallbacks, so they happened to work while being documented +as undeclared `??` fallbacks. That tolerance has graduated into the declared -nowhere. They are named as renames here rather than blessed: one strict +ADR-0087 conversion `datasource-config-driver-key-aliases` (#4456): stored -contract beats a spelling that works only because a reader is lenient +rows are rewritten to `filename` at load, the factory reads one spelling, -(AGENTS.md Prime Directive #12). The factory keeps its tolerance for records - -already persisted that way; no new one can be authored. +and authoring rejects both with the rename hint below. **Source:** `packages/spec/src/data/driver/sqlite.zod.ts` diff --git a/docs/protocol-upgrade-guide.md b/docs/protocol-upgrade-guide.md index d53bbf56f2..23216d5e6b 100644 --- a/docs/protocol-upgrade-guide.md +++ b/docs/protocol-upgrade-guide.md @@ -158,6 +158,8 @@ The same kind of retirement covers `wait`'s timeout pair (#4158). `waitEventConf 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. +The datasource close-out also graduates the four legacy `datasource.config` spellings the shared driver factory still tolerated via undeclared read-side `??` fallbacks (#4456, the #4410 follow-up): sqlite `file`/`database` (use `filename`), postgres/mysql `connectionString` (use `url`) and `user` (use `username`), and mongo `uri` (use `url`) and `user` (use `username`). #4410 made the authoring gate reject each with a rename hint, but a runtime datasource persisted in `sys_metadata` before the gate kept working only because the factory read leniently — and deleting that tolerance without a conversion would have silently moved data (a stored sqlite `file:` row falls back to `:memory:`). The `datasource-config-driver-key-aliases` conversion rewrites the stored shape to the canonical keys at every rehydration seam, the factory now reads exactly one spelling per key, and the four `??` chains are deleted. Driver-aware by construction: `database` renames only under sqlite, where it aliased the file path — for every other driver it is a canonical key and is untouched. Retired from the load path not for lying but because the authoring gate already rejects the spellings loudly; the chain and the stored-row replay are the seams that accept them. + The `script` flow node converges on its one real path (#4343). It had four ways to name what it ran and only one of them ran anything: `config.actionType: 'email' | 'slack'` were logger-backed stubs that wrote a line, reported success and delivered nothing under any configuration — with `config.template` / `.recipients` / `.variables` feeding a message no channel ever sent; inline `config.script` was recognized and never executed (the built-in runtime has no server-side JS sandbox), so the node warned and no-op'd; and every other `actionType` value was shorthand for a registered-function name, a second spelling of `config.function`. All five keys are retired and `function` becomes required, which is also what finally made the contract PARSEABLE: while the legal key set depended on `actionType`, a flat parse would either reject valid shapes or wave everything through, so `script` (with `subflow`) now runs through the same execute-time contract parse #4277 gave the flat builtins. A shorthand `actionType` CONVERTS into `function` — that is what it meant — unless `function` is already set, in which case it was dead metadata the executor never reached. The other four are dropped outright: nothing read them, so there is no value to preserve, and rebuilding the intent is an authoring decision the tombstones prescribe per branch (a `notify` node for mail — it delivers through the messaging service, the in-app inbox by default and real email once `@objectstack/plugin-email` is installed; a `connector_action` with the Slack connector, or an `http` node posting to a webhook, for Slack; a registered function for an inline body). Retired from the load path for the same reason as the rest: absorbing `actionType: 'email'` silently would let an author keep believing the flow sends mail. ### Mechanical (applied for you) @@ -190,6 +192,7 @@ The `script` flow node converges on its one real path (#4343). It had four ways | `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 | +| `datasource-config-driver-key-aliases` | `datasource.config` | datasource config keys → canonical per driver: sqlite 'file'/'database' → 'filename', postgres/mysql 'connectionString' → 'url' and 'user' → 'username', mongo 'uri' → 'url' and 'user' → 'username' (#4456 — driver-factory `??` fallback graduation) | 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/packages/services/service-datasource/src/__tests__/datasource-admin-plugin.test.ts b/packages/services/service-datasource/src/__tests__/datasource-admin-plugin.test.ts index af1911ed0b..49c2f937cc 100644 --- a/packages/services/service-datasource/src/__tests__/datasource-admin-plugin.test.ts +++ b/packages/services/service-datasource/src/__tests__/datasource-admin-plugin.test.ts @@ -2,6 +2,7 @@ import { describe, it, expect } from 'vitest'; import type { IDatasourceAdminService, IDatasourceDriverFactory } from '../contracts/index.js'; +import type { DatasourceAdminService } from '../datasource-admin-service.js'; import { DatasourceAdminServicePlugin, type DatasourceAdminServicePluginOptions, @@ -285,6 +286,48 @@ describe('DatasourceAdminServicePlugin: runtime datasource durability', () => { expect(after.find((d) => d.name === 'demo_ext')?.origin).toBe('runtime'); }); + // #4456 — this restore path is a stored-row rehydration seam (ADR-0087 D2 + // addendum, #3903): it reads sys_metadata directly, so it must replay the + // conversion chain itself. A row persisted before the #4410 config gate may + // carry the legacy spellings the factory's deleted `??` fallbacks used to + // tolerate; without the replay, a sqlite `file:` row would silently fall + // back to `:memory:` — the data-loss shape the conversion exists to prevent. + it('restores a pre-#4410 row with legacy config keys CANONICAL (conversion chain replayed)', async () => { + const data = fakeSysMetadataEngine(); + const now = new Date().toISOString(); + for (const [name, driver, config] of [ + ['legacy_sqlite', 'sqlite', { file: '/tmp/legacy.db' }], + ['legacy_pg', 'postgres', { connectionString: 'postgresql://db.internal/analytics', user: 'analyst' }], + ['legacy_mongo', 'mongo', { uri: 'mongodb://mongo.internal:27017/events' }], + ] as const) { + data.rows.push({ + id: `meta_${name}`, + name, + type: 'datasource', + scope: 'platform', + metadata: JSON.stringify({ name, driver, config, origin: 'runtime' }), + state: 'active', + version: 1, + created_at: now, + updated_at: now, + }); + } + + const b = await boot({ services: { data } }); + await b.plugin.start(b.ctx); + // The list DTO is a summary; `getDatasource` (concrete service) is the + // config-bearing read the admin routes serve. + const svc = b.service as unknown as DatasourceAdminService; + expect((await svc.getDatasource('legacy_sqlite'))?.config).toEqual({ filename: '/tmp/legacy.db' }); + expect((await svc.getDatasource('legacy_pg'))?.config).toEqual({ + url: 'postgresql://db.internal/analytics', + username: 'analyst', + }); + expect((await svc.getDatasource('legacy_mongo'))?.config).toEqual({ + url: 'mongodb://mongo.internal:27017/events', + }); + }); + it('removes the durable sys_metadata row when a datasource is deleted', async () => { const data = fakeSysMetadataEngine(); const b = await boot({ services: { data } }); diff --git a/packages/services/service-datasource/src/__tests__/default-datasource-driver-factory.test.ts b/packages/services/service-datasource/src/__tests__/default-datasource-driver-factory.test.ts index c10a0150bd..2dff075e37 100644 --- a/packages/services/service-datasource/src/__tests__/default-datasource-driver-factory.test.ts +++ b/packages/services/service-datasource/src/__tests__/default-datasource-driver-factory.test.ts @@ -263,3 +263,94 @@ describe('createDefaultDatasourceDriverFactory — declared keys reach the drive try { await handle.disconnect?.(); } catch { /* pool never opened */ } }); }); + +// #4456 — the four undeclared read-side `??` fallbacks are DELETED. The factory +// reads exactly the canonical key of each driver's config contract; the legacy +// spellings a pre-#4410 stored record may carry are rewritten to canonical at +// every rehydration seam by the ADR-0087 conversion +// `datasource-config-driver-key-aliases`, so they must never reach this code — +// and when one does anyway, it is ignored rather than quietly honoured. +describe('createDefaultDatasourceDriverFactory — legacy config spellings are no longer read (#4456)', () => { + function knexConfigOf(driver: any): any { + return driver?.config ?? driver?.knexConfig ?? driver?.options ?? {}; + } + + async function pgConnection(config: Record): Promise { + const handle: any = await factory().create({ driver: 'postgres', config }); + try { return knexConfigOf(handle.driver ?? handle).connection; } + finally { try { await handle.disconnect?.(); } catch { /* pool never opened */ } } + } + + it('pg: `connectionString` no longer selects the DSN path — discrete fields are used instead', async () => { + const conn = await pgConnection({ + connectionString: 'postgresql://legacy@db.internal/analytics', + host: 'db.internal', + database: 'analytics', + }); + expect(conn.connectionString).toBeUndefined(); + expect(conn).toMatchObject({ host: 'db.internal', database: 'analytics' }); + }); + + it('pg: `user` no longer reaches the client — only the canonical `username` does', async () => { + const legacyOnly = await pgConnection({ host: 'h', database: 'd', user: 'legacy' }); + expect(legacyOnly.user).toBeUndefined(); + const both = await pgConnection({ host: 'h', database: 'd', user: 'legacy', username: 'svc' }); + expect(both.user).toBe('svc'); + }); + + it('mysql: `connectionString`/`user` are ignored the same way', async () => { + const handle: any = await factory().create({ + driver: 'mysql', + config: { connectionString: 'mysql://legacy@db/orders', host: 'db', database: 'orders', user: 'legacy' }, + }); + const conn = knexConfigOf(handle.driver ?? handle).connection; + // Not the DSN string passthrough — the discrete-field object, with no user. + expect(typeof conn).toBe('object'); + expect(conn).toMatchObject({ host: 'db', database: 'orders' }); + expect(conn.user).toBeUndefined(); + try { await handle.disconnect?.(); } catch { /* pool never opened */ } + }); + + it('sqlite-wasm: `file`/`database` no longer name the database — the driver builds `:memory:`', async () => { + const dir = mkdtempSync(join(tmpdir(), 'os-4456-')); + try { + const legacyFile = join(dir, 'legacy.db'); + const handle: any = await factory().create({ + driver: 'sqlite-wasm', + config: { file: legacyFile, database: legacyFile }, + }); + const driver = handle.driver ?? handle; + await driver.connect(); + try { + await driver.syncSchema('note', { name: 'note', fields: { id: { type: 'text' } } }); + await driver.create('note', { id: 'n1' }); + } finally { + try { await driver.disconnect(); } catch { /* noop */ } + } + // An ephemeral `:memory:` database writes nothing at the legacy path. + expect(existsSync(legacyFile)).toBe(false); + } finally { + try { rmSync(dir, { recursive: true, force: true }); } catch { /* noop */ } + } + }, 30_000); + + it('mongo: `uri`/`user` are ignored — the URL is composed from canonical keys only', async () => { + const handle: any = await factory().create({ + driver: 'mongo', + config: { uri: 'mongodb://legacy.internal:27017/legacy', database: 'events', user: 'legacy' }, + }); + const driver: any = handle.driver ?? handle; + // No canonical `url`/`host`/`username` → composed from defaults + `database`, + // with no auth part; the legacy `uri` never passes through. + expect(driver.config.url).toBe('mongodb://localhost:27017/events'); + }); + + it('mongo: the canonical spellings still compose the URL (control)', async () => { + const handle: any = await factory().create({ + driver: 'mongo', + config: { host: 'mongo.internal', port: 27017, database: 'events', username: 'svc', password: 'pw' }, + }); + const driver: any = handle.driver ?? handle; + expect(driver.config.url).toBe('mongodb://svc:pw@mongo.internal:27017/events'); + }); +}); diff --git a/packages/services/service-datasource/src/datasource-admin-plugin.ts b/packages/services/service-datasource/src/datasource-admin-plugin.ts index 0c6966df1d..eec828264e 100644 --- a/packages/services/service-datasource/src/datasource-admin-plugin.ts +++ b/packages/services/service-datasource/src/datasource-admin-plugin.ts @@ -1,6 +1,7 @@ // Copyright (c) 2025 ObjectStack. Licensed under the Apache-2.0 license. import type { Plugin, PluginContext } from '@objectstack/core'; +import { applyConversionsToStoredItem } from '@objectstack/spec'; import { registerMetadataTypeActions } from '@objectstack/spec/kernel'; import type { IDatasourceDriverFactory, @@ -107,7 +108,15 @@ async function loadDatasourceRows(engine: DataEngineLike | undefined): Promise)); + const parsed = typeof raw === 'string' ? JSON.parse(raw) : (raw as Record); + // This is a stored-row rehydration seam (ADR-0087 D2 addendum, #3903): + // rows written under a past protocol replay the FULL conversion chain — + // e.g. a pre-#4410 sqlite record whose config still says `file:` is + // served with the canonical `filename`, which is the only spelling the + // driver factory reads since #4456. The direct sys_metadata read here + // bypasses the metadata service's own converting loaders, so the pass + // must happen locally. + out.push(applyConversionsToStoredItem(DS_META_TYPE, parsed)); } catch { /* skip corrupt row */ } diff --git a/packages/services/service-datasource/src/default-datasource-driver-factory.ts b/packages/services/service-datasource/src/default-datasource-driver-factory.ts index a2e1c0671b..bb018413a4 100644 --- a/packages/services/service-datasource/src/default-datasource-driver-factory.ts +++ b/packages/services/service-datasource/src/default-datasource-driver-factory.ts @@ -130,21 +130,17 @@ function buildSqlConnection(spec: DatasourceConnectionSpec, client: 'pg' | 'bett const cfg = (spec.config ?? {}) as Record; if (client === 'better-sqlite3') { - // `file` / `database` are pre-#4410 tolerance for shapes already persisted - // by the runtime store. Authoring rejects both with a rename hint - // (`SqliteConfigSchema`), so nothing new can arrive spelled this way. - const filename = - (cfg.filename as string | undefined) ?? - (cfg.file as string | undefined) ?? - (cfg.database as string | undefined) ?? - ':memory:'; - return { filename }; + // `filename` is the whole contract (`SqliteConfigSchema`). The legacy + // `file`/`database` spellings are rewritten to it at load by the ADR-0087 + // conversion `datasource-config-driver-key-aliases` (#4456), so no `??` + // tolerance survives here; authoring rejects them with a rename hint. + return { filename: (cfg.filename as string | undefined) ?? ':memory:' }; } - // pg — accept either a connection string (`url`/`connectionString`) or - // discrete fields. The secret is the password and is never part of `config`. + // pg — accept either a connection string (`url`) or discrete fields. The + // secret is the password and is never part of `config`. const ssl = resolveSslOption(spec); - const url = (cfg.url as string | undefined) ?? (cfg.connectionString as string | undefined); + const url = cfg.url as string | undefined; if (url) { // For a DSN, a separately-supplied secret overrides the embedded password. // TLS still applies: `sslmode` in a DSN and the `ssl` option are separate @@ -160,7 +156,7 @@ function buildSqlConnection(spec: DatasourceConnectionSpec, client: 'pg' | 'bett host: cfg.host, port: cfg.port, database: cfg.database, - user: cfg.user ?? cfg.username, + user: cfg.username, ...(spec.secret ? { password: spec.secret } : cfg.password ? { password: cfg.password } : {}), ...(ssl !== undefined ? { ssl } : {}), ...pgConnectionExtras(cfg), @@ -197,13 +193,13 @@ function buildSqlPool(spec: DatasourceConnectionSpec): Record { function buildMysqlConnection(spec: DatasourceConnectionSpec): unknown { const cfg = (spec.config ?? {}) as Record; const mysqlSsl = resolveSslOption(spec); - const url = (cfg.url as string | undefined) ?? (cfg.connectionString as string | undefined); + const url = cfg.url as string | undefined; if (url) return url; return { host: cfg.host, port: cfg.port, database: cfg.database, - user: cfg.user ?? cfg.username, + user: cfg.username, ...(spec.secret ? { password: spec.secret } : cfg.password ? { password: cfg.password } : {}), ...(mysqlSsl !== undefined ? { ssl: mysqlSsl } : {}), }; @@ -293,14 +289,16 @@ function buildMemoryConfig(spec: DatasourceConnectionSpec): Record; - // `uri` is pre-#4410 tolerance for already-persisted shapes; authoring - // rejects it with a rename hint to `url` (`MongoConfigSchema`). - const explicit = (cfg.url as string | undefined) ?? (cfg.uri as string | undefined); + // `url` is the one spelling (`MongoConfigSchema`); the legacy `uri`/`user` + // are rewritten to `url`/`username` at load by the ADR-0087 conversion + // `datasource-config-driver-key-aliases` (#4456), and authoring rejects + // them with a rename hint. + const explicit = cfg.url as string | undefined; if (explicit) return explicit; const host = (cfg.host as string | undefined) ?? 'localhost'; const port = (cfg.port as number | string | undefined) ?? 27017; const db = (cfg.database as string | undefined) ?? ''; - const user = (cfg.user as string | undefined) ?? (cfg.username as string | undefined); + const user = cfg.username as string | undefined; const password = spec.secret ?? (cfg.password as string | undefined) ?? ''; const auth = user ? `${encodeURIComponent(user)}:${encodeURIComponent(password)}@` : ''; const authSource = cfg.authSource as string | undefined; diff --git a/packages/spec/spec-changes.json b/packages/spec/spec-changes.json index 5a0c00f6ab..e2d76d5b65 100644 --- a/packages/spec/spec-changes.json +++ b/packages/spec/spec-changes.json @@ -224,6 +224,12 @@ "conversionId": "datasource-inert-blocks-removed", "toMajor": 17 }, + { + "surface": "datasource.config", + "to": "datasource config keys → canonical per driver: sqlite 'file'/'database' → 'filename', postgres/mysql 'connectionString' → 'url' and 'user' → 'username', mongo 'uri' → 'url' and 'user' → 'username' (#4456 — driver-factory `??` fallback graduation)", + "conversionId": "datasource-config-driver-key-aliases", + "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)", @@ -731,6 +737,12 @@ "conversionId": "datasource-inert-blocks-removed", "toMajor": 17 }, + { + "surface": "datasource.config", + "to": "datasource config keys → canonical per driver: sqlite 'file'/'database' → 'filename', postgres/mysql 'connectionString' → 'url' and 'user' → 'username', mongo 'uri' → 'url' and 'user' → 'username' (#4456 — driver-factory `??` fallback graduation)", + "conversionId": "datasource-config-driver-key-aliases", + "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/conversions.test.ts b/packages/spec/src/conversions/conversions.test.ts index 3d67017a6c..ee9788d859 100644 --- a/packages/spec/src/conversions/conversions.test.ts +++ b/packages/spec/src/conversions/conversions.test.ts @@ -507,4 +507,45 @@ describe('conversion layer (ADR-0087 D2)', () => { expect(() => FlowSchema.parse(withoutDefault)).toThrow(); }); }); + + // #4456 — the driver-factory `??` fallback graduation. The mappings are + // driver-scoped by construction; these pin the two edges the flat fixture + // pair cannot express as sharply: the same key converting under one driver + // and not another, and the load-path posture. + describe('datasource-config-driver-key-aliases (#4456)', () => { + const convert = (datasources: unknown[]) => + collectConversionNotices({ datasources }, { includeRetired: true }); + + it('renames `database` → `filename` ONLY under sqlite — it is canonical for the SQL/mongo drivers', () => { + const { stack, notices } = convert([ + { name: 'a', driver: 'sqlite', config: { database: './a.db' } }, + { name: 'b', driver: 'postgres', config: { host: 'db', database: 'analytics' } }, + { name: 'c', driver: 'mysql', config: { host: 'db', database: 'orders' } }, + { name: 'd', driver: 'mongo', config: { host: 'db', database: 'events' } }, + ]); + const [a, b, c, d] = stack.datasources as Array<{ config: Record }>; + expect(a!.config).toEqual({ filename: './a.db' }); + expect(b!.config).toEqual({ host: 'db', database: 'analytics' }); + expect(c!.config).toEqual({ host: 'db', database: 'orders' }); + expect(d!.config).toEqual({ host: 'db', database: 'events' }); + expect(notices.filter((n) => n.conversionId === 'datasource-config-driver-key-aliases')).toHaveLength(1); + }); + + it('does not touch a plugin-contributed driver id — no contract, no rewrite', () => { + const before = { datasources: [{ name: 'x', driver: 'com.vendor.snowflake', config: { user: 'svc' } }] }; + const { stack, notices } = collectConversionNotices(structuredClone(before), { includeRetired: true }); + expect(stack).toEqual(before); + expect(notices).toHaveLength(0); + }); + + it('is retired from the load path — an AUTHORED legacy spelling is not silently absorbed', () => { + // Without includeRetired (the normalizeStackInput posture) the stack is + // untouched: the authoring gate rejects `file:` with a rename hint, and + // this conversion must not run ahead of it and defeat #4410. + const before = { datasources: [{ name: 'a', driver: 'sqlite', config: { file: './a.db' } }] }; + const { stack, notices } = collectConversionNotices(structuredClone(before)); + expect(stack).toEqual(before); + expect(notices).toHaveLength(0); + }); + }); }); diff --git a/packages/spec/src/conversions/registry.ts b/packages/spec/src/conversions/registry.ts index 18df3ff4db..ca85bfcc00 100644 --- a/packages/spec/src/conversions/registry.ts +++ b/packages/spec/src/conversions/registry.ts @@ -16,7 +16,8 @@ */ import type { ConversionApplication, MetadataConversion } from './types.js'; -import { mapCollection, mapFlowNodes, mapPages, renameConfigKey, renameKey } from './walk.js'; +import { mapCollection, mapDatasources, mapFlowNodes, mapPages, renameConfigKey, renameKey } from './walk.js'; +import { resolveDriverId, type BuiltinDriverId } from '../data/driver/config-registry.zod.js'; /** * Flow callout node type rename (protocol 11.0). @@ -2427,6 +2428,129 @@ const datasourceReadReplicasRemoved: MetadataConversion = { }, }; +/** + * The legacy `datasource.config` spellings the driver factory used to read via + * undeclared `??` fallbacks, per canonical driver id — exactly the set that + * HAPPENED TO WORK before #4456, nothing more. + * + * Deliberately narrower than the schemas' rename-hint alias tables: an alias + * that only ever produced a rejection hint (`path:`, `dsn:`, `hostname:`, …) + * never worked at run time, so "converting" it would CHANGE behaviour — e.g. a + * stored sqlite `path:` fell back to `:memory:`, and rewriting it to + * `filename` would silently move the database. D2 scope guard: lossless and + * behaviour-preserving only. + * + * Driver-awareness is load-bearing: `database` means "rename to `filename`" + * ONLY under sqlite — for postgres/mysql/mongo it is a canonical key and must + * not be touched. + */ +const DATASOURCE_CONFIG_KEY_ALIASES: Readonly< + Partial>> +> = { + // `filename ?? file ?? database` was the factory's precedence; pair order + // mirrors it, so with both aliases present `file` wins and `database` is + // left shadowed — exactly what the `??` chain resolved to. + sqlite: [['file', 'filename'], ['database', 'filename']], + 'sqlite-wasm': [['file', 'filename'], ['database', 'filename']], + postgres: [['connectionString', 'url'], ['user', 'username']], + mysql: [['connectionString', 'url'], ['user', 'username']], + mongo: [['uri', 'url'], ['user', 'username']], +}; + +/** + * Datasource `config` legacy key aliases → canonical, per driver (protocol 17, + * #4456 — the #4410 close-out). + * + * #4410 gave `datasource.config` its per-driver zod gate, so the AUTHORING + * surface has exactly one spelling per key and rejects the legacy ones with a + * rename hint. What the gate could not fix is data at rest: a runtime + * datasource stored in `sys_metadata` before the gate may carry `file:` + * (sqlite), `connectionString:`/`user:` (postgres/mysql), or `uri:`/`user:` + * (mongo), and until now those kept working only because + * `createDefaultDatasourceDriverFactory` carried undeclared read-side `??` + * fallbacks — the exact PD #12 debt {@link flowNodeFilterAlias} pioneered the + * retirement path for. Deleting the fallbacks without this entry would + * silently change where a stored datasource's data lives (a sqlite `file:` + * row would fall back to `:memory:`). + * + * So the tolerance graduates here: every stored-row rehydration seam replays + * the full chain (`applyConversionsToStoredItem`, #3903), hands the factory + * the canonical key, and the factory reads ONE spelling. Precedence follows + * {@link renameKey}: a canonical key already present wins and the alias is + * left shadowed in place, which is also what the factory's `??` chains + * resolved to. + * + * **Retired from the load path** — not because the keys misdescribed + * themselves (they were honest spellings, merely undeclared), but because the + * authoring gate ALREADY rejects each of them with a rename hint + * (`strictUnknownKeyError` alias tables), and a live-window entry at + * `normalizeStackInput` would run before that gate and silently absorb the + * spelling #4410 deliberately made loud. Stored rows and `migrate meta` are + * exactly the `includeRetired` seams. + */ +const datasourceConfigDriverKeyAliases: MetadataConversion = { + id: 'datasource-config-driver-key-aliases', + toMajor: 17, + retiredFromLoadPath: true, + surface: 'datasource.config', + summary: + "datasource config keys → canonical per driver: sqlite 'file'/'database' → 'filename', " + + "postgres/mysql 'connectionString' → 'url' and 'user' → 'username', mongo 'uri' → 'url' " + + "and 'user' → 'username' (#4456 — driver-factory `??` fallback graduation)", + apply(stack, emit) { + return mapDatasources(stack, (ds, path) => { + const kind = resolveDriverId(ds.driver); + const pairs = kind ? DATASOURCE_CONFIG_KEY_ALIASES[kind] : undefined; + if (!pairs) return ds; + const config = ds.config; + if (!isDict(config)) return ds; + let nextConfig = config; + for (const [from, to] of pairs) { + const renamed = renameKey(nextConfig, from, to); + if (!renamed) continue; // absent, or canonical already wins (alias stays shadowed) + emit({ from, to, path: `${path}.config.${to}` }); + nextConfig = renamed; + } + return nextConfig === config ? ds : { ...ds, config: nextConfig }; + }); + }, + fixture: { + before: { + datasources: [ + { name: 'app_db', driver: 'sqlite', config: { file: './data/app.db' } }, + // driver-id aliases resolve too — `sqlite3` selects the sqlite contract + { name: 'archive_db', driver: 'sqlite3', config: { database: './data/archive.db' } }, + { + name: 'warehouse', + driver: 'pg', + config: { connectionString: 'postgresql://db.internal:5432/analytics', user: 'analyst' }, + }, + // `database` is CANONICAL for mysql — only `user` converts + { name: 'orders', driver: 'mysql', config: { host: 'db.internal', database: 'orders', user: 'svc_orders' } }, + { name: 'events', driver: 'mongodb', config: { uri: 'mongodb://mongo.internal:27017/events' } }, + // canonical already present → the shadowed alias is left alone (no notice) + { name: 'scratch', driver: 'sqlite', config: { filename: ':memory:', file: 'ignored.db' } }, + ], + }, + after: { + datasources: [ + { name: 'app_db', driver: 'sqlite', config: { filename: './data/app.db' } }, + { name: 'archive_db', driver: 'sqlite3', config: { filename: './data/archive.db' } }, + { + name: 'warehouse', + driver: 'pg', + config: { url: 'postgresql://db.internal:5432/analytics', username: 'analyst' }, + }, + { name: 'orders', driver: 'mysql', config: { host: 'db.internal', database: 'orders', username: 'svc_orders' } }, + { name: 'events', driver: 'mongodb', config: { url: 'mongodb://mongo.internal:27017/events' } }, + { name: 'scratch', driver: 'sqlite', config: { filename: ':memory:', file: 'ignored.db' } }, + ], + }, + // app_db 1 + archive_db 1 + warehouse 2 + orders 1 + events 1 + scratch 0. + expectedNotices: 6, + }, +}; + /** * `script` node config — the four retired dispatch branches (protocol 17, #4343). * @@ -2618,6 +2742,7 @@ export const CONVERSIONS_BY_MAJOR: Readonly expect(applyConversionsToStoredItem('object', arr)).toBe(arr); }); + // #4456 — the driver-factory `??` fallback graduation. A runtime datasource + // persisted before the #4410 config gate may carry the legacy spellings; the + // stored pass MUST serve it canonical, because the factory now reads exactly + // one spelling per key (deleting the fallbacks without this replay would + // silently move a sqlite `file:` row's data to `:memory:`). + describe('stored datasource rows (datasource-config-driver-key-aliases, #4456)', () => { + it.each([ + ['sqlite', { file: './data/app.db' }, { filename: './data/app.db' }], + ['sqlite', { database: './data/app.db' }, { filename: './data/app.db' }], + ['postgres', { connectionString: 'postgresql://db/x', user: 'svc' }, { url: 'postgresql://db/x', username: 'svc' }], + ['mysql', { host: 'db', database: 'orders', user: 'svc' }, { host: 'db', database: 'orders', username: 'svc' }], + ['mongo', { uri: 'mongodb://db/x', user: 'svc' }, { url: 'mongodb://db/x', username: 'svc' }], + ])('serves a stored %s row with legacy config keys canonical', (driver, config, expected) => { + const row = { name: 'legacy_ds', driver, config, origin: 'runtime' }; + const out = applyConversionsToStoredItem('datasource', row) as { config: Record }; + expect(out.config).toEqual(expected); + expect(out).toMatchObject({ name: 'legacy_ds', driver, origin: 'runtime' }); + }); + + it('leaves `database` alone for the drivers where it is canonical', () => { + const row = { name: 'wh', driver: 'postgres', config: { host: 'db', database: 'analytics' } }; + expect(applyConversionsToStoredItem('datasource', row)).toBe(row); + }); + + it('lets a canonical key win over a shadowed legacy alias', () => { + const row = { name: 'ds', driver: 'sqlite', config: { filename: './real.db', file: './stale.db' } }; + const out = applyConversionsToStoredItem('datasource', row) as { config: Record }; + // renameKey convention: canonical present → alias left shadowed, untouched. + expect(out.config).toEqual({ filename: './real.db', file: './stale.db' }); + }); + }); + it('threads the conflict guard context through (flow callers that own a registry)', () => { const flow = { name: 'notify_flow', diff --git a/packages/spec/src/conversions/walk.ts b/packages/spec/src/conversions/walk.ts index e4a2381757..81dc8df1c8 100644 --- a/packages/spec/src/conversions/walk.ts +++ b/packages/spec/src/conversions/walk.ts @@ -173,6 +173,21 @@ export function mapPages(stack: Dict, mapper: (page: Dict, path: string) => Dict return { ...stack, pages: nextPages }; } +/** + * Immutably map every datasource in `stack.datasources[]`. + * + * `mapper` receives each datasource dict and its path (`datasources[i]`) and + * returns the same reference (no change) or a new dict. The stack and the + * `datasources` array are copied only when a datasource actually changed — + * the {@link mapPages} contract, delegated to {@link mapCollection}. + */ +export function mapDatasources( + stack: Dict, + mapper: (datasource: Dict, path: string) => Dict, +): Dict { + return mapCollection(stack, 'datasources', mapper); +} + /** * Immutably map every dict item of a top-level array collection * (`stack[key][]`) — the generic form of {@link mapPages}, for conversions diff --git a/packages/spec/src/data/driver/sqlite.zod.ts b/packages/spec/src/data/driver/sqlite.zod.ts index 97b27b4604..f9ffd909b4 100644 --- a/packages/spec/src/data/driver/sqlite.zod.ts +++ b/packages/spec/src/data/driver/sqlite.zod.ts @@ -21,12 +21,11 @@ import { * fell back to `:memory:`, and their data vanished on restart with every signal * saying the datasource was configured. * - * `file` and `database` are a different case — the factory reads them as - * undeclared `??` fallbacks, so they happened to work while being documented - * nowhere. They are named as renames here rather than blessed: one strict - * contract beats a spelling that works only because a reader is lenient - * (AGENTS.md Prime Directive #12). The factory keeps its tolerance for records - * already persisted that way; no new one can be authored. + * `file` and `database` once also worked, purely because the factory read them + * as undeclared `??` fallbacks. That tolerance has graduated into the declared + * ADR-0087 conversion `datasource-config-driver-key-aliases` (#4456): stored + * rows are rewritten to `filename` at load, the factory reads one spelling, + * and authoring rejects both with the rename hint below. */ const SQLITE_CONFIG_KEYS = ['filename', 'autoMigrate'] as const; diff --git a/packages/spec/src/migrations/registry.ts b/packages/spec/src/migrations/registry.ts index 6e138577f4..c438eda8bb 100644 --- a/packages/spec/src/migrations/registry.ts +++ b/packages/spec/src/migrations/registry.ts @@ -542,6 +542,21 @@ const step17: MigrationStep = { + '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.\n\n' + + 'The datasource close-out also graduates the four legacy `datasource.config` spellings the ' + + 'shared driver factory still tolerated via undeclared read-side `??` fallbacks (#4456, the ' + + '#4410 follow-up): sqlite `file`/`database` (use `filename`), postgres/mysql ' + + '`connectionString` (use `url`) and `user` (use `username`), and mongo `uri` (use `url`) ' + + 'and `user` (use `username`). #4410 made the authoring gate reject each with a rename ' + + 'hint, but a runtime datasource persisted in `sys_metadata` before the gate kept working ' + + 'only because the factory read leniently — and deleting that tolerance without a ' + + 'conversion would have silently moved data (a stored sqlite `file:` row falls back to ' + + '`:memory:`). The `datasource-config-driver-key-aliases` conversion rewrites the stored ' + + 'shape to the canonical keys at every rehydration seam, the factory now reads exactly one ' + + 'spelling per key, and the four `??` chains are deleted. Driver-aware by construction: ' + + '`database` renames only under sqlite, where it aliased the file path — for every other ' + + 'driver it is a canonical key and is untouched. Retired from the load path not for lying ' + + 'but because the authoring gate already rejects the spellings loudly; the chain and the ' + + 'stored-row replay are the seams that accept them.\n\n' + 'The `script` flow node converges on its one real path (#4343). It had four ways to name ' + 'what it ran and only one of them ran anything: `config.actionType: \'email\' | \'slack\'` ' + 'were logger-backed stubs that wrote a line, reported success and delivered nothing under ' @@ -590,6 +605,7 @@ const step17: MigrationStep = { 'datasource-inert-blocks-removed', 'flow-node-wait-timeout-keys-removed', 'datasource-read-replicas-removed', + 'datasource-config-driver-key-aliases', 'flow-node-script-branch-keys-removed', ], semantic: [