From 4a4307bcb8e5250b6c76e789fb9d04be82d6f2b3 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 3 Aug 2026 10:19:24 +0000 Subject: [PATCH] fix(console): un-invert the validation sample's condition and drop 4 stripped keys (#3276) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `ScriptValidationSchema.condition` is the FAILURE predicate — "Predicate (CEL). If TRUE, validation fails." The `amount_positive` sample declared `condition: 'amount > 0'`, i.e. it failed every order with a positive amount and passed every non-positive one: the exact opposite of its own name and message. Corrected to `amount <= 0`, matching the direction of the spec's own examples (`record.amount < 0`, `discount_percent > 0.40`). The spec guard added in #3257 structurally cannot catch this — it asks whether a draft parses, never whether it means what its author intended — and an inverted sample is worse than a broken one: the broken one gets fixed, the inverted one gets copied, usually by a model. Also removes four keys that parsed and were then silently dropped, since neither the validation branches nor SkillSchema are `.strict()`: - validation.object a rule's scope IS its host object's validations[] - validation.field exists on state_machine/format/json_schema, not script - validation.expression not a key on any validation branch - skill.type SkillSchema classifies by `surface`, not `type` `field` was found by auditing every sample for input keys missing from the parse result; the same audit's other findings are out of scope here and filed as #3280 (view/job/email_template) and #3281 (ValidationPreview). No changeset: `preview-samples.ts` is dev-only. It is reached only from `preview-gallery.tsx` via `preview-gallery.html`, and `vite.config.ts` declares no `rollupOptions.input`, so the build's sole entry stays `index.html` — the samples never reach `dist/`, which is what the published package ships. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01NVPjPzmmAJ2Ngtvgg5MSRa --- apps/console/src/preview-samples.ts | 27 ++++++++++++++++++++++----- 1 file changed, 22 insertions(+), 5 deletions(-) diff --git a/apps/console/src/preview-samples.ts b/apps/console/src/preview-samples.ts index d496cf5ecd..1ca0982252 100644 --- a/apps/console/src/preview-samples.ts +++ b/apps/console/src/preview-samples.ts @@ -276,7 +276,10 @@ export const SAMPLES: Record> = { skill: { name: 'draft_email', label: 'Draft Email', - type: 'prompt', + // No `type`: `SkillSchema` has no such key, so `type: 'prompt'` parsed and was + // then silently dropped. The key that classifies a skill is `surface` + // (`ask` | `build` | `both`, default `ask`, ADR-0063 §3) — it gates which agents + // may bind the skill, and there is no 'prompt' among its values. description: 'Draft a follow-up email from a record context.', active: true, instructions: 'Write a concise, friendly follow-up email referencing the order.', @@ -327,16 +330,30 @@ export const SAMPLES: Record> = { healthCheck: { enabled: true, intervalMs: 60000 }, }, + // `condition` is the FAILURE predicate, not the invariant: `ScriptValidationSchema` + // documents it as "Predicate (CEL). If TRUE, validation fails." So the rule that + // enforces "amount must be positive" reads `amount <= 0` — the illegal case, the + // same direction as the spec's own examples (`record.amount < 0`, + // `discount_percent > 0.40`). The pre-#3276 sample said `amount > 0`, which parses + // fine and means the exact opposite of its name and message: it would have rejected + // every valid order and passed every invalid one. Nothing catches this — a spec + // guard can only ask whether a draft parses, never whether it means what it says — + // so the direction has to be right here, in the example people copy. validation: { name: 'amount_positive', label: 'Amount Must Be Positive', - object: 'sales_order', active: true, severity: 'error', + // A script rule carries ONLY `condition`. `expression` is not a key on any + // validation branch; `object` isn't either (a rule lives in its host object's + // `validations[]`, which IS its scope); and `field` exists on the + // state_machine / format / json_schema branches, not this one. All three parsed + // (the branch is not `.strict()`) and were then silently dropped. To point the + // error at a specific field instead of the whole record, the spec's construct is + // `type: 'cross_field'` with `fields: ['amount']` — same evaluation path, and + // `fields[0]` labels which field the violation attaches to. type: 'script', - field: 'amount', - condition: 'amount > 0', - expression: 'amount > 0', + condition: 'amount <= 0', message: 'Order amount must be greater than zero.', // The enum is the WRITE CONTEXT (`insert` / `update`), not a lifecycle-hook // name: the rule evaluator only runs on the insert/update write path, so