Files
accounted/lib/core
Jakob Wennberg c06395f633 feat(mcp): agent-native API sprint — quick wins (items 8/10/38/39/50) (#505)
* feat(mcp): agent-native API sprint — quick wins (items 8/10/38/39/50)

Five Tier-S items from dev_docs/api_ai_architecture/PLAN.md, picked for highest
impact-per-day on a solo budget. ~7.5 engineer-days of work.

Item 38 — gnubok_reverse_journal_entry MCP tool. Wraps the existing
reverseEntry() engine function (lib/bookkeeping/engine.ts) as a staged
high-risk operation. Description distinguishes pure makulering (use this) from
rättelse (use gnubok_correct_entry) per BFL 5 kap 5§ guidance — leaving a real
affärshändelse unbooked is itself a BFL violation, so agents must understand
which storno pattern to apply. New operation_type 'reverse_entry' wired through
PendingOperationType, risk-tiers (high), commit.ts executor, and TOOL_SCOPE_MAP
(bookkeeping:write). Six executor cases + three staging-gate cases cover the
new tool.

Item 39 — period_status threading. New helper resolvePeriodStatusForDate() in
lib/core/bookkeeping/period-service.ts returns { period_id, status, lock_date }
using the same two-layer logic as the v1 REST check (company-wide
bookkeeping_locked_through + fiscal_period flags). Threaded through
stagePendingOperation via a new dateForPeriodCheck option so agents and widgets
can detect locked/closed periods without round-trips. Applied to seven
bookkeeping-touching tools: categorize_transaction, create_transactions,
create_voucher, approve_supplier_invoice, mark_invoice_as_paid, correct_entry,
reverse_journal_entry. Resolution failure is non-fatal — DB triggers stay
authoritative.

Item 50 — gnubok://company/current expansion. Replaces the metadata-only
resource with per-company working memory: active fiscal period status, lock
dates, counts (customers, suppliers, open AR/AP, uncategorized transactions),
voucher series state across open periods, recency signals (last categorization,
last invoice sent, last bank sync), and the next five approaching deadlines.
All queries parallelized via Promise.all; payload stays well under 8 KB.
Mirrors the context.md pattern from Shipper+Claude's agent-native architecture
guidance and prevents the context-starvation anti-pattern.

Item 8 — schema strictness. additionalProperties: false on every one of the 67
inputSchemas in extensions/general/mcp-server/server.ts. New
strict-schemas.test.ts guards against regression on newly authored tools.
CLAUDE.md documents the tool-authoring contract (strict input schemas,
description ≤280 chars, STAGED_OPERATION_SCHEMA + next as the
completion-signal pattern — do NOT introduce a parallel S/H/C/O envelope).
Payload-size ceiling raised from 20K → 25K tokens with a comment pointing at
item 15 (Tool Search + defer_loading) as the long-term answer rather than
relaxing the watchdog further.

Item 10 — prompt cache groundwork. The only Anthropic SDK call site in the
codebase is the invoice-inbox extension's Bedrock-backed extractor; tagged the
~3.5 KB SYSTEM_PROMPT with cache_control: { type: 'ephemeral' } and added
usage logging (cache_read_input_tokens / cache_creation_input_tokens) so the
hit ratio is measurable. The plan's 1h TTL is direct-Anthropic-only;
documented the constraint and the MCP-side determinism contract (tool
definitions must be byte-stable across requests) in the new mcp-server
README.md.

Carry-over: includes a small untracked migration
(20260516060000_journal_entries_source_type_inbox_item) and its pg test guard
that fix a production CHECK-constraint gap for source_type='inbox_item' —
unrelated to the sprint but bundled per request.

Tests: 3615/3615 pass across 252 files. TypeScript build clean.

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

* fix(mcp): address PR #505 review — cross-tenant leaks, company-wide lock, PII

Five reviewer findings on PR #505 addressed:

1. Cross-tenant leak — voucher_sequences (OWASP V8.2.1, SOC 2 CC6.3).
   Resource query filtered by user_id only; switched to company_id since the
   table has both (added in the 2026-03 multi-tenant refactor migration).

2. Cross-tenant leak — deadlines (OWASP V8.2.1, GDPR Art.5(1)(f), ISO A.8.3).
   Same fix; the deadlines table also gained a company_id column in the
   multi-tenant refactor and the RLS policies enforce it. With the company_id
   filter active, the userId parameter is no longer needed in the resource —
   removed from the destructure.

3. Compliance gap — commitReverseEntry and commitCorrectEntry only checked
   fiscal_periods.is_closed, not company_settings.bookkeeping_locked_through.
   Agents could stage a reversal with period_status: locked warning (caught
   by resolvePeriodStatusForDate at staging time), have the user approve,
   and the commit would slip through. Both executors now run
   resolvePeriodStatusForDate at commit time so the gate matches the
   staging-time signal. Pre-existing gap on commitCorrectEntry also fixed.

4. Schema mismatch — period_status was spread into both `preview` and the
   top-level response, but STAGED_OPERATION_SCHEMA only declares it at the
   top level. Removed the preview-nested copy to match the schema and avoid
   ambiguous reads.

5. Tool description — swedish-compliance bot flagged that "pure makulering
   (storno)" conflates two distinct Swedish accounting terms: storno
   preserves the original; makulering voids it entirely. Code does storno;
   description now says so plainly and cites BFL 5 kap.

6. Input hardening — added ^\d{4}-\d{2}-\d{2}$ pattern to reversal_date in
   inputSchema plus a runtime regex check in execute(), so a malformed date
   never reaches the pending_operations payload.

7. GDPR — ai_extraction_usage and the two pre-existing fileName log
   emissions in extract-invoice-fields.ts replaced raw fileName with a
   12-char SHA-256 prefix. Raw invoice file names (e.g.
   "faktura_Sven_Andersson.pdf") can constitute personal data; hashing
   preserves operator correlation without exposing PII to log destinations
   that may lack documented retention controls.

Notes on findings NOT addressed:
- Double-reversal guard (Greptile/swedish-compliance): false positive.
  reverseEntry() flips the original's status to 'reversed' (engine.ts:538)
  and the staging tool already rejects anything not 'posted'. Engine also
  has a CAS guard at lines 541-551.
- Staging vs commit TOCTOU re-validation: pre-flight + DB triggers remain
  authoritative; the window is narrow enough that adding executor-side
  re-checks isn't load-bearing this sprint.
- Runtime Zod validation of args inside execute(): codebase doesn't do
  this for any MCP tool today; cross-cutting refactor deferred.

New test: voucher-executors.test.ts adds a case for the company-wide lock
branch on reverse_entry (verifies the new resolvePeriodStatusForDate gate
fires when bookkeeping_locked_through covers entry_date).

Tests: 3616/3616 pass. TypeScript build clean.

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

* fix(mcp): address second-round PR #505 review — locked_at, reason cap, log

Re-review by compliance-swarm and swedish-accounting-compliance bots after the
first fixes raised three more legitimate findings:

1. Per-period `locked_at` not directly checked from the fetched row
   (swedish-accounting-compliance). Both commitCorrectEntry and
   commitReverseEntry already call resolvePeriodStatusForDate which covers
   locked_at, but a transient DB blip in the resolve helper would silently
   skip that gate. Now reading locked_at directly from the inner-join row and
   checking it alongside is_closed before the resolve helper runs — same
   pattern, two defense-in-depth layers instead of one.

2. `reason` field had no maxLength (OWASP V4.5). Added maxLength: 500 to the
   inputSchema and a runtime length check; an adversarial agent could
   otherwise push an arbitrarily large string into pending_operations.

3. periodStatus resolution failure was silently swallowed (ISO 27001 A.8.15).
   Now logging via console.warn with operationType, companyId,
   dateForPeriodCheck, and error so a systematic outage (missing
   company_settings row, dropped query) is observable in audit logs rather
   than degraded silently.

Findings deliberately NOT addressed (pushed back to the bots):

- gnubok_reverse_journal_entry needs per-operation role check (V8.2.1) and
  narrower 'bookkeeping:reverse' scope (CC6.3) — cross-cutting refactor; no
  MCP tool in gnubok enforces per-operation roles today. Introducing it just
  for one tool would be inconsistent. Will surface as a separate item.
- Reduce line_description in reverse_entry preview (A.8.3, Art.5(1)(c)) —
  the preview is shown to the human approver who needs to see what they're
  approving under BFL 5 kap. Aggregate-only previews would harm the
  approval workflow.
- Audit company-current fields for PII (A.8.12, Art.25(1)) — vat_number,
  org_number, etc. are intentionally part of working memory; agents need
  them to make compliant booking decisions.
- Payload-size ADR reference (A.8.9) — the test comment already cites plan
  item 15 (Tool Search) as the long-term answer.
- mime_type classification label (CC7.2) — theoretical concern;
  ai_extraction_usage events are already operator-only.
- False positive: commitReverseEntry already has the closed-period check
  (V2.3); bot was hallucinating.

Tests: 3616/3616 pass. TypeScript build clean.

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

* fix(mcp,env): structured logger + description trim + env alias support

Two further follow-ups on PR #505:

1. resolvePeriodStatusForDate catch now uses the structured logger
   (createLogger from @/lib/logger) instead of console.warn. Three
   reviewers (compliance-swarm V16.1.1, ISO 27001 A.8.15, SOC 2 CC7.2)
   independently flagged that console.warn bypasses the centralized log
   aggregation pipeline used elsewhere, so systemic outages of the
   period-status resolver were invisible to the SIEM. log.warn now routes
   through the same sink as other server events.

2. Tool description for gnubok_reverse_journal_entry now routes the refund
   case explicitly to gnubok_credit_invoice. The Swedish accounting
   compliance bot flagged that the previous "cancelled credit invoice"
   example was ambiguous — a real credit invoice flow goes through
   gnubok_credit_invoice, not this tool. Description stays under 280 chars.

3. lib/init.ts: REQUIRED_EXTENSION_VARS now models each entry as a list of
   acceptable aliases instead of a single required name. The fallback in
   extensions/general/enable-banking/lib/jwt.ts already accepts the
   _PRODUCTION-suffixed variants (used by Vercel prod) as equivalent to
   the base names, but the env validator at boot didn't, so every cold
   start in prod warned about missing ENABLE_BANKING_APP_ID even though
   ENABLE_BANKING_APP_ID_PRODUCTION was set and the runtime was healthy.
   Each entry now satisfies if ANY listed alias is present; missing
   entries print all acceptable names so operators can pick either form.

Tests: 3616/3616 pass. TypeScript build clean.

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

* fix(mcp): staging tools reject locked_at periods too, not just is_closed

Swedish accounting compliance bot flagged that gnubok_reverse_journal_entry
and gnubok_correct_entry pre-flight checks only rejected closed periods —
locked-but-not-closed periods passed staging and only got rejected at
commit time. The commit-time gate was correct (both executors check
is_closed AND locked_at AND resolvePeriodStatusForDate), but the
staging-time signal was confusing: agent saw staged:true with
period_status:"locked" in the same envelope.

Now the staging pre-flight reads locked_at from the same inner-join and
rejects on either flag, matching the commit-time pattern. The error
message updated to "locked or closed" since both branches reach the same
throw. BFL 5 kap 5§ alignment is unchanged — both paths still block
mutations to locked/closed periods; only the layer at which the rejection
fires changes.

Findings pushed back (response in PR thread, not addressed here):
- companyId/mimeType in log.warn flagged as PII (overreach; tenant IDs
  are operational identifiers, not personal data, and the codebase logs
  them consistently elsewhere).
- HMAC-keyed file_name_hash instead of plain SHA-256 prefix (overreach;
  48 bits already addresses the immediate GDPR Art. 5(1)(f) concern).
- 'title' field in deadlines may contain PII (overreach; would require
  redacting every text field in every read resource).
- RLS regression test for voucher_sequences/deadlines (legitimate but
  pg-test scope; tracked for a follow-up sprint).
- Payload-size ADR record (comment already cites plan item 15).
- company-current data minimisation (already pushed back; agents need
  the fields for compliant booking decisions).
- Error message conflates "locked" and "closed" — minor UX nit not
  worth distinguishing here since the remediation step (unlock / omprövning)
  is the same for the user.
- reversal_date period attribution & voucher series integrity flagged as
  unverifiable from diff — false positives, both already handled by the
  engine (period_id from original, atomic voucher number).

Tests: 3616/3616 pass. TypeScript build clean.

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

* fix(mcp): address Swedish-accounting compliance round 4 — BFL invariant + VAT warning

Three legitimate findings from the swedish-accounting-compliance bot acted on
(out of five total; two pushed back as theoretical/false positive):

1. BFL 5 kap 5§ invariant assertion (finding 1). The engine guarantees that
   reverseEntry() posts the storno to original.fiscal_period_id (engine.ts:492
   — verified by reading the code), but the executor previously took that on
   faith. commitReverseEntry now asserts reversal.fiscal_period_id ===
   original.fiscal_period_id after the call and returns a 500 with an
   explicit "BFL invariant broken" error if the engine ever drifts. New
   executor test covers this. The reversal_date parameter is unchanged —
   it's used as the storno's entry_date (operational date), not for period
   attribution, per BFL practice (entry_date can differ from period_id's
   range for a rättelse made later).

2. resolvePeriodStatusForDate unhandled-rejection path (finding 2). Both
   commitCorrectEntry and commitReverseEntry now wrap the resolve call in
   try/catch, returning a clean Swedish 500 instead of letting the
   dispatcher surface a raw Postgres error message. Matches the
   log-and-degrade pattern already used at staging time in
   stagePendingOperation.

3. VAT-period warning in the reverse preview (finding 4 — swedish-vat).
   When the original entry contains 2610–2670 BAS accounts, the staged
   preview now includes a Swedish warnings[] field telling the approver
   that a storno is legally insufficient if the moms period has been
   filed with Skatteverket — they must use omprövning per ML 2023:200
   instead. Soft warning (not a hard block) since gnubok doesn't track
   per-VAT-period filing status today; the human decides at approval.

Pushed back:

- Finding 3 (TOCTOU between staging and commit on fiscal_period_id):
  posted entries are immutable per the enforce_journal_entry_immutability
  trigger (migration 20240101000017). fiscal_period_id can't change
  between staging and commit. Status change is already caught by the
  status !== 'posted' check.

- Finding 5 (migration 20260516060000 not wrapped in BEGIN/COMMIT):
  Supabase migration tooling runs each migration file in an implicit
  transaction. PostgreSQL DDL is transactional. The DROP/ADD pair is
  atomic in practice. The bot acknowledges this as low severity.

Tests: 3617/3617 pass (one new — BFL invariant assertion). Build 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>
2026-05-16 11:42:47 +02:00
..
2026-04-20 10:49:59 +02:00