diff --git a/.compliance/ropa.yaml b/.compliance/ropa.yaml index 46bbbffa..a43ef819 100644 --- a/.compliance/ropa.yaml +++ b/.compliance/ropa.yaml @@ -277,3 +277,44 @@ processing_activities: - not_used_for_model_training - rls_company_scoped - tls_to_bedrock + + - id: mcp.telemetry + name: MCP/agent-telemetri i event_log + purpose: >- + Varje MCP-verktygsanrop loggar metadata (verktygsnamn, felkod, + felmeddelande max 500 tecken, latens, aktör, session) och varje + skill-laddning loggar slug/tier till event_log. Syftet är + tillförlitlighetsanalys av agentgränssnittet: felfrekvens per verktyg, + korrelation mellan laddade skills och efterföljande fel, samt agenters + egenrapporterade feedback (agent.feedback). Inga verktygsargument + eller verktygsresultat persisteras. + lawful_basis: art_6_1_f # legitimate interest (service reliability/improvement) + special_category_basis: null + controller: gnubok-tenant + processor: supabase + data_subjects: + - business_owner + data_categories: + - user.unique_id # userId, actorId (API-nyckel-id), sessionId + - user.contact # actorLabel (användarvald API-nyckeletikett) + recipients: + - name: Supabase + country: EU + role: processor + international_transfers: + applicable: false + mechanism: null + note: EU-only processor; no third-country transfer. + retention: + # Differentiated TTL via /api/events/cleanup/cron: mcp.*/agent.*-rader + # behålls 180 dagar (felfrekvens-trender kräver mer än leveransfönstret); + # övriga event_log-rader (leveranshändelser) 30 dagar. + duration: 180d + basis: legitimate_interest_reliability + stored_in: + - event_log + security_measures: + - rls_user_scoped_select # event_log SELECT: auth.uid() = user_id + - service_role_only_writes + - error_message_truncated_500_chars + - no_tool_args_or_results_persisted diff --git a/.github/workflows/core-build.yml b/.github/workflows/core-build.yml index 608b8703..6e0ffa59 100644 --- a/.github/workflows/core-build.yml +++ b/.github/workflows/core-build.yml @@ -19,6 +19,12 @@ jobs: - name: Reset extensions config run: echo '{"extensions":[]}' > extensions.config.json - run: npm run setup:extensions + - name: Lint ratchet (no new ESLint errors) + # `npm run lint` was never wired into CI, so ~60 legacy errors + # accumulated. This ratchet (sibling of check:guards) fails only when + # a PR ADDS an error beyond scripts/checks/eslint-baseline.json; the + # baseline ratchets down as legacy errors get fixed. + run: npm run check:lint - run: npm run build - run: npm test - name: Antipattern ratchet (no new MFA-bypassing routes / naive öre-rounding) diff --git a/.github/workflows/test-pg-real.yml b/.github/workflows/test-pg-real.yml index c54ce984..43c8a775 100644 --- a/.github/workflows/test-pg-real.yml +++ b/.github/workflows/test-pg-real.yml @@ -7,6 +7,25 @@ concurrency: cancel-in-progress: true jobs: + coverage-gate: + # Enforces the database.md rule: a migration touching a trigger/RPC/RLS/ + # DEFERRABLE must come with a *.pg.test.ts change. Previously instruction- + # only. Escape hatch: `-- pg-test: covered-by ` / `-- pg-test: skip + # ()` comments inside the migration. + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v4 + with: + # Full history so the merge-base with the PR base branch exists. + fetch-depth: 0 + - uses: actions/setup-node@v4 + with: + node-version: 20 + - name: Require pg-real coverage for trigger/RPC/RLS migrations + env: + PG_GATE_BASE: origin/${{ github.base_ref }} + run: node scripts/check-pg-test-coverage.mjs + pg-real: runs-on: ubuntu-latest diff --git a/app/api/events/cleanup/cron/__tests__/route.test.ts b/app/api/events/cleanup/cron/__tests__/route.test.ts new file mode 100644 index 00000000..e17138d7 --- /dev/null +++ b/app/api/events/cleanup/cron/__tests__/route.test.ts @@ -0,0 +1,108 @@ +/** + * Tests for the event_log cleanup cron's differentiated retention: + * delivery events at 30 days, agent telemetry (mcp.*, agent.*) at 180 days. + */ +import { describe, it, expect, vi, beforeEach } from 'vitest' + +vi.mock('@/lib/auth/cron', () => ({ + verifyCronSecret: vi.fn(() => null), +})) + +interface FilterCall { + method: string + args: unknown[] +} + +interface DeleteCapture { + filters: FilterCall[] +} + +const deleteCalls: DeleteCapture[] = [] +let deleteResults: Array<{ error: unknown; count: number | null }> = [] + +vi.mock('@/lib/supabase/server', () => ({ + createServiceClient: vi.fn(() => ({ + from: vi.fn(() => { + const capture: DeleteCapture = { filters: [] } + deleteCalls.push(capture) + const result = deleteResults.shift() ?? { error: null, count: 0 } + const chain: Record = {} + chain.delete = vi.fn(() => chain) + chain.lt = vi.fn((...args: unknown[]) => { + capture.filters.push({ method: 'lt', args }) + return chain + }) + chain.not = vi.fn((...args: unknown[]) => { + capture.filters.push({ method: 'not', args }) + return chain + }) + // Thenable — awaiting the builder resolves the queued result. + chain.then = (resolve: (v: unknown) => unknown) => Promise.resolve(result).then(resolve) + return chain + }), + })), +})) + +import { GET } from '../route' + +function cronRequest(): Request { + return new Request('http://localhost:3000/api/events/cleanup/cron') +} + +function daysAgo(iso: string): number { + return (Date.now() - new Date(iso).getTime()) / 86_400_000 +} + +beforeEach(() => { + vi.clearAllMocks() + deleteCalls.length = 0 + deleteResults = [] +}) + +describe('GET /api/events/cleanup/cron', () => { + it('runs two delete passes: delivery at 30 days (telemetry excluded), everything at 180', async () => { + deleteResults = [ + { error: null, count: 12 }, + { error: null, count: 3 }, + ] + + const response = await GET(cronRequest()) + const json = await response.json() + + expect(json).toEqual({ + success: true, + deleted: 15, + deletedDelivery: 12, + deletedTelemetry: 3, + }) + + expect(deleteCalls).toHaveLength(2) + + // Pass 1: 30-day cutoff + telemetry exclusion filters. + const pass1 = deleteCalls[0] + const lt1 = pass1.filters.find((f) => f.method === 'lt')! + expect(lt1.args[0]).toBe('created_at') + expect(daysAgo(lt1.args[1] as string)).toBeCloseTo(30, 0) + const notFilters = pass1.filters.filter((f) => f.method === 'not') + expect(notFilters.map((f) => f.args)).toEqual([ + ['event_type', 'like', 'mcp.%'], + ['event_type', 'like', 'agent.%'], + ]) + + // Pass 2: 180-day cutoff, no exclusions — sweeps the telemetry rows. + const pass2 = deleteCalls[1] + const lt2 = pass2.filters.find((f) => f.method === 'lt')! + expect(daysAgo(lt2.args[1] as string)).toBeCloseTo(180, 0) + expect(pass2.filters.filter((f) => f.method === 'not')).toHaveLength(0) + }) + + it('short-circuits with an error envelope when the delivery pass fails', async () => { + deleteResults = [{ error: { message: 'boom', code: 'XX000' }, count: null }] + + const response = await GET(cronRequest()) + + expect(response.status).toBeGreaterThanOrEqual(500) + // The 180-day pass never ran. + expect(deleteCalls).toHaveLength(1) + }) +}) diff --git a/app/api/events/cleanup/cron/route.ts b/app/api/events/cleanup/cron/route.ts index 7be93af5..77c87834 100644 --- a/app/api/events/cleanup/cron/route.ts +++ b/app/api/events/cleanup/cron/route.ts @@ -5,26 +5,63 @@ import { errorResponse } from '@/lib/errors/get-structured-error' /** * GET /api/events/cleanup/cron — daily 02:00 UTC. - * Removes event_log rows older than 30 days. + * + * Differentiated retention: + * - Delivery events (invoice.created, transaction.synced, …): 30 days. They + * exist for external automation polling (n8n/Make/Zapier) and go stale fast. + * - Agent telemetry (mcp.*, agent.*): 180 days. Error-rate trends and + * skill-load correlation need more than one month of signal — a 30-day + * window made it impossible to tell whether a tool or skill change actually + * moved failure rates. + * + * Retention is declared in .compliance/ropa.yaml (id: mcp.telemetry). */ +const DELIVERY_RETENTION_DAYS = 30 +const TELEMETRY_RETENTION_DAYS = 180 + export const GET = withCronContext('cron.events_cleanup', async (_request, ctx) => { const supabase = createServiceClient() - const cutoff = new Date() - cutoff.setDate(cutoff.getDate() - 30) + const deliveryCutoff = new Date() + deliveryCutoff.setDate(deliveryCutoff.getDate() - DELIVERY_RETENTION_DAYS) + const telemetryCutoff = new Date() + telemetryCutoff.setDate(telemetryCutoff.getDate() - TELEMETRY_RETENTION_DAYS) - const { error, count } = await supabase + // Pass 1: delivery events past 30 days. Telemetry (mcp.*, agent.*) is + // excluded here and swept by the 180-day pass below. + const { error: deliveryError, count: deliveryCount } = await supabase .from('event_log') .delete({ count: 'exact' }) - .lt('created_at', cutoff.toISOString()) + .lt('created_at', deliveryCutoff.toISOString()) + .not('event_type', 'like', 'mcp.%') + .not('event_type', 'like', 'agent.%') - if (error) { - ctx.log.error('event log cleanup failed', error) - return errorResponse(error, ctx.log, { requestId: ctx.requestId }) + if (deliveryError) { + ctx.log.error('event log delivery cleanup failed', deliveryError) + return errorResponse(deliveryError, ctx.log, { requestId: ctx.requestId }) } - const deleted = count ?? 0 - ctx.log.info('event log cleanup summary', { deleted, cutoff: cutoff.toISOString() }) + // Pass 2: everything past 180 days — catches the telemetry rows pass 1 skipped. + const { error: telemetryError, count: telemetryCount } = await supabase + .from('event_log') + .delete({ count: 'exact' }) + .lt('created_at', telemetryCutoff.toISOString()) - return NextResponse.json({ success: true, deleted }) + if (telemetryError) { + ctx.log.error('event log telemetry cleanup failed', telemetryError) + return errorResponse(telemetryError, ctx.log, { requestId: ctx.requestId }) + } + + const deletedDelivery = deliveryCount ?? 0 + const deletedTelemetry = telemetryCount ?? 0 + const deleted = deletedDelivery + deletedTelemetry + ctx.log.info('event log cleanup summary', { + deleted, + deletedDelivery, + deletedTelemetry, + deliveryCutoff: deliveryCutoff.toISOString(), + telemetryCutoff: telemetryCutoff.toISOString(), + }) + + return NextResponse.json({ success: true, deleted, deletedDelivery, deletedTelemetry }) }) diff --git a/extensions/general/mcp-server/__tests__/pending-operations-tools.test.ts b/extensions/general/mcp-server/__tests__/pending-operations-tools.test.ts index 37a5ba2a..55b54ec9 100644 --- a/extensions/general/mcp-server/__tests__/pending-operations-tools.test.ts +++ b/extensions/general/mcp-server/__tests__/pending-operations-tools.test.ts @@ -107,12 +107,40 @@ describe('gnubok_approve_pending_operation', () => { // commit options always include commitMethod; userEmail is added when // the supabase mock supports auth.admin.getUserById (it doesn't here, so // the resolution silently fails and we fall back to just commitMethod). - expect(commitSpy.mock.calls[0][4]).toMatchObject({ commitMethod: 'user_accept' }) + // An api_key actor records 'api_key' in the immutable layer — MCP-relayed + // acknowledgment, not a first-party human session (vision §8 P0-1). + expect(commitSpy.mock.calls[0][4]).toMatchObject({ commitMethod: 'api_key' }) expect(result.status).toBe('committed') expect(result.operation_id).toBe('op-1') expect(result.data?.invoice_id).toBe('inv-1') }) + // No 'mcp_oauth' row: handleMcpRequest hardcodes actor.type='api_key' for + // ALL MCP traffic (the OAuth connector's access_token is a minted API key), + // so 'api_key' is the only agent-credential value a live request produces. + it.each([ + { actorType: 'api_key', expected: 'api_key' }, + { actorType: 'user', expected: 'user_accept' }, + ] as const)( + 'records commit_method=$expected when the approving actor is $actorType', + async ({ actorType, expected }) => { + const { supabase, enqueue } = createQueuedMockSupabase() + const op = { id: 'op-1', operation_type: 'create_invoice', company_id: 'company-1', status: 'pending', risk_level: 'medium', params: {} } + enqueue({ data: op, error: null }) // fetch + commitSpy.mockResolvedValue({ status: 'committed' }) + + await approveTool.execute( + { operation_id: 'op-1' }, + 'company-1', + 'user-1', + supabase as never, + { type: actorType } + ) + + expect(commitSpy.mock.calls[0][4]).toMatchObject({ commitMethod: expected }) + } + ) + it('refuses to approve a risk_level=high op without confirmed=true', async () => { const { supabase, enqueue } = createQueuedMockSupabase() const op = { diff --git a/extensions/general/mcp-server/__tests__/telemetry.test.ts b/extensions/general/mcp-server/__tests__/telemetry.test.ts index 1abc32b0..0c2d16d4 100644 --- a/extensions/general/mcp-server/__tests__/telemetry.test.ts +++ b/extensions/general/mcp-server/__tests__/telemetry.test.ts @@ -97,6 +97,7 @@ interface ToolCalledPayload { isError: boolean errorCode: string | null errorKind: 'execution' | 'scope_denied' | 'unknown_tool' | null + errorMessage: string | null requestId: string | number | null userId: string companyId: string @@ -177,6 +178,7 @@ describe('mcp.tool_called telemetry', () => { expect(event.isError).toBe(false) expect(event.errorCode).toBeNull() expect(event.errorKind).toBeNull() + expect(event.errorMessage).toBeNull() expect(event.actorType).toBe('api_key') expect(event.actorId).toBe('key-1') expect(event.actorLabel).toBe('Test Key') @@ -206,6 +208,9 @@ describe('mcp.tool_called telemetry', () => { expect(event.isError).toBe(true) expect(event.errorKind).toBe('scope_denied') expect(event.errorCode).toBe('INSUFFICIENT_SCOPE') + // The human message rides along for failure clustering. + expect(typeof event.errorMessage).toBe('string') + expect((event.errorMessage as string).length).toBeGreaterThan(0) // Scope denial exits before tool.execute() runs. expect(event.latencyMs).toBe(0) }) @@ -224,6 +229,9 @@ describe('mcp.tool_called telemetry', () => { expect(event.isError).toBe(true) expect(event.errorKind).toBe('unknown_tool') expect(event.errorCode).toBe('UNKNOWN_TOOL') + // Short deterministic message — NOT the full available-tools list the + // client response carries (that would blow the truncation budget). + expect(event.errorMessage).toBe('Unknown tool: "gnubok_does_not_exist"') expect(event.latencyMs).toBe(0) }) @@ -245,6 +253,11 @@ describe('mcp.tool_called telemetry', () => { expect(event.isError).toBe(true) expect(event.errorKind).toBe('execution') expect(event.errorCode).toBeTruthy() + // The structured error's human message is captured and bounded at 500 + // chars — the raw material for clustering execution failures into gotchas. + expect(typeof event.errorMessage).toBe('string') + expect((event.errorMessage as string).length).toBeGreaterThan(0) + expect((event.errorMessage as string).length).toBeLessThanOrEqual(500) // Execution path measures real latency, even if the tool exits quickly. expect(event.latencyMs).toBeGreaterThanOrEqual(0) }) @@ -356,8 +369,67 @@ describe('mcp.resource_read telemetry', () => { }) }) +describe('mcp.skill_loaded telemetry', () => { + beforeEach(() => { + vi.clearAllMocks() + eventBus.clear() + }) + + it('emits on every successful load — alongside mcp.workflow_started for workflow tier', async () => { + const skillLoadedPromise = new Promise>((resolve) => { + const off = eventBus.on('mcp.skill_loaded', (payload) => { + off() + resolve(payload as Record) + }) + }) + const workflowStartedPromise = new Promise>((resolve) => { + const off = eventBus.on('mcp.workflow_started', (payload) => { + off() + resolve(payload as Record) + }) + }) + + await handleMcpRequest( + mcpRequest('tools/call', { + name: 'gnubok_load_skill', + arguments: { slug: 'month-end-close' }, + }) + ) + + const event = await skillLoadedPromise + expect(event.slug).toBe('month-end-close') + expect(event.tier).toBe('workflow') + expect(event.actorType).toBe('api_key') + expect(event.actorId).toBe('key-1') + expect(event.userId).toBe('user-1') + expect(event.companyId).toBe('company-1') + + // The pre-existing workflow-funnel event still fires for workflow tier. + const wf = await workflowStartedPromise + expect(wf.slug).toBe('month-end-close') + }) + + it('does not emit when the slug is unknown (load throws before emission)', async () => { + const seen: unknown[] = [] + eventBus.on('mcp.skill_loaded', (payload) => { + seen.push(payload) + }) + + await handleMcpRequest( + mcpRequest('tools/call', { + name: 'gnubok_load_skill', + arguments: { slug: 'nope-not-real' }, + }) + ) + // Flush microtasks — emission is fire-and-forget. + await new Promise((resolve) => setTimeout(resolve, 0)) + + expect(seen).toHaveLength(0) + }) +}) + describe('event_log persistence registration', () => { - it('includes all three MCP telemetry events in the persisted event types', async () => { + it('includes all MCP telemetry events in the persisted event types', async () => { // Read the file as text — the constant is module-private. This is a // deliberate string-level guard so a future refactor that drops one // of the events from the list trips the test. @@ -368,5 +440,6 @@ describe('event_log persistence registration', () => { expect(text).toMatch(/'mcp\.tool_called'/) expect(text).toMatch(/'mcp\.tools_list_called'/) expect(text).toMatch(/'mcp\.resource_read'/) + expect(text).toMatch(/'mcp\.skill_loaded'/) }) }) diff --git a/extensions/general/mcp-server/server.ts b/extensions/general/mcp-server/server.ts index ea4f540b..e5846824 100644 --- a/extensions/general/mcp-server/server.ts +++ b/extensions/general/mcp-server/server.ts @@ -1678,6 +1678,12 @@ export const tools: McpTool[] = [ const available = all.map((s) => s.slug).join(', ') throw new Error(`Skill not found: "${slug}". Available skills: ${available}`) } + // Every load, every tier — records which skill/atom bodies agents + // actually pull (mcp.skill_loaded). Without this, "which atom was + // loaded" is unanswerable and atom effectiveness can't be measured. + if (actor) { + emitSkillLoaded({ slug: skill.slug, tier: skill.tier, actor, userId, companyId }) + } // Workflow-tier skills are the closed-form processes (month-end-close, // year-end-close, payroll-monthly). Loading one is a strong signal the // agent is starting that workflow — emit so we can track completion @@ -7516,7 +7522,7 @@ export const tools: McpTool[] = [ // entries, what balances) and a broken/unbalanced file is rejected HERE, // not after they approve a blind byte count. commitImportSie re-parses on // commit (defense-in-depth — the staged string could be tampered). - const { parseSIEFile, validateSIEFile } = await import('@/lib/import/sie-parser') + const { parseSIEFile, validateSIEFile, getEffectiveOpeningBalances } = await import('@/lib/import/sie-parser') let parsed try { parsed = parseSIEFile(fileContent) @@ -7528,7 +7534,10 @@ export const tools: McpTool[] = [ throw new Error(`SIE-filen är ogiltig och importeras inte: ${validation.errors.join('; ')}`) } - const ibCurrent = parsed.openingBalances.filter((b) => b.yearIndex === 0) + // Effective set: explicit #IB 0, or IB derived from #UB -1 when the + // source system exports none (issue #675) — so the approver sees the + // real IB total and UB-1-only files pass the coverage check below. + const ibCurrent = getEffectiveOpeningBalances(parsed).balances const ibTotal = Math.round(ibCurrent.reduce((s, b) => s + b.amount, 0) * 100) / 100 // Mapping-coverage check. The executor's per-voucher loop silently @@ -8591,12 +8600,34 @@ export const tools: McpTool[] = [ log.warn('Failed to resolve user email for MCP approval', { userId, err }) } + // commit_method provenance (agent_first_vision.md §8 P0-1): MCP + // approvals are relayed through an agent credential — record that in + // the immutable layer instead of claiming 'user_accept'. The positive + // acknowledgment (confirmed=true for high risk) is agent-attested, not + // a first-party human session; an auditor reading the GL can now tell + // the difference (BFNAR 2013:2 kap 8 behandlingshistorik). + // + // ALL MCP traffic authenticates as an api_key actor — the claude.ai + // OAuth connector's access_token is itself a minted gnubok_sk_ key + // (app/api/mcp-oauth/token/route.ts), indistinguishable from the + // bridge at this layer — so 'api_key' is the truthful value for every + // path through this handler. 'agent' (also in the CHECK) is reserved + // for first-party agent surfaces (e.g. in-app agent chat) once they + // commit through this layer with a distinguishable actor type. + // + // Note: commitPendingOperation currently threads commitMethod into the + // journal only for create_voucher ops (pre-existing); other operation + // types keep their per-handler defaults, with this approval's actor + // recorded in processing_history below either way. + const commitMethod = + actor?.type === 'api_key' ? ('api_key' as const) : ('user_accept' as const) + const result = await commitPendingOperation( supabase, userId, companyId, operation, - { commitMethod: 'user_accept', ...(userEmail ? { userEmail } : {}) } + { commitMethod, ...(userEmail ? { userEmail } : {}) } ) // Audit the MCP-initiated approval. Failure must not break the user @@ -8613,7 +8644,7 @@ export const tools: McpTool[] = [ operation_type: operation.operation_type, risk_level: operation.risk_level, outcome: result.status, - commit_method: 'user_accept', + commit_method: commitMethod, channel: 'mcp', confirmed: args.confirmed === true, }, @@ -8902,6 +8933,7 @@ function emitToolCallTelemetry(payload: { isError: boolean errorCode: string | null errorKind: 'execution' | 'scope_denied' | 'unknown_tool' | null + errorMessage: string | null requestId: string | number | null userId: string companyId: string @@ -8920,6 +8952,10 @@ function emitToolCallTelemetry(payload: { isError: payload.isError, errorCode: payload.errorCode, errorKind: payload.errorKind, + // Truncated: domain error messages are short, but unknown-tool / + // validation messages can embed long lists. 500 chars is plenty for + // clustering failures into gotchas without bloating event_log rows. + errorMessage: payload.errorMessage ? payload.errorMessage.slice(0, 500) : null, requestId: payload.requestId, userId: payload.userId, companyId: payload.companyId, @@ -9059,6 +9095,36 @@ function checkAndEmitNextHintFollowed( .catch((err) => console.error('[mcp] next_hint_followed emit failed:', err)) } +/** + * Fire-and-forget telemetry for every successful gnubok_load_skill, all tiers. + * Unlike mcp.workflow_started (workflow tier only), this records WHICH skill + * or atom body the agent pulled — the denominator for correlating a loaded + * atom with downstream tool-error rates. + */ +function emitSkillLoaded(payload: { + slug: string + tier: 'workflow' | 'horizontal' | 'vertical' | 'modifier' + actor: ActorContext + userId: string + companyId: string +}): void { + void eventBus + .emit({ + type: 'mcp.skill_loaded', + payload: { + slug: payload.slug, + tier: payload.tier, + sessionId: payload.actor.sessionId ?? null, + actorType: payload.actor.type, + actorId: payload.actor.id ?? null, + actorLabel: payload.actor.label ?? null, + userId: payload.userId, + companyId: payload.companyId, + }, + }) + .catch((err) => console.error('[mcp] skill_loaded emit failed:', err)) +} + /** Fire-and-forget telemetry for workflow lifecycle. */ function emitWorkflowStarted(payload: { slug: string @@ -9259,6 +9325,10 @@ export async function handleMcpRequest(request: Request): Promise { isError: true, errorCode: 'UNKNOWN_TOOL', errorKind: 'unknown_tool', + // Just the requested name — the full available-tools list returned + // to the client would blow the truncation budget without adding + // analytical signal. + errorMessage: `Unknown tool: "${toolName}"`, requestId: id ?? null, userId, companyId, @@ -9285,6 +9355,7 @@ export async function handleMcpRequest(request: Request): Promise { isError: true, errorCode: scopeError.error.code, errorKind: 'scope_denied', + errorMessage: scopeError.error.message_sv, requestId: id ?? null, userId, companyId, @@ -9347,6 +9418,7 @@ export async function handleMcpRequest(request: Request): Promise { isError: false, errorCode: null, errorKind: null, + errorMessage: null, requestId: id ?? null, userId, companyId, @@ -9364,6 +9436,10 @@ export async function handleMcpRequest(request: Request): Promise { isError: true, errorCode: structured.error.code, errorKind: 'execution', + // message_sv is the canonical domain message ("Verifikationen + // balanserar inte", "Perioden är låst", …) — the text worth + // clustering when mining failures for gotchas. + errorMessage: structured.error.message_sv, requestId: id ?? null, userId, companyId, diff --git a/lib/events/handlers/event-log-handler.ts b/lib/events/handlers/event-log-handler.ts index f38768ef..651cad29 100644 --- a/lib/events/handlers/event-log-handler.ts +++ b/lib/events/handlers/event-log-handler.ts @@ -36,7 +36,8 @@ const PERSISTED_EVENT_TYPES: CoreEventType[] = [ 'supplier_invoice.match_confirmed', 'supplier_invoice.confirmed', // MCP telemetry — every tool invocation, tools/list call, and resources/read. - // Lightweight metadata only; 30-day TTL on event_log bounds the volume. + // Lightweight metadata only. mcp.*/agent.* rows are retained 180 days by the + // cleanup cron (error-rate trends need more than the 30-day delivery window). 'mcp.tool_called', 'mcp.tools_list_called', 'mcp.resource_read', @@ -46,6 +47,10 @@ const PERSISTED_EVENT_TYPES: CoreEventType[] = [ 'mcp.workflow_started', 'mcp.workflow_completed', 'mcp.next_hint_followed', + // Every successful gnubok_load_skill, all tiers — which atoms agents + // actually load. Joined against mcp.tool_called error rates to measure + // whether a loaded atom helps or hurts. + 'mcp.skill_loaded', // Agent self-reported feedback — surfaces "this tool was missing", "this // description was wrong", etc. Quarterly review → roadmap. 'agent.feedback', diff --git a/lib/events/types.ts b/lib/events/types.ts index c71a9957..2bc600d1 100644 --- a/lib/events/types.ts +++ b/lib/events/types.ts @@ -138,7 +138,8 @@ export type CoreEvent = | { type: 'company.deleted'; payload: { companyId: string; userId: string; archivedAt: string } } | { type: 'account.deleted'; payload: { userId: string; deletedAt: string } } // MCP telemetry — fired from the MCP dispatcher. - // Persisted to event_log (30-day TTL) for hot-tool / error-rate / latency analytics. + // Persisted to event_log (180-day TTL for mcp.*/agent.* rows, vs 30 days for + // delivery events) for hot-tool / error-rate / latency analytics. // Intentionally lightweight: no args, no result body — only metadata. | { type: 'mcp.tool_called'; payload: { tool: string // e.g. 'gnubok_create_invoice' @@ -151,6 +152,9 @@ export type CoreEvent = isError: boolean // matches the JSON-RPC tool-result isError flag returned to the client errorCode: string | null // structured error code from tool-result.toToolError when applicable errorKind: 'execution' | 'scope_denied' | 'unknown_tool' | null + errorMessage: string | null // human-readable error message (truncated to 500 chars), null on success. + // Raw material for clustering real agent failures into curated gotchas — + // errorCode alone can't distinguish "period locked" from "unbalanced". requestId: string | number | null // JSON-RPC request id (helps correlate with client-side logs) userId: string companyId: string @@ -212,6 +216,21 @@ export type CoreEvent = userId: string companyId: string }} + // Fires on EVERY successful gnubok_load_skill — all tiers, unlike + // mcp.workflow_started which fires only for workflow-tier skills. Records + // WHICH skill/atom bodies agents actually pull, the denominator needed to + // correlate a loaded atom with downstream tool-error rates (a skill can + // make the model worse — measure, don't assume). + | { type: 'mcp.skill_loaded'; payload: { + slug: string // e.g. 'modifier/holding-ab', 'month-end-close' + tier: 'workflow' | 'horizontal' | 'vertical' | 'modifier' + sessionId: string | null + actorType: 'user' | 'api_key' | 'mcp_oauth' | 'cron' + actorId: string | null + actorLabel: string | null + userId: string + companyId: string + }} // Fires when the agent's next tool call matches the previous response's // nextHint.tool — measures whether `next` hints are actually followed. // Computed dispatcher-side by comparing the last response shape to the diff --git a/lib/import/__tests__/sie-import-coverage.test.ts b/lib/import/__tests__/sie-import-coverage.test.ts index 54d371f6..2f9e2278 100644 --- a/lib/import/__tests__/sie-import-coverage.test.ts +++ b/lib/import/__tests__/sie-import-coverage.test.ts @@ -232,3 +232,108 @@ describe('finalizeImportRecord — 0-entry downgrade', () => { expect(result.success).toBe(true) }) }) + +describe('executeSIEImport — coverage check with derived IB (issue #675)', () => { + // SIE type 1/2-style file: no vouchers, no #IB 0 — only #UB -1. The + // current-year IB must be derived from #UB -1, and the derived accounts + // must feed the coverage guard (before the fix this set was empty, so the + // guard never inspected UB-1-only files at all). + function makeUb1OnlyFile(): ParsedSIEFile { + return makeParsedFile({ + openingBalances: [], + closingBalances: [ + { yearIndex: -1, account: '1930', amount: 37400.78 }, + { yearIndex: -1, account: '2010', amount: -37400.78 }, + ], + vouchers: [], + stats: { + totalAccounts: 2, + totalVouchers: 0, + totalTransactionLines: 0, + fiscalYearStart: '2024-01-01', + fiscalYearEnd: '2024-12-31', + }, + }) + } + + it('refuses when mappings cover none of the derived IB accounts', async () => { + const { supabase } = createQueuedMockSupabase() + + const result = await executeSIEImport( + supabase as unknown as SupabaseClient, + 'company-1', + 'user-1', + makeUb1OnlyFile(), + [makeMapping('9999', '9999')], + { + filename: 'ub1-only.se', + fileContent: '#dummy', + createFiscalPeriod: false, + importOpeningBalances: true, + importTransactions: true, + }, + ) + + expect(result.success).toBe(false) + expect(result.importId).toBeNull() + expect(result.errors.join(' ')).toMatch(/täcker inga konton/i) + }) + + it('passes the coverage guard when mappings cover the derived IB accounts', async () => { + const { supabase, enqueueMany } = createQueuedMockSupabase() + // Past the guard the flow proceeds: dup check → stale cleanup → pending + // record insert → chart fetch → period-dup check → find fiscal period + // (null → clean stop with a NON-coverage error, which is all this test + // needs to prove). + enqueueMany([ + { data: null }, // checkDuplicateImport + { data: null }, // cleanupStaleImportRecords delete + { data: { id: 'imp-1' } }, // createPendingImportRecord insert + { data: [] }, // syncMappedAccounts chart fetch + { data: null }, // chart insert (missing accounts) + { data: null }, // checkDuplicatePeriodImport + { data: null }, // find existing fiscal period → stops here + ]) + + const result = await executeSIEImport( + supabase as unknown as SupabaseClient, + 'company-1', + 'user-1', + makeUb1OnlyFile(), + [makeMapping('1930', '1930'), makeMapping('2010', '2010')], + { + filename: 'ub1-only.se', + fileContent: '#dummy', + createFiscalPeriod: false, + importOpeningBalances: true, + importTransactions: true, + }, + ) + + expect(result.errors.join(' ')).not.toMatch(/täcker inga konton/i) + expect(result.errors.join(' ')).toMatch(/No matching fiscal period found/i) + }) + + it('skips the IB accounts in the guard when importOpeningBalances is false', async () => { + const { supabase } = createQueuedMockSupabase() + + const result = await executeSIEImport( + supabase as unknown as SupabaseClient, + 'company-1', + 'user-1', + makeUb1OnlyFile(), + [makeMapping('9999', '9999')], + { + filename: 'ub1-only.se', + fileContent: '#dummy', + createFiscalPeriod: false, + importOpeningBalances: false, + importTransactions: true, + }, + ) + + // No vouchers + IB import disabled → sourceAccountsInFile is empty and + // the guard does not fire (existing semantics preserved). + expect(result.errors.join(' ')).not.toMatch(/täcker inga konton/i) + }) +}) diff --git a/lib/import/__tests__/sie-import-derived-ib.test.ts b/lib/import/__tests__/sie-import-derived-ib.test.ts new file mode 100644 index 00000000..cad6292c --- /dev/null +++ b/lib/import/__tests__/sie-import-derived-ib.test.ts @@ -0,0 +1,276 @@ +/** + * Full-flow regression suite for issue #675. + * + * Some systems export SIE files without current-year #IB 0 records — the + * opening balances exist only implicitly via the SIE continuity invariant + * IB(year 0) = UB(year -1). executeSIEImport must derive the IB from the + * file's #UB -1 records, create a real opening-balance entry whose voucher + * text documents the derivation, and warn the user. + * + * The make-or-break line is the gate in executeSIEImport: it must open on + * the EFFECTIVE opening balances (getEffectiveOpeningBalances), not on raw + * parsed.openingBalances — the raw set is empty for these files. + */ +import { describe, it, expect, vi, beforeEach } from 'vitest' +import { executeSIEImport } from '../sie-import' +import { createJournalEntry } from '@/lib/bookkeeping/engine' +import type { ParsedSIEFile, AccountMapping } from '../types' +import type { SupabaseClient } from '@supabase/supabase-js' + +vi.mock('@/lib/bookkeeping/engine', () => ({ + createJournalEntry: vi.fn(async () => ({ id: 'ob-entry-1' })), + reverseEntry: vi.fn(), +})) + +// --- Helpers --- + +type QueuedResult = { data?: unknown; error?: unknown; count?: number | null } + +/** + * Table-routing supabase mock: each table has its own FIFO of results + * (consumed per .from(table) call), falling back to { data: null, error: + * null } when the queue is empty. Order-independent across tables, so the + * mock doesn't break when an unrelated query is added elsewhere in the flow. + */ +function buildRoutingSupabase(tableQueues: Record) { + const queues = new Map( + Object.entries(tableQueues).map(([k, v]) => [k, [...v]]) + ) + + const makeChain = (result: { data: unknown; error: unknown; count: number | null }): unknown => { + const handler: ProxyHandler = { + get(_target, prop) { + if (prop === 'then') { + return (resolve: (v: unknown) => void) => resolve(result) + } + return (..._args: unknown[]) => makeChain(result) + }, + } + return new Proxy({}, handler) + } + + const supabase = { + from: (table: string) => { + const next = queues.get(table)?.shift() ?? {} + return makeChain({ + data: next.data ?? null, + error: next.error ?? null, + count: next.count ?? null, + }) + }, + rpc: async () => ({ data: null, error: null }), + storage: { + from: () => ({ upload: async () => ({ error: null }) }), + }, + } + + return supabase as unknown as SupabaseClient +} + +function makeParsedFile(overrides?: Partial): ParsedSIEFile { + return { + header: { + sieType: 4, + flagga: 0, + program: 'TestProg', + programVersion: '1.0', + generatedDate: '2024-01-01', + format: 'PC8', + companyName: 'Continuity AB', + orgNumber: '5566778899', + address: null, + fiscalYears: [ + { yearIndex: 0, start: '2024-01-01', end: '2024-12-31' }, + { yearIndex: -1, start: '2023-01-01', end: '2023-12-31' }, + ], + currency: 'SEK', + kontoPlanType: null, + }, + accounts: [ + { number: '1930', name: 'Företagskonto' }, + { number: '2010', name: 'Eget kapital' }, + ], + // Issue #675 shape: no #IB 0 at all — only prior-year IB/UB and current UB. + openingBalances: [{ yearIndex: -1, account: '1930', amount: 9483.08 }], + closingBalances: [ + { yearIndex: -1, account: '1930', amount: 37400.78 }, + { yearIndex: -1, account: '2010', amount: -37400.78 }, + { yearIndex: 0, account: '1930', amount: 160406.0 }, + { yearIndex: 0, account: '2010', amount: -160406.0 }, + ], + resultBalances: [], + vouchers: [], + issues: [], + stats: { + totalAccounts: 2, + totalVouchers: 0, + totalTransactionLines: 0, + fiscalYearStart: '2024-01-01', + fiscalYearEnd: '2024-12-31', + }, + ...overrides, + } +} + +function makeMapping(source: string, target: string): AccountMapping { + return { + sourceAccount: source, + sourceName: `Account ${source}`, + targetAccount: target, + targetName: `Target ${target}`, + confidence: 1, + matchType: 'exact', + isOverride: false, + } +} + +function standardQueues() { + return { + sie_imports: [ + { data: null }, // checkDuplicateImport — no duplicate + {}, // cleanupStaleImportRecords delete + { data: { id: 'imp-1' } }, // createPendingImportRecord insert + { data: null }, // checkDuplicatePeriodImport — no duplicate + // finalizeImportRecord updates ride on defaults + ], + chart_of_accounts: [ + { + // syncMappedAccounts paged fetch — both accounts already exist + data: [ + { account_number: '1930', account_name: 'Företagskonto' }, + { account_number: '2010', account_name: 'Eget kapital' }, + ], + }, + ], + fiscal_periods: [ + { data: { id: 'fp-1' } }, // find existing fiscal period + { data: { opening_balances_set: false, opening_balance_entry_id: null } }, // IB-block check + // link update + resync next-period lookup ride on defaults (null) + ], + journal_entries: [ + { count: 0 }, // companyHasPriorActivity — first-ever import + ], + } +} + +const standardOptions = { + filename: 'continuity.se', + fileContent: '#dummy', + createFiscalPeriod: false, + importOpeningBalances: true, + importTransactions: true, + updateAccountNames: false, +} + +const standardMappings = [makeMapping('1930', '1930'), makeMapping('2010', '2010')] + +// --- Tests --- + +describe('executeSIEImport — derived IB from #UB -1 (issue #675)', () => { + beforeEach(() => { + vi.clearAllMocks() + }) + + it('creates the opening-balance entry from #UB -1 when #IB 0 is missing', async () => { + const supabase = buildRoutingSupabase(standardQueues()) + + const result = await executeSIEImport( + supabase, + 'company-1', + 'user-1', + makeParsedFile(), + standardMappings, + standardOptions, + ) + + expect(result.errors).toEqual([]) + expect(result.success).toBe(true) + expect(result.openingBalanceEntryId).toBe('ob-entry-1') + expect(result.journalEntriesCreated).toBe(1) + expect(result.warnings.join(' ')).toMatch(/kontinuitetsprincipen/) + + expect(createJournalEntry).toHaveBeenCalledTimes(1) + const input = vi.mocked(createJournalEntry).mock.calls[0][3] + expect(input.source_type).toBe('opening_balance') + expect(input.fiscal_period_id).toBe('fp-1') + expect(input.entry_date).toBe('2024-01-01') + expect(input.description).toBe( + 'Ingående balanser från SIE-import (härledda från föregående års utgående balans)' + ) + expect(input.lines).toEqual([ + { account_number: '1930', debit_amount: 37400.78, credit_amount: 0, line_description: 'IB 1930' }, + { account_number: '2010', debit_amount: 0, credit_amount: 37400.78, line_description: 'IB 2010' }, + ]) + }) + + it('uses the plain description and no continuity warning for explicit #IB 0', async () => { + const supabase = buildRoutingSupabase(standardQueues()) + const parsed = makeParsedFile({ + openingBalances: [ + { yearIndex: 0, account: '1930', amount: 37400.78 }, + { yearIndex: 0, account: '2010', amount: -37400.78 }, + ], + }) + + const result = await executeSIEImport( + supabase, + 'company-1', + 'user-1', + parsed, + standardMappings, + standardOptions, + ) + + expect(result.success).toBe(true) + expect(result.warnings.join(' ')).not.toMatch(/kontinuitetsprincipen/) + + const input = vi.mocked(createJournalEntry).mock.calls[0][3] + expect(input.description).toBe('Ingående balanser från SIE-import') + }) + + it('respects the continuation guard — no derived IB when the company has prior activity', async () => { + const queues = standardQueues() + queues.journal_entries = [{ count: 5 }] // posted entries exist + const supabase = buildRoutingSupabase(queues) + + const result = await executeSIEImport( + supabase, + 'company-1', + 'user-1', + makeParsedFile(), + standardMappings, + standardOptions, + ) + + expect(createJournalEntry).not.toHaveBeenCalled() + expect(result.openingBalanceEntryId).toBeNull() + expect(result.warnings.join(' ')).toMatch(/hoppades över eftersom bolaget redan har bokförda verifikationer/) + // Zero entries created → the finalizer safety net downgrades the run so + // the file slot stays free for a retry (existing behavior). + expect(result.success).toBe(false) + expect(result.errors.join(' ')).toMatch(/0 verifikationer/) + }) + + it('creates no IB entry when the file has neither #IB 0 nor #UB -1', async () => { + const supabase = buildRoutingSupabase(standardQueues()) + const parsed = makeParsedFile({ + openingBalances: [], + closingBalances: [ + { yearIndex: 0, account: '1930', amount: 160406.0 }, + { yearIndex: 0, account: '2010', amount: -160406.0 }, + ], + }) + + const result = await executeSIEImport( + supabase, + 'company-1', + 'user-1', + parsed, + standardMappings, + standardOptions, + ) + + expect(createJournalEntry).not.toHaveBeenCalled() + expect(result.openingBalanceEntryId).toBeNull() + }) +}) diff --git a/lib/import/__tests__/sie-import.test.ts b/lib/import/__tests__/sie-import.test.ts index d4f15d59..1d483b67 100644 --- a/lib/import/__tests__/sie-import.test.ts +++ b/lib/import/__tests__/sie-import.test.ts @@ -1072,4 +1072,170 @@ describe('importVouchers — per-voucher series preservation', () => { expect(journalEntryInserts[0].source_voucher_series).toBeNull() expect(journalEntryInserts[0].source_voucher_number).toBe(1) }) + + describe('opening-balance voucher tagging vs derived IB (issue #675)', () => { + const obMap = new Map([ + ['1930', '1930'], + ['2010', '2010'], + ]) + + it('tags a qualifying OB voucher opening_balance even when #UB -1 records exist', async () => { + // Precedence 2 beats 3: the OB-voucher candidate makes + // getEffectiveOpeningBalances yield no balances, so hasCurrentYearIb is + // false and the voucher keeps serving as the IB. Without that yield, a + // derived IB entry AND this voucher would both book the same amounts. + const { supabase, journalEntryInserts } = buildCapturingSupabase() + const parsed = makeParsedFile({ + openingBalances: [], + closingBalances: [ + { yearIndex: -1, account: '1930', amount: 37400.78 }, + { yearIndex: -1, account: '2010', amount: -37400.78 }, + ], + vouchers: [ + { + series: 'A', + number: 1, + date: new Date(2024, 0, 1), + description: 'Ingående balans', + lines: [ + { account: '1930', amount: 37400.78 }, + { account: '2010', amount: -37400.78 }, + ], + }, + ], + }) + + const result = await importVouchers( + supabase, + 'company-1', + 'user-1', + 'period-1', + parsed, + obMap, + 'A', + ) + + expect(result.created).toBe(1) + expect(journalEntryInserts[0].source_type).toBe('opening_balance') + }) + + it('keeps an FY-start voucher without IB wording as import when IB is derived from #UB -1', async () => { + const { supabase, journalEntryInserts } = buildCapturingSupabase() + const parsed = makeParsedFile({ + openingBalances: [], + closingBalances: [ + { yearIndex: -1, account: '1930', amount: 37400.78 }, + { yearIndex: -1, account: '2010', amount: -37400.78 }, + ], + vouchers: [ + { + series: 'A', + number: 1, + date: new Date(2024, 0, 1), + description: 'Omföring', + lines: [ + { account: '1930', amount: 1000 }, + { account: '2010', amount: -1000 }, + ], + }, + ], + }) + + await importVouchers( + supabase, + 'company-1', + 'user-1', + 'period-1', + parsed, + obMap, + 'A', + ) + + expect(journalEntryInserts[0].source_type).toBe('import') + }) + }) +}) + +describe('IB derivation from #UB -1 (issue #675)', () => { + const derivedOverrides: Partial = { + openingBalances: [], + closingBalances: [ + { yearIndex: -1, account: '1930', amount: 37400.78 }, + { yearIndex: -1, account: '2440', amount: -37400.78 }, + { yearIndex: 0, account: '1930', amount: 160406.0 }, + { yearIndex: 0, account: '2440', amount: -160406.0 }, + ], + } + + describe('generateImportPreview', () => { + it('computes opening balance totals from the derived set', () => { + const parsed = makeParsedFile(derivedOverrides) + const preview = generateImportPreview(parsed, [ + makeMapping('1930', '1930'), + makeMapping('2440', '2440'), + ]) + + // Derived from #UB -1: 37400.78 debit / 37400.78 credit. This is also + // what enables the IB toggle in ImportReviewStep (openingBalanceTotal > 0). + expect(preview.openingBalanceTotal).toBe(37400.78) + expect(preview.trialBalance.totalDebit).toBe(37400.78) + expect(preview.trialBalance.totalCredit).toBe(37400.78) + expect(preview.trialBalance.isBalanced).toBe(true) + }) + + it('appends an info issue explaining the derivation without mutating parsed.issues', () => { + const parsed = makeParsedFile(derivedOverrides) + const preview = generateImportPreview(parsed, [makeMapping('1930', '1930')]) + + const infoMessages = preview.issues.filter((i) => i.severity === 'info') + expect(infoMessages.map((i) => i.message).join(' ')).toMatch(/härleds från föregående års utgående balans/i) + expect(parsed.issues).toHaveLength(0) + }) + + it('does not append the derivation issue when explicit #IB 0 exists', () => { + const parsed = makeParsedFile() + const preview = generateImportPreview(parsed, [makeMapping('1930', '1930')]) + + expect(preview.issues).toHaveLength(0) + }) + }) + + describe('validateIBBalance', () => { + it('builds journal lines from the derived #UB -1 set', () => { + const parsed = makeParsedFile(derivedOverrides) + const accountMap = new Map([ + ['1930', '1930'], + ['2440', '2440'], + ]) + + const result = validateIBBalance(parsed, accountMap) + + expect(result.lines).toEqual([ + { account_number: '1930', debit_amount: 37400.78, credit_amount: 0, line_description: 'IB 1930' }, + { account_number: '2440', debit_amount: 0, credit_amount: 37400.78, line_description: 'IB 2440' }, + ]) + expect(result.roundingAdjustment).toBe(0) + expect(result.fileImbalance).toBe(0) + }) + + it('reports the imbalance when the derived set carries an unallocated prior-year result', () => { + const parsed = makeParsedFile({ + openingBalances: [], + closingBalances: [ + { yearIndex: -1, account: '1930', amount: 37400.78 }, + { yearIndex: -1, account: '2440', amount: -30000.0 }, + ], + }) + const accountMap = new Map([ + ['1930', '1930'], + ['2440', '2440'], + ]) + + const result = validateIBBalance(parsed, accountMap) + + // 37400.78 − 30000.00 → diff booked to 2099 by createOpeningBalanceEntry + expect(result.roundingAdjustment).toBe(7400.78) + expect(result.fileImbalance).toBe(7400.78) + }) + }) }) diff --git a/lib/import/__tests__/sie-parser.test.ts b/lib/import/__tests__/sie-parser.test.ts index d5b6222b..b2589ad1 100644 --- a/lib/import/__tests__/sie-parser.test.ts +++ b/lib/import/__tests__/sie-parser.test.ts @@ -1,5 +1,12 @@ import { describe, it, expect } from 'vitest' -import { parseSIEFile, validateSIEFile, detectEncoding, decodeBuffer } from '../sie-parser' +import { + parseSIEFile, + validateSIEFile, + detectEncoding, + decodeBuffer, + getEffectiveOpeningBalances, + hasOpeningBalanceVoucherCandidate, +} from '../sie-parser' // --- SIE content fixtures --- @@ -1090,3 +1097,195 @@ describe('parseSIEFile — silent-failure diagnostic warnings', () => { expect(spurious).toHaveLength(0) }) }) + +describe('getEffectiveOpeningBalances — derive IB from #UB -1 (issue #675)', () => { + // Issue #675 (ro66an): some systems export no #IB 0 records — the current + // year's IB exists only via the continuity invariant IB(0) = UB(-1). + const SIE_NO_IB0 = [ + '#FLAGGA 0', + '#SIETYP 4', + '#FNAMN "Continuity AB"', + '#RAR 0 20240101 20241231', + '#RAR -1 20230101 20231231', + '#KONTO 1930 "Företagskonto"', + '#KONTO 2010 "Eget kapital"', + '#IB -1 1930 9483.08', + '#UB 0 1930 160406.00', + '#UB -1 1930 37400.78', + '#UB -1 2010 -37400.78', + ].join('\n') + + it('derives current-year IB from #UB -1 when no #IB 0 exists (issue example)', () => { + const parsed = parseSIEFile(SIE_NO_IB0) + const { balances, derivedFromPriorYearUB } = getEffectiveOpeningBalances(parsed) + + expect(derivedFromPriorYearUB).toBe(true) + expect(balances).toEqual([ + { yearIndex: 0, account: '1930', amount: 37400.78 }, + { yearIndex: 0, account: '2010', amount: -37400.78 }, + ]) + }) + + it('never uses #IB -1 (previous year IB) as the derivation source', () => { + const parsed = parseSIEFile(SIE_NO_IB0) + const { balances } = getEffectiveOpeningBalances(parsed) + + expect(balances.some((b) => b.amount === 9483.08)).toBe(false) + }) + + it('returns explicit #IB 0 untouched when present — #UB -1 is never merged in', () => { + const content = [ + SIE_NO_IB0, + '#IB 0 1930 37400.78', + '#IB 0 2010 -37400.78', + ].join('\n') + const parsed = parseSIEFile(content) + const { balances, derivedFromPriorYearUB } = getEffectiveOpeningBalances(parsed) + + expect(derivedFromPriorYearUB).toBe(false) + expect(balances).toHaveLength(2) + expect(balances.every((b) => b.yearIndex === 0)).toBe(true) + }) + + it('yields to an opening-balance voucher candidate — no derivation (precedence 2 beats 3)', () => { + // The voucher serves as IB during import (tagged source_type + // 'opening_balance'); deriving from #UB -1 as well would double-count. + // Also the timezone regression test: the voucher date is a local-time + // Date, so a toISOString()-based comparison would miss the FY start on + // machines west or east of UTC and wrongly re-enable derivation. + const content = [ + SIE_NO_IB0, + '#VER A 1 20240101 "Ingående balans"', + '{', + '#TRANS 1930 {} 37400.78', + '#TRANS 2010 {} -37400.78', + '}', + ].join('\n') + const parsed = parseSIEFile(content) + + expect(hasOpeningBalanceVoucherCandidate(parsed)).toBe(true) + + const { balances, derivedFromPriorYearUB } = getEffectiveOpeningBalances(parsed) + expect(derivedFromPriorYearUB).toBe(false) + expect(balances).toEqual([]) + }) + + it('does not treat a share-capital voucher on FY start as an OB candidate', () => { + const content = [ + SIE_NO_IB0, + '#VER A 1 20240101 "Insättning aktiekapital ingående balans"', + '{', + '#TRANS 1930 {} 25000.00', + '#TRANS 2081 {} -25000.00', + '}', + ].join('\n') + const parsed = parseSIEFile(content) + + expect(hasOpeningBalanceVoucherCandidate(parsed)).toBe(false) + expect(getEffectiveOpeningBalances(parsed).derivedFromPriorYearUB).toBe(true) + }) + + it('does not treat a voucher with P&L lines as an OB candidate', () => { + const content = [ + SIE_NO_IB0, + '#VER A 1 20240101 "Ingående balans"', + '{', + '#TRANS 1930 {} 1000.00', + '#TRANS 3001 {} -1000.00', + '}', + ].join('\n') + const parsed = parseSIEFile(content) + + expect(hasOpeningBalanceVoucherCandidate(parsed)).toBe(false) + expect(getEffectiveOpeningBalances(parsed).derivedFromPriorYearUB).toBe(true) + }) + + it('does not treat an IB-worded voucher on another date as an OB candidate', () => { + const content = [ + SIE_NO_IB0, + '#VER A 1 20240315 "Ingående balans"', + '{', + '#TRANS 1930 {} 1000.00', + '#TRANS 2010 {} -1000.00', + '}', + ].join('\n') + const parsed = parseSIEFile(content) + + expect(hasOpeningBalanceVoucherCandidate(parsed)).toBe(false) + expect(getEffectiveOpeningBalances(parsed).derivedFromPriorYearUB).toBe(true) + }) + + it('filters P&L accounts out of the derived set (result accounts open at zero)', () => { + const content = [ + SIE_NO_IB0, + '#UB -1 3001 -5000.00', + ].join('\n') + const parsed = parseSIEFile(content) + const { balances } = getEffectiveOpeningBalances(parsed) + + expect(balances.some((b) => b.account === '3001')).toBe(false) + expect(balances).toHaveLength(2) + }) + + it('returns nothing when neither #IB 0 nor #UB -1 exists', () => { + const content = [ + '#FLAGGA 0', + '#SIETYP 4', + '#FNAMN "First Year AB"', + '#RAR 0 20240101 20241231', + '#KONTO 1930 "Företagskonto"', + '#IB -1 1930 9483.08', + '#UB 0 1930 160406.00', + ].join('\n') + const parsed = parseSIEFile(content) + const { balances, derivedFromPriorYearUB } = getEffectiveOpeningBalances(parsed) + + expect(derivedFromPriorYearUB).toBe(false) + expect(balances).toEqual([]) + }) + + it('carries quantity along on derived balances', () => { + const content = [ + SIE_NO_IB0.replace('#UB -1 1930 37400.78', '#UB -1 1930 37400.78 5'), + ].join('\n') + const parsed = parseSIEFile(content) + const { balances } = getEffectiveOpeningBalances(parsed) + + expect(balances.find((b) => b.account === '1930')?.quantity).toBe(5) + }) + + describe('validateSIEFile with derived IB', () => { + it('warns that IB will be derived from #UB -1', () => { + const parsed = parseSIEFile(SIE_NO_IB0) + const validation = validateSIEFile(parsed) + + expect(validation.valid).toBe(true) + expect(validation.warnings.join(' ')).toMatch(/härleds från föregående års utgående balans/i) + }) + + it('runs the imbalance check on the derived set (unallocated prior-year result)', () => { + const content = [ + '#FLAGGA 0', + '#SIETYP 4', + '#FNAMN "Obalans AB"', + '#RAR 0 20240101 20241231', + '#KONTO 1930 "Företagskonto"', + // Derived IB sums to +37400.78 — prior-year result never allocated + '#UB -1 1930 37400.78', + ].join('\n') + const parsed = parseSIEFile(content) + const validation = validateSIEFile(parsed) + + expect(validation.warnings.join(' ')).toMatch(/balanserar inte/i) + expect(validation.warnings.join(' ')).toMatch(/37400\.78/) + }) + + it('does not warn about derivation when explicit #IB 0 exists', () => { + const content = [SIE_NO_IB0, '#IB 0 1930 37400.78', '#IB 0 2010 -37400.78'].join('\n') + const parsed = parseSIEFile(content) + const validation = validateSIEFile(parsed) + + expect(validation.warnings.join(' ')).not.toMatch(/härleds från föregående års utgående balans/i) + }) + }) +}) diff --git a/lib/import/sie-import.ts b/lib/import/sie-import.ts index 3179ff3a..fab8bd9f 100644 --- a/lib/import/sie-import.ts +++ b/lib/import/sie-import.ts @@ -19,7 +19,17 @@ import type { import type { CreateJournalEntryLineInput } from '@/types' import { mappingsToMap, getMappingStats } from './account-mapper' import { syncMappedAccounts } from './account-sync' -import { calculateFileHash } from './sie-parser' +import { + calculateFileHash, + getEffectiveOpeningBalances, + isBalanceSheetAccount, + OPENING_BALANCE_DESCRIPTION_RE, + SHARE_CAPITAL_DESCRIPTION_RE, +} from './sie-parser' + +// Re-export from the parser (moved there to avoid an import cycle — +// getEffectiveOpeningBalances needs it) so existing importers keep working. +export { isBalanceSheetAccount } from './sie-parser' import { getBASReference } from '@/lib/bookkeeping/bas-reference' import { classifyAccount } from '@/lib/bookkeeping/account-classifier' import { computeSRUCode } from '@/lib/bookkeeping/bas-data/sru-mapping' @@ -43,8 +53,12 @@ export function generateImportPreview( parsed: ParsedSIEFile, mappings: AccountMapping[] ): ImportPreview { - // Calculate opening balance totals - const currentYearBalances = parsed.openingBalances.filter((b) => b.yearIndex === 0) + // Calculate opening balance totals from the effective set — for files + // without #IB 0 this is the IB derived from #UB -1 (issue #675), so the + // preview (and the IB toggle in ImportReviewStep, keyed off + // openingBalanceTotal > 0) reflects what the import will actually book. + const { balances: currentYearBalances, derivedFromPriorYearUB } = + getEffectiveOpeningBalances(parsed) let totalDebit = 0 let totalCredit = 0 @@ -79,7 +93,17 @@ export function generateImportPreview( lowConfidence: mappingStats.lowConfidence, }, excludedSystemAccounts: [], - issues: parsed.issues, + issues: derivedFromPriorYearUB + ? [ + ...parsed.issues, + { + severity: 'info', + line: 0, + message: + 'Ingående balanser härleds från föregående års utgående balans (#UB -1) — filen saknar #IB-poster för aktuellt räkenskapsår.', + }, + ] + : parsed.issues, } } @@ -464,7 +488,8 @@ export function validateIBBalance( fileImbalance: number excludedAccountsTotal: number } { - const currentYearBalances = parsed.openingBalances.filter((b) => b.yearIndex === 0) + // Effective set: explicit #IB 0, or IB derived from #UB -1 (issue #675). + const currentYearBalances = getEffectiveOpeningBalances(parsed).balances // First: check the raw file-level IB balance (all accounts, before mapping) const rawTotal = currentYearBalances.reduce((sum, b) => sum + b.amount, 0) @@ -525,7 +550,9 @@ async function createOpeningBalanceEntry( accountMap: Map, roundingAdjustment: number ): Promise { - const currentYearBalances = parsed.openingBalances.filter((b) => b.yearIndex === 0) + // Effective set: explicit #IB 0, or IB derived from #UB -1 (issue #675). + const { balances: currentYearBalances, derivedFromPriorYearUB } = + getEffectiveOpeningBalances(parsed) if (currentYearBalances.length === 0) { return null @@ -583,7 +610,11 @@ async function createOpeningBalanceEntry( const entry = await createJournalEntry(supabase, companyId, userId, { fiscal_period_id: fiscalPeriodId, entry_date: entryDate, - description: 'Ingående balanser från SIE-import', + // When derived, say so on the voucher itself — permanent documentation + // of where the amounts came from (BFNAR 2013:2 behandlingshistorik). + description: derivedFromPriorYearUB + ? 'Ingående balanser från SIE-import (härledda från föregående års utgående balans)' + : 'Ingående balanser från SIE-import', source_type: 'opening_balance', voucher_series: 'A', lines, @@ -912,17 +943,25 @@ export async function importVouchers( const preparedVouchers: PreparedVoucher[] = [] // A SIE file represents the opening balance either as #IB records (handled - // separately by createOpeningBalanceEntry → source_type='opening_balance') or, - // in some source systems, as an ordinary #VER dated on the fiscal-year start. - // When there are NO current-year #IB records, detect a clearly-labelled IB - // voucher and tag it opening_balance so bank reconciliation excludes it from - // the period movement (otherwise it lands as 'import' and surfaces as a phantom + // separately by createOpeningBalanceEntry → source_type='opening_balance'), + // as IB derived from #UB -1 when #IB 0 is missing (issue #675, also via + // createOpeningBalanceEntry) or, in some source systems, as an ordinary #VER + // dated on the fiscal-year start. When there is NO current-year IB from + // either of the first two paths, detect a clearly-labelled IB voucher and + // tag it opening_balance so bank reconciliation excludes it from the period + // movement (otherwise it lands as 'import' and surfaces as a phantom // difference equal to the IB). Deliberately conservative — requires the IB // wording AND a balance-sheet-only voucher on FY start, and never a // share-capital deposit. A missed IB still falls back to the manual "Märk som // ingående balans" action in Bankavstämning, so we never risk hiding a real // bank movement by over-classifying. - const hasCurrentYearIb = parsed.openingBalances.some((b) => b.yearIndex === 0) + // + // Using the effective set keeps this gate consistent with the helper's + // precedence: when an OB-voucher candidate exists the helper yields no + // balances (the voucher serves as IB and gets tagged here); when IB was + // derived from #UB -1 the gate is closed so the same amounts can never be + // booked twice. + const hasCurrentYearIb = getEffectiveOpeningBalances(parsed).balances.length > 0 const fyStart = parsed.stats.fiscalYearStart for (const voucher of parsed.vouchers) { @@ -1062,8 +1101,8 @@ export async function importVouchers( !!fyStart && fyStart.slice(0, 10) === voucherDateStr && lines.length > 0 && lines.every((l) => isBalanceSheetAccount(l.account_number)) && - /ing[åa]ende balans|ing[åa]ende saldo|opening balance/i.test(voucher.description || '') && - !/aktiekapital/i.test(voucher.description || '') + OPENING_BALANCE_DESCRIPTION_RE.test(voucher.description || '') && + !SHARE_CAPITAL_DESCRIPTION_RE.test(voucher.description || '') preparedVouchers.push({ sourceId: voucherId, @@ -1354,14 +1393,6 @@ export async function importVouchers( return results } -/** - * Determine if an account is balance sheet (class 1-2) or P&L (class 3-8) - */ -export function isBalanceSheetAccount(accountNumber: string): boolean { - const firstDigit = parseInt(accountNumber.charAt(0), 10) - return firstDigit >= 1 && firstDigit <= 2 -} - /** * Compute per-series voucher number ranges from the voucher number mapping. * SIE imports can span multiple series (B, C, V, ...), each with its own @@ -1427,8 +1458,11 @@ async function createMigrationAdjustmentEntry( // For P&L accounts (class 3-8): expectedMovement = RES (ignore IB/UB) const expectedMovements = new Map() - // Process IB — only for balance sheet accounts - for (const ib of parsed.openingBalances.filter((b) => b.yearIndex === 0)) { + // Process IB — only for balance sheet accounts. Effective set: explicit + // #IB 0, or IB derived from #UB -1 (issue #675) — so the expected BS + // movement is UB(0) − UB(-1), the correct one-year movement, instead of + // treating the whole opening balance as unexplained movement. + for (const ib of getEffectiveOpeningBalances(parsed).balances) { const target = accountMap.get(ib.account) if (!target) continue if (!isBalanceSheetAccount(target)) { @@ -1866,7 +1900,9 @@ export async function executeSIEImport( const sourceAccountsInFile = new Set() for (const v of parsed.vouchers) for (const l of v.lines) sourceAccountsInFile.add(l.account) if (options.importOpeningBalances) { - for (const b of parsed.openingBalances.filter((b) => b.yearIndex === 0)) { + // Effective set: also covers UB-1-only files (issue #675), whose + // derived IB accounts would otherwise bypass this guard entirely. + for (const b of getEffectiveOpeningBalances(parsed).balances) { sourceAccountsInFile.add(b.account) } } @@ -2065,7 +2101,12 @@ export async function executeSIEImport( // In both cases, the correct treatment is to book the diff to 2099 with // explicit documentation. We never reject based on IB imbalance — the // original goal was to stop SILENT equity alteration, not prevent it. - if (options.importOpeningBalances && parsed.openingBalances.length > 0 && result.fiscalPeriodId) { + // + // Gate on the EFFECTIVE set: for files without #IB 0, the IB derived + // from #UB -1 (issue #675) must still open this block — gating on raw + // parsed.openingBalances would silently skip the derived IB entirely. + const effectiveIB = getEffectiveOpeningBalances(parsed) + if (options.importOpeningBalances && effectiveIB.balances.length > 0 && result.fiscalPeriodId) { // Check if opening balances already exist for this period const { data: period } = await supabase .from('fiscal_periods') @@ -2097,6 +2138,13 @@ export async function executeSIEImport( const ibValidation = validateIBBalance(parsed, accountMap) if (ibValidation.lines.length > 0) { + if (effectiveIB.derivedFromPriorYearUB) { + result.warnings.push( + 'SIE-filen saknar ingående balanser (#IB) för räkenskapsåret. ' + + 'Ingående balanser härleddes från föregående års utgående balanser (#UB -1) enligt kontinuitetsprincipen.' + ) + } + const absAdj = Math.abs(ibValidation.roundingAdjustment) if (absAdj > 0.01) { diff --git a/lib/import/sie-parser.ts b/lib/import/sie-parser.ts index e1404f5b..895219d8 100644 --- a/lib/import/sie-parser.ts +++ b/lib/import/sie-parser.ts @@ -777,6 +777,104 @@ export function parseSIEFile(content: string): ParsedSIEFile { } } +/** + * Wording that identifies a voucher as the year's opening balance + * (ingående balans). Shared between the parser's OB-voucher candidate + * detection below and the importer's isLikelyOpeningBalance tagging + * (lib/import/sie-import.ts) so the two checks can never drift apart. + */ +export const OPENING_BALANCE_DESCRIPTION_RE = /ing[åa]ende balans|ing[åa]ende saldo|opening balance/i + +/** + * Vouchers mentioning share capital are never treated as opening balances — + * a share-capital deposit dated on the FY start is a real bank movement. + */ +export const SHARE_CAPITAL_DESCRIPTION_RE = /aktiekapital/i + +/** + * Determine if an account is balance sheet (class 1-2) or P&L (class 3-8) + */ +export function isBalanceSheetAccount(accountNumber: string): boolean { + const firstDigit = parseInt(accountNumber.charAt(0), 10) + return firstDigit >= 1 && firstDigit <= 2 +} + +/** + * Format a Date to "YYYY-MM-DD" using LOCAL components. + * parseSIEDate() builds local-time Dates, so toISOString() would shift the + * day across the UTC boundary in non-UTC timezones — never use it here. + */ +function formatLocalDate(date: Date): string { + const year = date.getFullYear() + const month = String(date.getMonth() + 1).padStart(2, '0') + const day = String(date.getDate()).padStart(2, '0') + return `${year}-${month}-${day}` +} + +/** + * True when the file contains a voucher that looks like the year's opening + * balance: dated on the fiscal-year start, only balance-sheet accounts, + * IB wording in the description and no share-capital mention. + * + * Raw-file mirror of the importer's isLikelyOpeningBalance check + * (lib/import/sie-import.ts), but deliberately MORE eager: it runs on + * source account numbers with no knowledge of account mappings, so a + * candidate containing an unmapped line still counts here even though the + * importer would later skip that voucher as unmapped. In that residual case + * no IB is created at all — the user falls back to the manual + * "Märk som ingående balans" action in Bankavstämning. + */ +export function hasOpeningBalanceVoucherCandidate(parsed: ParsedSIEFile): boolean { + const fyStart = parsed.stats.fiscalYearStart + if (!fyStart) return false + + return parsed.vouchers.some( + (v) => + v.lines.length > 0 && + formatLocalDate(v.date) === fyStart.slice(0, 10) && + v.lines.every((l) => isBalanceSheetAccount(l.account)) && + OPENING_BALANCE_DESCRIPTION_RE.test(v.description || '') && + !SHARE_CAPITAL_DESCRIPTION_RE.test(v.description || '') + ) +} + +/** + * Resolve the opening balances the import should actually book (issue #675). + * + * Some systems export no #IB 0 records at all — the current year's IB exists + * only implicitly via the SIE continuity invariant IB(year 0) = UB(year -1). + * Every IB consumer goes through this helper so the precedence below is the + * single source of truth: + * + * 1. Explicit #IB 0 records — trusted as-is, never merged with #UB -1. + * 2. An opening-balance #VER candidate — the voucher itself serves as IB + * during voucher import (tagged source_type 'opening_balance'); + * deriving from #UB -1 as well would double-count every + * balance-sheet account. + * 3. #UB -1 records, re-labeled to yearIndex 0 and filtered to + * balance-sheet accounts (result accounts must always open at zero). + * 4. Nothing — the file genuinely carries no opening balances. + */ +export function getEffectiveOpeningBalances(parsed: ParsedSIEFile): { + balances: SIEBalance[] + derivedFromPriorYearUB: boolean +} { + const explicit = parsed.openingBalances.filter((b) => b.yearIndex === 0) + if (explicit.length > 0) { + return { balances: explicit, derivedFromPriorYearUB: false } + } + + if (hasOpeningBalanceVoucherCandidate(parsed)) { + return { balances: [], derivedFromPriorYearUB: false } + } + + const derived = parsed.closingBalances + .filter((b) => b.yearIndex === -1 && isBalanceSheetAccount(b.account)) + .map((b) => ({ ...b, yearIndex: 0 })) + + return { balances: derived, derivedFromPriorYearUB: derived.length > 0 } +} + /** * Validate a parsed SIE file */ @@ -866,10 +964,18 @@ export function validateSIEFile(parsed: ParsedSIEFile): ValidationResult { ) } - // Check opening balance is balanced (for balance sheet accounts) - const ibTotal = parsed.openingBalances - .filter((b) => b.yearIndex === 0) - .reduce((sum, b) => sum + b.amount, 0) + // Check opening balance is balanced (for balance sheet accounts). + // Uses the effective set so files without #IB 0 — where IB is derived from + // #UB -1 (issue #675) — still get the 2099-adjustment heads-up. + const effectiveIB = getEffectiveOpeningBalances(parsed) + + if (effectiveIB.derivedFromPriorYearUB) { + warnings.push( + 'Filen saknar ingående balanser (#IB) för aktuellt räkenskapsår — de härleds från föregående års utgående balans (#UB -1) vid import.' + ) + } + + const ibTotal = effectiveIB.balances.reduce((sum, b) => sum + b.amount, 0) if (Math.abs(ibTotal) > 0.01) { warnings.push(`Ingående balanser balanserar inte (differens: ${ibTotal.toFixed(2)} kr). En automatisk justeringspost mot konto 2099 skapas vid import.`) diff --git a/lib/pending-operations/commit.ts b/lib/pending-operations/commit.ts index c3039370..7e7a1d9a 100644 --- a/lib/pending-operations/commit.ts +++ b/lib/pending-operations/commit.ts @@ -104,14 +104,19 @@ export interface CommitOptions { userEmail?: string /** * commit_method recorded on any journal_entries created by this operation. - * Must match the CHECK constraint on journal_entries.commit_method: - * 'user_accept' | 'bulk_accept' | 'timing_ceiling' | 'migration' | 'legacy'. - * Single-approval route passes 'user_accept' (default); bulk-approval passes - * 'bulk_accept'. Defaults to 'user_accept' since the dispatcher is only - * invoked from human-approval paths after agent auto-commit was removed - * (migration 20260505190027_drop_agent_auto_commit). + * Must match the CHECK constraint on journal_entries.commit_method + * (migration 20260618120001): 'user_accept' | 'bulk_accept' | + * 'timing_ceiling' | 'migration' | 'legacy' | 'agent' | 'api_key'. + * + * Web-UI single-approval passes 'user_accept'; bulk-approval passes + * 'bulk_accept'. MCP approvals pass the relaying credential — 'api_key' + * (gnubok-mcp bridge) or 'agent' (OAuth connector) — so the immutable layer + * records that the acknowledgment was agent-relayed rather than a + * first-party human session (agent_first_vision.md §8 P0-1). Every path is + * still human-approval-gated; agent auto-commit was removed in + * 20260505190027_drop_agent_auto_commit. */ - commitMethod?: 'user_accept' | 'bulk_accept' + commitMethod?: 'user_accept' | 'bulk_accept' | 'agent' | 'api_key' } // ── Helper: ensure fiscal period covers the date ────────────────── @@ -2470,10 +2475,11 @@ async function commitCreateVoucher( notes: (params.notes as string) || undefined, lines, }, - // commit_method records HOW it was committed, not who staged it. MCP- - // staged ops still go through human approval, so 'user_accept' (or - // 'bulk_accept' from the bulk route) is the correct value. The DB CHECK - // constraint rejects anything else (migration 20260420120001). + // commit_method records HOW it was committed, not who staged it. + // Web routes pass 'user_accept'/'bulk_accept'; the MCP approve path + // passes 'api_key'/'agent' so agent-relayed acknowledgments are + // distinguishable in the immutable layer. The DB CHECK constraint + // rejects anything else (migrations 20260420120001, 20260618120001). opts.commitMethod ?? 'user_accept' ) diff --git a/package.json b/package.json index e3d1aa00..51f1da1d 100644 --- a/package.json +++ b/package.json @@ -14,6 +14,7 @@ "start": "next start", "lint": "eslint", "check:guards": "node scripts/checks/no-new-antipatterns.mjs", + "check:lint": "node scripts/checks/no-new-lint-errors.mjs", "test": "vitest run --project unit", "test:pg": "vitest run --project pg-real" }, diff --git a/scripts/check-pg-test-coverage.mjs b/scripts/check-pg-test-coverage.mjs new file mode 100644 index 00000000..ec029385 --- /dev/null +++ b/scripts/check-pg-test-coverage.mjs @@ -0,0 +1,113 @@ +#!/usr/bin/env node +/** + * CI gate: a PR that adds or changes a migration touching a trigger, + * function/RPC, RLS policy, or DEFERRABLE constraint must also add or extend + * a *.pg.test.ts. + * + * This enforces the rule documented in .claude/rules/database.md ("pg-real + * tests: any PR touching a trigger/RPC/RLS/DEFERRABLE must include or extend + * a *.pg.test.ts") — previously instruction-only, which means it got skipped. + * + * Escape hatch: a migration may declare, in a SQL comment, either + * -- pg-test: covered-by tests/pg/.pg.test.ts + * -- pg-test: skip () + * Both are visible in review and greppable later. Use them sparingly — + * "covered-by" when an existing test already exercises the changed object, + * "skip" when the change is genuinely untestable (e.g. a NOTIFY-only fixup). + * + * Scope: the gate is PR-level, not per-migration — ANY *.pg.test.ts change + * satisfies it. With multiple risky migrations in one PR, reviewers must + * still confirm each one is actually covered (or carries an escape hatch); + * mapping tests to migrations automatically would be guesswork. + * + * Usage: node scripts/check-pg-test-coverage.mjs + * PG_GATE_BASE — git ref to diff against (default: origin/main) + */ +import { execFileSync } from 'node:child_process' +import { existsSync, readFileSync } from 'node:fs' + +const base = process.env.PG_GATE_BASE || 'origin/main' + +let changed +try { + // Three-dot diff: changes on the PR side since the merge-base with `base`. + // --diff-filter=ACMR skips deletions (a deleted migration has no content to + // scan). execFileSync with an argv array — no shell, so a hostile base-ref + // string can't inject (git just rejects an invalid rev via the catch below). + changed = execFileSync('git', ['diff', '--name-only', '--diff-filter=ACMR', `${base}...HEAD`], { + encoding: 'utf8', + }) + .trim() + .split('\n') + .filter(Boolean) +} catch (err) { + console.error(`check-pg-test-coverage: failed to diff against "${base}".`) + console.error('Set PG_GATE_BASE to a fetched ref (CI: origin/${{ github.base_ref }}).') + console.error(String(err)) + process.exit(2) +} + +const migrations = changed.filter( + (f) => f.startsWith('supabase/migrations/') && f.endsWith('.sql'), +) +const pgTests = changed.filter((f) => f.endsWith('.pg.test.ts')) + +// DDL that the database.md rule classifies as requiring real-Postgres coverage. +const RISKY_DDL = [ + { kind: 'trigger', re: /\bCREATE\s+(OR\s+REPLACE\s+)?(CONSTRAINT\s+)?TRIGGER\b/i }, + { kind: 'function/RPC', re: /\bCREATE\s+(OR\s+REPLACE\s+)?FUNCTION\b/i }, + { kind: 'RLS policy', re: /\b(CREATE|ALTER|DROP)\s+POLICY\b/i }, + { kind: 'RLS enable/disable', re: /\b(ENABLE|DISABLE)\s+ROW\s+LEVEL\s+SECURITY\b/i }, + { kind: 'DEFERRABLE constraint', re: /\bDEFERRABLE\b/i }, +] + +const ESCAPE_HATCH = /^\s*--\s*pg-test:\s*(covered-by\s+\S+|skip\b.*)$/im + +const flagged = [] +for (const file of migrations) { + if (!existsSync(file)) continue + const raw = readFileSync(file, 'utf8') + if (ESCAPE_HATCH.test(raw)) continue + // Strip SQL line comments so prose mentioning "CREATE POLICY" doesn't trip the gate. + const sql = raw.replace(/--.*$/gm, '') + const kinds = RISKY_DDL.filter(({ re }) => re.test(sql)).map(({ kind }) => kind) + if (kinds.length > 0) flagged.push({ file, kinds }) +} + +if (flagged.length === 0) { + console.log( + migrations.length === 0 + ? 'check-pg-test-coverage: no migrations in this diff.' + : `check-pg-test-coverage: ${migrations.length} migration(s) changed, none touch trigger/RPC/RLS/DEFERRABLE.`, + ) + process.exit(0) +} + +if (pgTests.length > 0) { + console.log( + `check-pg-test-coverage: ${flagged.length} risky migration(s) accompanied by pg-real test change(s):`, + ) + for (const t of pgTests) console.log(` test: ${t}`) + process.exit(0) +} + +console.error('check-pg-test-coverage: FAILED\n') +console.error( + 'These migrations touch trigger/RPC/RLS/DEFERRABLE but the PR adds or extends no *.pg.test.ts:\n', +) +for (const { file, kinds } of flagged) { + console.error(` ${file} (${kinds.join(', ')})`) +} +console.error(` +The repo rule (.claude/rules/database.md) requires real-Postgres coverage for +these objects — mocked Supabase tests cannot exercise them. + +Fix one of: + 1. Add or extend a *.pg.test.ts covering the changed trigger/RPC/policy + (helpers: tests/pg/setup.ts, tests/pg/fixtures.ts; run: npm run test:pg) + 2. If an existing pg test already covers it, annotate the migration: + -- pg-test: covered-by tests/pg/.pg.test.ts + 3. If genuinely untestable, annotate with a reason: + -- pg-test: skip () +`) +process.exit(1) diff --git a/scripts/checks/eslint-baseline.json b/scripts/checks/eslint-baseline.json new file mode 100644 index 00000000..e4a938ab --- /dev/null +++ b/scripts/checks/eslint-baseline.json @@ -0,0 +1,12 @@ +{ + "totalErrors": 60, + "perRule": { + "@next/next/no-assign-module-variable": 1, + "@typescript-eslint/no-explicit-any": 15, + "prefer-const": 3, + "react-hooks/preserve-manual-memoization": 6, + "react-hooks/purity": 1, + "react-hooks/set-state-in-effect": 28, + "react-hooks/static-components": 6 + } +} diff --git a/scripts/checks/no-new-lint-errors.mjs b/scripts/checks/no-new-lint-errors.mjs new file mode 100644 index 00000000..ff9d3a90 --- /dev/null +++ b/scripts/checks/no-new-lint-errors.mjs @@ -0,0 +1,121 @@ +#!/usr/bin/env node +/** + * Ratchet guard for ESLint errors (sibling of no-new-antipatterns.mjs). + * + * `npm run lint` was never wired into CI, so ~60 pre-existing errors + * accumulated across the repo. Fixing them all in one PR is churn; gating raw + * `eslint` would break every PR until then. So: ratchet. Error counts are + * tracked per rule in a committed baseline and can only go DOWN, never up — + * a PR introducing a NEW error of any rule fails CI, while legacy errors are + * burned down independently. + * + * Warnings stay advisory (only `--quiet` errors are counted). + * + * Known tradeoff: counts are per-rule repo-wide, not per-location — a PR that + * fixes one legacy error of a rule can absorb one NEW error of the same rule + * without tripping the gate. Acceptable for a burn-down ratchet; tighten to + * per-file fingerprints if that ever bites. + * + * Usage: + * node scripts/checks/no-new-lint-errors.mjs # check (CI) + * node scripts/checks/no-new-lint-errors.mjs --update # re-baseline after fixing legacy errors + * + * Exit code 1 if any rule's error count exceeds its baseline. + */ +import fs from 'node:fs' +import path from 'node:path' +import { spawnSync } from 'node:child_process' +import { fileURLToPath } from 'node:url' + +const ROOT = path.resolve(path.dirname(fileURLToPath(import.meta.url)), '..', '..') +const BASELINE_PATH = path.join(ROOT, 'scripts', 'checks', 'eslint-baseline.json') + +function runEslint() { + const result = spawnSync( + 'npx', + ['eslint', '.', '--quiet', '-f', 'json'], + { cwd: ROOT, encoding: 'utf8', maxBuffer: 64 * 1024 * 1024 }, + ) + // ESLint exits 1 when errors exist — that's expected; only treat a missing/ + // unparsable report as fatal. + if (!result.stdout) { + console.error('no-new-lint-errors: eslint produced no JSON output') + console.error(result.stderr ?? '') + process.exit(2) + } + try { + return JSON.parse(result.stdout) + } catch { + console.error('no-new-lint-errors: failed to parse eslint JSON output') + process.exit(2) + } +} + +function collectCounts(report) { + /** @type {Record} */ + const perRule = {} + /** @type {Record} */ + const locations = {} + for (const file of report) { + for (const msg of file.messages) { + if (msg.severity !== 2) continue + const rule = msg.ruleId ?? 'fatal' + perRule[rule] = (perRule[rule] ?? 0) + 1 + const rel = path.relative(ROOT, file.filePath) + ;(locations[rule] ??= []).push(`${rel}:${msg.line}:${msg.column}`) + } + } + return { perRule, locations } +} + +const { perRule, locations } = collectCounts(runEslint()) +const total = Object.values(perRule).reduce((a, b) => a + b, 0) + +if (process.argv.includes('--update')) { + const sorted = Object.fromEntries(Object.entries(perRule).sort(([a], [b]) => a.localeCompare(b))) + fs.writeFileSync( + BASELINE_PATH, + JSON.stringify({ totalErrors: total, perRule: sorted }, null, 2) + '\n', + ) + console.log(`no-new-lint-errors: baseline updated — ${total} error(s) across ${Object.keys(perRule).length} rule(s).`) + process.exit(0) +} + +if (!fs.existsSync(BASELINE_PATH)) { + console.error(`no-new-lint-errors: baseline missing at ${path.relative(ROOT, BASELINE_PATH)}.`) + console.error('Run: node scripts/checks/no-new-lint-errors.mjs --update') + process.exit(2) +} + +const baseline = JSON.parse(fs.readFileSync(BASELINE_PATH, 'utf8')) +const baselineRules = baseline.perRule ?? {} + +const regressions = [] +for (const [rule, count] of Object.entries(perRule)) { + const allowed = baselineRules[rule] ?? 0 + if (count > allowed) regressions.push({ rule, count, allowed }) +} + +if (regressions.length > 0) { + console.error('no-new-lint-errors: FAILED — new ESLint errors beyond the baseline:\n') + for (const { rule, count, allowed } of regressions) { + console.error(` ${rule}: ${count} (baseline ${allowed})`) + for (const loc of (locations[rule] ?? []).slice(0, 10)) { + console.error(` ${loc}`) + } + } + console.error(` +Fix the new error(s) — run \`npx eslint . --quiet\` locally to see them. +(If you fixed MORE legacy errors than you added and the rule still trips, +re-baseline with: node scripts/checks/no-new-lint-errors.mjs --update) +`) + process.exit(1) +} + +const improved = total < (baseline.totalErrors ?? 0) +console.log( + `no-new-lint-errors: OK — ${total} error(s), baseline ${baseline.totalErrors}.` + + (improved + ? ' Count went DOWN — ratchet it: node scripts/checks/no-new-lint-errors.mjs --update' + : ''), +) diff --git a/supabase/migrations/20260618120000_event_log_type_index.sql b/supabase/migrations/20260618120000_event_log_type_index.sql new file mode 100644 index 00000000..bd9dea33 --- /dev/null +++ b/supabase/migrations/20260618120000_event_log_type_index.sql @@ -0,0 +1,12 @@ +-- Telemetry analytics index for event_log. +-- +-- mcp.tool_called / mcp.skill_loaded / agent.feedback analytics filter by +-- event_type + time window (error rate per tool, feedback themes, skill-load +-- correlation). The existing indexes cover only (user_id, sequence) for +-- delivery polling and (created_at) for TTL cleanup — every per-type query +-- was a full scan. Also serves the differentiated-retention deletes in +-- /api/events/cleanup/cron (telemetry kept 180 days, delivery events 30). +CREATE INDEX IF NOT EXISTS idx_event_log_type_created + ON public.event_log (event_type, created_at); + +NOTIFY pgrst, 'reload schema'; diff --git a/supabase/migrations/20260618120001_commit_method_agent_provenance.sql b/supabase/migrations/20260618120001_commit_method_agent_provenance.sql new file mode 100644 index 00000000..363f9ead --- /dev/null +++ b/supabase/migrations/20260618120001_commit_method_agent_provenance.sql @@ -0,0 +1,31 @@ +-- Agent attribution into the immutable layer (agent_first_vision.md §8 P0-1). +-- +-- journal_entries.commit_method could not record that a commit was approved +-- through an agent credential: the MCP approve path hardcoded 'user_accept', +-- so an auditor reading the immutable GL could not distinguish agent-relayed +-- acknowledgments from first-party human sessions. BFNAR 2013:2 kap 8 +-- (behandlingshistorik) requires automated processing to be identifiable. +-- +-- 'api_key' — approval relayed by an agent authenticating with a gnubok_sk_ +-- API key. This covers ALL MCP traffic today: the gnubok-mcp +-- bridge AND the claude.ai OAuth connector, whose access_token +-- is itself a minted API key (app/api/mcp-oauth/token/route.ts) +-- and is indistinguishable from a bridge key at the server. +-- 'agent' — reserved for first-party agent surfaces (e.g. in-app agent +-- chat) once they commit through the approval layer with a +-- distinguishable actor type. Not written by any path yet. +-- +-- Web-UI approvals keep 'user_accept' / 'bulk_accept'. Every path remains +-- human-approval-gated; agent auto-commit stays removed (20260505190027). + +ALTER TABLE public.journal_entries + DROP CONSTRAINT IF EXISTS journal_entries_commit_method_check; + +ALTER TABLE public.journal_entries + ADD CONSTRAINT journal_entries_commit_method_check + CHECK (commit_method IS NULL OR commit_method IN ( + 'user_accept', 'bulk_accept', 'timing_ceiling', 'migration', 'legacy', + 'agent', 'api_key' + )); + +NOTIFY pgrst, 'reload schema'; diff --git a/tests/pg/commit-method-provenance.pg.test.ts b/tests/pg/commit-method-provenance.pg.test.ts new file mode 100644 index 00000000..dde19f37 --- /dev/null +++ b/tests/pg/commit-method-provenance.pg.test.ts @@ -0,0 +1,63 @@ +import { describe, expect, it } from 'vitest' +import { + seedCompany, + insertDraftJournalEntry, + insertBalancedLines, +} from '@/tests/pg/fixtures' +import { getPool } from '@/tests/pg/setup' + +/** + * Covers 20260618120001_commit_method_agent_provenance: + * - journal_entries.commit_method accepts the new 'agent' and 'api_key' + * values (MCP-relayed approvals — agent_first_vision.md §8 P0-1). + * - The pre-existing values are still accepted. + * - Unknown values are still rejected by the CHECK constraint. + * - Exactly one commit_method constraint exists (guards against the + * DROP CONSTRAINT IF EXISTS missing a differently-named original, which + * would leave the old, narrower CHECK in force). + */ + +async function postWithCommitMethod(commitMethod: string): Promise { + const { userId, companyId, fiscalPeriodId } = await seedCompany() + const entryId = await insertDraftJournalEntry({ userId, companyId, fiscalPeriodId }) + await insertBalancedLines(entryId) + // draft → posted with commit metadata — same transition the commit RPC does. + await getPool().query( + `UPDATE public.journal_entries + SET status = 'posted', voucher_number = 1, commit_method = $2 + WHERE id = $1`, + [entryId, commitMethod], + ) + return entryId +} + +describe('journal_entries.commit_method — agent provenance values', () => { + it.each(['agent', 'api_key', 'user_accept', 'bulk_accept'])( + 'accepts commit_method=%s', + async (method) => { + const entryId = await postWithCommitMethod(method) + const { rows } = await getPool().query( + `SELECT commit_method, status FROM public.journal_entries WHERE id = $1`, + [entryId], + ) + expect(rows[0]).toEqual({ commit_method: method, status: 'posted' }) + }, + ) + + it('rejects values outside the CHECK list', async () => { + await expect(postWithCommitMethod('robot')).rejects.toMatchObject({ + // 23514 = check_violation + code: '23514', + }) + }) + + it('exactly one commit_method CHECK constraint exists, under the canonical name', async () => { + const { rows } = await getPool().query( + `SELECT conname + FROM pg_constraint + WHERE conrelid = 'public.journal_entries'::regclass + AND conname LIKE '%commit_method%'`, + ) + expect(rows.map((r) => r.conname)).toEqual(['journal_entries_commit_method_check']) + }) +})