From 9982df2d0bca1dbf0bcfbae923c47493bbd8b68b Mon Sep 17 00:00:00 2001
From: Mao Nakamoto <maonakamoto@users.noreply.github.com>
Date: Fri, 4 Sep 2026 04:05:45 +0000
Subject: [PATCH] fix(api): stop a swallowed query error from mis-numbering
 compositions
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

POST /api/compositions computed nextVersion from `{ data: existing } =
await supabase...single()` without checking `error`. `.single()` throws
on zero rows — the normal case for a project's first composition — so
a real query failure (RLS misconfig, DB outage) left `data` null just
like "no composition yet," silently defaulting nextVersion to 1 instead
of surfacing the failure and risking a colliding/duplicate version.

Switched to `.maybeSingle()` with an explicit error check, matching the
project's convention for "zero rows is valid" (ownsProject/ownsFigure,
the surfaces/compositions GET handlers from #40). Added a structural
test that walks every API route for a `data` destructure from a
supabase query missing its `error`, so a call site can't drop back to
this pattern.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
---
 app/src/app/api/compositions/route.ts       |  7 +-
 app/src/app/api/query-error-checked.test.ts | 85 +++++++++++++++++++++
 2 files changed, 90 insertions(+), 2 deletions(-)
 create mode 100644 app/src/app/api/query-error-checked.test.ts

diff --git a/app/src/app/api/compositions/route.ts b/app/src/app/api/compositions/route.ts
index 36bcf5a..3688b99 100644
--- a/app/src/app/api/compositions/route.ts
+++ b/app/src/app/api/compositions/route.ts
@@ -41,13 +41,16 @@ export async function POST(request: NextRequest) {
     return NextResponse.json({ success: false, error: 'Not found' }, { status: 404 });
   }
 
-  const { data: existing } = await supabase
+  const { data: existing, error: existingError } = await supabase
     .from('compositions')
     .select('version')
     .eq('project_id', parsed.data.project_id)
     .order('version', { ascending: false })
     .limit(1)
-    .single();
+    .maybeSingle();
+
+  if (existingError)
+    return NextResponse.json({ success: false, error: existingError.message }, { status: 500 });
 
   const nextVersion = (existing?.version ?? 0) + 1;
 
diff --git a/app/src/app/api/query-error-checked.test.ts b/app/src/app/api/query-error-checked.test.ts
new file mode 100644
index 0000000..a377ac5
--- /dev/null
+++ b/app/src/app/api/query-error-checked.test.ts
@@ -0,0 +1,85 @@
+import { describe, expect, it } from 'vitest';
+import { readFileSync, readdirSync } from 'node:fs';
+import { join, relative } from 'node:path';
+import ts from 'typescript';
+
+/**
+ * compositions POST computed the next version from
+ * `const { data: existing } = await supabase...single()`, dropping `error`
+ * entirely. `.single()` throws when zero rows match — the normal case for a
+ * project's first composition — so a real query failure (RLS misconfig, DB
+ * outage) looked identical to "no composition yet": both leave `data` null.
+ * Silently defaulting to version 1 on a genuine failure risked inserting a
+ * colliding/duplicate version instead of surfacing the failure.
+ *
+ * This walks every API route for a destructured `data` bound to an awaited
+ * `supabase...` query and fails if the same destructure doesn't also bind
+ * `error`, so a call site can't drop back to reading `data` without checking
+ * whether the query actually succeeded.
+ */
+describe('every supabase query result destructure checks its error', () => {
+  const apiDir = join(process.cwd(), 'src/app/api');
+
+  function routeFiles(dir: string): string[] {
+    return readdirSync(dir, { withFileTypes: true }).flatMap((entry) => {
+      const path = join(dir, entry.name);
+      if (entry.isDirectory()) return routeFiles(path);
+      return entry.name === 'route.ts' ? [path] : [];
+    });
+  }
+
+  function rootIsSupabase(node: ts.Expression): boolean {
+    let cur: ts.Expression = ts.isAwaitExpression(node) ? node.expression : node;
+    while (ts.isCallExpression(cur) || ts.isPropertyAccessExpression(cur)) {
+      cur = cur.expression;
+    }
+    return ts.isIdentifier(cur) && cur.text === 'supabase';
+  }
+
+  function bindingName(el: ts.BindingElement, source: ts.SourceFile): string {
+    const nameNode = el.propertyName ?? el.name;
+    return ts.isIdentifier(nameNode) ? nameNode.getText(source) : '';
+  }
+
+  function missingErrorIn(file: string): number[] {
+    const source = ts.createSourceFile(
+      file,
+      readFileSync(file, 'utf8'),
+      ts.ScriptTarget.Latest,
+      true,
+      ts.ScriptKind.TS,
+    );
+    const missing: number[] = [];
+
+    function visit(node: ts.Node) {
+      if (
+        ts.isVariableDeclaration(node) &&
+        node.initializer &&
+        ts.isObjectBindingPattern(node.name) &&
+        rootIsSupabase(node.initializer)
+      ) {
+        const names = node.name.elements.map((el) => bindingName(el, source));
+        if (names.includes('data') && !names.includes('error')) {
+          missing.push(source.getLineAndCharacterOfPosition(node.getStart()).line + 1);
+        }
+      }
+      ts.forEachChild(node, visit);
+    }
+
+    visit(source);
+    return missing;
+  }
+
+  const files = routeFiles(apiDir);
+
+  it('finds the route files', () => {
+    expect(files.length).toBeGreaterThan(0);
+  });
+
+  it.each(files.map((f) => [relative(apiDir, f), f]))(
+    '%s checks the error on every data destructure',
+    (_name, file) => {
+      expect(missingErrorIn(file)).toEqual([]);
+    },
+  );
+});