fix(mcp): add link_document_to_voucher to pending_operations CHECK constraint (#871)

The gnubok_link_document_to_voucher tool shipped with its executor and
risk tier but its operation type was never added to the
pending_operations_operation_type_check constraint. Every real staging
INSERT failed with check_violation while dry_run previews were clean
(the INSERT is skipped). Reported 3 times by 2 companies via
agent.feedback; blocked Bokio attachment migration.

Adds a pg-real audit test that extracts every op type staged in
server.ts plus all OPERATION_RISK_TIERS keys and asserts each is
accepted by the constraint, so a staging tool can never again ship
without its constraint expansion (and a stale expand-types migration
can no longer silently drop a type).

Part of dev_docs/mcp_optimization_plan.md (P0-1).

Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
Jakob Wennberg
2026-07-03 09:41:31 +02:00
committed by GitHub
parent fb3f0a9cee
commit 4b5fe9ee57
2 changed files with 190 additions and 0 deletions
@@ -0,0 +1,81 @@
-- Add 'link_document_to_voucher' to the pending_operations operation_type
-- CHECK constraint.
--
-- Bug fix (dev_docs/mcp_optimization_plan.md P0-1): the MCP tool
-- gnubok_link_document_to_voucher shipped with its executor
-- (lib/pending-operations/commit.ts) and risk tier
-- (lib/pending-operations/risk-tiers.ts: 'medium') but its operation type was
-- never added to this constraint. Every real staging INSERT was rejected with
-- check_violation, while dry_run=true — which skips the INSERT — always
-- returned a clean preview. Reported 3 times by 2 companies via
-- agent.feedback; blocked the Bokio-attachment-migration flow entirely.
--
-- Risk tier: 'medium' — linking a doc to a posted verifikation becomes part of
-- räkenskapsinformation (BFL 5 kap 6 §) once approved, so a human confirms the
-- pairing; no journal entry is created or modified.
--
-- pg-test: this PR adds tests/pg/pending-operations-op-type-audit.pg.test.ts,
-- which asserts EVERY op type staged in server.ts or tiered in risk-tiers.ts
-- is accepted by this constraint — so a tool can never again ship without its
-- constraint expansion.
--
-- NOTE: the list below is the union with 20260702171000 (retag_line_dimensions,
-- dimensions PR6). An earlier draft of this migration was authored from a
-- checkout that predated that migration and briefly clobbered
-- 'retag_line_dimensions' on prod (repaired the same morning; zero staged
-- retag ops in the window) — the exact hand-copied-list hazard the audit test
-- exists to catch.
ALTER TABLE public.pending_operations
DROP CONSTRAINT IF EXISTS pending_operations_operation_type_check;
ALTER TABLE public.pending_operations
ADD CONSTRAINT pending_operations_operation_type_check
CHECK (operation_type IN (
'categorize_transaction',
'create_customer',
'create_invoice',
'mark_invoice_paid',
'send_invoice',
'mark_invoice_sent',
'match_transaction_invoice',
'close_period',
'lock_period',
'unlock_period',
'set_opening_balances',
'run_year_end',
'run_currency_revaluation',
'import_sie',
'explain_voucher_gap',
'uncategorize_transaction',
'approve_supplier_invoice',
'credit_supplier_invoice',
'credit_invoice',
'convert_invoice',
'create_transaction',
'attach_document_to_transaction',
'create_voucher',
'correct_entry',
'reverse_entry',
'create_supplier',
'create_supplier_invoice_from_inbox',
'post_annual_depreciation',
'link_invoice_voucher',
'undo_sie_import',
'match_batch_allocate',
'bulk_book_transactions',
'create_salary_run',
'generate_agi',
'link_transaction_journal_entry',
'link_supplier_invoice_voucher',
'submit_vat_declaration',
'submit_agi',
'create_article',
'update_article',
'bulk_book_inbox_items',
'create_dimension_value',
'retag_line_dimensions', -- audited retro-tagging of dimension maps on posted lines (dimensions PR6)
'link_document_to_voucher' -- koppla bilaga to a posted verifikat (imported/manual vouchers with no bank-tx row)
));
NOTIFY pgrst, 'reload schema';
@@ -0,0 +1,109 @@
import { readFileSync } from 'fs'
import { resolve } from 'path'
import { beforeAll, describe, expect, it } from 'vitest'
import { OPERATION_RISK_TIERS } from '@/lib/pending-operations/risk-tiers'
import { getPool } from './setup'
import { seedCompany } from './fixtures'
/**
* Audit: every operation type the codebase can stage must be accepted by the
* pending_operations_operation_type_check constraint.
*
* Guards against the bug class where an MCP tool ships with its executor and
* risk tier but without the constraint-expansion migration — the staging
* INSERT then fails with check_violation on every real call while dry_run
* (which skips the INSERT) previews clean. That exact gap shipped with
* gnubok_link_document_to_voucher and went unnoticed until agent feedback
* (dev_docs/mcp_optimization_plan.md P0-1). It also catches the inverse
* hazard: expand-types migrations hand-copy the full list, so one authored on
* a stale branch can silently drop a type added in between.
*
* Op types are collected from BOTH code-side sources of truth:
* 1. literal types passed to stagePendingOperation() in the MCP server
* 2. keys of OPERATION_RISK_TIERS (imported, not parsed)
*/
const SERVER_TS = resolve(__dirname, '../../extensions/general/mcp-server/server.ts')
// Matches `stagePendingOperation(<client>, <companyId>, <userId>, '<op_type>'`
// across line breaks. If the staging signature changes, the call-site count
// assertion below fails loudly — update this regex together with the signature.
const STAGE_CALL_RE = /stagePendingOperation\(\s*[\w.]+,\s*[\w.]+,\s*[\w.]+,\s*'([a-z_]+)'/g
function extractStagedOpTypes(): { types: Set<string>; callSites: number } {
const src = readFileSync(SERVER_TS, 'utf8')
const types = new Set<string>()
let callSites = 0
for (const match of src.matchAll(STAGE_CALL_RE)) {
types.add(match[1])
callSites++
}
return { types, callSites }
}
describe('pending_operations operation_type CHECK audit', () => {
let userId: string
let companyId: string
beforeAll(async () => {
const seeded = await seedCompany()
userId = seeded.userId
companyId = seeded.companyId
})
it('extraction still matches the staging call sites', () => {
const { types, callSites } = extractStagedOpTypes()
// 44 call sites / 43 distinct types as of 2026-07-03. The floor is a
// canary: a big drop means the regex no longer matches the code shape,
// not that tools were removed.
expect(callSites).toBeGreaterThanOrEqual(40)
expect(types.size).toBeGreaterThanOrEqual(40)
})
it('accepts every op type staged in code or tiered in risk-tiers', async () => {
const { types: staged } = extractStagedOpTypes()
const union = new Set([...staged, ...Object.keys(OPERATION_RISK_TIERS)])
const rejected: string[] = []
const client = await getPool().connect()
try {
for (const opType of union) {
await client.query('BEGIN')
try {
await client.query(
`INSERT INTO public.pending_operations (user_id, company_id, operation_type, title)
VALUES ($1, $2, $3, $4)`,
[userId, companyId, opType, `op-type audit: ${opType}`],
)
} catch (err) {
rejected.push(`${opType}: ${(err as Error).message}`)
} finally {
await client.query('ROLLBACK')
}
}
} finally {
client.release()
}
expect(
rejected,
`op types staged in code but rejected by pending_operations constraints — ` +
`add them to pending_operations_operation_type_check in a new migration:\n${rejected.join('\n')}`,
).toEqual([])
})
it('accepts link_document_to_voucher (regression: shipped without constraint expansion)', async () => {
const client = await getPool().connect()
try {
await client.query('BEGIN')
await client.query(
`INSERT INTO public.pending_operations (user_id, company_id, operation_type, title)
VALUES ($1, $2, 'link_document_to_voucher', 'regression: koppla bilaga till verifikat')`,
[userId, companyId],
)
await client.query('ROLLBACK')
} finally {
client.release()
}
})
})