test(vendors): regression tests + AT_RISK_THRESHOLD + live-risk sort (follow-up to #119)

src/lib/cron.ts

@@ -9,7 +9,7 @@ import { generatePaymentPlan, savePaymentPlan } from './payment-planner';
 import { reconcileNotionDisputes } from './dispute-sync';
 import { enqueueJob, processQueue, type ScrapeJobType } from './job-dispatcher';
 import { decayStaleRouxIntents } from './intent-decay';
-import { computeVendorRisk, vendorRiskInputFromRow } from './vendor-risk';
+import { computeVendorRisk, vendorRiskInputFromRow, AT_RISK_THRESHOLD } from './vendor-risk';
 
 /**
  * Cron sync orchestrator.
@@ -267,7 +267,7 @@ async function sweepVendorRisk(
     const { score } = computeVendorRisk(vendorRiskInputFromRow(r));
     ids.push(r.id as string);
     scores.push(score);
-    if (score >= 50) atRisk++;
+    if (score >= AT_RISK_THRESHOLD) atRisk++;
   }
   await sql`
     UPDATE cc_vendors SET risk_score = bulk.score, updated_at = NOW()

src/lib/vendor-risk.ts

@@ -13,6 +13,10 @@
  */
 import { urgencyLevel, type UrgencyLevel } from './urgency';
 
+// Score at/above which a vendor is "at risk" (high/critical). Mirrors the
+// urgencyLevel 'high' boundary; centralised so routes, MCP, and cron agree.
+export const AT_RISK_THRESHOLD = 50;
+
 export type VendorPaymentStatus = 'active' | 'failed' | 'limited' | 'unknown';
 export type VendorStatus = 'active' | 'paused' | 'cancelled' | 'zombie';
 

src/routes/mcp.ts

@@ -14,7 +14,7 @@ const TRIAGE_TOOL_NAMES = new Set([
 ]);
 import { getDb, typedRows } from '../lib/db';
 import type { NeonQueryFunction } from '@neondatabase/serverless';
-import { computeVendorRisk, vendorRiskInputFromRow, numOrNull } from '../lib/vendor-risk';
+import { computeVendorRisk, vendorRiskInputFromRow, numOrNull, AT_RISK_THRESHOLD } from '../lib/vendor-risk';
 import { listJobs, getJobStatus, retryJob, getDeadLetters, enqueueJob } from '../lib/job-dispatcher';
 import type { ScrapeJobType, ScrapeJobStatus } from '../lib/job-dispatcher';
 import { evidenceClient, ledgerClient, govClient } from '../lib/integrations';
@@ -1163,7 +1163,7 @@ async function executeTool(env: Env, sql: NeonQueryFunction<false, false>, toolN
         const risk = computeVendorRisk(vendorRiskInputFromRow(r));
         return { ...r, risk_score: risk.score, risk_level: risk.level, risk_reasons: risk.reasons };
       });
-      if (atRisk) vendors = vendors.filter((v) => (v.risk_score as number) >= 50);
+      if (atRisk) vendors = vendors.filter((v) => (v.risk_score as number) >= AT_RISK_THRESHOLD);
       vendors.sort((a, b) => (b.risk_score as number) - (a.risk_score as number));
       const limited = vendors.slice(0, limit);
       return { count: limited.length, vendors: limited };
@@ -1183,7 +1183,7 @@ async function executeTool(env: Env, sql: NeonQueryFunction<false, false>, toolN
         if (r.billing_cycle === 'monthly') monthlyCommitted += numOrNull(r.expected_amount) ?? 0;
         const risk = computeVendorRisk(vendorRiskInputFromRow(r));
         byLevel[risk.level] = (byLevel[risk.level] || 0) + 1;
-        if (risk.score >= 50) atRisk.push({ vendor_name: r.vendor_name, category: r.category, score: risk.score, level: risk.level, reasons: risk.reasons });
+        if (risk.score >= AT_RISK_THRESHOLD) atRisk.push({ vendor_name: r.vendor_name, category: r.category, score: risk.score, level: risk.level, reasons: risk.reasons });
       }
       atRisk.sort((a, b) => b.score - a.score);
       return {

src/routes/vendors.ts

@@ -1,7 +1,7 @@
 import { Hono } from 'hono';
 import type { Env } from '../index';
 import { getDb } from '../lib/db';
-import { computeVendorRisk, vendorRiskInputFromRow, numOrNull } from '../lib/vendor-risk';
+import { computeVendorRisk, vendorRiskInputFromRow, numOrNull, AT_RISK_THRESHOLD } from '../lib/vendor-risk';
 import { createVendorSchema, updateVendorSchema, vendorQuerySchema } from '../lib/validators';
 
 export const vendorRoutes = new Hono<{ Bindings: Env }>();
@@ -27,8 +27,11 @@ vendorRoutes.get('/', async (c) => {
       AND (${status}::text IS NULL OR status = ${status})
     ORDER BY risk_score DESC NULLS LAST, next_bill_date ASC NULLS LAST, vendor_name ASC
   `;
-  const vendors = rows.map((r) => ({ ...r, risk: computeVendorRisk(vendorRiskInputFromRow(r)) }));
-  const filtered = atRisk ? vendors.filter((v) => v.risk.score >= 50) : vendors;
+  const vendors = rows
+    .map((r) => ({ ...r, risk: computeVendorRisk(vendorRiskInputFromRow(r)) }))
+    // Sort by *live* risk, not the stored (possibly stale) risk_score column.
+    .sort((a, b) => b.risk.score - a.risk.score);
+  const filtered = atRisk ? vendors.filter((v) => v.risk.score >= AT_RISK_THRESHOLD) : vendors;
   return c.json({ count: filtered.length, vendors: filtered });
 });
 
@@ -62,7 +65,7 @@ vendorRoutes.get('/summary', async (c) => {
 
     const risk = computeVendorRisk(vendorRiskInputFromRow(r));
     byLevel[risk.level] = (byLevel[risk.level] || 0) + 1;
-    if (risk.score >= 50) {
+    if (risk.score >= AT_RISK_THRESHOLD) {
       atRisk.push({ id: r.id, vendor_name: r.vendor_name, category: cat, payment_status: r.payment_status, score: risk.score, level: risk.level, reasons: risk.reasons });
     }
     if (r.status === 'zombie') {
@@ -104,7 +107,10 @@ vendorRoutes.get('/:id', async (c) => {
   return c.json({ ...row, risk: computeVendorRisk(vendorRiskInputFromRow(row)) });
 });
 
-// Create or upsert a vendor (keyed on vendor_name). POST is a full representation.
+// Create or upsert a vendor (keyed on vendor_name). POST is a FULL representation:
+// on conflict every column is overwritten from this payload, and omitted fields
+// fall back to their defaults (e.g. mtd_spend→0, payment_status→'unknown'). Use
+// PATCH for partial updates so existing spend/status data isn't clobbered.
 vendorRoutes.post('/', async (c) => {
   const raw = await c.req.json();
   const result = createVendorSchema.safeParse(raw);
@@ -140,6 +146,7 @@ vendorRoutes.post('/', async (c) => {
       status = EXCLUDED.status,
       owner = EXCLUDED.owner,
       account_id = EXCLUDED.account_id,
+      metadata = EXCLUDED.metadata,
       risk_score = EXCLUDED.risk_score,
       updated_at = NOW()
     RETURNING *

tests/mcp-vendors.test.ts

@@ -0,0 +1,135 @@
+/**
+ * Hermetic regression tests for the vendor MCP tools (query_vendors,
+ * get_vendor_risk). getDb is mocked (same approach as tests/mcp.test.ts) so
+ * these run without a database and pin the exact bug Codex flagged on #119:
+ * at_risk filtering + risk recompute must happen BEFORE limiting, and ordering
+ * must use live risk — not the stored (possibly stale) risk_score column.
+ */
+import { describe, it, expect, vi, beforeEach } from 'vitest';
+import { Hono } from 'hono';
+import type { Env } from '../src/index';
+import { mcpAuthMiddleware } from '../src/middleware/auth';
+import type { AuthVariables } from '../src/middleware/auth';
+
+// Mutable row set the mocked sql tagged-template resolves to. Hoisted so the
+// vi.mock factory can close over it.
+const dbState = vi.hoisted(() => ({ rows: [] as Array<Record<string, unknown>> }));
+
+vi.mock('../src/lib/db', () => ({
+  // getDb returns an sql() tagged-template that ignores the query and resolves
+  // the current dbState.rows — each vendor tool issues a single SELECT.
+  getDb: () => async () => dbState.rows,
+  typedRows: <T>(rows: readonly Record<string, unknown>[]): T[] => rows as unknown as T[],
+}));
+
+import { mcpRoutes } from '../src/routes/mcp';
+
+function makeEnv(): Pick<Env, 'ENVIRONMENT' | 'COMMAND_KV'> & Partial<Env> {
+  return {
+    ENVIRONMENT: 'test',
+    COMMAND_KV: {
+      get: vi.fn().mockResolvedValue(null),
+      put: vi.fn().mockResolvedValue(undefined),
+    } as unknown as KVNamespace,
+  };
+}
+
+function buildApp() {
+  const app = new Hono<{ Bindings: Env; Variables: AuthVariables }>();
+  app.use('/mcp/*', mcpAuthMiddleware);
+  app.route('/mcp', mcpRoutes);
+  const env = makeEnv();
+  return async function callTool(name: string, args: Record<string, unknown> = {}) {
+    const req = new Request('http://localhost/mcp', {
+      method: 'POST',
+      headers: { 'Content-Type': 'application/json' },
+      body: JSON.stringify({ jsonrpc: '2.0', id: 1, method: 'tools/call', params: { name, arguments: args } }),
+    });
+    const res = await app.fetch(req, env as unknown as Env);
+    const json = (await res.json()) as Record<string, unknown>;
+    const result = json.result as { content: Array<{ text: string }>; isError?: boolean };
+    return { isError: result.isError === true, data: JSON.parse(result.content[0].text) };
+  };
+}
+
+// A vendor row as Neon returns it (snake_case, NUMERIC as strings).
+function row(over: Partial<Record<string, unknown>>): Record<string, unknown> {
+  return {
+    id: over.id ?? `id-${over.vendor_name}`,
+    vendor_name: over.vendor_name,
+    category: over.category ?? 'other',
+    billing_cycle: over.billing_cycle ?? 'monthly',
+    expected_amount: over.expected_amount ?? '10.00',
+    currency: 'USD',
+    next_bill_date: over.next_bill_date ?? null,
+    auto_pay: over.auto_pay ?? false,
+    payment_status: over.payment_status ?? 'active',
+    payment_method: null,
+    spending_limit: over.spending_limit ?? null,
+    mtd_spend: over.mtd_spend ?? null,
+    budget_limit: over.budget_limit ?? null,
+    status: over.status ?? 'active',
+    risk_score: over.risk_score ?? null, // deliberately stale/null to prove live recompute
+  };
+}
+
+// Fixture: 5 active vendors. Healthy/low first, high-risk LAST in array order,
+// every stored risk_score null — so any reliance on stored order/score breaks.
+const FIXTURE = [
+  row({ vendor_name: 'healthy', payment_status: 'active', auto_pay: true, mtd_spend: '10', spending_limit: '100' }), // 0 (low)
+  row({ vendor_name: 'unknown-low', payment_status: 'unknown' }), // 5 (low)
+  row({ vendor_name: 'limited', payment_status: 'limited' }), // 35 (medium)
+  row({ vendor_name: 'failed-hi', payment_status: 'failed', mtd_spend: '200', spending_limit: '100' }), // 75 (critical)
+  row({ vendor_name: 'failed-50', payment_status: 'failed', mtd_spend: '50', spending_limit: '100' }), // 50 (high)
+];
+
+beforeEach(() => {
+  dbState.rows = FIXTURE.map((r) => ({ ...r }));
+});
+
+describe('query_vendors (MCP)', () => {
+  it('at_risk=true keeps high-risk vendors even when a small limit would page them out', async () => {
+    const callTool = buildApp();
+    // limit=1: the buggy version applied LIMIT in SQL before filtering, which
+    // could drop the at-risk vendor entirely. Now: filter → sort → slice.
+    const { data } = await callTool('query_vendors', { at_risk: true, limit: 1 });
+    expect(data.count).toBe(1);
+    // Highest live risk among the two at-risk vendors wins the single slot.
+    expect(data.vendors[0].vendor_name).toBe('failed-hi');
+    expect(data.vendors[0].risk_score).toBe(75);
+  });
+
+  it('at_risk=true returns ALL vendors at/over threshold (not just the first page)', async () => {
+    const callTool = buildApp();
+    const { data } = await callTool('query_vendors', { at_risk: true });
+    expect(data.count).toBe(2);
+    expect(data.vendors.map((v: { vendor_name: string }) => v.vendor_name).sort()).toEqual(['failed-50', 'failed-hi']);
+  });
+
+  it('orders by LIVE risk, not the stale stored risk_score column', async () => {
+    const callTool = buildApp();
+    const { data } = await callTool('query_vendors', { limit: 2 });
+    expect(data.vendors.map((v: { vendor_name: string }) => v.vendor_name)).toEqual(['failed-hi', 'failed-50']);
+  });
+
+  it('recomputes risk live when stored risk_score is null', async () => {
+    const callTool = buildApp();
+    const { data } = await callTool('query_vendors', {});
+    const healthy = data.vendors.find((v: { vendor_name: string }) => v.vendor_name === 'healthy');
+    expect(healthy.risk_score).toBe(0);
+    expect(healthy.risk_level).toBe('low');
+  });
+});
+
+describe('get_vendor_risk (MCP)', () => {
+  it('aggregates by risk level and lists the at-risk vendors', async () => {
+    const callTool = buildApp();
+    const { data } = await callTool('get_vendor_risk', {});
+    expect(data.vendor_count).toBe(5);
+    expect(data.by_level).toEqual({ critical: 1, high: 1, medium: 1, low: 2 });
+    expect(data.at_risk).toHaveLength(2);
+    expect(data.at_risk[0].vendor_name).toBe('failed-hi'); // sorted desc by score
+    // total MTD spend: 10 + 200 + 50 = 260 (others null → 0)
+    expect(data.total_mtd_spend).toBe(260);
+  });
+});

tests/routes/vendors.spec.ts

@@ -0,0 +1,129 @@
+/**
+ * Integration tests for /api/vendors (vendor spend control).
+ *
+ * Real Neon. Skipped without DATABASE_URL — mirrors the established pattern in
+ * tests/routes/triage-roux.spec.ts (no-mocks rule per CLAUDE.md). The cc_vendors
+ * table is created by the vitest globalSetup (migration 0019 is registered in
+ * ADDITIVE_PREFIXES).
+ *
+ * Regression coverage for the two Codex findings on PR #119:
+ *   - PATCH must persist `metadata` (it was accepted but never written).
+ *   - at_risk filtering uses live recompute, not the stored risk_score.
+ * Plus the documented FULL-representation upsert semantics of POST.
+ */
+import { describe, it, expect, beforeAll, afterAll } from 'vitest';
+import { neon } from '@neondatabase/serverless';
+import { Hono } from 'hono';
+import type { Env } from '../../src/index';
+import { vendorRoutes } from '../../src/routes/vendors';
+
+const DATABASE_URL = process.env.DATABASE_URL;
+const SKIP = !DATABASE_URL || process.env.SKIP_INTEGRATION === '1';
+const TAG = `vtest-${Date.now()}-${Math.floor(Math.random() * 1e6)}`;
+
+const env = { DATABASE_URL } as unknown as Env;
+
+const app = new Hono<{ Bindings: Env }>();
+app.route('/api/vendors', vendorRoutes);
+
+async function api(method: string, path: string, body?: unknown) {
+  const req = new Request(`http://localhost/api/vendors${path}`, {
+    method,
+    headers: { 'Content-Type': 'application/json' },
+    body: body === undefined ? undefined : JSON.stringify(body),
+  });
+  const res = await app.fetch(req, env);
+  // eslint-disable-next-line @typescript-eslint/no-explicit-any
+  const json = (await res.json().catch(() => null)) as any;
+  return { status: res.status, json };
+}
+
+async function cleanup() {
+  if (!DATABASE_URL) return;
+  const sql = neon(DATABASE_URL);
+  await sql`DELETE FROM cc_vendors WHERE vendor_name LIKE ${TAG + '%'}`;
+}
+
+describe.skipIf(SKIP)('/api/vendors (real Neon)', () => {
+  beforeAll(cleanup);
+  afterAll(cleanup);
+
+  it('POST creates a vendor and computes a risk score', async () => {
+    const { status, json } = await api('POST', '/', {
+      vendor_name: `${TAG}-create`,
+      category: 'infra',
+      billing_cycle: 'monthly',
+      expected_amount: 25,
+      payment_status: 'active',
+      auto_pay: true,
+    });
+    expect(status).toBe(201);
+    expect(json.id).toBeTruthy();
+    expect(typeof json.risk_score).toBe('number');
+  });
+
+  it('PATCH persists metadata (regression: was accepted but never written)', async () => {
+    const created = await api('POST', '/', { vendor_name: `${TAG}-meta`, category: 'data' });
+    expect(created.status).toBe(201);
+    const id = created.json.id;
+
+    const patched = await api('PATCH', `/${id}`, { metadata: { owner_note: 'hello', team: 'ops' } });
+    expect(patched.status).toBe(200);
+
+    const got = await api('GET', `/${id}`);
+    expect(got.status).toBe(200);
+    expect(got.json.metadata).toMatchObject({ owner_note: 'hello', team: 'ops' });
+  });
+
+  it('GET ?at_risk=true filters by live risk (failed in, healthy out)', async () => {
+    await api('POST', '/', {
+      vendor_name: `${TAG}-failed`,
+      category: 'ai_inference',
+      payment_status: 'failed', // → risk 50 (at risk)
+    });
+    await api('POST', '/', {
+      vendor_name: `${TAG}-healthy`,
+      category: 'dev_tooling',
+      payment_status: 'active',
+      auto_pay: true, // → risk 0 (not at risk)
+    });
+
+    const { status, json } = await api('GET', '/?at_risk=true');
+    expect(status).toBe(200);
+    const names: string[] = json.vendors.map((v: { vendor_name: string }) => v.vendor_name);
+    expect(names).toContain(`${TAG}-failed`);
+    expect(names).not.toContain(`${TAG}-healthy`);
+  });
+
+  it('POST is a FULL upsert: a partial re-POST clobbers omitted fields to defaults', async () => {
+    const name = `${TAG}-upsert`;
+    const first = await api('POST', '/', {
+      vendor_name: name,
+      category: 'infra',
+      payment_status: 'failed',
+      mtd_spend: 200,
+      metadata: { note: 'first' },
+    });
+    expect(first.status).toBe(201);
+    expect(parseFloat(first.json.mtd_spend)).toBe(200);
+    expect(first.json.metadata).toEqual({ note: 'first' });
+
+    // Re-POST with only name+category — documented behaviour resets the rest,
+    // including metadata (ON CONFLICT now assigns metadata = EXCLUDED.metadata).
+    const second = await api('POST', '/', { vendor_name: name, category: 'data' });
+    expect(second.status).toBe(201);
+    expect(second.json.category).toBe('data');
+    expect(parseFloat(second.json.mtd_spend)).toBe(0); // clobbered to default
+    expect(second.json.payment_status).toBe('unknown'); // clobbered to default
+    expect(second.json.metadata).toEqual({}); // clobbered to default (was {note:'first'})
+  });
+
+  it('GET /summary returns spend rollups', async () => {
+    const { status, json } = await api('GET', '/summary');
+    expect(status).toBe(200);
+    expect(typeof json.total_mtd_spend).toBe('number');
+    expect(json.by_level).toHaveProperty('critical');
+    expect(Array.isArray(json.at_risk)).toBe(true);
+    expect(Array.isArray(json.upcoming_bills)).toBe(true);
+  });
+});