|
| 1 | +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. |
| 2 | + |
| 3 | +/** |
| 4 | + * #7428 — the authoring-time guard on the `confirmText` + `params` PAIR. |
| 5 | + * |
| 6 | + * #7278 and #7309 repaired the 16 shipped sites that opened two dialogs for one |
| 7 | + * decision (PRs #7592 and #7827). Repairing instances does not stop the next one |
| 8 | + * being written; this refusal is the structural half, and it is the reason the |
| 9 | + * card exists as a separate issue from either migration. |
| 10 | + * |
| 11 | + * **What these tests pin is the BOUNDARY, not just the refusal.** The pair is |
| 12 | + * wrong on `ActionSchema` and CORRECT on `BulkActionDefSchema` — measured on |
| 13 | + * #7428 (2026-08-11), and the distinction is the schema the def is validated by, |
| 14 | + * not a heuristic about whether the params happen to be optional. A guard |
| 15 | + * written against the raw `confirmText` + `params: [` key pair would land red on |
| 16 | + * four correct `examples/app-showcase` bulk defs on day one, which is the |
| 17 | + * permanently-noisy-check shape the card was filed to avoid. So the acceptance |
| 18 | + * direction is pinned as hard as the rejection direction: a future "helpful" |
| 19 | + * widening of this guard onto the bulk surface goes RED here. |
| 20 | + * |
| 21 | + * The rejection tests assert the issue PATH and the MESSAGE SUBSTANCE rather |
| 22 | + * than a bare `success === false`. One condition, one wording: a refusal whose |
| 23 | + * message does not name both keys and point at the remedy sends the author |
| 24 | + * looking for a different bug. |
| 25 | + */ |
| 26 | + |
| 27 | +import { describe, expect, it } from 'vitest'; |
| 28 | +import { ActionSchema, InlineActionSchema, defineAction } from './action.zod'; |
| 29 | +import { BulkActionDefSchema } from './bulk-action.zod'; |
| 30 | + |
| 31 | +/** |
| 32 | + * Minimum legal registered action. `type` defaults to `script`, whose own |
| 33 | + * refinement requires an inline `body` or a `target` naming a bundle function — |
| 34 | + * leaving both off would fail for a reason that has nothing to do with this |
| 35 | + * guard and would make every assertion below unreadable. |
| 36 | + */ |
| 37 | +const base = { name: 'approval_reject', label: 'Reject', target: 'rejectApproval' } as const; |
| 38 | + |
| 39 | +// `type` is narrowed rather than widened to `string` so this literal is also |
| 40 | +// assignable to `defineAction`'s typed input, which the last test below calls. |
| 41 | +const oneParam = [{ name: 'reason', label: 'Reason', type: 'textarea' as const, required: true }]; |
| 42 | + |
| 43 | +/** The single issue this guard raises, or `undefined` if it did not fire. */ |
| 44 | +const guardIssue = (result: ReturnType<typeof ActionSchema.safeParse>) => |
| 45 | + result.success |
| 46 | + ? undefined |
| 47 | + : result.error.issues.find((i) => i.path.join('.') === 'confirmText'); |
| 48 | + |
| 49 | +describe('#7428 — `confirmText` + non-empty `params` is refused on ActionSchema', () => { |
| 50 | + it('refuses the pair, at the `confirmText` path', () => { |
| 51 | + const result = ActionSchema.safeParse({ |
| 52 | + ...base, |
| 53 | + confirmText: 'Reject this request?', |
| 54 | + params: oneParam, |
| 55 | + }); |
| 56 | + |
| 57 | + expect(result.success).toBe(false); |
| 58 | + // The path is asserted because it is what an editor/CLI underlines. Pointing |
| 59 | + // at `params` would tell the author to delete the inputs they need; the key |
| 60 | + // that has to go is `confirmText`. |
| 61 | + expect(guardIssue(result)?.path).toEqual(['confirmText']); |
| 62 | + }); |
| 63 | + |
| 64 | + it('says WHY, naming both keys and the remedy — not a bare rejection', () => { |
| 65 | + const result = ActionSchema.safeParse({ |
| 66 | + ...base, |
| 67 | + confirmText: 'Reject this request?', |
| 68 | + params: oneParam, |
| 69 | + }); |
| 70 | + const message = guardIssue(result)?.message ?? ''; |
| 71 | + |
| 72 | + // Both halves of the offending pair, so the author can see what collided. |
| 73 | + expect(message).toContain('`confirmText`'); |
| 74 | + expect(message).toContain('`params`'); |
| 75 | + // The consequence, in user-visible terms rather than schema terms. |
| 76 | + expect(message).toContain('TWO dialogs'); |
| 77 | + // The remedy, which is the whole point of the #7278 ruling. |
| 78 | + expect(message).toContain('`description`'); |
| 79 | + // …and the remedy's own trap: the LLM-facing key one level down is NOT it. |
| 80 | + expect(message).toContain('ai.description'); |
| 81 | + // The exception that keeps `confirmText` a live key rather than a retired |
| 82 | + // one — an author who reads only this message must not conclude otherwise. |
| 83 | + expect(message).toContain('param-LESS'); |
| 84 | + }); |
| 85 | + |
| 86 | + it('fires on the localized-map form of `confirmText` too, not just a string', () => { |
| 87 | + // `confirmText` is `I18nLabelSchema`, so a bare truthiness check written |
| 88 | + // against a string would miss the map form and let the defect back in |
| 89 | + // through the localized door. |
| 90 | + const result = ActionSchema.safeParse({ |
| 91 | + ...base, |
| 92 | + confirmText: { en: 'Reject this request?', 'zh-CN': '拒绝该请求？' }, |
| 93 | + params: oneParam, |
| 94 | + }); |
| 95 | + |
| 96 | + expect(result.success).toBe(false); |
| 97 | + expect(guardIssue(result)?.path).toEqual(['confirmText']); |
| 98 | + }); |
| 99 | + |
| 100 | + it('cannot be smuggled in through an ALIAS spelling — that door is shut upstream', () => { |
| 101 | + // `confirm` → `confirmText` and `inputs` → `params` are declared aliases on |
| 102 | + // this surface, and this repo REJECTS a near-miss with a rename arrow rather |
| 103 | + // than folding it silently (Prime Directive #12 — one contract, no dialects). |
| 104 | + // So the aliased pair never reaches this refinement at all: it is refused one |
| 105 | + // layer earlier, by key recognition. Pinned because the guard's coverage claim |
| 106 | + // depends on it — if aliases ever became a silent fold, the pair would arrive |
| 107 | + // post-fold and this test is where that change gets noticed. |
| 108 | + const result = ActionSchema.safeParse({ |
| 109 | + ...base, |
| 110 | + confirm: 'Reject this request?', |
| 111 | + inputs: oneParam, |
| 112 | + }); |
| 113 | + |
| 114 | + expect(result.success).toBe(false); |
| 115 | + const issue = result.success ? undefined : result.error.issues[0]; |
| 116 | + expect(issue?.code).toBe('unrecognized_keys'); |
| 117 | + expect(issue?.message).toContain('Did you mean `confirm` → `confirmText`'); |
| 118 | + expect(issue?.message).toContain('`inputs` → `params`'); |
| 119 | + }); |
| 120 | + |
| 121 | + it('throws from `defineAction`, which is where an author meets it', () => { |
| 122 | + // A refusal is only worth having if it reaches the authoring call site — |
| 123 | + // `defineAction` is what the platform objects and every app actually call. |
| 124 | + expect(() => |
| 125 | + defineAction({ ...base, confirmText: 'Reject this request?', params: oneParam }), |
| 126 | + ).toThrow(/TWO dialogs/); |
| 127 | + }); |
| 128 | +}); |
| 129 | + |
| 130 | +describe('#7428 — what the guard must NOT touch', () => { |
| 131 | + it('accepts `confirmText` on a param-LESS action — the confirm is the only dialog', () => { |
| 132 | + const result = ActionSchema.safeParse({ |
| 133 | + ...base, |
| 134 | + confirmText: 'Reject this request?', |
| 135 | + }); |
| 136 | + |
| 137 | + expect(result.success).toBe(true); |
| 138 | + }); |
| 139 | + |
| 140 | + it('accepts `confirmText` beside an EMPTY `params` array', () => { |
| 141 | + // An empty array collects nothing, so no second dialog opens. Refusing it |
| 142 | + // would be a refusal with no user-visible defect behind it. |
| 143 | + const result = ActionSchema.safeParse({ ...base, confirmText: 'Sure?', params: [] }); |
| 144 | + |
| 145 | + expect(result.success).toBe(true); |
| 146 | + }); |
| 147 | + |
| 148 | + it('accepts `params` + `description` — the shape #7278 migrated TO', () => { |
| 149 | + // If this ever goes red the guard has swallowed its own remedy and the two |
| 150 | + // migrations have nowhere to land. |
| 151 | + const result = ActionSchema.safeParse({ |
| 152 | + ...base, |
| 153 | + description: 'Reject this request? Say why — the requester sees it.', |
| 154 | + params: oneParam, |
| 155 | + }); |
| 156 | + |
| 157 | + expect(result.success).toBe(true); |
| 158 | + }); |
| 159 | + |
| 160 | + it('does NOT additionally require `description` when `params` is present', () => { |
| 161 | + // Deliberately not widened (#7428 ruling 3): forbidding the pair is the |
| 162 | + // narrowest guard with measured pull behind it. Requiring dialog copy on |
| 163 | + // every param-collecting action is a strictly bigger authoring demand with |
| 164 | + // no measured failure behind it — it would need its own card. |
| 165 | + const result = ActionSchema.safeParse({ ...base, params: oneParam }); |
| 166 | + |
| 167 | + expect(result.success).toBe(true); |
| 168 | + }); |
| 169 | +}); |
| 170 | + |
| 171 | +describe('#7428 — the guard is scoped to ActionSchema by SCHEMA BOUNDARY', () => { |
| 172 | + it('BulkActionDefSchema still ACCEPTS `confirmText` + non-empty `params`', () => { |
| 173 | + // The pinning test the boundary ruling asks for. This pairing is INTENDED |
| 174 | + // on the bulk surface: per that schema's own describe() text the params are |
| 175 | + // "inputs collected once before the run", `confirmText` is shown "above the |
| 176 | + // affected-record summary", and a `required` param "blocks the Confirm |
| 177 | + // button until a value is present" — one dialog, so there is no second one |
| 178 | + // to collapse. `examples/app-showcase`'s four defs are this shape and are |
| 179 | + // correct as written. A widening of the guard onto this schema lands here. |
| 180 | + const result = BulkActionDefSchema.safeParse({ |
| 181 | + name: 'set_labels', |
| 182 | + label: 'Set Labels', |
| 183 | + operation: 'update', |
| 184 | + confirmText: 'Set these labels on every selected project?', |
| 185 | + params: [ |
| 186 | + { |
| 187 | + name: 'labels', |
| 188 | + label: 'Labels', |
| 189 | + type: 'select', |
| 190 | + multiple: true, |
| 191 | + required: true, |
| 192 | + options: [{ label: 'Frontend', value: 'frontend' }], |
| 193 | + }, |
| 194 | + ], |
| 195 | + }); |
| 196 | + |
| 197 | + expect(result.success).toBe(true); |
| 198 | + expect(result.success && result.data.confirmText) |
| 199 | + .toBe('Set these labels on every selected project?'); |
| 200 | + expect(result.success && result.data.params?.length).toBe(1); |
| 201 | + }); |
| 202 | + |
| 203 | + it('the two schemas are independent — the bulk def is not validated by ActionSchema', () => { |
| 204 | + // The structural claim behind the pin above, asserted rather than assumed: |
| 205 | + // `BulkActionDefSchema` is its own `strictObject`, so the same literal is |
| 206 | + // not even a legal ACTION (no `operation` key on that surface). If the two |
| 207 | + // were ever unified, this goes red before the guard silently widens. |
| 208 | + const asAction = ActionSchema.safeParse({ |
| 209 | + name: 'set_labels', |
| 210 | + label: 'Set Labels', |
| 211 | + operation: 'update', |
| 212 | + confirmText: 'Set these labels on every selected project?', |
| 213 | + params: [{ name: 'labels', label: 'Labels', type: 'select' }], |
| 214 | + }); |
| 215 | + |
| 216 | + expect(asAction.success).toBe(false); |
| 217 | + }); |
| 218 | + |
| 219 | + it('InlineActionSchema is out of reach too — it picks fields, not this refine chain', () => { |
| 220 | + // Recording the guard's real blast radius rather than assuming it. Inline |
| 221 | + // actions derive from the shared field factory via `.pick()`, so no |
| 222 | + // refinement on `ActionSchema` reaches them — and the pick deliberately |
| 223 | + // omits `description`, so the #7278 remedy has no slot on that surface to |
| 224 | + // move a question into. Same reason the bulk defs were struck from the |
| 225 | + // target set: a refusal whose remedy is unreachable is a dead end, not a |
| 226 | + // guard. Whether the inline surface should gain `description` FIRST and the |
| 227 | + // guard SECOND is left open on #7428 rather than presumed here. |
| 228 | + const result = InlineActionSchema.safeParse({ |
| 229 | + type: 'url', |
| 230 | + target: '/approvals?reject=1', |
| 231 | + label: 'Reject', |
| 232 | + confirmText: 'Reject this request?', |
| 233 | + params: [{ name: 'reason', label: 'Reason', type: 'textarea' }], |
| 234 | + }); |
| 235 | + |
| 236 | + expect(result.success).toBe(true); |
| 237 | + }); |
| 238 | +}); |
0 commit comments