|
| 1 | +/** |
| 2 | + * ObjectUI |
| 3 | + * Copyright (c) 2024-present ObjectStack Inc. |
| 4 | + * |
| 5 | + * objectstack#3913 ratchet — every `/api/v1/actions` caller goes through |
| 6 | + * `interpretActionResponse`. |
| 7 | + * |
| 8 | + * The bug this guards: the `/actions` response wraps TWICE (the route's own |
| 9 | + * `{success, data}` inside the dispatcher's), and a failure has three shapes |
| 10 | + * only one of which `res.ok` catches. Two copies of that rule existed — the |
| 11 | + * shared console runtime and RecordDetailView's — and they drifted: one learned |
| 12 | + * to inspect the inner envelope, the other did not. The one that did not was |
| 13 | + * the console's MAIN action path, so a failed action fired a green "completed" |
| 14 | + * toast on every list and page surface while the real error was swallowed. |
| 15 | + * `marketplaceApi.installPackage` had the same hole and could report a package |
| 16 | + * as installed when it was not. |
| 17 | + * |
| 18 | + * Reading the envelope by hand is the anti-pattern, not any particular way of |
| 19 | + * reading it wrong: four hand-rolled copies produced three different bugs |
| 20 | + * (missed inner failure, a `{message}` object handed to `toast.error()` as a |
| 21 | + * React child → React #31, and `redirectUrl` read one level too shallow so it |
| 22 | + * never fired). One helper, one rule. |
| 23 | + * |
| 24 | + * If this fails: don't hand-roll the check. Import `interpretActionResponse` |
| 25 | + * from `utils/actionResponse` — or, better, call the action through |
| 26 | + * `@objectstack/client`, which folds every shape into `{ success, data?, error? }`. |
| 27 | + */ |
| 28 | + |
| 29 | +import { describe, it, expect } from 'vitest'; |
| 30 | +import { readdirSync, readFileSync, statSync } from 'node:fs'; |
| 31 | +import path from 'node:path'; |
| 32 | +import { fileURLToPath } from 'node:url'; |
| 33 | + |
| 34 | +const here = path.dirname(fileURLToPath(import.meta.url)); |
| 35 | +const appShellSrc = here; |
| 36 | + |
| 37 | +/** The helper that owns the rule — exempt from its own guard. */ |
| 38 | +const OWNER = path.join(appShellSrc, 'utils', 'actionResponse.ts'); |
| 39 | + |
| 40 | +/** Files allowed to name the route without going through the helper. */ |
| 41 | +const EXEMPT = new Set<string>([ |
| 42 | + OWNER, |
| 43 | + // The cloud marketplace install posts to a *cloud* origin's action route |
| 44 | + // and consumes `InstallResponse`, not `ActionResult`. It still has to |
| 45 | + // honour the envelope, and does — covered by its own tests — but it is not |
| 46 | + // an ActionRunner handler, so it does not use this helper. |
| 47 | + path.join(appShellSrc, 'console', 'marketplace', 'marketplaceApi.ts'), |
| 48 | +]); |
| 49 | + |
| 50 | +function walk(dir: string, out: string[] = []): string[] { |
| 51 | + for (const entry of readdirSync(dir)) { |
| 52 | + if (entry === 'node_modules' || entry === 'dist' || entry === '__tests__') continue; |
| 53 | + const full = path.join(dir, entry); |
| 54 | + if (statSync(full).isDirectory()) walk(full, out); |
| 55 | + else if (/\.(ts|tsx)$/.test(entry) && !/\.test\.tsx?$/.test(entry)) out.push(full); |
| 56 | + } |
| 57 | + return out; |
| 58 | +} |
| 59 | + |
| 60 | +/** A source file that POSTs to the action route. */ |
| 61 | +const ROUTE = /\/api\/v1\/actions\//; |
| 62 | + |
| 63 | +/** |
| 64 | + * Comments name the route freely while explaining the routing rules (e.g. |
| 65 | + * ConsoleShell's note on why `modal` stays client-side). Only CODE counts. |
| 66 | + */ |
| 67 | +function stripComments(src: string): string { |
| 68 | + return src |
| 69 | + .replace(/\/\*[\s\S]*?\*\//g, '') |
| 70 | + .replace(/(^|[^:])\/\/.*$/gm, '$1'); |
| 71 | +} |
| 72 | + |
| 73 | +describe('objectstack#3913 ratchet — /actions callers use interpretActionResponse', () => { |
| 74 | + const callers = walk(appShellSrc) |
| 75 | + .filter((f) => !EXEMPT.has(f) && ROUTE.test(stripComments(readFileSync(f, 'utf8')))); |
| 76 | + |
| 77 | + const offenders = callers |
| 78 | + .filter((f) => !readFileSync(f, 'utf8').includes('interpretActionResponse')) |
| 79 | + .map((f) => path.relative(appShellSrc, f)); |
| 80 | + |
| 81 | + it('no app-shell file interprets an /actions response by hand', () => { |
| 82 | + expect(offenders).toEqual([]); |
| 83 | + }); |
| 84 | + |
| 85 | + it('still guards something — the known callers are present', () => { |
| 86 | + // A ratchet that matches nothing passes vacuously forever. Pin that the |
| 87 | + // two handlers it exists for are actually being scanned. |
| 88 | + expect(callers.map((f) => path.relative(appShellSrc, f))).toEqual( |
| 89 | + expect.arrayContaining([ |
| 90 | + path.join('hooks', 'useConsoleActionRuntime.tsx'), |
| 91 | + path.join('views', 'RecordDetailView.tsx'), |
| 92 | + ]), |
| 93 | + ); |
| 94 | + }); |
| 95 | +}); |
0 commit comments