From 962f0bef923610e2a47e655d9b6389bbe83b93c8 Mon Sep 17 00:00:00 2001 From: like Date: Sat, 17 Oct 2026 08:10:26 +0800 Subject: [PATCH] fix: validate conditional branch structure when requested --- apps/web/content/docs/api/core.mdx | 10 ++++ packages/core/README.md | 10 ++++ packages/core/src/spec-validator.test.ts | 68 ++++++++++++++++++++++++ packages/core/src/spec-validator.ts | 40 +++++++++++++- skills/core/SKILL.md | 10 ++++ 5 files changed, 137 insertions(+), 1 deletion(-) diff --git a/apps/web/content/docs/api/core.mdx b/apps/web/content/docs/api/core.mdx index e45068df..a05a794f 100644 --- a/apps/web/content/docs/api/core.mdx +++ b/apps/web/content/docs/api/core.mdx @@ -697,6 +697,16 @@ const patches = diffToPatches(oldSpec, newSpec); // [{ op: "replace", path: "/elements/main/props/title", value: "New Title" }] ``` +Hosts can opt in to `validateSpec(spec, { checkConditionalBranches: true })` +to require `$then` and `$else` beside `$cond` in props and action parameters +(including nested branches and watch actions). For example, +`{ "$cond": { "$state": "/saved", "$then": "Saved", "$else": "Save" } }` +returns `invalid_conditional_branches` with the expression's JSON Pointer in +`issue.path` and the formatted error message. The valid form is +`{ "$cond": { "$state": "/saved" }, "$then": "Saved", "$else": "Save" }`. +The option defaults to false. It does not inspect literal initial state, +evaluate conditions, rewrite expressions, or change rendering behavior. + ## evaluateVisibility Evaluates a visibility condition against the state model. diff --git a/packages/core/README.md b/packages/core/README.md index 977fb331..c1b9331c 100644 --- a/packages/core/README.md +++ b/packages/core/README.md @@ -611,6 +611,16 @@ const { spec: fixed, fixes, fixDetails } = autoFixSpec(spec); `validateSpec` checks structure beyond the catalog schema: missing or dangling `children` and named `slots` references, malformed `visible` conditions (anything outside the documented forms evaluates to hidden at runtime, so it is rejected with code `invalid_visible`), `repeat` containers with no children (`repeat_without_children`), relative repeat paths outside an enclosing repeat (`repeat_item_outside_scope`), and `repeat.statePath` values that do not reference an array in the spec's own `state` (`repeat_state_mismatch`). +Hosts can opt in to `validateSpec(spec, { checkConditionalBranches: true })` +to require `$then` and `$else` beside `$cond` in props and action parameters +(including nested branches and watch actions). For example, +`{ "$cond": { "$state": "/saved", "$then": "Saved", "$else": "Save" } }` +returns `invalid_conditional_branches` with the expression's JSON Pointer in +`issue.path` and the formatted error message. The valid form is +`{ "$cond": { "$state": "/saved" }, "$then": "Saved", "$else": "Save" }`. +The option defaults to false. It does not inspect literal initial state, +evaluate conditions, rewrite expressions, or change rendering behavior. + `autoFixSpec` distinguishes lossless fixes (relocating `visible`/`on`/`repeat`/`watch` out of `props`) from lossy ones (pruning `children` or named `slots` references to elements that were never defined). Each entry in `fixDetails` carries `{ message, lossy }`. Callers with a repair loop should apply lossless fixes immediately and prefer re-prompting over lossy fixes, passing `{ lossy: false }` to withhold pruning until retries are exhausted: ```typescript diff --git a/packages/core/src/spec-validator.test.ts b/packages/core/src/spec-validator.test.ts index 71853bfe..2bdde025 100644 --- a/packages/core/src/spec-validator.test.ts +++ b/packages/core/src/spec-validator.test.ts @@ -766,3 +766,71 @@ describe("autoFixSpec", () => { expect(fixes).toHaveLength(0); }); }); + +describe("optional conditional branch validation", () => { + it("reports misplaced branches in resolved props and action parameters", () => { + const misplaced = { + $cond: { $state: "/favorite", $then: "Saved", $else: "Save" }, + }; + const spec: Spec = { + root: "star", + state: { favorite: false }, + elements: { + star: { + type: "IconButton", + props: { icon: "star", label: misplaced }, + on: { + press: { + action: "setState", + params: { statePath: "/favorite", value: misplaced }, + }, + }, + }, + }, + }; + expect(validateSpec(spec).valid).toBe(true); + expect(validateSpec(spec, { checkConditionalBranches: true })).toMatchObject({ + valid: false, + issues: [ + { + code: "invalid_conditional_branches", + path: "/elements/star/props/label", + }, + { + code: "invalid_conditional_branches", + path: "/elements/star/on/press/params/value", + }, + ], + }); + }); + + it("accepts sibling branches including null and ignores literal initial state", () => { + const spec: Spec = { + root: "star", + state: { favorite: false, literal: { $cond: { example: true } } }, + elements: { + star: { + type: "IconButton", + props: { + icon: "star", + label: { + $cond: { $state: "/favorite" }, + $then: "Saved", + $else: "Save", + }, + }, + on: { + press: { + action: "setState", + params: { + statePath: "/favorite", + value: { $cond: { $state: "/favorite" }, $then: false, $else: null }, + }, + }, + }, + }, + }, + }; + expect(validateSpec(spec, { checkConditionalBranches: true }).valid).toBe(true); + }); +}); diff --git a/packages/core/src/spec-validator.ts b/packages/core/src/spec-validator.ts index b9fa53ee..1e01332a 100644 --- a/packages/core/src/spec-validator.ts +++ b/packages/core/src/spec-validator.ts @@ -25,12 +25,15 @@ export interface SpecIssue { message: string; /** The element key where the issue was found (if applicable) */ elementKey?: string; + /** JSON Pointer to the offending expression. */ + path?: string; /** Machine-readable issue code for programmatic handling */ code: | "missing_root" | "root_not_found" | "missing_child" | "invalid_visible" + | "invalid_conditional_branches" | "repeat_without_children" | "repeat_item_outside_scope" | "repeat_state_mismatch" @@ -61,6 +64,11 @@ export interface ValidateSpecOptions { * Defaults to false since orphans are harmless (just unused). */ checkOrphans?: boolean; + /** + * Require $then/$else alongside $cond in dynamic expression positions. + * Defaults to false; never inspects literal initial state or rewrites values. + */ + checkConditionalBranches?: boolean; } /** @@ -85,7 +93,7 @@ export function validateSpec( spec: Spec, options: ValidateSpecOptions = {}, ): SpecValidationIssues { - const { checkOrphans = false } = options; + const { checkOrphans = false, checkConditionalBranches = false } = options; const issues: SpecIssue[] = []; // 1. Check root @@ -347,6 +355,36 @@ export function validateSpec( } } + if (checkConditionalBranches) { + const pointerKey = (key: string) => + key.replaceAll("~", "~0").replaceAll("/", "~1"); + const visit = (value: unknown, path: string, elementKey: string): void => { + if (!value || typeof value !== "object") return; + const record = value as Record; + if ( + Object.hasOwn(record, "$cond") && + (!Object.hasOwn(record, "$then") || !Object.hasOwn(record, "$else")) + ) { + issues.push({ + severity: "error", + code: "invalid_conditional_branches", + path, + elementKey, + message: `Conditional expression at ${path} requires $then and $else alongside $cond, not inside it.`, + }); + } + for (const [key, child] of Object.entries(record)) { + visit(child, `${path}/${pointerKey(key)}`, elementKey); + } + }; + for (const [key, element] of Object.entries(spec.elements)) { + const path = `/elements/${pointerKey(key)}`; + visit(element.props, `${path}/props`, key); + visit(element.on, `${path}/on`, key); + visit(element.watch, `${path}/watch`, key); + } + } + const hasErrors = issues.some((i) => i.severity === "error"); return { valid: !hasErrors, issues }; } diff --git a/skills/core/SKILL.md b/skills/core/SKILL.md index 8838e3ae..63d80289 100644 --- a/skills/core/SKILL.md +++ b/skills/core/SKILL.md @@ -209,6 +209,16 @@ const { spec: fixed, fixDetails } = autoFixSpec(spec); const attempt = autoFixSpec(spec, { lossy: retriesExhausted }); ``` +Hosts can opt in to `validateSpec(spec, { checkConditionalBranches: true })` +to require `$then` and `$else` beside `$cond` in props and action parameters +(including nested branches and watch actions). For example, +`{ "$cond": { "$state": "/saved", "$then": "Saved", "$else": "Save" } }` +returns `invalid_conditional_branches` with the expression's JSON Pointer in +`issue.path` and the formatted error message. The valid form is +`{ "$cond": { "$state": "/saved" }, "$then": "Saved", "$else": "Save" }`. +The option defaults to false. It does not inspect literal initial state, +evaluate conditions, rewrite expressions, or change rendering behavior. + ## Visibility Conditions Control element visibility with state-based conditions. `VisibilityContext` is `{ stateModel: StateModel }`.