fix(server): phase 1 review follow-ups — awaited returns, logged upload-filter rejections, covered handshake/notice branches, call-graph gate in CI, dead test imports

2026-10-22 06:56:35 +00:00 · 2026-09-22 07:54:09 +02:00
parent f202227bbc
commit ae2af05cad
107 changed files with 324 additions and 209 deletions
@@ -0,0 +1,100 @@
+/**
+ * The Phase 1 async-sweep gate (`scripts/db-call-graph.mjs`) had no test of its
+ * own — CI trusted the tool's own output without ever proving the detector still
+ * works. This runs the real script as a child process (never through a shell or
+ * PATH, so nothing but the pinned Node binary decides what runs) against the
+ * small, hand-documented fixture tree in `scripts/__fixtures__/`, once for each
+ * flag combination the CI step and the recipe's R5 gate use.
+ *
+ * The script's own contract (see its final `process.exit(found ? 1 : 0)`) is to
+ * exit non-zero when it has findings, and the `--unawaited` run against this
+ * fixture is SUPPOSED to find the five documented BAD shapes, so `execFileSync`
+ * throws on it — Node still attaches the child's stdout/status to that error,
+ * which `run()` below reads back out instead of treating the throw as a test
+ * failure.
+ */
+import { describe, it, expect } from 'vitest';
+import { execFileSync } from 'node:child_process';
+import path from 'node:path';
+
+const SERVER_ROOT = path.join(__dirname, '../../..');
+const SCRIPT = path.join(SERVER_ROOT, 'scripts/db-call-graph.mjs');
+
+interface UnawaitedFinding {
+  file: string;
+  class: string;
+  method: string;
+  reason: string;
+}
+
+interface UnawaitedReport {
+  unawaitedCalls: UnawaitedFinding[];
+  unawaitedCallCount: number;
+}
+
+interface SyncTxReport {
+  syncDbMethods: unknown[];
+  syncDbMethodCount: number;
+  transactionSites: unknown[];
+  transactionSiteCount: number;
+}
+
+interface ExecError {
+  status: number | null;
+  stdout: string;
+}
+
+function run(args: string[]): { status: number | null; report: unknown } {
+  try {
+    const stdout = execFileSync(process.execPath, [SCRIPT, ...args], {
+      cwd: SERVER_ROOT,
+      encoding: 'utf8',
+    });
+    return { status: 0, report: JSON.parse(stdout) };
+  } catch (err) {
+    // A non-zero exit (the gate has findings) surfaces as a thrown error whose
+    // stdout/status carry the same JSON report the caller would have seen on
+    // a clean run.
+    const { status, stdout } = err as ExecError;
+    return { status, report: JSON.parse(stdout) };
+  }
+}
+
+describe('db-call-graph.mjs', () => {
+  it('CALLGRAPH-001: --unawaited finds exactly the five documented BAD shapes, one per reason tag, and exits 1', () => {
+    const { status, report } = run(['--root', 'scripts/__fixtures__', '--unawaited', '--json']);
+    const { unawaitedCalls, unawaitedCallCount } = report as UnawaitedReport;
+
+    // A gate with findings must fail the CI step it runs in.
+    expect(status).toBe(1);
+    expect(unawaitedCallCount).toBe(5);
+    expect(unawaitedCalls).toHaveLength(5);
+    expect(unawaitedCalls.every((f) => f.file === 'scripts/__fixtures__/unawaited-sample.ts')).toBe(true);
+    expect(unawaitedCalls.every((f) => f.class === 'Sample')).toBe(true);
+
+    // The fixture's own doc comment: three GOOD shapes produce nothing, five BAD
+    // ones each produce exactly one finding with a distinct reason.
+    const byMethod = Object.fromEntries(unawaitedCalls.map((f) => [f.method, f.reason]));
+    expect(byMethod).toEqual({
+      bad1: 'stored-then-used',
+      bad2: 'argument',
+      bad3: 'property',
+      bad4: 'condition',
+      bad5: 'statement',
+    });
+    expect(byMethod.good1).toBeUndefined();
+    expect(byMethod.good2).toBeUndefined();
+    expect(byMethod.good3).toBeUndefined();
+  });
+
+  it('CALLGRAPH-002: --sync --tx finds nothing in the fixture tree (it touches no database) and exits 0', () => {
+    const { status, report } = run(['--root', 'scripts/__fixtures__', '--sync', '--tx', '--json']);
+    const { syncDbMethods, syncDbMethodCount, transactionSites, transactionSiteCount } = report as SyncTxReport;
+
+    expect(status).toBe(0);
+    expect(syncDbMethodCount).toBe(0);
+    expect(syncDbMethods).toEqual([]);
+    expect(transactionSiteCount).toBe(0);
+    expect(transactionSites).toEqual([]);
+  });
+});