diff --git a/.changeset/olive-pugs-sniff.md b/.changeset/olive-pugs-sniff.md new file mode 100644 index 0000000000..bce67bc41d --- /dev/null +++ b/.changeset/olive-pugs-sniff.md @@ -0,0 +1,19 @@ +--- +'@objectstack/spec': patch +--- + +Fix `snap.grid` on the Studio flow-builder config pointing authors at the wrong key, and gate the cause repo-wide. + +`strictUnknownKeyError` indexes an alias table by `aliasProbe(key)` — lowercased, with `_`, `-` and spaces stripped. Two keys in one table that normalise identically therefore share one index, and the later entry silently overwrites the earlier one. Nothing checked for that. + +`these snap settings` listed `grid: 'gridSize'` and, at the end of the same table, `grid_: 'showGrid'`. Writing `grid: 24` (the grid's pixel pitch) was answered with: + +``` +Did you mean `grid` -> `showGrid`? +``` + +`showGrid` is a boolean, so following that advice was rejected a second time. It now answers `grid` -> `gridSize`, and `gridSize: 24` parses. `visible` still covers the show/hide intent. + +Three other tables carried a second spelling of a key they already had — `rollup`/`rollUp` on a field, `object_name`/`objectName` on a webhook, `strokeDasharray`/`strokeDashArray` on a chart series. Both spellings pointed at the same target there, so the overwrite changed nothing and no author could trip on it; the redundant entries are removed. Because the probe already folds case and separators, the surviving entry accepts every spelling the deleted one did — no authoring input changes meaning. + +`alias-integrity.test.ts` now rejects any alias table containing two keys that share a probe, judged with the real `aliasProbe` rather than a copy of it, so this class cannot come back. diff --git a/packages/spec/authorable-surface.base.json b/packages/spec/authorable-surface.base.json index b5239b7db6..fcfe5cf368 100644 --- a/packages/spec/authorable-surface.base.json +++ b/packages/spec/authorable-surface.base.json @@ -1,6 +1,6 @@ { "description": "In-tree anchor for the authorable-surface deletion gate (#4650, #5235): a verbatim copy of the keys in authorable-surface.json as they stood at `baseRev`, a commit on origin/main. A build that CAN reach origin/main anchors on the merge base instead, and re-verifies this file against `baseRev` — so a PR that edits it to hide a deletion goes red wherever the network exists. A build that CANNOT reach GitHub (image-build stages, air-gapped, fork, historical-tag reproduction) anchors here instead of failing. Written only by `gen:schema`, only from a git-resolved baseline — never from the build that is being checked. See #5235.", - "baseRev": "cdfbee2f08e3316d4606d21dd8e7c2403a59030b", + "baseRev": "168f60f1adf5e4f44ed818eccbba442052722328", "keys": [ "ai/AIModelConfig:maxTokens", "ai/AIModelConfig:model", @@ -2132,6 +2132,7 @@ "automation/DecisionOutputDef:required", "automation/DecisionOutputDef:type", "automation/DeleteRecordConfig:filter", + "automation/DeleteRecordConfig:multi", "automation/DeleteRecordConfig:objectName", "automation/ETLDestination:config", "automation/ETLDestination:connector", @@ -2418,6 +2419,7 @@ "automation/TryCatchConfig:try", "automation/UpdateRecordConfig:fields", "automation/UpdateRecordConfig:filter", + "automation/UpdateRecordConfig:multi", "automation/UpdateRecordConfig:objectName", "automation/WaitExecutorConfig:conditionMaxPolls", "automation/WaitExecutorConfig:conditionPollIntervalMs", diff --git a/packages/spec/src/automation/webhook.zod.ts b/packages/spec/src/automation/webhook.zod.ts index f64df3b14d..15c08f7dc9 100644 --- a/packages/spec/src/automation/webhook.zod.ts +++ b/packages/spec/src/automation/webhook.zod.ts @@ -138,8 +138,9 @@ export const WebhookSchema = lazySchema(() => strictObject({ // `active` (`mapWebhookToRow` in plugin-webhooks does the remap). An admin // or agent reading a row back and re-authoring it from those column names // is the concrete path here, and neither word is within edit distance. + // The column spelling alone covers `objectName` too — `aliasProbe` folds + // case and separators, so both spellings share one probe (#5481). object_name: 'object', - objectName: 'object', active: 'isActive', enabled: 'isActive', // The trigger list, named as the neighbouring surfaces name it. diff --git a/packages/spec/src/data/field.zod.ts b/packages/spec/src/data/field.zod.ts index ce6780e87c..2d758a55f1 100644 --- a/packages/spec/src/data/field.zod.ts +++ b/packages/spec/src/data/field.zod.ts @@ -397,7 +397,10 @@ export const FieldSchema = lazySchema(() => strictObject({ relatedTo: 'reference', referenceTo: 'reference', target: 'reference', targetObject: 'reference', lookupObject: 'reference', onDelete: 'deleteBehavior', deleteRule: 'deleteBehavior', cascade: 'deleteBehavior', formula: 'expression', calculation: 'expression', compute: 'expression', - rollup: 'summaryOperations', rollUp: 'summaryOperations', summary: 'summaryOperations', aggregate: 'summaryOperations', + // `rollup` alone covers `rollUp` / `roll_up` / `Roll-Up` — `aliasProbe` + // folds case and separators, so a second spelling was never reachable + // (#5481). + rollup: 'summaryOperations', summary: 'summaryOperations', aggregate: 'summaryOperations', length: 'maxLength', size: 'maxLength', decimals: 'scale', decimalPlaces: 'scale', digits: 'precision', isReadonly: 'readonly', disabled: 'readonly', diff --git a/packages/spec/src/shared/alias-integrity.test.ts b/packages/spec/src/shared/alias-integrity.test.ts index 19f13ad853..e04339a89a 100644 --- a/packages/spec/src/shared/alias-integrity.test.ts +++ b/packages/spec/src/shared/alias-integrity.test.ts @@ -54,8 +54,24 @@ * - Tables reaching `strictUnknownKeyError` directly, which carry a transcribed * `knownKeys` array instead of a shape — measured clean, pinned shrink-only * below, extension tracked as #5483. - * - Two alias keys in one table colliding under the suggester's own - * `aliasProbe` normalisation, where the later entry silently wins (#5481). + * + * ## The third claim (#5481) + * + * The two claims above are about a table and its *schema*. The third is about a + * table and *itself*: `strictUnknownKeyError` indexes the alias table by + * `aliasProbe(key)`, so two keys in one table that normalise identically + * collapse — **the later one silently wins**. That is not a stylistic + * redundancy. `these snap settings` listed `grid: 'gridSize'` and, at the end + * of the same table, `grid_: 'showGrid'`; the probe strips `_`, so `grid: 24` + * was answered *"Did you mean `grid` → `showGrid`?"* and `showGrid` is a + * boolean — a second rejection, ledger finding 7's exact shape, from a table + * whose author had written the correct mapping one line earlier. + * + * The other three instances on `main` pointed both keys at the same target, so + * the overwrite changed nothing and nobody could trip on them — which is the + * general lesson rather than an exemption: since the probe already folds case + * and separators, a second spelling of one probe is **never** reachable. It is + * dead either way; it is only sometimes also a defect. */ import fs from 'node:fs'; @@ -65,6 +81,7 @@ import { fileURLToPath, pathToFileURL } from 'node:url'; import { describe, it, expect, beforeAll } from 'vitest'; import ts from 'typescript'; +import { aliasProbe } from './alias-probe'; import { acceptsNothing, strictObjectDeclarations, type StrictObjectDeclaration } from './strict-object'; const HERE = path.dirname(fileURLToPath(import.meta.url)); @@ -377,6 +394,33 @@ describe('alias integrity — every table is a true claim about its schema', () expect(broken.sort()).toEqual([]); }); + it('no two alias keys in one table collapse onto the same probe (#5481)', () => { + // The table is indexed by `aliasProbe(key)`, so a colliding pair does not + // produce two entries — it produces one, decided by source order, with the + // earlier key gone before any author can reach it. Judged with the REAL + // probe (imported, not transcribed): a gate carrying its own copy of the + // expression would keep passing if the normalisation ever widened. + const collisions: string[] = []; + for (const s of SURFACES) { + const byProbe = new Map(); + for (const key of Object.keys(s.options.aliases ?? {})) { + byProbe.set(aliasProbe(key), [...(byProbe.get(aliasProbe(key)) ?? []), key]); + } + for (const [probe, keys] of byProbe) { + if (keys.length < 2) continue; + // Name the winner explicitly. When the targets differ this is the whole + // defect (`grid` lost `gridSize` to `showGrid`); when they agree the + // entry is merely dead, and the fix is the same — keep one spelling. + const written = keys.map((k) => `\`${k}\` -> \`${s.options.aliases?.[k]}\``).join(', '); + collisions.push( + `${entry(s, keys[0], s.options.aliases?.[keys[0]] ?? '?')} — ${keys.length} keys share the probe \`${probe}\`: ${written}` + + ` — only \`${s.options.aliases?.[keys[keys.length - 1]]}\` survives`, + ); + } + } + expect(collisions.sort()).toEqual([]); + }); + it('no guidance key is itself a declared key (the same dead entry, other channel)', () => { // `guidance` is consulted from the same `unrecognized_keys` path, so a // prescription filed under a key the shape DECLARES is unreachable in diff --git a/packages/spec/src/shared/alias-probe.ts b/packages/spec/src/shared/alias-probe.ts new file mode 100644 index 0000000000..0d493205fc --- /dev/null +++ b/packages/spec/src/shared/alias-probe.ts @@ -0,0 +1,21 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * The normalisation an alias table's keys are indexed under (#5481). + * + * This lives in its own leaf module for one reason: **two readers must agree.** + * `strictUnknownKeyError` folds every alias key through it to build the lookup + * a rejected key is answered from, and `alias-integrity.test.ts` folds the same + * keys through it to prove no two of them collide. A hand-copied regex in the + * gate would be a second copy of the truth — the exact shape #5013 and #5481 + * were both filed against — and it would fail silently: widen the probe here + * (strip `.`, fold plurals) and a gate carrying the old expression keeps + * passing over collisions that now exist. + * + * It is deliberately **not** re-exported from `shared/index.ts`. Like + * `strict-object.ts`'s `strictObjectDeclarations`, it is an internal seam the + * gate reaches by relative path, not part of the `@objectstack/spec` contract. + */ + +/** `reference_to` / `referenceTo` / `Reference-To` all collapse onto one probe. */ +export const aliasProbe = (key: string): string => key.toLowerCase().replace(/[_\-\s]/g, ''); diff --git a/packages/spec/src/shared/strict-object.test.ts b/packages/spec/src/shared/strict-object.test.ts index 086ce75536..bf2f6bdf77 100644 --- a/packages/spec/src/shared/strict-object.test.ts +++ b/packages/spec/src/shared/strict-object.test.ts @@ -57,6 +57,22 @@ describe('strictObject', () => { expect(r.error!.issues[0].message).toContain('`visibleWhen` → `visible`'); }); + it('ONE alias entry already covers every case/separator spelling of itself (#5481)', () => { + // The fact that makes a second spelling of the same probe not merely + // redundant but unreachable: the table is indexed by `aliasProbe`, which + // folds case, `_`, `-` and spaces. Three tables on `main` carried a second + // spelling (`rollup`/`rollUp`, `object_name`/`objectName`, + // `strokeDasharray`/`strokeDashArray`) that could never have fired — the + // later one simply overwrote the earlier at the same index. Deleting them + // is behaviour-preserving, and this is the assertion that says so. + for (const spelling of ['visible_when', 'VISIBLE-WHEN', 'Visible When', 'visiblewhen']) { + const r = WidgetSchema.safeParse({ name: 'x', [spelling]: true }); + expect(r.success, `${spelling} should be rejected`).toBe(false); + expect(r.error!.issues[0].message, `${spelling} should still reach the alias`) + .toContain(`\`${spelling}\` → \`visible\``); + } + }); + it('still honours a tombstone, and suppresses the rename for it', () => { const r = WidgetSchema.safeParse({ name: 'x', span: 2 }); const msg = r.error!.issues[0].message; diff --git a/packages/spec/src/shared/suggestions.zod.ts b/packages/spec/src/shared/suggestions.zod.ts index 6d48bcf444..740ebbffce 100644 --- a/packages/spec/src/shared/suggestions.zod.ts +++ b/packages/spec/src/shared/suggestions.zod.ts @@ -3,6 +3,8 @@ import type { z } from 'zod'; import { FieldType } from '../data/field.zod'; +import { aliasProbe } from './alias-probe'; + /** * "Did you mean?" Suggestion Utilities * @@ -216,9 +218,6 @@ export function formatSuggestion(suggestions: string[]): string { return `Did you mean one of: ${suggestions.map((s) => `'${s}'`).join(', ')}?`; } -/** `reference_to` / `referenceTo` / `Reference-To` all collapse onto one probe. */ -const aliasProbe = (key: string): string => key.toLowerCase().replace(/[_\-\s]/g, ''); - /** Options for {@link strictUnknownKeyError}. */ export interface StrictUnknownKeyErrorOptions { /** Prose name of the authoring surface the key was written on (e.g. `'this permission set'`). */ @@ -273,6 +272,14 @@ export function strictUnknownKeyError(options: StrictUnknownKeyErrorOptions): z. const { surface, knownKeys, guidance = {}, history } = options; const aliases: Record = {}; for (const [key, canonical] of Object.entries(options.aliases ?? {})) { + // Two keys in ONE table that share a probe collapse here, later silently + // winning — which pointed `snap.grid` at the boolean `showGrid` instead of + // `gridSize` until #5481. Nothing can be recovered at this point (the + // colliding key is already gone), so the defect is caught where it is + // authored: `alias-integrity.test.ts` rejects any table with two keys + // sharing an `aliasProbe`. Since the probe already eats case and + // separators, such a pair is redundant even when both point at the same + // target — one entry always covered both spellings. aliases[aliasProbe(key)] = canonical; } return (issue) => { diff --git a/packages/spec/src/studio/flow-builder.test.ts b/packages/spec/src/studio/flow-builder.test.ts index 5e3564f703..3fb24e7adf 100644 --- a/packages/spec/src/studio/flow-builder.test.ts +++ b/packages/spec/src/studio/flow-builder.test.ts @@ -217,6 +217,28 @@ describe('FlowBuilderConfigSchema', () => { expect(config.nodeDescriptors).toHaveLength(1); expect(config.undoLimit).toBe(100); }); + + it('answers `snap.grid` with `gridSize`, and the advice actually parses (#5481)', () => { + // A `grid_: 'showGrid'` entry used to sit after `grid: 'gridSize'` in this + // table; `aliasProbe` strips `_`, so the two shared one index and the later + // one won. An author writing `grid: 24` — the pixel pitch — was pointed at + // `showGrid`, a boolean, and rejected a second time for taking the advice. + const rejected = FlowBuilderConfigSchema.safeParse({ snap: { grid: 24 } }); + expect(rejected.success).toBe(false); + const message = rejected.error!.issues[0].message; + expect(message).toContain('`grid` → `gridSize`'); + expect(message).not.toContain('showGrid'); + + // The half that makes it a fix rather than a reworded rejection: doing what + // the message says has to work. + const followed = FlowBuilderConfigSchema.safeParse({ snap: { gridSize: 24 } }); + expect(followed.success).toBe(true); + expect(followed.data!.snap.gridSize).toBe(24); + + // `visible` still carries the show/hide intent `grid_` was reaching for. + const visible = FlowBuilderConfigSchema.safeParse({ snap: { visible: false } }); + expect(visible.error!.issues[0].message).toContain('`visible` → `showGrid`'); + }); }); // --------------------------------------------------------------------------- diff --git a/packages/spec/src/studio/flow-builder.zod.ts b/packages/spec/src/studio/flow-builder.zod.ts index bf3fda8d75..fb1d29228e 100644 --- a/packages/spec/src/studio/flow-builder.zod.ts +++ b/packages/spec/src/studio/flow-builder.zod.ts @@ -241,7 +241,13 @@ export const FlowBuilderConfigSchema = lazySchema(() => strictObject({ snap: strictObject({ surface: 'these snap settings', history: FLOW_BUILDER_HISTORY, - aliases: { active: 'enabled', on: 'enabled', grid: 'gridSize', size: 'gridSize', step: 'gridSize', visible: 'showGrid', grid_: 'showGrid' }, + // `grid` points at `gridSize`, NOT at `showGrid`: an author writing `grid` + // on a snap config is reaching for the pixel pitch, and `visible` already + // carries the show/hide intent. A `grid_: 'showGrid'` entry used to sit at + // the end of this table and, because `aliasProbe` strips `_`, overwrote + // `grid` — so `grid: 24` was answered "Did you mean `grid` → `showGrid`?" + // and `showGrid` is a boolean, rejecting 24 a second time (#5481). + aliases: { active: 'enabled', on: 'enabled', grid: 'gridSize', size: 'gridSize', step: 'gridSize', visible: 'showGrid' }, }, { enabled: z.boolean().default(true).describe('Enable snap-to-grid'), gridSize: z.number().int().min(1).default(16).describe('Snap grid size in pixels'), diff --git a/packages/spec/src/ui/chart.zod.ts b/packages/spec/src/ui/chart.zod.ts index 484ff9991d..274094b8aa 100644 --- a/packages/spec/src/ui/chart.zod.ts +++ b/packages/spec/src/ui/chart.zod.ts @@ -209,7 +209,8 @@ export const ChartSeriesSchema = lazySchema(() => strictObject( stackId: 'stack', stackGroup: 'stack', group: 'stack', axis: 'yAxis', yAxisId: 'yAxis', side: 'yAxis', role: 'variant', - strokeDasharray: 'dashArray', strokeDashArray: 'dashArray', dashed: 'dashArray', + // Recharts' own casing alone covers `strokeDashArray` — one probe (#5481). + strokeDasharray: 'dashArray', dashed: 'dashArray', alpha: 'opacity', fillOpacity: 'opacity', strokeOpacity: 'opacity', colour: 'color', fill: 'color', stroke: 'color', },