feat(invoices): draft invoice delete on v1 and MCP with staged approval (#2036)

* feat(invoices): draft invoice delete on v1 and MCP with staged approval Draft customer-invoice deletion was web-only. This makes the same semantics available on the v1 API-key surface and as an MCP write tool: unnumbered drafts are hard deleted (no F-series number was consumed, so no gap arises), numbered drafts are makulerade (status 'cancelled', number retained so the F-series stays gap-free per ML 17 kap 24 and BFNAR 2013:2). Non-drafts are refused; posted invoices can only be reversed via a credit note. - extract the web DELETE logic into lib/invoices/delete-draft-invoice.ts with an explicit userId param (service-role clients null auth.uid()); the cookie route behavior is unchanged - add DELETE /api/v1/companies/{companyId}/invoices/{id}: 409 INVOICE_DELETE_NOT_DRAFT for non-drafts (status override; the cookie route keeps its 400), 404 generic NOT_FOUND, dry-run preview of the outcome, mandatory Idempotency-Key; scope invoices:write - fix the stale v1 PATCH pitfall that claimed a DELETE handler existed - new MCP tool gnubok_delete_draft_invoice: staged operation requiring approval, risk 'high' (both outcomes irreversible, never auto-committed), catalogVisibility 'search' (tools/list budget at zero headroom) - delete_draft_invoice commit executor delegating to the shared service, plus pending_operations CHECK constraint migration pair (20260830100000/100001), risk tier, scope map, Granskning vocabulary and sv/en labels Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NtvffGr6uVk2J2Skuz6L98 * fix(migrations): renumber delete_draft_invoice pair after 20260830101500 on main Merging origin/main brought 20260830101500_seed_agent_atom_bodies; the constraint pair must sort after every version already on main so it never applies out of order at merge time. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NtvffGr6uVk2J2Skuz6L98 * docs(api-skill): regenerate accounted-api skill for the new invoices.delete endpoint apiskill:check failed on CI: registering DELETE /invoices/{id} makes the generated skills/accounted-api docs stale. Output of npm run apiskill:generate, no hand edits. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NtvffGr6uVk2J2Skuz6L98 * fix(invoices): pin staged delete outcome and align v1 risk metadata Skeptic findings on PR #2036: - Outcome pin: gnubok_delete_draft_invoice stages expected_invoice_number alongside invoice_id; the executor passes it to deleteDraftInvoice, which refuses with INVOICE_CANCEL_RACE when the draft's number changed since staging. An unnumbered draft finalized between staging and approval is now auto-rejected with a message naming the new number, instead of silently switching from the approved hard delete to a makulering. Ops staged without the pin keep legacy semantics; single-phase callers (web, v1) are unaffected. - v1 invoices.delete registerEndpoint risk raised medium -> high to match the delete_draft_invoice pending-op tier (both outcomes irreversible); generated accounted-api docs regenerated. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NtvffGr6uVk2J2Skuz6L98 * fix(migrations): renumber delete_draft_invoice pair after skattekonto collision Merging origin/main brought PR #2039's 20260830130000/130001 pair, which collides with this branch's versions AND re-creates the same pending_operations CHECK wholesale. Renumber to 20260830150000/150001 and rebuild the value list as a strict superset (skattekonto list plus delete_draft_invoice) so applying last revokes nothing. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NtvffGr6uVk2J2Skuz6L98 --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
2026-08-30 16:57:28 +02:00
parent 32cb8a22ce
commit 9f8fa1b692
21 changed files with 1257 additions and 83 deletions
@@ -0,0 +1,167 @@
+import { describe, expect, it } from 'vitest'
+import { createQueuedMockSupabase } from '@/tests/helpers'
+import { TOOL_SCOPE_MAP } from '@/lib/auth/api-keys'
+import { OPERATION_RISK_TIERS } from '@/lib/pending-operations/risk-tiers'
+import { deriveToolMeta, tools } from '../server'
+
+const INVOICE_ID = '22222222-2222-4222-8222-222222222222'
+const tool = () => tools.find((candidate) => candidate.name === 'gnubok_delete_draft_invoice')!
+
+function draftInvoice(overrides: Record<string, unknown> = {}) {
+  return {
+    id: INVOICE_ID,
+    invoice_number: null,
+    status: 'draft',
+    total: 12500,
+    currency: 'SEK',
+    customer: { name: 'Testbrand AB' },
+    ...overrides,
+  }
+}
+
+describe('gnubok_delete_draft_invoice: registration', () => {
+  it('is a strict, staged, destructive invoices:write tool at high risk', () => {
+    expect(tool()).toBeDefined()
+    expect(tool().inputSchema.additionalProperties).toBe(false)
+    expect(tool().annotations.readOnlyHint).toBe(false)
+    expect(tool().annotations.destructiveHint).toBe(true)
+    expect(tool().annotations.idempotentHint).toBe(false)
+    // tools/list budget is at zero headroom: search-only catalog visibility.
+    expect(tool().catalogVisibility).toBe('search')
+    expect(TOOL_SCOPE_MAP.gnubok_delete_draft_invoice).toBe('invoices:write')
+    // 'high' risk: never auto-committed, approval is always required.
+    expect(OPERATION_RISK_TIERS.delete_draft_invoice).toBe('high')
+  })
+
+  it('returns the staged-operation envelope and derives requires_approval meta', () => {
+    const schema = tool().outputSchema as { properties?: Record<string, unknown>; required?: string[] }
+    expect(schema?.properties?.staged).toBeDefined()
+    expect(schema?.required).toContain('staged')
+    // deriveToolMeta keys off the STAGED_OPERATION_SCHEMA reference: the
+    // machine-readable approval contract must come for free.
+    expect(deriveToolMeta(tool())).toMatchObject({
+      requires_approval: true,
+      approve_tool: 'gnubok_approve_pending_operation',
+    })
+  })
+
+  it('keeps its description within the 280-char budget and declares drafts-only staging', () => {
+    expect(tool().description.length).toBeLessThanOrEqual(280)
+    expect(tool().description).toMatch(/stag(e|ing)/i)
+    expect(tool().description).toMatch(/draft/i)
+    expect(tool().description).toContain('gnubok_credit_invoice')
+  })
+})
+
+describe('gnubok_delete_draft_invoice: staging', () => {
+  it('requires invoice_id', async () => {
+    const { supabase } = createQueuedMockSupabase()
+
+    await expect(
+      tool().execute({}, 'company-1', 'user-1', supabase as never),
+    ).rejects.toThrow(/invoice_id is required/i)
+    expect(supabase.from).not.toHaveBeenCalled()
+  })
+
+  it('throws for an invoice that does not exist in the company', async () => {
+    const { supabase, enqueue } = createQueuedMockSupabase()
+    enqueue({ data: null, error: { message: 'not found' } })
+
+    await expect(
+      tool().execute({ invoice_id: INVOICE_ID }, 'company-1', 'user-1', supabase as never),
+    ).rejects.toThrow(/not found/i)
+    expect(supabase.from).not.toHaveBeenCalledWith('pending_operations')
+  })
+
+  it.each([
+    ['sent invoice', 'sent'],
+    ['paid invoice', 'paid'],
+    ['credited invoice', 'credited'],
+    ['already cancelled invoice', 'cancelled'],
+  ])('refuses a %s at staging time, before anything is staged', async (_label, status) => {
+    const { supabase, enqueue } = createQueuedMockSupabase()
+    enqueue({ data: draftInvoice({ status, invoice_number: 'F-2026042' }) })
+
+    await expect(
+      tool().execute({ invoice_id: INVOICE_ID }, 'company-1', 'user-1', supabase as never),
+    ).rejects.toThrow(/only draft invoices can be deleted/i)
+    expect(supabase.from).toHaveBeenCalledTimes(1)
+    expect(supabase.from).not.toHaveBeenCalledWith('pending_operations')
+  })
+
+  it('stages a hard delete for an unnumbered draft, requiring approval', async () => {
+    const { supabase, enqueue, findCall } = createQueuedMockSupabase()
+    enqueue({ data: draftInvoice() })
+    enqueue({ data: { id: 'op-delete-1' } })
+
+    const result = (await tool().execute(
+      { invoice_id: INVOICE_ID },
+      'company-1',
+      'user-1',
+      supabase as never,
+    )) as { staged: boolean; operation_id?: string; risk_level: string; preview: Record<string, unknown> }
+
+    expect(result).toMatchObject({
+      staged: true,
+      operation_id: 'op-delete-1',
+      risk_level: 'high',
+    })
+    expect(supabase.from).toHaveBeenCalledTimes(2)
+    expect(supabase.from).toHaveBeenNthCalledWith(2, 'pending_operations')
+    expect(result.preview).toMatchObject({
+      invoice_id: INVOICE_ID,
+      invoice_number: null,
+      customer_name: 'Testbrand AB',
+    })
+    expect(String(result.preview.method)).toMatch(/hard delete/i)
+    // The staged params pin the approved outcome: null = hard delete.
+    const stagedRow = findCall('pending_operations', 'insert')?.[0] as
+      | { params?: Record<string, unknown> }
+      | undefined
+    expect(stagedRow?.params).toMatchObject({
+      invoice_id: INVOICE_ID,
+      expected_invoice_number: null,
+    })
+  })
+
+  it('stages a makulering for a numbered draft, retaining the F-series number', async () => {
+    const { supabase, enqueue, findCall } = createQueuedMockSupabase()
+    enqueue({ data: draftInvoice({ invoice_number: 'F-2026042' }) })
+    enqueue({ data: { id: 'op-delete-2' } })
+
+    const result = (await tool().execute(
+      { invoice_id: INVOICE_ID },
+      'company-1',
+      'user-1',
+      supabase as never,
+    )) as { staged: boolean; risk_level: string; preview: Record<string, unknown> }
+
+    expect(result).toMatchObject({ staged: true, risk_level: 'high' })
+    expect(result.preview).toMatchObject({ invoice_number: 'F-2026042' })
+    expect(String(result.preview.method)).toMatch(/makulering/i)
+    // The pin records the number whose makulering was approved.
+    const stagedRow = findCall('pending_operations', 'insert')?.[0] as
+      | { params?: Record<string, unknown> }
+      | undefined
+    expect(stagedRow?.params).toMatchObject({ expected_invoice_number: 'F-2026042' })
+  })
+
+  it('returns a dry-run preview without staging', async () => {
+    const { supabase, enqueue } = createQueuedMockSupabase()
+    enqueue({ data: draftInvoice({ invoice_number: 'F-2026042' }) })
+
+    const result = (await tool().execute(
+      { invoice_id: INVOICE_ID, dry_run: true },
+      'company-1',
+      'user-1',
+      supabase as never,
+    )) as { staged: boolean; dry_run?: boolean; preview: Record<string, unknown> }
+
+    expect(result.staged).toBe(false)
+    expect(result.dry_run).toBe(true)
+    expect(result.preview).toMatchObject({ invoice_number: 'F-2026042' })
+    // Exactly one read; pending_operations was never touched.
+    expect(supabase.from).toHaveBeenCalledTimes(1)
+    expect(supabase.from).not.toHaveBeenCalledWith('pending_operations')
+  })
+})
@@ -17336,6 +17336,80 @@ export const tools: McpTool[] = [
    },
  },

+  {
+    name: 'gnubok_delete_draft_invoice',
+    title: 'Delete Draft Invoice',
+    description:
+      'Stage removal of a DRAFT invoice for approval. Drafts only: an unnumbered draft is hard deleted; a numbered draft is makulerad (status cancelled, number kept so the F-series stays gap-free). Sent/paid/credited invoices are refused: use gnubok_credit_invoice instead.',
+    inputSchema: {
+      type: 'object',
+      additionalProperties: false,
+      properties: {
+        invoice_id: { type: 'string', description: 'UUID of the draft invoice, from gnubok_list_invoices.' },
+        dry_run: { type: 'boolean', description: 'Validate and preview without staging or changing data.' },
+        idempotency_key: { type: 'string', description: 'Random per-operation UUID. Reusing it with the same payload returns the original staged response.' },
+      },
+      required: ['invoice_id'],
+    },
+    outputSchema: STAGED_OPERATION_SCHEMA,
+    annotations: { readOnlyHint: false, destructiveHint: true, idempotentHint: false, openWorldHint: false },
+    catalogVisibility: 'search',
+    async execute(args, companyId, userId, supabase, actor) {
+      const invoiceId = args.invoice_id as string
+      if (!invoiceId) throw new Error('invoice_id is required. Use gnubok_list_invoices to find IDs.')
+
+      const { data: inv } = await supabase
+        .from('invoices')
+        .select('id, invoice_number, status, total, currency, customer:customers(name)')
+        .eq('id', invoiceId).eq('company_id', companyId).single()
+
+      if (!inv) throw new Error('Invoice not found')
+      if (inv.status !== 'draft') {
+        throw new Error(
+          `Only draft invoices can be deleted (status: ${inv.status}). ` +
+          'Issued invoices are immutable per BFL: use gnubok_credit_invoice instead.'
+        )
+      }
+
+      // Unnumbered draft = hard delete (no F-series number was consumed, so
+      // no gap arises); numbered draft = makulering (number retained so the
+      // F-series stays gap-free). The commit executor re-validates status
+      // with TOCTOU write guards, so a draft sent between staging and
+      // approval is refused, never cancelled.
+      const willHardDelete = !inv.invoice_number
+
+      return stagePendingOperation(supabase, companyId, userId, 'delete_draft_invoice',
+        willHardDelete
+          ? 'Ta bort fakturautkast (onumrerat)'
+          : `Makulera fakturautkast ${inv.invoice_number}`,
+        // expected_invoice_number pins the APPROVED outcome: if the draft is
+        // finalized (gains a number) between staging and approval, the
+        // executor refuses instead of silently makulera what the approver saw
+        // as a hard delete.
+        { invoice_id: invoiceId, expected_invoice_number: inv.invoice_number ?? null },
+        {
+          invoice_id: inv.id,
+          invoice_number: inv.invoice_number ?? null,
+          customer_name: (inv.customer as { name?: string } | null)?.name ?? null,
+          total: inv.total,
+          currency: inv.currency,
+          method: willHardDelete
+            ? 'hard delete: the unnumbered draft and its lines are removed; no F-series number was consumed, so no gap arises'
+            : 'makulering: status flips to cancelled and the invoice number is retained, keeping the F-series gap-free',
+        },
+        actor,
+        {
+          description: 'After approval the draft is removed (unnumbered) or makulerad (numbered). Both outcomes are permanent.',
+          tool: 'gnubok_list_invoices',
+        },
+        {
+          dryRun: Boolean(args.dry_run),
+          idempotencyKey: typeof args.idempotency_key === 'string' ? args.idempotency_key : undefined,
+        },
+      )
+    },
+  },
+
  {
    name: 'gnubok_create_sie_upload',
    keywords: ['sie', 'sie-fil', 'importera bokföring'],