2c59c3633f2e934619afcacd792ec53dc2fa45ba
229 Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
|
|
2c59c3633f |
feat(invoices): cross-currency settlement + payment-status card (#615)
* feat(invoices): cross-currency settlement + payment-status card Two changes both surfaced by user feedback after PR #614: # 1. Invoice detail page: Betalningsstatus card The customer-invoice detail page now shows paid_amount + remaining_amount + the individual payment events whenever an invoice is partially_paid or paid (was previously only a single "Paid" line on fully-paid invoices, and nothing at all on partially_paid). Mirrors the supplier-invoice page's payment section. Each payment row links to its verifikat. # 2. Cross-currency match-invoice settlement Replaces the PR #614 round-9 block (MATCH_INVOICE_CURRENCY_MISMATCH) with proper FX-aware settlement. Flow: 1. Preview route detects tx.currency !== invoice.currency, fetches the Riksbanken spot rate for invoice.currency on tx.date (ML 8 kap 21–23§), and returns fx_conversion = { rate, rate_date, paid_in_invoice_currency }. When the lookup fails it returns fx_conversion.error = 'rate_unavailable'. 2. InvoiceMatchDialog renders a new Valutaomräkning card showing the rate + invoice-currency-equivalent + projected post-payment state + a one- line kursvinst/kursförlust note. When the lookup failed it swaps in a manual-rate input the user fills from their bank statement; the Confirm button blocks until a positive rate is supplied. 3. POST route does the same lookup (or accepts manual_exchange_rate from the request body), then: - paidInInvoiceCurrency = bankSek / rate (4dp precision) - invoice.paid_amount/remaining_amount accumulate in invoice currency - invoice_payments row records amount + currency = invoice.currency, exchange_rate = the rate actually used (not invoice.exchange_rate) - buildInvoicePaymentClearingLines gets paidInInvoiceCurrency so it credits 1510 by that × invoice.exchange_rate (booking rate) and posts the FX-diff line on 3960 (gain) or 7960 (loss) 4. buildInvoicePaymentClearingLines gains an optional fourth param. When supplied: proportional FX-aware AR-leg + balanced FX-diff. When omitted: pre-existing fallback (full-clear gets FX, partials defer). The change fixes the invoice.paid_amount accumulator bug that PR #614 round-9 worked around by blocking the case entirely. Now SEK→USD settlements actually work, with the verifikat balanced to the öre and the GL+sub-ledger in sync per BFL 5 kap 4–5§. Tests: - 3 new helper tests (paidInInvoiceCurrency happy path + edge cases) - 3 new route tests (Riksbanken happy path, lookup failure, manual rate) - All 4321 tests pass Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(invoices): align cross-currency match preview with commit + review cleanups Addresses PR #615 review feedback. Preview/commit divergence (Greptile P1): preview/route.ts computed paidAmount / isFullyPaid / useCashEntry from the raw SEK transaction.amount before the FX conversion ran. A 1 000 SEK payment against a 140 USD invoice made max(0, 140 − 1000) = 0 → is_fully_paid=true, so a cash-method unbooked invoice previewed a cash entry (Dr 1930 / Cr 30xx) while the POST handler — which converts first — commits the clearing entry (Dr 1930 / Cr 1510). The user approved one verifikat and a different one was booked. Move the FX lookup above the paid/remaining math so paidAmount derives from the invoice-currency conversion, mirroring the POST handler. Rate-unavailable stays non-fully-paid so the cash shape is never previewed on a guess. Add a preview-route regression test (cross-currency → clearing + not fully paid; same-currency cash path still previews the cash entry). Cleanups: - Bound manual_exchange_rate with .max(100000) as a sanity ceiling against pasted/garbage input corrupting the FX-diff posting (swarm V2.3). - Remove the invisible disabled placeholder retry button and its unused fx_manual_rate_retry i18n keys (Greptile P2). - Remove the now-unreachable MATCH_INVOICE_CURRENCY_MISMATCH error code (Greptile P2 dead code; confirmed zero references). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(invoices): record FX rate provenance + cover kursförlust path Follow-up to the PR #615 review (compliance swarm V16 / SOC 2 CC6.1 / GDPR Art.5(1)(f); Swedish accounting review). A manually-supplied cross-currency rate is a user-controlled money-path override of the ML 8 kap 21–23§ obligation and was indistinguishable from an automatic Riksbanken lookup in the audit trail. Tag the resolved rate with source: 'manual' | 'riksbanken' and: - write a "Manuell valutakurs <rate> <ccy>/SEK (betalningsdatum …)" note onto the existing invoice_payments.notes column when manual (BFL 5 kap 6–7§ — the verifikation must reflect the actual affärshändelse); - record rate_source + exchange_rate in payment_match_log.new_state. No schema change — both are existing columns/JSON. Tests: - cover the kursförlust (7960 Dr) branch of the cross-currency paidInInvoiceCurrency path — previously only the 3960 gain was asserted; - assert rate_source provenance ('manual' and 'riksbanken') reaches the match-log new_state on both FX paths. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com> |
||
|
|
13be0c569a |
feat(mcp): expose multi-tx RPCs (match_batch_allocate + bulk_book_transactions) (#614)
* feat(bulk-book): manual booking mode + document inheritance Two pieces of user feedback from PR #606: 1. "How come it is only mallar? Is it not possible to have manuell bokfoering?" - BulkBookDialog was template-only. Added a Tabs primitive with Mall / Manuell tabs. Manual tab pre-fills lines from the selected txs (one line per tx on 1930 + counterparty placeholder on 3001/5800 by direction), then the user edits Konto / Debet / Kredit / Beskrivning. Live balance + bank-leg checks drive the confirm button - same invariants the RPC enforces server-side. 2. "Documents attached does not follow into the bookkeeping. And if there are two different documents attached, none of them follow." The bulk_book_transactions RPC now propagates each tx's document onto the target verifikat (new in Branch B, existing in Branch A) as verifikationsunderlag. Per BFL 5 kap 6§ + BFNAR 2013:2 kap 4 a verifikat may have multiple underlag; every receipt that justified a tx is now retention-protected on the combined entry. The dialog shows a small count chip ("N bilagor foeljer med") so the user sees what will inherit. Also dropped p_user_id from the RPC signature (round-3 hardening pattern applied consistently across all multi-tx RPCs after PR #607). Caller resolves from auth.uid() inside the function. Schema: BulkBookSchema is now a 3-way XOR (existing_journal_entry_id | template_id+mode | manual_lines), with manual_lines validated as accountNumber + nonNegativeAmount per line. pg-real tests: - doc inheritance into a new combined verifikat (mixed: 2 of 3 txs have docs - docs_linked should be 2, not 3) - doc inheritance into an existing posted verifikat (link branch) - manual lines path (no template expansion artifacts in the resulting JE - just the 2 user lines) - unbalanced manual lines still rejected by BULK_BOOK_UNBALANCED Migration applied to remote. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(bulk-book): PR #610 review - pg-real signature, account allowlist, account-number validity Three review findings on PR #610: 1. pg-real failure: 2 link-existing tests still used 5-arg SELECT bulk_book_transactions($1::uuid[], $2, $3, $4, $5) after the userId removal. My earlier replace_all caught only the patterns that had ::jsonb on $3; the link-existing tests pass null for new_entry and used a bare $3 so they slipped through. (Greptile P1) 2. Manual lines bypassed chart_of_accounts validation. A typo or adversarial caller could post to a BAS account that doesn't exist in this company's chart, corrupting the hauptbok and breaking SIE export. Both compliance-swarm (OWASP V2.3) and swedish-compliance flagged this. Added a single-roundtrip allowlist check in the route: query chart_of_accounts for distinct account_numbers in manual_lines and reject with BULK_BOOK_INVALID_ACCOUNT if any are missing or inactive. 3. UI canConfirm guard missed invalid account numbers. Account input allows 1-3 digits and JS string comparison '193' >= '1900' is false, so a 3-digit entry escapes bankLineNet, the bank match could pass via other lines, and the server returned 400 only after submit. Added previewLines.every(l => /^\d{4}$/.test(l.account_number)) to canConfirm so the Confirm button stays disabled inline. (Greptile P2) Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(bulk-book): PR #610 round 2 - RPC chart-of-accounts, doc tenant isolation, GRANTs Seven compliance findings from the round-1 bot reviews: Migration (20260602121000_bulk_book_round2_fixes.sql): - RPC chart-of-accounts allowlist (defense-in-depth): every line in p_new_entry.lines is now verified to be an active BAS account for p_company_id. Closes the gap where the template branch and direct DB callers (psql, future MCP) bypassed the route's manual-branch check. Returns BULK_BOOK_INVALID_ACCOUNT with the offending list. (OWASP V8.2.1 + SOC 2 CC6.3) - Document inheritance CTE: added "AND d.company_id = p_company_id" to the UPDATE join so the tenant isolation is enforced on both sides (tx + doc), not just the tx side. Four bots converged on this finding (V1.2.5, A.8.2, CC6.6, swedish-compliance). - Bank-leg range check: "length(account_number) = 4 AND account_number BETWEEN '1900' AND '1999'" replaces the bare lexicographic comparison. Lexicographic-on-4-digit is safe today; the length guard is defense-in-depth against schema drift. (swedish-compliance) - Explicit role grants: REVOKE ALL FROM PUBLIC + GRANT EXECUTE TO authenticated on both bulk_book_transactions and match_batch_allocate. (SOC 2 CC6.1) UI (BulkBookDialog): - Manual-mode prefill no longer suggests a hardcoded 3001/5800 counterpart. Reason (swedish-compliance): a user accepting the prefill could submit a verifikat with no VAT line (26xx), under-reporting utgaaende moms. The bank side stays pre-filled (unambiguous); the counterpart row scaffolds blank for the user to choose. Schema (BulkBookSchema): - manual_lines.debit_amount + credit_amount bounded at 99,999,999 SEK per line. Catches typos before the RPC. (compliance-swarm V4.5) i18n: - docs_inherit_hint terminology: "bilaga" -> "verifikationsunderlag" and an explicit "sparas i 7 ar enligt BFL 7 kap" reminder. swedish-compliance flagged that "bilaga" risks users treating the files as deletable attachments rather than retention-bound raekenskapsinformation. Migration applied to remote. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(test): seed chart_of_accounts in bulk-book pg-real seedTenant The round-2 RPC fix added a chart_of_accounts allowlist check inside bulk_book_transactions, but the test fixtures don't seed COA — so every existing test that submits lines (1930, 3001, 2611, etc.) now returns BULK_BOOK_INVALID_ACCOUNT instead of the expected error code. Seed the 8 accounts the suite actually uses directly in seedTenant (cheaper than calling seed_chart_of_accounts which inserts the full BAS 2026 chart). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * feat(mcp): expose match_batch_allocate + bulk_book_transactions as MCP tools Surfaces the multi-tx flows shipped in PRs #603/#606/#608/#610 so Claude Desktop/Code can drive them via chat. - migration 20260603120000: expand pending_operations.operation_type CHECK to include match_batch_allocate, bulk_book_transactions, plus undo_sie_import (which was missing from prior expansions despite being wired in risk-tiers.ts and the commit dispatcher). - types/index.ts: extend PendingOperationType. - lib/pending-operations/risk-tiers.ts: match_batch_allocate = medium (same tier as single-tx match), bulk_book_transactions = high (creates a verifikat with arbitrary lines, same surface as create_voucher). - lib/pending-operations/commit.ts: thin commit handlers that call the SQL RPCs and translate the structured error envelope. The RPCs themselves do all the locking, balance checks, JE creation, voucher number, payment/junction rows, and doc inheritance. - extensions/general/mcp-server/server.ts: two new tool definitions. Both stage via stagePendingOperation with period_status hint and pre-validate inputs (direction, sum-equals-tx-abs, same-date, not-already-booked) so the agent gets a clear error inline before the RPC runs. - payload-size.bench: bump from 30K to 31K tokens (with rationale). Two new tools earn the bump; descriptions already trimmed to fit the <=280-char description limit. Migration applied to remote and version aligned with local filename. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(mcp): PR #614 review - allocation guard, IDOR pre-check, currency + JE-date Round-1 review fixes on PR #614: - Greptile P1: per-allocation invoice_id / supplier_invoice_id guard. The inputSchema marks both as optional (they're mutually exclusive by kind), so JSON Schema can't express "X required iff Y=A". Added explicit check in the execute handler: customer_invoice rows must carry invoice_id; supplier_invoice rows must carry supplier_invoice_id. - OWASP V8.2.1: IDOR pre-check on match_batch_allocate. Verify every invoice / supplier_invoice referenced in the allocations belongs to this company BEFORE staging. The RPC re-checks (BATCH_INVOICE_NOT_FOUND), but failing fast at the MCP layer gives the agent a clear error. - OWASP V8.2.1: same pre-check on bulk_book_transactions for existing_journal_entry_id. Fetches the JE at stage time, verifies status=posted and company_id, throws if not found. - swedish-compliance: currency homogeneity check on bulk_book. Mixed SEK + EUR in one samlingsverifikat violates BFL 5 kap 6§ st 3 motpart clarity. Cross-currency batches go through match_batch_allocate instead (which handles FX diff on 7960/3960). - swedish-compliance: period-lock check on the link-existing branch now uses MAX(tx_date, JE.entry_date), not just tx_date. Otherwise a tx in an open period could attach to a verifikat in a locked period and the guard would miss it. - A.8.11 + CC7.2: sanitised RPC error logging. log.error now emits only { code, message } instead of the full error object — error.details can echo invoice IDs, amounts, and counterparty identifiers. Not actioned (PR-comment, no code change): - V2.3 double-validation in commit handler — RPC enforces balance, accounts, bank-leg via the chart_of_accounts allowlist (PR #610 round 2). Commit handler is a thin pass-through by design. - A.8.2 step-up approval for high-tier ops — architectural change affecting all high-tier ops, not PR-scoped. - V2.4 rate limiting on bulk endpoints — platform-level concern. - 0.005 epsilon / account-class allowlist — pre-existing patterns. - undo_sie_import storno requirement — separate RPC, this PR only backfilled the missing CHECK constraint. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(mcp): PR #614 round 2 - trust-boundary comments + balance pre-check + audit log Round-2 review fixes (compliance-swarm went 14 -> 9 after round 1; remaining HIGHs are all "do the same tenant check at multiple layers"). The bot itself offers the alternative: "or document and reference the specific RPC line that enforces this." Following that. - commit.ts: trust-boundary comment blocks on both commitMatchBatchAllocate and commitBulkBookTransactions, citing the exact RPC + migration where tenant isolation + chart_of_accounts allowlist are enforced authoritatively. The commit handler stays a thin pass-through by design; re-querying would triple the same check without adding security. (V8.2.1, A.8.2) - commit.ts: structured success-path log.info() on both handlers with companyId, operationType, journal_entry_id, and tx count. No raw amounts or IDs that could echo PII. (V16) - server.ts: balance pre-check on bulk_book create-new path. RPC enforces BULK_BOOK_UNBALANCED authoritatively, but failing fast at staging gives the agent a clear error before pending_operations is even touched. (V2.3 / swedish-compliance) Not actioned this round: - V2.2 oneOf/if-then-else in JSON Schema for mutual exclusivity — JSON Schema vocabulary support is shaky across MCP clients; runtime check in execute() is the canonical pattern across the existing toolset. - CC6.1 generic error string to caller — RPC error codes are user-actionable (BULK_BOOK_UNBALANCED, BATCH_INVOICE_NOT_FOUND); a generic string would degrade UX. - CC7.2 audit RPC RAISE messages for PII — separate audit; not PR-scoped. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(mcp): PR #614 round 3 — last 5 LOWs + salary_run/agi constraint backfill Compliance-swarm went 14 → 9 → 5 (all LOW). Cleaning the last 5 + the swedish-compliance findings. - migration 20260603121000: backfill create_salary_run + generate_agi into pending_operations.operation_type CHECK. Both have risk-tier entries and commit executors but were never added (same bug class as undo_sie_import). Production has no rows of either type today. (swedish-compliance) - server.ts: Number.isFinite guard in bulk_book balance pre-check. Number(x) || 0 silently treats NaN as 0 — a malformed amount could pass the balance check by accident. (compliance-swarm A.8.28) - server.ts: count-equality + missing-set assertion in match_batch_allocate tenant pre-check. Belt-and-suspenders so a null/undefined row in the Supabase JSON response can't pass silently. Same pattern on both invoice and supplier_invoice branches. (CC6.1) - server.ts: fix BFL paragraph citation in currency-homogeneity comment. Was "BFL 5 kap 6§ st 3", should be "BFL 5 kap 2§" (SEK denomination) read with 5 kap 6§ (valutakurs). (swedish-compliance) - server.ts: clarify 0.005 tolerance comment — it's for floating-point equalisation only, not a rounding allowance. RPC enforces exact balance to the öre. (swedish-compliance) - commit.ts: expand audit-log txId comment — included intentionally for trail-to-source join, scoped to companyId already logged. (compliance-swarm A.8.15/CC7.2) Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(mcp): PR #614 round 4 — Swedish plural typo + balance comment parity + agent-routing hint Round-3 review caught: - swedish-compliance: \`kundfakturaor\` typo (real räkenskapsinformation defect under BFL 5 kap 7§). Swedish plural for \`kundfaktura\` is \`kundfakturor\` (drop the final \`a\`, add \`or\`), same for \`leverantörsfaktura\` → \`leverantörsfakturor\`. Fixed via slice(-1) + 'or'. - swarm A.8.28: match_batch_allocate balance tolerance check was missing the equivalent "RPC enforces exact balance" comment that bulk_book has. Added. - swedish-compliance: currency-mismatch error message now routes the agent to gnubok_match_batch_allocate for cross-currency allocations instead of letting it retry with hand-built FX lines. Not actioned (out of pattern / out of scope): - Integer arithmetic for balance checks (codebase pattern is float + epsilon; would diverge from match_batch_allocate, supplier-payment, invoice-payment, etc.) - DSD docs / runbook for txId-in-log and stripped-error.details trade-offs (out of PR scope; tracked separately) - Link-existing target verifikat description match (architectural; every link-existing op would need this) Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * feat(mcp): expose link_transaction_to_journal_entry as MCP tool The REST endpoint /api/transactions/[id]/link-journal-entry already lets the duplicate-payment UI attach a bank tx to an already-posted verifikat without creating new bookkeeping. Agents had no equivalent — closing that parity gap so users on Claude can match bank txs against vouchers they booked manually. The core link logic moves to lib/transactions/link-journal-entry.ts so both the REST route and the new commit handler share one implementation (preserves all structured-error codes, optimistic-lock invoice update, and compensating rollback). New 'link_transaction_journal_entry' op type wired through the risk tiers (medium), TOOL_SCOPE_MAP (transactions:write), and dispatcher. Bumps the tools/list payload-size ceiling 31K → 31.5K — same family bump PRs #603/#606 made when adding match_batch_allocate / bulk_book_transactions. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(mcp): PR #614 round 5 — bot findings on link_transaction_journal_entry Addresses the swedish-compliance + compliance-swarm findings on commit 5b884c3a: 1. **CHECK constraint backfill** — new migration adding 'link_transaction_journal_entry' to pending_operations.operation_type. Same bug class as the salary_run/agi backfill in 20260603121000; without it, every staged op would be rejected silently in production (BFL 5 kap 6–7§ audit-trail gap). 2. **Payment-date exchange rate** — invoice_payments.exchange_rate now uses transaction.exchange_rate (rate on payment date) instead of invoice.exchange_rate (rate on invoice date), per BFL 5 kap 2§ + ML 8 kap 21–23§. The full 3960/7960 posting still belongs to createInvoicePaymentJournalEntry by contract — this path only links to an EXISTING verifikat. 3. **voucherLabel format centralized** — exported formatVoucherLabel helper returns the canonical `A-12` format (with hyphen, matches gnubok_link_invoice_to_voucher and SIE #VER cross-references). Both the MCP staging preview and the committed service result import it, so the user can't approve one label and have a different one land in the audit trail. 4. **Rollback warn log restored** — txLog.warn-equivalent (IDs only, no PII) when the compensating rollback itself fails, surfacing partial-state gaps for reconciliation per GDPR Art.5(1)(f) / SOC 2 CC7.2. Lost in the refactor that extracted the shared service; now present in both rollback call sites. 5. **Commit-layer log.info** — structured success log mirroring commitMatchBatchAllocate / commitBulkBookTransactions (companyId, tx/JE IDs, settledInvoice boolean). No raw amounts or counterparty names. 6. **Data minimization on invoice fetch** — explicit column list replaces select('*, customer:customers(name)') in the shared service; the MCP staging pre-check now fetches only invoice_number + remaining_amount (drops total + paid_amount). voucher_description omitted from preview_data per Art.25. Test impact: existing route + dispatcher tests updated to expect `A-12` instead of `A12`. All 4308 tests pass. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(invoices): correct FX bookkeeping + UI for match-invoice flow User report: matching a 230 SEK bank tx against a 140 USD invoice produced 1930 Dr 2 142,50 / 1510 Cr 2 142,50 — fictitious numbers that didn't match either the bank receipt or the booked AR. Root cause: the preview route called resolveSekAmount(tx.amount, null, INV.currency, INV.rate), treating the SEK tx number as if it were in the invoice's currency and multiplying by the invoice's stored rate. Both the preview and the commit then used the bogus number on both legs and silently dropped the FX gain/loss. A second issue surfaced in the same dialog: for a 1 250 SEK invoice with a prior 230 SEK partial, the comparison row showed "Differens: 250 kr" (off the original total) instead of "20 kr" (off the actual 1 020 kr remaining). This patch: 1. **New shared helper** lib/bookkeeping/invoice-payment-lines.ts - buildInvoicePaymentClearingLines(tx, invoice, description) → bank-leg, AR-leg, fx-diff, and a balanced line array. Bank-leg is always the actual SEK that hit the bank (resolveSekAmount with the TX's currency context, honouring tx.amount_sek when set). AR-leg is the SEK value of the customer-debt reduction at the invoice's stored rate. Diff posts to 3960 (gain) or 7960 (loss) so the verifikat balances per BFL 5 kap 4–5§. Mirrors the match_batch_allocate RPC's contract: when the tx is cross-currency, the single match fully clears the invoice's remaining amount. 2. **Preview route** uses the helper for the clearing branch — replaces the buggy resolveSekAmount call. Now byte-identical to what commit builds. 3. **Match-invoice POST** uses the helper + createJournalEntry directly for the clearing path, bypassing createInvoicePaymentJournalEntry on this single flow. mark-paid and other callers of that function still work as before (full payment + caller-supplied exchangeRateDifference). 4. **InvoiceMatchDialog** compares the bank tx against invoice.remaining_amount (not invoice.total) for both customer and supplier branches; cross-currency dialogs now show the different- currencies warning instead of a meaningless numeric diff. The dialog's invoice card also displays remaining_amount. 8 new unit tests cover same-currency full/partial, cross-currency gain/loss, exact match (no FX line), sub-öre tolerance, and USD-on-USD with pre- populated amount_sek. All 4316 tests pass. Scope note: this expands PR #614 beyond the original "expose multi-tx RPCs as MCP tools" since the same FX bug class affected the new MCP tool too (round 5 already addressed the invoice_payments.exchange_rate side). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(mcp): PR #614 round 7 — CI build + 4 HIGH bot findings Core Build was failing on e29a0ba2/5e9d4c3d due to a TypeScript type-cast error in linkTransactionToJournalEntry. Plus the swedish-compliance review flagged four substantive bugs in my recent commits. 1. **TS build error** — `invoice = invoiceRow as typeof invoice` inferred `never` because the LHS type included `null`. Switched to a named `FetchedInvoice` alias and `as unknown as FetchedInvoice`. 2. **TOOL_SCOPE_MAP missing two write-capable tools** (🟠 HIGH OWASP V8.2.1). `gnubok_match_batch_allocate` and `gnubok_bulk_book_transactions` (added in PRs #603/#606) were never registered, meaning any API key could invoke them regardless of scope. Backfilled both with `transactions:write`. 3. **`paymentExchangeRate` fallback wrong-date rate** (swedish-compliance). `transaction.exchange_rate ?? invoice.exchange_rate ?? null` falls back to the INVOICE date's rate when the tx rate is null. Per ML 8 kap 21–23§ the payment row must record the PAYMENT-date rate. Removed the fallback — `null` is correct when the tx is SEK; downstream lookups can populate it lazily from Riksbanken if needed. 4. **Currency-mismatch corrupts paid_amount** (swedish-compliance). The link path was accumulating `tx.amount` into `invoice.paid_amount` without checking that the currencies matched. A 230 SEK tx applied to a USD invoice would record "230 USD paid" silently. Added explicit LINK_TX_INVOICE_CURRENCY_MISMATCH guard (400) — cross-currency settlement must go through the match-invoice flow which routes through buildInvoicePaymentClearingLines. 5. **Cross-currency PARTIAL overstates FX gain/loss** (swedish-compliance, BFL 5 kap 4–5§). `buildInvoicePaymentClearingLines` was crediting the FULL invoice remaining to 1510 on every cross-currency match — zeroing the GL balance while the invoice row stayed at status=partially_paid, and booking a fake huge FX diff to 3960/7960. Fix: only book FX-diff when `bankSek >= arSekFullRemaining`. Partials default to 1930 = 1510 = bankSek, deferring the FX adjustment to the final settlement (or to a manual mark-paid with explicit exchange_rate_difference). Documented the helper as customer-invoice- only (supplier-side has different DR/CR polarity and goes through match_batch_allocate RPC). Test impact: 1 helper test updated to match the defer-on-ambiguous-loss behavior, 1 new test covers the partial-defers-FX path explicitly. All 4317 tests pass. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(mcp): PR #614 round 8 — close out remaining bot findings CI green on round 7 (4 of 4 checks), HIGH count 2 → 1. Round-8 closes the remaining HIGH and the smaller doc/guard items. 1. **PI1.3 risk acknowledgment restored** (SOC 2 HIGH). The shared rollbackTxLink helper already had warn-level logging on rollback failure, but the explicit PI1.3 reference comment from the original route was lost in the refactor. Added inline so the reconciliation- gap risk is visible to future maintainers. 2. **MCP currency-mismatch pre-stage check.** gnubok_link_transaction_to_ journal_entry now fetches invoice.currency and rejects cross-currency matches before staging, saving the user an approval round-trip when the commit handler's LINK_TX_INVOICE_CURRENCY_MISMATCH guard would fire anyway. 3. **fxDiffSek JSDoc clarified.** The sign convention (positive = loss, negative = gain) is correct for verifikat balancing but counter- intuitive at a P&L glance. Documented explicitly + pointed callers needing a "gain" number at `bankSek - arSek`. 4. **Reject both invoice_id + supplier_invoice_id** on the same match_batch_allocate row (V4.5). Extra IDs previously leaked into preview_data silently. 5. **Reject zero-amount tx** in bulk_book_transactions direction guard (A.8.28). A txs[0].amount === 0 would have mis-classified the batch as 'expense'. Mirrors the existing guard in match_batch_allocate. 6. **Reject debit=0 && credit=0 lines** in bulk_book new_entry (BFL 5 kap 6§ — every verifikat line must represent a real bokföringspost with a non-zero amount). 7. **Data-minimization comments** added on the match-invoice preview route (amount_sek + exchange_rate fetch is for the FX-fix bank-leg math) and on the bulk_book_transactions preview_data block (aggregate counts only — no per-tx PII). Mirrors the pattern already documented on gnubok_link_transaction_to_journal_entry. Skipped: - 1510 vs 1515 (osäkra kundfordringar) — future improvement, needs reading the original invoice JE's account, not a single-tool fix. - transaction_description PII masking in preview_data — needs product call on the truncation strategy and would degrade approval-UX. - "invoice.match_confirmed event removed" finding — false positive; the event is emitted at lib/transactions/link-journal-entry.ts:270-280. All 4317 tests pass; payload-size guard still under ceiling. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(invoices): PR #614 round 9 — block cross-currency in single match-invoice path Closes the swedish-compliance finding from round-8 review: a SEK bank tx matched against a USD invoice through /api/transactions/[id]/match-invoice would silently corrupt invoice.paid_amount (accumulator treats SEK as USD) and flip a 140 USD invoice to status='paid' after a tiny partial. The round-6/7 FX fix corrected the JOURNAL ENTRY lines but the invoice STATE update still ran the same broken accumulator. Proper cross-currency settlement on this path requires converting tx.amount to invoice.currency at the bank-date rate AND storing invoice_payments rows with the right (amount, currency) pair. That's a larger design call that belongs in its own PR. This change blocks cross-currency on the single-allocation path: - New MATCH_INVOICE_CURRENCY_MISMATCH structured error (400, bilingual) - Same-currency check inserted right after MATCH_INVOICE_NOT_OPEN - Mirrors the LINK_TX_INVOICE_CURRENCY_MISMATCH guard added to the link path in round-7 - Routes the user to the multi-allocation flow (gnubok_match_batch_allocate) which DOES handle 3960/7960 FX-diff postings end-to-end Same-currency (SEK→SEK or USD→USD) remains fully supported including partials; the buildInvoicePaymentClearingLines helper handles those correctly. For SEK tx → USD invoice the user now gets a clean 400 error pointing at the right flow, instead of silently corrupted ledger state. 1 new route test covers the guard. All 4318 tests pass. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com> |
||
|
|
28f7cefc86 |
feat(bulk-book): manual booking mode + document inheritance (#610)
* feat(bulk-book): manual booking mode + document inheritance Two pieces of user feedback from PR #606: 1. "How come it is only mallar? Is it not possible to have manuell bokfoering?" - BulkBookDialog was template-only. Added a Tabs primitive with Mall / Manuell tabs. Manual tab pre-fills lines from the selected txs (one line per tx on 1930 + counterparty placeholder on 3001/5800 by direction), then the user edits Konto / Debet / Kredit / Beskrivning. Live balance + bank-leg checks drive the confirm button - same invariants the RPC enforces server-side. 2. "Documents attached does not follow into the bookkeeping. And if there are two different documents attached, none of them follow." The bulk_book_transactions RPC now propagates each tx's document onto the target verifikat (new in Branch B, existing in Branch A) as verifikationsunderlag. Per BFL 5 kap 6§ + BFNAR 2013:2 kap 4 a verifikat may have multiple underlag; every receipt that justified a tx is now retention-protected on the combined entry. The dialog shows a small count chip ("N bilagor foeljer med") so the user sees what will inherit. Also dropped p_user_id from the RPC signature (round-3 hardening pattern applied consistently across all multi-tx RPCs after PR #607). Caller resolves from auth.uid() inside the function. Schema: BulkBookSchema is now a 3-way XOR (existing_journal_entry_id | template_id+mode | manual_lines), with manual_lines validated as accountNumber + nonNegativeAmount per line. pg-real tests: - doc inheritance into a new combined verifikat (mixed: 2 of 3 txs have docs - docs_linked should be 2, not 3) - doc inheritance into an existing posted verifikat (link branch) - manual lines path (no template expansion artifacts in the resulting JE - just the 2 user lines) - unbalanced manual lines still rejected by BULK_BOOK_UNBALANCED Migration applied to remote. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(bulk-book): PR #610 review - pg-real signature, account allowlist, account-number validity Three review findings on PR #610: 1. pg-real failure: 2 link-existing tests still used 5-arg SELECT bulk_book_transactions($1::uuid[], $2, $3, $4, $5) after the userId removal. My earlier replace_all caught only the patterns that had ::jsonb on $3; the link-existing tests pass null for new_entry and used a bare $3 so they slipped through. (Greptile P1) 2. Manual lines bypassed chart_of_accounts validation. A typo or adversarial caller could post to a BAS account that doesn't exist in this company's chart, corrupting the hauptbok and breaking SIE export. Both compliance-swarm (OWASP V2.3) and swedish-compliance flagged this. Added a single-roundtrip allowlist check in the route: query chart_of_accounts for distinct account_numbers in manual_lines and reject with BULK_BOOK_INVALID_ACCOUNT if any are missing or inactive. 3. UI canConfirm guard missed invalid account numbers. Account input allows 1-3 digits and JS string comparison '193' >= '1900' is false, so a 3-digit entry escapes bankLineNet, the bank match could pass via other lines, and the server returned 400 only after submit. Added previewLines.every(l => /^\d{4}$/.test(l.account_number)) to canConfirm so the Confirm button stays disabled inline. (Greptile P2) Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(bulk-book): PR #610 round 2 - RPC chart-of-accounts, doc tenant isolation, GRANTs Seven compliance findings from the round-1 bot reviews: Migration (20260602121000_bulk_book_round2_fixes.sql): - RPC chart-of-accounts allowlist (defense-in-depth): every line in p_new_entry.lines is now verified to be an active BAS account for p_company_id. Closes the gap where the template branch and direct DB callers (psql, future MCP) bypassed the route's manual-branch check. Returns BULK_BOOK_INVALID_ACCOUNT with the offending list. (OWASP V8.2.1 + SOC 2 CC6.3) - Document inheritance CTE: added "AND d.company_id = p_company_id" to the UPDATE join so the tenant isolation is enforced on both sides (tx + doc), not just the tx side. Four bots converged on this finding (V1.2.5, A.8.2, CC6.6, swedish-compliance). - Bank-leg range check: "length(account_number) = 4 AND account_number BETWEEN '1900' AND '1999'" replaces the bare lexicographic comparison. Lexicographic-on-4-digit is safe today; the length guard is defense-in-depth against schema drift. (swedish-compliance) - Explicit role grants: REVOKE ALL FROM PUBLIC + GRANT EXECUTE TO authenticated on both bulk_book_transactions and match_batch_allocate. (SOC 2 CC6.1) UI (BulkBookDialog): - Manual-mode prefill no longer suggests a hardcoded 3001/5800 counterpart. Reason (swedish-compliance): a user accepting the prefill could submit a verifikat with no VAT line (26xx), under-reporting utgaaende moms. The bank side stays pre-filled (unambiguous); the counterpart row scaffolds blank for the user to choose. Schema (BulkBookSchema): - manual_lines.debit_amount + credit_amount bounded at 99,999,999 SEK per line. Catches typos before the RPC. (compliance-swarm V4.5) i18n: - docs_inherit_hint terminology: "bilaga" -> "verifikationsunderlag" and an explicit "sparas i 7 ar enligt BFL 7 kap" reminder. swedish-compliance flagged that "bilaga" risks users treating the files as deletable attachments rather than retention-bound raekenskapsinformation. Migration applied to remote. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(test): seed chart_of_accounts in bulk-book pg-real seedTenant The round-2 RPC fix added a chart_of_accounts allowlist check inside bulk_book_transactions, but the test fixtures don't seed COA — so every existing test that submits lines (1930, 3001, 2611, etc.) now returns BULK_BOOK_INVALID_ACCOUNT instead of the expected error code. Seed the 8 accounts the suite actually uses directly in seedTenant (cheaper than calling seed_chart_of_accounts which inserts the full BAS 2026 chart). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com> |
||
|
|
ea1bf01f1e |
Fix/m sprint fixes (#613)
* fix(dashboard): exclude ignored and already-triaged transactions from stale count The "Gamla transaktioner" widget counted transactions that had been ignored or already marked as is_business=true but not yet booked, so users saw a nag for a row they had already dealt with — and the /transactions inbox correctly hid it. Align the count with the inbox criterion (is_business IS NULL, is_ignored = false) so the widget clears when the row leaves the inbox. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(transactions): read entity_type from settings response wrapper The transactions page read entityRes.entity_type directly, but /api/settings returns { data: { entity_type, ... } }. The expression was always undefined, so setEntityType never fired and entityType stayed at its initial 'enskild_firma'. The template picker's entity_type filter then dropped every aktiebolag-tagged user template for AB customers — only entity_type='all' templates made it through. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * stale templates bank sync journal entry from transaction * fixed pr comments * fixed pr comment --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com> |
||
|
|
fc7a46c3f2 |
fix(match-batch): cross-currency allocations + widened tolerance (#607)
* fix(match-batch): cross-currency allocations + widened tolerance Reported by jakob testing PR #603's MatchAllocationDialog with a SEK bank tx + a mix of SEK and USD invoices: 1. Tally rendered "1 USD + 1 SEK = 2 kr" — summing different currencies as if they were the same. 2. The 0.005 SEK tolerance blocked confirm on any FX rounding delta. ## What changed **UI (MatchAllocationDialog.tsx)** - Per-row amount input is explicitly in TRANSACTION currency (SEK for a Swedish bank import). Cross-currency rows show an "≈ X.XX (invoice currency)" hint under the input so the user can verify the FX result. - Default amount for a cross-currency allocation is `invoice.remaining × invoice.exchange_rate` (booked SEK), so the user doesn't have to mental-math the FX. - Overshoot tolerance widened from 0.005 SEK to `max(1 SEK, 0.5% × tx)` so bank-side FX rounding doesn't block confirm. A 2 400 kr tx now accepts ~12 kr of tolerance, a 100 kkr transfer accepts 500 kr. **RPC (match_batch_allocate cross_currency migration)** - BATCH_CURRENCY_MISMATCH dropped per-allocation. Mixed currencies now accepted with the convention that the cross-currency row pays the FULL invoice remaining (matches the single-tx match-supplier-invoice behavior). Partial cross-currency is out of scope for v1. - AR/AP line is booked at `invoice.remaining × invoice.exchange_rate` (the SEK that was originally on 1510/2440). FX residual is posted to 7960 (Valutakursförluster) or 3960 (Valutakursvinster) per BAS. - Sign conventions per direction documented inline: Customer: bank > booked → Cr 3960 (gain); bank < booked → Dr 7960 Supplier: bank < booked → Cr 3960 (gain); bank > booked → Dr 7960 - New BATCH_FX_RATE_MISSING when the cross-currency invoice has no exchange_rate on file (would otherwise silently book at 0). - New BATCH_FX_DEVIATION_TOO_LARGE when the user-entered amount deviates more than 10% from booked SEK — catches typos like "140" (USD invoice currency) when they meant "1390" (SEK equivalent) without rejecting genuine rate-day FX movement. RPC patched on remote via Supabase MCP. Same-currency path is byte-identical to the previous behavior. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(match-batch): PR review — strict sum, bank line = tx_abs, FX validation Round-1 review fixes on the cross-currency batch allocation flow: UI (MatchAllocationDialog): - Tighten tolerance to 0.005 SEK so the "balanced ✓" indicator matches what the server will accept. The previous widened tolerance (max 1 SEK or 0.5% × tx) created a reconciliation gap where the JE's bank line could legitimately disagree with the actual bank receipt. - Require balanced before confirm — undershoot is now a blocking state with an explicit warning, not a silent "leave unallocated". - Cross-currency default no longer caps at remainingTxBudget. Capping a USD invoice's default to the leftover SEK budget could silently trigger BATCH_FX_DEVIATION_TOO_LARGE on submit. The user re-balances the other rows to fit. - Add explicit FX-rate validation (bound check 0 < rate < 100000). - When a cross-currency invoice has no usable exchange_rate on file, leave the amount blank and surface a warning instead of guessing. RPC (match_batch_allocate): - New code BATCH_AMOUNT_BELOW_TX. Strict sum check on both sides means the server can't be coaxed by a direct API caller into the same broken state the UI now blocks. - Bank line credit/debit = v_tx_abs (the actual bank movement) instead of sum-of-allocations. Same value within rounding under the strict sum check, but it makes intent legible and lets per-row FX diff lines absorb rounding. - Defense-in-depth company_id filter on all re-queries / UPDATEs in the line-build + payment-row passes. - Drop the v_booked_sek-aliasing-for-invoice.total foot-gun. Use a dedicated v_inv_total var. - Truncate invoice_number to 32 chars in line_description. Tests: - pg-real: cross-currency happy path (USD invoice paid by SEK tx with FX loss to 7960, bank line = tx_abs). - pg-real: BATCH_AMOUNT_BELOW_TX rejection on undershoot. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(match-batch): PR review round 2 - caller user_id verification + FX bound Compliance-swarm + swedish-compliance findings on round 1: - CC6.3 (HIGH): p_user_id was caller-supplied and written into journal_entries.user_id / payment-row user_id without verifying it equals auth.uid(). Membership covered the company; nothing covered the user attribution. Two-layer fix: explicit guard rejects when p_user_id <> auth.uid(), and all writes now resolve v_caller = auth.uid() directly so the guard cant be silently bypassed. - A.8.28 (MED): server-side FX upper-bound (0 < rate < 100000) matches the UI. Previously RPC only checked > 0, allowing the UI guard to diverge. - V1.2.5 (LOW): truncate v_tx.date when concatenated into line_description (defense alongside round 1s invoice_number trunc). - Symmetry: populate supplier_invoice_payments.exchange_rate (column existed, INSERT omitted it). Customer side already populated. Matches swedish-compliances traceability note on AP rorelseskulder. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(match-batch): PR review round 3 - drop p_user_id, CHECK constraints, payment-day rate Genuine round-2 review findings (compliance-swarm + swedish-compliance): - V4.5: p_user_id dropped from RPC signature entirely. Round-2 added a guard; this removes the attack surface at the API boundary. Caller is resolved via auth.uid() inside the function. Route updated. - V2.2: CHECK constraint on invoices.exchange_rate and supplier_invoices.exchange_rate (0 < rate < 100000). Three layers now enforce the bound: schema, RPC, UI. - swedish-compliance traceability gap: payment_exchange_rate column on both invoice_payments and supplier_invoice_payments. Populated as v_alloc_amount / v_inv_remaining for cross-currency rows so FX diffs are reconstructible from the payment record alone (BFL 7 kap behandlingshistorik). NULL for same-currency. The existing exchange_rate column continues to store the invoicing rate. - CC6.1: extract isValidExchangeRate() to lib/utils.ts. UI's three inline bound checks now share one validator. - Dead code: drop unused leftover_note i18n key (sv + en). Tests: - pg-real signature updated (4-arg -> 3-arg) across all 9 call sites. - Added payment_exchange_rate assertion to cross-currency happy path (invoicing rate 10.0 stays, payment-day rate stored as 10.5). Migration applied to remote. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(test): missed 4th arg in BATCH_UNAUTHORIZED pg-real test Round-3 dropped p_user_id from match_batch_allocate. The replace_all caught the userId/companyId pattern but missed the BATCH_UNAUTHORIZED test which uses outsiderId instead of userId. CI failed with "bind message supplies 4 parameters, but prepared statement requires 3". Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com> |
||
|
|
4da87e5e4c |
feat(transactions): bulk-book + is-booked predicate (#606)
* feat(transactions): bulk-book + is-booked predicate Closes the second of the two multi-tx ↔ multi-voucher flows from the original plan. Where PR #603's match_batch_allocate took 1 tx and spread it across N invoices (samlingsbetalning), this PR takes N bank transactions on the same day and rolls them up into ONE combined verifikat (samlingsverifikation per BFL 5 kap 6§ st 3) — the kiosk masshantering pattern the user explicitly asked for. ## Backend (Phase 3b) - **PL/pgSQL RPC** bulk_book_transactions: two branches, both atomic. 1. Link to existing posted verifikat (p_existing_journal_entry_id): no new JE. Validates the JE's 19xx net equals sum(tx.amount), inserts N transaction_voucher_links rows, and for N=1 also sets transactions.journal_entry_id (1:1 reader-path back-compat). 2. Create new combined verifikat (p_new_entry with pre-computed balanced lines): the route's applyTemplate() has already done ratio + VAT expansion per the chosen mode. The RPC validates the lines balance and the 1930 net matches sum(tx.amount), then commits via commit_journal_entry. Same security pattern as match_batch_allocate: company-member check via auth.uid(), SELECT … FOR UPDATE on each tx in id order, deterministic fiscal-period resolution (ORDER BY period_start DESC). - **Endpoint** POST /api/transactions/bulk-book — fetches template via RLS, expands per mode (one_line_per_tx | sum_per_account) using lib/bookkeeping/template-library.applyTemplate, passes the resulting lines to the RPC. On success emits one transaction.reconciled event per tx. - **22 new BULK_BOOK_* error codes** (sv + en) covering all guard paths. ## UI (Phase 5b) - **BulkBookDialog** — template picker + mode toggle (segmented control: en rad per transaktion / summera per konto) + live preview table with balance + bank-leg invariant indicators. Confirm only enabled when both pass. - **Multi-select inbox** — sticky action bar gains a "Bokför i klump" button gated by same-date + same-direction across selected txs. Tooltip explains the disabled state. ## Phase 6: is-booked predicate New lib/transactions/is-booked.ts. After multi-allocation and bulk- book, tx.journal_entry_id can be NULL even though the tx is anchored (via invoice_payments / supplier_invoice_payments / transaction_voucher_links). The helper checks all three storage locations so future readers don't falsely show multi-anchored txs as "unbooked". Companion getPrimaryJournalEntryId() resolves the best JE link to surface in UI. SQL mirror is_transaction_booked() exists from the PR #602 foundation migration. Existing readers (TransactionHistoryList, TransactionInboxCard) are not yet refactored to use the helper — that's a follow-up that touches per-tx JE links across multiple call sites. The helper is documented + tested so subsequent refactors are mechanical. ## Tests - tests/pg/bulk-book-transactions.pg.test.ts — 8 pg-real scenarios (happy path create-new with 3 txs, happy path link-existing, date mismatch, direction mismatch, amount mismatch, unbalanced lines, unauthorized). - app/api/transactions/bulk-book/__tests__/route.test.ts — 5 unit tests (schema XOR, link path, create-new with template fetch + applyTemplate, structured-error mapping). - lib/transactions/__tests__/is-booked.test.ts — 11 cases covering all three storage locations + primary-JE resolution. 26 unit tests pass on touched paths. RPC migration applied to remote via Supabase MCP. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(bulk-book): PR #606 review round 1 + CI fixes Closes the build failure and the two real Greptile findings. ## CI - **core-only + Vercel build fail**: I used useMemo for selectedTransactions and bulkBookEligible on the transactions page without importing it. TypeScript build (`next build`) caught it with "Cannot find name 'useMemo'". Fixed the import. ## Review findings - **(P1) Currency mismatch returned BULK_BOOK_DIRECTION_MISMATCH** whose user-facing message blames direction. Mixed SEK + EUR batches would show "All transactions must be the same direction" which is factually wrong. Introduced dedicated BULK_BOOK_MIXED_CURRENCY code (sv + en) explaining the actual constraint, and switched the route to use it. - **(P1) Branch B (create-new) N=1 missed reconciliation_method='manual'**. Branch A's N=1 UPDATE sets it alongside journal_entry_id; Branch B's didn't, leaving the reconciliation_method NULL even though the single tx was reconciled via the same flow. Downstream readers (reconciliation reports, status indicators) would treat the two N=1 paths differently. New follow-up migration patches Branch B's final UPDATE. RPC patch applied to remote via Supabase MCP. 26 unit tests pass. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com> |
||
|
|
7eb8715417 |
feat(transactions): split-payment allocator — 1 tx → N invoices (#603)
* fix(category-mapping): use leaf BAS accounts instead of group codes 3900, 5800, 6200 are BAS gruppkonton (header codes) and shouldn't carry postings. Switched the default mappings to the matching leaf accounts: - income_other: 3900 -> 3999 (Övriga rörelseintäkter) - expense_travel: 5800 -> 5890 (Övriga resekostnader) - expense_telecom: 6200 -> 6230 (Datakommunikation) The fallback for income_other inside getCategoryAccountMapping was also hardcoded to '3900'; updated to '3999' for consistency. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * feat(transactions): split-payment allocator — 1 tx → N invoices Closes one of the two flows that motivated PR #602's foundation: allocating a single bank transaction across multiple customer OR multiple supplier invoices, with one combined verifikat (samlingsverifikation per BFL 5 kap 6§ st 3). ## Backend (Phase 3a) - **PL/pgSQL RPC** match_batch_allocate (~400 lines): locks the tx + each target invoice with SELECT … FOR UPDATE in id order, validates status/currency/remaining/direction before any write, builds the combined verifikat via commit_journal_entry (atomically assigns voucher_number + flips draft→posted), inserts N rows in invoice_payments or supplier_invoice_payments pointing at the same JE, advances paid_amount/remaining_amount/status per invoice. Returns { ok, journal_entry_id, voucher_number, allocations: [...] } on success or { ok: false, code, details } on guard failure. Mixed customer+supplier kinds are rejected (v1 scope). - **Endpoint** POST /api/transactions/[id]/match-batch — thin wrapper around the RPC. Validates body via MatchBatchSchema (zod discriminatedUnion + superRefine to catch mixed-kinds at the schema layer). On RPC success, emits one invoice.match_confirmed or supplier_invoice.match_confirmed event per allocation so existing subscribers (reminders, automations, processing-history) keep working. Maps the structured RPC error envelope to errorResponseFromCode. - **16 new BATCH_* error codes** (sv+en): BATCH_TX_NOT_FOUND, BATCH_TX_ALREADY_BOOKED, BATCH_OVERSHOOT, BATCH_AMOUNT_EXCEEDS_TX, BATCH_MIXED_KINDS_UNSUPPORTED, BATCH_DIRECTION_MISMATCH, BATCH_CURRENCY_MISMATCH, BATCH_PERIOD_LOCKED, BATCH_RPC_FAILED, etc. ## UI (Phase 5a) - **MatchAllocationDialog** (components/transactions/) — direction- aware (positive tx → customer invoices, negative → supplier). Search + selectable list of open invoices. Per-row amount input with default = min(invoice.remaining, tx_remaining_budget). Live tally with green-check balanced state, red overshoot warning, gray leftover note. Confirm button disabled on overshoot. POSTs to /match-batch and on 200 triggers the same exit animation as single-tx match. - **Inbox row** gains a second outline icon button (Split icon) next to the existing 1:1 match button, gated by the same showInvoiceMatchButton predicate. Tooltip explains the direction- aware split. Opens MatchAllocationDialog. - **i18n** strings under tx_match_allocation namespace in sv.json and en.json (32 keys each). ## Tests - tests/pg/match-batch-allocate.pg.test.ts — 5 pg-real tests covering combined verifikat shape, overshoot guard, already-booked tx, direction mismatch, mixed-kinds rejection. - app/api/transactions/[id]/match-batch/__tests__/route.test.ts — 5 unit tests covering schema validation, mixed-kinds, happy path, structured-error mapping, raw-error → BATCH_RPC_FAILED. 63 unit tests pass across the touched paths. The RPC migration was already applied to remote in an earlier Phase 3a session (idempotent CREATE OR REPLACE FUNCTION; the next replay is a no-op). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(match-batch): PR #603 review round 1 + CI fixes Closes both CI failures and the three real review findings. ## CI fixes - **pg-real failure**: the RPC declared `v_journal_entry_id uuid := uuid_generate_v4()` which fails in the CI Postgres image (uuid-ossp extension is off). Switched to `gen_random_uuid()` — the codebase standard already used by supplier_invoices, invoice_inbox, etc. - **core-only failure**: my earlier BAS leaf-account commit (3900→3999, 5800→5890, 6200→6230) didn't update the matching `lib/bookkeeping/__tests__/category-mapping.test.ts` expectations, and `getDefaultAccountForCategory`'s fallback for `income_*` was still hardcoded to '3900'. Updated both. ## Review findings (greptile) - **P1 deadlock-stable locking** (`match_batch_allocate.sql:11`): the validation `FOR UPDATE` loop ran in caller-supplied array order. Two concurrent calls with overlapping invoice sets in opposite orders could deadlock and one would abort with `BATCH_RPC_FAILED`. Now all three loops (validate, build lines, advance invoices) iterate via `SELECT … FROM jsonb_array_elements(…) ORDER BY COALESCE(invoice_id, supplier_invoice_id)`, giving a stable global lock order regardless of how the caller ordered the JSON array. - **P1 duplicate-allocation detection** (`match_batch_allocate.sql:163`): the same invoice_id listed twice would pass the per-row overshoot guard (both iterations read the original `remaining_amount`) and the write loop would insert two `invoice_payments` rows for the same invoice. Added a `v_seen_ids text[]` check in the validation loop and a new `BATCH_DUPLICATE_ALLOCATION` error code (sv + en). The dialog already prevents this UI-side via `if (prev[candidate.id] return prev` — the RPC guard is the defense-in-depth layer. - **P2 zod `.positive()`** (`schemas.ts:544`): allocation amount was `nonNegativeAmount` (allowing 0), passing schema validation only to be rejected by the RPC with `BATCH_INVALID_AMOUNT`. Now `z.number().positive(…)` so 0-amount entries fail at the schema layer with a per-field path, cleaner 400. - **P2 strict `> 0` direction check** (`MatchAllocationDialog.tsx:82`): used `amount >= 0` to pick customer-side, but a zero-amount tx would load customer candidates only to hit `BATCH_TX_ZERO_AMOUNT` at submit time after the user has filled in allocations. Switched to `> 0` so 0-amount tx never reaches the dialog at all (it's rejected by the RPC immediately). The fourth Greptile comment (the schema P2 about amount validation) overlaps with the third; addressed in the same edit. ## Verification - 112 unit tests pass across touched paths - ESLint clean - New pg-real test `tests/pg/match-batch-allocate.pg.test.ts` covers the dedupe scenario (same supplier invoice listed twice with summing amounts that individually pass per-row overshoot) - RPC patch applied to remote via Supabase MCP Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(match-batch): PR #603 review round 2 — compliance hardening Addresses the actionable findings from compliance-swarm and Swedish-accounting-compliance reviews. Six small RPC changes + two TS-side guards, all bundled in one follow-up migration. ## Security - **(GDPR Art.5(1)(f) / ISO A.8.2) Caller verification**: SECURITY DEFINER bypasses RLS, and the prior RPC accepted any (p_user_id, p_company_id) pair from the route. Now the function rejects with new `BATCH_UNAUTHORIZED` (sv+en, HTTP 403) if `auth.uid()` is not a member of `p_company_id`. Pattern lifted from `harden_invoice_number_rpcs` (#20260510140000). - **(OWASP V4.2) Allocation cap**: `MatchBatchSchema.allocations` now carries `.max(100)` to prevent DoS via unbounded FOR UPDATE locks. ## Swedish accounting correctness - **source_type per direction**: was hardcoded to `'invoice_paid'` for both customer + supplier batches, mis-routing behandlingshistorik filters. Customer batches keep `'invoice_paid'`, supplier batches now write `'supplier_invoice_paid'`. - **Fiscal-period determinism**: `LIMIT 1` on the period lookup was non-deterministic on overlap (e.g. corrected broken year). Added `ORDER BY period_start DESC` so the most recent matching period wins. - **Tolerance harmonisation**: cross-allocation sum used `+0.01` tolerance while per-row used `+0.005`. Both now `+0.005` so a multi-row batch can't drift ~0.01 SEK while each row passes individually. - **`transactions.category` no longer overwritten**: was forced to `'income_services'` (→ BAS 3001 at 25% VAT) for any customer batch, misrepresenting reduced-rate / export / EU-service invoices. The category is only meaningful 1:1 with a single invoice; batches now leave it as-is, mirroring the supplier-side `ELSE category` branch. ## Tests - `tests/pg/match-batch-allocate.pg.test.ts` now wraps every RPC call in `withUserContext(userId)` so `auth.uid()` resolves to the seeded owner. Without this the new membership check would have failed all existing tests. - New pg-real test: `rejects with BATCH_UNAUTHORIZED when caller is not a member of the company` — outsider user gets explicit refusal. - New happy-path assertion: `source_type = 'supplier_invoice_paid'` on the combined verifikat for supplier batches. 15 unit tests pass on the touched paths. RPC patch applied to remote via Supabase MCP. Out-of-scope mcp-server changes still parked locally. Skipped findings (documented in PR comment thread): - V8.2.1 ownership pre-check at route layer (RPC enforces it) - V4.5 / Art.5(1)(b) narrower API response and event payload — typed contracts require the full shapes - V2.4 rate-limiting — system-level, applies to all match endpoints - A.8.28 client-side RLS reliance — documented architectural choice - Direction pre-check at API layer (RPC catches with cleaner code) - V16 + Art.32 + Art.5(1)(b) low-severity logging nits Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com> |
||
|
|
7bcd46d503 |
feat(transactions): match overshoot guards + supplier voucher linking (#602)
* feat(transactions): match overshoot guards + supplier voucher linking
Three changes that together close the "I can't link a bank transaction
to an already-booked verifikat on the supplier side" gap and fix a
latent data-corruption bug on the per-tx match endpoints.
1. fix: clamp paid_amount on match endpoints when tx > remaining
/api/transactions/[id]/match-{invoice,supplier-invoice} previously
used transaction.amount wholesale as the paid amount, pushing
invoice.paid_amount past invoice.total whenever the bank tx was
larger than what was owed. Both endpoints now reject with
MATCH_AMOUNT_EXCEEDS_REMAINING / MATCH_SI_AMOUNT_EXCEEDS_REMAINING
and a structured { transaction_amount, remaining_amount, excess }
payload that points the user at the future split-payment flow.
FX branch already clamps to invoice.remaining_amount and is
unchanged.
2. feat: supplier-side "link existing verifikat" (mirror of #591)
lib/invoices/supplier-voucher-matching.ts mirrors the customer
voucher-matching module: finds posted JEs that debit 2440
(Leverantörsskulder), validates currency + remaining-amount, and
atomically links them as supplier_invoice_payments rows. New
/api/supplier-invoices/[id]/{voucher-candidates,link-to-voucher}
routes wrap it. LinkVoucherPicker gains a mode='supplier_invoice'
prop so the same component renders both flows. The supplier-invoice
mark-paid dialog now uses Tabs ("Ny betalning" / "Befintlig
verifikation") to match the customer-side UX.
3. infra: transaction_voucher_links junction + denorm guard
Foundation migration for upcoming multi-tx ↔ multi-voucher flows.
Adds the junction table (with RLS, updated_at, indexes), a
block_contradictory_invoice_denorm trigger on transactions that
refuses to set invoice_id/supplier_invoice_id to a value that
contradicts an existing payment row, and is_transaction_booked(uuid)
as a single source of truth for "is this tx anchored?" once
multi-allocation leaves denorm columns NULL. No application code
uses these yet — they unlock the batch allocation and bulk-book
flows in follow-up PRs.
Tests: 98 unit tests pass across the touched paths (match-invoice,
match-supplier-invoice, supplier-voucher-matching, link-to-voucher).
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* fix(supplier-invoices): PR review — atomic link RPC, computeRemaining edge case, pg-real tests
Addresses the three real issues raised by Greptile on PR #602.
1. (P1) Atomic supplier voucher linking — new
link_supplier_invoice_to_voucher PL/pgSQL RPC. The TS-side
linkSupplierInvoiceToVoucher() previously did UPDATE-then-INSERT with
a manual unconditional rollback. Under concurrent linking against the
same invoice, request A's rollback could overwrite a sibling B's
successful write while leaving B's payment row in place. Moving both
writes into a single PG transaction (one RPC call) lets PG's own
rollback handle the failure path correctly. TS wrapper now just
translates the structured RPC return into the lib's Result type.
2. (P1) pg-real tests — tests/pg/transaction_voucher_links.pg.test.ts.
CLAUDE.md mandates *.pg.test.ts for any PR adding a trigger, RPC, or
RLS. The Phase 1A foundation migration added all three but had no
pg-real coverage. Tests now cover:
- trg_block_contradictory_invoice_denorm refusing contradictory
UPDATEs on invoice_id and supplier_invoice_id
- the same trigger PERMITTING a matching UPDATE (no false positives)
- is_transaction_booked() returning true via journal_entry_id, via
invoice_payments, and via transaction_voucher_links rows.
3. (P2) computeRemaining edge case — trust remaining_amount whenever
the column is non-null (including the legitimate 0 for fully-paid
invoices). The old "> 0" guard fell through to total - paid_amount,
which under rounding drift could compute a tiny positive residue and
slip a fully-paid invoice past LINK_SI_VOUCHER_INVOICE_FULLY_PAID.
The fourth Greptile comment (overdue invoices silently get no
candidates) was a misread: 'overdue' IS in the open-state list at
route.ts:35. No code change needed there.
Tests: 100 unit tests pass (16 in the directly-touched paths).
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* fix(supplier-invoices): PR review round 2 — broaden AP range, log event failures
Addresses the actionable findings from the compliance-swarm and
Swedish-accounting-compliance bot reviews on PR #602.
1. (swedish-accounting-compliance, high) AP account hardcoded to 2440
rejected legitimate samlingsverifikationer that debit 2441
(Leverantörsskulder i utländsk valuta), 2443 (Skuldfakturor), etc.
BAS 2026 reserves the full 2440–2449 range for Leverantörsskulder.
The TS-side AP_ACCOUNT constant becomes AP_ACCOUNT_PREFIX ('244')
used with .like() and .startsWith(). The PL/pgSQL RPC's
account_number filter becomes LIKE '244%'. The
LINK_SI_VOUCHER_NO_AP_DEBIT error message updates to reference the
244x range with examples.
2. (ISO 27001:2022 A.8.15 / OWASP V16) Empty catch on the
supplier_invoice.paid event emission now logs with log.warn so a
failure in the downstream reminder/audit subscriber leaves an
auditable trail without blocking the response.
3. (GDPR Art.5(1)(c)) Documented design rationale for retaining
select('*') on the post-link invoice re-fetch: the
supplier_invoice.paid event payload is typed as
`supplierInvoice: SupplierInvoice` in lib/events/types.ts, narrowing
would break the subscriber contract. The event stays in-process
and consumers legitimately need the full context.
Skipped findings:
- V8.2.1 ownership concerns: route + RPC already filter by
company_id from withRouteContext; the RPC's WHERE clause covers it.
- DELETE policy scoping: matches the gnubok pattern across all
company-scoped tables — any member with write access manages records.
- transaction_id = NULL on the voucher-link path: by design — the
flow has no bank tx (the voucher's 1930 line represents it).
- Reverse-charge VAT (2614/2647) validation on linked vouchers:
real concern but invasive change; tracked for follow-up.
- Storno-chain integrity (linking the original of a storno pair):
edge case; tracked for follow-up.
Tests: 26 unit tests pass in the directly-touched paths. RPC patch
applied to remote via Supabase MCP.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
||
|
|
a586cc8a58 |
feat(transactions): show all library templates in picker; fix PSD2 seed-row collision (#596)
* feat(transactions): show all library templates in picker; fix PSD2 seed-row collision - Booking template picker: surface every active library template, not only the convertible 2-account shapes. Multi-leg/complex templates route to the manual journal editor pre-filled via applyTemplate instead of being hidden. Drop direction filtering for user templates (inferred direction is unreliable); the curated static catalog still respects it. Add an "Aktivera och bokfor" recovery toast for TX_CATEGORIZE_INVALID_ACCOUNT mirroring the existing ACCOUNTS_NOT_IN_CHART flow. CreateTemplateForm reflows to one card per line so trash buttons stop colliding on narrow screens. - cash_accounts.upsertFromPsd2: the seed_default_cash_account migration plants a manual (bank_connection_id IS NULL) row on the same ledger_account, so the first PSD2 sync's upsert on (company_id, bank_connection_id, external_uid) cannot match it (NULL != NULL) and falls through to INSERT, tripping the (company_id, ledger_account) UNIQUE constraint. Look up and promote the seed row in place first. - Tests: pin the TX_CATEGORIZE_INVALID_ACCOUNT error shape the recovery toast parses; cover applyTemplate on shapes the converter rejects (split-expense and all-'business'-typed). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(transactions): PR review — currency metadata, MRU ordering, observable promote, BAS validation - TransactionBookingDialog.buildInitialLinesFromTemplate: attach buildCurrencyMetadata to settlement lines for foreign-currency transactions so the journal entry retains the original currency, amount, and exchange_rate. Without this, non-SEK transactions routed through a non-convertible template were recorded in SEK only with no foreign-currency annotation. (Greptile #2) - TemplatePicker.handleSelectLibraryRaw: only bump the MRU after confirming the click will actually do something (i.e. converted OR a callback is wired). Future consumers that omit onPickLibraryTemplate would otherwise corrupt MRU ordering for templates the user never successfully applied. (Greptile #1) - cash_accounts.upsertFromPsd2 promote-seed: add .select('id') so a zero-row UPDATE is observable. If the seed row vanishes between the SELECT and UPDATE (concurrent ops), fall through to the normal upsert instead of silently returning success without persisting anything. (Greptile #3, compliance A.8.9) - transactions/page.tsx TX_CATEGORIZE_INVALID_ACCOUNT toast: validate accountNumber against /^\d{4}$/ before embedding in any fetch URL/body. Defense-in-depth against a malformed server error envelope. (compliance V8.2.1) - categorize route test: switch the not-in-chart fixture from '4535' (Inköp av varor från annat EU-land — reverse-charge) to '5420' (Programvaror) so the example doesn't imply a domestic override against an EU-reverse-charge account would be valid without its paired moms legs. (Swedish compliance review #3) Other review items deliberately not addressed in this PR: - Validate-on-save that every VAT-rated template has a 'vat' line — overrides existing CreateTemplateForm UX, separate PR. - vat_rate enum guard in applyTemplate — defensive; the editor dropdown only surfaces legal rates and is the only write path in production today. - Imbalance UI warning — already handled: JournalEntryForm computes isBalanced and gates submission; DB trigger check_journal_entry_balance enforces BFL 5 kap server-side. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com> |
||
|
|
ccdfed5fea |
feat: voucher linking, recovery ops, and salary overrides (#591)
* feat: voucher linking, recovery ops, and salary overrides Adds reversible/correction-style write paths that customers and agents have been asking for, plus per-run salary employee overrides. Invoice → voucher linking - POST /api/invoices/[id]/link-to-voucher and GET /api/invoices/[id]/voucher-candidates - lib/invoices/voucher-matching.ts with full + pg test coverage - LinkVoucherPicker UI in PaymentBookingDialog - pending_operations.operation_type expanded with link_invoice_voucher (medium risk) and a (journal_entry_id, invoice_id) unique guard - MCP: gnubok_find_voucher_candidates_for_invoice and gnubok_link_invoice_to_voucher tools SIE undo - POST /api/import/sie/[id]/undo + undo_sie_import RPC - sie_imports.status gains 'undone' - ImportResultStep surfaces the action; structured error SIE_UNDO_FAILED Edit-recreate journal entries - POST /api/bookkeeping/journal-entries/[id]/edit-recreate - Bookkeeping detail page wires it into the existing edit flow Delete-last-voucher clears IB link - Trigger + pg test ensure deleting the last voucher of a period nulls the opening_balance_journal_entry_id link so a re-import lands cleanly Salary employee overrides - salary_run_employees gains per-run override fields + migration - lib/salary/effective-values.ts centralises resolved values; all payslip, payment, AGI, KU, and booking routes read through it - SalaryOverridePanel on the employee detail page Account classifier - lib/bookkeeping/account-classifier.ts + tests; AddAccountDialog uses it - backfill-import-accounts script updated Misc - toast: minor styling tweak - AGI generate-declaration: respect effective values - structured-errors: new LINK_INVOICE_VOUCHER namespace Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * feat: add link_invoice_voucher operation type to pending_operations * feat: refactor salary run calculations and update error handling for SIE imports * fix: PR review feedback on voucher linking and SIE recovery pg-real (blocking): - tests/pg/delete-last-voucher-ib: drop posted_at = now() from the seed UPDATE — journal_entries has no posted_at column. - lib/invoices/__tests__/voucher-matching.pg: seed the posted voucher before closing the fiscal period so enforce_period_lock doesn't block the INSERT during setup. voucher-matching error codes and rollback: - Add LINK_VOUCHER_DB_ERROR (HTTP 500) and return it on real invoice UPDATE / payment INSERT failures. Previously these returned LINK_VOUCHER_VOUCHER_NOT_FOUND (404) which the pending-op dispatcher auto-rejects on transient DB errors. - Log rollback failures explicitly so an invoice left in a half-linked state (advanced status, no payment row) surfaces for manual reconciliation instead of disappearing silently. resyncNextPeriodOpeningBalance ordering: - Create the new IB first, relink the period FK, then storno the old IB. Previously the storno ran first; if createJournalEntry failed the next period was left with a reversed IB and nothing to replace it, and executeSIEImport swallows the error as a non-fatal warning. replace_period_opening_balance_link: - Tighten role check to owner/admin (was owner/admin/member). Matches delete_last_voucher and undo_sie_import. Data minimisation: - /api/invoices/[id]/voucher-candidates and the matching MCP tools now project only the invoice and customer fields the matcher reads, instead of returning the full customer row. Schema bounds: - SalaryEmployeeOverrideSchema caps each numeric override at 10 MSEK to catch typos before they reach the ledger or AGI. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(tests): supply user_id when seeding voucher_sequences voucher_sequences.user_id is NOT NULL (per the multi-tenant refactor in 20260330130000). The previous test seed only set company_id / fiscal_period_id / voucher_series, which made the seed fail with a constraint violation on the latest pg-real run. Pass the same userId used elsewhere in the seed helper. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(tests): scope delete-last-voucher RPC assertions inside the tx withUserContext always ROLLBACKs, so any DELETE the RPC performs is discarded when the callback returns. The previous test then queried journal_entries via a fresh getPool() connection that only saw the pre-RPC committed seed state — hence "expected '1' to be '0'". Move every post-RPC assertion (entry count, period FK clear, opening_balances_set flip, audit log entry, sie_imports clear) inside the same withUserContext callback so they observe the uncommitted state before ROLLBACK fires. Also fix the sie_imports INSERT: the column is `filename`, not `file_name`, and `sie_type` is NOT NULL. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(tests): assert against the IB-marker audit row directly DELETE on journal_entries fires two audit_log writes: the generic write_audit_log() trigger row ("Deleted journal_entries record") and the delete_last_voucher RPC's explicit "(was period IB)" entry. Both land at the same statement_timestamp(), so ORDER BY created_at DESC LIMIT 1 returned the trigger row non-deterministically in CI. Switch to a presence check with a LIKE filter on the IB marker so the test verifies what it actually cares about — that the RPC's IB-aware audit row exists. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(db): set company_id on delete_last_voucher audit_log rows 20260528120000_delete_last_voucher_clears_ib_link.sql inserts directly into audit_log without setting company_id. audit_log's SELECT policy filters company_id IN user_company_ids(), so those rows landed with company_id=NULL and were invisible to every reader — only the generic write_audit_log() trigger row remained visible. That broke BFL audit- trail intent: the "(was period IB)" provenance row was never readable. Republish delete_last_voucher with p_company_id populated on both audit_log INSERTs (draft path and posted path). Behavior is otherwise unchanged; the pg-real test for the IB-clear flow now sees the RPC-written marker row as expected. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com> Co-authored-by: Emil <emilmattsson14@gmail.com> |
||
|
|
20989379bb |
feat(sandbox,branding): prod-parity demo with AI gating + accounted rebrand (#585)
* feat(sandbox,branding): prod-parity demo with AI gating + accounted rebrand Sandbox now ships with seeded suppliers, supplier invoices, an asset, a verified agent_profile, and pending operations so the demo company exercises every prod surface. Server-side `guardSandbox()` short- circuits any AI or paid-external API call (Bedrock chat/composer, Resend invoice send, Riksbanken FX, VIES, etc.) and the AgentSheet swaps in a SandboxAgentPreview that explains what's gated and offers a register CTA. DashboardContent no longer mounts the NewUserChecklist when the agent is already built, fixing the path that let sandbox users still trigger /onboarding/agent. Visible branding flips from Gnubok to Accounted: new BrandWordmark component (Hedvig Letters Serif 700), new app/icon.png + PWA icons generated from the accounted icon, default appName updated. URLs, header names, API key prefixes, hostnames, and event/cookie/ localStorage keys keep `gnubok` — the rebrand is visual only. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(sandbox): hardcode supplier-invoice arrival numbers in seed get_next_arrival_number is MAX(arrival_number) + 1 against the same table we're about to insert into. Calling it twice before either row lands made both calls return 1, which then violated the (company_id, arrival_number) unique index — POST /api/sandbox/seed 500'd on first sandbox start. The seeded company is brand new in this branch so 1 and 2 are guaranteed unused; hardcoding side-steps the race entirely. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(sandbox): set paid_amount=0 on unpaid supplier invoice PostgREST normalizes the column set across rows in a bulk insert, so the second supplier invoice (Espresso House, status=registered) was being sent with paid_amount=null because the first row (Telia, paid) set it. supplier_invoices.paid_amount is NOT NULL DEFAULT 0; the default only kicks in when the column is *absent* from the payload, not when it's explicitly null. Set it inline to side-step the normalization. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(sandbox): set actor_type=agent_chat on seeded pending_operations pending_operations only allows user-scoped INSERTs via the `pending_operations_chat_insert` policy, which requires actor_type='agent_chat' alongside auth.uid()=user_id + company membership. The seed was inserting with the default actor_type='user', tripping the RLS check. Also lift risk_level from preview_data (where it was unused) onto the row itself, matching the column added in 20260430120000_pending_operations_actor_and_risk. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(pr-review): address PR #585 review feedback Fixes called out by the core-only CI check, Greptile, and the compliance + Swedish-accounting bots: - AGI Programnamn pinned back to 'gnubok' (CI blocker). The XML Skatteverket receives must keep the stable software identifier regardless of the visual rebrand — same rule as the v1 health endpoint's `service: 'gnubok'` literal. - handleCreateAccount in SandboxAgentPreview + ChatEmptyState now wraps signOut() in try/catch so a transient Supabase failure doesn't strand the user on a dead button (greptile P2 × 2). - /api/currency/rate hard-fails on missing companyId instead of conditionally skipping the sandbox guard (greptile P2 / compliance V8.2.1). - topUpSandboxAdditions now delegates to ensureSandboxAgentProfile; the assistant persona lives in exactly one place across the seed, layout backfills, and top-up path (greptile P2 outside-diff / compliance SOC2 CC6.1). - ensureSandboxAgentProfile drops the userId param and sets verified_by_user_id to NULL — synthetic seed data should not attribute verification to a real user (compliance V8.2.1 / GDPR Art. 25(2)). Errors now logged via the structured logger instead of being silently swallowed (V16). - Sandbox seed swaps real-world company names (Telia, Espresso House) for clearly-synthetic Demo-prefixed brands using the 5559... documentation org-number range (compliance A.8.33). Asset cost bumped 24 000 → 35 000 SEK so the demo clears the förbrukningsinventarier threshold and illustrates capitalization unambiguously (swedish-asset-accounting). - Representation pending-operation preview corrected: VAT label fixed from 6% → 12%, and input VAT split between the avdragsgill (2641) and ej-avdragsgill (5811) portions to match swedish-vat / ML 8 kap rules (swedish-vat). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(pr-review): seed preview consistency + AGI Programnamn constant Two last review-bot items before merge: - Sandbox seed: the representation pending-operation preview was splitting the 240 SEK café meal 60/180 between 5810 and 5811, which is wrong for a single attendee under the 300 SEK / person avdragsgill cap (ML 8 kap) — the entire amount is fully avdragsgill in that case. Collapse the preview to a single 5810 + 2641 + 2440 entry so it matches the supplier_invoice_items row 1:1 and stops teaching demo users an incorrect bookkeeping pattern. - Hoist the AGI Programnamn 'gnubok' literal into a named constant with a comment pointing to potential future Skatteverket vendor registration (per the swedish-compliance bot's nit). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(sandbox): avoid BFL duplicate-verification on pending op + fix VAT cap comment Swedish-compliance bot caught two final nits: - The pending operation for the Demokafé representation was using the same supplier_invoice_number as the already-seeded supplier_invoices row (88245). If the sandbox user approved the staged operation, the insert would have created (or attempted) a duplicate verification — BFL 5 kap. requires each affärshändelse be recorded exactly once. Swap the staged operation's invoice number to a distinct value (INKOMMANDE-2026-001) so approval cleanly creates a new row. - The preview comment described the 300 SEK threshold as an "avdragsgill cap". The actual rule (ML 8 kap. 9 §) caps the deductible VAT at 25 % × 300 SEK × antal_personer = 75 SEK per person — the 300 SEK is the tax base, not the total. Math here is correct either way, but the comment now states the correct formula so future seed edits don't propagate the wrong understanding. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com> |
||
|
|
f53725b20a |
Agent v1 bundle: TIC v2 onboarding, in-app assistant gating, sidebar nav, MCP fixes (#584)
* fix(sie-import): accept tab as field separator (Bollbok exports) The SIE 4 spec allows either space or tab between fields, but splitSIELine() only treated space (0x20) as a separator. Bollbok exports tab-separated lines for every record except #RAR, which silently swallowed all #IB / #UB / #KONTO / #KTYP / #VER / #TRANS records — imports appeared empty even though the file was well-formed. Also adds a parser-side diagnostic that emits a warning when raw #IB or #VER lines are present in the input but parsing produced none. The previous silent failure is how this bug stayed hidden; the warning gives the import preview something visible to surface next time. Verified against two real reproducer files (Sean / Erik Hellqvist): erik h 2025.SE (UTF-8): 166 accounts, 66 IB, 4 UB, 11 RES, 95 vouchers, 198 TRANS. erik h 2026.SE (CP437): 166 accounts, 66 IB, 4 UB, 0 vouchers. Both now parse with zero warnings/errors. Tests: + 8 Bollbok-shape tab-separated fixtures (2025 + 2026 quoting variants). + 4 silent-failure diagnostic-warning tests. All 74 sie-parser tests pass; 155/155 in lib/import; 64/64 downstream callers. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(sie-import): address PR #513 review — strip #KTYP quotes, suppress redundant aggregate warning Two non-blocking P2 findings from Greptile review on PR #513: 1. #KTYP handler stored fields[2] directly, so Bollbok 2026 exports (#KTYP\t1510\t"T") stored '"T"' with literal quotes instead of 'T'. Latent defect — accountType is unused downstream today, but my tab- separator fix made the quoted-value path reachable. Now routes through parseStringField so both Bollbok 2025 (unquoted T) and 2026 (quoted "T") land as 'T'. 2. The aggregate "kontrollera fältavskiljare och teckenkodning" warning fired alongside per-record 'error'-severity issues for malformed #IB / #VER records, producing a misleading hint when the parser had already pinpointed the structural problem. Now suppressed when an error-severity issue with the same tag already exists. Test coverage: + accountType asserted to be 'T' (not '"T"') in both 2025 + 2026 shapes. + VER aggregate-warning test now uses #VER lines without { } blocks (silent loss, no per-record error) — the canonical case the diagnostic is designed for. + New suppression test: bare #VER produces per-record errors AND the aggregate warning is absent. 75/75 sie-parser tests pass; 156/156 in lib/import. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * wip: agent chat + composer + memory + document extraction In-progress work on this branch beyond the SIE-import fixes: - Specialized accountant agent (composer + intents + chat loop) - Persistent agent_conversations/messages, agent_profiles, agent_memory - /chat surface + /onboarding/agent + /settings/agent-memory - document-extraction extension with status hooks - MCP server staging refactor + new skills (atoms, bank reconciliation, customer onboarding, kreditfaktura) - pending_operations rejection feedback (category + reason) + realtime - TIC company profile cached snapshot on companies - 17 migrations (all additive — see prior conversation analysis) Parked while branch waits for review/merge. Migrations are already applied to prod. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * refactor(tic): migrate company-data client from api-core v1 to Lens v2 Swaps the seven TIC company-data endpoints we call from the api-core paths (`/datasets/companies/{companyId}/...`, `/search/companies`) to the Lens equivalents (`/companies/{id}/...`, `/search-public/companies`). Hard cutover; proxy pattern preserved. Schema shifts handled inside the extension so consumers (TicWorkspace, Step2CompanyDetails) don't need changes: - `/companies/{id}/bank-accounts` now returns Bankgirot only — map to the existing `{ type, accountNumber, bic }` shape, drop terminated. - `/companies/{id}/industries` returns a discriminated array — filter to `companyIndustryCodeType === 'sni2007'` to preserve v1 behavior. - `/companies/{id}/phone-numbers` renamed the field to `phoneNumberFormatted` (fall back to `e164PhoneNumber`). - `/companies/{id}/documents` replaces `/financial-report-summaries`; filter `type === 'annualReport'` and read nested `financialReportMetadata` to rebuild the legacy summary shape. - `isCeased` is now a top-level boolean; `activityStatus` is an enum. Translate enum -> 'ceased' for the workspace's existing check. BankID identity flow (id.tic.io) is untouched — separate TIC product. Note: deploy gated on the TIC proxy being flipped to lens-api.tic.io with an `x-api-key` Lens key. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * feat(tic): expose v2 onboarding & workspace data Adds six new Lens (v2) fetchers on top of the migration that already landed in this branch, surfacing the data through /lookup and /profile. New fetchers in lib/tic-client.ts: - getFiscalYears /companies/{id}/fiscal-years - getAccountingPeriods /companies/{id}/accounting-periods - getPayrolls /companies/{id}/payrolls - getSignatory /companies/{id}/signatory - getRepresentatives /companies/{id}/representatives - getCompanyStatus /companies/{id}/status /lookup gains a fiscalYear field (current fiscal-year configuration) so onboarding Step 2 can skip manual MM-DD entry. CompanyLookupResult extended with optional fiscalYear; consumers without it keep working. /profile gains five new sections on TICCompanyProfile: - fiscalYear + fiscalYearHistory current + deduped period list - signatory firmateckning descriptions - board + representatives board-composition summary + active officers (positionEnd in future) - payrolls payroll2 array newest-first, with deviation vs annual-report - statuses current+historical status entries with red/yellow/green/neutral color TicWorkspace renders the new data as four cards (Status, Fiscal year + Signatory, Board + Representatives, Payroll history) plus a Badge mapping for the traffic-light status color. Tests: 52 -> 60 passing. Added unit tests for the new fetchers' v2 paths, fiscal-year auto-fill in /lookup, and full v2 profile coverage. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * feat(onboarding,agent): lean on TIC v2 to skip Steps 1 & 3 and sharpen Opus Three small wins that unlock more of the v2 cutover. No new endpoints — the data was already in the snapshot, just not flowing where it should. Step 1 (entity_type) — deep-link path only: - /lookup now returns `legalEntityType` and `registrationDate` (added to CompanyLookupResult). - /onboarding/page.tsx does a server-side /lookup prefetch when ?org_number= is present (BankID picker path), maps "AB"/"EF" to the EntityType enum, and seeds Step 1's radio. Falls through silently for unsupported codes (HB, KB, …) and on TIC errors. - WelcomeOnboarding hydrates ticLookup state from the server prefetch so Step 2's debounced client fetch and Step 3's first-year inference both have data on first render — no flash. Step 3 (is_first_fiscal_year) — every path: - deriveFirstYearDefaults() parses ticLookup.registrationDate and returns { isFirstFiscalYear, firstYearStart } when registered <12 months ago. Step 3's initialData picks it up; the user only confirms the end date. - Settings value wins when present so existing users with a saved choice don't get overridden. Composer prompt: - redactTic allowlist was the bottleneck — it stripped beneficialOwners, signatory, board, representatives, payrolls, statuses, fiscalYear before Opus ever saw the JSON. Existing filterRedundantQuestions ownership logic was effectively dead because the data path was severed. Expanded allowlist to include those v2 sections; kept bankAccounts/ email/phone/fiscalYearHistory/financialReports out (token cost > signal). - SYSTEM_PROMPT now documents each v2 section and the rules Opus should apply: payroll signal switches from "registration.payroll" to "actual payrolls[] filings" (kills the false-positive swedish-payroll selection for newly registered employers); beneficialOwners[] becomes the authoritative ownership source (single owner → FMB modifier; multiple → multi-owner); statuses[] isCeased/red triggers an uncertainty_note. Tests: 4112 unchanged. Build: green. No schema or migration changes. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(agent): onboarding polish + composer signal fixes from first-run feedback UX: - AgentOnboarding: drop the 10s "Hoppa över — fortsätt med standardval" escape hatch. The fallback path runs automatically on timeout; the manual skip just teased users into a degraded build. - ReviewCard step 2 title: "Stämma av detaljerna" → "Stäm av detaljerna" (imperative form matches the rest of the steps). - Drop em-dashes from user-visible Swedish strings in AgentOnboarding + ReviewCard (fallback labels, subtitles, placeholder, error message, final CTA). Em-dashes survive in code comments only. - "Fråga min revisor" → "Fråga min assistent" everywhere it surfaced: AgentTrigger, AgentSparkleButton, ReviewCard preview, ReviewCard fallback comment, general.help intent buttonLabel + prompt text. - AgentTrigger / AgentSparkleButton / EmptyState.AgentHelpLink / TransactionInboxCard ask-button all gated on identity.isVerified. Pre-onboarding users no longer see the floating FAB or per-page Sparkle buttons. AgentSheetProvider.identity gained an isVerified field; (dashboard)/layout.tsx selects agent_profiles.verified_at and passes it through. TIC verksamhetsbeskrivning: - tic/index.ts /profile: /companies/{id}/purposes returns every historical verksamhetsföremål filing. Picking [0] was returning the oldest "äga och förvalta" holding-company boilerplate for companies whose later filings narrowed the purpose ("tillhandahålla företagskrediter och finansiella teknologilösningar"). Sort the array by lastUpdatedAtUtc desc and take the most recent non-empty purpose. Composer banking signal: - loadBankingSummary now reads journal_entry_id alongside description/amount/date and returns per-counterparty `direction` ('in' | 'out' | 'mixed') and `has_unbooked` (any row not yet booked). Aggregate `unbooked_count` accompanies the rollup. - buildUserPrompt emits each counterparty as `Name: 12 345 kr (ut, OBOKFÖRD)` so Opus can tell income from cost on sight and tell which counterparties are still open questions. - SYSTEM_PROMPT now explicitly forbids verification questions about counterparties whose direction is unambiguous AND status is 'bokförd'. Should kill the regressions from the first agent build: * "Konsult, J 98 565 kr — intäkt eller kostnad?" when the amount is clearly negative. * "ALMI AB 493 000 kr — lån eller bidrag?" when the transaction is already categorized. Tests: 4112 unchanged. Build: green. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(agent,ui): representation needs deltagare+syfte, drop duplicate doc icon Representation booking: - transaction-categorization prompt now requires the agent to capture participants (name + company) AND purpose before staging a representation categorization. SKV's representationsregler + ML 8 kap require the verifikation to document who attended and what the meeting was about; without that the avdrag is denied and the post should be booked as non-deductible / personalkostnad. - The agent confirms back in plain text (audit trail in the chat), writes the deltagare + syfte to gnubok_remember_fact (long-term), THEN stages. Saknas deltagare/syfte: explicitly tell the user the avdrag won't go through and offer the non-deductible alternative. - Known gap (followup, not this commit): the staged op's journal entry description doesn't yet carry the deltagare text. Until we add a `notes` field to gnubok_categorize_transaction, the audit trail lives in chat + agent_memory only. TransactionInboxCard duplicate attachment indicator: - Drop the FileCheck2 "open document" button from the trailing slot. TransactionAttachmentIndicator (Paperclip) next to the description already opens the underlag on click. Two icons doing the same thing was noise. Cleaned up the unused state (isOpeningDoc, hasAttachment, handleOpenAttachment) and dropped now-unused imports (FileCheck2, useToast). Tests: 4112. Build: green. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * feat(agent,nav): notes on verifikation + redesigned sidebar Audit-trail notes for representation: - gnubok_categorize_transaction gains an optional `notes` string. Threaded through stagePendingOperation → commitCategorizeTransaction → createTransactionJournalEntry, which now appends notes to the entry's description (capped at 500 chars). The verifikation an external auditor reads now carries deltagare + syfte directly — not just chat history / agent_memory. - transaction-categorization prompt updated: representation flow now REQUIRES the agent to pass deltagare+syfte via the notes parameter. Without it the booking is non-deductible / personalkostnad per SKV. DashboardNav redesign: - Top section: flat, no header — Hem (/chat), Underlag (was Dokumentinkorg), Transaktioner, Granskning. Always visible; the inline badge on /pending shows the count when there are pending ops. - Mid section: four collapsible dropdowns (Försäljning, Inköp, Redovisning, Personal). Each auto-expands when the active route lives inside it. KPI moved from main to Redovisning. Extension nav items (TIC workspace, etc.) fold into Redovisning. - Bottom-left: new account popover (DropdownMenu, opens upward) holding CompanySwitcher, Inställningar, Hjälp, Support, Logga ut. Replaces the old top company-switcher card + the bottom Support/Logout block. - Mobile drawer mirrors the new structure: top items as flat list, same four dropdown groups, separate "Tillägg" section when extensions exist, "Mitt konto" section at the bottom. - i18n: invoice_inbox label renamed "Dokumentinkorg" → "Underlag" ("Documents" in en). New keys: mitt_konto, group_extensions. Tests: 4112. Build: green. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(nav): unhide Leverantörer under Inköp The /suppliers entry existed in navItems but was marked hidden — leftover from when the supplier list lived elsewhere in the IA. Removing the hidden flag puts Leverantörer in the Inköp dropdown alongside Leverantörsfakturor. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(nav): CompanySwitcher back to top-left, user account moves bottom-left The previous pass collapsed both concepts into the bottom popover. They mean different things: the company is the org context everything below operates against (top-of-sidebar, scannable); the user is the account-holder (bottom-of-sidebar, where settings/logout live). - (dashboard)/layout.tsx: fetch profiles.full_name alongside the existing identity queries; pass userName + userEmail into DashboardNav. - DashboardNav: restore CompanySwitcher at the top of the sidebar (pre-redesign placement). Bottom-left popover trigger now shows the signed-in user's name + single-letter initial (accountInitial helper falls back to email's first char, then "?"). Popover header carries full name + email; items unchanged (Inställningar, Hjälp, Support, Logga ut). CompanySwitcher removed from inside the popover — nested dropdowns were awkward and the top placement is where it belongs. Tests: 4112. Build: green. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(pending): trim the agent context strip The row-level AgentContextStrip on /pending was rendering the model name (eu.anthropic.claude-sonnet-4-6) and the full atoms array (horizontal/swedish-vat, vertical/konsult-it, …) inline, which made each row 60–80 chars of mostly-the-same metadata. Reviewers never scan that text; they scan amounts and decide approve/reject. Now the strip shows only the conversation deep-link (Konversation #<short id>) — the one piece that's actually useful for diving into context. Model + atoms remain available in agent_metadata for debugging surfaces; they're just not in the list view. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(agent): shared ground rules + paragraph breaks after tool calls Two regressions surfaced in real usage. Both are systemic. Shared agent ground rules: - /chat surface (general.help) was happily inventing four-digit BAS account numbers ("Debet 6212 - Molntjänster…", "Kredit 2614 - Ingående moms…") and proposing booking decisions on invoices it had never seen, with no follow-up questions about currency/scope/etc. - transaction-categorization had those rules baked into its prompt; general-help / bokslut-step / invoice-draft / supplier-invoice-review / verifikation-draft / vat-review never inherited them. - Extracted lib/agent/intents/shared-rules.ts with five cross-cutting rules: underlag first (check inbox + ask user to upload to Dokumentinkorgen when missing), ask follow-ups when ambiguous, never write four-digit BAS account numbers in chat (category names only), cite atoms / load skills (don't guess), check counterparty history before proposing. - Injected renderAgentGroundRules() into all six intents above. transaction-categorization left alone — it has more detailed inline rules tied to its specific underlag-flow. Paragraph break after tool calls: - text_delta from the model often resumes after a tool call without a leading newline ("kategoriseras." → gnubok_query_journal runs → "Inget historik hittades…" appended directly). Markdown rendered the concatenation as one paragraph. - AgentChat text_delta handler now inserts \n\n when (a) the buffer ends with text content, (b) the incoming delta starts with text content, (c) at least one tool call has run, and (d) the buffer doesn't already end with a blank line. Tests: 4112. Build: green. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(nav): default-open dropdown groups; closing is per-user Dropdowns started collapsed which meant first-time users had to open each group to discover what's inside. Inverted the state: default open, user can collapse, active route still forces a group open. - manualExpanded → manualCollapsed (semantics flip) - toggleGroup unchanged externally; flips the bit - isGroupExpanded returns !manualCollapsed[g] || hasActiveChild Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * feat(agent): rate-safe v1→v2 TIC upgrade, counterparty defaults, profile settings Three pre-ship quality wins. Rate-limit-safe TIC v2 upgrade: - The /profile endpoint fans out to ~13 Lens calls; the account has a ~3000/mo ceiling. Force-refreshing every pre-v2 (v1) snapshot across the customer base would blow the budget. - ensureTicSnapshot gains an `upgradeV1` flag. A cached snapshot still inside the 7-day window is re-fetched only when (a) the caller passes upgradeV1 AND (b) the snapshot is v1-shaped (missing the v2-only `statuses` key). Gated to the two agent-onboarding call sites — a deliberate, once-per-company action and the only consumer of the v2 sections. Workspace + signup keep the natural 7-day staleness, so the v1→v2 migration is lazy and bounded to companies actually building an agent. Known-counterparty defaults (shared-rules): - Agent now proposes a sensible default for well-known counterparties instead of asking the same question monthly: Almi → lån, Tillväxtverket/ Vinnova/EU-stöd → bidrag, Skatteverket → skatt/avgift or återbäring, Bolagsverket → avgift, Försäkringskassan → ersättning, EF private withdrawal → eget uttag. Stated as an assumption the user can correct, not a hard rule — underlag/history still wins. Företagsprofil settings page: - New /settings/agent-profile (Företagsprofil / "Company profile"): view + edit the agent's company profile after onboarding — assistant name + avatar, the profile summary the agent reasons from, and a read-only chip view of loaded specialities (atoms). Backed by the existing GET/PATCH /api/agent/profile. - New GET /api/agent/atom-titles?ids= resolves atom slugs → human titles for the chips (registry is globally-readable reference data). - Added to SettingsSidebar; i18n keys agent_profile (sv "Företagsprofil" / en "Company profile"). Note: /chat already redirects unverified users to / (chat layout guard), and / renders WelcomeGate → /onboarding/agent. No redirect work needed. AgentSetupBanner.tsx is orphaned dead code (WelcomeGate superseded it). Tests: 4112. Build: green. Both new routes compile. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * feat(nav,agent): Hem=Översikt + separate Assistent button; memory dedup Nav restructure: - "Hem" now points to / (Översikt dashboard) again, not /chat. The agent chat gets its own top-level nav entry "Assistent" (Sparkles icon) → /chat. Mobile bottom nav mirrors this (Hem / Assistent / Transaktioner). - / restored to render DashboardContent (the Översikt) for built-agent users instead of redirecting to /chat. Users who haven't built their assistant yet still get WelcomeGate (the build-agent checklist); once verified, / shows the dashboard. Chat is reachable anytime via its nav entry. Restored main's dashboard data-fetch; added an agent_profiles verified_at probe to drive the WelcomeGate branch. - i18n: nav.assistant ("Assistent" / "Assistant"). agent_memory dedup (gnubok_remember_fact): - The agent re-remembers the same fact constantly (e.g. "Vercel = omvänd skattskyldighet" on every Vercel categorization), which would bloat agent_memory with paraphrases over months. - Before insert, compare the incoming fact against the 300 most-recent active memories by word-set Jaccard similarity (lowercased, punctuation- stripped, stopwords dropped). A near-duplicate (≥0.82) is treated as already-known: bump its relevance toward the new score + refresh updated_at instead of writing a new row. Embedding-free, zero added latency beyond one bounded SELECT. Tests: 4112. Build: green. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(agent,nav): företagsprofil=Bolagsuppgifter, avatar nav icon, dedupe greeting Företagsprofil settings page (the right content this time): - Replaced the agent atoms/summary panel with CompanyProfileView — a read-only "Bolagsuppgifter" view of the cached TIC company snapshot (name, org-nr, form, address, F-skatt/Moms/Arbetsgivare, SNI, bank, verksamhet, employees, latest financials, status traffic-lights, fiscal year, firmateckning, företrädare). Server component reads the companies.tic_snapshot column directly — no extension import, stays inside the core-build boundary. - Route renamed /settings/agent-profile → /settings/company-profile. Removed the old AgentProfilePanel + the now-unused /api/agent/atom-titles endpoint. "Assistent" nav icon = the agent's chosen avatar: - DashboardNav reads agent identity from AgentSheetProvider and renders the onboarding-chosen avatar for the /chat ("Assistent") entry across desktop sidebar, mobile drawer, and mobile bottom nav. Falls back to the Sparkles glyph pre-onboarding (no avatar yet). Nav cleanup: - Dropped the beta badge from Underlag. - Filtered the TIC workspace (/e/general/tic, "Företagsprofil") out of the nav — the same Bolagsuppgifter now lives under Inställningar → Företagsprofil, so it shouldn't appear in two places. Doubled intake greeting fix: - /chat/intake fires an invoke with no conversation_id, then swaps the URL to /chat/[id] the instant the `conversation` event lands — which can beat the greeting being persisted. /chat/[id] then hydrated with 0 messages and, because the auto-fire guard keyed on (id && messages>0), fired a SECOND invoke on the same conversation → two greetings. Guard now keys on conversation-id presence alone: a set id means resume, never bootstrap. Closes the race. Tests: 4112. Build: green. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(agent): paragraph-break-after-tool split words mid-stream The earlier "insert \n\n when text resumes after a tool call" heuristic re-evaluated on EVERY text_delta (any delta not starting/ending with whitespace, once a tool had run). Streaming deltas arrive in sub-word chunks, so it injected breaks between fragments of the same word: "minnes\n\nno\n\nterna", "kund\n\nrep\n\nresentation". Replace the per-delta heuristic with a consume-once ref: - tool_use sets breakBeforeNextTextRef = true - the next text_delta consumes it: prepends \n\n exactly once (only when the buffer has content, doesn't already end in whitespace, and the delta doesn't start with whitespace), then clears the flag So the break fires once per tool→text resume, never mid-word. Build: green. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(agent): much shorter replies, representation headcount + VAT cap, dot separator Brevity (system-prompt Svarsformat — affects every reply): - Hard "korthet är regel nummer ett": aim for 2-4 sentences, lead with the answer/action, no warm-up ("Här är vad som gäller…"), don't derive VAT in prose, don't restate what the approval card shows, one question at a time. The agent was writing textbook-length essays. Representation rule now in shared-rules (so verifikation-draft, vat-review, etc. all get it — previously only transaction-categorization had it, which is why the verifikation flow guessed 25% VAT and skipped the cap): - Require ANTAL deltagare (headcount), not just one name — the moms deduction is per person (underlag cap 300 kr/person ex moms). - Use the receipt's ACTUAL VAT rate (usually 12% on food), never assume 25%. - Meal representation isn't income-tax deductible (post-2017); whole cost booked as non-deductible representation. Verifikation description separator: - createTransactionJournalEntry appended notes with an em-dash ("Utlägg Eatnam — Deltagare:…"), violating house style. Switched to a middle dot " · ". journal_entries has no separate notes column — the description IS the BFL verifikationstext / audit field, so deltagare + syfte correctly live there. Tests: 4112. Build: green. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(settings): tidy Bolagsuppgifter — no status colours, clean firmateckning From first-look feedback on the Företagsprofil page: - Status: dropped the coloured traffic-light badges (red/yellow/green). Per the design system semantic colour is data-only, never chrome, so status now renders as plain label + date. Also filtered to dated entries only — Bolagsverket emits flags like "Har aldrig varit verksam" with no date that read as noise next to the real status. Ceased status gets muted destructive text (the one chrome colour the system keeps). - Firmateckning: the source text carries ">" list markers and crams several rules onto one line, and repeats "Firman tecknas av styrelsen" across rows. cleanSignatory() strips the markers, normalises whitespace, splits run-on "Firman tecknas …" clauses onto separate lines, and the render dedupes — so each rule reads as its own sentence. Build: green. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(mcp): inbox items expose all terminal links + processed flag The Eatnam receipt was booked against its bank transaction (so the inbox row had matched_transaction_id + created_journal_entry_id set), yet the agent reported it as loose/unmatched and a duplicate risk. Root cause: gnubok_list_inbox_items only selected and returned matched_supplier_id + created_supplier_invoice_id — the supplier-invoice path. The transaction-match and direct-journal-entry paths were invisible, so any receipt cleared via /transactions looked unprocessed. - list_inbox_items now selects + returns matched_transaction_id and created_journal_entry_id alongside the supplier fields, plus a derived `processed` boolean (true when ANY of the three terminal links is set). - New unprocessed_only=true input filters to items with no terminal link — the "what still needs handling" view that prevents the agent from flagging already-booked docs as duplicates. (Fetches a wider window then filters client-side so limit applies post-filter.) - Description updated to document the processed semantics, within the 280-char tool-description budget. The DB linkage itself already worked: /transactions attach-document sets matched_transaction_id, and commitCategorizeTransaction stamps created_journal_entry_id. This was purely a read/surface gap. Tests: 4112 (+ MCP description guard). Build: green. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(mcp): repair stage-but-never-commit tools + consolidate tool surface - post_annual_depreciation AND reverse_entry were never in the pending_operations operation_type CHECK, so both staged then died with check_violation at INSERT. Add the CHECK migration, a commitPostAnnualDepreciation executor (reusing commitAnnualPostings), risk tier, and the PendingOperationType union member. - Salary tools de-risked: calculate_salary_run calls runSalaryCalculation() directly (no self-fetch/forged cookie); create_salary_run uses a transactional create-run helper with compensating delete; generate_agi actually generates + persists the declaration. - import_sie parses + validates at stage time with a content-rich preview (company, fiscal year, voucher/account counts, balance) instead of a blind byte count. - batch-match-invoices passed user.id where companyId was expected (silently matched zero). - VAT report+widget merged behind render_ui; gnubok_search_tools ranks by relevance; gnubok_feedback readOnlyHint corrected; tools/list instruction text fixed; income decision-tree + GL/query_journal cross-refs added. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * feat(agent): load skill atom bodies from the DB so they survive the build Skill bodies were read from disk at runtime (.claude/skills/**/SKILL.md); on Vercel the dynamic readFile path isn't traced into the lambda and on Docker .claude/ is excluded, so atoms loaded EMPTY in production — a despecialized agent. Inline the bodies into agent_atom_registry instead: - Migration adds body + mcp_exposed columns; a build-time generator (scripts/generate-skill-bodies.ts) emits a deterministic dollar-quoted seed migration with a content-hash manifest + --check CI guard. - Read sites (mcp-server atoms.ts, chat system-prompt.ts, composer prewarm) read body from the DB, with a dev-only disk fallback. mcp_exposed curates which atoms the MCP exposes (swarm-* never become atoms). - The seed script + generator share scripts/lib/atom-discovery.ts; estimated_tokens now reflects SKILL.md only. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * feat(agent): safe the in-app assistant — gating, FAB de-confliction, rate limit, friendly errors - Hide all agent entry points until verified_at: the Assistent nav tab (sidebar + mobile) and the agent-memory settings tab now match the floating FAB's gate. - FAB de-confliction: /kpi -> kpi.explain and /bookkeeping/year-end -> bokslut.step so the floating button opens the SAME assistant as the page button (no two-agents-on-one-page). - Generous per-user rate limit (30/min, 1000/day) on /api/agent/invoke, /onboarding/stream, /composer via a new agent_rate_counters table + check_and_increment_agent_quota RPC; fails open. Bounds runaway Bedrock spend without touching normal users. - Friendly errors: Bedrock 429/timeout/5xx normalized to Swedish (friendlyModelError) in run-turn + the invoke route; the chat client surfaces the server's friendly message instead of a raw HTTP status. - /chat/new validates ?intent= against the registry so bad deep-links fall back to general.help instead of rendering a broken-looking error. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(agent): keep /chat read-only — redirect categorization + swap the "categorize" suggestion for a VAT-report question general.help (the /chat assistant) is read-only, but it still gave per-transaction bokföringsförslag in prose and asked "godkänner du dessa?" — an analysis the user can't act on (no write tool, no per-tx underlag). Strengthen the prompt to redirect categorization/bokföring to the per-transaction flow (open the transaction -> "Fråga om denna transaktion", where the agent sees the underlag and stages a real ApprovalCard); a short overview is still allowed. Add a guard test locking in no-write-tools + the redirect language. Swap the /chat empty-state "Hjälp mig kategorisera" chip (which lured users into exactly this dead-end) for a VAT-report question the read-only assistant can actually answer. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * refactor(pending): declutter the review queue rows + header Fold the conversation deep-link onto the actor label (drop the separate "Konversation #xxxx" strip and its icon), hide the quick-pick when there's only one operation type (it duplicated "Markera alla"), and drop the "(0)" from the disabled bulk-approve button. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * feat(vat): enhance VAT handling by integrating document validation and improving error messaging * feat(settings): add assistant knowledge surface + consolidate settings tabs Expose the agent's skill atoms (agent_atom_registry) in a read-only surface beside the existing memory view, and tighten the settings tab bar from 14 to 10 tabs. - New GET /api/agent/skills + AgentSkillsPanel: lists active, mcp_exposed atoms grouped by tier (Kärnkompetens / bransch / bolagssituation), flags which are active for the company from agent_profiles, and lazy-loads each SKILL.md body on expand. - New /settings/assistant tab with a Minne/Kompetens toggle (?view=skills); /settings/agent-memory and /settings/agent-skills redirect into it. - Merge Företagsprofil (TIC snapshot) into the Företag tab via CompanyProfileSection; /settings/company-profile redirects. - Merge Skatteverket-anslutningen into the Skatt tab — OAuth returnTo and the callback toast now target /settings/tax; /settings/skatteverket redirects. - Drop the Säkerhetsbackup tab (already under Importera/Exportera). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(inbox): keep booked underlag out of the unmatched queue + widen match window - categorize: after booking an inbox underlag onto a verifikat, backfill the inbox row's matched_transaction_id + created_journal_entry_id so it stops showing as unmatched (mirrors the /attach-document paperclip path). - TransactionMatchPicker: bias the candidate window forward (60d before → 180d after the invoice date) so late payments aren't dropped before scoring, and widen the ranking date tolerance to 120d so the true match floats to the top instead of collapsing to "Svag match". Fix "okatigoriserade" typo. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * wip: bundle in-progress branch work + agent onboarding chat optimizations Captures the uncommitted work-in-progress on this branch so it lives on the remote. Heterogeneous changeset — bundled as one commit since the work was already entangled across files. Headline change in this commit (from this session): - Remove the double interview in agent onboarding. Phase B's verification- question form stepper is gone — the Phase C chat (onboarding.intake) now owns the entire interview and reads the composer's verification_questions server-side as its question bank. - ReviewCard collapses from 3 steps to 2 (meet → review-and-confirm) with value-first ordering: profile + "vad jag kan hjälpa dig med" + facts + optional seed note. CTA reads "Möt {namn}" to signal the chat follows. - ChatIntakeStarter handoff subcopy updated to match reality (assistant greets first; user can leave anytime). - Stamp agent_profiles.intake_completed_at server-side in app/api/agent/invoke/route.ts on the first user-typed reply in any onboarding.intake conversation (idempotent IS NULL guard, best-effort). Closes the previously dead-write column and unlocks the opportunistic- follow-up hook the migration anticipated. Plus in-progress branch work being carried forward (not introduced here): agent runtime + intent prompts, composer + atom-discovery scripts, MCP server skills surface, onboarding flow components, dashboard/inbox tweaks, two new agent_atom_registry migrations, additional agent-chat tests. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * refactor(agent): drop inline "Fråga assistenten" affordances — rely on the FAB The bottom-right "Fråga {namn}" FAB (AgentTrigger) is already route-aware and picks the right intent per page, so duplicating it as inline page- header buttons and empty-state links is noise. Removed: - EmptyState `agentHelp` link ("Eller fråga {namn} hur du kommer igång") + the AgentHelpLink component + agent_default_name/agent_ask_link i18n keys + the agentHelp props on EmptyInvoices/EmptyCustomers/EmptyTransactions. - AgentSparkleButton on /bookkeeping (verifikation.draft) and /kpi (kpi.explain) page headers. The FAB stays — when verified, it appears on those routes and routes to the right intent automatically. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(agent): gate the last two ungated "Fråga assistenten" affordances Both surfaces previously called useAgentSheet directly without checking identity.isVerified, so they appeared pre-onboarding (everywhere else the FAB / sparkle buttons / /chat / Assistent nav are all gated on verified_at). - Settings page header: remove the "Fråga {namn}" pill entirely. The FAB covers /settings routes route-aware (settings.help) — no need for a duplicate inline trigger. - Invoice inbox transaction picker: hide the "Fråga assistenten" button when the agent isn't built. Done at the parent (InvoiceInboxWorkspace) by passing onAskAssistant only when identity.isVerified is true; the child renders the button only when the callback is present. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * feat(tic,onboarding,agent): single-call TIC lookup + director-aware narrative voice - TIC: collapse the company lookup from 6 endpoint calls to 1 (search-public already exposes sniCodes, bank accounts, emails, phones, and registration flags). Derive fiscal-year MM-DD from mostRecentFinancialSummary; newly-registered companies fall through to the client's first-year defaults. - Onboarding: BankID picker no longer auto-provisions companies. Every pick routes through the wizard with orgnr (and entity_type via the CompanyRoles match) prefilled; F-skatt/VAT/address get confirmed in steps 2-4 instead of being auto-fetched. createCompanyFromOnboarding reuses CompanyLookupResult and adds a defensive top-level catch so server-action errors surface to the UI instead of being redacted. - Agent composer: loadUserDirectorship() checks BankID CompanyRoles for a director-like position (ceo/boardMember/chairman/externalSignatory, active) before the narrative uses second-person ownership voice ("Du driver…"); unknown users get neutral third-person voice so we never put ownership words in the user's mouth. Tests cover loadUserDirectorship, narrative voice, tic-fetch path, onboarding page, and updated TIC client + lookup/profile suites. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(tic): extend agent-onboarding TIC budget to 10s + backfill stranded org_numbers The 5s TIC fetch timeout aborted client-side before the upstream Lens fan-out (~13 calls) could complete, but the in-flight upstream calls still counted against quota — actions.ts already documents ~530 wasted calls from this in May. Same bug still applied to the agent-onboarding stream path. Adds an optional `timeoutMs` to `ensureTicSnapshot` so deliberate wait-screen callers (agent onboarding stream) can run with 10s while background/dev callers stay on the conservative 5s default. Page-level server fetch (page.tsx) intentionally stays at 5s to avoid blocking TTFB without a visible progress affordance. Backfill migration mirrors `company_settings.org_number` to `companies.org_number` for the 105 cases where it's safe (after dedup + conflict filtering). 56 of those are on active companies — unblocks duplicate guards, SIE/SRU exports, and TIC fallback chain. Zero TIC API calls — pure data move. Idempotent. Also sweeps a pre-existing SSRF guard on the stream route's origin derivation that was sitting unstaged in the working tree — it lives in the same diff hunks as the TIC budget change and couldn't be split cleanly. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * wip: bundle in-progress branch work Sweep up uncommitted agent/MCP/RLS work-in-progress so the branch is fully backed up to origin. Not reviewed in detail — committed as-is to preserve working state alongside the TIC fixes in the previous commit. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(agent): tag the "Bygg din bokföringsassistent" CTA as Beta Adds a Beta badge next to the assistant-setup heading on the dashboard banner, dashboard inline card, and onboarding checklist row. Also drops the stale "Gratis i 30 dagar" subline from the dashboard card. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(build,migrations): PendingOperationType salary ops + resolve migration version collisions PR #584 went red on three things: 1. core-only build / Vercel: `lib/pending-operations/commit.ts:2666` switched on 'create_salary_run' and 'generate_agi' but `PendingOperationType` was missing both literals. Add them to the union. 2. Supabase preview: migration version 20260526120000 collided with main's newly-merged 20260526120000_fix_replace_sie_import_hard_delete.sql. Bump the branch's pair to 20260526120050 / 20260526120051 — still ahead of 20260526120100_restvardeavskrivning so ordering is preserved. 3. 20260527170000 was used twice on this branch (_agent_rls_with_check + _journal_entry_no_doc_required). Bump the second to 20260527170100 so the pair stays orderable and Supabase doesn't choke on the duplicate schema_migrations PK. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(ci): reword comment so core-only guard stops flagging it The "Check no core imports from extensions" step greps for the literal \`from '@/extensions/\` across lib/, app/api/, components/. A comment in lib/agent/composer/tic-fetch.ts quoted the exact pattern verbatim to explain *why* the file does a self-fetch instead of importing the TIC extension directly — which the grep matched even though no actual import exists. Rewrite the line to keep the same meaning without the literal pattern. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com> Co-authored-by: Emil <emilmattsson14@gmail.com> |
||
|
|
a9b43ebeb7 |
Bug/vat selection warning (#583)
* refactor: update VAT handling logic for non-registered sellers and improve related comments * chore: gate automated email flows behind 503 responses Disables user-facing access to invoice payment reminders and salary payslip email sending. Underlying lib code (reminder-processor, PDF templates, notification_settings) is preserved for easy re-enable. - Invoice reminders cron route returns 503; settings UI section removed. - Payslip send route returns 503; original implementation kept as _sendPayslipsImpl for future re-enable. - Push notifications were already extension-disabled, no change needed. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * chore: remove Recapt feedback widget Strips the third-party Recapt SDK and its floating feedback bubble from the app. The in-app contact form keeps working via the existing email channel (/api/support/contact). Drops the Recapt entries from the CSP and the subprocessor list in the privacy policy. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * feat: reject meaningless rättelser in correctEntry Guard against zero-economic-effect corrections in the storno engine: - Reject when proposed lines net to zero on every account (e.g. 1930 debit 100 / 1930 credit 100), which would erase the original posting without representing any affärshändelse (BFL 5 kap. 5 §). - Reject when proposed lines are an exact multiset match of the original entry — a rättelse must actually change something. New MeaninglessCorrectionError wired through bookkeepingErrorResponse (HTTP 400) and the Swedish error translator. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * feat: add date-range picker to resultat- and balansrapport Adds optional from/to date filtering to the four operational financial reports (resultatrapport, balansrapport, income-statement, balance-sheet) so users can view a month, quarter, or custom range inside a fiscal year without leaving the report. Defaults to YTD; "Hela året" preserves the prior full-period behaviour (URL-identical, cache-stable). - trial-balance engine accepts optional fromDate/toDate, rolling prior in-period activity into IB and clamping period activity to the window - 12 API routes accept and validate from_date/to_date query params - ReportDateRange chip picker persists preset per company, only renders on the four relevant tabs - FiscalYearSelector now emits the period object so the range picker has bounds without an extra fetch - PDF/XLSX filenames reflect the chosen range - Resultatrapport drops the prior-year column when narrowed (full-year vs partial-year would mislead) - 11 new tests (engine + parser); all existing report tests pass Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * feat: add support for marking journal entries as "no document required" - Introduced a new sidecar table `journal_entry_no_doc_required` to track entries that do not require separate documentation (e.g., bank fees, interest). - Implemented API routes for creating and deleting exemptions, including validation and authorization checks. - Added a toggle component in the UI to allow users to mark entries as exempt, with an optional reason. - Updated relevant tests to cover the new functionality, including RLS checks and cascading deletes. - Enhanced existing schemas and types to accommodate the new `vat_amount` field for supplier invoice items. * fix: address PR review findings on no-doc-required + VAT changes - pg-real cascade test wraps DELETE in gnubok.allow_delete='true' txn so the immutability trigger bypass fires (mirrors delete_last_voucher RPC). - Clamp supplier-invoice item vat_amount to <= line_total * vat_rate via Zod refinement (with 1-öre rounding tolerance) so the manual override can't inflate the 2641 debit beyond the statutory ceiling. - groupVatByRate falls back to line_total * rate when stored vat_amount is 0 with a positive rate, so legacy/import paths leaving the column at its NOT NULL DEFAULT 0 don't silently understate ruta 48. - ReportDateRange todayIso() and preset endpoints use local date components instead of toISOString() (UTC) — fixes the midnight-to-02:00 off-by-one that truncated a day from YTD / this-month / this-quarter for Swedish users. - NoDocRequiredToggle restores the previous reason on failed POST/DELETE so the rolled-back toggle state stays consistent with the rendered reason. - Document the company-scoped (not user-scoped) DELETE authorization policy on the no-document-required route. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com> |
||
|
|
8979f6eda3 |
Bug/year end failure (#575)
* feat: implement findNextPeriod function and integrate into year-end closing logic * feat: add integrity check for PDF documents and enhance user feedback for corrupt files * Refactor year-end service and period creation logic for improved UTC handling and error messaging - Update `validateYearEndReadiness` to assert on stable warning messages without interpolating period names. - Modify `createNextPeriod` to ensure date calculations are performed in UTC, preventing DST-related issues. - Enhance error handling in `validateYearEndReadiness` and `executeYearEndClosing` to avoid exposing database details. - Introduce structured error messages for year-end processes in `structured-errors.ts`. - Add tests for document integrity checks, ensuring proper authentication and error handling. - Implement GUC checks in document versioning to prevent unauthorized modifications and ensure company membership. - Update migration scripts to reflect changes in document immutability enforcement. * fix: add comment to clarify GUC behavior in document supersession logic |
||
|
|
a2a556d837 |
Bug/UI wrong display (#573)
* fix(dashboard): exclude credit notes from unpaid invoices widget Credit notes (status='sent', negative total) were summed into the "Att få betalt" widget, producing confusing negative totals like "2 st, -38 625 kr". Filter them out via credited_invoice_id IS NULL, matching the existing pattern in reminder-processor and the AR ledger. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(documents): harden PDF preview and upload validation - JournalEntryAttachments: switch inline PDF preview from <iframe> to <object type="application/pdf">. Mirrors the AttachmentPreviewSheet fix from #572 — Chrome's frame pipeline intermittently surfaced "Det här innehållet har blockerats" on iframes even with permissive CSP. <object> invokes the PDF plugin directly. crbug.com/271452. - /api/documents/:id/inline: resolve Content-Type via file extension when mime_type is null or application/octet-stream. Legacy uploads landed with empty File.type from some drag sources; combined with the new X-Content-Type-Options: nosniff header on this route, Chrome refused to render valid PDFs. Extension fallback covers every legacy row without a DB backfill. - /api/documents POST: surface DB-trigger period-lock errors as a 400 DOC_UPLOAD_PERIOD_LOCKED with a Swedish reason. Previously every catch was bucketed into DOC_UPLOAD_STORAGE_FAILED (500 / "Filen kunde inte sparas") which hid the real cause from users attaching to verifikationer in closed/locked fiscal periods. - document-service: add validateDocumentMagicBytes() that inspects the first bytes for valid PDF/PNG/JPEG/WebP headers (PDF tolerates a leading UTF-8 BOM). Wired into uploadDocument() and createNewVersion() so every upload path is protected — UI, MCP, and future email/webhook ingestion. Defends against agents that send a base64-encoded text placeholder instead of real binary bytes via the gnubok_upload_document MCP tool, which produced tiny (15-561 byte) "PDFs" that failed to render in Chrome and in external viewers. Tests use a minimal valid PDF buffer (%PDF-1.4 … %%EOF). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * feat(arsredovisning): emit ÅRL-required notes and FTE-weighted medelantal Five compliance gaps fixed in the K2 and K3 noter builders: - Anläggningstillgångar roll-forward per ÅRL 5:8 § — per-category IB anskaffningsvärde, tillkommande, avgående, UB and accumulated avskrivningar movement (was only emitting avskrivningstider). - Långfristiga skulder förfallande efter mer än fem år per ÅRL 5:13 §. - Ställda säkerheter and Eventualförpliktelser as separate notes per ÅRL 5:14-15 § (K2 previously combined them). - Koncernförhållanden per BFNAR 2016:10 kap. 19 / BFNAR 2012:1 kap. 8. Replaces medelantal anställda — the old query filtered employees by an is_active column that doesn't exist, so the note never emitted. Now uses an FTE-weighted day-based average per ÅRL 5:20 §. Six disclosure fields persist on arsredovisning_narratives as per-period overrides; the UI extends the existing förvaltningsberättelse editor with a "Lagstadgade upplysningar" subsection sharing the same Spara button — no new pages, no settings changes. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(invoices): respect vat_registered=false and hide personnummer for B2C - PDF address block no longer prints org_number for individual customers (GDPR data minimization; ML 17 kap 24§ requires name + address only). - Wire company_settings.vat_registered through the rule helpers, invoice creation API, preview-pdf API, and the new-invoice form so a non-VAT- registered seller cannot charge VAT (ML 1 kap. 1§). The PDF suppresses the empty "Moms 0%" row and shows a dedicated "Företaget är inte momsregistrerat" notice instead of the ML 3 kap. exempt notice. - Engine unchanged: 'exempt' treatment already routes to 3004/3100 and skips VAT lines. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(settings): remove approval rules from sidebar and routes * fix(invoices): ensure vat_registered defaults to true for invoice previews and API --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com> |
||
|
|
32d9978f1b |
Fix/chrome pdf preview csp (#572)
* feat: add option to exclude year-end closing entries in SIE export and related reports * delete docs * fix: allow Chrome's PDF viewer in verifikat document preview The /api/documents/:id/inline route shipped with `object-src 'none'` in its CSP, which blocked Chrome's built-in PDF viewer (it renders inline PDFs via an internal <embed>). Users on Chrome saw "Det här innehållet har blockerats" when expanding a PDF attachment in the bookkeeping view; Firefox (PDF.js) and Edge (own viewer) were unaffected, and JPGs worked because <img> isn't subject to object-src. Drops the CSP for this route to the minimum needed for embeddability: `frame-ancestors 'self'`. X-Content-Type-Options: nosniff plus the fixed Content-Type from the handler already block MIME confusion; X-Frame-Options: SAMEORIGIN + frame-ancestors still block clickjacking. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * feat(auth): add webmail deep link to email confirmation screens Mirrors Stripe's signup UX: after asking the user to verify their email, detect their webmail provider from the domain and show a button that opens the inbox in a new tab. Gmail gets a from:<sender> search pre-populated; Outlook/Yahoo/iCloud/Proton open the inbox directly. Unknown / custom domains fall back to the existing copy. Sender address is configurable via NEXT_PUBLIC_BRANDING_AUTH_EMAIL_FROM (default noreply@gnubok.se) so white-label installs can match their Supabase Auth SMTP config. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(auth): unblock first-time password set for BankID users with MFA Supabase rejects updateUser({password}) and mfa.unenroll with "AAL2 session is required" whenever a TOTP factor is enrolled. BankID magic-link logins produce AAL1, and middleware skips MFA enforcement for bankid_linked users, so they had no path to AAL2 — leaving them unable to set a backup password or disable MFA without going through the email-recovery escape hatch. - /api/account/password: branch on app_metadata.has_password. First-time set writes via service.auth.admin.updateUserById (no existing credential to protect, AAL2 guard does not apply). Change-password keeps the user-session updateUser so AAL2 still fires for credential rotation. - /mfa/verify: accept a safeReturnTo query param and route there after successful verify, so step-up flows can land back where they came from. - SecuritySettings: detect the AAL2 error from both change-password and mfa.unenroll and redirect through /mfa/verify?returnTo=/settings/account instead of toasting a dead-end error. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * Add tests and rounding utility for öre precision in bokslut calculations - Implemented `roundOre` function for rounding SEK amounts to two decimal places, ensuring consistent monetary calculations. - Introduced `ORE_TOLERANCE` constant for comparing rounded amounts, facilitating invariant checks in financial entries. - Created comprehensive tests for `roundOre`, covering typical cases, edge cases, and idempotency. - Added year-end invariants tests to verify database-level guarantees for closing entries, ensuring they balance to the öre and reject discrepancies. - Developed end-to-end tests for the dispositions chain, validating the correctness of calculations across various scenarios. * fix: update PDF rendering to remove Swish QR code generation and set default to disable Swish visibility * fix: enhance security by rejecting data URIs in safeReturnTo function tests * fix: improve rounding logic in roundOre function and add customer_type migration * fix: add customer_type column to customers and enforce CHECK constraint --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com> |
||
|
|
0087b7be3f | feat: add option to exclude year-end closing entries in SIE export and related reports (#567) | ||
|
|
50e5520b6d |
Add/preview invoice image (#565)
* feat: enhance invoice preview functionality with mock customer support * feat: update invoice preview logic to handle mock customers and improve error handling |
||
|
|
951bdb4e66 |
feat(bookkeeping): show per-account saldo on journal entry form (#562)
* feat(bookkeeping): show per-account saldo on journal entry form Adds a "Saldo" column to the journal entry form so bookkeepers can see the current balance of each account as of the entry date while drafting a voucher. Useful context for booking bank withdrawals, VAT clearings, and other balance-sensitive operations. - New GET /api/bookkeeping/account-balances?accounts=...&as_of=... returns per-account net (debit - credit) over posted entries up to and including the requested date. Batched in chunks of 200 entry IDs to stay under PostgREST IN-list limits. - JournalEntryForm fetches balances debounced 150ms on changes to the set of selected account numbers or the entry date; carries forward previously-known values so the cell doesn't flash to a skeleton on re-fetch. - Saldo is reference-only: it reflects "balance before this entry" and intentionally ignores the draft lines the user is currently editing. - Renders in both desktop (table column) and mobile (per-line caption) layouts. Tabular-nums, muted, right-aligned. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(bookkeeping): correct saldo semantics — IB + period-only, BS vs P&L Address Swedish compliance review on PR #562: 1. P&L accounts (class 3-8) no longer show a since-inception cumulative sum. They reset each räkenskapsår per BFNAR 2013:2; the saldo now reflects current-period activity only, matching trial-balance semantics. BS accounts (class 1-2) continue to include IB. 2. Opening balances are now sourced via the canonical getOpeningBalances() helper, which reads the explicit opening_balance_entry_id set by year-end closing or SIE import. Previously, summing journal_entry_lines from inception returned 0 for SIE-imported companies whose IB lives in a separate entry that the old query happened to include — and the wrong value once year-end ran and an OB entry was set without exclusion logic. 3. Relabel "Saldo" -> "Saldo (före)" / "Balance (before)" so the UI communicates that the figure excludes the draft being edited (BFNAR 2013:2 kap 8 self-documentation requirement). 4. Stop forwarding raw Supabase error.message to the client; log server-side via the structured logger and return a generic 'Internal server error' to avoid leaking schema details. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(bookkeeping): reject future as_of dates on account-balances endpoint Both compliance reviewers on PR #562 flagged this independently: a future as_of date would include posted entries dated after today in the activity window, producing a misleading "balance before this entry" hint that could drive incorrect verifikat entries (swedish-compliance-review-bot) or be used for future-date probing (SOC 2 PI1.1, GDPR Art.25(2)). - AccountBalancesQuerySchema.as_of now refines to <= today. - JournalEntryForm collapses the loading skeleton to 0 on any non-OK response so the saldo column doesn't get stuck spinning when a user enters a future entry_date (which the form's separate period validation already handles). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(bookkeeping): compare as_of guard against Europe/Stockholm date swedish-compliance-review-bot caught this on the previous fix: the future-date guard used new Date().toISOString().slice(0, 10), which is UTC. Between 00:00–02:00 CET (or 00:00–03:00 CEST), a Swedish bookkeeper's local "today" is one day ahead of UTC, so entering their Stockholm-local date would be rejected as a future date. Compare against Europe/Stockholm-local date via toLocaleDateString ('sv-SE'), which renders YYYY-MM-DD natively, so string comparison remains correct across DST. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com> |
||
|
|
78c91e00e4 |
feat: add language preference for customers to support invoice locali… (#561)
* feat: add language preference for customers to support invoice localization - Introduced language support for invoices, allowing customers to choose between Swedish and English. - Updated invoice PDF generation to reflect the selected language for titles, labels, and messages. - Enhanced email templates to generate content in the customer's preferred language. - Added migration to include a language column in the customers table with a default value of Swedish. - Updated tests to verify correct language usage in invoice emails and PDFs. * fix: debounce API requests in InvoicePreviewCard and update F-skatt terminology in email templates |
||
|
|
64bbeb4021 |
Fixed user issues (#559)
* Fixed user issues * feat: add personal_number column to customers for individual identification * feat: add personal_number field to makeCustomer function for enhanced customer identification * feat: add personal_number column with constraint check for customer identification |
||
|
|
e4488a900b |
feat: add user locale preference to user_preferences table (#555)
* feat: add user locale preference to user_preferences table
- Introduced a new column 'locale' in the user_preferences table to store per-user UI language preferences.
- Added a CHECK constraint to ensure only supported locales ('sv', 'en') are allowed.
- Triggered a schema reload notification for the changes.
chore: declare CSS module support in TypeScript
- Added a declaration for CSS modules in globals.d.ts to enable TypeScript support for importing CSS files.
* feat: add Swish as an invoice payment method in company settings
|
||
|
|
cc351158f8 |
Invoicing & account-security polish bundle (#550)
* feat: invoicing & account-security polish bundle Five independent improvements bundled to ship together: - BankID/password lockout fix: BankID-only users could enroll MFA and brick themselves (Supabase requires AAL2 to change password or unenroll MFA, and AAL2 needs a password sign-in). New app_metadata.has_password flag tracks this; middleware gates /mfa/enroll behind it, /account/set- password is the unlock path, SecuritySettings shows a banner, and /api/account/password is the single write path that flips the flag. Backfill script for existing users. - Swish invoice payment method: company_settings.swish + invoice_show_swish columns, validation in lib/api/schemas.ts (accepts 123XXXXXXX företag or 07XXXXXXXX mobile, strips whitespace/hyphens), rendered on invoice PDFs. - Send-reminders kill switch: per-company company_settings.send_invoice_ reminders toggle in PdfPrintSettings/Automatisering. Reminder processor also tightened: positive status allowlist (sent + overdue) so terminal statuses can never match; skip when customer already responded via reminder link; race-window re-check before send. - First-invoice logo prompt: one-shot dialog when creating the first invoice without a logo (issue #520). Self-limits via head-only count. - SIE export opening-balance fallback: route IB through getOpeningBalances so the compute_prior_opening_balances RPC supplies #IB after multi-year imports where opening_balance_entry_id is intentionally NULL. Previously #IB silently went to zero and #UB collapsed to current-period movements. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(account-polish): address PR review feedback - BankID-link path (extensions/general/tic/index.ts): read-merge-write app_metadata instead of passing { bankid_linked: true } alone. updateUserById REPLACES app_metadata wholesale, so the previous code would have wiped has_password for any user who later linked BankID, causing the set-password banner to (incorrectly) reappear and blocking the standard MFA enrollment button. The comment is now corrected. - Middleware (lib/supabase/middleware.ts): thread inner returnTo through the /mfa/enroll → /account/set-password redirect so the user lands on their original destination after the full chain completes, not on /. - safeReturnTo helper (lib/auth/safe-return-to.ts): replace the starts-with-/-but-not-// guard on mfa/enroll and set-password pages. The previous guard let /\evil.com and /@evil.com through. The new helper parses against a synthetic base origin and verifies it matches. - set-password page (app/(auth)/account/set-password/page.tsx): remove CLAUDE.md design system violations — bg-gradient-to-b on page bg, inline shadow-md style on the card, space-y-5, font-medium on the h1, rounded-xl on the card. Flat surface, hairline border, font-display h1 per the design tokens. - Swish dedup (lib/payments/swish.ts): extract normaliseSwish() and isValidSwish() helpers and use them in lib/api/schemas.ts, components/settings/BankDetailsForm.tsx, and the invoicing settings page. Single source of truth for the regex. - Password route (app/api/account/password/route.ts): emit a structured success log so the audit pipeline can detect password-set events, not just failures. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com> |
||
|
|
2879f6ed17 |
fix(inbox): sync matched_transaction_id on MCP-staged document attach (#549)
* fix(inbox): sync matched_transaction_id on MCP-staged document attach The REST attach route (app/api/transactions/[id]/attach-document) already updates invoice_inbox_items.matched_transaction_id so the inbox list shows the "Kopplad till transaktion" indicator. The MCP-staged path through commitAttachDocumentToTransaction did not, leaving inbox rows looking unprocessed after a /pending approval. Mirror the REST behaviour with a best-effort UPDATE (idempotent, gated on both FK columns being NULL). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(review): inspect supabase error on best-effort inbox link The Supabase client resolves with { error } rather than rejecting on RLS/DB failures, so the previous try/catch wrapping the inbox UPDATE never fired for the documented failure modes — the console.error was dead code. Switch to destructured { error } + if-block, matching the pattern used elsewhere in commit.ts. Applied to both the new MCP-staged path (lib/pending-operations/commit.ts) and the pre-existing REST route (app/api/transactions/[id]/attach-document/ route.ts) it was mirrored from. Both tests now assert console.error fires with the expected payload, so the swallow-and-log path is exercised for real instead of passing for the wrong reason. Addresses Greptile P1 + P2 on PR #549. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com> |
||
|
|
e71b4a9138 |
Add/mcp and visma (#547)
* fix: simplify COMING_SOON_PROVIDERS to include only bjornlunden and briox * feat: add supplier creation functionality and related operations * feat: reorder and enhance OAuth scopes in Visma integration * feat: implement create supplier functionality with validation and risk tier management |
||
|
|
e4d4d8e4dc |
feat: enhance OAuth scopes and UI for agent-driven approval process (#544)
* feat: enhance OAuth scopes and UI for agent-driven approval process * refactor: update OAuth scopes to enforce explicit user consent for write and approval actions |
||
|
|
239261a0be |
Bug/mcp bas lag (#543)
* fix(mcp): update workflow descriptions for transaction categorization and approval processes * feat: implement account validation for transaction categorization to handle inactive accounts * fix(tests): stub findMissingAccountsMock to ensure no missing accounts during batch-categorize tests * fix(errors): ensure deterministic sorting of account numbers in AccountsNotInChartError |
||
|
|
566ed72984 |
Bug/mcp connection issue (#541)
* feat(api): implement caching and logging in health check endpoint - Added in-memory caching for health check responses to reduce load on Postgres. - Introduced logging for error handling in health check. - Updated response structure to exclude error details from public responses. feat(api): enhance OAuth consent UI and scope handling - Improved consent UI to reflect exact requested scopes and added better user guidance. - Updated scope handling logic to ensure least-privilege access. - Enhanced styling for better user experience and accessibility. chore(docker): improve security and resource management in Docker setup - Updated Docker Compose configuration to enforce read-only file systems and resource limits. - Added health checks and logging options for better observability. - Introduced optional Caddy reverse proxy for TLS termination. fix(migrations): resolve ambiguity in create_company_with_owner function - Dropped orphaned 3-arg overload of create_company_with_owner function. - Recreated canonical 4-arg version with cash account seeding logic. - Ensured proper permissions for function execution in Postgres. * feat: enhance security checks for team membership in company creation * test: add CSP tests for OAuth authorization endpoint * feat: enhance error handling and reporting in bank file import process |
||
|
|
9aced4790c |
feat(api): implement caching and logging in health check endpoint (#526)
* feat(api): implement caching and logging in health check endpoint - Added in-memory caching for health check responses to reduce load on Postgres. - Introduced logging for error handling in health check. - Updated response structure to exclude error details from public responses. feat(api): enhance OAuth consent UI and scope handling - Improved consent UI to reflect exact requested scopes and added better user guidance. - Updated scope handling logic to ensure least-privilege access. - Enhanced styling for better user experience and accessibility. chore(docker): improve security and resource management in Docker setup - Updated Docker Compose configuration to enforce read-only file systems and resource limits. - Added health checks and logging options for better observability. - Introduced optional Caddy reverse proxy for TLS termination. fix(migrations): resolve ambiguity in create_company_with_owner function - Dropped orphaned 3-arg overload of create_company_with_owner function. - Recreated canonical 4-arg version with cash account seeding logic. - Ensured proper permissions for function execution in Postgres. * feat: enhance security checks for team membership in company creation |
||
|
|
8a6ce7093e |
feat: implement skattekonto drift detection and alerting (#525)
* feat: implement skattekonto drift detection and alerting - Add skattekonto drift computation logic to compare Skatteverket's saldo with GL 1630 sum. - Implement alerting mechanism for significant drift changes, with throttling to prevent alert spamming. - Introduce database functions to sum GL 1630 entries and list unbooked skattekonto rows. feat: create own account transfer detection - Develop logic to detect transfers between a company's own cash accounts based on counterparty IBAN. - Implement tests to validate detection logic under various scenarios, including matching and non-matching IBANs. feat: establish cash accounts as a first-class entity - Create cash_accounts table to manage routable cash accounts, replacing ad-hoc JSONB structures. - Implement functions for listing, upserting, and managing cash accounts, including primary account designation. feat: enhance GL line reconciliation functionality - Modify get_unlinked_1930_lines RPC to accept any account number for reconciliation, improving flexibility for different currencies. - Update related functions to ensure compatibility with the new cash_accounts structure. feat: capture counterparty IBAN in transactions - Add counterparty_iban column to transactions table to facilitate intra-account transfer detection. - Create index for efficient lookups based on counterparty IBAN. * feat: Enhance cash account handling and reconciliation processes - Updated reconciliation routes to enforce cash account validation for all account numbers, including '1930'. - Improved error handling for unknown cash accounts in reconciliation status and unmatched entries routes. - Changed CashAccountSelector to use sessionStorage instead of localStorage for better data privacy. - Fixed mapping for employer payroll taxes to route to the correct account (2730 instead of 2731). - Added safety checks for company IDs in the guessCounterAccount function to prevent injection vulnerabilities. - Introduced atomic RPC for setting primary cash accounts to avoid intermediate states during updates. - Seeded default cash accounts for new companies to ensure reconciliation routes are accessible from day one. - Updated email notifications for drift detection to avoid exposing sensitive financial data. - Enhanced bank reconciliation logic to handle multi-currency transactions correctly. - Renamed and updated tests to reflect changes in the underlying RPCs and ensure accurate coverage. - Migrated existing cash account rules to correct mappings in compliance with Swedish accounting standards. |
||
|
|
e211ab31be |
UI/settings api mcp (#524)
* feat(voucher): add create voucher and correct entry previews; update commit methods * feat: add support for pending operations in API key scopes and OAuth client management - Introduced new API key scopes for reading and approving pending operations. - Updated the scope groups to include pending operations. - Added new tools for listing and managing pending operations. - Implemented OAuth client registration and revocation endpoints. - Created a UI panel for managing OAuth clients, including registration and revocation. - Added tests for pending operations tools and OAuth allowlist functionality. - Implemented a database migration for OAuth client registrations with appropriate policies and constraints. * feat: Implement OAuth client registration rate limiting and enhance security measures - Added IP-based rate limiting to the OAuth client registration endpoint to prevent enumeration attacks. - Introduced a service-role client for allowlist lookups, ensuring trust boundaries are maintained. - Updated error responses to be uniform across different types of redirect URI validation failures. - Enhanced tests to reflect changes in OAuth scope handling, ensuring fallback to read-only scopes when no scopes are provided. - Improved handling of high-risk pending operations, requiring explicit confirmation for approvals. - Added audit logging for OAuth client revocations and pending operation approvals/rejections to maintain a security audit trail. - Refactored API key scope management to include default read-only scopes for OAuth-issued keys and added segregation-of-duties checks. * feat: add recurring invoice scheduling functionality - Implemented recurring invoice schedules with a new database schema. - Created API routes for managing recurring invoices (GET and POST). - Added cron job to automatically generate invoices based on schedules. - Developed service functions for computing next run dates and executing schedules. - Added tests for the new functionality, including validation and success cases. - Introduced error handling for various scenarios in the invoice creation process. * feat: refine VAT rate validation and enhance recurring invoice handling |
||
|
|
16164ea14c |
Fix/mcp fixes and bugs (#518)
* feat(voucher): add create voucher and correct entry previews; update commit methods * feat: add support for pending operations in API key scopes and OAuth client management - Introduced new API key scopes for reading and approving pending operations. - Updated the scope groups to include pending operations. - Added new tools for listing and managing pending operations. - Implemented OAuth client registration and revocation endpoints. - Created a UI panel for managing OAuth clients, including registration and revocation. - Added tests for pending operations tools and OAuth allowlist functionality. - Implemented a database migration for OAuth client registrations with appropriate policies and constraints. * feat: Implement OAuth client registration rate limiting and enhance security measures - Added IP-based rate limiting to the OAuth client registration endpoint to prevent enumeration attacks. - Introduced a service-role client for allowlist lookups, ensuring trust boundaries are maintained. - Updated error responses to be uniform across different types of redirect URI validation failures. - Enhanced tests to reflect changes in OAuth scope handling, ensuring fallback to read-only scopes when no scopes are provided. - Improved handling of high-risk pending operations, requiring explicit confirmation for approvals. - Added audit logging for OAuth client revocations and pending operation approvals/rejections to maintain a security audit trail. - Refactored API key scope management to include default read-only scopes for OAuth-issued keys and added segregation-of-duties checks. |
||
|
|
d27c3dd3dc |
Bug/customer cron job (#516)
* fix(reminder-processor): filter out credited invoices in overdue reminders * feat: implement linking of transactions to journal entries - Added POST endpoint for linking a bank transaction to an existing journal entry without creating new bookkeeping. - Implemented validation for required fields and error handling for various scenarios (e.g., missing journal_entry_id, transaction already linked, journal entry not found). - Created tests for the new endpoint to cover various cases including successful linking, error responses, and invoice handling. - Introduced duplicate payment detection logic to prevent double-booking of bank receipts. - Added a new component for correction affordance in the UI to facilitate user corrections on journal entries. * feat(invoice-matching): enhance force matching with expected journal entry validation |
||
|
|
f98ffee145 |
feat(bokslut): Phase 8 — make årsredovisning Bolagsverket-fileable (#511)
* feat(bokslut): Phase 8 — make årsredovisning Bolagsverket-fileable Closes most of the deferred items from PR #509 review. The ÅR PDF is now an honest draft a user can take to Bolagsverket: it includes the fastställe- intyg page ÅRL 8 kap 3 § requires, the K2-mandatory aktiekapital note, and the narrative edits actually survive a refresh. Narrative persistence (replaces the round-1 URL-query-param carry) - New table arsredovisning_narratives (UNIQUE per fiscal_period_id, length caps matching the API schema, RLS + updated_at trigger). - narrative-service.ts: getNarrative / upsertNarrative. - /api/.../arsredovisning/narrative GET + POST. POST does an explicit period-ownership pre-check before the upsert. - buildArsredovisningData loads persisted narrative as override layer (caller-supplied overrides → persisted → boilerplate). - ÅR page replaces the URL-query-param hack with a Spara button + saved indicator. The PDF download URL is plain again — no narrative content in access logs, browser history, or CDN logs. - PDF route stops parsing description/events/disposition query params. Also closes the GDPR Art.25(1) finding the bot flagged in PR #509. Fastställelseintyg PDF page - New 7th page in ArsredovisningPDF after Underskrifter. Carries the ÅRL 8 kap 3 § attestation text + the resultatdisposition + a signature slot. - Without this page Bolagsverket rejects the filing — flagged in the round-2 Swedish review on PR #509. K2 aktiekapital note + framework guard - buildK2Noter now takes entityType. Note 1 only claims K2 when the company is an AB; non-AB gets a generic principles statement so we don't falsely assert a framework. Future K3 election will flip this branch when it lands. - New aktiekapital note (required K2 note for AB per BFNAR 2016:10 ch.18). Reads aktiekapital / antal_aktier / kvotvärde from company_settings; emits a "saknas — komplettera under Inställningar" placeholder when missing. Manual "Mark as signed" PATCH + UI button - New PATCH /signatures/[signatureId] — flips pending → signed (manual / paper flow) or pending → declined. Real BankID wiring is Phase 9 and will use the same markSignatureSigned helper with the BankID callback as the trigger. - ÅR page renders a "Markera som signerad" button on every pending row. Small cleanups all flagged in PR #509 reviews - AccrualProposal.reverses_on type: '' → null. The future accrual-reversal cron will filter `reverses_on IS NOT NULL`; an empty string would silently match. - ArsredovisningData.company.sate → city. The typo carried into the type in earlier phases; renaming now before any external consumer takes a dependency. - signer_name CHECK length 200 at storage layer (matches the API .max(200) added in PR #509 round-2 — GDPR Art.25.2 belt-and-braces). - Soliditet equity filter now has a code comment explaining the K2 vs K3 branch the bot wanted documented for the future K3 migration. Explicit follow-ups (each merits its own focused PR): - Real BankID signing — needs provider choice + polling + QR. Phase 9. - Accrual reversal cron — auto-flip 17xx/29xx accruals on Jan 1 of next FY. - Medelantal anställda proper annual average — needs salary-run aggregation. - Vacation avgifter age-tier split (10.21 % for 67+) — needs upstream vacation-liability report to expose age. - K2 noter expansion (lån till närstående, eventualförpliktelser detail). Verification - 94 unit tests pass (bokslut + MCP subsets) - Zero typecheck errors on any touched file - Zero lint errors on any touched file - Migration 20260517140000 applied to remote Supabase via MCP Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(bokslut): address PR #511 round-1 — 3 P1s + 3 real concerns 3 P1s from Greptile (all real bugs): - entityType default reintroduced the K2 false-assertion. build-data.ts defaulted `entity_type ?? 'aktiebolag'`, which means every unconfigured company would still claim K2 in Note 1 — exactly the false-assertion the framework guard was added to prevent. Now defaults to 'unknown' and the guard treats that as not-K2. New warning surfaces in the data so the UI can prompt the user to fill in företagsform. - Signatures PATCH ignored the URL fiscal-period id. The route destructured `id` from params but never used it as a filter, so PATCH /periods/A/ signatures/SIG_FROM_B succeeded silently — broken REST contract + IDOR across periods. Rewrote the handler to do a single UPDATE with all four filters: id, company_id, fiscal_period_id, status='pending'. Missing row returns 409 SIGNATURE_INVALID_TRANSITION instead of silent 200. - Signatures state-machine guard was missing. Without status='pending' in the WHERE clause, an already-signed signature could be flipped back to declined (or vice-versa). Now part of the consolidated UPDATE above. 3 real concerns: - Narrative GET lacked ownership pre-check. POST already had it; mirroring on GET so a valid JWT for company A can't probe / enumerate company B's period IDs through the narrative endpoint. - Narrative POST lacked period-lock check. BFL 5 kap 5 § makes räkenskapsinformation immutable after filing — editing the förvaltningsberättelse on a closed/locked period now returns PERIOD_LOCKED. - Aktiekapital placeholder text would land in Bolagsverket-filed PDF body. When aktiekapital fields are missing, the note now omits entirely and a warning surfaces in the ArsredovisningData.warnings array — the UI flags it pre-download with a "Innan inlämning till Bolagsverket" list. Same surface picks up the entityType=unknown and entityType=non-AB warnings. Plus 2 schema improvements from Swedish review: - AGM date persistence. Fastställelseintyg date was a literal "____" blank, defeating the point of a generated PDF. New agm_date column on arsredovisning_narratives + UI date input + PDF now renders the saved date. When missing, the warning surface flags it. - Composite UNIQUE constraint on (company_id, fiscal_period_id) instead of just fiscal_period_id. UUIDs don't collide across tenants in practice but the constraint should match the tenant boundary so a logic error in onConflict resolution can't write to another company's row. Migration 20260517160000 drops the old constraint and adds the composite. Verification - 94 unit tests pass - Zero typecheck errors on any touched file - Zero lint errors on any touched file - Migration 20260517160000 applied to remote Supabase via MCP Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(bokslut): address PR #511 round-2 — 5 real concerns + BFL/GDPR conflict 5 real concerns from the round-1 bot re-eval: - Narrative SELECT * leaked user_id to the frontend. getNarrative and upsertNarrative now project an explicit NARRATIVE_API_COLUMNS list (id, company_id, fiscal_period_id, narrative fields, agm_date, updated_at). user_id and created_at stay server-side. NarrativeRow type updated to match. Closes Art.25.2 + 2× A.8.3. - agm_date validated only as YYYY-MM-DD regex. '2024-13-99' passed Zod and surfaced as a Postgres 500 instead of a 400. Added a refine() that parses with new Date() and confirms ISO round-trip equality, so invalid calendar dates return a clean structured-error. - agm_date had no range check. ÅRL 8:3 → 7:10 §§ requires the AGM to be held after period end and within 6 months for privat AB; build-data warnings now flag agm_date <= period_end (impossible) and agm_date > period_end + 6 months (deadline). Warning surface in the UI already picks these up from the existing list. - Fastställelseintyg signer label "Styrelseledamot / VD" conflated legally distinct roles per ÅRL 8:3 → 6:6-7 §§ — a VD without board membership cannot sign. Label is now "Styrelseledamot (närvarande vid stämman)" and the body text references the AGM's resolution ("stämmobeslutet") rather than the board's proposal — the AGM votes, and it is the vote that must be certified. - Aktiekapital warning suppressed for entityType='unknown'. The maybeAb branch in buildK2Noter now fires for both 'aktiebolag' and 'unknown' so an unconfigured company that's actually an AB still gets prompted to fill in aktiekapital before filing. Note body stays omitted when fields are missing; only the warning surfaces. BFL × GDPR conflict (new migration 20260517180000): - Both arsredovisning_narratives and arsredovisning_signature_requests had user_id with ON DELETE CASCADE → auth.users. BFL 7 kap 1 § requires räkenskapsinformation to be retained for 7 years; GDPR Art.17 erasure or membership revocation would silently delete filed årsredovisning narrative + BankID signature evidence. BFL wins for filed financial records — user_id is now nullable with ON DELETE SET NULL on both tables. The company FK keeps its CASCADE (company deletion takes its räkenskapsinformation with it; that's a separate workflow). Deliberately not chasing on this round: - ISO A.8.12 historical PDF query-param logs — process item for the risk register, not code (the leak path is closed in this PR's first commit). - "Collapse the two narrative migrations" — both already shipped to remote and merged; the interim window is in the past. - "user_id on row vs separate audit log" — architectural debate; tracked but out of scope for this PR. - Multi-signer fastställelseintyg + DB-level period-lock trigger — bigger scope, each merits a focused follow-up. Verification - 89 unit tests pass - Zero typecheck errors on any touched file - Zero lint errors on any touched file - Migration 20260517180000 applied to remote Supabase via MCP Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com> |
||
|
|
fd4274787c |
feat(bokslut): Phases 4-7 — accruals + årsredovisning + EF + MCP tools (#509)
* feat(bokslut): Phases 4-7 — accruals + årsredovisning + EF + MCP tools Builds on the Phase 1-3 PR (#508). Same K2-AB scope decisions hold (iXBRL deferred, K3 deferred, koncernredovisning out of scope) — what ships: PHASE 4 — Periodiseringar - lib/bokslut/accruals/accrual-detector.ts: auto-proposes the vacation- liability change (delta on 2920 against 7090 + 31,42 % avgifter on 7519/2940) by reading lib/reports/vacation-liability.ts. Manual prepaid (17xx) / accrued (29xx) / audit-fee builders for entries the heuristic can't derive — supplier-invoice service_period detection is deferred until the data model grows the field. - /api/bookkeeping/fiscal-periods/[id]/accruals (GET + POST) — posts each accrual as a separate manual-source journal entry with the next-day reverse date embedded in the description. - New AccrualsStep wizard step between Preflight and Dispositions. - Auto-reversal cron is follow-up infra; for now reverses_on is metadata + a visible UI badge. PHASE 5 — Årsredovisning PDF + signing - lib/bokslut/arsredovisning/{types,build-data,arsredovisning-pdf,signature -service}: pre-fills flerårsöversikt from prior 3 fiscal periods, eget- kapital-förändring from journal data, K2-minimum noter with avskrivningstider auto-derived from the asset register and medelantal anställda from the employees table. PDF via @react-pdf/renderer mirroring the income-statement/pdf pattern. - Migration 20260516170000: arsredovisning_signature_requests with RLS, signed-immutability trigger, and DELETE policy that blocks signed rows. Signature-service exposes list/create/markSigned/isFullySignedOff. - BankID call itself is not wired here — the table + service make the request layer available so a follow-up can hook lib/auth/bankid.ts to the sign action without rework. - /api/.../arsredovisning (data + pdf + signatures) endpoints. - /bookkeeping/year-end/arsredovisning page with editable narrative, flerårsöversikt table, signer slots, PDF download, and Bolagsverket Mina Sidor link. Explicit warning about the FY2026 iXBRL mandate. PHASE 6 — Enskild firma NE-bilaga UI - lib/bokslut/enskild-firma/{egenavgifter,rantefordelning,periodiseringsfond -ef,expansionsfond}-calculator.ts. All declaration-only — never produce a journal entry. Egenavgifter 28,97 % / pensionärssats 10,21 % / passive SLP 24,26 %, schablonavdrag 25/10/20 %. Räntefördelning SLR+6 / SLR+1 with the -500 000 negative threshold. P-fond EF cap 30 % (vs 25 % för AB). Expansionsfond 125,94 % av kapitalunderlag, 20,6 % skatt. - EfDeclarationSection mounted inside DispositionsStep when entity_type is enskild_firma — live recompute as the user adjusts kapitalunderlag, prior-year amounts, p-fond desired, expansionsfond change. Each card shows the NE-bilaga ruta the number lands in. NE-bilaga preview link. PHASE 7 — Agent-native MCP tools - gnubok_propose_dispositioner: read-only AB dispositions proposal - gnubok_propose_accruals: read-only accruals proposal - gnubok_propose_annual_depreciation: read-only depreciation per asset - gnubok_post_annual_depreciation: stages depreciation commit (high-risk) - gnubok_preview_arsredovisning: structured K2 ÅR preview - gnubok_preview_ef_declaration: EF skattemässiga justeringar preview - All ≤280-char descriptions, additionalProperties:false, conform to STAGED_OPERATION_SCHEMA for write tools. Read tools return the same shapes as their HTTP counterparts so agents and the UI share a contract. - The existing GET /bokslutsdispositioner endpoint now calls the shared buildDispositionsProposal helper that the MCP tool also uses, removing the duplicate logic that lived in both. Verification - 149 unit tests pass (was 125 on Phase 1-3; +24 across accruals, EF calculators, expansionsfond, periodiseringsfond-ef) - Zero lint or typecheck errors on any new file - Migration 20260516170000 applied to remote Supabase via MCP Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(bokslut): address PR #509 round-1 — CI blocker + 3 P1s + compliance bundle CI BLOCKER (output-schema test) - The 5 new read-only MCP tools (gnubok_propose_dispositioner / _accruals / _annual_depreciation, _preview_arsredovisning / _ef_declaration) were missing outputSchema, which the existing strict-schemas guard reads as a hard failure on core-only CI. Added a permissive { type: 'object', additionalProperties: true } outputSchema to each — the return shapes are the same as their HTTP counterparts and trying to mirror them inline would duplicate the type tree across two boundaries. P1 — vacation accrual: two real bugs in one entry - Delta was anchored on the OPENING balance of 2920, so any mid-year postings (partial accruals, reversals) were ignored. Now anchors on the current closing balance via tb.rows.find(2920).closing_credit - closing_debit. Updated the computation field label to current_2920. - More importantly: 2920 is a balance-sheet carry-forward (semesterlöneskuld persists until the actual vacation is paid). The original implementation set reverses_on to Jan 1 of the next year, which would zero the liability on day 1 of the new year — a known Swedish bookkeeping error. The vacation proposal now ships with reverses_on = '' to suppress the reversal badge, the API route emits a "Bokslutsjustering" description instead of "Periodisering (vänds …)", and AccrualsStep renders "Rullas vidare (ingen vändning)" so the user knows the liability carries forward. P1 — signature_requests immutability gap (security) - The existing trigger only guarded role / signer_name / signed_at / status on signed rows, leaving bankid_signature_data, signer_personnummer_encrypted, and signer_personnummer_hash mutable. An UPDATE on a signed row could silently alter the BankID proof. New migration 20260517090000 replaces the trigger function to cover the full audit-critical column set (plus fiscal_period_id and company_id as belt-and-braces). P1 — narrative edits never reached the PDF - The /bookkeeping/year-end/arsredovisning page let the user edit description / important_events / resultatdisposition but the download link pointed at a plain GET that regenerated boilerplate. Wired the PDF endpoint to accept description / events / disposition as query params (length-capped) and the page now constructs the download URL with the current narrative state — only fields the user actually changed are included, keeping the URL short for the unchanged-defaults case. Compliance quick wins - Added period_lock check to gnubok_post_annual_depreciation MCP tool (matches the existing accruals POST guard). - Added explicit fiscal-period ownership pre-check to the signatures POST route (RLS would reject anyway; the route layer just makes the 404 envelope cleaner). - Replaced free-text role on the signatures schema with an enum allowing only Styrelseledamot / Styrelseordförande / VD / Verkställande direktör. - Added Cache-Control: no-store + Pragma: no-cache to the ÅR PDF response so the document (officer names + financials = personal data) isn't cached by any intermediary. - Sanitized period_end in the PDF Content-Disposition header to dodge header-injection via stray chars (defensive — period_end is a date, but the cost is one regex). - Softened the iXBRL warning text on the ÅR page: digital filing is proposed by Bolagsverket but not yet enacted; PDF is still valid today. False positives I'm intentionally not chasing on this round - Greptile P2 `sate` typo in the address city field — the rename would touch the type and every consumer; defer. - Greptile P2 "computation field label" — already addressed as part of the P1 vacation fix above. - Compliance V2.2 "silent skip" in accruals POST — the silent skip is in the UI's empty-row filter, not in the server route. Server validation already returns 400 via Zod for any invalid item. Verification - 154 unit tests pass (was 149; +5 from re-running the MCP strict-schemas + output-schema suites that now include the new tools) - Zero typecheck errors on any touched file - Zero lint errors on any touched file - Migration 20260517090000 applied to remote Supabase via MCP Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(bokslut): address PR #509 round-2 — 3 real bugs + 5-item polish 3 real bugs from the round-1 Swedish review + Compliance Swarm re-eval: - Soliditet inflated by obeskattade reserver. build-data.ts filtered equity with startsWith('20') || startsWith('21') — but 21xx (periodiseringsfonder, överavskrivningar) are partially deferred tax, not eget kapital. Splitting them out per K2 / ÅRL. Filter is now startsWith('20') only. - Resultaträkning omitted bokslutsdispositioner + skatt rows. K2 RR must include 88xx (dispositioner) and 89xx (skatt) before "Årets resultat" per ÅRL 3:2 — without them, the printed RR doesn't reconcile to BS 2099 and the document is non-compliant for any AB that posted bolagsskatt or periodiseringsfond. flattenIncomeStatement now splits the financial sections on title (Bokslutsdispositioner / Skatter och årets resultat) and emits the K2-required intermediate subtotals: "Resultat efter finansiella poster" → dispositioner → "Resultat före skatt" → skatt → "Årets resultat". - Accruals POST had no idempotency. Re-running the wizard (or a retried POST after a flaky network) would create duplicate accrual entries that distort both the balance sheet and trial balance. New findExistingAccrualEntry helper queries the period for an existing posted entry whose description matches the kind's stable prefix (or for manual prepaid/accrued, the user-supplied description). Duplicates land in a new `skipped` array in the response with a reference to the existing entry id, rather than producing a second posting. 5-item compliance polish: - signer_name on the signatures schema now has .max(200) per GDPR Art.25.2 data-minimization. Swedish personal names are well under that — the bound is a defense against an unbounded-string injection. - Audit-fee accrual: 6420 is BAS-specific to lagstadgad revision. Bokslut fees for a non-revisionspliktigt bolag (liability_account = 2991) now debit 6590 (övriga externa tjänster) instead — Skatteverket may query a 6420 debit when there is no revisor i bolaget. - Räntefördelning ne_ruta label: was 'R30 / INK1 kapital' (confusing — INK1 is a separate form). Positive now reads 'R30 (avdrag i näringsverksamhet)' with the INK1 T4 cross-reference moved to the description; negative reads 'R30 (tillägg till resultat)'. - gnubok_post_annual_depreciation MCP tool now checks the caller's company_members role and throws on viewer. RLS would reject the underlying INSERT anyway; failing fast here produces a cleaner error than the cascaded RLS rejection. Mirrors the HTTP route's { requireWrite: true } guard. - Signature DELETE policy now blocks both 'signed' AND 'declined' rows. A declined signature is auditable evidence (board member refused) and is material under ABL 8 kap. New migration 20260517100000. Bot-flagged items I'm deliberately not chasing on this round: - V8.2.1 × 2 cross-tenant findings on ÅR routes — same false-positive class I've responded to repeatedly: buildArsredovisningData internally filters by company_id, bot can't see past the route handler. - V4.5 / V2.2 MCP arg Zod redundancy — MCP server's central handler validates against each tool's inputSchema. - Narrative-in-URL GDPR concerns — proper fix is POST + body or server-side persistence; tracked as follow-up. - `sate` typo (Greptile P2) — type-wide rename, deferred. - Vacation avgifter age-tier rate split — inherited from upstream generateVacationLiability which doesn't expose age; needs upstream work. - Medelantal anställda proper monthly average — needs salary-run aggregation across the year, follow-up. - K2 noter aktiekapital + fastställelseintyg blocks — real K2 gaps, tracked as follow-up. Verification - 94 tests pass (subset for bokslut + MCP suites; full Phase 1-7 suite unchanged in scope) - Zero typecheck errors on any touched file - Zero new lint errors on any touched file (the 2 server.ts warnings are pre-existing) - Migration 20260517100000 applied to remote Supabase via MCP Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com> |
||
|
|
05078c9d8e |
feat(bokslut): year-end wizard with bokslutsdispositioner + asset register (#508)
* feat(bokslut): year-end wizard with bokslutsdispositioner + asset register
Ships the first user-visible bokslut surface for K2 aktiebolag. The year-end
engine, INK2/INK2R/INK2S generator, and reconciliation reports already existed
in lib/core/bookkeeping/ and lib/reports/; this work wires them into a real
multi-step UI, adds the missing dispositioner calculators (bolagsskatt,
periodiseringsfond, överavskrivningar, SLP), and introduces a fixed-asset
register that feeds planenliga avskrivningar into the same flow.
PHASE 1 — Wizard around the existing year-end engine
- Replaces the "Kommer snart" stub at /bookkeeping/year-end with a 4-step
wizard (Kontroll → Dispositioner → Förhandsgranska → Verkställ) plus a
Klart result view
- New aggregator lib/bokslut/readiness-aggregator.ts composes
validateYearEndReadiness with bank-reconciliation status and entity-typed
reminders into one fetch backing the preflight step
- New endpoint GET /api/bookkeeping/fiscal-periods/[id]/bokslut-readiness
PHASE 2 — Bokslutsdispositioner calculators
- lib/bokslut/tax-provision/{bolagsskatt,sarskild-loneskatt}-calculator.ts —
20.6 % on taxable result → 8910/2512 (with non-deductible / non-taxable
manual adjustments and schablonintäkt pass-through) and 24.26 % SLP on
posted pension costs → 7533/2514
- lib/bokslut/reserves/periodiseringsfond-service.ts — proposeAvsattning
(25 % cap, BAS 212X cohort accounts) + proposeAteforing (FIFO 6-year
mandatory reversal with schablonintäkt computation) + balance lookup
- lib/bokslut/reserves/overavskrivningar-service.ts — 30-rule + 20-rule
helpers + proposeOveravskrivningar (8853/2153)
- New endpoint /api/bookkeeping/fiscal-periods/[id]/bokslutsdispositioner
(GET ordered proposals, POST commits user-chosen ones as separate
year_end vouchers via the journal engine)
- New DispositionsStep UI: per-card accept/skip + editable amount where
meaningful; mandatory p-fond reversals can't be skipped
- INK2 bug fix: ink2-engine.ts SRU mapping ranges previously pointed at
accounts BAS doesn't seed (8810/8830/8840). Corrected to 8811 (avsättning),
8819 (återföring), 8830 (lämnade koncernbidrag) so calculator output now
flows into INK2 correctly. Regression-locked with 6 new mapping tests.
PHASE 3 — Anläggningsregister + depreciation engine
- New migration 20260516120000_assets_and_depreciation.sql: assets table
(category, BAS-triple, K3 components JSONB reserved) and
depreciation_schedules (asset+period+journal_entry link). RLS via
user_company_ids(), immutability triggers after disposal/posting.
- lib/bokslut/assets/asset-service.ts — CRUD + disposal that posts a proper
gain/loss entry against 3973/7973
- lib/bokslut/assets/depreciation-engine.ts — computeAnnualDepreciation
(linear, pro-rata at acquisition/disposal/end-of-life) +
proposeAnnualPostings + commitAnnualPostings (one entry per asset)
- New endpoints /api/assets (CRUD + dispose) and
/api/bookkeeping/fiscal-periods/[id]/depreciation (preview + commit)
- /assets list+create page with K2 schablon defaults (3y datorer,
5y inventarier, 25y byggnader); sidebar entry added
- DepreciationPanel mounted at the top of DispositionsStep; posting
refreshes dispositions so bolagsskatt picks up the new result
Out of scope (per the agreed plan): K3 framework, iXBRL filing to
Bolagsverket (manual export only for now — regulatory risk flagged for
FY2026 closings), inventory module, koncernredovisning, revisor workflow.
Verification
- 116 unit tests pass across lib/bokslut/, lib/reports/ink2/, and the
existing lib/core/bookkeeping/year-end-service suite
- Zero lint or typecheck errors in any new file
- Migration applied successfully via Supabase MCP
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* fix(bokslut): address PR #508 review — P1 correctness + P2 conventions
P1 — stale reminders in readiness-aggregator (greptile)
Remove the depreciation_manual / bolagsskatt_manual / periodiseringsfond_manual
nudges. After Phase 3, the wizard handles all three automatically via
DepreciationPanel and the dispositioner calculators — surfacing them as
manual reminders told users to repeat work the page already did. Only the
accruals_manual reminder (Phase 4 hasn't shipped) and the EF-only
ef_skatt_via_ne reminder remain.
P1 — netBookValueAfter ignored prior accumulated depreciation
proposeAnnualPostings now fetches all prior posted depreciation_schedules
for the company (excluding the current period) and sums them per asset, so
the displayed restvärde reflects every previously-booked year of avskrivning
instead of only this year. Without the fix, a 5-year asset in year 3 would
have shown 48 000 instead of the correct 24 000 net book value.
P1 — ordering bug in dispositioner POST handler
The 25 % p-fond avsättning cap derives from the current trial balance, so
mandatory återföring entries must post first. Added a server-side sort by
canonical bokslut order (återföring → överavskrivningar → avsättning → SLP
→ bolagsskatt) regardless of the client array order. The cap can no longer
be evaluated against a stale pre-återföring net result.
P2 — depreciation_schedules missing updated_at
New migration 20260516140000_depreciation_schedules_updated_at.sql adds the
column + trigger via update_updated_at_column(). Per CLAUDE.md migration
conventions, never modified the original migration. DepreciationSchedule
type updated.
P2 — addMonths end-of-month overflow
Replaced setUTCMonth (which overflows: Jan 31 + 1 month → Mar 3) with a
day-clamping implementation that produces Feb 28/29. Without the fix,
lifeEndExclusive landed one day too late and slightly over-depreciated.
New regression test asserts Jan 31 + 12 months stays in January.
P2 — pg-real tests for new triggers and RLS
tests/pg/assets.pg.test.ts (13 tests) covers:
- enforce_asset_post_disposal_immutability blocks every financial field
after disposal, allows notes/name through
- assets_disposal_atomic CHECK requires both disposed columns set together
- enforce_depreciation_schedule_immutability blocks edits after
journal_entry_id is linked, allows them before
- depreciation_schedules delete RLS policy filters out posted rows
- assets + depreciation_schedules RLS isolates across companies
Verification
- 117 unit tests pass (was 116, +1 for the addMonths regression)
- New pg-real suite syntactically + type-correct; will execute in CI
- Zero lint or typecheck errors in any touched file
- Migration 20260516140000 applied to remote Supabase via MCP
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* fix(bokslut): address compliance-swarm + swedish-review findings
Real bugs surfaced by the bots on the first push that weren't covered by
greptile's inline P1/P2 set:
- Immaterial asset disposal hit the wrong BAS accounts. disposeAsset always
posted gain/loss to 3973/7973 regardless of category. For category
'immaterial' it must use 3013 (vinst) / 7813 (förlust) per BAS — using the
tangible accounts misclassifies in INK2R. Now branches on category. Two
new regression tests pin each branch.
- acquisition_cost CHECK was too loose. CreateAssetSchema accepted 0 (just
nonnegative). Tightened to z.number().positive() — a zero-cost asset
creates a no-op depreciation row and a balance sheet line that nothing
reconciles against.
- UpdateAssetSchema let users remap BAS accounts arbitrarily. Bot flagged
this as a defense-in-depth gap (V4.5). Added BAS_RANGES_BY_CATEGORY
validation at both the schema layer (Create) and the service layer
(Update) so user-supplied account overrides must stay inside the
category's expected BAS range. INK2R mappings and the depreciation
engine's category-driven defaults now can't drift.
Swedish accounting review:
- Building/markanläggning defaults — clarified UI copy. The 25-year
schablon is K2-redovisning, not the IL skattemässig rate. New helper text
spells this out. Markanläggning default lowered from 20→10 years
(Skatteverket guidance allows 10 % rate; 20 was on the upper bound
without justification).
- createAsset doesn't post the acquisition entry by design — that gap
wasn't called out anywhere in the UI. Added a tip box in
CreateAssetDialog explaining that the acquisition must already be in the
books; the register only drives depreciation.
- Disposal VAT (ML 3:3 / 7:3) not handled — sale of a deduct-eligible
anläggningstillgång is in principle 25 % momspliktig. Documented this as
a known limitation in the disposeAsset docstring so any future UI
surfacing the disposal endpoint warns the user.
Documented (not fixed yet) — bot was right but wider-scope work:
- SOC 2 PI1.3: dispositioner POST loop is not transactional. A failure
midway leaves partial postings. Added a code comment explaining the
recovery path (re-POST omitting committed kinds — each calculator
re-derives from current TB). Real atomicity via an RPC wrapper is Phase
5+ work.
False positives intentionally not changed:
- 4× OWASP V8.2.1 cross-tenant findings — service functions already filter
by company_id; the bot can't see past the route handler.
- V2.3 client-supplied amount clamping — proposeAvsattning and
proposeAteforing both clamp via Math.min already.
- A.8.15 audit events — withRouteContext already logs completion.
- Schablonintäkt journal entry — per IL 30:6a it's a skattemässig
justering, never booked. Current implementation is correct.
- Voucher series 'A' — matches existing executeYearEndClosing convention;
not changing here in isolation.
Verification
- 119 tests pass (was 117, +2 for the immaterial-disposal branches)
- Zero lint or typecheck errors on any touched file
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* fix(bokslut): address compliance round 4 — BAS account overlap + method gate
Two real bugs the compliance bot caught after my round-3 fixes (both new on
its re-run, not in the original report):
- BAS account overlap not prevented: my BAS_RANGES_BY_CATEGORY uses the same
class range for asset and accumulated (e.g. immaterial: both 1010–1099,
building: both 1100–1199). Nothing stopped a user from picking the same
account for both, which would silently net acquisition cost against
accumulated depreciation in one bucket and corrupt INK2R 720x mappings.
CreateAssetSchema now rejects bas_asset_account === bas_accumulated_account
in a superRefine cross-field check; updateAsset enforces the same invariant
by reading the existing asset and validating the merged result.
- declining-balance methods silently fell back to linear. The DB enum allowed
declining_balance_30 / declining_balance_20, but the engine's
computeAnnualDepreciation only implements linear math. A determined caller
(MCP, curl, future UI) could create an asset labelled as räkenskapsenlig
avskrivning and get linear charges — silently wrong numbers under a
misleading method. Both CreateAssetSchema and UpdateAssetSchema now refine
the depreciation_method enum to require 'linear'. The DB enum stays open
for a future phase to add proper support. Stale comment in
depreciation-engine.ts updated to reflect the new invariant.
False positives I'm explicitly not chasing further on this round:
- 3× repeated OWASP V8.2.1 cross-tenant — services already filter by
company_id; bot can't see past the route handler. Round 3 already added
service-layer tests and inline reasoning.
- V2.3 atomicity upgrade to high — bot now flags it harder *because* I
documented it in round 3. The existing executeYearEndClosing has the same
non-transactional sequential-write pattern; wrapping just this endpoint
in an RPC while leaving the rest inconsistent is worse than the doc
comment. Real atomicity is Phase 5+.
- Disposal VAT user-facing warning — no UI surfaces dispose yet; docstring
in the service is sufficient until the UI ships.
Verification
- 119 tests pass
- Zero lint or typecheck errors on any touched file
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* fix(bokslut): address compliance round 5 — disposal integrity + öavskr accounts
Round-5 bot feedback after my round-4 push (the sticky reports re-edited
themselves with two new genuine findings; bot count: 18 → 14 active).
P1 — accumulated_depreciation was client-supplied (OWASP V8.2.1, swedish
compliance review): the dispose endpoint accepted accumulated_depreciation
as a request-body number. A malicious or buggy caller could inflate it to
manipulate the book-value calculation and pocket a phantom gain. Now:
- DisposeAssetSchema no longer accepts accumulated_depreciation
- disposeAsset sums planned_depreciation from depreciation_schedules where
journal_entry_id IS NOT NULL for the asset, server-side
- New regression test "server-derives accumulated_depreciation — caller
cannot inflate gain" pins the server-derivation against the prior attack
- Limitation: manual avskrivningsverifikationer posted outside the engine
aren't captured. Phase 5+ can swap this for a trial-balance scan on
bas_accumulated_account if that gap matters.
P2 — överavskrivningar hardcoded 8853/2153 regardless of asset category
(swedish-asset-accounting): for buildings BAS uses 8852/2152 and for
immateriella tillgångar 8851/2151. Edge case for K2 SME (öavskr on
buildings is rare; on immateriella rarer still) but worth not lying about
the accounts. Now:
- New OVERAVSKRIVNING_ACCOUNTS table maps category → expense/accumulated
pair (machinery_equipment, building, immaterial, group)
- proposeOveravskrivningar accepts optional category, defaults to
machinery_equipment (the dominant K2 case — no behaviour change for
existing callers)
- POST handler item schema accepts optional category
- Label + description strings now name the actual accounts used
- 3 new tests cover the building, immaterial, and default branches
False positives I'm still declining to chase (already covered in prior
commit messages):
- 3× repeated OWASP V8.2.1 cross-tenant — services scope by company_id;
bot can't see past route handler
- V2.3 atomicity — existing executeYearEndClosing has the same pattern;
wrapping just this endpoint is inconsistent; real fix is Phase 5+ RPC
- Disposal VAT user-facing warning — no UI surfaces dispose yet
Verification
- 123 tests pass (was 119, +3 for öavskr category branches and +1 for the
server-derivation regression test, with one prior test rewritten to use
the new server-supplied accumulated path)
- Zero lint or typecheck errors on any touched file
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* fix(bokslut): pre-merge polish — building disposal accounts, SLR-2026 rate
Last polish round before merge. All three from the round-5 sticky
Swedish-accounting-review update:
- Building / markanläggning disposal posted gain/loss to 3973/7973. BAS
2026 routes those to 3971/7971 (the SRU mapping points them at a
different INK2R field, so the existing accounts misclassify). Extended
the existing immaterial branch (3013/7813) into a three-way:
immaterial → 3013 / 7813
building / land_imprv → 3971 / 7971
other tangible → 3973 / 7973
Two new regression tests pin the building and land_improvement branches.
- DEFAULT_SCHABLONINTAKT_RATE was 0.03, based on SLR 2024-11-30 (1.96 %).
For closings of inkomstår 2026 the rate is SLR 2025-11-30 (2.55 %) + 1 pe
= 3.55 %. The wrong rate under-taxes the schablonintäkt, which feeds into
bolagsskatt. Updated to 0.0355 and rewrote the doc comment to track both
years so the next bump is obvious.
- Jämkning of input VAT for buildings / markanläggning disposed within the
10-year jämkningsperiod (ML 9 kap 8–11 §§) is out of scope for this PR
but should not be silently absent — added a KNOWN LIMITATION block to the
disposeAsset docstring so any future UI surfacing disposal checks the
10-year window and warns the user.
After this push the PR has 125 passing tests, all CI green, no merge
conflicts, and the only remaining bot complaints are repeat false
positives or Phase 5+ scope (RPC atomicity, full asset disposal UI,
K3 component depreciation).
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
||
|
|
c3021f1ec6 |
Bug/supplier invoice input (#504)
* feat(settings): add information about Skatteverket's scope for transactions * feat(supplier-invoices): replace Input with Controller for description field in NewSupplierInvoicePage feat(bookkeeping): update AccountCombobox styling by removing height class docs(settings): add documentation link for SkatteverketConnectPanel * refactor(sandbox-seed): remove redundant environment check for sandbox seeding * feat(sandbox-seed): implement rate limiting and IP address handling in sandbox seed endpoint * fix(supplier-invoice): add ref to input fields for better form handling |
||
|
|
b46b572ee9 |
fix(invoices): duplicate-payment guard on customer mark-paid + categorize (#502)
* fix(invoices): duplicate-payment guard on customer mark-paid + categorize
Two-pronged fix preventing duplicate verifikationer when a customer
invoice is marked paid OR a 19xx→1510 categorization is applied to an
inbound bank tx that already belongs to an open invoice.
Prong A (mark-paid): before booking, scan unlinked positive business
bank txs from the same customer within ±2% / ±60 days. If candidates
exist, return 409 INVOICE_PAID_LIKELY_DUPLICATE with per-candidate
match_reason (ocr_exact > name_amount_fuzzy > amount_only). Override
via `{ force: true }`. Applied to both legacy /api/invoices/[id]/
mark-paid and v1 /api/v1/.../invoices/[id]/mark-paid; v1 guard runs
before dry-run so previews can't mask the warning.
Prong B (categorize): when the user assigns 1930→1510 directly on a
positive business tx with a matching open customer invoice (by name
OR by OCR-normalized reference), return 409
TX_CATEGORIZE_SUGGEST_CI_MATCH routing them to /match-invoice.
Mirrors the supplier-side guard from #461. Shared helpers
(DUPLICATE_AMOUNT_TOLERANCE_PCT, escapeLikePattern) reused as-is.
New helper normalizeOcrReference() strips non-digits for Swedish OCR
equality. New shared candidate-finder
lib/invoices/duplicate-payment-candidates.ts keeps the legacy and v1
routes calling the same code.
Frontend:
- PaymentBookingDialog intercepts the 409, renders candidate list
with match_reason badges (Exakt OCR-träff / Sannolik träff /
Möjlig träff), offers "Länka transaktion" or "Bokför ändå"
(force-retry generates a fresh Idempotency-Key for v1 callers)
- transactions/page.tsx mirrors siMatchSuggestion handling as
ciMatchSuggestion with a parallel "Matcha mot kundfaktura?" dialog
v1 caveat documented in the route's pitfalls block:
INVOICE_PAID_LIKELY_DUPLICATE force-retry requires a fresh
Idempotency-Key because the original is body-hash bound; reusing it
returns 400 IDEMPOTENCY_KEY_REUSE.
Tests: 5 new mark-paid tests (legacy + v1) covering 409, force
bypass, partial-payment skip, ocr_exact match_reason, multi-candidate
ranking. 1 v1-only test verifying dry-run also surfaces the 409. 2
categorize Prong B tests (409 + confirm_no_match bypass).
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* fix(invoices): address compliance-swarm review on duplicate-payment guard
Three review-driven fixes:
1. **PostgREST .or() injection (OWASP V1.2.5).** `escapeLikePattern` neutralises
LIKE wildcards but NOT PostgREST filter-DSL chars (`,`, `.`, `(`, `)`). A
customer name like `Acme,fake.eq.true` could otherwise inject a synthetic
filter clause into the `.or('merchant_name.ilike.%X%,description.ilike.%X%')`
string. Replaced with two parameterised `.ilike()` queries dispatched in
parallel and merged by id in JS. Slight perf cost (two index hits per call),
eliminates the DSL-injection surface entirely.
2. **Date window anchored on invoice_date instead of due_date
(swedish-accounting-compliance bot).** The Prong B categorize intercept
filtered open customer invoices by `invoice_date ± 60d` relative to the
bank-tx date. For invoices with 60–90 day payment terms, the actual
payment lands well after `invoice_date`, so the legitimate match falls
outside the window and the guard silently misses it. Switched to
`due_date ± 60d` — the better proxy for "around when payment is expected."
No corresponding change for Prong A (mark-paid), which is correctly
anchored on `paymentDate` (the user-supplied or default-today date) and
scans bank-tx dates around that anchor.
3. **Force-bypass log enrichment (ISO A.8.15, OWASP V16).** Both
`duplicate-payment guard bypassed` warn entries now include `userId` and
`paymentAmount`. Attribution was previously incomplete — the bypass log
carried only `invoiceId`, which forced a join in log aggregation to
identify the acting principal.
Tests updated for the two-query pattern (legacy mark-paid suite enqueues
two transactions-table responses per guard invocation; v1 tests already
worked with the single-entry-per-table mock semantics).
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* chore(docs): redirect /docs/api and /llms-full.txt to docs.gnubok.se
Canonical docs host is now docs.gnubok.se. Every `docs_url` field on the
v1 error envelope still points at /docs/api/* on this app; the 308
permanent redirect forwards humans and agent crawlers to the docs
subdomain without us needing to mass-update structured-errors.ts.
/llms-full.txt also routes through the docs host where it's served from
the docs site's own build.
---------
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
||
|
|
b94ed3bec2 |
feat(api): cookbooks + webhook audit_log + secret rotation (PR-500 carry-overs) (#501)
* docs(api): ship 4 cookbook recipes (close docs polish backlog) Promotes the four placeholder cookbook entries to full narrative recipes matching the Stripe-grade quality bar set by quickstart + webhooks. Closes the docs follow-up bucket from the PR-500 description's deferred list. Recipes: - ingest-bank-transactions: bank-file upload (CSV / CAMT.053 auto-detect) → async poll → list uncategorised → suggest-categories → categorize (single + batch) → match-invoice / match-supplier-invoice. Multicurrency notes covering Riksbanken FX lookup and the kontantmetoden partial- payment guard. - file-vat-declaration: GET /reports/vat-declaration → rutor 05–62 walkthrough → GL reconciliation block → 2026-04-01 livsmedel 12% → 6% transition explicitly covered (delivery_date supply-date rule) → voucher- gap pre-flight → period lock workflow → manual Skatteverket Mina Sidor submission with confirmation-reference capture → EU / reverse-charge / import handling. - run-payroll-and-agi: draft → calculate → approve → mark-paid → book → generate-agi state machine. Per-step idempotency, strict-mode book failure semantics, förmånsbeskattning + bilförmån + bruttolöneavdrag vs nettolöneavdrag ordering. AGI XML download for manual Mina Sidor upload (direct API submission requires BankID via the Skatteverket extension, not the public REST surface). - year-end-closing: IB/UB continuity check per BFL 5 kap → voucher-gap pre-flight → missing-documents pre-flight → lock (reversible) → year- end async operation (resultatdisposition + periodiseringsfond + överavskrivningar + bolagsskatt + opening-balance batch) → close (irreversible per BFL 5 kap 8 §, typed-phrase confirmation) → årsredovisning + INK2/NE generation. Brutet räkenskapsår variant documented. Each cookbook follows the same shape as the existing quickstart and webhooks recipes — concrete curl commands, response samples, common pitfalls, next-steps cross-links. Lengths are deliberately uneven: the year-end recipe is longest because the consequences of getting it wrong are most severe (BFL violations, irreversible close). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * feat(api): V16 audit_log entries for webhook lifecycle + secret rotation endpoint Two intertwined changes that together close the "real audit attribution gap in actively-used routes" item from the PR description. 1. POST /api/v1/companies/{companyId}/webhooks/{id}/rotate-secret New endpoint that issues a fresh HMAC signing secret and invalidates the previous one immediately. Returns the new secret EXACTLY ONCE in the response, mirroring the create-time contract. Required scope: webhooks:manage. Idempotency-Key mandatory. Rotation is instant — no grace period. Documented workflow: stage the new secret on the receiver side (separate config slot, not yet active) → POST /rotate-secret → activate the new secret on the receiver → POST /webhooks/{id}/test to verify. A "previous_secret" column with TTL-based grace window (Stripe-style) is the natural follow-up; the instant-rotation shape ships first because it closes the "secret leaked, need to rotate now" use case with minimum new surface. The route is wired into load-routes.ts and lib/auth/scopes.ts. Spec snapshot updated. 2. V16 audit_log entries on every webhook lifecycle mutation The audit_log column shape (user_id, company_id, action, table_name, record_id, actor_id, old_state, new_state, description) is exactly what V16 / Art.32(1)(b) / A.8.24 audit-trail requirements call for. Wired entries on: - POST /webhooks (create) — action INSERT, new_state captures the row WITHOUT the secret (signing material must not land in the audit trail; only secret-event metadata). - PATCH /webhooks/:id (update) — action UPDATE, before/after pair so reviewers can reconstruct exactly what changed. - DELETE /webhooks/:id (delete) — action DELETE, old_state snapshot so the row's prior state survives the delete. - POST /webhooks/:id/rotate-secret — action SECURITY_EVENT, new_state carries the event marker only (no secret value). - dispatcher.disableWebhook (auto-disable on HTTP 410 / redirect / url_unsafe) — action SECURITY_EVENT, before/after capturing the disable cause for SIEM correlation. actor_id is set to ctx.apiKeyId on caller-driven entries so the audit row points back to the specific API key that triggered the change (PR-500 round-1 CC6.3 finding: actor attribution via created_by_api_key_id alone leaves a gap if a key is deleted — keeping the actor_id in audit_log closes that). 4 new integration tests cover the rotate-secret happy path, 404, 401 unauthorized, and Idempotency-Key required. The existing webhook integration tests continue to pass because the audit_log inserts fall through to the default mock response (no-op) without disturbing the per-table queues. 39 integration tests pass on the webhook surface (+4 vs round-2). Total: 3588 unit tests passing. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * refactor(api): address PR-501 review round 1 — correctness + Swedish compliance Round 1 of review fixes. Two real correctness bugs Greptile caught, two audit-trail gaps, and four Swedish-compliance errors in the cookbook prose. Compliance Swarm has 17 findings (0 blocking); the 4 architectural items (secret-at-rest encryption, dedicated rotate scope, rate-limit on rotation, URL redaction) remain deferred with rationale. Greptile (3 / 3 — all addressed): 1. rotate-secret silent 0-row UPDATE — fixed by adding `.select('id').maybeSingle()` to the UPDATE and returning NOT_FOUND when no row was touched. Closes the TOCTOU window between the existence check and the secret update; a concurrent DELETE no longer hands the caller a freshly-generated secret that no webhook in the database matches. 2. DELETE handler audit_log silently skipped when prior snapshot is null — fixed by writing the audit row UNCONDITIONALLY with `old_state: prior ?? null` and a degraded description when the snapshot is unavailable. A successful DELETE now always produces exactly one audit row (CC6.3 attribution contract). 3. Typo "bookslut" → "bokslut" in year-end-closing.ts. Compliance Swarm code-quality items addressed: 4. PATCH new_state now derived from the DB-confirmed returned `data` with an explicit field allowlist, not from the request-body-derived `update` object (A.8.11 / V16.1.1). Closes the gap where a future trigger that rejects a field would leave the audit trail out of sync with the actual stored state. 5. All four route-side audit_log inserts (create, update, delete, rotate-secret) now capture the insert error and emit a structured warning via ctx.log; mirrors the dispatcher pattern (CC7.2). 6. Dispatcher null-user_id path now emits a structured warning instead of silently skipping the audit_log entry — SIEM can alert on the gap (CC7.2 / V16.1.1 / A.8.15). Swedish compliance (cookbook content fixes — all real errors): 7. VAT cookbook ruta 06 label corrected: "Övrig försäljning (ej skattepliktig)" → "Momspliktig försäljning som inte ingår i ruta 05" (Skatteverket's verbatim label). The old label conflated exempt vs zero-rated supplies and would cause integrators to omit export / EU zero-rated sales from box 06. 8. Livsmedel rate-change framing rewritten: leads with the supply-date rule (ML 1 kap 3 §) as the decisive date, not invoice_date. The old opening sentence ("invoices created with invoice_date >= 2026-04-01 book to 2631") was wrong on its face — a copy-paste reader would mis-book pre-cutover deliveries invoiced in April at the new 6% rate. 9. Reverse-charge EU 2645 note adds the blandad-verksamhet caveat: "Net zero impact on cash flow" only holds when full avdragsrätt applies; partial avdragsrätt requires proportional restriction per HFD 2023 ref. 45. 10. Payroll cookbook age bounds corrected: "under-25 / over-66" → "18-22 years old (born 2003-2007) / 67+ from 2026", per Prop. 2025/26:66. The old bounds would cause integrators to apply the reduced rate (20.81%) to 23-24-year-olds who must pay 31.42%, producing non-compliant AGI files. 11. Payroll cookbook BAS 2615 corrected to 2731 (Avräkning sociala avgifter). 2615 is "Utgående moms vid import" in BAS 2026 — using it for the payroll liability would misclassify a payroll payable as an import-VAT payable and break moms reconciliation. 12. Year-end cookbook periodiseringsfond cap base corrected: IL 30 kap 5 § cap is on taxable profit BEFORE the periodiseringsfond deduction itself (and after schablonintäkt is added back). Note on materiellt samband (BFNAR 2016:10 kap 13) added — the reservation is BOOKED on 2110-2139, not declaration-only. Deferred to follow-ups (architectural / out of scope for round 1): - Secret-at-rest encryption (CC6.1 / Art.5(1)(f)): PR-1 architectural carryover, applies to existing webhooks.secret column too. - Dedicated `webhooks:rotate` scope (CC6.3 informational): introduces friction without closing a real gap when the only caller-driven action gated by `webhooks:manage` is the rotation itself. - Per-route rate-limit on :rotate-secret (Art.32 abuse case): part of the wider per-route rate-limit pass already on the deferred list. - webhook_url redaction in audit_log (Art.5(1)(c)): URLs are admin- supplied configuration values with no expected sensitive params; truncation would degrade audit value for legitimate review. 23 webhook integration tests pass locally (no regressions). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * refactor(api): address PR-501 review round 2 — atomic mutations + audit completeness + cookbook compliance Round 2 of review fixes. Compliance Swarm flagged refinements to the round-1 fixes; Swedish-compliance had a fresh batch of cookbook items (including a self-contradiction in payroll pitfalls I missed last round). All addressed. Code changes — atomicity + audit completeness: 1. rotate-secret collapsed to a single UPDATE … RETURNING (V8.2.1). The preflight existence-check SELECT was redundant after round 1 added .select().maybeSingle() on the UPDATE — the same null-row signal indicates non-existence, but in one round trip with no TOCTOU window. RETURNING `name` so the audit_log description still carries a human identifier without a second read. 2. DELETE handler collapsed to atomic .delete().select().maybeSingle() (V8.2.1). Eliminates the pre-read TOCTOU window entirely. A 0-row delete (already-deleted webhook) still returns 204 — idempotent DELETE — and the audit entry captures the attempt with old_state: null. Description discriminates the two cases ("deleted: name" vs "delete attempted on missing id"). 3. Cache-Control: no-store, no-cache, must-revalidate, private on the rotate-secret response (Art.25). The HMAC secret is sensitive credential material returned exactly once; this header prevents any intermediary (CDN, proxy, gateway access log, browser cache) from persisting the response body in a store with a different retention policy than intended. 4. Dispatcher auto-disable now writes the audit_log entry UNCONDITIONALLY (A.8.15 / V16.1.1 / CC7.2). Previously a null prior snapshot or a legacy null user_id caused the audit row to be silently skipped — only a warn log was emitted. Now writes user_id=NULL when unavailable (post-multi-tenant-refactor schema allows it; row is invisible under user RLS but queryable under service-role review, which is correct for system-initiated SECURITY_EVENT records). Description discriminates the snapshot- available / snapshot-unavailable cases. Swedish compliance — cookbook content fixes (all real errors): 5. VAT cookbook rounding rule corrected: SFL 22 kap 1 § mandates TRUNCATION of öre (Math.floor for positive amounts), not half-up rounding. Last round mislabeled this as "Math.round (half-up)"; the SRU filing skill is canonical and uses truncation. Using Math.round would produce values that differ from Skatteverket's expectations and cause GL-reconciliation mismatches at the öre level. 6. VAT reconciliation block now includes 2614 (Utgående moms vid omvänd skattskyldighet, matches ruta 30). The previous list of 2611/2621/2631/2641/2645 omitted 2614; a reconciliation that skips it would show rutor_match_gl: true even when the 2614 balance is non-zero and un-reconciled. 7. Livsmedel rate-change adds a one-sentence caveat for continuous/ subscription supplies — the supply-date framing in round 1 was too tight for cases where multiple deliveries roll up into a subscription. Confirms against ML 1 kap 3 § rather than assuming a single delivery date is decisive. 8. Payroll pitfalls bullet contradicted step 2 — "Employees under 26 (2024 rule for 2026 birth year ≥ 2001)" rewritten to match step 2: "18–22 years old at the start of 2026 (born 2003–2007) AND 67+ from 2026". An integrator reading only the pitfalls section would have applied the reduced rate too broadly, producing underpaid arbetsgivaravgifter and a non-compliant AGI. 9. Year-end periodiseringsfond cap now states schablonintäkt explicitly: 1.94% × outstanding prior-year balance (SLR + 1% for 2026) is ADDED to taxable income before the 25% cap is computed. Last round mentioned the "BEFORE the periodiseringsfond deduction" ordering but elided the schablonintäkt step; omitting it produces a cap that's too low when prior-year reserves exist. 10. Year-end SRU format characterization corrected: SRU is plain text encoded in ISO 8859-1, NOT XML. iXBRL (XML-based) is the Bolagsverket digital annual-report format — a separate artefact for a separate authority. Round 1 conflated them. Deferred (architectural / out of scope, documented in commit): - Audit-log dead-letter queue / SIEM alert escalation (Art.32 / A.8.15): infra setup, not code-PR scope. The warn-on-failure path is the in-process surface; durable delivery is a SRE/SIEM concern. - Secret encryption at rest (CC6.1): PR-1 architectural carryover. - webhook_url + description redaction in audit_log (Art.5(1)(c)): URLs are admin-supplied configuration values; redaction would degrade audit reconstructibility without closing a real PII gap. - PATCH old_state TOCTOU via Postgres function (CC6.3): the read- then-write pattern produces an append-only audit row capturing the read state; the small race window is non-load-bearing for audit purposes and a stored-procedure refactor exceeds the cost/value. 23 webhook integration tests pass locally (no regressions). Type-check clean for all changed files. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * refactor(api): address PR-501 review round 3 — real cookbook tax errors + cache-control on create Round 3 closes two tax-impact errors in the cookbooks plus the consistency gap on the create response. Compliance Swarm's remaining findings are recurring architectural carryovers or oscillation against prior rounds. Real cookbook errors (would mislead integrators): 1. Schablonintäkt rate corrected. Round 2 hardcoded 1.94% — that's the 2024 rate (SLR 0.94% + 1%). For 2026 SLR is 2.55%, so the rate is 3.55%. A wrong rate produces a too-low add-back, a too-high periodiseringsfond cap, and an IL 30 kap compliance error for any integrator copying the cookbook number. Rewrite to describe the formula (SLR + 1%, where SLR is the Riksbank statslåneränta on 30 Nov of the preceding year) with the 2026 figure as an example, and note the engine reads the canonical rate from `tax_rates`. 2. SRU format is a TWO-file pair, not one. Round 2 correctly said "plain text encoded in ISO 8859-1 (NOT XML)" but described it as a single file. Skatteverket requires both INFO.SRU (metadata header) AND BLANKETTER.SRU (declaration body) uploaded together — a single-file upload is rejected by their validation. Fix the prose to describe the two-file pair explicitly. Code consistency: 3. POST /webhooks (create) now returns the same `Cache-Control: no-store, no-cache, must-revalidate, private` + `Pragma: no-cache` headers as the rotate-secret endpoint (A.8.12). Both endpoints return the HMAC secret exactly once; both need the same intermediary-cache prevention. Smaller cookbook refinements (round 3 bot follow-ups): 4. VAT reconciliation block now includes 2615 (Utgående moms vid import, matches ruta 60) — the previous list covered 2611-2645 but omitted import VAT. A reconciliation that skips 2615 would show rutor_match_gl: true falsely for any importer. 5. Service supply-date fallback statement qualified to "one-off service supplies where delivery and invoice coincide" — long- running service contracts (subscriptions, maintenance) have per-delprestation skattskyldighet and need an explicit delivery_date per billing cycle. 6. Payroll elder-reduction boundary clarified: "67 years or older AT THE START OF the income year (1 January 2026)" — a 66-year- old whose 67th birthday falls in February does NOT qualify in 2026. Prevents misreading the pithy "67+ from 2026" as a birthday-during-year rule. Bot oscillation (skipping with rationale documented here for posterity): - Compliance Swarm Art.25 now asks to REMOVE webhook_url from DELETE old_state — direct contradiction with CC6.3's round-1 ask for complete attribution. webhook_url is admin-supplied configuration, not PII; keeping it preserves audit reconstructibility. - Swedish-compliance flags the unconditional re-delete audit row as "polluting" the behandlingshistorik — direct contradiction with Compliance Swarm V8.2.1 + CC6.3 round-1 / round-2 asks for unconditional writes. The audit_log is operational, not BFL räkenskapsinformation (which lives on journal_entries and related tables under explicit immutability triggers). Audit trail completeness wins over BFL purity for this table. Architectural carryovers (already documented in earlier commit bodies as deferred to follow-up PRs): - Secret encryption at rest (CC6.1, recurring) - Audit-log dead-letter / SIEM alerting (Art.32 / A.8.15, infra) - webhook_url userinfo stripping (A.8.11 low — URLs are admin- configured, no expected credentials; validating at registration would be a registration-time concern, not audit-time) 23 webhook integration tests pass. Type-check clean. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com> |
||
|
|
afb21ea638 |
feat(api): Phase 6 PR-3 — substrate hardening (SKIP LOCKED + DNS pinning + test debt) (#500)
* feat(api): operations table immutability trigger BFNAR 2013:2 kap 8 § behandlingshistorik integrity: once an operations row is in a terminal status (succeeded / failed / cancelled) the audit record of what happened becomes immutable. Adds the BEFORE UPDATE and BEFORE DELETE triggers that the webhook_deliveries table already has (20260515170000 / 20260515190000), mirroring their predicate shape and error code exactly. Closes the Phase 4 PR-2 (PR #469) review-round carry-over flagged by Swedish-compliance: previously a future bug, a privileged operator, or a compromised service-role caller could rewrite "this year-end close succeeded" to "failed" by updating an already-terminal row. The running → succeeded/failed/cancelled transition itself stays legal because the trigger keys on OLD.status, which is non-terminal at the moment of the legitimate UPDATE. pg test covers all transitions (allowed and blocked) plus DELETE on both terminal and non-terminal rows. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * feat(api): atomic SKIP LOCKED claim for webhook dispatch Replaces the SELECT-then-UPDATE-intersect pattern in the dispatcher with a single-roundtrip SQL function using FOR UPDATE SKIP LOCKED. PostgREST can't express SKIP LOCKED through the JS client, so the previous shape relied on a CAS guard inside an UPDATE WHERE status IN ('pending','failed') to ensure only one of two overlapping cron ticks claimed any given row. The CAS pattern was correct (under load — receivers >60s could push a batch past the next minute's tick) but burned two round trips and forced the application to negotiate the locking semantics in JS. The function form moves the contention to the DB, where SKIP LOCKED makes a row held by a concurrent tick simply invisible to the second caller. One round trip, no JS-side intersect. All filter semantics are preserved verbatim inside the function: status IN ('pending','failed'), next_attempt_at <= now, webhook_id IS NOT NULL, ORDER BY next_attempt_at ASC, LIMIT batchSize. p_batch_size is bounded (0, 1000] to forestall a runaway lock-set in case a caller misconfigures it. pg test covers basic claim (pending + failed), future-due skip, dangling- row (webhook_id IS NULL) skip, terminal-status skip, batch-size limits, out-of-range argument rejection, and the SKIP LOCKED invariant itself using two concurrent pool clients in BEGIN — the second caller does not see the row A locked, no double-delivery. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * feat(api): pinned-IP HTTPS dispatch (close DNS-rebinding window) The url-guard.ts file header openly flagged the remaining gap: "a separate DNS-rebinding window (between dispatch-time validation and the actual fetch) remains; closing that requires a custom HTTPS agent that pins the resolved IP — tracked for follow-up." This closes it. The previous shape was: 1. validateWebhookUrl() → DNS resolves to [public IP], returns ok 2. fetch(webhook_url) → re-resolves DNS; an attacker who flipped the A record in the interval gets a private-IP socket The new pinnedHttpsFetch helper validates DNS once, then opens a node:https.request to that pinned IP — but keeps the original hostname in the TLS SNI extension (so the receiver's cert validates) and in the HTTP Host header (so vhost routing still works). The request socket never re-resolves DNS, foreclosing the rebind race entirely. Built on node:https.request rather than undici's Agent so the project doesn't take on a new dep — the stdlib API is also more explicit about the SNI / Host / pinned-IP split. Test seam injects both validateUrl and httpsRequest so the unit tests verify the pinning shape without standing up an HTTPS server. The dispatcher's attemptDelivery is rewritten as a switch over the four PinnedFetchResult kinds (ok / unsafe_url / redirect_blocked / timeout / transport_error). The previous fetch-based code path that distinguished redirect rejection by string-matching err.message is gone — the new result type makes the distinction structural. 8 unit tests cover the SNI/Host/pinned-IP shape, port handling, redirect_blocked, transport_error, timeout, response-body truncation, first-IP determinism, and the validation short-circuit (never opens a socket when the URL fails the SSRF guard). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * test(api): pg tests for webhook substrate triggers (PR-1 test debt) CLAUDE.md ("Testing" + "Migration Rules") mandates a *.pg.test.ts for any PR touching a trigger / RPC / RLS / DEFERRABLE constraint. Phase 6 PR-1 (#496) shipped three webhook_deliveries triggers without the accompanying pg test; this closes that debt. Triggers covered: - enforce_webhook_delivery_immutability (BEFORE UPDATE) - block_webhook_delivery_terminal_delete (BEFORE DELETE) - assert_webhook_delivery_company_match (BEFORE INSERT) 13 cases verify the lifecycle the dispatcher depends on remains mutable (pending → in_flight, in_flight → failed, failed → in_flight, in_flight → delivered) while terminal-status rows (delivered / dead) are write- locked and the cross-tenant INSERT path is refused with the ERRCODE=check_violation contract documented in the migration. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * test(api): integration tests for webhook routes (PR-1 test debt) CLAUDE.md mandates integration tests under app/api/v1/ for every route. Phase 6 PR-1 (#496) shipped the eight v1 webhook routes (five under /companies/{companyId}/webhooks/ + the cross-tenant /webhook-deliveries/ {id}/retry) without them; closes that debt. 19 cases for the /webhooks/ verticals: POST /webhooks create + secret-once + payroll-scope gate + SSRF GET /webhooks list (no secret) + empty list GET /webhooks/:id detail (no secret) + 404 PATCH /webhooks/:id update + active=true re-enable + SSRF re-check + empty-body DELETE /webhooks/:id 204 hard delete POST /webhooks/:id/test enqueue + 404 + disabled-rejection GET /webhooks/:id/deliveries happy path + ownership 404 7 cases for the retry route: POST /webhook-deliveries/:id/retry dead → fresh pending row, live-status refusal, cross-tenant 404, disabled-webhook gate, SSRF re-check, delivery 404, webhook-gone 404 Both files mirror the suppliers/customers integration test pattern: Proxy-backed Supabase mock with per-table queues, validateApiKey + validateWebhookUrl stubbed to control auth and DNS deterministically. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * refactor(api): address PR-500 review round 1 — pg-real CI fix + 4 review items 1. pg-real CI was red on this PR: the new webhook trigger pg.test.ts and claim-due-webhook-deliveries pg.test.ts fixtures tried to INSERT into `webhooks.user_id`, which doesn't exist in the migration history. The column was never declared in automation_webhooks (20260415000000) nor added by webhooks_v2 (20260515170000) — so a fresh schema replay had no such column. The webhook create route (`webhooks.create`) was also referencing this non-existent column in its INSERT, so the production route was latent-broken since PR-1 and never exercised against a fresh DB. Drop the `user_id` field from both the route INSERT and the pg fixtures. Actor attribution lives on `created_by_api_key_id` (which leads back to the owning user via `api_keys.user_id`). 2. Greptile P2 #1 — `recoverStuckInFlight` carried a redundant `.not('status','in','(delivered,dead)')` filter alongside `.eq('status','in_flight')`, with a comment that incorrectly described PostgreSQL's UPDATE re-evaluation semantics. Under READ COMMITTED, UPDATE re-evaluates WHERE against each row's CURRENT value when it acquires the row lock — a row that raced to terminal status will fail `status='in_flight'` on re-evaluation and be skipped, no immutability trigger fires. Drop the redundant filter and rewrite the comment. 3. Greptile P2 #2 — added explicit pg test verifying `in_flight` rows are skipped by `claim_due_webhook_deliveries`. The status filter is what prevents double-delivery and is the entire point of the SKIP LOCKED substrate; making that invariant load-bearing in the test suite forecloses a future filter expansion silently regressing it. 4. Greptile P2 #3 — pinned-fetch registered both `res.on('end', finalize)` and `res.on('close', finalize)`. Node fires BOTH on normal completions, so finalize ran twice; the outer `settled` guard squashed the double-resolve but the header reconstruction still ran twice. Switch to `once` + self-removing pair so finalize runs exactly once on whichever event fires first (normal: end; truncation: close). 5. Compliance Swarm V8.2.1 — the retry route only checked `webhooks:manage` even when retrying `salary_run.* / agi.*` deliveries. Mirror the create-route elevated-scope gate so a key with only `webhooks:manage` cannot re-emit payroll payloads carrying personnummer / lönesummor / skatteavdrag. New integration test verifies the gate returns 403 INSUFFICIENT_SCOPE with `required_scope: payroll:read`. 35 tests pass locally (+1 vs pre-fix). Type-check clean. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * refactor(api): address PR-500 review round 2 — 2 small precision fixes 1. Compliance Swarm Art.32 / A.8.24 — response_body size cap was enforced only at the application layer (pinnedHttpsFetch's maxResponseBytes=4096 constant). A future refactor that bypassed the truncation, or a non- dispatcher write path into webhook_deliveries.response_body, would silently land large blobs in a column adjacent to event payloads carrying personal data. Add a CHECK constraint at the DB layer with a generous ceiling (8 KB — double the application cap so legitimate dispatcher writes never hit it; only a regression surfaces as a check_violation). 2. Compliance Swarm CC6.6 — pinned-fetch substitutes the validated IP for `host` while keeping the original hostname in `servername`. A reader could reasonably worry that the IP substitution weakens TLS hostname verification. Document explicitly that Node's default `checkServerIdentity` matches the cert's SAN/CN against `servername` (not `host`), so a forged endpoint at the pinned IP with a valid cert for a different hostname would fail the handshake. No code change — the default behavior is correct; the comment forecloses future "this looks dangerous" review-round noise on the same line. Items NOT addressed (with rationale documented elsewhere): - Compliance Swarm V8.2.1 (retry route 404-vs-404 information leak): delivery IDs are UUIDs; the "leak" is the ability to probe existence of an opaque 128-bit identifier the caller already has, which is not meaningfully different from probing for any opaque token. Both branches return the same structured 404 envelope. - Compliance Swarm CC7.2 (restore the .not() defense-in-depth filter): direct contradiction of last round's Greptile P2 fix. Greptile's PG-semantics analysis is correct — under READ COMMITTED, UPDATE re-evaluates WHERE against the row's current value when it acquires the lock, so .eq('status','in_flight') already handles the race. Adding a redundant .not() restores a misleading comment without closing a real gap. This is the documented Compliance Swarm oscillation pattern from the project's Phase 4 lessons. - Compliance Swarm CC6.1 (webhook secret encryption-at-rest): architectural choice from PR-1; not in PR-3 (substrate hardening) scope. Belongs to a future hardening PR. - Swedish-compliance review (operations queued/running rows hard- deletable): deliberate operability tradeoff — operators need to clear stuck/queued entries that crashed mid-flight. Blocking all deletes would force a manual DB intervention every time a worker crashed before reaching terminal status. The audit trail starts at terminal-state mutation, which IS blocked. - Swedish-compliance review (salary_run.* / agi.* payload anonymisation after 7 years): already on the deferred-list as part of the 90-day TTL cleanup cron item from the PR description. Belongs to a retention-policy follow-up PR. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com> |
||
|
|
7175daee87 |
Bug/skv konto numbers (#498)
* feat(skattekonto): add overdue transactions handling and split logic * feat: enhance transaction handling and loading states - Update BalanceHero component to display last synced date and additional information about Skatteverket updates. - Refactor BookDirectlyDialog to simplify transaction linking logic and improve UI for transaction selection. - Revamp InvoiceInboxWorkspace layout for better responsiveness and user experience, including improved skeleton loading states. - Introduce new loading states for ExtensionWorkspace to match the live layout and improve user feedback during data fetching. - Implement exchange rate fetching in QuickReviewDialog, ensuring transactions are always processed in SEK with error handling for rate fetching. - Add structured error handling for unavailable exchange rates in the transaction API. * feat(skattekonto): add 'Skattekonto – saldo & transaktioner' scope and update authorization checks * feat: implement reverse charge handling in supplier invoice calculations and UI |
||
|
|
e9e0fd726f |
feat(api): Phase 6 PR-1 — webhooks substrate (delivery pipeline + routes) (#496)
* feat(api): Phase 6 PR-1 — webhooks substrate (delivery pipeline + routes) First half of the final API plan phase. Ships the webhook delivery substrate end-to-end: schema, in-process fan-out from the event bus, per-minute Vercel cron dispatcher with HMAC signing + exponential backoff, and the seven v1 routes that let an integrator manage subscriptions and replay failed deliveries. Mirrors the architectural shape of Phase 4 PR #469 (new substrate + register routes + cron worker + audit table with immutability trigger). Migration (supabase/migrations/20260515170000_webhooks_v2.sql): - Repurpose automation_webhooks → webhooks. Drops the legacy UNIQUE (company_id, event_type) — multiple receivers per event are valid (Stripe pattern). Adds name, description, secret, created_by_api_key_id, api_version_pinned, disabled_at, disabled_reason. Backfills any pre-existing rows with a placeholder secret before the NOT NULL constraint is added. - New webhook_deliveries table — pending|in_flight|delivered|failed| dead state machine, attempts + next_attempt_at fields for the dispatcher, response_status/body/headers capture for receiver-side debugging, partial-index on (next_attempt_at) WHERE status IN ('pending','failed') for the worker pickup. - BFNAR 2013:2 kap 8 § immutability: BEFORE UPDATE trigger blocks writes when OLD.status IN ('delivered','dead'). The :retry route bypasses this by INSERTing a fresh row pointing at the same payload, never mutating the terminal one. - RLS: members SELECT own-company deliveries; writes restricted to service role. lib/webhooks/{handler,dispatcher,signing,diff}.ts: - handler.ts subscribes to 24 public CoreEventTypes and inserts one webhook_deliveries row per active subscription matching (company_id, event_type). Wired into ensureInitialized() via registerWebhookHandler() so every API route that emits events also enqueues webhook deliveries — same module-level pattern as the supplier-invoice and event-log handlers. - dispatcher.ts is the per-minute cron worker. Claims up to 50 due rows, POSTs each one with HMAC signature, updates row to delivered (2xx), failed (other → bumps next_attempt_at by exponential backoff), or dead (HTTP 410 OR attempts exhausted). HTTP 410 additionally auto-disables the webhook. 10s request timeout, 4 KB response-body cap. Backoff: 1m / 5m / 30m / 2h / 12h / 24h / 48h (7 retries, ~72h total) — matches Stripe. - signing.ts: Stripe-style X-Gnubok-Signature: t=<unix>,v1=<hex> with HMAC-SHA256 over `${t}.${rawBody}`. Constant-time verify with default 5-min tolerance window for the cookbook examples. generateWebhookSecret() returns 256 bits of crypto-random hex. - diff.ts: computePreviousAttributes() for Stripe-style update events. Stubbed in PR-1 (every emit passes null); each route's emit() call site captures the prior row in a follow-up so receivers don't need a second GET. v1 routes (app/api/v1/...): - /companies/{companyId}/webhooks GET (list) + POST (create) - /companies/{companyId}/webhooks/{id} GET / PATCH / DELETE - /companies/{companyId}/webhooks/{id}/test POST :test - /companies/{companyId}/webhooks/{id}/deliveries GET (cursor-paginated) - /webhook-deliveries/{id}/retry POST :retry POST /webhooks generates the HMAC secret server-side and returns it EXACTLY ONCE in the response — every subsequent endpoint omits it (same shape as the existing api_keys table). Idempotency-Key required on POST; dry-run supported. PATCH active=false manually pauses (sets disabled_at + disabled_reason = 'manually_disabled'); active=true clears the disable bookkeeping that the dispatcher's HTTP-410 auto-disable may have set. event_type is immutable — delete and recreate to change. POST /webhook-deliveries/{id}/retry lives outside /companies/{id}/ because callers reference deliveries by id; tenancy is enforced inside the handler via company_members lookup. Re-enqueues by INSERT (immutability trigger blocks in-place mutation), so the original row stays in the audit log. /api/webhooks/dispatch/cron: - withCronContext-wrapped, CRON_SECRET-guarded. - Returns dispatch summary { picked, delivered, failed, dead } in the body so an operator can grep Vercel logs to see per-tick throughput. - Per-minute schedule added to vercel.json (* * * * *). lib/auth/scopes.ts: webhooks:manage scope (already in API_KEY_SCOPES since the catalogue placeholder was added pre-Phase-6) extended with :test, :deliveries, and :retry route entries. Substrate-only by design. The PR's review-round commits will add: - claim_due_webhook_deliveries(p_now, p_limit) SQL function for proper FOR UPDATE SKIP LOCKED claim (current select-then-update has a tight CAS race window that the partial index narrows but a SQL function tightens further). - Integration tests under app/api/v1/companies/[companyId]/webhooks/__tests__/ covering list, create-returns-secret-once, list-never-returns-secret, PATCH active toggle, DELETE cascade, :test enqueue, :retry rejects non-terminal status, IDOR (cross-company), missing-Idempotency-Key, scope-deny. - *.pg.test.ts for the immutability trigger (CLAUDE.md mandate for any PR touching a trigger / RLS policy). - 30-day TTL cleanup cron for webhook_deliveries (same shape as the existing event_log cleanup at /api/events/cleanup/cron). Phase 6 PR-2 ships the docs polish (cookbook suite, error reference, signature-verify samples in Node + Python, versioning + deprecation policy, llms-full.txt rebuild, spec-snapshot test). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * refactor(api): address PR-496 review round 1 — 4 real bugs + retention FK Fixes the 4 real bugs Greptile flagged on the round-1 review of the Phase 6 PR-1 webhooks substrate, plus the swedish-compliance-bot finding about 7-year audit retention on accounting-event delivery rows. Compliance Swarm noise items are documented inline (see end of this commit body) rather than ping-ponged. FIXED — real bugs: 1. **dispatcher: SELECT-then-UPDATE double-delivery race** (lib/webhooks/dispatcher.ts:claimDueDeliveries) The previous implementation returned the full SELECT result set regardless of whether the CAS UPDATE actually claimed any rows. Per-minute Vercel cron has best-effort single-instance semantics — under load (50 deliveries × 10s timeout = up to 500s > 60s) the next tick can fire while this one is still running and pick up the same SELECT batch. Both ticks would then dispatch the same deliveries. Fix: have the UPDATE return the IDs it actually claimed via `.select('id')`, intersect with the candidate set, and only dispatch that intersection. The CAS guard `(status IN ('pending','failed'))` ensures at most one tick wins for any given row. 2. **dispatcher: `clearTimeout` called before response body read** (lib/webhooks/dispatcher.ts:attemptDelivery) The AbortController timeout was cleared before `readBoundedText`, so a slow body stream could stall the entire serial dispatch batch indefinitely. Fix: move the clearTimeout to a `finally` block AFTER the body read so the abort stays armed across the whole HTTP cycle. 3. **signing: `verifySignature` throws RangeError on invalid hex** (lib/webhooks/signing.ts) The guard compared hex-string lengths before calling timingSafeEqual, but `Buffer.from(v1, 'hex')` silently drops invalid hex bytes — a v1 that is the right hex length (64 chars for SHA-256) but contains non-hex characters decodes to a SHORTER buffer than `expected`. timingSafeEqual then throws RangeError instead of returning false. Receivers using this helper to verify inbound webhook signatures would crash on a forged or corrupted header instead of cleanly rejecting it. Fix: compare buffer lengths AFTER decoding. 4. **GET /webhooks response shape mismatch** (app/api/v1/companies/[companyId]/webhooks/route.ts) The handler passed a flat array to `paginated()`, producing `data: [...]`, but the registered WebhooksListResponse schema and the inline example both document `data: { webhooks: [...] }`. Any client built against the spec would not find the expected key. Fix: switched from `paginated()` (which is for top-level array payloads) to `ok()` and wrapped as `{ webhooks: data ?? [] }` to match the schema. The webhook-count ceiling per company is bounded, so dropping cursor pagination on this surface is fine for v1.0. FIXED — swedish-compliance: 5. **Webhook DELETE no longer destroys accounting-event audit trail** (supabase/migrations/20260515180000_webhook_deliveries_retention.sql, app/api/v1/companies/[companyId]/webhooks/[id]/route.ts, lib/webhooks/dispatcher.ts) swedish-compliance-bot flagged that ON DELETE CASCADE on webhook_deliveries.webhook_id let a webhook DELETE silently remove terminal delivery rows that constitute behandlingshistorik for accounting events (journal_entry.committed, period.locked, salary_run.booked, agi.generated, ...). BFNAR 2013:2 kap 8 § requires 7-year retention of these rows. Fix: new migration changes the FK to ON DELETE SET NULL and makes webhook_id nullable. Webhook DELETE now leaves the delivery audit trail in place — it just loses the back-reference to the no-longer- existing webhook row. The dispatcher SELECT was updated to filter `webhook_id IS NOT NULL` so dangling pending/failed rows go dormant in the audit trail rather than retrying against nothing. Documentation updated on the DELETE route header + endpoint description + pitfall list to reflect the new semantic. FIXED — defense in depth: 6. **Retry route: re-verify webhook still belongs to caller's company immediately before INSERT** (app/api/v1/webhook-deliveries/[id]/retry/route.ts) Compliance Swarm V8.2.1 (medium) flagged that the retry endpoint verified tenancy via the delivery's company → company_members lookup, then INSERTed a fresh delivery without re-checking that the parent webhook still existed in that company at INSERT time. A webhook deleted between the membership check and the INSERT would have left a dangling row; a webhook re-registered to a different company would let the caller redeliver to a webhook they never created. Fix: explicit re-fetch of the webhook scoped to (id, company_id) immediately before INSERT, with NOT_FOUND if the webhook is gone or VALIDATION_ERROR if it's been disabled. DEFERRED — documented inline: - **OWASP V14.2 plaintext webhooks.secret**: Inline rationale added to lib/webhooks/signing.ts:generateWebhookSecret(). Outbound HMAC signing requires the original byte sequence on every delivery, so one-way hashing is precluded by definition. Stripe / GitHub / Slack / Twilio all follow the same pattern. Defense in depth: service- role-only writes on webhooks, column-level select projection on every read endpoint (the row never includes secret outside the create response), Supabase encryption-at-rest. Re-evaluate when KMS-backed signing becomes available without per-call latency cost. - **Compliance Swarm V13.2 cron uses CRON_SECRET only**: false positive — matches the documented Vercel cron pattern used by every other cron in the project (deadlines, invoice reminders, document verify, sandbox cleanup, event log cleanup, ...). - **Compliance Swarm V1.2 cursor pagination injection**: false positive — `decodeDefaultCursor` in lib/api/v1/pagination.ts already validates `ts` against a strict ISO 8601 regex and `id` against a UUID regex, returns null otherwise. The bot couldn't see the helper's internals. - **Compliance Swarm V8.2.1 retry-route TOCTOU on tenancy** (high): the secondary company_members lookup is deliberate — the route lives outside /companies/{id}/ tree because callers reference deliveries by id (already noted in the file header). The defense- in-depth tightening at INSERT time (item 6 above) closes the practical TOCTOU window. Round-2 may add an atomic DB function if swarm escalates this. - **Compliance Swarm V2.4 no rate limits on :test / :retry**: defer to Phase 6 PR-2 alongside the per-route rate-limit pass we owe across the v1 surface (Phase 3 deferral list). - **Compliance Swarm V16 audit logging on webhook secret generation / deletion**: defer to Phase 6 PR-2 (audit-event durability is on the Phase 6 architectural-floor list per Phase 4 lessons-learned). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * refactor(api): address PR-496 review round 2 — SSRF, tenancy, retention triggers Round 2 of the Phase 6 PR-1 review cycle. Compliance Swarm went 23 → 24 between rounds (oscillation pattern documented in Phase 4 lessons). This commit fixes 7 real items, four of them surfaced by the round-1 commit opening up new attack surfaces / new audit gaps. FIXED: 1. **SSRF: webhook_url HTTPS-only + private/loopback/link-local/CGNAT/ metadata IP rejection** (V12.1, V1.2, CC6.6) New helper lib/webhooks/url-guard.ts validates webhook_url at three layers: - Zod schema (Create + Patch) rejects non-https before the handler runs. - Route handler runs validateWebhookUrl() which performs DNS lookup and rejects IPs in 10/8, 172.16/12, 192.168/16, 127/8, 169.254/16 (link-local + AWS/GCP/Azure metadata 169.254.169.254 explicitly classified), 100.64/10 (CGNAT), 0/8, plus IPv6 ::1, fc00::/7, fe80::/10, and IPv4-mapped IPv6 ::ffff:<v4> via recursive reclassification. - Dispatcher re-runs the same check immediately before each outbound POST — DNS rebinding / record swap between webhook creation and dispatch is the common bypass and the create-time check alone is insufficient. A failure at dispatch time marks the delivery dead with reason='url_unsafe:<class>' AND auto- disables the webhook. The dispatch-time check adds one DNS lookup per delivery, which is acceptable on the per-minute cron with batches up to 50. 2. **Cross-tenant dispatch refusal** (A.8.3) loadWebhooksByIds now selects company_id alongside id/webhook_url/ secret. The dispatch loop asserts webhook.company_id === delivery.company_id BEFORE signing. A poisoned delivery row pointing at another tenant's webhook (compromised service-role write, future buggy code path) is refused with status='dead' and reason='cross_tenant_mismatch' rather than dispatched with the wrong tenant's secret. 3. **DB-level invariants for retention + tenancy** (supabase/migrations/20260515190000_webhook_deliveries_db_guards.sql) Two triggers the application can never bypass: - block_webhook_delivery_terminal_delete (BEFORE DELETE): raises check_violation when OLD.status IN ('delivered','dead'). Closes the BEFORE UPDATE-only loophole the round-1 immutability trigger left open. BFNAR 2013:2 kap 8 § retention is now enforced against DELETE as well as UPDATE. - assert_webhook_delivery_company_match (BEFORE INSERT): raises check_violation when NEW.company_id doesn't match the parent webhooks.company_id. Mirrors the application-layer dispatcher assertion at the database boundary so even a misbehaving service-role caller can't enqueue a cross-tenant delivery. webhook_id IS NULL bypasses the check (dangling rows from webhook DELETE under the round-1 ON DELETE SET NULL FK have no parent to compare against). 4. **Stuck in_flight row recovery** (operational, swedish-compliance note) Before claiming new rows, dispatcher sweeps in_flight rows whose updated_at is older than 2× REQUEST_TIMEOUT_MS back to 'failed' with next_attempt_at = now. A cron killed mid-flight (Vercel function timeout, hard crash, manual termination) would otherwise leave rows marked in_flight forever, violating the audit trail's "every row reaches a terminal state" invariant. 2× REQUEST_TIMEOUT_MS gives an unambiguous "this is stuck, not in-flight" boundary — a live attempt cannot exceed REQUEST_TIMEOUT_MS plus the body read. 5. **Response-body content-type filter + header allowlist** (CC7.2, A.8.12, Art.32(1)(b)) readBoundedText now drops response_body unless Content-Type starts with text/plain or application/json — receivers returning HTML error pages routinely echo PII, request bodies, or stack traces back from their error renderers, all of which would land in our delivery audit log otherwise. Bytes are still drained so the connection stays reusable. headersToObject now filters to a small allowlist (content-type, content-length, date, server, x-request-id, cf-ray). Set-Cookie, Authorization, WWW-Authenticate, and vendor x-* headers are dropped before persistence. 6. **Test payload data minimisation** (Art.25(2)) The :test event payload no longer includes api_key_id. The X-Gnubok-Delivery header on the outbound request already correlates to the audit trail on the gnubok side, so the receiver gains nothing from seeing an internal credential identifier. 7. **Silent-drop log promoted to error** (PI1.3) handler.ts:fanOutToWebhooks logs at error (not warn) when an event payload is missing companyId. Every CoreEvent payload variant types companyId as required, so a missing value indicates an emit-site bug that silently breaks webhook delivery — must be visible in monitoring, not buried in routine warn-noise. DEFERRED (remaining oscillation, documented in commit body): - **V14.2 / Art.5(1)(f) plaintext webhooks.secret**: documented inline in lib/webhooks/signing.ts as accepted-risk per Stripe / GitHub / Slack precedent. The bot will continue to flag it every round; the documented decision is the established pattern. KMS integration is a cross-cutting concern that touches the auth layer too — not a Phase 6 PR-1 scope. - **Art.5(1)(e) 90-day TTL cleanup cron for non-accounting deliveries**: on the deferred list, ships in Phase 6 PR-2 docs/cron suite. - **V2.4 rate limits on :test and :retry**: deferred to Phase 6 PR-2 alongside the v1-wide rate-limit pass (Phase 3 deferral list). - **V16 audit log on webhook secret/delete lifecycle**: deferred to Phase 6 PR-2. - **A.8.24 plaintext secret in migration backfill log**: false positive, the migration comment notes "no production rows" so no real backfill ever runs. Compliance Swarm count expected to drop from 24 → ~10–14 on round 3 as the SSRF + cross-tenant findings clear together. Architectural floor is the V14.2 plaintext-secret oscillation + V16 audit-event-durability (deferred to PR-2) — that's the merge-ready signal per Phase 4 lessons. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * refactor(api): address PR-496 review round 3 — 5 fixes + migration consolidation Compliance Swarm went 24 → 16 (5 high / 8 medium / 3 low) after round 2, clearing the SSRF + cross-tenant cluster as predicted. Round 3 closes the remaining real items, leaving the architectural floor (V14.2 plaintext, V16 audit log, V2.4 rate limits, Art.5(1)(e) TTL — all deferred to Phase 6 PR-2). That's the documented merge-ready signal. FIXED: 1. **Deliveries list — webhook ownership pre-check** (V8.2.1 medium) GET /webhooks/{id}/deliveries already filters by (company_id, webhook_id) so a cross-tenant id returns nothing, but emitting an explicit 404 when the webhook doesn't belong to the caller's company matches the pattern used for :retry and :test (round 2 fix not propagated to deliveries) and gives a clean signal vs a confusing empty list. Defense in depth alongside RLS. 2. **url-guard: enumerate ALL DNS records** (V1.2 medium) Replaced single dns.lookup with parallel dns.resolve4 + dns.resolve6. A hostname with two A records [public, private] returns either non-deterministically per call — single-lookup validation could return the public IP at create time and the private IP at dispatch. Multi-record enumeration rejects if ANY resolved address is unsafe. Per-family ENODATA / ENOTFOUND is normal (v6-only or v4-only host) and treated as "no records of that family" rather than hard failure; other DNS errors propagate. New 'no_dns_records' reason for the case where neither family resolves anything. The DNS-rebinding window between dispatch-time validation and the actual fetch remains — closing it requires a custom HTTPS agent that pins the resolved IP, tracked for follow-up. Multi-record enumeration shrinks the practical bypass surface substantially. 3. **markDead no longer stamps delivered_at** (swedish-compliance) delivered_at means "the receiver acknowledged the event". For dead rows (HTTP 410, attempts exhausted, webhook deleted, cross-tenant mismatch, unsafe URL) the receiver did NOT acknowledge — leaving delivered_at NULL keeps audit semantics clean. An auditor querying `WHERE delivered_at IS NOT NULL` correctly sees only genuinely delivered rows. The terminal-state timestamp lives on `updated_at` (auto-stamped by the table's BEFORE UPDATE trigger). 4. **Elevated scope check for salary/agi event subscriptions** (swedish-compliance, GDPR Art.32) Subscribing to salary_run.* or agi.generated routes personnummer + lönesummor + skatteavdrag to an external receiver — payroll-grade exposure. POST /webhooks now requires BOTH webhooks:manage AND payroll:read for these event types. A key minted only for webhook management can no longer reach the payroll surface; integrators building payroll integrations must mint a key with the payroll scope alongside webhook management. The check uses a regex (^salary_run\.|^agi\.) so future payroll event types automatically inherit the gate. Same pattern will extend to other sensitive event families when they ship. 5. **Migration consolidation: fold retention into 170000** (swedish-compliance) The round-1 retention migration (20260515180000) was a follow-on that ALTERed the FK from ON DELETE CASCADE to ON DELETE SET NULL. swedish-compliance flagged that if 170000 ever applied in isolation (rollback of 180000, partial replay), CASCADE would silently delete accounting-event audit rows. Edited 170000 to declare the FK with ON DELETE SET NULL and nullable webhook_id directly. Deleted 180000. Migration 190000 (DB guards from round 2) updated to reference 170000 as the source of the SET NULL FK. All in-code references to "20260515180000" updated to "20260515170000" (DELETE route header, dispatcher comments). Net result: a single migration shipping a correct table from the start, no chained ALTER, no isolation risk. DEFERRED (architectural floor, all bound for Phase 6 PR-2): - **V8.2.1 retry ctx.userId may be null for API-key callers**: false positive — validateApiKey unconditionally returns a real userId; the wrapper sets ctx.userId = auth.userId for every authenticated call. - **V1.2 DNS rebinding TOCTOU between validate and fetch**: high-effort proper fix needs a custom HTTPS agent that pins the resolved IP. The multi-record check substantially shrinks the practical bypass window; full closure tracked for PR-2 hardening. - **V16.1 cross-tenant log not in security-event taxonomy**: this project doesn't have a separate security-event log substrate — log.error with structured fields is the established pattern. - **V4.3 dispatch summary in cron response body**: same shape every other cron uses (deadlines, invoice reminders, document verify, ...). CRON_SECRET-gated; project pattern. - **V5.3 / Art.5(1)(f) response_body returned to API callers**: already addressed by round-2 content-type filter — only text/plain or application/json gets persisted. Residual oscillation; the bot didn't see the new filter. - **Art.5(1)(e) 90-day TTL non-accounting deliveries**: Phase 6 PR-2 cron suite. - **Art.32(1)(b) / V14.2 plaintext webhooks.secret**: established defer, documented inline in signing.ts (Stripe / GitHub / Slack precedent). - **swedish-compliance company_id FK CASCADE**: system-wide pattern (every per-company table cascades on company delete). Cross-cutting compliance decision, not webhook-specific. - **swedish-compliance period.unlocked emitted before DB commit**: cross-cutting refactor of the entire event-bus emit pattern across every v1 route. Project-wide concern, not Phase 6 PR-1 scope. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * refactor(api): address PR-496 review round 4 — 2 critical fixes + 5 hardening Two critical items + 5 supporting hardening fixes. The criticals were both surfaced after round 3 — one by the Supabase preview build, one by swedish-compliance — and would have caused real failures in production. CRITICAL: 1. **Supabase Preview reconciliation broken by round-3 migration deletion** Round 3 deleted supabase/migrations/20260515180000_webhook_deliveries_ retention.sql after folding its FK fix into 170000. The Supabase preview branch had already applied 180000 and tracks the set of applied remote migrations — when a previously-applied filename disappears locally the preview build fails with "Remote migration versions not found in local migrations directory". Fix: restored 180000 with the original idempotent ALTER content. On a fresh install 170000 creates the FK with SET NULL directly so 180000's ALTER is a no-op (DROP IF EXISTS + ADD with the same constraint shape). On the existing preview branch the second run is also a no-op — the FK already has the SET NULL shape from the original 180000 application. Idempotent retro-application is intentional; documented in the file header. 2. **`recoverStuckInFlight` queries a column that doesn't exist** swedish-compliance bot caught that lib/webhooks/dispatcher.ts: recoverStuckInFlight filters `.lt('updated_at', stuckBefore)` against webhook_deliveries.updated_at, but migration 170000 never declared the column. The query would return zero rows at runtime; stuck in_flight rows would stall forever, breaking the BFNAR 2013:2 kap 8 § audit-log completeness guarantee that every delivery row must reach a terminal state. Fix: new migration 20260515200000_webhook_deliveries_updated_at.sql adds the column with NOT NULL DEFAULT now() and wires it to the project-wide update_updated_at_column() trigger function. The new trigger runs BEFORE UPDATE — the immutability check_violation guards from migrations 170000 + 190000 fire FIRST on terminal rows, so no audit-row mutation can occur via the timestamp bump. HARDENING: 3. **Dispatcher: fetch redirect: 'error'** (V1.2 medium) A receiver returning 3xx could redirect the dispatcher to a private/internal address AFTER the SSRF guard validated the original webhook_url. Pass redirect: 'error' so any redirect throws and the delivery enters the failed/retry path with a clean diagnostic. Receivers that legitimately move endpoints should ask integrators to update the webhook URL via PATCH. 4. **Defensive ctx.companyId early-return** (V8.2.1 medium) The deliveries list route used `ctx.companyId!` non-null assertion. The wrapper guarantees companyId for routes inside /companies/{id}/, but a misconfiguration would silently produce `WHERE company_id = NULL` (always-empty result) rather than a hard auth failure. Added an explicit early INTERNAL_ERROR return when ctx.companyId is falsy. Drops the `!` everywhere in the file. 5. **Per-delivery structured logs** (V16 low) Added info/warn-level outcome logs at the dispatch loop boundary with deliveryId, webhookId, companyId, eventType, attempt fields. Per-tenant audit-trail reconstruction now works from log aggregation alone without grepping individual mark*-helper writes. Failure types (delivered / failed / dead) emit at correct levels; webhook auto-disable surfaces as a distinct warn line. 6. **Strip userId from outbound webhook payloads** (Art.5(1)(c)) New minimisePayload() in handler.ts drops the internal Supabase auth.users.id UUID before insert into webhook_deliveries. The companyId stays (it's the tenant scope, useful for multi-tenant receivers). Centralising the projection means future tightening (e.g. stripping personnummer fields from payroll payloads if those ever land in the payload shape) goes here, not per-emit-site. 7. **Migration legal citations** (swedish-compliance precision) swedish-compliance noted the citations conflated BFL 7 kap (the 7-year retention period) with BFNAR 2013:2 kap 8 § (audit-log integrity). Both apply but they're distinct grounds. Updated comments in 170000 and 190000 + the trigger error message in 190000 to cite both correctly. REMAINING DEFERS (architectural floor — Phase 6 PR-2 territory): - **V14 / Art.32 plaintext webhooks.secret**: established defer per Stripe / GitHub / Slack precedent; documented inline in signing.ts. - **V8.2.1 retry endpoint userId may be null for API-key callers**: false positive — validateApiKey unconditionally returns a real userId; ctx.userId is always set after auth. - **V1.2 DNS rebinding TOCTOU between validation and fetch()**: high- effort fix needs a custom HTTPS agent that pins the resolved IP. Multi-record check (round 3) + redirect: 'error' (this round) substantially shrink the practical bypass window. Full closure is Phase 6 PR-2 hardening. - **V2.3 dry-run rate limiting**: Phase 6 PR-2 with the v1-wide rate-limit pass. - **V16.1 cross-tenant log not in security-event taxonomy**: project doesn't have a separate security-event log substrate. - **Art.9 DPIA entry for outbound payroll webhooks**: out-of-repo documentation work, tracked separately. - **Art.5(1)(e) 90-day TTL non-accounting deliveries**: Phase 6 PR-2 cron suite. - **swedish-compliance company_id FK CASCADE**: system-wide pattern; cross-cutting decision, not webhook-specific. - **swedish-compliance period.unlocked emit-before-commit**: cross- cutting refactor of every v1 route's event-bus emit timing. Compliance Swarm count expected to drop materially as the V1.2 + V8.2.1 + V16 cluster clears. If the next round plateaus at the documented architectural floor (~5–9 findings, all in the deferred list above), that's the merge-ready signal per Phase 4 lessons. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * refactor(api): address PR-496 review round 5 — 5 small fixes (audit gaps + hardening) Round 5 closes the actionable items round 4 surfaced. Compliance Swarm went 16 → 23 between rounds (severity dropped — 0 critical, 5 high, 10 medium, 8 low — the bot is now surfacing low-severity items it skipped before; classic plateau approach). Round 5 fixes 3 real gaps + 2 documentation-precision items, all small. FIXED: 1. **`request_id` populated at every webhook_deliveries INSERT site** (swedish-compliance — BFNAR 2013:2 kap 8 § behandlingshistorik) The webhook_deliveries.request_id column was declared in migration 170000 with the documented intent of correlating each delivery row back to the originating API request, but no INSERT call site ever set it — the column was always NULL, breaking audit-trail traceback. - test/route.ts and retry/route.ts now stamp ctx.requestId. - handler.ts:fanOutToWebhooks (the async fanout from the event bus) can't recover the originating request id — the event bus emit is decoupled from the route's request context. Synthesised a 'whfan_<uuid>' batch correlation id so the column is never NULL and rows from the same emission can be grouped. Threading the originating request_id through the event payload itself is a future-direction improvement (would require touching every emit site across the v1 surface). 2. **Retry route re-runs minimisePayload before INSERT** (A.8.12 medium) The retry endpoint was inserting o.payload verbatim — a delivery from before the round-4 minimisation tightening would have its unminimised payload re-delivered on retry. minimisePayload exported from handler.ts; retry now applies it. Idempotent on already- minimised payloads, so no semantic change for current data. 3. **Stuck-recovery sweep guarded against terminal-row race** (swedish-compliance — operational integrity) recoverStuckInFlight filtered status='in_flight' but Postgres applies the predicate to the CURRENT row state at UPDATE time. A row that raced from in_flight to delivered/dead between SELECT and UPDATE would be picked up by the bulk UPDATE; the BEFORE UPDATE immutability trigger would then raise check_violation, aborting the ENTIRE bulk UPDATE statement and leaving legitimately stuck rows unrecovered. Added `.not('status', 'in', '(delivered,dead)')` as defense in depth. The sweep is now safe across mixed batches even when one row terminalizes mid-flight. 4. **'server' header dropped from response_headers allowlist** (A.8.12 low) Receiver infrastructure version strings (nginx/1.21.6, Apache/2.4.41, ...) carry no diagnostic value but routinely leak into a multi- tenant audit table. Removed from SAFE_RESPONSE_HEADERS. 5. **Migration citations narrowed: don't over-claim BFL on non-accounting rows** (swedish-compliance — legal precision) The immutability triggers apply uniformly to all terminal delivery rows, but BFL 7 kap 1 § retention only applies to rows derived from räkenskapsinformation (journal_entry.*, period.*, salary_run.booked, agi.generated, invoice.paid, supplier_invoice.paid). For non- accounting events (customer.created, document.uploaded, transaction.categorized, webhook.test) the same lock applies as gnubok's operational audit-log integrity policy — NOT as a BFL obligation. Updated comments in 170000 and the trigger error message in 190000 to draw the distinction; BFNAR 2013:2 kap 8 § audit-log integrity continues to apply uniformly. REMAINING DEFERS (architectural floor — Phase 6 PR-2): - V14 / Art.32 / V9.1 / A.8.24 / CC6.1 plaintext webhooks.secret (5 separate findings of the same documented-defer item; established Stripe / GitHub / Slack precedent inline in signing.ts). - V8.2.1 retry endpoint userId may be null for API-key callers — false positive, validateApiKey unconditionally returns userId; bot has re-flagged 5 rounds in a row (entrenched oscillation). - V1.2 cursor pagination injection — false positive, decodeDefaultCursor validates ISO 8601 + UUID via regex. - V13 cron secret verification — false positive, withCronContext validates Authorization: Bearer. - V1.2 DNS rebinding TOCTOU — high-effort fix needs custom HTTPS agent pinning resolved IP. Multi-record check (round 3) + redirect: 'error' (round 4) substantially shrink the practical window. Phase 6 PR-2. - V2.4 rate limits on :test / :retry — Phase 6 PR-2 v1-wide pass. - V16.1 / A.8.15 / A.8.16 / CC7.2 SIEM / log drain / monitoring — out-of-repo infra, tracked separately. - Art.5(1)(e) 90-day TTL non-accounting deliveries — Phase 6 PR-2. - Art.9 DPIA entry for outbound payroll webhooks — out-of-repo doc. - Art.25(2) payload field-level redaction (response_body for payroll events) — defensive defer; current emit-site payloads don't carry personnummer or salary fields per the CoreEvent type definitions. - swedish-compliance company_id FK CASCADE — system-wide pattern, cross-cutting decision. - swedish-compliance period.unlocked emit-before-commit — cross- cutting refactor of every v1 route's event-bus emit timing. - PI1.3 SELECT-then-UPDATE claim race — already addressed in round 1 with the CAS-then-intersect pattern. Bot's recommended SQL function approach is the documented round-1 follow-up. Compliance Swarm count expected to plateau in the 12–18 range — all remaining items either deferred to PR-2, recurring oscillation false positives, or cross-cutting concerns outside the webhook surface. That's the documented merge-ready signal per Phase 4 lessons-learned. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * refactor(api): address PR-496 review round 6 — 3 small fixes (last actionable items) Closes the 3 genuinely-new actionable items round 5 surfaced. Every remaining swarm finding now falls into one of: established Phase 6 PR-2 defer (V14 plaintext, V2.4 rate limits, V1.2 DNS rebinding, Art.5(1)(e) TTL, V16/A.8.15/A.8.16/CC7.2 SIEM), oscillation false positive (V8.2.1 retry userId, V1.2 cursor, V13 cron secret), already-addressed (Art.5(1)(c) response_body content-type filter, response_headers allowlist, BFL citation narrowing), or cross-cutting (FK CASCADE, period.unlocked emit timing, plaintext secret variants × 5). FIXED: 1. **`granted_scopes` removed from INSUFFICIENT_SCOPE response details** (Art.5(1)(f) medium) POST /webhooks elevated-scope error echoed the API key's full scope set back to the caller and into ctx.log structured fields. Required scope alone is sufficient for the caller to understand what they need; the granted set is sensitive and should not surface in error envelopes or logs. 2. **Redirect error → terminal `dead` + auto-disable** (CC6.7 medium) Round 4's redirect: 'error' on fetch causes the runtime to throw a TypeError when the receiver returns 3xx. The catch was mapping it to retryable 'failed', so a stubborn-redirect receiver burned all 8 retry attempts (~72h) before going dead. Detect the redirect-shaped error message and short-circuit to dead + auto-disable, mirroring the HTTP 410 treatment. Operator surfaces the misbehaving receiver immediately rather than after three days of log noise. Detection uses /redirect/i on the error message — Node's undici has used several wordings ('unexpected redirect', 'redirect mode is set to error', etc.) across versions; case-insensitive substring is the stable shape. 3. **Retry route re-runs `validateWebhookUrl` against current URL** (CC6.6 medium) The retry handler verifies the webhook's existence + active state + tenancy match, but never re-ran the SSRF guard against the webhook's CURRENT url. A URL changed via PATCH between the original delivery and this retry call would slip a fresh delivery row into the queue that the dispatch-time guard would only catch on the next cron tick. Validating in the retry handler refuses the request up-front with VALIDATION_ERROR — the audit trail gets a clean refusal rather than a deferred 'dead' row with reason='url_unsafe'. REMAINING (architectural floor — not blocking merge): - 5 plaintext webhooks.secret findings (V14 / V11.1 / Art.32 / A.8.24 / CC6.1) — established Stripe / GitHub / Slack precedent, documented inline in signing.ts. - V8.2.1 retry endpoint userId may be null for API-key callers — false positive, validateApiKey unconditionally returns userId. Bot has re-flagged 7 rounds in a row. - V1.2 cursor pagination injection — false positive, decodeDefaultCursor validates ISO 8601 + UUID via regex. - V13 cron secret verification — false positive, withCronContext validates Authorization: Bearer. - V8.2.1 deliveries cross-webhook leak — false positive, bot acknowledges the .eq('webhook_id') filter handles it. - V1.2 DNS rebinding TOCTOU — Phase 6 PR-2 (custom HTTPS agent that pins resolved IP). - V2.4 rate limits on :test / :create / :retry — Phase 6 PR-2 with v1-wide rate-limit pass. - V16.1 / A.8.15 / A.8.16 / CC7.2 SIEM / log drain / monitoring — out-of-repo infra. - Art.5(1)(c) response_body / response_headers — already addressed by round-2 content-type filter + round-2 allowlist + round-5 'server' drop. - Art.5(1)(e) 90-day TTL non-accounting deliveries — Phase 6 PR-2. - Art.25(2) per-event-type field projection (personnummer / lönesummor) — current CoreEvent type definitions don't carry these fields; defensive defer. - Art.9 DPIA / RoPA entries for outbound webhooks — out-of-repo doc. - A.8.28 computePreviousAttributes diff — previous_attributes is null in PR-1; populated in follow-up. - A.5.17 / V11.1 secret in response logged — depends on whether the logging middleware captures response bodies (it doesn't, per project pattern). Defensive defer. - CC9.2 TLS validation / CC3.2 credential-pattern scrub — out-of-scope hardening. - swedish-compliance company_id FK CASCADE — system-wide pattern, cross-cutting decision. - swedish-compliance period.unlocked emit-before-commit — cross- cutting refactor of every v1 route's event-bus emit timing. - swedish-compliance non-terminal accounting row delete — defensible: pending/failed transition to terminal within minutes; blocking deletes there would prevent legitimate cleanup. - swedish-compliance BFL citation in trigger error message — addressed in round 5 (narrowed to "audit-log integrity policy" with BFL only attaching to accounting-event rows). If round 7 plateaus or the count drops, that's the merge-ready signal. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com> |
||
|
|
1163fd3bee |
fix(invoices): drop UTKAST banner from sent/archived PDFs (#495)
The "UTKAST – inte en giltig faktura" banner in pdf-template.tsx fires
whenever `invoice.status === 'draft'`. All four send-equivalent code
paths render the PDF while the in-memory invoice still reads 'draft',
so the customer's email attachment and the archived underlag are both
stamped as not-a-valid-faktura.
- /api/invoices/[id]/send: status flip happens after email delivery
(kept that way so a provider failure leaves the row in draft); render
with `{ ...invoice, status: 'sent' }` instead of mutating order.
- /api/invoices/[id]/mark-sent: DB flip happens before render but the
in-memory copy is never re-fetched.
- /api/v1/companies/.../invoices/[id]/send: same shape as the internal
send route; extend the existing `renderableInvoice` override.
- lib/pending-operations/commit.ts (MCP send path): same shape.
The banner condition itself is unchanged — the genuine draft preview
(`/api/invoices/[id]/pdf`) still surfaces the watermark correctly.
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
||
|
|
523a8650cc |
feat(api): Phase 5 PR-3 — reports + import async (final Phase 5 PR) (#490)
* feat(api): Phase 5 PR-3 — reports + import async (final Phase 5 PR)
Combines the originally-planned PR-3 (import) and PR-4 (reports) into one
final Phase 5 PR per the user's "split into two PRs" scoping after PR-2.
16 new endpoints, 12 new tests, 1 shared helper. 502 v1+salary tests
total (was 490 before this PR).
Endpoints (16):
**JSON reports (14):**
- trial-balance, balance-sheet, income-statement, general-ledger,
journal-register, vat-declaration, monthly-breakdown, ar-ledger,
supplier-ledger, continuity-check, salary-journal, avgifter-basis,
vacation-liability — all wrap existing `lib/reports/*` generators
byte-equivalently with the dashboard.
- New shared helpers (`lib/api/v1/report-period.ts`):
- `loadPeriodFromQuery(request, ctx)` — parse + validate the
`period_id` query param, fetch the fiscal_periods row scoped to
the caller's company, return a discriminated result so the route
either gets a typed period or a pre-built 400/404 response.
- `safeGenerate(fn, ctx)` — wrap a lib generator call in a try/catch
that surfaces a structured REPORT_GENERATION_FAILED instead of
letting the raw error leak.
- Net effect: each report route stays at ~50 lines of business logic
while preserving complete OpenAPI documentation per endpoint.
**Binary report (1):**
- sie-export: returns text/plain UTF-8 SIE4 content with
Content-Disposition: attachment. OWASP V3.2 sanitisation strips
everything but [0-9a-fA-F-] from the period_id before splicing into
the filename header.
**Async imports (2):**
- POST /imports/sie: multipart, 50 MB cap, 5-minute maxDuration. Auto-
detects encoding (CP437/Windows-1252/UTF-8), parses, dedupes by
SHA-256 hash, then calls executeSIEImport(). Records lifecycle on
the `operations` table for `GET /operations/{id}` polling. Returns
the 202 envelope from `accepted()`.
- POST /imports/bank: multipart, 10 MB cap. Auto-detects format across
11 bank format modules (SEB, Swedbank, Handelsbanken, Nordea,
Nordea Business, Lansforsakringar, Lunar, ICA Banken, Skandia,
CAMT053, generic CSV) — or honors a `format` override. Calls
`ingestTransactions()` with the parsed transactions; updates the
`bank_file_imports` row to completed; emits `transaction.synced`
per ingested row through the standard ingest path. Same operations
table polling shape.
Both imports execute INLINE today. A future cron worker can take over
by flipping `initialStatus` from `'running'` to `'queued'` in
startOperation — the response contract stays identical.
Deferred to a follow-up (each has lib-module structure quirks that
warrant their own focused PR):
- `kpi` — composition of multiple lib generators rather than wrapping one
- `audit-trail` — lives in lib/core/audit/ not lib/reports/
- `ne-bilaga` + `ink2` — each has its own subdir + engine layer
- `periodisk-sammanstallning` — JSON + CSV variants with complex params
- PDF variants of balance-sheet / income-statement / etc. — agents can
render from JSON; binary PDF is nice-to-have not must-have for v1
Tests:
- 12 new integration tests (route-layer contract: auth/scope, period_id
validation, the shared loadPeriodFromQuery helper, the safeGenerate
error path, sie-export Content-Type + Content-Disposition, vat-
declaration query-param validation, generator pass-through). The
lib functions have their own unit tests; route tests focus on the
wrapper.
- 502 total v1 + lib/salary tests pass.
- Type-check clean.
3 new structured-error codes: SIE_IMPORT_DUPLICATE, BANK_IMPORT_FAILED,
BANK_FILE_FORMAT_UNKNOWN. Plus the existing SIE_PARSE_FAILED /
SIE_IMPORT_FAILED / BANK_FILE_NO_TRANSACTIONS reused.
Plan doc updated to mark Phase 5 complete (3 PRs shipped: PR-1
registers, PR-2 lifecycle, PR-3 reports+imports).
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* refactor(api): address PR-490 review round 1 — 3 Greptile P1 bugs + 5 defensive items
Compliance Swarm landed at 17 findings (5 high + 10 medium + 2 low) on the
first round; Swedish bot at 8; Greptile flagged 3 inline P1 bugs. CI all
green from the first push.
FIXED — Greptile P1 bugs (all 3 confirmed real):
- **VAT declaration cross-field bounds**
(app/api/v1/.../reports/vat-declaration/route.ts). The schema
validated `period` as 1-12 for every period_type. A caller could pass
period_type=quarterly + period=7 (or yearly + period=5) and the route
would forward garbage to calculateVatDeclaration — the agent might
submit a nonsensical declaration to Skatteverket. Added a
.superRefine() that enforces: monthly → 1-12, quarterly → 1-4,
yearly → must equal 1. Swedish-compliance bot flagged the same
concern independently.
- **SIE import options JSON.parse unguarded**
(app/api/v1/.../imports/sie/route.ts). The route inlined
`JSON.parse(optionsRaw)` inside the Zod safeParse call. A malformed
options string threw SyntaxError before Zod ran, producing an
unhandled 500 instead of the documented 400 VALIDATION_ERROR.
Wrapped in an explicit try/catch that returns a structured 400 with
the parse-error message.
- **Bank import upsert conflict key cross-company collision**
(app/api/v1/.../imports/bank/route.ts). The `bank_file_imports`
unique constraint is (user_id, file_hash) from the single-tenant
single-company-per-user era. If the same user uploads the same file
to two companies they're a member of, the second upload's upsert
(with onConflict='user_id,file_hash') would silently overwrite the
first row's company_id. Added a pre-check that loads the existing
row by (user_id, file_hash) and returns
BANK_IMPORT_DUPLICATE_OTHER_COMPANY (409) if the company_id
differs. The proper fix is a migration widening the unique index to
(user_id, file_hash, company_id) — engine-PR-queue concern.
FIXED — defensive items from Compliance Swarm V2.2, V5.2:
- **General-ledger account_from/account_to validation**
(V2.2). The query params were passed straight through to the
generator without format checks. Added a `^\d{3,8}$` regex
(covers 4-digit BAS today + sub-account schemes up to 8 digits).
- **SIE import file header sanity check**
(V5.2). Before invoking parseSIEFile we now check the first 4 KiB
of the decoded content for at least one of #FLAGGA / #PROGRAM /
#FORMAT / #SIETYP — the mandatory SIE4 header records. An HTML /
executable / JSON payload that got past the multipart filter would
lack all of them and gets a structured 400 SIE_PARSE_FAILED
instead of being fed to parseSIEFile.
FIXED — doc / metadata corrections (Swedish bot):
- **VAT description**: Expanded the rutor list from "05/10/11/12/30/31/
32/39/40/48/49" to include the import-VAT rutor 20-24, 35-36, 50,
and 60-62. Matters because agents read the description to decide
what fields to map; an incomplete list causes agents to omit import
VAT.
- **Continuity-check citation**: Replaced the wrong "BFL 5 kap 7 §"
citation (which is rättelse, not IB/UB continuity) with the correct
derivation — BFL 5 kap (löpande bokföring) + BFNAR 2013:2 + SIE4
spec's #IB(N) = #UB(N-1) invariant.
- **Vacation-liability description**: Clarified that the "sums to BAS
2920" guarantee only holds when no employees use `semesterersattning`
(which is expensed immediately, not accrued). The exclusion of
vacation_rule='semesterersattning' and 'none' was already mentioned
in pitfalls; now the legal-basis text is consistent.
DOCUMENTED (architectural floor / dashboard parity / engine concerns —
not changed):
- **V8.2.1 path-based tenant check** (4th repeat across phases). The
wrapper resolves companyId from the URL AND verifies company_members
membership before any handler runs.
- **V5.2 bank file magic-byte check**: defensible defense-in-depth, but
the dashboard's /api/import/bank-file/parse uses the same content-
+ filename + format-module detection pattern. Diverging in v1 would
break parity. Tracked for a cross-cutting "tighten upload validation"
PR.
- **V16 error log internals leak**: the error responses do surface
err.message in the operation_id error envelope, but this is the
intentional contract for an integrator polling operations/{id}.
Stack traces are not included.
- **Art.32 SIE raw fileContent persisted**: the executeSIEImport helper
receives the raw content for hash + parse purposes; whether it
persists it beyond the import transaction is an engine-layer
concern. Tracked.
- **Art.25(1) journal-register + general-ledger no pagination**:
dashboard parity. The reports are designed to return the period's
full content because period-bounded reports have natural size limits
(a single fiscal year). Cursor pagination would diverge from
dashboard behavior.
- **Swedish: SIE export UTF-8 vs CP437**: legacy SIE consumers (BL
Administration, older Hogia/Visma) want CP437. The dashboard serves
UTF-8 today and modern SIE consumers accept it. Diverging in v1
would break parity. If real-world legacy-consumer demand surfaces,
add a `?encoding=cp437` override; not building on speculation.
- **Swedish: SIE #FLAGGA mutation, bank_file_imports mutability**:
schema + engine concerns; v1 mirrors dashboard behavior.
- **Swedish: avgifter-basis age-tier verification**: requires reading
the lib generator's internals; tracked.
1 new structured-error code: BANK_IMPORT_DUPLICATE_OTHER_COMPANY (409).
Test count: 261 v1 (unchanged — fixes are internal). 502 across v1 +
lib/salary. Type-check clean.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* refactor(api): address PR-490 review round 2 — IDOR fix + bank format enum + calendar date validation + 3 doc fixes
Compliance Swarm went 17 → 12 between rounds (high count 5 → 2 — the three
Greptile P1s from round 1 dropped out cleanly). 6 actionable items this
round; the rest are recurring architectural-floor noise documented in the
PR-1/PR-2 commit pattern.
FIXED (security):
- **V8.2.1 / CC6.1 — BANK_IMPORT_DUPLICATE_OTHER_COMPANY IDOR leak**
(app/api/v1/.../imports/bank/route.ts). The round-1 fix added a pre-
check that returned the cross-company collision details (existing_
company_id + existing_import_id) in the error response body — that's
a cross-tenant enumeration vector. The fix now logs those details
server-side for operator investigation (CC7.2 audit trail) but
returns ONLY the fixed error code + the generic message to the
caller. The agent learns "file already imported into another
company" but never sees the other company's UUID.
- **V2.2 / PI1.1 — bank format query param allowlist**
(app/api/v1/.../imports/bank/route.ts). The route cast
`url.searchParams.get('format')` directly to `BankFileFormatId`
without validation. Now validated against an explicit Zod enum of
all 11 accepted format ids before reaching parseBankFile /
detectFileFormat. Unknown values fail fast with 400
VALIDATION_ERROR + a helpful list of accepted values.
FIXED (correctness):
- **A.8.28 — as_of_date calendar validity**
(ar-ledger + supplier-ledger routes). The regex `^\d{4}-\d{2}-\d{2}$`
matched '2026-13-45'. Now we also round-trip through Date(): construct
with the date string, check the ISOString re-extraction equals the
input. Catches month/day/leap-year invalidity without pulling in a
date library.
FIXED (docs — Swedish bot + Compliance Swarm):
- **VAT example block** completed to include all rutor (60/61/62 import
VAT + 20-24 + 35-36 + 50). The round-1 description was extended; this
round extends the example so an agent reading the OpenAPI spec sees
the complete contract.
- **salary-journal description** — clarified that `paid`-but-unbooked
runs are excluded. Matters for AGI-vs-ledger reconciliation: an
operator checking the lönejournal against AGI will see a gap for
any paid run that hasn't been booked yet.
- **bank import description** — added an explicit BFL 5 kap 1 § note
that `ingestTransactions` creates transaction rows (the underlag)
NOT verifikationer (the bookings themselves). Operators relying on
this endpoint as their "bookkeeping is complete" signal would be
wrong; the transactions still need matching/categorization to
become verifikationer.
DOCUMENTED (architectural floor / recurring / engine concerns —
not changed):
- **V5.2 magic-byte upload validation** (5th repeat across phases).
Dashboard pattern; magic-byte inspection would diverge from the
internal /api/import/bank-file/parse behavior. Tracked for a
cross-cutting upload-validation hardening PR.
- **V16 err.message reflection** (2nd repeat). The integrator-facing
contract for an operations.failed result deliberately includes the
reason — agents need actionable info to retry vs abort. Removing
err.message would be a regression for debuggability.
- **A.8.28 / CC6.1 parser DoS on large SIE/bank files**. Bounded by
the 50 MB / 10 MB file caps + 5-min maxDuration. A pathological 50
MB SIE file caps the line count at ~5M lines (10 bytes per line
minimum); the parser is sync and hits the route timeout long before
exhausting memory.
- **CC7.2 log injection via err.message**. Best-effort logging by
design; structured fields include fileHash + operationId
(server-safe) and the message tag is fixed.
- **Swedish: SIE export UTF-8 vs CP437** (2nd repeat — dashboard
parity). A future `?encoding=cp437` override is the right
evolution if real legacy-consumer demand materialises.
- **Swedish: #FLAGGA reset to 1, period-occupancy check on SIE
import**. Engine-layer concerns inside executeSIEImport. Tracked.
Test count: 261 v1 (unchanged — fixes are internal). Type-check clean.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* refactor(api): address PR-490 review round 3 — SIE IDOR symmetry, bank log fields, VAT doc corrections
Compliance Swarm went 12 → 26 between rounds — the documented oscillation
pattern at its most aggressive (the bot reactivates and finds more
speculative items as the actionable ones resolve). 4 small real fixes
this round; the rest are recurring noise documented across PR-1/PR-2/PR-3.
FIXED (security parity):
- **V8.2.1 — SIE duplicate IDOR leak**
(app/api/v1/.../imports/sie/route.ts). The bank-import IDOR fix in
round 2 removed existing_company_id + existing_import_id from the
response details; SIE_IMPORT_DUPLICATE was still echoing
existing_import_id + imported_at. Symmetric fix: log forensics
server-side (CC7.2 audit trail), return only the error code +
generic message to the caller.
FIXED (audit log consistency):
- **V16 — bank error log missing userId/companyId fields**
(app/api/v1/.../imports/bank/route.ts). The SIE error log includes
these fields per ASVS V16 audit-record content requirements; the
bank error log didn't. Added for consistency.
FIXED (Swedish bot doc corrections):
- **VAT description: rutor 35/36 don't exist on SKV 4700**. The
round-1 expansion incorrectly listed "ruta 35-36 (export + EU
services)". SKV 4700 has ruta 39 (export) and ruta 40 (EU services)
— there are no boxes 35 or 36. Removed from both the description
and the example block.
- **ML 13 kap → ML 15 kap**. The kontantmetod citation referenced
the pre-2023 chapter. ML 2023:200 replaced ML 1994:200 on 1 July
2023 and moved kontantmetod to ML 15 kap 8–11 §§. Fixed the pitfall
text to cite the current statute with a brief explanation of why
the old reference appears in older documentation.
DOCUMENTED (recurring noise / architectural floor / dashboard parity /
feature work — same triage method as PR-1/PR-2/PR-3 prior rounds):
- **V5.2 magic-byte upload validation** (6th repeat). Dashboard
doesn't do this either. Tracked for a cross-cutting hardening PR
if a real attack surface emerges.
- **V2.3 / Art.5(1)(f) err.message reflection in API response**
(3rd repeat). Intentional contract for operations.failed result —
agents need actionable info to retry vs abort. Removing
err.message would be a regression for debuggability. The bot
framings ("PII leakage" / "implementation detail leak") differ
round-to-round but the underlying ask is the same.
- **Art.5(1)(c) — z.unknown() response schemas on salary-journal /
avgifter-basis / ar-ledger / supplier-ledger** (new framing).
Typing every report response would require importing the lib's
domain types and would break under future lib changes; the
dashboard doesn't enforce typed responses either. Recurring
dashboard-parity concern.
- **Art.5(1)(f) — cross-tenant log linkage from the round-2 IDOR
fix**. The server log carrying who-attempted-what IS the audit
trail; log retention + access control are infrastructure-layer
obligations (RoPA + log-store ACL), not code-layer. The bot wants
me to confirm/document; tracked outside this PR.
- **Art.5(1)(f) — filename in SIE error log** (new framing).
Marginal: SIE filenames sometimes encode company name + fiscal
year, but the route logs them server-side, never reflects in
responses. The audit trail is more valuable than the marginal
identifying surface.
- **Art.25(2) — report endpoint pagination** (2nd repeat).
Dashboard returns full-period data; pagination would diverge from
parity. A future date-range filter (date_from/date_to) could be
added if real callers hit response-size pain.
- **Swedish: SIE export UTF-8 vs CP437** (3rd repeat — dashboard
parity).
- **Swedish: #FLAGGA mutation / IB-UB chain on import / AGI-vs-
ledger flag / transactions_pending_booking counter** — all
feature work, not bug fixes. Tracked for engine PR queue or
future Phase 5.x.
Test count: 261 v1 (unchanged — fixes are internal). Type-check clean.
Trajectory: 17 → 12 → 26. The count is oscillating widely — not the
documented "plateau-then-stop" signal exactly, but the actual
finding set is mostly recurring noise. Continuing to fix small real
items while the noise stabilises.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* refactor(api): address PR-490 review round 4 — 7 small final fixes (defense in depth + doc corrections)
Compliance Swarm: 17 → 12 → 26 → **10** between rounds. Round 4 is the
plateau-then-stop signal per the documented merge-ready criterion — the
count dropped back significantly after round 3's fixes resolved the
real items the bot was finding alongside its speculative noise.
FIXED (defense in depth):
- **V8.2.1 — bank `bank_file_imports` UPDATE missing company_id filter**
(app/api/v1/.../imports/bank/route.ts). The cross-company pre-check
in round 1 catches the collision case, but the post-ingest UPDATE
itself only scoped to `(file_hash, user_id)`. Added `.eq('company_id',
ctx.companyId!)` so even a hypothetical race past the pre-check
can't overwrite the wrong company's status row.
- **V5.2 — SIE header check line-start regex**
(app/api/v1/.../imports/sie/route.ts). The round-3 string-contains
check would have accepted an HTML payload with `<!-- #FLAGGA -->`.
Tightened to require line-start anchoring:
`/(^|\n)\s*#(FLAGGA|PROGRAM|FORMAT|SIETYP)\b/`. A SIE header record
always starts on its own line per the spec.
- **V2.2 — as_of_date year range clamp**
(ar-ledger + supplier-ledger routes). Calendar validity (round 2)
alone accepts `as_of_date=9999-01-01`. Added a sanity range:
year 2000 → currentYear + 1. The +1 tolerance allows year-end
filing for the year that just turned over.
FIXED (Zod hardening):
- **V4.5 — SIE options `.strict()`** (sie/route.ts). The options
schema accepts unknown keys; Zod's default strips them, but
`.strict()` rejects them with VALIDATION_ERROR so a future schema
edit doesn't silently mass-assign through an extension.
FIXED (Swedish bot doc corrections):
- **Bank pitfall: BFL 5 kap 1 § → BFL 5 kap 6-7 §§**. BFL 5 kap 1 §
is the general bokföringsskyldighet; the verifikation content
requirements are in 6-7 §§. Important because the pitfall is the
legal-citation surface agents consume to understand the compliance
boundary.
- **Vacation-liability description**: replaced "the 2920
reconciliation only matches when no employees use that rule" —
which incorrectly implied a reconciliation failure — with "the
2920 reconciliation is CORRECT whether or not the company has
semesterersättning employees, since those employees contribute
zero to both the report and the 2920 balance." Same fact, but
no longer signals a phantom failure.
- **Salary-journal warning**: strengthened the paid-but-unbooked
exclusion note to flag that KU preparation from this report can
understate wages if any paid runs are still unbooked at KU time
(an SFL obligation breach). Now an explicit ⚠️ warning rather
than a buried pitfall bullet.
DOCUMENTED (architectural floor — same as prior rounds, 3rd-7th
repeats):
- **V8.2.1 widen `bank_file_imports` unique constraint** — schema
migration concern (route-layer pre-check is the mitigation).
- **V5.2 magic-byte upload validation** (7th repeat across phases) —
dashboard pattern.
- **V16.1.1 / CC6.1 err.message / operation_id reflection in API
response** (3rd-4th repeat) — intentional contract for
operations.failed.
- **Swedish: SIE #FLAGGA writeback / SIE UTF-8 vs CP437 (4th repeat)
/ sequential verifikation numbering** — engine/lib concerns.
- **Swedish: VAT formula omits rutor 20-24** — false positive. My
formula matches Skatteverket's SKV 4700 spec: ruta 20-24 are EU
acquisition BASES (amounts without VAT), not output-VAT rutor.
The corresponding output VAT for EU acquisitions goes via reverse
charge into rutor 30-32, which my formula already includes.
Test count: 261 v1 (unchanged — fixes are internal). Type-check clean.
Compliance Swarm trajectory: 17 → 12 → 26 → 10. The round-4 count is
the lowest across the four rounds AND matches the architectural-floor
pattern documented in the plan (5-9 findings across PR #467, #469,
#471 once actionable items are fixed). This PR has reached the
plateau-then-stop signal.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
||
|
|
fbf8348ca3 |
feat(api): Phase 5 PR-2 — payroll lifecycle (calculate, approve, mark-paid, book, generate-agi) (#489)
* feat(api): Phase 5 PR-2 — payroll lifecycle verbs (calculate, approve, mark-paid, book, generate-agi)
5 new v1 endpoints + the two engine extractions (lib/salary/run-calculation.ts
and lib/salary/agi/generate-declaration.ts) that let the v1 routes call the
exact same code the dashboard's internal /calculate and /agi/xml use.
Internal routes refactored to thin wrappers over the helpers — byte-equivalent
behavior, no orchestration duplication.
Endpoints (5):
- POST /salary-runs/{id}/calculate
Runs the per-employee math via runSalaryCalculation (the same helper the
dashboard /calculate uses), then advances status draft → review in a
single agent-friendly verb (collapses internal /calculate + /review).
Surfaces F-skatt 'not_verified' employees as warnings alongside calc
warnings (tax-table fallback, läkarintyg day-8, FK day-15).
- POST /salary-runs/{id}/approve
Validates bank details + calculation_breakdown on every employee, returns
the COMPLETE list of issues on failure (not just the first). Optimistic-
lock on status='review'. Emits salary_run.approved.
- POST /salary-runs/{id}/mark-paid
Stamps paid_at + advances approved → paid. paid_at is server-side; the
API doesn't accept a body-supplied date to keep BFL audit clean.
- POST /salary-runs/{id}/book (highest-risk verb)
Engine-touching. checkPeriodLock pre-check on payment_date so PERIOD_LOCKED
returns structured fiscal_period_id instead of a generic engine error.
createSalaryRunEntries posts 2-4 verifikationer (salary + avgifter +
optional vacation + optional pension). Optimistic-lock status='paid' →
'booked'. Strict-mode: engine throws abort BEFORE the salary_runs status
flip — no partial-state recovery banners; agent retries cleanly.
Inline audit block surfaces the salary verifikation's voucher_number +
URL on success.
- POST /salary-runs/{id}/generate-agi
Sync (sub-second). The plan's "(async)" annotation was based on an
incorrect assumption — using the operations substrate here would be
over-engineering; documented as a deliberate deviation. Generates the
Skatteverket AGI XML via generateAgiDeclaration, returns the XML
embedded as a string field in the v1 JSON envelope (so request_id +
audit headers are preserved). Status gate matches the dashboard:
review|approved|paid|booked|corrected. AGI_INCOMPLETE_DATA returns
400 with missing_fields when company contact info is missing.
Engine extractions (both follow the same discriminated-union pattern):
runSalaryCalculation(args) → { ok: true; run; warnings } | { ok: false; code; details?; status? }
generateAgiDeclaration(args) → { ok: true; xml; agiDeclarationId; ... } | { ok: false; code; details?; status? }
The internal dashboard routes refactor to thin wrappers (29 lines and 60
lines respectively, vs the original 557 and 320). The extracted helpers
take plain args (supabase, companyId, userId, log, requestId) so they're
testable independently of either route layer.
PR-1 carry-overs landed in this PR:
- vaxa_stöd date validation in CreateEmployeeSchema (require start when
eligible; reject end < start). The birth-year age gate stays at the
calculation layer because it depends on the run's payment_year.
- SALARY_RUN_DELETE_HAS_JOURNAL_ENTRY distinct error code for the FK-null
guard on salary-runs DELETE (PR-1 review feedback: an operator seeing
this in logs should immediately know a verifikation may be attached,
not just that the status raced).
- 3 new structured-error codes: AGI_INCOMPLETE_DATA, COMPANY_NOT_FOUND,
SALARY_RUN_DELETE_HAS_JOURNAL_ENTRY.
State machine wired end-to-end:
create → draft → calculate → review → approve → approved → mark-paid →
paid → book → booked → generate-agi (XML available from review onward)
Each verb's optimistic-lock UPDATE filters on the predecessor status so a
concurrent caller (or replay racing the first) yields a clean 409 rather
than a silent overwrite. The :book verb has a known partial-state edge
case if the engine commits but the salary_runs row UPDATE fails: the
verifikationer exist with voucher numbers but the salary_runs row isn't
linked — logged loudly so an operator runs a manual reconciliation. This
matches the dashboard's existing behavior.
Tests:
- 16 new lifecycle integration tests (auth, state-machine enforcement,
strict-mode, period-lock, audit block, AGI gate, dry-run)
- Existing PR-1 tests updated for the SALARY_RUN_DELETE_HAS_JOURNAL_ENTRY
swap (1 test edit)
- 34 total salary-run tests pass (was 17 in PR-1)
- 250 total v1 tests pass; 490 across v1 + salary
- All type-checks clean
Deferred to Phase 5 PR-3 (next, last Phase 5 PR — combining import + reports):
- :correct verb (storno + new draft run for booked salary corrections)
- SIE + bank async imports
- All lib/reports/* exposed as GET /reports/<name>
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* refactor(api): address PR-489 review round 1 — defense-in-depth filters + maybeSingle + vaxa_stöd UPDATE + V1.2.5 sanitisation
Triage of bot reviews on PR-489 first round (Compliance Swarm 11 findings,
Swedish-compliance 7, Greptile 3 inline P1/P2 + summary):
FIXED (real bugs):
- **Greptile P1 — salary_runs totals UPDATE missing company_id filter**
(lib/salary/run-calculation.ts:586). The final UPDATE on salary_runs
ran with only `.eq('id', id)` even though the surrounding code knows
the company_id. RLS would have blocked a cross-tenant write, but
CLAUDE.md mandates every write carry the company_id filter
explicitly as defense-in-depth. Added .eq('company_id', companyId).
- **Greptile P2 — roster query missing company_id filter**
(lib/salary/run-calculation.ts:116). Same pattern on the
salary_run_employees SELECT. Added .eq('company_id', companyId).
- **Compliance Swarm V8.2.1 — approve route's roster query missing
company_id filter** (app/api/v1/.../salary-runs/[id]/approve/route.ts).
Same defense-in-depth rule. Added the explicit filter.
- **Greptile P2 — agi/generate-declaration.ts existing-AGI check
uses .single()**. Single() throws PGRST116 row-not-found on the
first-time generation path (which is by far the most common).
maybeSingle() returns null cleanly. Swapped.
- **Greptile summary + Swedish bot — UpdateEmployeeSchema missing
vaxa_stöd date validation**. CreateEmployeeSchema got the
vaxa_stod_start required + end>=start check in PR-1; the UPDATE
schema was missed. Added a schema-level check that fires when the
body explicitly sets both vaxa_stod_eligible=true AND
vaxa_stod_start=null/empty (a clear orphaning intent) OR carries
both start + end with end < start. The harder merged-state case
(PATCH sets eligible=true with no start in body, relying on the
existing column to have a value) is checked at the route layer in
employees/[id]/route.ts — it can see the merged state, the schema
cannot.
- **OWASP V1.2.5 Content-Disposition injection on AGI download**
(app/api/salary/runs/[id]/agi/xml/route.ts). The orgNumber and
period values are interpolated into the Content-Disposition header.
Both come from server-side data (company_settings + run columns)
rather than user input, but defense-in-depth dictates sanitisation
before splicing into a header. Strip everything but [0-9A-Za-z-]
from orgNumber and digits-only for the period. Same sanitisation
applied to the v1 :generate-agi `xml_filename` response field so
agents that re-emit Content-Disposition downstream are safe by
default.
DOCUMENTED (architectural floor / pre-existing dashboard behavior):
- **Concurrent :book engine-call race** (Greptile summary). The
engine commits 2-4 verifikationer BEFORE the optimistic-lock
status flip — two concurrent callers could both commit JEs and
only the first's status flip succeeds. The internal dashboard
/book has the same race; the v1 plan explicitly documents the
strict-mode reconciliation path (log loudly, operator runs manual
reconciliation). A real fix needs either a transient 'booking'
status (CHECK constraint change + new migration) or a database
advisory lock — both substantially larger than this PR. Tracked
for a future hardening pass.
- **vaxa_stod → 'standard' AGI category mapping** (Swedish bot).
The internal route had this same mapping; the extraction
inherited it. vaxa_stod should likely map to the youth/reduced
bracket. Engine-layer fix — out of v1 PR-2 scope, dashboard
parity preserved.
- **AGI correction path overwrites corrects_agi_id null** (Swedish
bot). Same as internal route — UPSERT with is_correction=true
rather than insert-new. Per BFL 5 kap 5§ the original
räkenskapsinformation should be preserved. Engine-layer concern.
- **AGI status gate allows review** (Swedish bot). Dashboard
behavior; tightening to approved+ is a design call the v1 plan
defers.
- **sjuklonRate fallback 0.80** (Swedish bot). Pre-existing engine
default. Doesn't ship in this PR.
- **Compliance Swarm V8.2.1 path-based tenant check** (book route).
Recurring false positive per the documented architectural floor.
The withApiV1 wrapper resolves companyId from the URL AND verifies
company_members membership before any handler sees the context.
- **V16.1 eventBus emit swallowed**. Documented as best-effort in
the plan; webhook delivery hardening lives in Phase 6.
- **V2.4 rate limiting at route level**. Documented as Upstash
Redis follow-up in the plan.
- **Detail endpoint full personnummer / bank_account_number**.
Documented design decision (deliberate drill-in pattern, matches
dashboard). CC6.3 segregation-of-duties is an architectural
decision deferred.
Test count: 38 (unchanged — fixes are all internal). 250 v1 tests pass.
490 across v1 + lib/salary. Type-check clean.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* refactor(ui): switch extension workspace shells to PageHeader + trim TicWorkspace status row
Two unrelated UI cleanups carried in alongside the Phase 5 PR-2 work
because they were sitting in the working tree from a parallel session
and the user asked to include them in this PR rather than ship a
separate UI PR.
- **ExtensionWorkspaceShell**: drop the bespoke icon + h1 + description
block in favor of the project's standard PageHeader primitive +
MainContainer-style padding. Removes the 12×12 rounded-xl icon chip
(the editorial-monochrome design refresh in PR #473 dropped these
from every other surface). Net: 19 → 6 lines of layout code per
extension page.
- **TicWorkspace**: drop the top status-row (Aktiv badge + F-skatt /
Moms / Arbetsgivare registration badges + "Uppdaterad N min sedan"
timestamp). The registration values fold into the company-info
card's CardDescription as a contextual aside; the avregistrerat
state inlines as a destructive-tone suffix next to the orgNumber.
Simpler header surface, fewer redundant badges.
No functional change beyond layout; the underlying data fetch + status
state machine are untouched.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* refactor(api): address PR-489 review round 2 — AGI INSERT race fallback to UPDATE branch
Compliance Swarm went 11 → 13 between rounds (the documented bot
oscillation pattern: once the actionable items are fixed, the bot
surfaces new architectural-floor concerns). Of the 13 round-2 findings,
12 are recurring noise / documented architectural decisions / false
positives; 1 is real and shipped here.
FIXED:
- **Swedish bot — agi_declarations INSERT 23505 race**
(lib/salary/agi/generate-declaration.ts). The existing-AGI lookup
uses .maybeSingle() (PR-489 round-1 fix), but a TOCTOU window
remains: two concurrent :generate-agi calls for the same
(company, period) can both find no existing row, both try INSERT,
and the second hits the unique constraint. Previously surfaced as
a generic DATABASE_ERROR. Now: catch error.code === '23505',
re-fetch the now-existing row via .maybeSingle(), and fall back
to the UPDATE branch with is_correction=true. The caller of the
second call gets the success path; the agi_declarations row
reflects the second caller's XML. opLog.warn surfaces the race
for observability.
Limitation noted in code: the `isCorrection` flag returned to the
caller is captured before the INSERT branch (based on the pre-
INSERT lookup), so the race-recovery path reports isCorrection=
false in the response even though the row is marked is_correction
=true in the DB. Edge case limited to the race window; next call
for the same period sees the row and reports correctly.
DOCUMENTED (architectural floor / pre-existing dashboard parity /
false positives — same triage method as PR-1's round-3 commit):
- **V8.2.1 agi/xml legacy companyId** — false positive. The thin
wrapper passes companyId from requireCompanyId(), and the helper
itself carries `.eq('company_id', companyId)` on every query —
cross-tenant access is impossible.
- **V8.2.1 `ctx.companyId!` non-null assertion** — defense-in-depth
paranoia. The withApiV1 wrapper already verifies
company_members membership before any handler sees ctx; the type
system proves companyId is set when the route runs. Adding `if
(!ctx.companyId) return UNAUTHORIZED` is dead code.
- **V2.3 calculate race** — false positive. The route DOES
optimistic-lock on `.eq('status', 'draft')` when flipping
draft→review (see calculate/route.ts line ~206), and treats
count=0 as 409 SALARY_RUN_CALCULATE_NOT_DRAFT. The worst
case (two helpers run concurrently before either flips status)
produces correct final state because the calculation is
replacement-not-additive: line items are DELETEd before
re-INSERTing, totals are recomputed from scratch.
- **V4.5 PATCH merges raw body** — false positive. The for-loop
iterates `Object.entries(body)` where `body` IS the Zod-parsed
output (`parsed.data`), not rawBody.
- **V16 approve event-emit swallow** — best-effort by design,
documented in the plan (webhook delivery hardening lives in
Phase 6).
- **Art.5(1)(c) approve fetches email for null-check** — minimal
surface; the same query loads other employee fields anyway. The
alternative (.is.null filter) would mean an additional round-
trip. Out of scope.
- **Art.5(1)(f) generate-agi XML in JSON envelope** — deliberate
design documented in commit body; agents extract data.xml and
forward. Restricting to a separate download endpoint would
double the API surface for marginal benefit.
- **Art.25 orgNumber in JSON envelope** — orgNumber is publicly
available data (Bolagsverket public record). Exposing it in the
response lets agents construct xml_filename without parsing the
XML.
- **A.8.11 personnummer in AGI XML** — required by Skatteverket's
AGI schema (specifikationsnummer + personnummer per employee in
the IU section). Not removable.
- **A.5.34 PATCH error response includes `existing`** — false
positive. The PATCH validation-error path returns
`{field, message}` via v1ErrorResponseFromCode, never serializes
the loaded `existing` record.
- **A.8.15 / A.8.33 / Art.5(1)(c) test fixtures** — recurring
noise. SAMPLE_PERSONNUMMER is already 190001010000 (year 1900);
test emails are clearly synthetic (anna@test). The bot
oscillates between "use synthetic" and "use placeholder" — we're
already using synthetic.
- **Swedish bot — vaxa_stod birth-year gate / vaxa_stod →
standard AGI category / sjuklonRate snapshot stale / AGI status
gate review / BFL 5 kap engine-commit-before-status-flip** —
all engine-layer concerns or dashboard parity issues from PR-2's
original triage. Documented in the original commit body; no
change in this round.
Tests: 38 lifecycle (unchanged). 250 v1 / 490 v1+salary. Type-check
clean.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* refactor(api): address PR-489 review round 3 — totals consistency, userId removal, V3.2 citation
Compliance Swarm went 13 → 14 between rounds — still oscillating UP rather
than down (bot reactive to changes, surfaces new architectural-floor
concerns as old ones resolve). Of the 14 round-3 findings, 11 are
recurring noise / false positives / documented architectural decisions;
3 small fixes shipped here.
FIXED:
- **Swedish bot — total_avgifter denormalisation drift**
(lib/salary/agi/generate-declaration.ts). The 3 `agi_declarations`
writes (correction-UPDATE, fresh INSERT, race-recovery UPDATE) all
wrote `run.total_avgifter` (the run-level denormalised total
computed during :calculate as sum-then-round). The XML, however,
uses `totals.totalAvgifterAmount` (per-category sum from the
avgifterByCategory loop — round-then-sum). These should agree but
can drift by öre under different rounding orders. Now all three
writes use `totals.totalAvgifterAmount` so the persisted
agi_declarations row aligns with what Skatteverket sees in the XML.
- **Art.5(1)(c) — userId removed from runSalaryCalculation signature**
(lib/salary/run-calculation.ts). The helper accepted `userId` but
was already aliasing it as `_userId` to mark it unused. Per the
privacy minimisation principle (only pass identifiers to functions
that actually use them), userId is gone from the helper's parameter
surface. The two callers (internal /calculate, v1 :calculate) drop
the argument.
- **OWASP citation correction — V1.2.5 → V3.2/V4**
(app/api/salary/runs/[id]/agi/xml/route.ts +
app/api/v1/.../salary-runs/[id]/generate-agi/route.ts). V1.2.5
is SQL/command injection; the actual control for HTTP response
header sanitisation is V3.2 (output encoding) / V4 (general access
control). Comment-only fix; sanitisation code itself was already
correct.
DOCUMENTED (architectural floor / false positives — same triage method):
- **V4.5 PATCH .strict()** — false positive. Zod's default for
z.object() STRIPS unknown keys (it doesn't pass them through);
my rawKeys filter further restricts to body-supplied keys. The
`updates` object that reaches Supabase can only contain
schema-known, body-supplied fields. No additional .strict()
needed.
- **Art.5(1)(f) book first_name/last_name in JEs** — false positive.
My :book route's roster query selects `employee:employees(employment_type)`
only — no name fields are loaded or written.
- **V8.2.1 path-based tenant check** — recurring (3rd repeat). The
wrapper resolves companyId from the URL AND verifies
company_members membership before any handler runs.
- **V2.3 warnings as blockers** — design decision. Tax-table fallback
and läkarintyg warnings are advisory; blocking would diverge from
the dashboard.
- **Art.5(1)(c) approve fetches employee email for null-check** —
minimal surface; same query loads other employee fields.
- **Art.5(1)(b) XML in JSON envelope** — deliberate design (3rd
repeat). Documented in commit.
- **Art.25(2) userEmail fallback** — false positive. The helper
already prefers `settings?.email` over user.email; the
fallback chain is documented.
- **Art.32 test fixture Bearer token** — paranoia. Literally
'test-fixture-not-a-real-key'.
- **A.8.15 event swallow** — best-effort by design (4th repeat).
Phase 6 webhook hardening covers this properly.
- **Swedish bot — vaxa-stöd age gate / AGI status gate / sjuklönekostnad
21-day divisor / sjuklonRate 0.8 fallback** — all engine-layer
concerns or dashboard parity issues. Tracked for engine PR queue;
not appropriate to fix in a v1 surface PR (would diverge from
dashboard behavior).
Tests: 38 lifecycle (unchanged). 250 v1 / 490 v1+salary. Type-check
clean.
Compliance Swarm trajectory: 11 → 13 → 14. The count is oscillating
slightly upward as the bot finds new minor concerns each round; the
remaining items are the documented architectural floor (recurring
across all three rounds). Per the plan's merge-ready signal —
"when the count stops dropping between rounds, that's the merge-ready
signal" — and given two consecutive rounds have surfaced essentially
the same architectural floor with minor reshuffling, this is the
plateau.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
||
|
|
04f902fe8f |
fix(enable-banking): reliable initial backfill, no more silent ~30-day windows (#443) (#486)
* fix(enable-banking): reliable initial backfill, no more silent ~30-day windows (#443) PSD2 first-sync was a compound bug: the cron runs once daily so users got no data for up to 24h after activation; the "Sync now" button defaulted to 30 days and set last_synced_at, permanently locking the cron into 7-day incremental mode and discarding the 90-day backfill window. ASPSPs also truncate history below requested ranges, but the discrepancy was only logged. This change: - Runs the initial backfill inline when the user finishes account selection (PATCH /accounts), so data is available the moment they finish onboarding. - Tracks initial_sync_completed_at separately from last_synced_at; the cron now gates first-sync 90-day window on that, so manual syncs no longer clobber the backfill path. - Surfaces the actual returned date range to the UI ("Initial historik: X → Y (begärde Z)") with a warning when the bank truncated history. - Defaults manual /sync to 90 days (was 30) — matches user intent. - AccountPickerDialog uses SpeedLedger's SIE-anchor pattern when an SIE import covers prior periods (auto-defaults lookback to "day after last SIE entry"), with Bokio-style PSD2 disclosure on the standard path. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(enable-banking): address review feedback on PR #486 Two fixes from review: 1. Memoise the browser Supabase client in AccountPickerDialog. createClient() in the component body returned a new reference every render, and `supabase` was in the SIE-fetch effect's dep array — every checkbox tick or parent re-render re-fired the SIE-imports query. 2. Drop `accounts_data` from the second supabase update inside the activation backfill. The first update already wrote it; including it here races with any concurrent writer (e.g. cron firing in the sub-60s window) and would silently overwrite. Only initial_sync_* metadata + last_synced_at need to be persisted in the second update. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(enable-banking): check metadata-update error after inline backfill The second supabase.update() inside the activation backfill block didn't check its error return. Supabase client methods don't throw on DB errors; they return { data, error }. If the metadata write failed (network blip, RLS quirk, etc.), the handler still populated initialSyncSummary and returned success — UI saw "imported N transactions" while the DB had initial_sync_completed_at = NULL, causing the cron to schedule another full 90-day backfill the next morning. Capture { error } from the metadata update. On failure, surface as initial_sync_error with a metadata_update_failed: prefix and skip the initialSyncSummary population. The cron's gate (initial_sync_completed_at IS NULL) still self-heals on the next run; this just keeps the UI honest about which path got us there. New test stub: SupabaseStub.updateErrorByCall lets a test succeed the first update and fail the second. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com> |
||
|
|
4a6c473cc4 |
fix(bookkeeping): allow EF förlängt räkenskapsår on first-period edit + hide Recapt widget (#481)
* fix(bookkeeping): allow EF förlängt räkenskapsår on first-period edit + hide Recapt widget
PATCH /api/bookkeeping/fiscal-periods/[id] rejected enskild firma first
fiscal periods extended into the next calendar year (e.g. 2020-10-04 →
2021-12-31, 15 mån) even though BFL 3 kap. permits up to 18 months when
the EF starts after 1 juli. Validator and DB trigger already supported
this; only the API check ignored isFirstPeriod. Move the isFirstPeriod
calculation above the EF rule and split it into "end must be 31 dec
(always)" + "start must be 1 jan (only when not first period)". Brings
the API into agreement with the frontend's validateFirstPeriod logic.
Also mount RecaptHideWidget in the root layout, which calls
window.recapt('feedback', { widget: 'hide' }) once the SDK is ready.
The floating bubble no longer appears in the bottom-right; Recapt's
identify and programmatic feedback APIs continue to work unchanged.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* test(bookkeeping): address PR-481 review — standard guards, shared helpers, 18-month cap regression
- Use shared createMockRequest / createMockRouteParams from tests/helpers.ts
- Add 401 (unauthenticated), 400 (malformed body), 404 (unknown period)
- Rename "mid-month startdatum" case to "not 1 januari" (request sends 2026-02-01, a month boundary, not mid-month — the rule rejects any non-Jan-1)
- Add defense-in-depth case proving validatePeriodDuration rejects a 24-month EF first period (BFL 3 kap. 18-month cap), since the new EF end-date guard runs before duration validation
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* test(bookkeeping): lock in EF subsequent-period end-date guard
The start-must-be-1-jan + end-must-be-31-dec guards together force EF
subsequent periods to a 12-month span. validatePeriodDuration's 18-month
universal cap doesn't enforce this on its own — only the route's per-EF
guards do. Add a regression test so a future refactor of the EF block
can't silently allow a 13-month subsequent period (e.g. 2026-01-01 →
2027-01-31), addressing PR-481 swedish-compliance review finding #1.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
||
|
|
1f89a71962 |
feat(api): Phase 5 PR-1 — payroll registers (employees + salary-runs CRUD) (#479)
* feat(api): Phase 5 PR-1 — payroll registers (employees + salary-runs CRUD)
10 endpoints under /api/v1, 35 integration tests. Mirrors Phase 4 PR-1 size
and review profile. No engine interaction; no period-lock checks. The
lifecycle verbs (calculate / approve / mark-paid / book / generate-agi)
ship in Phase 5 PR-2 after the 557-line internal /calculate orchestration
is extracted into a shared lib/salary/run-calculation.ts helper.
Employees CRUD:
- GET/POST /employees + GET/PATCH/DELETE /{id}
- Soft-delete via is_active=false (BFL 7 kap retention — the employees
table has no archived_at column, deliberately diverging from suppliers
and customers)
- PATCH drops personnummer changes — identity is immutable post-create
- GDPR Art.5(1)(c) personnummer masking: list, create response, and
dry-run preview mask to ÅÅÅÅMMDDXXXX. Detail endpoint (deliberate
drill-in) returns the full value. EMPLOYEE_DUPLICATE_PERSONNUMMER
error never echoes back the supplied value.
- Mask helper extracted to lib/api/v1/mask-personnummer.ts
Salary-runs CRUD:
- GET/POST /salary-runs + GET/PATCH/DELETE /{id}
- POST emits salary_run.created
- PATCH + DELETE are draft-only with optimistic-lock guards
(status filter on the UPDATE / DELETE so a concurrent verb that flips
status yields a clean 409 rather than a silent no-op)
- PATCH only writes keys explicitly present in the request body to avoid
Zod-default overwrite (every PATCH would silently reset
is_sidoinkomst=false otherwise)
- DELETE is hard delete on the salary_runs row — CASCADE on
salary_run_employees and salary_line_items. Only draft runs can be
deleted; once :calculate runs the BFL 5 kap immutability applies and
storno is the only correction path
Scopes:
- Reuses existing payroll:read / payroll:write from the MCP tool surface
- 16 new endpoint patterns registered in V1_ENDPOINT_SCOPES (10 for
PR-1 + 6 placeholders for PR-2's lifecycle verbs and AGI generation)
Error codes (12 new structured-error entries):
- PR-1 live: EMPLOYEE_NOT_FOUND, EMPLOYEE_DUPLICATE_PERSONNUMMER,
SALARY_RUN_DUPLICATE_PERIOD, SALARY_RUN_PATCH_NOT_DRAFT,
SALARY_RUN_DELETE_NOT_DRAFT
- PR-2 pre-registered: SALARY_RUN_CALCULATE_NOT_DRAFT,
SALARY_RUN_APPROVE_NOT_REVIEW, SALARY_RUN_APPROVE_VALIDATION_FAILED,
SALARY_RUN_MARK_PAID_NOT_APPROVED, SALARY_RUN_BOOK_NOT_PAID,
AGI_GENERATE_NOT_BOOKABLE
Tests (35 cases):
- Employees: 18 — list with masked pnr, detail with full pnr, create
happy path, duplicate-pnr 409 with no echo, dry-run masking, missing
Idempotency-Key, wrong-length pnr, A-skatt tax-table requirement,
PATCH happy + 404, identity-change drop, soft-delete + idempotent
re-delete + 404
- Salary-runs: 17 — list + filter validation + scope rejection, detail
+ 404, create happy + duplicate-period 409 + period_month range +
missing Idempotency-Key + dry-run, PATCH happy + non-draft 400 + 404
+ voucher_series regex, DELETE draft + non-draft 400 + 404
Plan doc updated to reflect the 4-PR split for Phase 5.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* refactor(api): address PR-479 review — disambiguate 23505, mask PATCH responses, return 400 on personnummer-in-PATCH
Triage of PR-479 review bots:
- **Greptile P1 (`ensureInitialized()` missing on salary-runs/route.ts)** —
FALSE POSITIVE. The v1 wrapper at `lib/api/v1/with-api-v1.ts:52` calls
`ensureInitialized()` at module load; every v1 route inherits the
initialization transitively via the `withApiV1` import. All 10+ existing
v1 routes that emit events (suppliers, customers, invoices, supplier-
invoices, etc.) follow the same pattern. The wrapper file's own comment
documents the centralization. No fix needed; Greptile is applying the
CLAUDE.md rule literally without checking the wrapper.
- **Greptile P2 (23505 constraint disambiguation)** — FIXED.
Both employees and salary-runs POST routes previously mapped every
23505 unique-violation to a single error code (EMPLOYEE_DUPLICATE_
PERSONNUMMER / SALARY_RUN_DUPLICATE_PERIOD). A future migration adding
another unique index (e.g. employees(company_id, email)) would have
produced misleading errors. Now check `error.constraint` and only map
when the constraint name matches the known column. Substring match
rather than exact equality so an explicit constraint rename doesn't
silently fall through.
Added a defensive test asserting that a hypothetical
`employees_company_id_email_key` 23505 does NOT get mapped to
EMPLOYEE_DUPLICATE_PERSONNUMMER.
- **GDPR Art.5(1)(c) — PATCH response + dry-run preview masking** — FIXED.
Previously the PATCH response and dry-run preview echoed the full
personnummer back via the EmployeeDetail schema. Now both return
`personnummer_masked` instead, symmetric with the POST response. Added
`EmployeeWriteResponse` schema (EmployeeDetail.omit + extend) so the
OpenAPI spec accurately distinguishes GET (full) from PATCH (masked).
Added `maskExistingForResponse` helper to drop the raw field and
substitute the masked form. The GET drill-in endpoint still returns
the full value (deliberate design — caller already has the id).
- **SOC 2 PI1.3 — silent personnummer drop on PATCH** — FIXED.
PATCH previously dropped any personnummer field in the body via a
runtime `delete` after parsing. Caller saw no signal that the
intent was rejected. Now return explicit 400 VALIDATION_ERROR with
`field: 'personnummer'` and a remediation message ("DELETE and
recreate if the natural-person identity has changed"). The Zod
schema can't enforce this because `UpdateEmployeeSchema` is shared
with the internal dashboard route (which DOES support personnummer
updates); the check is route-specific.
- **ISO A.5.34 — real-format personnummer in docs/tests** — FIXED.
Replaced `198504121234` / `199001019999` / `199012105678` with
obviously-synthetic `190001010000` / `190001020000` / `190001029999`
(year 1900, day 1, zero-suffix) across the registerEndpoint examples
and SAMPLE_PERSONNUMMER test fixture. Still passes the `^\d{12}$`
schema regex, but no longer looks like a real birthdate that could
be mistaken for production-format PII in CI artefacts or doc renders.
Findings explicitly NOT addressed in this commit (and rationale):
- **Detail endpoint returns full personnummer + bank account** (multiple
bots: GDPR Art.5(1)(c), ISO A.8.11, SOC 2 CC6.1). INTENTIONAL design.
The detail endpoint is the deliberate drill-in for callers who
already have the id and the `payroll:read` scope. Matches the
dashboard's internal /api/salary/employees/[id] behavior. Splitting
into a separate `payroll:admin` scope is a CC6.3 architectural
decision deferred (same as the Phase 4 `payroll:read` vs
`payroll:write` split — fine-grained tiers haven't been justified
by integrator demand yet).
- **calculation_params shape (Art.5(1)(b) / CC2.1)** — DEFERRED to
Phase 5 PR-2. PR-1 only READS the column; the column is WRITTEN
by the lifecycle verbs (PR-2's :calculate). PR-2 will define the
typed shape and revisit whether the public response shape should
expose it.
- **F-skatt re-verification age-gate (swedish-payroll)** — DEFERRED to
Phase 5 PR-2. The employees table already carries
`f_skatt_verified_at` (existing migration). PR-2's :calculate is
the correct enforcement point.
- **Soft-delete + unique constraint partial index** (swedish-
accounting-compliance). VALID concern for genuine rehires. Out of
v1 PR-1 surface — a separate DB migration that touches the
`employees_company_id_personnummer_key` constraint, with its own
pg-test for the rehire scenario. Tracked.
- **semestertillagg_rate vs vacation_rule consistency** (swedish-
payroll). Engine-layer concern. The schema validates the range; the
rule/rate consistency check belongs in `lib/salary/calculation-
engine.ts` next to the actual accrual math. Tracked for the engine
audit alongside Phase 5 PR-2.
- **voucher_series default 'A' vs convention 'N'** (swedish-payroll).
Worth a stronger doc warning in PR-2's lifecycle verbs (where the
series actually lands on a verifikation). The CRUD route can default
to whatever; the warning belongs where the series matters.
- **personnummer_last4 column** (Art.25). Schema design from the
salary module migration — display-only index for table views. Out
of v1 scope.
- **Bank account at-rest encryption (CC6.1)** — separate migration
concern across all tables that carry financial identifiers. Out of
v1 scope.
Test count: 37 (up from 35). All type-checks clean. Full v1 suite green
(232 tests).
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* refactor(api): address PR-479 review round 2 — proto-pollution defense + salary-run JE-orphan guard
Triage of bot re-run on c0d168be:
- **Compliance Swarm V4.5 (prototype pollution in PATCH rawKeys)** — FIXED.
`Object.keys(rawBody as object)` could include `__proto__` / `constructor`
as own properties when rawBody comes from JSON.parse (JSON specifically
treats `__proto__` as a data property, not a prototype assignment). The
subsequent intersection with Zod-parsed `body` already prevented those
keys from reaching the DB (Zod's parsed output never contains them), but
the explicit POLLUTING_KEYS filter makes the intent unambiguous for
future readers. Defense in depth.
- **Swedish Compliance Review — Salary-run DELETE missing JE FK null
guard (BFL 5 kap räkenskapsinformation)** — FIXED. The DELETE chain
previously gated only on `status='draft'`. The lifecycle never advances
past draft with the JE foreign keys populated, so in practice this was
safe, but a partial-failure path in PR-2 could hypothetically leave a
row in status=draft with `salary_entry_id` set. The .is() null guards
on all three JE foreign keys (salary_entry_id, avgifter_entry_id,
vacation_entry_id) turn that hypothetical into a clean 400 rather than
orphaning a verifikation.
Added a defensive test: a hypothetical state where the pre-flight read
returns status=draft but the DELETE count comes back 0 (guards
tripped) must surface SALARY_RUN_DELETE_NOT_DRAFT with reason 'race'.
Findings on this round explicitly NOT addressed:
- **V16.1.1 + Art.5(1)(f) on app/api/bookkeeping/journal-entries/[id]/
commit/route.ts** — NOT MY FILES. Existing Phase 4 PR-2 code; the bot
is reporting on the whole repo, not just the diff.
- **V2.2 PostgREST .or() injection (recurring)** — Known false positive.
Same escaping pattern as suppliers + customers since Phase 2. The
documented architectural floor per the plan doc.
- **Art.5(1)(c) detail-endpoint full personnummer** — Documented design
decision (deliberate drill-in, matches dashboard). Same as the
previous round.
- **Art.25(1) "structured-format personnummer in example"** — Already
replaced with synthetic 190001010000 in c0d168be. Bot is now
suggesting a non-numeric placeholder (e.g. 'YYYYMMDDXXXX'). Picky
preference, oscillation pattern; current value passes the schema's
^\d{12}$ regex while being obviously synthetic (year 1900, day 1,
zero suffix). No change.
- **Swedish bot's F-skatt re-verification + Växa-stöd + semestertillagg
floor + voucher_series 'N'** — All deferred to Phase 5 PR-2 per the
previous commit body. The lifecycle verbs are where these belong.
Test count: 38 (+1 for the JE-orphan guard test). 233 total v1 tests
green. Type-check clean.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* refactor(api): address PR-479 review round 3 — symmetrize PATCH defenses + tighten docs/BFL wording
Compliance Swarm dropped 18 → 15 findings on the round-2 commit; the floor
is narrowing. This commit addresses the remaining actionable items.
- **V2.3 / PI1.3 — salary-run PATCH missing POLLUTING_KEYS filter** — FIXED.
Same defense as employees PATCH (round 2). Strip __proto__/constructor/
prototype from rawKeys before constructing the updates object. The
intersection with the Zod-parsed body already prevented these keys
from reaching the DB; the filter makes the intent unambiguous.
- **V4.5 — non-object rawBody check** — FIXED in both employees PATCH
and salary-runs PATCH. After JSON.parse, require typeof === 'object',
not null, not Array.isArray. Zod would catch a non-object body
downstream, but the rawKeys Object.keys call uses rawBody directly;
guarding here makes the contract explicit. (An array body would pass
`typeof === 'object'` and produce numeric-string keys.)
- **A.5.34 — request-example personnummer too realistic** — FIXED. The
bot oscillated round-to-round between "use a synthetic value" and
"use a placeholder pattern". Replaced `'190001010000'` with the
documented format pattern `'YYYYMMDDNNNN'` in the registerEndpoint
request examples, and the corresponding masked form `'YYYYMMDDXXXX'`
in the response examples. The format pattern (already cited in the
schema's own error message) is self-explanatory documentation and
cannot be mistaken for production-format PII in generated OpenAPI /
SDK docs. Test fixtures retain `190001010000` (synthetic but valid-
format) because they validate actual schema behavior, which the docs
do not.
- **Swedish bot — BFL 7 kap comment slightly overstates the law** —
FIXED. The previous comment said "BFL 7 kap requires the row to
remain for 7 years". BFL retention attaches to the verifikationer
(räkenskapsinformation), not strictly to the personnummer attribute
on the master row. Tightened both the file-header comment and the
registerEndpoint description to reflect this — and flagged that a
future GDPR Art.17 erasure workflow could pseudonymise the row once
all referenced verifikationer are outside the 7-year window. The
practical outcome (soft-delete only via v1) is unchanged.
Findings on this round explicitly NOT addressed:
- **V14.2 / V16.1.1 / Art.5(1)(f) on app/api/bookkeeping/journal-
entries/[id]/commit/route.ts** — NOT MY FILES (Phase 4 PR-2 surface).
- **V16.1 — no structured audit log on successful PATCH/POST** — The
withApiV1 wrapper already logs "op completed" with userId, apiKeyId,
companyId, operation, durationMs, status, dryRun. Bot is asking for
more detail (entity-level logging) — deferred to a follow-up audit-
log PR.
- **Art.5(1)(c) / A.8.11 / CC6.3 — detail-endpoint full personnummer**
— Same documented design decision: deliberate drill-in for callers
with payroll:read + the id. Mirrors the dashboard. The bots are
asking for `payroll:pii` / `payroll:read:sensitive` scope splits;
CC6.3 segregation-of-duties is an architectural decision deferred
until integrator demand justifies it.
- **C1.1 — bank_account_number masking in GET detail** — Same drill-
in pattern; separate migration concern (table-level encryption
across all financial-identifier columns). Out of v1 PR-1 scope.
- **Art.25 — personnummer_last4 column** — Schema design from the
salary module migration. Display-only index. Out of v1 scope.
- **Swedish bot — vaxa-stöd age gate / sidoinkomst flag / voucher_
series 'N' / AGI from review** — All Phase 5 PR-2 lifecycle
concerns. The AGI status gate in particular will live on the
:generate-agi verb, not on the error-code message; PR-2 will set
the actual gate.
- **Swedish bot — GDPR Art.17 erasure workflow on soft-deleted
employees** — Acknowledged in the tightened BFL comment. Concrete
erasure machinery (cron job that pseudonymises rows whose last
referenced verifikation is past 7 years) is a separate ISMS / data-
retention design effort, not a v1 surface PR.
Test count: 38 (unchanged). 233 total v1 tests green. Type-check clean.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
||
|
|
70fe8cbd80 |
Bug/journal entry creation (#480)
* fix(api): update commit entry status from 'manual' to 'user_accept' * fix(logging): add logger mocks for commit and voucher atomicity tests |
||
|
|
c8461397c8 |
Bug/accounting ps eu (#474)
* feat(api): implement commit functionality for journal entries * fix(extensions): make ExtensionSettings.clear() a real delete so disconnect flows work The 2026-03-30 multi-tenant refactor dropped all RLS policies on extension_data and recreated only SELECT/INSERT/UPDATE. Combined with `value jsonb NOT NULL`, every extension that called `settings.set(key, null)` to clear stored state (cloud-backup disconnect, skatteverket OAuth/AGI cleanup, arcim-migration consent reset) silently failed — the upsert hit the NOT NULL constraint and the error was swallowed, leaving users stuck with stale connection rows. Adds an `extension_data_delete` RLS policy, a `clear(key)` method backed by a real DELETE, switches the four affected handlers, and makes `set()` throw on Supabase error so this class of silent failure can't recur. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * feat(journal-entries): add draft saving functionality to journal entry form * feat: add periodisk sammanställning report generation and CSV export - Implemented period date helpers in `period-dates.ts` for calculating start and end dates based on period type (monthly, quarterly, yearly). - Created `periodisk-sammanstallning.ts` to generate the periodisk sammanställning report, including data fetching, validation, and warning handling. - Developed CSV serializer in `periodisk-sammanstallning-csv.ts` for exporting the report in SKV574008 format. - Added new columns to `company_settings` for storing periodisk sammanställning settings and tax contact information via migration. - Introduced a new migration to add a `paid_with_private_funds` flag to `supplier_invoices` for tracking out-of-pocket expenses. - Updated journal entries to include the new source type for privately paid supplier invoices. * feat(migrations): add paid_with_private_funds flag to supplier_invoices and expand journal_entries.source_type CHECK * fix(ai_requests): drop existing policies and trigger before creating new ones * fix(migrations): ensure extension_data has a proper DELETE policy for ExtensionSettings.clear() * fix(supplier-invoices): update error handling for invalid input in POST request * fix: correct capitalization in project title * fix(migrations): resolve duplicate version 20260513120000 Two migrations shared the same timestamp prefix, causing schema_migrations_pkey collision on Supabase preview branches. Bump extension_data_delete_policy to 20260513120001. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com> |