From e868328717162497b7259ef78b192654d14cecdd Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 11 Aug 2026 04:17:53 +0000 Subject: [PATCH] fix(metadata-admin): DatasourcePreview reads only keys DatasourceSchema accepts (#4131) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `retryPolicy`, `healthCheck` and `capabilities` were removed from the datasource document by objectstack#4583 under ADR-0049 enforce-or-remove: connection retry and health probing belong to the runtime driver, and pushdown is decided by that driver's own `supports.*`, never by datasource metadata. `DatasourceSchema` is `.strict()`, so it refuses all three by name — while this preview kept painting a `Retry Policy` SideBlock, a `Health Check` SideBlock and a `Capabilities` chip strip for them. An author who typed any of the three saw the designer confirm a draft that cannot be saved, and the preview was the only surface acknowledging the keys at all, so it was also the strongest signal they worked. Deleted: both SideBlocks, the chip strip, `enabledCapabilities()`, the reads at `d.retryPolicy` / `d.healthCheck` / `d.capabilities`, and the now unused `RotateCcw` import. `pool` and `ssl` are still declared and still render. This is the third wave of the same defect on this one file (#3275 deleted `d.type`, `isDefault` and the `Array.isArray(capabilities)` branch; #3143 deleted the read-replica pill), and the removals sat in `main` for eight days because nothing compared the two halves of the contract. They are compared mechanically now: `DatasourcePreview.spec-keys.test.ts` derives the preview's read set from the component's own AST — every key reached off the `draft` prop, through dot access, optional chain, bracket or destructure, following a renamed prop and alias chains — and derives the accepted set from the schema object's `.keyof()`. Neither side is written down as a list, so a key added or removed in `@objectstack/spec` moves the pin on the next dependency bump instead of leaving it stale. The pin also asserts things about itself, because a derivation that silently returned nothing would satisfy a subset check forever: that it still finds this file's real reads, and that every use of the draft binding is a read form it understands, so it fails loudly rather than going blind if the preview is refactored. Reverse-verified both ways — restoring the pre-fix file turns it red naming all three keys at their exact lines, and a freshly planted `d.readReplicas` read turns it red naming that one. The AST is load-bearing rather than a grep: the header comment now names all three removed keys in prose, so a text scan would report violations that do not exist. Fixes #4131 Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_017Qqyix2QcnpUC9XeYVDzx3 --- ...tasource-preview-strict-key-groups-4131.md | 24 ++ .../DatasourcePreview.spec-keys.test.ts | 379 ++++++++++++++++++ .../previews/DatasourcePreview.test.tsx | 100 +++-- .../previews/DatasourcePreview.tsx | 81 ++-- 4 files changed, 495 insertions(+), 89 deletions(-) create mode 100644 .changeset/datasource-preview-strict-key-groups-4131.md create mode 100644 packages/app-shell/src/views/metadata-admin/previews/DatasourcePreview.spec-keys.test.ts diff --git a/.changeset/datasource-preview-strict-key-groups-4131.md b/.changeset/datasource-preview-strict-key-groups-4131.md new file mode 100644 index 0000000000..0e787b329a --- /dev/null +++ b/.changeset/datasource-preview-strict-key-groups-4131.md @@ -0,0 +1,24 @@ +--- +'@object-ui/app-shell': patch +--- + +`DatasourcePreview` no longer renders three key groups `DatasourceSchema` rejects +(objectui#4131). `retryPolicy`, `healthCheck` and `capabilities` were removed from the +datasource document by objectstack#4583 under ADR-0049 enforce-or-remove — connection +retry and health probing belong to the runtime driver, and pushdown is decided by that +driver's own `supports.*`, never by datasource metadata. The schema is `.strict()`, so it +refuses all three by name, while the preview kept painting a `Retry Policy` SideBlock, a +`Health Check` SideBlock and a `Capabilities` chip strip for them. An author who typed any +of the three saw the designer confirm a draft that cannot be saved — the preview was the +only surface acknowledging the keys at all, so it was also the strongest signal they +worked. `pool` and `ssl` are still declared and still render. + +This is the third wave of the same defect on this one file (objectui#3275 deleted +`d.type` / `isDefault` / the `Array.isArray(capabilities)` branch; objectui#3143 deleted +the read-replica pill), and the removals had been in `main` eight days before anyone +noticed — nothing compared the two halves of the contract. They are now compared +mechanically: `DatasourcePreview.spec-keys.test.ts` derives the keys the preview reads off +the draft from the component's own AST, derives the keys the schema accepts from the +schema object's `.keyof()`, and fails on any read the schema would reject. Neither side is +written down as a list, so a key added or removed in `@objectstack/spec` moves the pin on +the next dependency bump instead of leaving it stale. diff --git a/packages/app-shell/src/views/metadata-admin/previews/DatasourcePreview.spec-keys.test.ts b/packages/app-shell/src/views/metadata-admin/previews/DatasourcePreview.spec-keys.test.ts new file mode 100644 index 0000000000..a77d4351ad --- /dev/null +++ b/packages/app-shell/src/views/metadata-admin/previews/DatasourcePreview.spec-keys.test.ts @@ -0,0 +1,379 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * `DatasourcePreview` reads no key `DatasourceSchema` rejects (objectui#4131). + * + * This is the third time one file has drifted the same way. objectui#3275 + * deleted three reads (`d.type`, `isDefault ?? default`, the `Array.isArray` + * capabilities branch); objectui#3143 deleted the read-replica pill; and + * objectui#4131 deleted the `Retry Policy` / `Health Check` SideBlocks and the + * `Capabilities` chip strip after objectstack#4583 removed all three key groups + * from a `.strict()` `DatasourceSchema`. That last one sat in `main` for eight + * days: the schema moved in the framework repo, and nothing here noticed. + * + * Nothing here could notice, because the two halves of the contract were only + * ever compared by a human reading both. So this file compares them + * mechanically, and it does so by DERIVING both sides — neither is written down + * as a list that can go stale next to the thing it describes: + * + * • the READ set comes from `DatasourcePreview.tsx`'s own AST — every key + * reached off the `draft` prop, however it is spelled (`d.pool`, `d?.pool`, + * `d['pool']`, `const { pool } = d`); + * • the ACCEPTED set comes from the schema object itself (`.keyof()`), so a + * key added or removed in `@objectstack/spec` moves this test with it on the + * next dependency bump. + * + * The AST matters rather than a grep: this preview's header comment names + * `d.retryPolicy`, `d.healthCheck`, `d.capabilities` and `d.type` in prose, to + * record why they are gone. A text scan of the file reports four violations + * that do not exist; the compiler sees a comment. + * + * Two things the pin deliberately also asserts about ITSELF, because a + * derivation that quietly returns nothing would pass the subset check forever: + * that it still finds this file's known reads (`live file, non-vacuity`), and + * that every mention of the draft binding is a read form it understands, so it + * fails loudly instead of going blind if the preview is refactored into a shape + * it cannot follow. + * + * Scope note: this is a TOP-LEVEL key pin, matching what `.strict()` rejects by + * name on the datasource document. It says nothing about nested value shapes. + */ + +import { describe, it, expect } from 'vitest'; +import ts from 'typescript'; +import { readFileSync } from 'node:fs'; +import { fileURLToPath } from 'node:url'; + +import { DatasourceSchema } from '@objectstack/spec/data'; + +const PREVIEW_PATH = fileURLToPath(new URL('./DatasourcePreview.tsx', import.meta.url)); +const PREVIEW_SOURCE = readFileSync(PREVIEW_PATH, 'utf8'); + +/** A draft key the preview reads, with the 1-based line it was read on. */ +interface DraftRead { + key: string; + line: number; +} + +interface Derivation { + /** Local names that hold the draft object (`draft`, and aliases of it). */ + bindings: Set; + reads: DraftRead[]; + /** + * Uses of a draft binding this walker could NOT resolve to a key read — + * e.g. the whole object handed to a helper. Non-empty means the derivation + * has a blind spot and the subset check below is no longer complete. + */ + opaqueUses: { text: string; line: number }[]; +} + +/** Peel `(x)`, `x!`, `x as T`, `x satisfies T` down to the value underneath. */ +function unwrap(node: ts.Expression): ts.Expression { + let cur = node; + for (;;) { + if (ts.isParenthesizedExpression(cur) || ts.isNonNullExpression(cur)) cur = cur.expression; + else if (ts.isAsExpression(cur) || ts.isSatisfiesExpression(cur)) cur = cur.expression; + else return cur; + } +} + +function propertyNameText(name: ts.PropertyName | undefined): string | undefined { + if (!name) return undefined; + if (ts.isIdentifier(name) || ts.isStringLiteral(name)) return name.text; + return undefined; +} + +/** + * The draft keys a preview module reads, derived from its source. + * + * Seeded from the component's `draft` prop (by name, so a destructuring rename + * is followed too), extended across simple aliases (`const d = draft as …`), + * then every property/element access and destructure off one of those bindings + * is collected. + */ +function deriveDraftReads(source: string, fileName = 'DatasourcePreview.tsx'): Derivation { + const sf = ts.createSourceFile(fileName, source, ts.ScriptTarget.Latest, true, ts.ScriptKind.TSX); + const lineOf = (node: ts.Node): number => + sf.getLineAndCharacterOfPosition(node.getStart(sf)).line + 1; + + const bindings = new Set(); + + // Seed: the `draft` prop, however the component destructures it. + const seed = (node: ts.Node): void => { + if (ts.isParameter(node)) { + if (ts.isIdentifier(node.name) && node.name.text === 'draft') bindings.add('draft'); + else if (ts.isObjectBindingPattern(node.name)) { + for (const el of node.name.elements) { + const from = propertyNameText(el.propertyName) ?? propertyNameText(el.name as ts.PropertyName); + if (from === 'draft' && ts.isIdentifier(el.name)) bindings.add(el.name.text); + } + } + } + ts.forEachChild(node, seed); + }; + seed(sf); + + // Aliases: `const d = draft as Record` — to a fixpoint, so a + // chain of aliases is followed rather than only the first hop. + const aliasDecls = new Set(); + for (let pass = 0; pass < 8; pass++) { + const before = bindings.size; + const visit = (node: ts.Node): void => { + if (ts.isVariableDeclaration(node) && node.initializer && ts.isIdentifier(node.name)) { + const init = unwrap(node.initializer); + if (ts.isIdentifier(init) && bindings.has(init.text)) { + bindings.add(node.name.text); + aliasDecls.add(init); + } + } + ts.forEachChild(node, visit); + }; + visit(sf); + if (bindings.size === before) break; + } + + const reads: DraftRead[] = []; + const accounted = new Set(aliasDecls); + + const collect = (node: ts.Node): void => { + // `d.pool`, `d?.pool` + if (ts.isPropertyAccessExpression(node)) { + const target = unwrap(node.expression); + if (ts.isIdentifier(target) && bindings.has(target.text)) { + accounted.add(target); + if (ts.isIdentifier(node.name)) reads.push({ key: node.name.text, line: lineOf(node) }); + } + } + // `d['pool']` + if (ts.isElementAccessExpression(node)) { + const target = unwrap(node.expression); + if (ts.isIdentifier(target) && bindings.has(target.text)) { + accounted.add(target); + const arg = unwrap(node.argumentExpression); + if (ts.isStringLiteral(arg) || ts.isNoSubstitutionTemplateLiteral(arg)) { + reads.push({ key: arg.text, line: lineOf(node) }); + } else { + // A computed key cannot be derived — record it as a blind spot + // rather than silently dropping it. + reads.push({ key: ``, line: lineOf(node) }); + } + } + } + // `const { pool, ssl: tls } = d` + if (ts.isVariableDeclaration(node) && node.initializer && ts.isObjectBindingPattern(node.name)) { + const init = unwrap(node.initializer); + if (ts.isIdentifier(init) && bindings.has(init.text)) { + accounted.add(init); + for (const el of node.name.elements) { + const key = propertyNameText(el.propertyName) ?? propertyNameText(el.name as ts.PropertyName); + if (key) reads.push({ key, line: lineOf(el) }); + } + } + } + ts.forEachChild(node, collect); + }; + collect(sf); + + // Anything left over is a use of the draft this walker cannot see through. + const opaqueUses: { text: string; line: number }[] = []; + const audit = (node: ts.Node): void => { + if (ts.isIdentifier(node) && bindings.has(node.text) && !accounted.has(node)) { + const p = node.parent; + const isDeclarationName = + (ts.isVariableDeclaration(p) || ts.isParameter(p) || ts.isBindingElement(p)) && p.name === node; + const isPropertyName = ts.isPropertyAccessExpression(p) && p.name === node; + const isBindingPropertyName = ts.isBindingElement(p) && p.propertyName === node; + if (!isDeclarationName && !isPropertyName && !isBindingPropertyName) { + opaqueUses.push({ text: `${node.text} (in ${ts.SyntaxKind[p.kind]})`, line: lineOf(node) }); + } + } + ts.forEachChild(node, audit); + }; + audit(sf); + + return { bindings, reads, opaqueUses }; +} + +/** Every top-level key `DatasourceSchema` declares, from the schema itself. */ +function acceptedKeys(): Set { + const keyof = (DatasourceSchema as { keyof?: () => { options: string[] } }).keyof; + if (typeof keyof === 'function') return new Set(DatasourceSchema.keyof().options); + return new Set(Object.keys(DatasourceSchema.shape)); +} + +/** A datasource draft that parses clean — the control for the strictness probe. */ +const DECLARED_ONLY_DRAFT = { + name: 'warehouse', + label: 'Analytics Warehouse', + description: 'Read-only Postgres replica for reporting.', + driver: 'postgres', + active: true, + ssl: { enabled: true, rejectUnauthorized: true }, + config: { host: 'db.internal', port: 5432, database: 'analytics' }, + pool: { min: 2, max: 10 }, +} satisfies Record; + +function rejectedKeysIn(draft: Record): string[] { + const parsed = DatasourceSchema.safeParse(draft); + if (parsed.success) return []; + return parsed.error.issues.flatMap((issue) => + issue.code === 'unrecognized_keys' ? ((issue as { keys: string[] }).keys ?? []) : [], + ); +} + +describe('DatasourceSchema is the strict contract this pin leans on', () => { + it('accepts a draft of declared keys only', () => { + const parsed = DatasourceSchema.safeParse(DECLARED_ONLY_DRAFT); + expect(parsed.success ? [] : parsed.error.issues).toEqual([]); + }); + + it('rejects an undeclared key BY NAME — the pin is meaningless without this', () => { + // Scanner control, in the shape objectui#4131's triage used: the accepted + // draft above is the same object plus one key. If strictness were dropped + // upstream, this goes red and the subset check below stops meaning + // anything — better to learn it here than from a user's unsaveable draft. + expect(rejectedKeysIn({ ...DECLARED_ONLY_DRAFT, notADatasourceKey: 1 })).toContain( + 'notADatasourceKey', + ); + }); + + it('rejects the three key groups objectstack#4583 removed', () => { + expect( + rejectedKeysIn({ + ...DECLARED_ONLY_DRAFT, + retryPolicy: { maxAttempts: 3 }, + healthCheck: { enabled: true, intervalMs: 60000 }, + capabilities: { readOnly: true }, + }).sort(), + ).toEqual(['capabilities', 'healthCheck', 'retryPolicy']); + }); + + it('still declares the two this preview keeps', () => { + const accepted = acceptedKeys(); + expect(accepted.has('pool')).toBe(true); + expect(accepted.has('ssl')).toBe(true); + }); +}); + +describe('DatasourcePreview reads no key DatasourceSchema rejects', () => { + it('every derived read is an accepted key', () => { + const accepted = acceptedKeys(); + const { reads } = deriveDraftReads(PREVIEW_SOURCE); + const offenders = reads.filter((r) => !accepted.has(r.key)); + expect( + offenders.map((r) => `${r.key} (DatasourcePreview.tsx:${r.line})`), + `DatasourcePreview reads ${offenders.length} key(s) DatasourceSchema rejects. ` + + `A preview that paints a rejected key makes an unsaveable draft look correct ` + + `(AGENTS.md #0.1) — delete the read, do not widen the schema to match it. ` + + `Accepted keys: ${[...accepted].sort().join(', ')}`, + ).toEqual([]); + }); + + it('live file, non-vacuity: the derivation sees the reads that are really there', () => { + // Without this, a walker that returned an empty array would satisfy the + // subset check above forever. + const { bindings, reads } = deriveDraftReads(PREVIEW_SOURCE); + const keys = new Set(reads.map((r) => r.key)); + expect(bindings.has('draft')).toBe(true); + expect([...keys].sort()).toEqual( + expect.arrayContaining(['active', 'config', 'driver', 'label', 'name', 'pool', 'ssl']), + ); + expect(keys.size).toBeGreaterThanOrEqual(8); + }); + + it('live file: the derivation has no blind spot it is not reporting', () => { + const { opaqueUses } = deriveDraftReads(PREVIEW_SOURCE); + expect( + opaqueUses.map((u) => `${u.text} at line ${u.line}`), + 'The draft object is used in a way this derivation cannot follow, so the ' + + 'subset check above is no longer complete. Either express the read as a ' + + 'property access / destructure, or teach deriveDraftReads() the new form.', + ).toEqual([]); + }); + + it('the keys objectstack#4583 removed are named in prose only, never read', () => { + // The header comment records all three by name; the compiler sees comments, + // a grep does not. This is the assertion objectui#4131 exists to make. + const { reads } = deriveDraftReads(PREVIEW_SOURCE); + const keys = new Set(reads.map((r) => r.key)); + for (const gone of ['retryPolicy', 'healthCheck', 'capabilities']) { + expect(keys.has(gone), `${gone} is read again — see objectui#4131`).toBe(false); + } + // …and the #3275 wave stays deleted too. + for (const gone of ['type', 'isDefault', 'default', 'readReplicas']) { + expect(keys.has(gone), `${gone} is read again — see objectui#3275 / #3143`).toBe(false); + } + }); +}); + +describe('the derivation itself is honest', () => { + /** A preview-shaped module carrying a planted read of each rejected key. */ + const PLANTED = ` + export function P({ name, draft }: MetadataPreviewProps) { + const d = draft as Record; + const pool = d.pool; + const retry = d.retryPolicy as Record | undefined; + const health = d?.healthCheck; + const caps = d['capabilities']; + const { autoConnect } = d; + return
{String(pool ?? retry ?? health ?? caps ?? autoConnect ?? name)}
; + } + `; + + it('a planted read of a rejected key turns the pin RED, naming the key', () => { + const accepted = acceptedKeys(); + const { reads } = deriveDraftReads(PLANTED, 'planted.tsx'); + const offenders = reads.filter((r) => !accepted.has(r.key)).map((r) => r.key); + expect(offenders.sort()).toEqual(['capabilities', 'healthCheck', 'retryPolicy']); + // The declared keys in the same fixture are NOT flagged — the guard + // separates the two, it does not just fail on everything. + expect(reads.map((r) => r.key)).toEqual( + expect.arrayContaining(['pool', 'autoConnect']), + ); + }); + + it('reads through every spelling: dot, optional chain, bracket, destructure', () => { + const keys = deriveDraftReads(PLANTED, 'planted.tsx').reads.map((r) => r.key); + expect(keys).toContain('retryPolicy'); // d.retryPolicy + expect(keys).toContain('healthCheck'); // d?.healthCheck + expect(keys).toContain('capabilities'); // d['capabilities'] + expect(keys).toContain('autoConnect'); // const { autoConnect } = d + }); + + it('follows a renamed prop and an alias chain', () => { + const renamed = ` + export function P({ draft: raw }: MetadataPreviewProps) { + const mid = raw; + const d = mid as Record; + return
{String(d.retryPolicy)}
; + } + `; + const { bindings, reads } = deriveDraftReads(renamed, 'renamed.tsx'); + expect([...bindings].sort()).toEqual(['d', 'mid', 'raw']); + expect(reads.map((r) => r.key)).toEqual(['retryPolicy']); + }); + + it('a rejected key named in a comment or a string is not a read', () => { + const prose = ` + /** Deleted in #4131: d.retryPolicy, d.healthCheck, d.capabilities. */ + export function P({ draft }: MetadataPreviewProps) { + const d = draft as Record; + // d.capabilities was a chip strip here; see also d.type. + const label = 'd.healthCheck'; + return
{String(d.pool) + label}
; + } + `; + expect(deriveDraftReads(prose, 'prose.tsx').reads.map((r) => r.key)).toEqual(['pool']); + }); + + it('reports an unfollowable use instead of silently missing it', () => { + const opaque = ` + export function P({ draft }: MetadataPreviewProps) { + const d = draft as Record; + return
{JSON.stringify(Object.entries(d))}
; + } + `; + const { opaqueUses } = deriveDraftReads(opaque, 'opaque.tsx'); + expect(opaqueUses.length).toBeGreaterThan(0); + }); +}); diff --git a/packages/app-shell/src/views/metadata-admin/previews/DatasourcePreview.test.tsx b/packages/app-shell/src/views/metadata-admin/previews/DatasourcePreview.test.tsx index b4e0367774..bce6f2947f 100644 --- a/packages/app-shell/src/views/metadata-admin/previews/DatasourcePreview.test.tsx +++ b/packages/app-shell/src/views/metadata-admin/previews/DatasourcePreview.test.tsx @@ -1,28 +1,39 @@ // Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. /** - * DatasourcePreview vs `DatasourceSchema` (objectui#3275). + * DatasourcePreview vs `DatasourceSchema` (objectui#3275, objectui#4131). * * `DatasourceSchema` is `.strict()`. Three reads in this preview were for keys * it rejects outright, and each one made an unsaveable draft look correct: * - * • `capabilities` — a `DatasourceCapabilities` OBJECT of boolean flags. The - * preview tested `Array.isArray(d.capabilities)`, which is exactly - * backwards: it lit the CAPABILITIES block up only for the pre-17 token - * array the schema refuses, and left it dark for the object form the schema - * requires. objectui#3266 watched the block vanish the moment the sample was - * corrected. + * • `capabilities` — the preview tested `Array.isArray(d.capabilities)`, which + * was exactly backwards: it lit the CAPABILITIES block up only for the pre-17 + * token array the schema refuses, and left it dark for the object form the + * schema then required. objectui#3266 watched the block vanish the moment the + * sample was corrected. objectstack#4583 has since removed the key outright, + * so the block is gone with it (objectui#4131) — there is no longer a shape + * of `capabilities` a datasource can carry. * • `driver ?? d.type` — `type` is rejected with an explicit `type` → `driver` * alias hint, so the fallback taught the wrong spelling. * • `isDefault` / `default` — not datasource keys at all; routing is declared * at stack level via `datasourceMapping`. * + * objectui#4131 deleted a second wave on the same grounds: `retryPolicy` and + * `healthCheck` joined `capabilities` in objectstack#4583's removal, so their + * SideBlocks were painting draft state the schema refuses by name. + * * A preview that renders a rejected key is not being helpful; it is making the * author's mistake invisible until publish (AGENTS.md #0.1). + * + * These are RENDER pins — what a reader sees for a given draft. The companion + * `DatasourcePreview.spec-keys.test.ts` pins the READ set against the schema's + * own key list, so a fourth wave fails here by its rendering and there by its + * mere existence. */ import { describe, it, expect, afterEach, vi } from 'vitest'; import { render, screen, cleanup } from '@testing-library/react'; +import { DatasourceSchema } from '@objectstack/spec/data'; // The federation panel makes REST calls against a saved datasource; this test // is about which keys the preview reads, so stub it out. @@ -34,7 +45,11 @@ import { DatasourcePreview } from './DatasourcePreview'; afterEach(cleanup); -/** Parses clean against `ObjectStackSchema` — mirrors the gallery's sample. */ +/** + * Declared keys only — and asserted to parse clean below rather than merely + * claimed to, so this fixture cannot drift into the very state it is here to + * rule out. Mirrors the console gallery's sample. + */ const VALID_DRAFT = { name: 'warehouse', label: 'Analytics Warehouse', @@ -44,8 +59,6 @@ const VALID_DRAFT = { ssl: { enabled: true, rejectUnauthorized: true }, config: { host: 'db.internal', port: 5432, database: 'analytics' }, pool: { min: 2, max: 10 }, - capabilities: { readOnly: true, queryAggregations: true }, - healthCheck: { enabled: true, intervalMs: 60000 }, } satisfies Record; /** Pre-17 shape: array capabilities, `type` instead of `driver`, `isDefault`. */ @@ -58,38 +71,54 @@ const STALE_DRAFT = { capabilities: ['read', 'aggregate'], } satisfies Record; +/** A draft carrying the three key groups objectstack#4583 removed. */ +const REMOVED_KEYS_DRAFT = { + ...VALID_DRAFT, + retryPolicy: { maxAttempts: 3, backoffMs: 250 }, + healthCheck: { enabled: true, intervalMs: 60000 }, + capabilities: { readOnly: true, queryAggregations: true }, +} satisfies Record; + function renderPreview(draft: Record) { return render( , ); } -describe('DatasourcePreview renders object-shaped capabilities', () => { - it('renders a chip for each capability the author set to true', () => { - renderPreview(VALID_DRAFT); - expect(screen.getByText('Capabilities')).toBeTruthy(); - expect(screen.getByText('readOnly')).toBeTruthy(); - expect(screen.getByText('queryAggregations')).toBeTruthy(); +describe('the fixtures mean what they say', () => { + it('VALID_DRAFT carries declared keys only', () => { + const parsed = DatasourceSchema.safeParse(VALID_DRAFT); + expect(parsed.success ? [] : parsed.error.issues).toEqual([]); }); - it('omits flags left false — a default is not an assertion worth a chip', () => { - renderPreview({ - ...VALID_DRAFT, - capabilities: { readOnly: true, joins: false, transactions: false }, - }); - expect(screen.getByText('readOnly')).toBeTruthy(); - expect(screen.queryByText('joins')).toBeNull(); - expect(screen.queryByText('transactions')).toBeNull(); + it('REMOVED_KEYS_DRAFT is rejected, by all three names', () => { + const parsed = DatasourceSchema.safeParse(REMOVED_KEYS_DRAFT); + expect(parsed.success).toBe(false); + const rejected = parsed.success + ? [] + : parsed.error.issues.flatMap((i) => + i.code === 'unrecognized_keys' ? ((i as { keys: string[] }).keys ?? []) : [], + ); + expect(rejected.sort()).toEqual(['capabilities', 'healthCheck', 'retryPolicy']); }); +}); - it('renders no capabilities block when every flag is false', () => { - renderPreview({ ...VALID_DRAFT, capabilities: { readOnly: false } }); +describe('DatasourcePreview renders nothing from keys the spec rejects', () => { + it('paints no Retry Policy / Health Check / Capabilities block (objectui#4131)', () => { + // The whole defect: an author types these, the preview confirms them, and + // the save fails. Nothing they wrote may appear — not the block titles, not + // the values inside them. + renderPreview(REMOVED_KEYS_DRAFT); + expect(screen.queryByText('Retry Policy')).toBeNull(); + expect(screen.queryByText('Health Check')).toBeNull(); expect(screen.queryByText('Capabilities')).toBeNull(); + expect(screen.queryByText('readOnly')).toBeNull(); + expect(screen.queryByText('queryAggregations')).toBeNull(); + expect(screen.queryByText(/maxAttempts/)).toBeNull(); + expect(screen.queryByText(/intervalMs/)).toBeNull(); }); -}); -describe('DatasourcePreview renders nothing from keys the spec rejects', () => { - it('renders no capabilities block for the array form', () => { + it('renders no capabilities block for the array form either', () => { renderPreview(STALE_DRAFT); expect(screen.queryByText('Capabilities')).toBeNull(); expect(screen.queryByText('read')).toBeNull(); @@ -123,10 +152,19 @@ describe('DatasourcePreview still renders everything the spec accepts', () => { expect(screen.getByText('db.internal')).toBeTruthy(); }); - it('keeps the pool / ssl / health-check rail blocks', () => { + it('keeps the pool / ssl rail blocks — the two key groups still declared', () => { renderPreview(VALID_DRAFT); expect(screen.getByText('Pool')).toBeTruthy(); expect(screen.getByText('SSL')).toBeTruthy(); - expect(screen.getByText('Health Check')).toBeTruthy(); + // …and renders their contents, so "present" is not just a title. + expect(screen.getByText('min:')).toBeTruthy(); + expect(screen.getByText('rejectUnauthorized:')).toBeTruthy(); + }); + + it('renders the surviving rail and nothing beside it', () => { + renderPreview(VALID_DRAFT); + expect(screen.queryByText('Retry Policy')).toBeNull(); + expect(screen.queryByText('Health Check')).toBeNull(); + expect(screen.queryByText('Capabilities')).toBeNull(); }); }); diff --git a/packages/app-shell/src/views/metadata-admin/previews/DatasourcePreview.tsx b/packages/app-shell/src/views/metadata-admin/previews/DatasourcePreview.tsx index 68cb5d94be..2f30b86266 100644 --- a/packages/app-shell/src/views/metadata-admin/previews/DatasourcePreview.tsx +++ b/packages/app-shell/src/views/metadata-admin/previews/DatasourcePreview.tsx @@ -13,10 +13,7 @@ * name matches /pass|secret|key|token|credential/i is replaced * with `••••••` and a "redacted" badge. Other primitives render * verbatim; nested objects render as their key count. - * • Pool, SSL, retry, health-check pills derived from optional - * sibling blocks. - * • Capabilities chip strip — the `DatasourceCapabilities` flags set to - * `true`. + * • Pool and SSL pills derived from optional sibling blocks. * * Three reads were deleted in objectui#3275 because `DatasourceSchema` is * `.strict()` and rejects every one of them, so each made an unsaveable @@ -27,6 +24,20 @@ * backwards, lighting up only for the array form the schema refuses and * staying dark for the object form it requires. * + * A second wave went the same way in objectui#4131, for keys the spec + * itself dropped rather than ones this file spelled wrong: the `Retry + * Policy` and `Health Check` SideBlocks, the `Capabilities` chip strip and + * the `enabledCapabilities()` helper behind it, with their reads at + * `d.retryPolicy` / `d.healthCheck` / `d.capabilities`. All three key + * groups were removed from `DatasourceSchema` by objectstack#4583 (the + * 11-flag `capabilities` block, the 4-key `retryPolicy` block, the 3-key + * `healthCheck` block) under ADR-0049 enforce-or-remove — declared, + * strict-guarded and read by nobody: connection retry and health probing + * belong to the runtime driver, and pushdown is decided by that driver's + * own `supports.*`, never by datasource metadata. `.strict()` now rejects + * each of them by name, so every one of those blocks was again painting a + * draft that cannot be saved. `pool` and `ssl` are still declared and stay. + * * A read-replica count pill used to sit in that strip. It is gone with * `datasource.readReplicas` itself (objectstack#4468): nothing in the * platform ever opened a replica connection, so the pill reported a @@ -35,20 +46,19 @@ * that it worked. A preview echoes what was typed; it can never stand in * for a runtime consumer (`packages/spec/liveness/README.md`). * + * Three waves of the same defect on one file is why the read set is now + * PINNED rather than merely corrected: `DatasourcePreview.spec-keys.test.ts` + * derives the keys this file reads off the draft from its own AST and the + * keys `DatasourceSchema` accepts from the schema object, and fails on any + * read the schema would reject. A fourth wave lands as a red test, not as + * another issue eight days later. + * * The preview never attempts a live "test connection" — it runs * inside the editor sandbox and must remain side-effect free. */ import * as React from 'react'; -import { - Activity, - Database, - HardDrive, - Lock, - Power, - RotateCcw, - ShieldCheck, -} from 'lucide-react'; +import { Activity, Database, HardDrive, Lock, Power, ShieldCheck } from 'lucide-react'; import type { MetadataPreviewProps } from '../preview-registry'; import { PreviewShell, PreviewMessage, PreviewErrorBoundary } from './PreviewShell'; import { ExternalDatasourcePanel } from '../external/ExternalDatasourcePanel'; @@ -64,23 +74,6 @@ function redactValue(v: unknown): string { return '••••••'; } -/** - * The capability names an author has switched ON. - * - * `DatasourceCapabilities` is a flat object of optional booleans, each - * defaulting to `false`. Only `true` is an assertion the author made, so - * only `true` earns a chip — listing the falses would bury the two facts - * that matter under nine that don't. A non-object (e.g. the pre-17 string - * array) yields nothing: the block staying empty IS the signal that the - * shape is wrong. - */ -function enabledCapabilities(raw: unknown): string[] { - if (!raw || typeof raw !== 'object' || Array.isArray(raw)) return []; - return Object.entries(raw as Record) - .filter(([, v]) => v === true) - .map(([k]) => k); -} - function renderValue(v: unknown): string { if (v == null) return '∅'; if (typeof v === 'string') return v; @@ -104,16 +97,6 @@ export function DatasourcePreview({ name, draft }: MetadataPreviewProps) { const config = (d.config as Record | undefined) ?? {}; const pool = d.pool as Record | undefined; const ssl = d.ssl as Record | boolean | undefined; - const retryPolicy = d.retryPolicy as Record | undefined; - const healthCheck = d.healthCheck as Record | undefined; - // `capabilities` is a DatasourceCapabilities OBJECT of boolean flags - // (`{ readOnly, queryAggregations, joins, … }`), never a token array. - // Reading it with `Array.isArray` meant a spec-valid draft rendered NO - // capabilities block at all, while the old array form — which the schema - // rejects — was the only thing that lit it up. Show the flags the author - // turned ON; a flag left false is the schema's own default, not a - // statement worth a chip. - const capabilities = enabledCapabilities(d.capabilities); // External Datasource Federation (ADR-0015): a non-'managed' schemaMode // marks this datasource as federated. The panel keys off the *saved* item @@ -214,25 +197,7 @@ export function DatasourcePreview({ name, draft }: MetadataPreviewProps) { icon={ShieldCheck} value={typeof ssl === 'boolean' ? { enabled: ssl } : ssl} /> - - - - {/* Capabilities */} - {capabilities.length > 0 && ( -
-
- {capabilities.map((c) => ( - - {c} - - ))} -
-
- )}