|
| 1 | +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. |
| 2 | + |
| 3 | +/** |
| 4 | + * #8796 — the resume body's OUTER envelope is a closed set. |
| 5 | + * |
| 6 | + * `POST /automation/:name/runs/:runId/resume` assembles its engine signal |
| 7 | + * field-by-field from the body — deliberately (#3801: never spread the body, or |
| 8 | + * a caller could forge the service-authority marker). What was missing was the |
| 9 | + * other half: nothing told the caller that the keys it sent were not among the |
| 10 | + * ones read. Measured on GA: `{"nodeId":"ask","values":{…}}` — no key of which |
| 11 | + * the route reads — answered HTTP 200 `success:true` with the screen submission |
| 12 | + * treated as EMPTY; the run completed and the submitted value never reached the |
| 13 | + * flow. Maintainer ruling 2026-08-15 (Option A, on #8796): an unknown top-level |
| 14 | + * key is refused, located, naming the offending key(s) and the accepted set — |
| 15 | + * exactly `inputs` / `variables` / `output` / `branchLabel`. |
| 16 | + * |
| 17 | + * The refusal is thrown as the same duck-typed validation failure the toggle |
| 18 | + * arm's closed set throws; both HTTP error exits map it to `400 |
| 19 | + * VALIDATION_FAILED` + `details.fields[]` (#3918, |
| 20 | + * `dispatcher-validation-error.test.ts` pins that mapping end-to-end for both |
| 21 | + * exits). It is deliberately NOT `FLOW_FAILED`: the console treats 400 |
| 22 | + * `FLOW_FAILED` as terminal — the engine consumed the suspension and ran |
| 23 | + * (#8684, objectui PR #4899) — while this refusal never reaches the engine and |
| 24 | + * the suspension stays live, so the caller can retry with a corrected body. It |
| 25 | + * sits with `INVALID_SIGNAL` / `INVALID_SCREEN_INPUT` on the retryable side. |
| 26 | + * |
| 27 | + * Both halves of the closed-set policy are pinned here (Route & surface |
| 28 | + * ownership rule 5): the refusal pin includes THE SERVICE WAS NEVER CALLED, and |
| 29 | + * the preservation pin asserts the arguments the service actually received for |
| 30 | + * a body made only of accepted keys. |
| 31 | + */ |
| 32 | + |
| 33 | +import { describe, it, expect, vi } from 'vitest'; |
| 34 | + |
| 35 | +import { HttpDispatcher } from '../http-dispatcher.js'; |
| 36 | +import { validationFailureDetails } from '../validation-failure.js'; |
| 37 | + |
| 38 | +function makeDispatcher() { |
| 39 | + const spies = { |
| 40 | + resume: vi.fn(async () => ({ success: true, output: {}, durationMs: 7 })), |
| 41 | + }; |
| 42 | + const services: Record<string, unknown> = { automation: spies }; |
| 43 | + const resolve = (name: string) => services[name]; |
| 44 | + const kernel: any = { |
| 45 | + getService: resolve, |
| 46 | + getServiceAsync: async (name: string) => resolve(name), |
| 47 | + context: { getService: resolve }, |
| 48 | + }; |
| 49 | + return { dispatcher: new HttpDispatcher(kernel), spies }; |
| 50 | +} |
| 51 | + |
| 52 | +const CTX = { request: {}, executionContext: { userId: 'user_1' } } as any; |
| 53 | + |
| 54 | +const RESUME = '/flow_a/runs/run_1/resume'; |
| 55 | + |
| 56 | +async function expectRefusal(run: Promise<unknown>, label: string) { |
| 57 | + let thrown: unknown; |
| 58 | + try { |
| 59 | + await run; |
| 60 | + } catch (e) { |
| 61 | + thrown = e; |
| 62 | + } |
| 63 | + expect(thrown, `${label} was accepted instead of refused`).toBeDefined(); |
| 64 | + const details = validationFailureDetails(thrown); |
| 65 | + expect(details?.code).toBe('VALIDATION_FAILED'); |
| 66 | + expect(details?.fields.length).toBeGreaterThan(0); |
| 67 | + return thrown as Error; |
| 68 | +} |
| 69 | + |
| 70 | +describe('#8796 — the resume body refuses an unknown top-level key', () => { |
| 71 | + it('refuses the measured GA body — both keys named, engine never consulted', async () => { |
| 72 | + const { dispatcher, spies } = makeDispatcher(); |
| 73 | + const thrown = await expectRefusal( |
| 74 | + dispatcher.handleAutomation( |
| 75 | + RESUME, 'POST', |
| 76 | + { nodeId: 'ask', values: { resolution: 'submitted via the wrong key' } }, |
| 77 | + CTX, |
| 78 | + ), |
| 79 | + '{"nodeId":"ask","values":{…}}', |
| 80 | + ); |
| 81 | + // Located: every offending key is named, each as its own field entry. |
| 82 | + expect(validationFailureDetails(thrown)?.fields).toMatchObject([ |
| 83 | + { field: 'nodeId', code: 'unknown_field' }, |
| 84 | + { field: 'values', code: 'unknown_field' }, |
| 85 | + ]); |
| 86 | + // The message names the offending keys AND the accepted set, so a |
| 87 | + // guessed `values` is corrected at authoring time. |
| 88 | + expect(thrown.message).toMatch(/`nodeId`/); |
| 89 | + expect(thrown.message).toMatch(/`values`/); |
| 90 | + expect(thrown.message).toMatch(/`inputs`/); |
| 91 | + expect(thrown.message).toMatch(/`variables`/); |
| 92 | + expect(thrown.message).toMatch(/`output`/); |
| 93 | + expect(thrown.message).toMatch(/`branchLabel`/); |
| 94 | + // The refusal pin's load-bearing half: the request never reached the |
| 95 | + // engine, so the suspension was not consumed and a corrected retry is |
| 96 | + // legitimate. |
| 97 | + expect(spies.resume).not.toHaveBeenCalled(); |
| 98 | + }); |
| 99 | + |
| 100 | + it('refuses a single guessed key and stays off FLOW_FAILED — the retryable side', async () => { |
| 101 | + const { dispatcher, spies } = makeDispatcher(); |
| 102 | + const thrown = await expectRefusal( |
| 103 | + dispatcher.handleAutomation(RESUME, 'POST', { values: { x: 1 } }, CTX), |
| 104 | + '{"values":{…}}', |
| 105 | + ); |
| 106 | + // ⚠️ #8684 hazard pin: the console treats 400 FLOW_FAILED as terminal |
| 107 | + // (the wizard closes). This refusal leaves the suspension intact, so |
| 108 | + // it must never wear that code. |
| 109 | + expect((thrown as any).code).toBe('VALIDATION_FAILED'); |
| 110 | + expect((thrown as any).code).not.toBe('FLOW_FAILED'); |
| 111 | + expect(validationFailureDetails(thrown)?.fields).toMatchObject([{ field: 'values' }]); |
| 112 | + expect(spies.resume).not.toHaveBeenCalled(); |
| 113 | + }); |
| 114 | + |
| 115 | + it('refuses a half-wrong body — a valid `inputs` does not buy an unknown sibling through', async () => { |
| 116 | + // Option B (refuse only when NOTHING is recognized) was explicitly |
| 117 | + // declined: the half-wrong body must not be silently half-dropped. |
| 118 | + const { dispatcher, spies } = makeDispatcher(); |
| 119 | + const thrown = await expectRefusal( |
| 120 | + dispatcher.handleAutomation( |
| 121 | + RESUME, 'POST', |
| 122 | + { inputs: { resolution: 'real value' }, values: { resolution: 'stray' } }, |
| 123 | + CTX, |
| 124 | + ), |
| 125 | + '{"inputs":{…},"values":{…}}', |
| 126 | + ); |
| 127 | + expect(validationFailureDetails(thrown)?.fields).toMatchObject([{ field: 'values' }]); |
| 128 | + // Ordering (#3899): the envelope refusal precedes every engine verdict |
| 129 | + // — permission, signal, screen-input — because the engine is never |
| 130 | + // consulted on an illegal body. |
| 131 | + expect(spies.resume).not.toHaveBeenCalled(); |
| 132 | + }); |
| 133 | + |
| 134 | + it('refuses a non-empty array body — indices are not accepted keys', async () => { |
| 135 | + const { dispatcher, spies } = makeDispatcher(); |
| 136 | + await expectRefusal( |
| 137 | + dispatcher.handleAutomation(RESUME, 'POST', [{ inputs: {} }], CTX), |
| 138 | + '[{"inputs":{}}]', |
| 139 | + ); |
| 140 | + expect(spies.resume).not.toHaveBeenCalled(); |
| 141 | + }); |
| 142 | + |
| 143 | + it('escapes dispatch() as the recognized validation-failure shape', async () => { |
| 144 | + // dispatch()'s catch rethrows non-permission errors on purpose — the |
| 145 | + // HTTP envelope is built by the error exits #3918 pinned |
| 146 | + // (`dispatcher-validation-error.test.ts`, both exits: 400, |
| 147 | + // `error.code: 'VALIDATION_FAILED'`, `details.fields[]`). This pins |
| 148 | + // that the refusal arrives at those exits as the shape they recognise, |
| 149 | + // with the service still never called. |
| 150 | + const { dispatcher, spies } = makeDispatcher(); |
| 151 | + (dispatcher as any).timedResolveExecutionContext = async () => ({ userId: 'user_1' }); |
| 152 | + let thrown: unknown; |
| 153 | + try { |
| 154 | + await dispatcher.dispatch( |
| 155 | + 'POST', '/automation/flow_a/runs/run_1/resume', |
| 156 | + { nodeId: 'ask', values: {} }, {}, {} as any, |
| 157 | + ); |
| 158 | + } catch (e) { |
| 159 | + thrown = e; |
| 160 | + } |
| 161 | + expect(thrown, 'the refusal must reach the HTTP error exits').toBeDefined(); |
| 162 | + expect(validationFailureDetails(thrown)?.code).toBe('VALIDATION_FAILED'); |
| 163 | + expect(spies.resume).not.toHaveBeenCalled(); |
| 164 | + }); |
| 165 | +}); |
| 166 | + |
| 167 | +describe('#8796 — a body made only of accepted keys is unaffected', () => { |
| 168 | + it('forwards all four accepted keys exactly as before', async () => { |
| 169 | + const { dispatcher, spies } = makeDispatcher(); |
| 170 | + const result = await dispatcher.handleAutomation( |
| 171 | + RESUME, 'POST', |
| 172 | + { |
| 173 | + inputs: { new_assignee: 'ada' }, |
| 174 | + output: { comment: 'ok' }, |
| 175 | + branchLabel: 'approve', |
| 176 | + }, |
| 177 | + CTX, |
| 178 | + ); |
| 179 | + expect(result.response?.status).toBe(200); |
| 180 | + // The preservation pin: the arguments the service actually received — |
| 181 | + // #3801's field-by-field assembly, byte-for-byte what it always sent. |
| 182 | + expect(spies.resume).toHaveBeenCalledWith('run_1', { |
| 183 | + variables: { new_assignee: 'ada' }, |
| 184 | + output: { comment: 'ok' }, |
| 185 | + branchLabel: 'approve', |
| 186 | + }); |
| 187 | + }); |
| 188 | + |
| 189 | + it('keeps the `variables` alias working', async () => { |
| 190 | + const { dispatcher, spies } = makeDispatcher(); |
| 191 | + const result = await dispatcher.handleAutomation( |
| 192 | + RESUME, 'POST', { variables: { note: 'hi' } }, CTX, |
| 193 | + ); |
| 194 | + expect(result.response?.status).toBe(200); |
| 195 | + expect(spies.resume).toHaveBeenCalledWith('run_1', { variables: { note: 'hi' } }); |
| 196 | + }); |
| 197 | + |
| 198 | + it.each([ |
| 199 | + ['empty object', {}], |
| 200 | + ['undefined body', undefined], |
| 201 | + ['null body', null], |
| 202 | + ])('still accepts %s as an empty submission', async (_label, body) => { |
| 203 | + // An empty submission is a legal one (a screen whose declared fields |
| 204 | + // are all optional) — the closed set refuses unknown KEYS, it does not |
| 205 | + // demand keys exist. |
| 206 | + const { dispatcher, spies } = makeDispatcher(); |
| 207 | + const result = await dispatcher.handleAutomation(RESUME, 'POST', body, CTX); |
| 208 | + expect(result.response?.status).toBe(200); |
| 209 | + expect(spies.resume).toHaveBeenCalledWith('run_1', {}); |
| 210 | + }); |
| 211 | +}); |
0 commit comments