From cf352b4d659095c463dfcc1c638ace66ba14f827 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 10 Aug 2026 02:52:31 +0000 Subject: [PATCH] feat(spec): a skill trigger condition's `value` must have the shape its operator reads (#7113) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `SkillTriggerConditionSchema.operator` and `.value` were declared independently, so every operator accepted every shape: `{ operator: 'in', value: 'admin' }` — a membership test whose list is not a list — was spec-valid. The dormant twin of #6227 on `ViewFilterRuleSchema`; the fix mirrors that one (PR #7114) key for key. Dormant is the point: the sole consumer (`SkillRegistry.evaluateCondition`, cloud agent runtime) coerces the scalar itself, so nothing ever failed. What is closed is a second dialect — a consumer-side lenient coercion standing in for a contract the producer never declared. That coercion becomes a no-op here; removing it is a producer-first follow-up in the cloud repo. - `in` / `not_in` (SKILL_TRIGGER_LIST_VALUE_OPERATORS) require an array. - `eq` / `neq` (SKILL_TRIGGER_SCALAR_VALUE_OPERATORS) require a string — `===` on an array is reference identity, so an array comparand is a dead predicate. - `contains` is deliberately unchanged: it has two live branches (string substring, array subset), and #5685 rules against a schema stricter than its runtime. Both vocabularies are exported so producers enumerate from the contract. Authoring impact censused first across this repo and the cloud repo: no real skill authors the scalar-on-set-operator form. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01HCb6mPxnEjvhKnnka1RNxw --- ...gger-condition-value-shaped-by-operator.md | 56 +++++ packages/spec/api-surface/ai.json | 2 + packages/spec/export-origins/ai.json | 2 + ...kill-trigger-condition-value-shape.test.ts | 205 ++++++++++++++++++ packages/spec/src/ai/skill.test.ts | 11 +- packages/spec/src/ai/skill.zod.ts | 177 ++++++++++++++- 6 files changed, 449 insertions(+), 4 deletions(-) create mode 100644 .changeset/skill-trigger-condition-value-shaped-by-operator.md create mode 100644 packages/spec/src/ai/skill-trigger-condition-value-shape.test.ts diff --git a/.changeset/skill-trigger-condition-value-shaped-by-operator.md b/.changeset/skill-trigger-condition-value-shaped-by-operator.md new file mode 100644 index 0000000000..21795d1624 --- /dev/null +++ b/.changeset/skill-trigger-condition-value-shaped-by-operator.md @@ -0,0 +1,56 @@ +--- +"@objectstack/spec": minor +--- + +feat(spec): a skill trigger condition's `value` must have the shape its OPERATOR reads (#7113) + +`SkillTriggerConditionSchema.operator` and `.value` were declared independently +— `z.enum(['eq','neq','in','not_in','contains'])` beside +`z.union([z.string(), z.array(z.string())])` — so every operator accepted every +shape. `{ field: 'userRole', operator: 'in', value: 'admin' }` was a spec-valid +skill trigger: a membership test whose list is not a list. + +This is the **dormant twin** of #6227 on `ViewFilterRuleSchema`, and the fix +mirrors that one (PR #7114) key for key — the exported operator vocabularies, +the `superRefine`, the single issue at path `['value']`. + +**Why "dormant" is the whole point.** #6227's shape genuinely failed at query +time (`assertListComparandShapes`, 400 `INVALID_FILTER`), which made it a +two-stage failure. This one never failed at all: the sole consumer, +`SkillRegistry.evaluateCondition` in the cloud agent runtime, coerces the scalar +itself with `Array.isArray(expected) ? expected : [expected]`. Nothing 400s and +the predicate evaluates the way the author meant. What is being closed is +therefore not a break but a **second dialect** — a consumer-side lenient +coercion standing in for a contract the producer never declared, on a surface +whose authors are increasingly AI-generated, where "declared = enforced" is what +keeps generated metadata honest. That coercion becomes a no-op once this ships; +removing it is a follow-up in the cloud repo, producer-first. + +**The constraint, and its deliberate limit:** + +| operator | `value` must be | why | +|---|---|---| +| `in` / `not_in` (`SKILL_TRIGGER_LIST_VALUE_OPERATORS`) | an array, any length | the consumer answers them with `list.includes(fieldValue)` — the authored value IS the list | +| `eq` / `neq` (`SKILL_TRIGGER_SCALAR_VALUE_OPERATORS`) | a string | `===` / `!==` on an array is reference identity, so an array comparand is a DEAD predicate: `eq` never fires, `neq` always does | +| `contains` | **unchanged — either shape** | it has two live branches: string∈string substring, and array⊆array subset (`expected.every(v => fieldValue.includes(v))`) | + +`contains` is left alone on purpose. #5685 ruled on the opposite error — a +schema stricter than its runtime in ways the runtime deliberately allows — and +`SkillContext` is indexed `[extraField: string]: unknown`, so an array-valued +context field is a shape the consumer is written for. Refusing it here would +un-declare a working capability, which is an ADR-0049 retirement decision and +not a rider on a shape fix. + +Both vocabularies are **exported** so a producer — a condition editor, a +generator, a test — asks the question the schema asks instead of keeping its own +copy of the list, the same reason `VIEW_FILTER_LIST_VALUE_OPERATORS` is exported +one module over. + +**Authoring impact: measured, not assumed.** Censused before landing across this +repo (`packages/`, `examples/`, `content/`, `docs/`) and the cloud repo +(`packages/service-ai` skill definitions, seeds, fixtures, docs corpora): no +real (non-test) skill authors `triggerConditions` in the scalar-on-set-operator +form. The framework's six built-in skills declare no `triggerConditions` at all, +and both authored examples already use the array form on `in`. The one in-repo +test that handed a scalar to all five operators was asserting the decoupling +itself and is updated to enumerate the shape each operator reads. diff --git a/packages/spec/api-surface/ai.json b/packages/spec/api-surface/ai.json index 2097bc3b25..80b47f6e6e 100644 --- a/packages/spec/api-surface/ai.json +++ b/packages/spec/api-surface/ai.json @@ -126,6 +126,8 @@ "PromptVariable (type)", "PromptVariableParsed (type)", "PromptVariableSchema (const)", + "SKILL_TRIGGER_LIST_VALUE_OPERATORS (const)", + "SKILL_TRIGGER_SCALAR_VALUE_OPERATORS (const)", "Skill (type)", "SkillParsed (type)", "SkillSchema (const)", diff --git a/packages/spec/export-origins/ai.json b/packages/spec/export-origins/ai.json index c339364e93..e87553c3e9 100644 --- a/packages/spec/export-origins/ai.json +++ b/packages/spec/export-origins/ai.json @@ -126,6 +126,8 @@ "PromptVariable": "src/ai/model-registry.zod.ts#PromptVariable (type)", "PromptVariableParsed": "src/ai/model-registry.zod.ts#PromptVariableParsed (type)", "PromptVariableSchema": "src/ai/model-registry.zod.ts#PromptVariableSchema (const)", + "SKILL_TRIGGER_LIST_VALUE_OPERATORS": "src/ai/skill.zod.ts#SKILL_TRIGGER_LIST_VALUE_OPERATORS (const)", + "SKILL_TRIGGER_SCALAR_VALUE_OPERATORS": "src/ai/skill.zod.ts#SKILL_TRIGGER_SCALAR_VALUE_OPERATORS (const)", "Skill": "src/ai/skill.zod.ts#Skill (type)", "SkillParsed": "src/ai/skill.zod.ts#SkillParsed (type)", "SkillSchema": "src/ai/skill.zod.ts#SkillSchema (const)", diff --git a/packages/spec/src/ai/skill-trigger-condition-value-shape.test.ts b/packages/spec/src/ai/skill-trigger-condition-value-shape.test.ts new file mode 100644 index 0000000000..48542d4092 --- /dev/null +++ b/packages/spec/src/ai/skill-trigger-condition-value-shape.test.ts @@ -0,0 +1,205 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * [#7113] `SkillTriggerConditionSchema.value` is shaped by the condition's + * OPERATOR — the dormant twin of #6227 (`ViewFilterRuleSchema`, PR #7114). + * + * "Dormant" is the whole difference and these pins are written around it. The + * #6227 shape genuinely failed at query time; this one never failed at all — + * the sole consumer (`SkillRegistry.evaluateCondition`, cloud + * `packages/service-ai/src/skill-registry.ts`) coerces the scalar with + * `Array.isArray(expected) ? expected : [expected]`. So what these pins hold is + * not a break-fix but the contract-first property: the producer declares the + * one spelling instead of letting a consumer quietly accept two. + * + * Every rejection pin asserts the issue CODE and PATH, not merely that a throw + * happened: a bare `.toThrow()` cannot tell "refused for the right reason at + * the right key" from "refused because the value union rejected the type", and + * those are different defects (#6142). + * + * The accept pins matter as much as the reject pins. `contains` keeps BOTH + * spellings on purpose — the consumer has a live array⊆array branch for it — + * and #5685 rules that a schema stricter than its runtime is the wrong side of + * the fix. A pin that only checked rejections would let that regress silently. + */ + +import { describe, expect, it } from 'vitest'; +import { + SKILL_TRIGGER_LIST_VALUE_OPERATORS, + SKILL_TRIGGER_SCALAR_VALUE_OPERATORS, + SkillSchema, + SkillTriggerConditionSchema, +} from './skill.zod'; + +/** Parse helper — the authored object form, exactly as a skill carries it. */ +const parse = (condition: Record) => + SkillTriggerConditionSchema.safeParse(condition); + +/** The single `value`-path issue a shape refusal must produce. */ +function valueIssue(result: ReturnType) { + expect(result.success).toBe(false); + if (result.success) throw new Error('unreachable'); + const issues = result.error.issues.filter((i) => i.path.join('.') === 'value'); + expect(issues).toHaveLength(1); + return issues[0]!; +} + +describe('#7113 — the reported shape is refused at authoring time', () => { + it('refuses the card example: a set operator carrying a scalar', () => { + const result = parse({ field: 'userRole', operator: 'in', value: 'admin' }); + const issue = valueIssue(result); + + expect(issue.code).toBe('custom'); + expect(issue.path).toEqual(['value']); + expect(issue.message).toContain( + 'Operator "in" on field "userRole" requires an ARRAY of values.', + ); + // The refusal carries what the author has to DO, not just what is wrong. + expect(issue.message).toContain('Received a string ("admin")'); + expect(issue.message).toContain('write ["admin"] for a single value'); + expect(issue.message).toContain('or use "eq" to compare against it'); + // And it says the empty list is NOT what is being refused. + expect(issue.message).toContain('An empty list [] is allowed'); + }); + + it('names the consumer-side coercion as the thing being replaced', () => { + const issue = valueIssue(parse({ field: 'userRole', operator: 'not_in', value: 'admin' })); + expect(issue.message).toContain('coerces the scalar today'); + expect(issue.message).toContain('#7113'); + }); +}); + +describe('#7113 — list operators require an array', () => { + it.each(SKILL_TRIGGER_LIST_VALUE_OPERATORS)('%s refuses a scalar', (operator) => { + const issue = valueIssue(parse({ field: 'objectName', operator, value: 'lead' })); + expect(issue.code).toBe('custom'); + expect(issue.path).toEqual(['value']); + expect(issue.message).toContain(`Operator "${operator}"`); + expect(issue.message).toContain('requires an ARRAY of values'); + }); + + it.each(SKILL_TRIGGER_LIST_VALUE_OPERATORS)('%s accepts an array', (operator) => { + const result = parse({ field: 'objectName', operator, value: ['lead', 'opportunity'] }); + expect(result.success).toBe(true); + }); + + it.each(SKILL_TRIGGER_LIST_VALUE_OPERATORS)( + '%s accepts an EMPTY array — it is a real predicate, not the defect', + (operator) => { + expect(parse({ field: 'objectName', operator, value: [] }).success).toBe(true); + }, + ); + + it('refuses a missing value with ONE issue — the required check, not two', () => { + // Measured, not assumed: Zod 4 skips a `superRefine` when the object's own + // shape already failed, so an omitted `value` reports only the required + // issue. Pinned because the refinement's "no value" wording exists for the + // case where a future carrier makes `value` optional — this records that + // today it is unreachable, rather than leaving a reader to guess that a + // missing value produces two competing complaints at one key. + const result = parse({ field: 'objectName', operator: 'in' }); + expect(result.success).toBe(false); + if (result.success) throw new Error('unreachable'); + const atValue = result.error.issues.filter((i) => i.path.join('.') === 'value'); + expect(atValue).toHaveLength(1); + expect(atValue[0]!.code).not.toBe('custom'); + }); +}); + +describe('#7113 — identity operators require a string', () => { + it.each(SKILL_TRIGGER_SCALAR_VALUE_OPERATORS)('%s refuses an array', (operator) => { + const issue = valueIssue(parse({ field: 'objectName', operator, value: ['lead'] })); + expect(issue.code).toBe('custom'); + expect(issue.path).toEqual(['value']); + expect(issue.message).toContain(`Operator "${operator}"`); + expect(issue.message).toContain('requires a single STRING value'); + // The message must explain the DEAD-predicate mechanism, since nothing + // errors today — an author has no runtime symptom to reason from. + expect(issue.message).toContain(operator === 'eq' ? 'never fire' : 'always fire'); + expect(issue.message).toContain(operator === 'eq' ? 'use "in"' : 'use "not_in"'); + }); + + it.each(SKILL_TRIGGER_SCALAR_VALUE_OPERATORS)('%s accepts a string', (operator) => { + expect(parse({ field: 'objectName', operator, value: 'lead' }).success).toBe(true); + }); +}); + +describe('#7113 — `contains` keeps BOTH shapes (#5685: no stricter than the runtime)', () => { + it('accepts a string comparand — the substring branch', () => { + expect(parse({ field: 'viewName', operator: 'contains', value: 'kanban' }).success).toBe(true); + }); + + it('accepts an array comparand — the live array⊆array subset branch', () => { + // `evaluateCondition`: `expected.every(v => fieldValue.includes(v))` when the + // context field is an array. `SkillContext` is indexed `[k: string]: unknown`, + // so that is a shape the cloud runtime is deliberately written for. + // Refusing it here would un-declare a working capability (an ADR-0049 + // retirement decision), not tighten a contract. + expect(parse({ field: 'tags', operator: 'contains', value: ['a', 'b'] }).success).toBe(true); + }); + + it('is in neither constrained vocabulary', () => { + expect(SKILL_TRIGGER_LIST_VALUE_OPERATORS).not.toContain('contains'); + expect(SKILL_TRIGGER_SCALAR_VALUE_OPERATORS).not.toContain('contains'); + }); +}); + +describe('#7113 — the exported vocabularies are the contract, not a copy', () => { + it('the two vocabularies are disjoint and both subsets of the operator enum', () => { + const all = [ + ...SKILL_TRIGGER_LIST_VALUE_OPERATORS, + ...SKILL_TRIGGER_SCALAR_VALUE_OPERATORS, + ]; + expect(new Set(all).size).toBe(all.length); + for (const operator of all) { + // Every declared member must actually be an operator the schema accepts. + expect(parse({ + field: 'f', + operator, + value: (SKILL_TRIGGER_LIST_VALUE_OPERATORS as readonly string[]).includes(operator) + ? ['x'] + : 'x', + }).success).toBe(true); + } + }); + + it('pins the membership so a future operator has to be classified', () => { + expect([...SKILL_TRIGGER_LIST_VALUE_OPERATORS]).toEqual(['in', 'not_in']); + expect([...SKILL_TRIGGER_SCALAR_VALUE_OPERATORS]).toEqual(['eq', 'neq']); + }); +}); + +describe('#7113 — the refinement does not disturb the carrier', () => { + it('an unrelated operator/value pair still parses through Skill.triggerConditions', () => { + const skill = SkillSchema.parse({ + name: 'order_management', + label: 'Order Management', + instructions: 'Manage orders.', + tools: ['create_order'], + triggerConditions: [ + { field: 'objectName', operator: 'eq', value: 'order' }, + { field: 'userRole', operator: 'in', value: ['sales', 'support'] }, + ], + }); + expect(skill.triggerConditions).toHaveLength(2); + }); + + it('a bad condition inside a skill reports at the nested value path', () => { + // The path prefix proves the refinement travels with the carrier rather + // than only firing on a standalone parse. + const result = SkillSchema.safeParse({ + name: 'order_management', + label: 'Order Management', + instructions: 'Manage orders.', + tools: ['create_order'], + triggerConditions: [{ field: 'userRole', operator: 'in', value: 'admin' }], + }); + expect(result.success).toBe(false); + if (result.success) throw new Error('unreachable'); + const issue = result.error.issues.find( + (i) => i.path.join('.') === 'triggerConditions.0.value', + ); + expect(issue).toBeDefined(); + expect(issue!.code).toBe('custom'); + }); +}); diff --git a/packages/spec/src/ai/skill.test.ts b/packages/spec/src/ai/skill.test.ts index 681f92c9f6..9b6659c0fe 100644 --- a/packages/spec/src/ai/skill.test.ts +++ b/packages/spec/src/ai/skill.test.ts @@ -1,5 +1,6 @@ import { describe, it, expect } from 'vitest'; import { + SKILL_TRIGGER_LIST_VALUE_OPERATORS, SkillSchema, SkillTriggerConditionSchema, defineSkill, @@ -7,14 +8,20 @@ import { } from './skill.zod'; describe('SkillTriggerConditionSchema', () => { - it('should accept all operators', () => { + it('should accept all operators — each with the value shape it reads', () => { + // #7113: `value` is coupled to `operator`. This used to hand a scalar to + // ALL FIVE, which is precisely the shape the tightening refuses — `in` / + // `not_in` are membership tests and take the list. The list vocabulary is + // read from the schema's own export so a future operator cannot be added + // without being classified there. const operators = ['eq', 'neq', 'in', 'not_in', 'contains'] as const; + const listOperators = SKILL_TRIGGER_LIST_VALUE_OPERATORS as readonly string[]; operators.forEach(operator => { expect(() => SkillTriggerConditionSchema.parse({ field: 'objectName', operator, - value: 'support_case', + value: listOperators.includes(operator) ? ['support_case'] : 'support_case', })).not.toThrow(); }); }); diff --git a/packages/spec/src/ai/skill.zod.ts b/packages/spec/src/ai/skill.zod.ts index 501edb09ec..ef9a53ab7e 100644 --- a/packages/spec/src/ai/skill.zod.ts +++ b/packages/spec/src/ai/skill.zod.ts @@ -14,9 +14,182 @@ import { MetadataProtectionFields } from '../kernel/metadata-protection.zod'; * Defines programmatic conditions under which a skill becomes active. * Allows context-aware activation based on object type, user role, etc. */ +// ⚠️ Position matters: `build-docs.ts` takes the FIRST docblock in the file as +// this module's blurb in `content/docs/references/ai/skill.mdx`. Keep the module +// summary above the per-export docs below, or the reference page leads with +// whichever export happens to be declared first (#7113). import { lazySchema } from '../shared/lazy-schema'; import { strictObject } from '../shared/strict-object'; import { retiredKey } from '../shared/retired-key'; + +/** + * The trigger-condition operators whose `value` is a LIST rather than a scalar + * (#7113). + * + * These are the membership tests: `SkillRegistry.evaluateCondition` (cloud + * `packages/service-ai/src/skill-registry.ts`) answers them with + * `list.includes(fieldValue)`, so the authored `value` IS the list. Exported so + * a producer — a skill designer's condition editor, a generator, a test — + * asks the question the schema asks instead of hard-coding its own copy of the + * vocabulary. Same reasoning, and the same naming, as + * `VIEW_FILTER_LIST_VALUE_OPERATORS` (#6227, `ui/view.zod.ts`). + */ +export const SKILL_TRIGGER_LIST_VALUE_OPERATORS = ['in', 'not_in'] as const; + +/** + * The trigger-condition operators whose `value` is a SCALAR (#7113). + * + * `eq` / `neq` are answered by `fieldValue === expected` / `!==` in the cloud + * consumer. An array comparand there is not a stricter predicate, it is a DEAD + * one: JavaScript `===` on an array is reference identity, and the context + * value is never that same reference — so `eq` with an array is always false + * and `neq` with an array is always true, whatever the context holds. + * + * Deliberately does NOT include `contains` — see + * {@link checkSkillTriggerConditionValueShape}. + */ +export const SKILL_TRIGGER_SCALAR_VALUE_OPERATORS = ['eq', 'neq'] as const; + +/** `a string` / `an array of 3` / `null` … — the word the refusal uses. */ +function describeConditionValue(value: unknown): string { + if (value === null) return 'null'; + if (value === undefined) return 'no value'; + if (Array.isArray(value)) return `an array of ${value.length}`; + return `a ${typeof value}`; +} + +/** A short, bounded rendering of the offending value — a refusal, not a dump. */ +function previewConditionValue(value: unknown): string { + if (value === undefined) return '(omitted)'; + let text: string; + try { + text = JSON.stringify(value) ?? String(value); + } catch { + text = String(value); + } + return text.length > 40 ? `${text.slice(0, 39)}…` : text; +} + +/** + * [#7113] A trigger condition's `value` must have the shape its OPERATOR reads. + * + * ## What was wrong + * + * `operator` and `value` were declared independently — `z.enum([...])` beside + * `z.union([z.string(), z.array(z.string())])` — so every operator accepted + * every shape. `{ field: 'userRole', operator: 'in', value: 'admin' }` was a + * spec-valid skill trigger: a membership test whose list is not a list. This is + * the dormant twin of #6227 on `ViewFilterRuleSchema`, and the fix mirrors that + * one (PR #7114) key for key. + * + * ## Why it is the DORMANT twin — and why it is still worth closing + * + * #6227's shape genuinely fails at query time (`assertListComparandShapes`, + * 400 `INVALID_FILTER`), which made it a two-stage failure. This one does not + * fail at all: the sole consumer, + * `SkillRegistry.evaluateCondition` (cloud `packages/service-ai/src/skill-registry.ts`), + * coerces the scalar itself — + * + * ```ts + * case 'in': { + * const list = Array.isArray(expected) ? expected : [expected]; + * return list.includes(fieldValue as string); + * } + * ``` + * + * — so nothing 400s and the predicate evaluates the way the author meant. What + * is being closed is therefore not a break but a SECOND DIALECT: a + * consumer-side lenient coercion standing in for a contract the producer never + * declared, on a surface whose authors are increasingly AI-generated, where + * "declared = enforced" is what keeps generated metadata honest. + * + * That coercion becomes a **no-op** once this tightening ships: no + * schema-valid condition can reach it carrying a scalar on `in` / `not_in` + * again. Removing it is deliberately NOT part of this change — + * contract-first sequencing, the producer moves first — and is filed as a + * follow-up in the cloud repo. + * + * ## `contains` is deliberately left accepting BOTH shapes + * + * The check refuses exactly what the consumer cannot meaningfully execute, and + * nothing more (#5685: a schema stricter than its runtime is the wrong side of + * the fix — the same line #6227's pins hold). `contains` has TWO live branches + * in `evaluateCondition`, not one: + * + * - string context value + string comparand → substring test; + * - **array** context value + **array** comparand → `expected.every(v => fieldValue.includes(v))`, + * a real subset test. `SkillContext` is `{ [extraField: string]: unknown }`, + * so an array-valued context field is a shape the runtime is written for. + * + * Constraining `contains` to `z.string()` would therefore un-declare a working + * capability, which is a retirement decision (ADR-0049) and not a rider on a + * shape fix. `in` / `not_in` / `eq` / `neq` have no such branch — see the two + * exported vocabularies above. + * + * ## Why `superRefine` and not `z.discriminatedUnion` + * + * Same three reasons measured for the #6227 twin: a refinement adds no + * JSON-Schema structure (`ai/Skill` is a published def, and a union would fan + * one def into N branches re-declaring the same three keys), it emits ONE issue + * at path `['value']` naming the operator rather than blaming the + * discriminator for a defect in `value`, and in Zod 4 it lives inside the + * schema so `.shape` and the `ZodObject` class survive for every carrier — + * here `z.array(SkillTriggerConditionSchema)` on `Skill.triggerConditions`. + */ +function checkSkillTriggerConditionValueShape( + condition: { field?: unknown; operator?: unknown; value?: unknown }, + ctx: z.RefinementCtx, +): void { + const operator = condition.operator as string; + const value = condition.value; + const field = typeof condition.field === 'string' ? condition.field : ''; + + const isList = (SKILL_TRIGGER_LIST_VALUE_OPERATORS as readonly string[]).includes(operator); + const isScalar = (SKILL_TRIGGER_SCALAR_VALUE_OPERATORS as readonly string[]).includes(operator); + + if (isList) { + if (Array.isArray(value)) return; + ctx.addIssue({ + code: 'custom', + path: ['value'], + message: + `Operator "${operator}" on field "${field}" requires an ARRAY of values. ` + + `Received ${describeConditionValue(value)} (${previewConditionValue(value)}). ` + + `"${operator}" tests membership of a list — write ` + + `${value === undefined ? '["…"]' : previewConditionValue([value])} for a single value, ` + + `or use "${operator === 'in' ? 'eq' : 'neq'}" to compare against it. ` + + `An empty list [] is allowed and is a real predicate. The cloud agent runtime ` + + `coerces the scalar today; the contract never declared that spelling (#7113).`, + }); + return; + } + + if (!isScalar) return; + if (!Array.isArray(value)) return; + ctx.addIssue({ + code: 'custom', + path: ['value'], + message: + `Operator "${operator}" on field "${field}" requires a single STRING value. ` + + `Received ${describeConditionValue(value)} (${previewConditionValue(value)}). ` + + `"${operator}" is an identity comparison, so an array can never match a context ` + + `field and the condition would ${operator === 'eq' ? 'never' : 'always'} fire — ` + + `use "${operator === 'eq' ? 'in' : 'not_in'}" to test membership of that list (#7113).`, + }); +} + +/** + * A programmatic condition under which a skill becomes active — one + * `{ field, operator, value }` triple, ANDed with its siblings. + * + * `value`'s shape is coupled to `operator` (#7113, mirroring #6227): + * + * | operator | `value` must be | + * |---|---| + * | `in` / `not_in` ({@link SKILL_TRIGGER_LIST_VALUE_OPERATORS}) | an array, any length | + * | `eq` / `neq` ({@link SKILL_TRIGGER_SCALAR_VALUE_OPERATORS}) | a string | + * | `contains` | either — both spellings execute (see {@link checkSkillTriggerConditionValueShape}) | + */ export const SkillTriggerConditionSchema = lazySchema(() => z.object({ /** Condition field (e.g. 'objectName', 'userRole', 'channel') */ field: z.string().describe('Context field to evaluate'), @@ -24,9 +197,9 @@ export const SkillTriggerConditionSchema = lazySchema(() => z.object({ /** Comparison operator */ operator: z.enum(['eq', 'neq', 'in', 'not_in', 'contains']).describe('Comparison operator'), - /** Expected value(s) */ + /** Expected value(s) — an array for `in`/`not_in`, a string for `eq`/`neq` */ value: z.union([z.string(), z.array(z.string())]).describe('Expected value or values'), -})); +}).superRefine(checkSkillTriggerConditionValueShape)); export type SkillTriggerCondition = z.input;