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
7 changes: 4 additions & 3 deletions .changeset/action-param-strict-unknown-keys.md
Original file line number Diff line number Diff line change
Expand Up @@ -18,9 +18,10 @@ offending key, and — when the key is a recognisable spelling of a declared one
the canonical key to use instead:

```
Unrecognized key(s) on this action param: `reference_to`. Until #3405 these were
dropped silently — the param still parsed, so a mis-spelled config shipped as a
control that quietly ignored it. Did you mean `reference_to` → `reference`?
Unrecognized key(s) on this action param: `reference_to`. Did you mean
`reference_to` → `reference`? Until #3405 these were dropped silently — the param
still parsed, so a mis-spelled config shipped as a control that quietly ignored
it.
```

**Migration.** A param that previously carried an extra key now fails to parse.
Expand Down
52 changes: 52 additions & 0 deletions .changeset/strict-unknown-key-history-last.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,52 @@
---
"@objectstack/spec": patch
---

fix(spec): put the unknown-key fix before the surface history sentence (#5955)

`strictUnknownKeyError` — the error map behind every `strictObject` authoring
surface — assembled its message as *front matter → history → fix*. The
`history` sentence a surface declares ("why this key used to be dropped
silently") therefore sat between the two things an author actually needs: the
name of the key that is wrong, and the key to write instead.

That was tolerable while these rejections were warnings. It stopped being
tolerable when #5762 promoted `flow-time-relative-descriptor-invalid` to
**error**, because several consumers render a finding on ONE line — `os
validate`'s `• where: message`, CI logs, and `validateFlowTriggerReadiness`,
which deliberately flattens the newlines out of the schema's own text so the
CLI's bulleted list stays aligned. `TimeRelativeTriggerSchema`'s history
sentence is 224 characters, so on the descriptor from #5496 the words
`Did you mean` landed at character 443 of a 480-character line, behind a
sentence about 2026 that carries no instruction. The author — often an AI —
reads the front of that line and acts on it.

The sentence now goes last:

```text
Unrecognized key(s) on {surface}: `k1`, `k2`. which keys are wrong
[ Did you mean `k1` → `canonical`? ] fix, channel 1 (renames)
[ newline + " • " + {guidance} ] fix, channel 2 (prescriptions)
{history} why it used to be silent
```

Measured on `TimeRelativeTriggerSchema`, before and after:

| case | length before | length after | `Did you mean` at |
|---|---|---|---|
| #5496 descriptor (`field` + missing `dateField` + scalar `offsetDays`) | 480 | 480 | 443 → 219 |
| single misspelled key (`offsetDay`) | 366 | 366 | 329 → 105 |
| guidance hit (`schedule`) | 544 | 544 | n/a (bullet at 92) |

**Nothing was deleted, nothing became conditional.** Every declared `history`
is still emitted, verbatim, exactly once per message — message lengths are
byte-identical, only the position moved. Both fix channels moved ahead of it:
a `guidance` prescription is as actionable as a rename, so it could not be left
behind the sentence either.

**Migration.** No authoring change, and no schema change: `history` is still a
required option, spelled the same way, on all 62 `strictObject` surfaces and
44 direct `strictUnknownKeyError` call sites. A test that asserts the full
message text in order needs its expectation reordered; a test that asserts
fragments with `toContain` is unaffected. The order itself is now pinned in
`strict-object.test.ts`, so it cannot silently regress.
75 changes: 75 additions & 0 deletions packages/spec/src/shared/strict-object.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -170,6 +170,81 @@ describe('strictObject', () => {
});
});

/**
* The ORDER the message emits its parts in — pinned as order, not as presence.
*
* Every assertion above this block is a `toContain` on one fragment, so all of
* them stayed green while `history` sat in the MIDDLE of the message, between
* "which key is wrong" and "here is the fix". That mattered once #5762 promoted
* `flow-time-relative-descriptor-invalid` to **error**: several consumers render
* a finding on ONE line (`os validate`'s `• where: message`, CI logs, and
* `validateFlowTriggerReadiness`, which flattens the newlines out of the schema's
* own text), and `TimeRelativeTriggerSchema`'s history sentence is 224
* characters — so the author, often an AI, read the front of the line and found
* a sentence about 2026 instead of the key to write (#5955).
*
* Direction A of that issue's ruling: move the sentence to the end. Nothing is
* deleted and nothing is conditional — which is exactly why it needs an ORDER
* pin rather than another presence check. A future edit that folds `history`
* back into the front matter passes every `toContain` in this file; it fails
* here.
*/
describe('message order — the fix comes before the history (#5955)', () => {
const HISTORY = 'Until #4001 these were dropped silently — the widget still rendered.';

const messageFor = (body: Record<string, unknown>) => {
const r = WidgetSchema.safeParse({ name: 'x', ...body });
expect(r.success).toBe(false);
return r.error!.issues[0]!.message;
};

it('names the wrong key first, then the rename, then the history', () => {
const m = messageFor({ colummSpan: 2 });
// 1. which key is wrong — and nothing before it
expect(m.startsWith('Unrecognized key(s) on this widget: `colummSpan`.')).toBe(true);
// 2. the fix, immediately after it (this is the whole point of the reorder)
expect(m).toContain('`colummSpan`. Did you mean `colummSpan` → `columnSpan`?');
// 3. the history sentence, verbatim, last — moved, never dropped
expect(m.endsWith(` ${HISTORY}`)).toBe(true);
expect(m.indexOf('Did you mean')).toBeLessThan(m.indexOf(HISTORY));
});

it('puts a guidance prescription ahead of the history too', () => {
// The other fix channel. A tombstone/wrong-layer prescription is as
// actionable as a rename, so it cannot sit behind the sentence either.
const m = messageFor({ span: 2 });
expect(m.startsWith('Unrecognized key(s) on this widget: `span`.')).toBe(true);
expect(m).toContain('\n • `span` was retired in vX. Use `columnSpan`.');
expect(m.endsWith(` ${HISTORY}`)).toBe(true);
expect(m.indexOf('was retired in vX')).toBeLessThan(m.indexOf(HISTORY));
});

it('keeps BOTH fix channels ahead of the history in one message', () => {
const m = messageFor({ span: 2, colummSpan: 3 });
expect(m.startsWith('Unrecognized key(s) on this widget: `span`, `colummSpan`.')).toBe(true);
expect(m.indexOf('Did you mean')).toBeLessThan(m.indexOf(HISTORY));
expect(m.indexOf('was retired in vX')).toBeLessThan(m.indexOf(HISTORY));
expect(m.endsWith(` ${HISTORY}`)).toBe(true);
});

it('emits the history exactly once, whatever the key count', () => {
// It is a per-SURFACE sentence, not a per-key one: zod raises a single
// `unrecognized_keys` issue naming every offending key, so the sentence is
// appended to that one message once — the property that makes "last" a
// well-defined position at all.
const m = messageFor({ colummSpan: 2, alsoWrong: 3, andThis: 4 });
expect(m.split(HISTORY)).toHaveLength(2);
});

it('is unchanged when there is no fix to offer', () => {
// No rename, no prescription — the sentence follows the key statement
// directly, exactly as it always did. Full-message pin, so any stray
// separator or duplicated clause fails here.
expect(messageFor({ nonsense: 1 }))
.toBe(`Unrecognized key(s) on this widget: \`nonsense\`. ${HISTORY}`);
});
});

/**
* Never suggest a key the schema cannot accept.
*
Expand Down
39 changes: 35 additions & 4 deletions packages/spec/src/shared/suggestions.zod.ts
Original file line number Diff line number Diff line change
Expand Up @@ -241,7 +241,12 @@ export interface StrictUnknownKeyErrorOptions {
* key. Matched case-sensitively (exact authored spelling).
*/
guidance?: Readonly<Record<string, string>>;
/** One sentence of history: why this key would previously have failed silently. */
/**
* One sentence of history: why this key would previously have failed
* silently. Rendered **last**, after both fix channels (`Did you mean` and
* the `guidance` bullets) — see the ordering note on
* {@link strictUnknownKeyError}.
*/
history: string;
}

Expand Down Expand Up @@ -269,6 +274,24 @@ export interface StrictUnknownKeyErrorOptions {
* First consumers: `ui/action.zod.ts` (#3746, the template this generalizes),
* `security/permission.zod.ts`, `automation/flow.zod.ts`.
*
* ## Message order: the fix comes before the history (#5955)
*
* One message per rejected object — every unknown key of that object is named
* in it, and the surface's `history` sentence appears exactly once, whatever
* the key count. The parts are emitted in the order an author has to read
* them:
*
* ```text
* Unrecognized key(s) on {surface}: `k1`, `k2`. ← which keys are wrong
* [ Did you mean `k1` → `canonical`? ] ← fix, channel 1 (renames)
* [ \n • {guidance} ] ← fix, channel 2 (prescriptions)
* {history} ← why it used to be silent
* ```
*
* `history` sat in the middle until #5955, which pushed the fix past character
* ~220 on the single-line displays several consumers use. It is still emitted
* verbatim and unconditionally — only its position moved.
*
* ## The table is recorded as it is built (#5483)
*
* Every call registers its `{ surface, knownKeys, aliases, guidance }` with
Expand Down Expand Up @@ -317,10 +340,18 @@ export function strictUnknownKeyError(options: StrictUnknownKeyErrorOptions): z.
aliases[aliasProbe(key)] ?? findClosestMatches(key, knownKeys, maxDistance, 1)[0];
if (canonical && canonical !== key) renames.push(`\`${key}\` → \`${canonical}\``);
}
let message =
`Unrecognized key(s) on ${surface}: ${keys.map((k) => `\`${k}\``).join(', ')}. ${history}`;
// Order: WHICH KEY IS WRONG → HOW TO FIX IT → why it used to be silent.
// `history` used to sit in the middle, between the key statement and the
// suggestion, which put the fix past character ~220 of a message several
// consumers render on ONE line (`os validate`'s `• where: message`, CI
// logs, and `validateFlowTriggerReadiness`, which flattens the newlines).
// Since #5762 promoted one of those rules to error level the author — often
// an AI — reads the front of that line and acts on it, so the prescription
// has to be there. Nothing is dropped or made conditional: the sentence is
// still emitted verbatim, once per message, just last (#5955).
let message = `Unrecognized key(s) on ${surface}: ${keys.map((k) => `\`${k}\``).join(', ')}.`;
if (renames.length) message += ` Did you mean ${renames.join(', ')}?`;
if (prescriptions.length) message += `\n${prescriptions.map((p) => ` • ${p}`).join('\n')}`;
return message;
return `${message} ${history}`;
};
}
Loading