From 2563ed2e8f7327857dbc4da5e25d97c3e3047181 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 7 Aug 2026 14:49:53 +0000 Subject: [PATCH] =?UTF-8?q?fix(spec):=20=E6=8A=8A=20strictObject=20?= =?UTF-8?q?=E7=9A=84=20history=20=E5=8F=A5=E7=A7=BB=E5=88=B0=E6=B6=88?= =?UTF-8?q?=E6=81=AF=E6=9C=AB=E5=B0=BE,=E4=BF=AE=E6=B3=95=E5=89=8D?= =?UTF-8?q?=E7=BD=AE=20(#5955)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `strictUnknownKeyError` 的消息此前拼成「前置语 → history → 修法」, 每个 surface 自己声明的沿革句(「这个键以前会被静默丢弃」)正好夹在 作者真正需要的两件事之间:哪个键错了,和该写什么。 #5762 把 `flow-time-relative-descriptor-invalid` 升为 error 之后这一点 开始碍事:多个消费方把 finding 打成单行 —— `os validate` 的 `• where: message`、CI 日志,以及 `validateFlowTriggerReadiness` (它刻意把 schema 原文里的换行压平,好让 CLI 的项目符号列表保持对齐)。 `TimeRelativeTriggerSchema` 的 history 句是 224 字符,于是在 #5496 的 描述符上,`Did you mean` 落在 480 字符行的第 443 位,前面挡着一句没有 动作价值的沿革。作者(常常是 AI)只读这行的开头就动手了。 现在沿革句排到最后,两个修法通道(rename 与 guidance 处方)都排在它 前面 —— 一条处方和一次改名一样可执行,不能留在句子后面。 一处拼接点改动,消息长度逐字节不变(480/366/544/598),没有删任何文本、 没有加任何条件分支。新增顺序钉测试 5 条置于 `strict-object.test.ts`; 反向验证:把旧拼接式放回去,其中 3 条转红。 Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01JTSZAjgtL3oR6YcpNDhW3T --- .../action-param-strict-unknown-keys.md | 7 +- .changeset/strict-unknown-key-history-last.md | 52 +++++++++++++ .../spec/src/shared/strict-object.test.ts | 75 +++++++++++++++++++ packages/spec/src/shared/suggestions.zod.ts | 39 +++++++++- 4 files changed, 166 insertions(+), 7 deletions(-) create mode 100644 .changeset/strict-unknown-key-history-last.md diff --git a/.changeset/action-param-strict-unknown-keys.md b/.changeset/action-param-strict-unknown-keys.md index 0fa7b1337a..887fbe852a 100644 --- a/.changeset/action-param-strict-unknown-keys.md +++ b/.changeset/action-param-strict-unknown-keys.md @@ -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. diff --git a/.changeset/strict-unknown-key-history-last.md b/.changeset/strict-unknown-key-history-last.md new file mode 100644 index 0000000000..563f961513 --- /dev/null +++ b/.changeset/strict-unknown-key-history-last.md @@ -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. diff --git a/packages/spec/src/shared/strict-object.test.ts b/packages/spec/src/shared/strict-object.test.ts index bf2f6bdf77..069dc230f0 100644 --- a/packages/spec/src/shared/strict-object.test.ts +++ b/packages/spec/src/shared/strict-object.test.ts @@ -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) => { + 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. * diff --git a/packages/spec/src/shared/suggestions.zod.ts b/packages/spec/src/shared/suggestions.zod.ts index f35824a60c..313e056252 100644 --- a/packages/spec/src/shared/suggestions.zod.ts +++ b/packages/spec/src/shared/suggestions.zod.ts @@ -241,7 +241,12 @@ export interface StrictUnknownKeyErrorOptions { * key. Matched case-sensitively (exact authored spelling). */ guidance?: Readonly>; - /** 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; } @@ -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 @@ -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}`; }; }