diff --git a/.changeset/driver-memory-persistence-strict.md b/.changeset/driver-memory-persistence-strict.md new file mode 100644 index 0000000000..05b8f5f9dc --- /dev/null +++ b/.changeset/driver-memory-persistence-strict.md @@ -0,0 +1,23 @@ +--- +'@objectstack/spec': minor +--- + +Close the memory driver's `persistence` sub-shapes against unknown keys (#4001 batch B) + +zod's default is `.strip`: a key a schema does not declare is silently +discarded and the parse still succeeds. `datasource.config` for a `memory` +driver has been parsed since #4410, but `.strict()` does not recurse — the +top-level `MemoryConfigSchema` was closed then, while the five variant shapes +nested under its `persistence` union stayed open, so a typo written *inside* +`persistence` (e.g. `{ type: 'file', filepath: '/data.json' }`) parsed clean +and the driver came up on its persistence defaults with no signal at all. + +`PersistenceAdapterSchema`, `FilePersistenceConfigSchema`, +`LocalStoragePersistenceConfigSchema`, `CustomPersistenceConfigSchema` and +`AutoPersistenceConfigSchema` now raise a named, fixable error — the surface, +the offending key, and (where the schema declares one) an edit-distance "did +you mean" suggestion — instead of dropping the key. + +No field was added or removed; every existing valid payload still parses the +same. Only a config that was already writing an unrecognised key under +`persistence` sees a new, loud rejection in place of the old silent no-op. diff --git a/docs/audits/2026-07-unknown-key-strictness-ledger.counts.md b/docs/audits/2026-07-unknown-key-strictness-ledger.counts.md index 8f4303cbde..afbf077da2 100644 --- a/docs/audits/2026-07-unknown-key-strictness-ledger.counts.md +++ b/docs/audits/2026-07-unknown-key-strictness-ledger.counts.md @@ -22,14 +22,14 @@ regenerate. |---|---| | Triaged directories | 5 | | Object sites in them | 438 | -| Still-open (strip) sites | 180 | -| Files carrying at least one | 27 | +| Still-open (strip) sites | 175 | +| Files carrying at least one | 26 | Remaining strip sites by class: | Bucket | Sites | |---|---| -| authorable — the ruling's forced scope | 40 | +| authorable — the ruling's forced scope | 35 | | unresolved — needs a per-schema verdict | 34 | | wire / open — out of forced scope | 104 | | no door — no carrier, ADR-0049 territory | 1 | @@ -45,11 +45,11 @@ The `strict` column is the one the campaign schedules against; it counts both th | Dir | Sites | strict | passthrough | catchall | strip | |---|---|---|---|---|---| | `ui/` | 161 | 119 | 5 | 0 | 37 | -| `data/` | 165 | 57 | 1 | 0 | 107 | +| `data/` | 165 | 62 | 1 | 0 | 102 | | `automation/` | 65 | 42 | 0 | 0 | 23 | | `security/` | 20 | 7 | 0 | 0 | 13 | | `studio/` | 27 | 27 | 0 | 0 | 0 | -| **total** | **438** | **252** | **6** | **0** | **180** | +| **total** | **438** | **257** | **6** | **0** | **175** | ## File-level triage — site counts @@ -179,7 +179,7 @@ over it is here. ### `data/` — open -**107 strip of 165**, in 16 file(s). +**102 strip of 165**, in 15 file(s). | File | Strip | Sites | |---|---|---| @@ -189,7 +189,6 @@ over it is here. | `driver-nosql.zod.ts` | 10 | 10 | | `driver-sql.zod.ts` | 2 | 2 | | `driver.zod.ts` | 9 | 9 | -| `driver/memory.zod.ts` | 5 | 6 | | `external-catalog.zod.ts` | 4 | 4 | | `external-lookup.zod.ts` | 12 | 12 | | `field-value.zod.ts` | 2 | 3 | @@ -199,11 +198,11 @@ over it is here. | `object.zod.ts` | 1 | 20 | | `query.zod.ts` | 4 | 5 | | `seed-loader.zod.ts` | 12 | 12 | -| **total** | **107** | **165** | +| **total** | **102** | **165** | | Bucket | Sites | |---|---| -| authorable — the ruling's forced scope | 8 | +| authorable — the ruling's forced scope | 3 | | unresolved — needs a per-schema verdict | 34 | | wire / open — out of forced scope | 65 | | no door — no carrier, ADR-0049 territory | 0 | diff --git a/docs/audits/2026-07-unknown-key-strictness-ledger.md b/docs/audits/2026-07-unknown-key-strictness-ledger.md index 36b3b34f48..348ad7ff37 100644 --- a/docs/audits/2026-07-unknown-key-strictness-ledger.md +++ b/docs/audits/2026-07-unknown-key-strictness-ledger.md @@ -1182,7 +1182,6 @@ triage row record which one was taken. | `driver.zod.ts` | wire | **out of scope** — driver capability contract | | `analytics.zod.ts` | mixed (p) | `Metric` / `Dimension` / `Cube` / `AnalyticsQuery` — cube definitions are authored; needs a per-schema read | | `document.zod.ts` | wire (p) | `DocumentTemplate` / `ESignatureConfig` read authorable on their face — the `(p)` is unresolved, verify before scheduling either way | -| `driver/memory.zod.ts` | authorable | The persistence-adapter union under `datasource.config`; `datasource.config` HAS been parsed against these since #4410, so strictness here now binds | | `query.zod.ts` | open | ~~⚠️ classification conflict — see #4721~~ **RESOLVED (11:41Z ruling, closed by #4721).** The conflict was real and the answer was that per-FILE classification was the imprecise instrument: `SortNodeSchema` was carved out as `authorable` and closed (`strictObject` + `aliases: { direction: 'order' }`), the other 4 sites keep `open`. Those 4 are the dialect proper — `BaseQuerySchema`, `AggregationNodeSchema`, `FullTextSearchSchema`, `GroupByNodeSchema`'s object arm — and `BaseQuerySchema`'s own top-level strictness is #4001's to schedule, deliberately **not** taken by #4721 | | `external-catalog.zod.ts` | wire (p) | **out of scope** | | `hook.zod.ts` | wire | **out of scope** — `HookContextSchema` + `.session`/`.provenance`/`.user` are the runtime shape handed to a handler; verified in the data step | @@ -1192,13 +1191,18 @@ triage row record which one was taken. **Authorable strip in `data/`:** [the counts file](./2026-07-unknown-key-strictness-ledger.counts.md#data--open) splits this -directory three ways, and the middle bucket is the one to read: `object`, `driver/memory` -and `field` are **firm** authorable; `external-lookup`, `seed-loader`, `analytics` and -`field-value` are **unresolved** — they still carry `mixed (p)`, so the ledger is saying -"nobody has done the per-schema read" rather than "these are ready". The rest is wire/open -and out of the ruling's forced scope; that count fell by one when #4721 closed -`query.zod.ts`'s `SortNodeSchema`, the one row in this directory where the per-schema read -moved a site OUT of `open` rather than confirming it. +directory three ways, and the middle bucket is the one to read: `object` and `field` are +**firm** authorable; `external-lookup`, `seed-loader`, `analytics` and `field-value` are +**unresolved** — they still carry `mixed (p)`, so the ledger is saying "nobody has done the +per-schema read" rather than "these are ready". The rest is wire/open and out of the +ruling's forced scope; that count fell by one when #4721 closed `query.zod.ts`'s +`SortNodeSchema`, the one row in this directory where the per-schema read moved a site OUT +of `open` rather than confirming it, and by one more when **#4001 batch B** closed +`driver/memory.zod.ts`'s remaining 5 sites (the persistence-adapter union under +`datasource.config` — `PersistenceAdapterSchema`, `FilePersistenceConfigSchema`, +`LocalStoragePersistenceConfigSchema`, `CustomPersistenceConfigSchema`, +`AutoPersistenceConfigSchema`), dropping its row from the remaining-strip map entirely: the +file's 6th site, `MemoryConfigSchema`, was already `strictObject` since #4410. **批 20 closed 13 of `object.zod.ts`'s 14 and parked the row at 1**, which makes it the fourth row in this ledger to shrink without disappearing — after `flow` (批 11), diff --git a/packages/spec/src/data/driver/memory.test.ts b/packages/spec/src/data/driver/memory.test.ts index 0a6f6c3984..ca80929848 100644 --- a/packages/spec/src/data/driver/memory.test.ts +++ b/packages/spec/src/data/driver/memory.test.ts @@ -295,6 +295,23 @@ describe('FilePersistenceConfigSchema', () => { expect(config.type).toBe('file'); expect(config.path).toBeUndefined(); }); + + // #4001 batch B: this shape was a bare `z.object` — an unrecognised key was + // silently stripped and the file adapter came up on its defaults with no + // signal at all. `.strict()` makes that loud. + it('rejects an unrecognised key instead of silently stripping it', () => { + const result = FilePersistenceConfigSchema.safeParse({ + type: 'file', + path: '/data/store.json', + filepath: '/data/other.json', // typo'd key, not a real field + }); + + expect(result.success).toBe(false); + expect(result.error!.issues[0]!.message).toContain( + "this memory datasource's file persistence config", + ); + expect(result.error!.issues[0]!.message).toContain('filepath'); + }); }); describe('LocalStoragePersistenceConfigSchema', () => { @@ -316,6 +333,20 @@ describe('LocalStoragePersistenceConfigSchema', () => { expect(config.type).toBe('local'); expect(config.key).toBeUndefined(); }); + + // #4001 batch B — see the FilePersistenceConfigSchema case above. + it('rejects an unrecognised key instead of silently stripping it', () => { + const result = LocalStoragePersistenceConfigSchema.safeParse({ + type: 'local', + storageKey: 'myapp:db', // typo'd key, not a real field + }); + + expect(result.success).toBe(false); + expect(result.error!.issues[0]!.message).toContain( + "this memory datasource's localStorage persistence config", + ); + expect(result.error!.issues[0]!.message).toContain('storageKey'); + }); }); describe('CustomPersistenceConfigSchema', () => { @@ -332,6 +363,58 @@ describe('CustomPersistenceConfigSchema', () => { expect(typeof config.adapter.save).toBe('function'); expect(typeof config.adapter.flush).toBe('function'); }); + + // #4001 batch B — see the FilePersistenceConfigSchema case above. + it('rejects an unrecognised key instead of silently stripping it', () => { + const result = CustomPersistenceConfigSchema.safeParse({ + adapter: { + load: async () => null, + save: async () => {}, + flush: async () => {}, + }, + options: { retries: 3 }, // not a real field on this shape + }); + + expect(result.success).toBe(false); + expect(result.error!.issues[0]!.message).toContain( + "this memory datasource's custom-adapter persistence config", + ); + expect(result.error!.issues[0]!.message).toContain('options'); + }); +}); + +describe('PersistenceAdapterSchema', () => { + it('should accept a valid adapter (load/save/flush)', () => { + const config = PersistenceAdapterSchema.parse({ + load: async () => null, + save: async () => {}, + flush: async () => {}, + }); + + expect(typeof config.load).toBe('function'); + expect(typeof config.save).toBe('function'); + expect(typeof config.flush).toBe('function'); + }); + + // #4001 batch B: this shape was a bare `z.object` — an unrecognised key + // (e.g. a typo'd lifecycle method) was silently stripped instead of being + // reported, so a custom adapter missing `flush` because the author wrote + // `close` instead got a clean parse and a driver that never persisted on + // shutdown. + it('rejects an unrecognised key instead of silently stripping it', () => { + const result = PersistenceAdapterSchema.safeParse({ + load: async () => null, + save: async () => {}, + flush: async () => {}, + close: async () => {}, // not a real field on this shape + }); + + expect(result.success).toBe(false); + expect(result.error!.issues[0]!.message).toContain( + "this memory datasource's custom persistence adapter", + ); + expect(result.error!.issues[0]!.message).toContain('close'); + }); }); describe('AutoPersistenceConfigSchema', () => { @@ -366,6 +449,20 @@ describe('AutoPersistenceConfigSchema', () => { autoSaveInterval: 50, // Below minimum of 100 })).toThrow(); }); + + // #4001 batch B — see the FilePersistenceConfigSchema case above. + it('rejects an unrecognised key instead of silently stripping it', () => { + const result = AutoPersistenceConfigSchema.safeParse({ + type: 'auto', + interval: 5000, // meant `autoSaveInterval`, not a real field + }); + + expect(result.success).toBe(false); + expect(result.error!.issues[0]!.message).toContain( + "this memory datasource's auto-detect persistence config", + ); + expect(result.error!.issues[0]!.message).toContain('interval'); + }); }); describe('MemoryPersistenceConfigSchema', () => { diff --git a/packages/spec/src/data/driver/memory.zod.ts b/packages/spec/src/data/driver/memory.zod.ts index b50600a8a9..5436a45d8b 100644 --- a/packages/spec/src/data/driver/memory.zod.ts +++ b/packages/spec/src/data/driver/memory.zod.ts @@ -15,7 +15,7 @@ import { * * Defines the configuration options for the in-memory driver. * Reference: objectql/packages/drivers/memory (Mingo-powered production-ready driver) - * + * * The memory driver is ideal for: * - Unit testing (no database setup required) * - Development & prototyping @@ -29,6 +29,27 @@ import { // 1. Persistence Configuration // ========================================================================== +/** + * Shared history line for the five `persistence` sub-shapes below (#4001 batch B). + * + * Until #4410 `datasource.config` was parsed by nothing at all, so an + * unrecognised key anywhere under `persistence` — including inside one of + * these variant shapes — was accepted in silence and the driver came up on + * its defaults instead. #4410 gave the top-level `MemoryConfigSchema` a + * parse door; `.strict()` does not recurse through it, so each variant + * object nested under the `persistence` union needed its own gate. + * + * ⚠️ Deliberately placed AFTER the module header above, not before it: + * `findModuleDocBlock` (#5059) takes the FIRST top-level doc comment in the + * file as the page description and returns null for the whole file if that + * comment is immediately followed by a declaration — it does not fall back + * to search further. A doc comment on this const, placed ahead of the real + * header, would have silently dropped this file's published description. + */ +const PERSISTENCE_HISTORY = + "Until #4410 nothing parsed `datasource.config` at all, so an unrecognised key here was " + + 'accepted in silence and the requested persistence mode came up on its defaults instead.'; + /** * Persistence adapter interface for custom persistence implementations. * Adapters must implement load/save/flush lifecycle methods. @@ -38,11 +59,17 @@ import { * via `PersistenceAdapterInterface` in the driver implementation. */ import { lazySchema } from '../../shared/lazy-schema'; -export const PersistenceAdapterSchema = lazySchema(() => z.object({ - load: z.function().describe('Load persisted data on startup. Returns Promise | null>'), - save: z.function().describe('Save data to persistent storage. Accepts Record, returns Promise'), - flush: z.function().describe('Flush pending writes and ensure data is persisted. Returns Promise'), -}).describe('Custom persistence adapter interface')); +export const PersistenceAdapterSchema = lazySchema(() => strictObject( + { + surface: "this memory datasource's custom persistence adapter", + history: PERSISTENCE_HISTORY, + }, + { + load: z.function().describe('Load persisted data on startup. Returns Promise | null>'), + save: z.function().describe('Save data to persistent storage. Accepts Record, returns Promise'), + flush: z.function().describe('Flush pending writes and ensure data is persisted. Returns Promise'), + }, +).describe('Custom persistence adapter interface')); export type PersistenceAdapter = z.input; @@ -62,13 +89,19 @@ export type PersistenceType = z.input; * File-system persistence configuration. * Used in Node.js environments to save data to a JSON file. */ -export const FilePersistenceConfigSchema = lazySchema(() => z.object({ - type: z.literal('file'), - /** File path to persist data (JSON format). Defaults to `.objectstack/data/memory-driver.json`. */ - path: z.string().optional().describe('File path to persist data'), - /** Auto-save interval in milliseconds. Default: 2000ms. */ - autoSaveInterval: z.number().min(100).default(2000).describe('Auto-save interval in ms'), -}).describe('File-system persistence configuration')); +export const FilePersistenceConfigSchema = lazySchema(() => strictObject( + { + surface: "this memory datasource's file persistence config", + history: PERSISTENCE_HISTORY, + }, + { + type: z.literal('file'), + /** File path to persist data (JSON format). Defaults to `.objectstack/data/memory-driver.json`. */ + path: z.string().optional().describe('File path to persist data'), + /** Auto-save interval in milliseconds. Default: 2000ms. */ + autoSaveInterval: z.number().min(100).default(2000).describe('Auto-save interval in ms'), + }, +).describe('File-system persistence configuration')); export type FilePersistenceConfig = z.input; /** Post-parse shape of {@link FilePersistenceConfig} — defaults applied, transforms run (ADR-0122). */ @@ -78,11 +111,17 @@ export type FilePersistenceConfigParsed = z.infer z.object({ - type: z.literal('local'), - /** localStorage key. Defaults to `objectstack:memory-db`. */ - key: z.string().optional().describe('localStorage key for persisted data'), -}).describe('localStorage persistence configuration')); +export const LocalStoragePersistenceConfigSchema = lazySchema(() => strictObject( + { + surface: "this memory datasource's localStorage persistence config", + history: PERSISTENCE_HISTORY, + }, + { + type: z.literal('local'), + /** localStorage key. Defaults to `objectstack:memory-db`. */ + key: z.string().optional().describe('localStorage key for persisted data'), + }, +).describe('localStorage persistence configuration')); export type LocalStoragePersistenceConfig = z.input; @@ -90,9 +129,15 @@ export type LocalStoragePersistenceConfig = z.input z.object({ - adapter: PersistenceAdapterSchema, -}).describe('Custom adapter persistence configuration')); +export const CustomPersistenceConfigSchema = lazySchema(() => strictObject( + { + surface: "this memory datasource's custom-adapter persistence config", + history: PERSISTENCE_HISTORY, + }, + { + adapter: PersistenceAdapterSchema, + }, +).describe('Custom adapter persistence configuration')); export type CustomPersistenceConfig = z.input; @@ -112,15 +157,21 @@ export type CustomPersistenceConfig = z.input z.object({ - type: z.literal('auto'), - /** File path override when running in Node.js. */ - path: z.string().optional().describe('File path override for Node.js environments'), - /** Auto-save interval override when running in Node.js. */ - autoSaveInterval: z.number().min(100).optional().describe('Auto-save interval override for Node.js environments'), - /** localStorage key override when running in a browser. */ - key: z.string().optional().describe('localStorage key override for browser environments'), -}).describe('Auto-detect persistence configuration')); +export const AutoPersistenceConfigSchema = lazySchema(() => strictObject( + { + surface: "this memory datasource's auto-detect persistence config", + history: PERSISTENCE_HISTORY, + }, + { + type: z.literal('auto'), + /** File path override when running in Node.js. */ + path: z.string().optional().describe('File path override for Node.js environments'), + /** Auto-save interval override when running in Node.js. */ + autoSaveInterval: z.number().min(100).optional().describe('Auto-save interval override for Node.js environments'), + /** localStorage key override when running in a browser. */ + key: z.string().optional().describe('localStorage key override for browser environments'), + }, +).describe('Auto-detect persistence configuration')); export type AutoPersistenceConfig = z.input; diff --git a/packages/spec/test-typecheck-debt.json b/packages/spec/test-typecheck-debt.json index 770655de97..9593689031 100644 --- a/packages/spec/test-typecheck-debt.json +++ b/packages/spec/test-typecheck-debt.json @@ -31,7 +31,6 @@ "src/data/driver-nosql.test.ts": 2, "src/data/driver-sql.test.ts": 1, "src/data/driver.test.ts": 11, - "src/data/driver/memory.test.ts": 1, "src/data/field.test.ts": 2, "src/data/object-strictness-batch20.test.ts": 1, "src/data/query.test.ts": 25,