fix: cancel orphan draft when commitEntry fails + add compliance review CI (#302)

createJournalEntry now cancels the draft with a CAS guard (status='draft')
if commitEntry throws, so callers don't leave undeletable stuck drafts when
the commit RPC rejects (balance trigger, period lock, overload ambiguity).

Also adds a PR-triggered GitHub Actions workflow that runs Claude against
the diff using the swedish-* skills as authoritative references and posts
advisory compliance feedback as a PR comment.

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

.github/workflows/swedish-compliance-review.yml

@@ -0,0 +1,44 @@
+name: Swedish Accounting Compliance Review
+
+on:
+  pull_request:
+    types: [opened, synchronize, reopened]
+
+permissions:
+  contents: read
+  pull-requests: write
+
+jobs:
+  review:
+    runs-on: ubuntu-latest
+    steps:
+      - uses: actions/checkout@v4
+        with:
+          fetch-depth: 0
+      - uses: actions/setup-node@v4
+        with:
+          node-version: 20
+      - name: Install Anthropic Bedrock SDK
+        run: npm install --no-save --no-package-lock @anthropic-ai/bedrock-sdk@latest
+      - name: Run compliance review
+        env:
+          AWS_ACCESS_KEY_ID: ${{ secrets.AWS_ACCESS_KEY_ID }}
+          AWS_SECRET_ACCESS_KEY: ${{ secrets.AWS_SECRET_ACCESS_KEY }}
+          AWS_REGION: ${{ secrets.AWS_REGION || 'eu-north-1' }}
+          GITHUB_BASE_REF: ${{ github.base_ref }}
+          REVIEW_MODEL: eu.anthropic.claude-sonnet-4-6
+        run: node scripts/swedish-compliance-review.mjs
+      - name: Find previous compliance comment
+        uses: peter-evans/find-comment@v3
+        id: find-comment
+        with:
+          issue-number: ${{ github.event.pull_request.number }}
+          comment-author: 'github-actions[bot]'
+          body-includes: '<!-- swedish-compliance-review-bot -->'
+      - name: Post or update PR comment
+        uses: peter-evans/create-or-update-comment@v4
+        with:
+          issue-number: ${{ github.event.pull_request.number }}
+          comment-id: ${{ steps.find-comment.outputs.comment-id }}
+          body-path: review.md
+          edit-mode: replace

lib/bookkeeping/__tests__/voucher-atomicity.test.ts

@@ -6,7 +6,7 @@ vi.mock('@/lib/events', () => ({
   eventBus: { emit: vi.fn().mockResolvedValue([]) },
 }))
 
-import { commitEntry, getNextVoucherNumber } from '../engine'
+import { commitEntry, getNextVoucherNumber, createJournalEntry } from '../engine'
 
 describe('voucher number atomicity', () => {
   beforeEach(() => {
@@ -133,3 +133,195 @@ describe('voucher number atomicity', () => {
     })
   })
 })
+
+describe('createJournalEntry orphan draft cleanup', () => {
+  beforeEach(() => {
+    vi.clearAllMocks()
+  })
+
+  /**
+   * Regression test for #292: when commit_journal_entry RPC fails (e.g. overload
+   * ambiguity, balance trigger, period lock), the draft created by createDraftEntry
+   * must be cancelled so it doesn't linger as an undeletable stuck draft.
+   */
+  it('cancels the draft when commit RPC fails', async () => {
+    const draftId = 'entry-1'
+    const cancelUpdate = vi.fn().mockReturnValue({
+      eq: vi.fn().mockReturnValue({
+        eq: vi.fn().mockResolvedValue({ error: null }),
+      }),
+    })
+
+    const supabase = {
+      from: vi.fn().mockImplementation((table: string) => {
+        if (table === 'fiscal_periods') {
+          return {
+            select: vi.fn().mockReturnValue({
+              eq: vi.fn().mockReturnValue({
+                eq: vi.fn().mockReturnValue({
+                  single: vi.fn().mockResolvedValue({
+                    data: { name: 'FY 2025', period_start: '2025-01-01', period_end: '2025-12-31' },
+                    error: null,
+                  }),
+                }),
+              }),
+            }),
+          }
+        }
+        if (table === 'chart_of_accounts') {
+          return {
+            select: vi.fn().mockReturnValue({
+              eq: vi.fn().mockReturnValue({
+                in: vi.fn().mockResolvedValue({
+                  data: [
+                    { account_number: '1930', id: 'acc-1930' },
+                    { account_number: '1510', id: 'acc-1510' },
+                  ],
+                  error: null,
+                }),
+              }),
+            }),
+          }
+        }
+        if (table === 'journal_entries') {
+          return {
+            insert: vi.fn().mockReturnValue({
+              select: vi.fn().mockReturnValue({
+                single: vi.fn().mockResolvedValue({
+                  data: { id: draftId, status: 'draft' as JournalEntryStatus },
+                  error: null,
+                }),
+              }),
+            }),
+            select: vi.fn().mockReturnValue({
+              eq: vi.fn().mockReturnValue({
+                single: vi.fn().mockResolvedValue({
+                  data: { id: draftId, status: 'draft', lines: [] },
+                  error: null,
+                }),
+              }),
+            }),
+            update: cancelUpdate,
+          }
+        }
+        if (table === 'journal_entry_lines') {
+          return {
+            insert: vi.fn().mockResolvedValue({ error: null }),
+          }
+        }
+        return {}
+      }),
+      // commit_journal_entry RPC fails — simulates overload ambiguity or balance error
+      rpc: vi.fn().mockResolvedValue({
+        data: null,
+        error: { message: 'Could not choose the best candidate function' },
+      }),
+    }
+
+    await expect(
+      createJournalEntry(supabase as never, 'co-1', 'user-1', {
+        fiscal_period_id: 'fp-1',
+        entry_date: '2025-06-15',
+        description: 'Payment',
+        source_type: 'invoice_paid',
+        lines: [
+          { account_number: '1930', debit_amount: 1000, credit_amount: 0 },
+          { account_number: '1510', debit_amount: 0, credit_amount: 1000 },
+        ],
+      })
+    ).rejects.toThrow('Failed to commit journal entry')
+
+    // The orphan draft must have been cancelled with CAS guard (status='draft')
+    expect(cancelUpdate).toHaveBeenCalledWith({ status: 'cancelled' })
+    const firstEq = cancelUpdate.mock.results[0].value.eq
+    expect(firstEq).toHaveBeenCalledWith('id', draftId)
+    const secondEq = firstEq.mock.results[0].value.eq
+    expect(secondEq).toHaveBeenCalledWith('status', 'draft')
+  })
+
+  it('surfaces original commit error even if cleanup update fails', async () => {
+    const draftId = 'entry-1'
+
+    const supabase = {
+      from: vi.fn().mockImplementation((table: string) => {
+        if (table === 'fiscal_periods') {
+          return {
+            select: vi.fn().mockReturnValue({
+              eq: vi.fn().mockReturnValue({
+                eq: vi.fn().mockReturnValue({
+                  single: vi.fn().mockResolvedValue({
+                    data: { name: 'FY 2025', period_start: '2025-01-01', period_end: '2025-12-31' },
+                    error: null,
+                  }),
+                }),
+              }),
+            }),
+          }
+        }
+        if (table === 'chart_of_accounts') {
+          return {
+            select: vi.fn().mockReturnValue({
+              eq: vi.fn().mockReturnValue({
+                in: vi.fn().mockResolvedValue({
+                  data: [
+                    { account_number: '1930', id: 'acc-1930' },
+                    { account_number: '1510', id: 'acc-1510' },
+                  ],
+                  error: null,
+                }),
+              }),
+            }),
+          }
+        }
+        if (table === 'journal_entries') {
+          return {
+            insert: vi.fn().mockReturnValue({
+              select: vi.fn().mockReturnValue({
+                single: vi.fn().mockResolvedValue({
+                  data: { id: draftId, status: 'draft' as JournalEntryStatus },
+                  error: null,
+                }),
+              }),
+            }),
+            select: vi.fn().mockReturnValue({
+              eq: vi.fn().mockReturnValue({
+                single: vi.fn().mockResolvedValue({
+                  data: { id: draftId, status: 'draft', lines: [] },
+                  error: null,
+                }),
+              }),
+            }),
+            // Cleanup throws — original error should still propagate
+            update: vi.fn().mockImplementation(() => {
+              throw new Error('Network error during rollback')
+            }),
+          }
+        }
+        if (table === 'journal_entry_lines') {
+          return {
+            insert: vi.fn().mockResolvedValue({ error: null }),
+          }
+        }
+        return {}
+      }),
+      rpc: vi.fn().mockResolvedValue({
+        data: null,
+        error: { message: 'Period is locked' },
+      }),
+    }
+
+    await expect(
+      createJournalEntry(supabase as never, 'co-1', 'user-1', {
+        fiscal_period_id: 'fp-1',
+        entry_date: '2025-06-15',
+        description: 'Payment',
+        source_type: 'invoice_paid',
+        lines: [
+          { account_number: '1930', debit_amount: 1000, credit_amount: 0 },
+          { account_number: '1510', debit_amount: 0, credit_amount: 1000 },
+        ],
+      })
+      // Original commit error surfaces, not the cleanup error
+    ).rejects.toThrow('Period is locked')
+  })
+})

lib/bookkeeping/engine.ts

@@ -285,6 +285,10 @@ export async function commitEntry(
  * Convenience wrapper: creates draft + commits in one step.
  * The voucher number is only assigned after lines are successfully inserted,
  * preventing gaps in the voucher sequence (BFL 5 kap. 7§).
+ *
+ * If commitEntry fails (e.g. balance trigger rejection, period lock, RPC error),
+ * the orphan draft is cancelled so callers don't leave an undeletable stuck draft.
+ * The commit RPC is atomic — no voucher number is burned on failure.
  */
 export async function createJournalEntry(
   supabase: SupabaseClient,
@@ -295,7 +299,23 @@ export async function createJournalEntry(
   rubricVersion?: string
 ): Promise<JournalEntry> {
   const draft = await createDraftEntry(supabase, companyId, userId, input)
-  return commitEntry(supabase, companyId, userId, draft.id, commitMethod, rubricVersion)
+  try {
+    return await commitEntry(supabase, companyId, userId, draft.id, commitMethod, rubricVersion)
+  } catch (commitError) {
+    // CAS guard: only cancel if still in draft. If the RPC actually posted
+    // before failing downstream, immutability trigger blocks draft→cancelled
+    // on a posted row anyway — the filter just avoids firing the trigger.
+    try {
+      await supabase
+        .from('journal_entries')
+        .update({ status: 'cancelled' })
+        .eq('id', draft.id)
+        .eq('status', 'draft')
+    } catch {
+      // Swallow rollback failure — surface the original commit error
+    }
+    throw commitError
+  }
 }
 
 /**

scripts/swedish-compliance-review.mjs

@@ -0,0 +1,184 @@
+#!/usr/bin/env node
+// Runs Claude against the PR diff using all swedish-* skills under
+// .claude/skills/ as the authoritative reference. Writes advisory
+// feedback to review.md for the workflow to post as a PR comment.
+
+import AnthropicBedrock from '@anthropic-ai/bedrock-sdk';
+import { readFileSync, readdirSync, writeFileSync } from 'node:fs';
+import { execSync } from 'node:child_process';
+import path from 'node:path';
+
+const SKILLS_DIR = '.claude/skills';
+const ALWAYS_LOAD = 'swedish-accounting-compliance';
+const MODEL = process.env.REVIEW_MODEL || 'eu.anthropic.claude-sonnet-4-6';
+const MAX_DIFF_CHARS = 180_000;
+const OUTPUT_FILE = 'review.md';
+const COMMENT_MARKER = '<!-- swedish-compliance-review-bot -->';
+
+function loadSkills() {
+  const ids = readdirSync(SKILLS_DIR).filter((n) => n.startsWith('swedish-'));
+  if (!ids.includes(ALWAYS_LOAD)) {
+    throw new Error(`Required skill missing: ${ALWAYS_LOAD}`);
+  }
+  const primary = readFileSync(path.join(SKILLS_DIR, ALWAYS_LOAD, 'SKILL.md'), 'utf8');
+  const others = ids
+    .filter((id) => id !== ALWAYS_LOAD)
+    .map((id) => ({
+      id,
+      content: readFileSync(path.join(SKILLS_DIR, id, 'SKILL.md'), 'utf8'),
+    }));
+  return { primary: { id: ALWAYS_LOAD, content: primary }, others };
+}
+
+function getDiff() {
+  const baseRef = process.env.GITHUB_BASE_REF || 'main';
+  execSync(`git fetch origin ${baseRef} --depth=1`, { stdio: 'ignore' });
+  const mergeBase = execSync(`git merge-base origin/${baseRef} HEAD`).toString().trim();
+  const files = execSync(`git diff --name-only ${mergeBase} HEAD`).toString().trim();
+  let diff = execSync(`git diff ${mergeBase} HEAD`).toString();
+  let truncated = false;
+  if (diff.length > MAX_DIFF_CHARS) {
+    diff = diff.slice(0, MAX_DIFF_CHARS);
+    truncated = true;
+  }
+  return { files, diff, truncated };
+}
+
+function buildSystemPrompt({ primary, others }) {
+  const otherBlocks = others
+    .map((s) => `### Skill: ${s.id}\n\n${s.content}`)
+    .join('\n\n---\n\n');
+
+  return `You are reviewing a pull request in **gnubok**, a Swedish accounting SaaS built in Next.js + TypeScript on top of Supabase. Your job is to flag compliance risks against Swedish accounting law (Bokföringslagen / BFL), BFNAR, tax law, BAS 2026 chart, and VAT rules (ML 2023:200).
+
+You have been given a corpus of compliance skills below. Use them as your authoritative source — prefer them over your training data whenever they conflict.
+
+## Primary skill (ALWAYS consult)
+
+### Skill: ${primary.id}
+
+${primary.content}
+
+---
+
+## Topic-specific skills (consult when relevant)
+
+${otherBlocks}
+
+---
+
+## Your task
+
+1. **Route**: identify which topic-specific skills are relevant to the diff (in addition to the primary skill). State them upfront.
+2. **Review**: produce advisory findings for compliance risks only. Examples of in-scope findings:
+   - BAS account misuse (wrong account number for the purpose, wrong VAT account)
+   - VAT errors (wrong rate, missing reverse charge marker, incorrect ruta mapping, representation moms > 300 SEK deduction)
+   - Accounting guard-rail violations (editing posted entries, direct inserts into journal tables, non-storno corrections)
+   - Retention / WORM violations (deleting documents linked to posted entries, mutable audit rows)
+   - Period-lock bypasses
+   - SIE/SRU encoding or field errors
+   - Year-end / tax calculation errors (periodiseringsfond, överavskrivningar, bolagsskatt, egenavgifter)
+   - Payroll errors (arbetsgivaravgifter rate, skatteavdrag, förmånsbeskattning, semesterlöneskuld)
+   - Invoice field requirements (ML 17 kap 24§), kreditfaktura handling, Peppol/e-faktura
+3. **Cite**: for each finding, cite the specific skill and section that supports it.
+4. **Be concise**: use short bullets. No restating the diff. No style/formatting/naming comments. No praise.
+5. **Allow the empty case**: if nothing in the diff touches compliance (pure UI tweak, refactor of non-accounting code, docs, tests), say so in one line and stop.
+6. **Never fabricate a rule**: if uncertain, mark as "unsure" rather than asserting.
+
+## Output format
+
+Start with the marker literal ${COMMENT_MARKER} on its own line.
+
+Then:
+
+\`\`\`
+## Swedish Accounting Compliance Review
+
+**Skills consulted**: <comma-separated list including ${primary.id}>
+
+### Findings
+
+- **[SKILL_ID] <one-line summary>** — <1–3 sentence explanation with file:line references and the fix>.
+
+(or: "No compliance concerns in this diff — changes are outside the scope of the Swedish accounting skills.")
+
+### Notes (optional)
+
+<Only if there's something worth flagging that isn't a hard finding, e.g. "worth double-checking with swedish-vat skill if the customer is EU-based">
+\`\`\`
+
+Render no emojis. Do not wrap the final output in a code fence.`;
+}
+
+function buildUserMessage({ files, diff, truncated }) {
+  const note = truncated
+    ? `\n\n> Note: diff exceeded ${MAX_DIFF_CHARS} chars and was truncated. Review is based on the first ${MAX_DIFF_CHARS} chars only.`
+    : '';
+  return `## Changed files
+
+\`\`\`
+${files}
+\`\`\`
+
+## Diff
+
+\`\`\`diff
+${diff}
+\`\`\`${note}`;
+}
+
+async function main() {
+  if (!process.env.AWS_ACCESS_KEY_ID || !process.env.AWS_SECRET_ACCESS_KEY) {
+    writeFileSync(
+      OUTPUT_FILE,
+      `${COMMENT_MARKER}\n\n## Swedish Accounting Compliance Review\n\nSkipped: AWS Bedrock credentials (\`AWS_ACCESS_KEY_ID\` / \`AWS_SECRET_ACCESS_KEY\`) are not set.\n`,
+    );
+    console.warn('AWS credentials missing — wrote skip notice and exiting 0.');
+    return;
+  }
+
+  const skills = loadSkills();
+  const { files, diff, truncated } = getDiff();
+
+  if (!diff.trim()) {
+    writeFileSync(
+      OUTPUT_FILE,
+      `${COMMENT_MARKER}\n\n## Swedish Accounting Compliance Review\n\nNo diff detected against the base branch.\n`,
+    );
+    return;
+  }
+
+  const client = new AnthropicBedrock({
+    awsRegion: process.env.AWS_REGION || 'eu-north-1',
+    awsAccessKey: process.env.AWS_ACCESS_KEY_ID,
+    awsSecretKey: process.env.AWS_SECRET_ACCESS_KEY,
+  });
+  const system = buildSystemPrompt(skills);
+  const user = buildUserMessage({ files, diff, truncated });
+
+  const resp = await client.messages.create({
+    model: MODEL,
+    max_tokens: 4096,
+    system,
+    messages: [{ role: 'user', content: user }],
+  });
+
+  const text = resp.content
+    .filter((b) => b.type === 'text')
+    .map((b) => b.text)
+    .join('\n')
+    .trim();
+
+  const body = text.startsWith(COMMENT_MARKER) ? text : `${COMMENT_MARKER}\n\n${text}`;
+  writeFileSync(OUTPUT_FILE, body + '\n');
+  console.log(`Wrote ${OUTPUT_FILE} (${body.length} chars, model=${MODEL}).`);
+}
+
+main().catch((err) => {
+  console.error('Compliance review failed:', err);
+  writeFileSync(
+    OUTPUT_FILE,
+    `${COMMENT_MARKER}\n\n## Swedish Accounting Compliance Review\n\nReview failed: \`${String(err.message || err)}\`. This is advisory only — the PR is not blocked.\n`,
+  );
+  process.exit(0);
+});