|
| 1 | +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. |
| 2 | + |
| 3 | +/** |
| 4 | + * [#7113] `SkillTriggerConditionSchema.value` is shaped by the condition's |
| 5 | + * OPERATOR — the dormant twin of #6227 (`ViewFilterRuleSchema`, PR #7114). |
| 6 | + * |
| 7 | + * "Dormant" is the whole difference and these pins are written around it. The |
| 8 | + * #6227 shape genuinely failed at query time; this one never failed at all — |
| 9 | + * the sole consumer (`SkillRegistry.evaluateCondition`, cloud |
| 10 | + * `packages/service-ai/src/skill-registry.ts`) coerces the scalar with |
| 11 | + * `Array.isArray(expected) ? expected : [expected]`. So what these pins hold is |
| 12 | + * not a break-fix but the contract-first property: the producer declares the |
| 13 | + * one spelling instead of letting a consumer quietly accept two. |
| 14 | + * |
| 15 | + * Every rejection pin asserts the issue CODE and PATH, not merely that a throw |
| 16 | + * happened: a bare `.toThrow()` cannot tell "refused for the right reason at |
| 17 | + * the right key" from "refused because the value union rejected the type", and |
| 18 | + * those are different defects (#6142). |
| 19 | + * |
| 20 | + * The accept pins matter as much as the reject pins. `contains` keeps BOTH |
| 21 | + * spellings on purpose — the consumer has a live array⊆array branch for it — |
| 22 | + * and #5685 rules that a schema stricter than its runtime is the wrong side of |
| 23 | + * the fix. A pin that only checked rejections would let that regress silently. |
| 24 | + */ |
| 25 | + |
| 26 | +import { describe, expect, it } from 'vitest'; |
| 27 | +import { |
| 28 | + SKILL_TRIGGER_LIST_VALUE_OPERATORS, |
| 29 | + SKILL_TRIGGER_SCALAR_VALUE_OPERATORS, |
| 30 | + SkillSchema, |
| 31 | + SkillTriggerConditionSchema, |
| 32 | +} from './skill.zod'; |
| 33 | + |
| 34 | +/** Parse helper — the authored object form, exactly as a skill carries it. */ |
| 35 | +const parse = (condition: Record<string, unknown>) => |
| 36 | + SkillTriggerConditionSchema.safeParse(condition); |
| 37 | + |
| 38 | +/** The single `value`-path issue a shape refusal must produce. */ |
| 39 | +function valueIssue(result: ReturnType<typeof parse>) { |
| 40 | + expect(result.success).toBe(false); |
| 41 | + if (result.success) throw new Error('unreachable'); |
| 42 | + const issues = result.error.issues.filter((i) => i.path.join('.') === 'value'); |
| 43 | + expect(issues).toHaveLength(1); |
| 44 | + return issues[0]!; |
| 45 | +} |
| 46 | + |
| 47 | +describe('#7113 — the reported shape is refused at authoring time', () => { |
| 48 | + it('refuses the card example: a set operator carrying a scalar', () => { |
| 49 | + const result = parse({ field: 'userRole', operator: 'in', value: 'admin' }); |
| 50 | + const issue = valueIssue(result); |
| 51 | + |
| 52 | + expect(issue.code).toBe('custom'); |
| 53 | + expect(issue.path).toEqual(['value']); |
| 54 | + expect(issue.message).toContain( |
| 55 | + 'Operator "in" on field "userRole" requires an ARRAY of values.', |
| 56 | + ); |
| 57 | + // The refusal carries what the author has to DO, not just what is wrong. |
| 58 | + expect(issue.message).toContain('Received a string ("admin")'); |
| 59 | + expect(issue.message).toContain('write ["admin"] for a single value'); |
| 60 | + expect(issue.message).toContain('or use "eq" to compare against it'); |
| 61 | + // And it says the empty list is NOT what is being refused. |
| 62 | + expect(issue.message).toContain('An empty list [] is allowed'); |
| 63 | + }); |
| 64 | + |
| 65 | + it('names the consumer-side coercion as the thing being replaced', () => { |
| 66 | + const issue = valueIssue(parse({ field: 'userRole', operator: 'not_in', value: 'admin' })); |
| 67 | + expect(issue.message).toContain('coerces the scalar today'); |
| 68 | + expect(issue.message).toContain('#7113'); |
| 69 | + }); |
| 70 | +}); |
| 71 | + |
| 72 | +describe('#7113 — list operators require an array', () => { |
| 73 | + it.each(SKILL_TRIGGER_LIST_VALUE_OPERATORS)('%s refuses a scalar', (operator) => { |
| 74 | + const issue = valueIssue(parse({ field: 'objectName', operator, value: 'lead' })); |
| 75 | + expect(issue.code).toBe('custom'); |
| 76 | + expect(issue.path).toEqual(['value']); |
| 77 | + expect(issue.message).toContain(`Operator "${operator}"`); |
| 78 | + expect(issue.message).toContain('requires an ARRAY of values'); |
| 79 | + }); |
| 80 | + |
| 81 | + it.each(SKILL_TRIGGER_LIST_VALUE_OPERATORS)('%s accepts an array', (operator) => { |
| 82 | + const result = parse({ field: 'objectName', operator, value: ['lead', 'opportunity'] }); |
| 83 | + expect(result.success).toBe(true); |
| 84 | + }); |
| 85 | + |
| 86 | + it.each(SKILL_TRIGGER_LIST_VALUE_OPERATORS)( |
| 87 | + '%s accepts an EMPTY array — it is a real predicate, not the defect', |
| 88 | + (operator) => { |
| 89 | + expect(parse({ field: 'objectName', operator, value: [] }).success).toBe(true); |
| 90 | + }, |
| 91 | + ); |
| 92 | + |
| 93 | + it('refuses a missing value with ONE issue — the required check, not two', () => { |
| 94 | + // Measured, not assumed: Zod 4 skips a `superRefine` when the object's own |
| 95 | + // shape already failed, so an omitted `value` reports only the required |
| 96 | + // issue. Pinned because the refinement's "no value" wording exists for the |
| 97 | + // case where a future carrier makes `value` optional — this records that |
| 98 | + // today it is unreachable, rather than leaving a reader to guess that a |
| 99 | + // missing value produces two competing complaints at one key. |
| 100 | + const result = parse({ field: 'objectName', operator: 'in' }); |
| 101 | + expect(result.success).toBe(false); |
| 102 | + if (result.success) throw new Error('unreachable'); |
| 103 | + const atValue = result.error.issues.filter((i) => i.path.join('.') === 'value'); |
| 104 | + expect(atValue).toHaveLength(1); |
| 105 | + expect(atValue[0]!.code).not.toBe('custom'); |
| 106 | + }); |
| 107 | +}); |
| 108 | + |
| 109 | +describe('#7113 — identity operators require a string', () => { |
| 110 | + it.each(SKILL_TRIGGER_SCALAR_VALUE_OPERATORS)('%s refuses an array', (operator) => { |
| 111 | + const issue = valueIssue(parse({ field: 'objectName', operator, value: ['lead'] })); |
| 112 | + expect(issue.code).toBe('custom'); |
| 113 | + expect(issue.path).toEqual(['value']); |
| 114 | + expect(issue.message).toContain(`Operator "${operator}"`); |
| 115 | + expect(issue.message).toContain('requires a single STRING value'); |
| 116 | + // The message must explain the DEAD-predicate mechanism, since nothing |
| 117 | + // errors today — an author has no runtime symptom to reason from. |
| 118 | + expect(issue.message).toContain(operator === 'eq' ? 'never fire' : 'always fire'); |
| 119 | + expect(issue.message).toContain(operator === 'eq' ? 'use "in"' : 'use "not_in"'); |
| 120 | + }); |
| 121 | + |
| 122 | + it.each(SKILL_TRIGGER_SCALAR_VALUE_OPERATORS)('%s accepts a string', (operator) => { |
| 123 | + expect(parse({ field: 'objectName', operator, value: 'lead' }).success).toBe(true); |
| 124 | + }); |
| 125 | +}); |
| 126 | + |
| 127 | +describe('#7113 — `contains` keeps BOTH shapes (#5685: no stricter than the runtime)', () => { |
| 128 | + it('accepts a string comparand — the substring branch', () => { |
| 129 | + expect(parse({ field: 'viewName', operator: 'contains', value: 'kanban' }).success).toBe(true); |
| 130 | + }); |
| 131 | + |
| 132 | + it('accepts an array comparand — the live array⊆array subset branch', () => { |
| 133 | + // `evaluateCondition`: `expected.every(v => fieldValue.includes(v))` when the |
| 134 | + // context field is an array. `SkillContext` is indexed `[k: string]: unknown`, |
| 135 | + // so that is a shape the cloud runtime is deliberately written for. |
| 136 | + // Refusing it here would un-declare a working capability (an ADR-0049 |
| 137 | + // retirement decision), not tighten a contract. |
| 138 | + expect(parse({ field: 'tags', operator: 'contains', value: ['a', 'b'] }).success).toBe(true); |
| 139 | + }); |
| 140 | + |
| 141 | + it('is in neither constrained vocabulary', () => { |
| 142 | + expect(SKILL_TRIGGER_LIST_VALUE_OPERATORS).not.toContain('contains'); |
| 143 | + expect(SKILL_TRIGGER_SCALAR_VALUE_OPERATORS).not.toContain('contains'); |
| 144 | + }); |
| 145 | +}); |
| 146 | + |
| 147 | +describe('#7113 — the exported vocabularies are the contract, not a copy', () => { |
| 148 | + it('the two vocabularies are disjoint and both subsets of the operator enum', () => { |
| 149 | + const all = [ |
| 150 | + ...SKILL_TRIGGER_LIST_VALUE_OPERATORS, |
| 151 | + ...SKILL_TRIGGER_SCALAR_VALUE_OPERATORS, |
| 152 | + ]; |
| 153 | + expect(new Set(all).size).toBe(all.length); |
| 154 | + for (const operator of all) { |
| 155 | + // Every declared member must actually be an operator the schema accepts. |
| 156 | + expect(parse({ |
| 157 | + field: 'f', |
| 158 | + operator, |
| 159 | + value: (SKILL_TRIGGER_LIST_VALUE_OPERATORS as readonly string[]).includes(operator) |
| 160 | + ? ['x'] |
| 161 | + : 'x', |
| 162 | + }).success).toBe(true); |
| 163 | + } |
| 164 | + }); |
| 165 | + |
| 166 | + it('pins the membership so a future operator has to be classified', () => { |
| 167 | + expect([...SKILL_TRIGGER_LIST_VALUE_OPERATORS]).toEqual(['in', 'not_in']); |
| 168 | + expect([...SKILL_TRIGGER_SCALAR_VALUE_OPERATORS]).toEqual(['eq', 'neq']); |
| 169 | + }); |
| 170 | +}); |
| 171 | + |
| 172 | +describe('#7113 — the refinement does not disturb the carrier', () => { |
| 173 | + it('an unrelated operator/value pair still parses through Skill.triggerConditions', () => { |
| 174 | + const skill = SkillSchema.parse({ |
| 175 | + name: 'order_management', |
| 176 | + label: 'Order Management', |
| 177 | + instructions: 'Manage orders.', |
| 178 | + tools: ['create_order'], |
| 179 | + triggerConditions: [ |
| 180 | + { field: 'objectName', operator: 'eq', value: 'order' }, |
| 181 | + { field: 'userRole', operator: 'in', value: ['sales', 'support'] }, |
| 182 | + ], |
| 183 | + }); |
| 184 | + expect(skill.triggerConditions).toHaveLength(2); |
| 185 | + }); |
| 186 | + |
| 187 | + it('a bad condition inside a skill reports at the nested value path', () => { |
| 188 | + // The path prefix proves the refinement travels with the carrier rather |
| 189 | + // than only firing on a standalone parse. |
| 190 | + const result = SkillSchema.safeParse({ |
| 191 | + name: 'order_management', |
| 192 | + label: 'Order Management', |
| 193 | + instructions: 'Manage orders.', |
| 194 | + tools: ['create_order'], |
| 195 | + triggerConditions: [{ field: 'userRole', operator: 'in', value: 'admin' }], |
| 196 | + }); |
| 197 | + expect(result.success).toBe(false); |
| 198 | + if (result.success) throw new Error('unreachable'); |
| 199 | + const issue = result.error.issues.find( |
| 200 | + (i) => i.path.join('.') === 'triggerConditions.0.value', |
| 201 | + ); |
| 202 | + expect(issue).toBeDefined(); |
| 203 | + expect(issue!.code).toBe('custom'); |
| 204 | + }); |
| 205 | +}); |
0 commit comments