From 9a86b2b7de9d4b7bbe3ce796a28236fb574f88b4 Mon Sep 17 00:00:00 2001 From: Mao Nakamoto <41178744+maonakamoto@users.noreply.github.com> Date: Wed, 2 Sep 2026 06:05:01 +0200 Subject: [PATCH] fix(figures): the styled upload could fail without saying so MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit FigureCard's styled-image upload patched the figure row with no onError. The file reached storage, the row write could fail, and nothing was shown: the card kept offering "Upload styled" over an upload that looked like it worked, and the styled image — the artifact the whole manual pipeline exists to produce — was silently not attached. This is the third save shipped with no failure path (#36 compose placement, e399f4a label/delete, this one), so the class is closed rather than the instance: a guard walks every .mutate()/.mutateAsync() call site with the TypeScript parser and fails on any that passes no onError. Mutation-verified — reverting the FigureCard fix fails the guard at components/figures/FigureCard.tsx:47. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01BjpTz8zzJhUTK9UWw9SPgS --- app/src/components/figures/FigureCard.tsx | 8 +- .../hooks/mutations-report-failure.test.ts | 82 +++++++++++++++++++ 2 files changed, 89 insertions(+), 1 deletion(-) create mode 100644 app/src/hooks/mutations-report-failure.test.ts diff --git a/app/src/components/figures/FigureCard.tsx b/app/src/components/figures/FigureCard.tsx index aac390c..176916b 100644 --- a/app/src/components/figures/FigureCard.tsx +++ b/app/src/components/figures/FigureCard.tsx @@ -46,7 +46,13 @@ export function FigureCard({ figure, projectId }: FigureCardProps) { updateFigure.mutate( { id: figure.id, data: { styled_url: result.path, status: 'styled' } }, - { onSuccess: () => toast.success('Styled version uploaded') }, + { + onSuccess: () => toast.success('Styled version uploaded'), + // The file is already in storage; only this row makes it the figure's + // styled version. Failing quietly leaves the card showing "Upload + // styled" over an upload that appeared to work. + onError: (err) => toast.error(err.message), + }, ); } diff --git a/app/src/hooks/mutations-report-failure.test.ts b/app/src/hooks/mutations-report-failure.test.ts new file mode 100644 index 0000000..8ed2fc9 --- /dev/null +++ b/app/src/hooks/mutations-report-failure.test.ts @@ -0,0 +1,82 @@ +import { describe, expect, it } from 'vitest'; +import { readFileSync, readdirSync } from 'node:fs'; +import { join, relative } from 'node:path'; +import ts from 'typescript'; + +/** + * Three separate saves shipped with no failure path: figure placement in the + * compose editor (#36), a figure's label and delete (e399f4a), and the styled + * upload on FigureCard. Each looked identical in the UI whether it stored the + * work or dropped it — the user was told nothing and lost the edit on refresh. + * + * A mutation that cannot report failure is a save the user cannot trust, so + * this walks every `.mutate(...)` / `.mutateAsync(...)` call site in the app + * and fails on any that passes no `onError`. Fixing the fourth instance by + * hand is what this test exists to prevent. + */ +describe('every mutation call site reports its failures', () => { + const srcDir = join(process.cwd(), 'src'); + + function filesUnder(dir: string): string[] { + return readdirSync(dir, { withFileTypes: true }).flatMap((entry) => { + const path = join(dir, entry.name); + if (entry.isDirectory()) return filesUnder(path); + if (!/\.tsx?$/.test(entry.name) || /\.test\.tsx?$/.test(entry.name)) return []; + return [path]; + }); + } + + /** react-query takes the options object as the last argument of either call. */ + function handlesError(call: ts.CallExpression): boolean { + return call.arguments.some( + (arg) => + ts.isObjectLiteralExpression(arg) && + arg.properties.some((prop) => prop.name?.getText(arg.getSourceFile()) === 'onError'), + ); + } + + function unhandledIn(file: string): string[] { + const source = ts.createSourceFile( + file, + readFileSync(file, 'utf8'), + ts.ScriptTarget.Latest, + true, + ts.ScriptKind.TSX, + ); + const unhandled: string[] = []; + + function visit(node: ts.Node) { + if ( + ts.isCallExpression(node) && + ts.isPropertyAccessExpression(node.expression) && + (node.expression.name.text === 'mutate' || node.expression.name.text === 'mutateAsync') && + !handlesError(node) + ) { + const line = source.getLineAndCharacterOfPosition(node.getStart()).line + 1; + unhandled.push(`${relative(srcDir, file)}:${line} ${node.expression.getText(source)}`); + } + ts.forEachChild(node, visit); + } + + visit(source); + return unhandled; + } + + const sources = filesUnder(srcDir); + + it('finds the app sources', () => { + expect(sources.length).toBeGreaterThan(0); + }); + + it('finds the mutation call sites it is meant to guard', () => { + const calls = sources.flatMap((file) => { + const text = readFileSync(file, 'utf8'); + return text.match(/\.mutate(Async)?\(/g) ?? []; + }); + expect(calls.length).toBeGreaterThan(0); + }); + + it('leaves no mutation without an onError', () => { + expect(sources.flatMap(unhandledIn)).toEqual([]); + }); +});