Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 19 additions & 0 deletions .changeset/olive-pugs-sniff.md
Original file line number Diff line number Diff line change
@@ -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.
4 changes: 3 additions & 1 deletion packages/spec/authorable-surface.base.json
Original file line number Diff line number Diff line change
@@ -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",
Expand Down Expand Up @@ -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",
Expand Down Expand Up @@ -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",
Expand Down
3 changes: 2 additions & 1 deletion packages/spec/src/automation/webhook.zod.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
5 changes: 4 additions & 1 deletion packages/spec/src/data/field.zod.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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',
Expand Down
48 changes: 46 additions & 2 deletions packages/spec/src/shared/alias-integrity.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand All @@ -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));
Expand Down Expand Up @@ -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<string, string[]>();
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
Expand Down
21 changes: 21 additions & 0 deletions packages/spec/src/shared/alias-probe.ts
Original file line number Diff line number Diff line change
@@ -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, '');
16 changes: 16 additions & 0 deletions packages/spec/src/shared/strict-object.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
13 changes: 10 additions & 3 deletions packages/spec/src/shared/suggestions.zod.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
*
Expand Down Expand Up @@ -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'`). */
Expand Down Expand Up @@ -273,6 +272,14 @@ export function strictUnknownKeyError(options: StrictUnknownKeyErrorOptions): z.
const { surface, knownKeys, guidance = {}, history } = options;
const aliases: Record<string, string> = {};
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) => {
Expand Down
22 changes: 22 additions & 0 deletions packages/spec/src/studio/flow-builder.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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`');
});
});

// ---------------------------------------------------------------------------
Expand Down
8 changes: 7 additions & 1 deletion packages/spec/src/studio/flow-builder.zod.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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'),
Expand Down
3 changes: 2 additions & 1 deletion packages/spec/src/ui/chart.zod.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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',
},
Expand Down
Loading