From 1fa34aa7cada5cdd416e59f885f084a3ab05f991 Mon Sep 17 00:00:00 2001 From: Mattsson <111893710+mattssonn@users.noreply.github.com> Date: Tue, 25 Aug 2026 12:09:20 +0200 Subject: [PATCH] feat(skatteverket): repair notification recipients + make the agent the SKV notification surface (#1887) * feat(skatteverket): repair notification recipients + make the agent the SKV notification surface The company_members -> profiles!inner(email) PostgREST embed has no FK to traverse (company_members.user_id references auth.users), so it 400'd and silently killed all four notification emails since they shipped. Recipient lookup is now a shared two-step helper (lib/notifications/member-email): kvittens confirmations, skattekonto drift alerts (tax-contact routing preserved via the plural variant) and backup alerts deliver again. The connection-expired email is deleted instead of fixed: with SKV's 65-minute personal sessions it was one mail per connect (see DECISIONS.md); the event and needs_reconsent flagging stay. For MCP-first users the agent is the notification surface, so: - SKATTEVERKET_NOT_CONNECTED copy is now agent-directive: session expiry is normal (~1h by SKV design), only a person can reconnect with BankID, do not retry until they confirm. Inline strings (declaration-status, read routes, v1 pitfalls, accounted-api skill) aligned. - gnubok_get_agent_briefing gains an optional skatteverket_connection block (status/source/connected_at + directive message on needs_reconsent), emitted only when a connection or verified system grant exists, so agents warn the user at session start instead of failing mid-task. Payload bench ceiling bumped 59.95K -> 60.15K for the outputSchema contract. Co-Authored-By: Claude Fable 5 * fix(skatteverket): drift email resolves recipients via service client; review fixes The skeptic pass refuted the drift-email repair: skattekonto.drift_detected is emitted only by the nightly cron, and the extension registry builds each event handler a fresh ctx from the anonymous cookie client (or none at all on cookieless requests), so RLS returned zero company_members rows and the two-step lookup still resolved no recipient. The handler now builds its own service-role client, the same documented pattern as the retired connection-expired handler; drift tests exercise the handler without ctx, matching the cron reality. CodeRabbit findings: resolveMemberEmails pages both queries through fetchAllRows with stable ordering (PostgREST caps unpaged reads at 1000 rows); the v1 vat-declarations pitfall and regenerated accounted-api docs now name both auth paths (member BankID connection or verified ombud grant); the briefing's system-before-user priority carries a cross-reference to resolveReadAuth explaining why it is not reused. member-email.ts JSDoc states the service-role-client requirement (profiles RLS is own-row-only). Co-Authored-By: Claude Fable 5 --------- Co-authored-by: Claude Fable 5 --- DECISIONS.md | 3 + .../skatteverket/vat-declarations/route.ts | 2 +- .../lib/__tests__/backup-alert.test.ts | 14 +- .../general/cloud-backup/lib/backup-alert.ts | 24 +- .../__tests__/agent-briefing.test.ts | 108 +++++++- .../__tests__/payload-size.bench.test.ts | 8 +- extensions/general/mcp-server/server.ts | 63 +++++ .../connection-expired-notification.test.ts | 230 ---------------- .../__tests__/kvittens-notification.test.ts | 10 +- .../__tests__/resolve-auth.test.ts | 3 + .../__tests__/skattekonto-drift-email.test.ts | 110 +++++--- extensions/general/skatteverket/index.ts | 12 +- .../lib/connection-expired-notification.ts | 246 ------------------ .../skatteverket/lib/declaration-status.ts | 4 +- .../skatteverket/lib/kvittens-notification.ts | 25 +- .../general/skatteverket/lib/resolve-auth.ts | 14 +- .../lib/skattekonto-drift-email.ts | 73 ++---- lib/errors/structured-errors.ts | 12 +- .../__tests__/member-email.test.ts | 199 ++++++++++++++ lib/notifications/member-email.ts | 139 ++++++++++ skills/accounted-api/references/periods.md | 2 +- 21 files changed, 664 insertions(+), 637 deletions(-) delete mode 100644 extensions/general/skatteverket/__tests__/connection-expired-notification.test.ts delete mode 100644 extensions/general/skatteverket/lib/connection-expired-notification.ts create mode 100644 lib/notifications/__tests__/member-email.test.ts create mode 100644 lib/notifications/member-email.ts diff --git a/DECISIONS.md b/DECISIONS.md index 12daf160..5d2391c9 100644 --- a/DECISIONS.md +++ b/DECISIONS.md @@ -1194,6 +1194,9 @@ One line per decision: `[YYYY-MM-DD] : `. Appended by agents and [2026-08-24] Single-call chat console (general.help, AskConsole → /api/agent/ask) now carries the thread's earlier turns into every model call, via a new optional `history` on the provider-agnostic GenerateTextRequest (real message turns before the prompt in BOTH adapters: Anthropic-family messages array, OpenAI-compatible via AI SDK `messages`; an absent/empty history leaves the request byte-identical to the single-turn call, so hosted extraction and every other caller are untouched). The 08-20 RIP-3 cutover made each turn stateless (conversationId was only the tool actor id), so a follow-up in a resumed thread was answered blind (user report: "frågar vad jag refererar till"). History is loaded server-side from agent_messages (loadChatHistory: text only, hidden + tool rows dropped, alternation repaired, newest 16 rows / 10k chars) rather than sent by the client, so the client cannot forge earlier turns and old streaming threads replay cleanly. Rejected: inlining a transcript into the prompt (works everywhere but weaker turn semantics and blurs data vs instructions) and loading history in AskConsole (client-trusted history). Separately: the docked assistant panel now remembers its open thread per tab in sessionStorage (lib/agent-panel/session-restore) and reopens it after a full reload (the deploy prompt's "Ladda om" wiped it); sessionStorage, not user_preferences, because this is this-tab-this-session state that must not follow the user to other devices or tabs. And DeployReloadPrompt's full-width wrapper gets pointer-events-none: at z-[60] after the panel in DOM order it swallowed clicks on the panel's composer ("går ej att skriva"). [2026-08-25] /reports/bank-reconciliation retired behind a redirect to /reconciliation instead of kept as a "power" page: everything it did (matcher, manual N:1 matching, residual booking, IB tag, move-to-account) lives on the account-keyed page, and two reconciliation surfaces meant two truths. The catalog slug stays so old links, the report library and ?autorun=1 deep links keep working. [2026-08-25] reconciliation_residual staged op tiered 'medium', not create_voucher's 'high': it books one typed verifikat (6570/8410/8310/3740 vs bank) bounded by RESIDUAL_MAX_AMOUNT and is undone by storno + unmatch, i.e. the same blast radius as categorize_transaction. Scope is transactions:write (same as the v1 route) because it writes the ledger. +[2026-08-25] Notification recipient lookup is a two-step query (lib/notifications/member-email), not the company_members -> profiles!inner(email) embed and not a new FK: company_members.user_id references auth.users, so PostgREST has no relationship to traverse and the embed 400'd, silently killing all four notification emails (kvittens, drift, backup, connection-expired) since they shipped. Adding an FK to profiles would be a migration on a core tenancy table for zero functional gain. Second lesson recorded: the drift path DID log the failure and nobody read it, so the guard is post-deploy delivery verification, not more logging. +[2026-08-25] The skattekonto connection-expired email is deleted, not fixed: SKV's per-flow refresh tokens live 65 minutes, so per-consent-episode dedup means one "your connection expired" mail per connect, arriving an hour after every successful BankID login; that trains users to ignore mail. Rejected alternative (kept for revisit): fix + throttle to one mail per user per 7 days, fired only when a scheduled sync actually failed. Residual accepted knowingly: web-only users now have NO proactive channel for a dead SKV connection (banner needs a visit, briefing needs an agent, drift email needs a working sync); the agent briefing's skatteverket_connection block and the rewritten SKATTEVERKET_NOT_CONNECTED copy are the compensating surfaces. The skattekonto.connection.expired event and needs_reconsent flagging stay. +[2026-08-25] SKATTEVERKET_NOT_CONNECTED stays one code for both never-connected and expired: splitting would ripple through every consumer (error-map, v1 routes, MCP dispatch, UI), and the declaration-status path already differentiates in its message. The copy is agent-directive on purpose (only a person can run BankID; do not retry until the user confirms) because the old English copy "Reconnect with BankID before retrying" invited agents to retry something only a human can fix. [2026-08-24] Manual reconciliation adapter (Reko bilagor, PR 1) computes the ledger side per fiscal period via generateTrialBalance (IB + movement through the balansdag), never as an all-history sumAccountBalance: year-end posts an opening_balance verifikat that re-books every balance account in the new year, so an all-history sum counts a closed year twice. Reskontra/semesterskuld specifications are "per idag" (open items now), labeled so in the bridge; a per-date reskontra is a follow-up. A typed external_balance is accepted only on manual accounts without a system specification (EXTERNAL_BALANCE_NOT_ALLOWED elsewhere): letting a stated number override the bank, Skatteverket or the reskontra would hide the very difference the sign-off exists to record. [2026-08-24] Reconciliation underlag (Reko bilagor, PR 2) is its own table (account_reconciliation_attachments) scoped by (company, account_key, through_date), not extra columns on document_attachments: that table's link is a verifikat and its WORM version chain is about digitized receipts, while a bilaga belongs to a balansdag and may be attached before the sign-off exists. Files stay in the `documents` bucket under `documents//reconciliation/...` so the bucket's company-scoped RLS applies unchanged; removal is a stamp (never a delete, BFL 7 kap.) enforced by trigger; the full archive copies the files into `bilagor/` with a hash manifest. No v1 API endpoints in this PR on purpose: the concurrent reconciliation-residual work edits the v1 route loader, spec snapshot and scopes, and files cannot be uploaded by an agent anyway. [2026-08-24] Bokslut checklist (Reko bilagor, PR 3) keeps the item catalogue in code and only the per-period state in bokslut_checklist_items: steps the system can judge (drafts, voucher gaps, trial balance, sign-offs through balansdagen, reskontra tie-outs) are computed live every time and a stored row only overrides them, so the checklist never claims a state the ledger contradicts; manual steps (inventering, osäkra fordringar, dispositioner) are what the konsult ticks. Mutable on purpose (a late verifikat reopens a step), no DELETE policy. The missing-fiscal-year check is a pure helper reused by the readiness warnings and the SIE import result; the non-adjacent previous_period_id fix is #1849 and is not duplicated here. diff --git a/app/api/v1/companies/[companyId]/skatteverket/vat-declarations/route.ts b/app/api/v1/companies/[companyId]/skatteverket/vat-declarations/route.ts index bb7fb8bd..b4827bbc 100644 --- a/app/api/v1/companies/[companyId]/skatteverket/vat-declarations/route.ts +++ b/app/api/v1/companies/[companyId]/skatteverket/vat-declarations/route.ts @@ -62,7 +62,7 @@ registerEndpoint({ doNotUseFor: 'Computing the declaration from the books (use the VAT report), or filing: submission is a separate BankID-signed flow.', pitfalls: [ - 'This is a live Skatteverket read: it fails with SKATTEVERKET_NOT_CONNECTED (401) until someone in the company has connected with BankID under Installningar, and the response reflects SKV\'s state, not the books.', + 'This is a live Skatteverket read: it fails with SKATTEVERKET_NOT_CONNECTED (401) when the company has neither a member\'s BankID connection (made under Installningar) nor a verified ombud grant, and the response reflects SKV\'s state, not the books. Personal BankID sessions expire after ~1 hour by design, so an expired connection is normal: ask the user to reconnect; only a person can, so do not retry until they confirm.', 'submitted=null and decided=null with HTTP 200 means "nothing on file for the period": it is not an error.', 'A submitted declaration can lack a beslut for days: poll decided separately rather than assuming both appear together.', 'redovisningsperiod is SKV\'s YYYYMM format (the period\'s LAST month): quarterly period 1 is 03, not 01.', diff --git a/extensions/general/cloud-backup/lib/__tests__/backup-alert.test.ts b/extensions/general/cloud-backup/lib/__tests__/backup-alert.test.ts index c6cfc5ad..d32b25ac 100644 --- a/extensions/general/cloud-backup/lib/__tests__/backup-alert.test.ts +++ b/extensions/general/cloud-backup/lib/__tests__/backup-alert.test.ts @@ -80,8 +80,8 @@ describe('shouldSendBackupAlert', () => { }) /** - * Supabase stub: company_members lookup resolves a member with an email, - * company_settings resolves a company name. + * Supabase stub: the two-step recipient lookup (company_members membership + * check, then profiles email), and company_settings resolves a company name. */ function makeSupabase(options: { member?: unknown; companyName?: string | null } = {}) { const from = vi.fn().mockImplementation((table: string) => { @@ -91,14 +91,16 @@ function makeSupabase(options: { member?: unknown; companyName?: string | null } maybeSingle: vi.fn().mockImplementation(() => { if (table === 'company_members') { return Promise.resolve({ - data: - options.member !== undefined - ? options.member - : { user_id: 'u-1', profiles: { email: 'emil@example.com' } }, + data: options.member !== undefined ? options.member : { user_id: 'u-1' }, + error: null, }) } + if (table === 'profiles') { + return Promise.resolve({ data: { email: 'emil@example.com' }, error: null }) + } return Promise.resolve({ data: { company_name: options.companyName ?? 'Testbolag AB' }, + error: null, }) }), } diff --git a/extensions/general/cloud-backup/lib/backup-alert.ts b/extensions/general/cloud-backup/lib/backup-alert.ts index 9fed2f6d..fda7d0e0 100644 --- a/extensions/general/cloud-backup/lib/backup-alert.ts +++ b/extensions/general/cloud-backup/lib/backup-alert.ts @@ -14,6 +14,7 @@ import type { SupabaseClient } from '@supabase/supabase-js' import { getEmailService } from '@/lib/email/service' import { createLogger } from '@/lib/logger' +import { resolveMemberEmail } from '@/lib/notifications/member-email' const log = createLogger('cloud-backup-alert') @@ -114,29 +115,6 @@ export async function sendBackupFailureAlert( } } -/** - * The recipient must still be an active member of the company: a schedule - * owner who has since been removed must not receive alerts for it. - */ -async function resolveMemberEmail( - supabase: SupabaseClient, - companyId: string, - userId: string -): Promise { - const { data: member } = await supabase - .from('company_members') - .select('user_id, profiles!inner(email)') - .eq('company_id', companyId) - .eq('user_id', userId) - .maybeSingle() - if (!member) return null - - type ProfileRef = { email?: string | null } | { email?: string | null }[] | null - const profiles = (member as { profiles: ProfileRef }).profiles - const profile = Array.isArray(profiles) ? profiles[0] : profiles - return profile?.email ?? null -} - async function fetchCompanyName( supabase: SupabaseClient, companyId: string diff --git a/extensions/general/mcp-server/__tests__/agent-briefing.test.ts b/extensions/general/mcp-server/__tests__/agent-briefing.test.ts index 66afd463..4447fde5 100644 --- a/extensions/general/mcp-server/__tests__/agent-briefing.test.ts +++ b/extensions/general/mcp-server/__tests__/agent-briefing.test.ts @@ -2,7 +2,7 @@ * Tests for gnubok_get_agent_briefing: session-bootstrap context for the * specialized accountant agent over MCP. */ -import { describe, it, expect, vi, beforeEach } from 'vitest' +import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest' import { tools } from '../server' import { RECOMMENDED_WORKFLOW_LOADOUTS, assertRecommendedLoadoutsValid } from '../recommended-tools' import { workflowSkills } from '../skills' @@ -57,6 +57,8 @@ function mockSupabase(opts: { dimensionValueRows?: Array<{ dimension_id: string; code: string; name: string }> dimensionRuleRows?: Array<{ account_number: string; rule_type: string; dimension_id: string }> dimensionsEnabled?: boolean + // skatteverket_tokens rows for the connection-health block. Default: none. + skvTokenRows?: Array<{ user_id: string; status: string | null; created_at: string | null }> errors?: { profile?: string; memory?: string; atoms?: string } }) { const profile = opts.profile === undefined ? null : opts.profile @@ -171,6 +173,9 @@ function mockSupabase(opts: { // PR10: the briefing surfaces active rules; default none. return chainResolving(opts.dimensionRuleRows ?? []) } + if (table === 'skatteverket_tokens') { + return chainResolving(opts.skvTokenRows ?? []) + } throw new Error(`Unexpected table in test mock: ${table}`) }), } @@ -455,6 +460,107 @@ describe('gnubok_get_agent_briefing tool', () => { }) }) +describe('skatteverket_connection health block', () => { + const tool = () => tools.find((t) => t.name === 'gnubok_get_agent_briefing')! + const originalEnabled = process.env.SKATTEVERKET_ENABLED + + beforeEach(() => { + process.env.SKATTEVERKET_ENABLED = 'true' + }) + + afterEach(() => { + if (originalEnabled === undefined) delete process.env.SKATTEVERKET_ENABLED + else process.env.SKATTEVERKET_ENABLED = originalEnabled + }) + + it('omits the block entirely for a company that never connected', async () => { + const supabase = mockSupabase({ profile: null, skvTokenRows: [] }) + const result = (await tool().execute( + {}, + 'company-1', + 'user-1', + supabase as never, + { type: 'api_key' } + )) as Record + expect('skatteverket_connection' in result).toBe(false) + }) + + it('omits the block when the skatteverket extension is disabled, even with a token row', async () => { + process.env.SKATTEVERKET_ENABLED = 'false' + const supabase = mockSupabase({ + profile: null, + skvTokenRows: [{ user_id: 'user-1', status: 'active', created_at: '2026-08-25T09:12:00Z' }], + }) + const result = (await tool().execute( + {}, + 'company-1', + 'user-1', + supabase as never, + { type: 'api_key' } + )) as Record + expect('skatteverket_connection' in result).toBe(false) + }) + + it('reports an active personal session with its connect time (the ~65 min fuse)', async () => { + const supabase = mockSupabase({ + profile: null, + skvTokenRows: [{ user_id: 'user-1', status: 'active', created_at: '2026-08-25T09:12:00Z' }], + }) + const result = (await tool().execute( + {}, + 'company-1', + 'user-1', + supabase as never, + { type: 'api_key' } + )) as { + skatteverket_connection?: { + status: string + source: string + connected_at?: string | null + message?: string + } + } + expect(result.skatteverket_connection).toEqual({ + status: 'active', + source: 'user', + connected_at: '2026-08-25T09:12:00Z', + }) + }) + + it('reports needs_reconsent with the directive message so the agent warns the user up front', async () => { + const supabase = mockSupabase({ + profile: null, + skvTokenRows: [ + { user_id: 'user-1', status: 'needs_reconsent', created_at: '2026-08-25T08:00:00Z' }, + ], + }) + const result = (await tool().execute( + {}, + 'company-1', + 'user-1', + supabase as never, + { type: 'api_key' } + )) as { + skatteverket_connection?: { status: string; source: string; message?: string } + } + expect(result.skatteverket_connection).toMatchObject({ + status: 'needs_reconsent', + source: 'user', + }) + expect(result.skatteverket_connection?.message).toContain('BankID') + expect(result.skatteverket_connection?.message).toContain('ca 1 timme') + }) + + it('declares the block in the outputSchema as optional (never required)', () => { + const output = tool().outputSchema as { + properties: Record + required: string[] + } + expect(output.properties.skatteverket_connection).toBeDefined() + expect(output.required).not.toContain('skatteverket_connection') + }) +}) + describe('recommended_tools workflow loadouts (issue #1098)', () => { it('every tool name in every loadout exists in the actual tool registry (drift guard)', () => { const known = new Set(tools.map((t) => t.name)) diff --git a/extensions/general/mcp-server/__tests__/payload-size.bench.test.ts b/extensions/general/mcp-server/__tests__/payload-size.bench.test.ts index cfbfeb55..31ca110e 100644 --- a/extensions/general/mcp-server/__tests__/payload-size.bench.test.ts +++ b/extensions/general/mcp-server/__tests__/payload-size.bench.test.ts @@ -203,9 +203,15 @@ describe('tools/list payload size guard', () => { // silently, and agents inferred "consumed" from status 'failed' both // ways. The enum is the contract; the description is one clause; // headroom before the change was ~15 tokens, so even that crossed. + // * 60K to 60.2K with skatteverket_connection on the briefing: the + // connection-health block (status/source/connected_at) that lets an + // agent warn the user about a dead 65-minute SKV session at session + // start instead of mid-task. The runtime block is emitted only for + // companies with a connection; this cost is the outputSchema contract + // (~140 tokens), already trimmed to two short description strings. // Long-term answer to growth is leaning harder on gnubok_search_tools: if this // fires again, prefer trimming descriptions or making a tool opt-in via search // before bumping further. - expect(approxTokens).toBeLessThan(60_000) + expect(approxTokens).toBeLessThan(60_200) }) }) diff --git a/extensions/general/mcp-server/server.ts b/extensions/general/mcp-server/server.ts index d7a45ddd..cdf3dd25 100644 --- a/extensions/general/mcp-server/server.ts +++ b/extensions/general/mcp-server/server.ts @@ -260,6 +260,8 @@ import { readAgiSubmissionStatus } from '@/extensions/general/skatteverket/lib/a import { buildMomsuppgift, resolveRedovisare } from '@/extensions/general/skatteverket/lib/declaration-prep' import { writeSkatteverketAudit } from '@/extensions/general/skatteverket/lib/audit' import { skvAuthCodeToStructured } from '@/extensions/general/skatteverket/lib/error-map' +import { findCompanyTokenUser, hasVerifiedGrant } from '@/extensions/general/skatteverket/lib/resolve-auth' +import { getSystemAuthMode, isSystemAuthConfigured } from '@/extensions/general/skatteverket/lib/system-auth/config' import { formatRedovisningsperiod } from '@/lib/skatteverket/format' import { createExtensionContext } from '@/lib/extensions/context-factory' import { commitPendingOperation } from '@/lib/pending-operations/commit' @@ -3812,6 +3814,22 @@ export const tools: McpTool[] = [ }, required: ['tool', 'when', 'include'], }, + skatteverket_connection: { + type: 'object', + additionalProperties: false, + description: + 'Present only when a Skatteverket connection exists. needs_reconsent: only a person can fix it (BankID under Inställningar → Skatteverket); warn the user before starting SKV work.', + properties: { + status: { type: 'string', enum: ['active', 'needs_reconsent'] }, + source: { type: 'string', enum: ['user', 'system'] }, + connected_at: { + type: ['string', 'null'], + description: 'Personal sessions last ~65 min from this time; absent for system (ombud) connections.', + }, + message: { type: 'string' }, + }, + required: ['status', 'source'], + }, }, required: ['company', 'user_name', 'profile_summary', 'atoms', 'memory', 'recommended_tools'], }, @@ -3876,6 +3894,49 @@ export const tools: McpTool[] = [ } })() + // Skatteverket connection health, so the agent warns the user at + // session start instead of discovering a dead session mid-task. + // Best-effort and emitted only when a connection (or verified system + // grant) exists: never-connected companies pay no payload for it. + // The system-before-user priority mirrors resolveReadAuth + // (skatteverket/lib/resolve-auth.ts); not reused directly because the + // briefing needs token metadata (createdAt, reconsent status) that + // resolveReadAuth deliberately collapses into an auth result. + const safeSkvConnection = (async (): Promise< + | { + status: 'active' | 'needs_reconsent' + source: 'user' | 'system' + connected_at?: string | null + message?: string + } + | null + > => { + try { + if (process.env.SKATTEVERKET_ENABLED !== 'true') return null + if ( + getSystemAuthMode() === 'on' && + isSystemAuthConfigured() && + (await hasVerifiedGrant(companyId, 'lasombud')) + ) { + return { status: 'active', source: 'system' } + } + const token = await findCompanyTokenUser(supabase, companyId) + if (!token) return null + if (token.needsReconsent) { + return { + status: 'needs_reconsent', + source: 'user', + connected_at: token.createdAt, + message: + 'Skatteverket-sessionen har gått ut. Skatteverkets personliga inloggning gäller bara ca 1 timme, så detta är normalt. Be användaren ansluta igen med BankID under Inställningar → Skatteverket; bara en person kan göra det, så försök inte med Skatteverket-verktyg förrän användaren bekräftat.', + } + } + return { status: 'active', source: 'user', connected_at: token.createdAt } + } catch { + return null + } + })() + const [profileRes, memoryRes, userRes, companyRes, settingsRes, dimensionsRes] = await Promise.all([ supabase .from('agent_profiles') @@ -4068,6 +4129,7 @@ export const tools: McpTool[] = [ } const ledgerDigest = await safeLedgerDigest + const skvConnection = await safeSkvConnection return { company, @@ -4083,6 +4145,7 @@ export const tools: McpTool[] = [ })), ...(dimensionsBlock ? { dimensions: dimensionsBlock } : {}), ...(ledgerDigest ? { ledger_context: ledgerDigest } : {}), + ...(skvConnection ? { skatteverket_connection: skvConnection } : {}), // Static per-workflow loadouts (issue #1098): lets a deferred-loading // harness batch-load a whole workflow cluster in one call. Validated // against the tool registry at module init (assertRecommendedLoadoutsValid). diff --git a/extensions/general/skatteverket/__tests__/connection-expired-notification.test.ts b/extensions/general/skatteverket/__tests__/connection-expired-notification.test.ts deleted file mode 100644 index bb21cee4..00000000 --- a/extensions/general/skatteverket/__tests__/connection-expired-notification.test.ts +++ /dev/null @@ -1,230 +0,0 @@ -/** - * Connection-expired email: the atomic claim-then-send dedup. - * - * One email per consent episode: the reference key is derived from - * (userId, token.created_at), claimed through notification_log under type - * 'skv_connection_expired' (partial unique index from migration - * 20260720090000). A re-observed expiry of the same token never notifies - * twice; a reconnect (new token row, new created_at) resets the episode. - */ -import { describe, it, expect, vi, beforeEach } from 'vitest' -import { createHash } from 'crypto' -import type { SupabaseClient } from '@supabase/supabase-js' - -const mockIsConfigured = vi.fn() -const mockSendEmail = vi.fn() -vi.mock('@/lib/email/service', () => ({ - getEmailService: () => ({ isConfigured: mockIsConfigured, sendEmail: mockSendEmail }), -})) - -import { sendConnectionExpiredNotification } from '../lib/connection-expired-notification' - -interface RecordedOp { - table: string - op: 'select' | 'insert' | 'delete' - payload?: Record - filters: Record -} - -const TOKEN_CREATED_AT = '2026-06-30T13:09:50.729013+00:00' - -function expectedReferenceUuid(userId: string, createdAt: string): string { - const hex = createHash('sha256') - .update(`skv-connection-expired|${userId}|${createdAt}`) - .digest('hex') - return `${hex.slice(0, 8)}-${hex.slice(8, 12)}-${hex.slice(12, 16)}-${hex.slice(16, 20)}-${hex.slice(20, 32)}` -} - -/** - * Hand-rolled mock (instead of createQueuedMockSupabase) because the - * assertions need the recorded operation ORDER and payloads: claim insert - * before send, delete filters on release. - */ -function makeSupabase(opts: { - token?: { created_at: string } | null - alreadyRow?: { id: string } | null - member?: Record | null - insertError?: { code?: string; message: string } | null -} = {}) { - const ops: RecordedOp[] = [] - const from = (table: string) => { - const call: RecordedOp = { table, op: 'select', filters: {} } - ops.push(call) - const builder: Record = {} - Object.assign(builder, { - select: () => builder, - insert: (payload: Record) => { - call.op = 'insert' - call.payload = payload - return Promise.resolve({ data: null, error: opts.insertError ?? null }) - }, - delete: () => { - call.op = 'delete' - return builder - }, - eq: (key: string, value: unknown) => { - call.filters[key] = value - return builder - }, - maybeSingle: async () => { - if (table === 'skatteverket_tokens') { - const token = - opts.token === undefined ? { created_at: TOKEN_CREATED_AT } : opts.token - return { data: token, error: null } - } - if (table === 'notification_log') return { data: opts.alreadyRow ?? null, error: null } - if (table === 'company_members') { - const member = - opts.member === undefined - ? { user_id: 'user-1', profiles: { email: 'user@example.com' } } - : opts.member - return { data: member, error: null } - } - return { data: null, error: null } - }, - then: (resolve: (v: unknown) => void) => resolve({ data: null, error: null }), - }) - return builder - } - return { supabase: { from } as unknown as SupabaseClient, ops } -} - -const baseInput = { companyId: 'company-1', userId: 'user-1' } - -beforeEach(() => { - vi.clearAllMocks() - mockIsConfigured.mockReturnValue(true) - mockSendEmail.mockResolvedValue({ success: true }) -}) - -describe('sendConnectionExpiredNotification', () => { - it('claims the notification_log row BEFORE sending the email', async () => { - const { supabase, ops } = makeSupabase() - let claimsAtSendTime = -1 - mockSendEmail.mockImplementation(async () => { - claimsAtSendTime = ops.filter((o) => o.op === 'insert').length - return { success: true } - }) - - const result = await sendConnectionExpiredNotification(supabase, baseInput) - - expect(result).toEqual({ sent: true }) - expect(claimsAtSendTime).toBe(1) - const insert = ops.find((o) => o.op === 'insert') - expect(insert?.table).toBe('notification_log') - expect(insert?.payload).toMatchObject({ - user_id: 'user-1', - company_id: 'company-1', - notification_type: 'skv_connection_expired', - reference_id: expectedReferenceUuid('user-1', TOKEN_CREATED_AT), - delivery_status: 'sent', - }) - }) - - it('sends the reconnect instructions to the token owner', async () => { - const { supabase } = makeSupabase() - - await sendConnectionExpiredNotification(supabase, baseInput) - - expect(mockSendEmail).toHaveBeenCalledTimes(1) - const mail = mockSendEmail.mock.calls[0][0] as { - to: string - subject: string - text: string - } - expect(mail.to).toBe('user@example.com') - expect(mail.subject).toContain('Skatteverket') - expect(mail.text).toContain('BankID') - expect(mail.text).toContain('alla behörigheter') - expect(mail.text).toContain('/settings/tax') - }) - - it('skips via the fast path when the episode was already notified', async () => { - const { supabase } = makeSupabase({ alreadyRow: { id: 'log-1' } }) - - const result = await sendConnectionExpiredNotification(supabase, baseInput) - - expect(result).toEqual({ sent: false, reason: 'duplicate' }) - expect(mockSendEmail).not.toHaveBeenCalled() - }) - - it('treats a 23505 unique violation on the claim as a duplicate', async () => { - const { supabase } = makeSupabase({ - insertError: { code: '23505', message: 'duplicate key value' }, - }) - - const result = await sendConnectionExpiredNotification(supabase, baseInput) - - expect(result).toEqual({ sent: false, reason: 'duplicate' }) - expect(mockSendEmail).not.toHaveBeenCalled() - }) - - it('skips the send when the claim insert fails for another reason', async () => { - const { supabase } = makeSupabase({ - insertError: { code: '57014', message: 'canceling statement' }, - }) - - const result = await sendConnectionExpiredNotification(supabase, baseInput) - - expect(result).toEqual({ sent: false, reason: 'claim_failed' }) - expect(mockSendEmail).not.toHaveBeenCalled() - }) - - it('does nothing when the email service is not configured', async () => { - mockIsConfigured.mockReturnValue(false) - const { supabase, ops } = makeSupabase() - - const result = await sendConnectionExpiredNotification(supabase, baseInput) - - expect(result).toEqual({ sent: false, reason: 'email_not_configured' }) - expect(ops).toHaveLength(0) - expect(mockSendEmail).not.toHaveBeenCalled() - }) - - it('keys the episode on the (user, company) token row, not the user alone', async () => { - const { supabase, ops } = makeSupabase() - - await sendConnectionExpiredNotification(supabase, baseInput) - - // Rows are per (user, company): a multi-company operator holds several, - // and a user-only maybeSingle() errored out and silently skipped the - // email (#1673 follow-through). - const tokenRead = ops.find((o) => o.table === 'skatteverket_tokens') - expect(tokenRead?.filters).toMatchObject({ user_id: 'user-1', company_id: 'company-1' }) - }) - - it('skips when no token row exists (already disconnected)', async () => { - const { supabase } = makeSupabase({ token: null }) - - const result = await sendConnectionExpiredNotification(supabase, baseInput) - - expect(result).toEqual({ sent: false, reason: 'no_token' }) - expect(mockSendEmail).not.toHaveBeenCalled() - }) - - it('skips without claiming when the token owner is no longer a member', async () => { - const { supabase, ops } = makeSupabase({ member: null }) - - const result = await sendConnectionExpiredNotification(supabase, baseInput) - - expect(result).toEqual({ sent: false, reason: 'no_recipient' }) - expect(ops.find((o) => o.op === 'insert')).toBeUndefined() - expect(mockSendEmail).not.toHaveBeenCalled() - }) - - it('releases the claim when the email send reports failure', async () => { - mockSendEmail.mockResolvedValue({ success: false, error: 'smtp down' }) - const { supabase, ops } = makeSupabase() - - const result = await sendConnectionExpiredNotification(supabase, baseInput) - - expect(result).toEqual({ sent: false, reason: 'send_failed' }) - const del = ops.find((o) => o.op === 'delete') - expect(del?.table).toBe('notification_log') - expect(del?.filters).toMatchObject({ - user_id: 'user-1', - notification_type: 'skv_connection_expired', - reference_id: expectedReferenceUuid('user-1', TOKEN_CREATED_AT), - }) - }) -}) diff --git a/extensions/general/skatteverket/__tests__/kvittens-notification.test.ts b/extensions/general/skatteverket/__tests__/kvittens-notification.test.ts index dd5e3c91..e9beef49 100644 --- a/extensions/general/skatteverket/__tests__/kvittens-notification.test.ts +++ b/extensions/general/skatteverket/__tests__/kvittens-notification.test.ts @@ -56,13 +56,15 @@ function makeSupabase(opts: { }, maybeSingle: async () => { if (table === 'notification_log') return { data: opts.alreadyRow ?? null, error: null } + // Two-step recipient lookup (lib/notifications/member-email): the + // membership check first, then the profile email. if (table === 'company_members') { - const member = - opts.member === undefined - ? { user_id: 'user-1', profiles: { email: 'user@example.com' } } - : opts.member + const member = opts.member === undefined ? { user_id: 'user-1' } : opts.member return { data: member, error: null } } + if (table === 'profiles') { + return { data: { email: 'user@example.com' }, error: null } + } return { data: null, error: null } }, then: (resolve: (v: unknown) => void) => resolve({ data: null, error: null }), diff --git a/extensions/general/skatteverket/__tests__/resolve-auth.test.ts b/extensions/general/skatteverket/__tests__/resolve-auth.test.ts index 1a234b6d..bcaa9e86 100644 --- a/extensions/general/skatteverket/__tests__/resolve-auth.test.ts +++ b/extensions/general/skatteverket/__tests__/resolve-auth.test.ts @@ -256,15 +256,18 @@ describe('findCompanyTokenUser', () => { expect(await findCompanyTokenUser(supabase, 'company-1')).toEqual({ userId: 'user-b', needsReconsent: false, + createdAt: '2', }) expect(await findCompanyTokenUser(supabase, 'company-1', { preferUserId: 'user-a' })).toEqual({ userId: 'user-a', needsReconsent: false, + createdAt: '1', }) // Preferring a user whose row is dead still yields the live row. expect(await findCompanyTokenUser(supabase, 'company-1', { preferUserId: 'user-c' })).toEqual({ userId: 'user-b', needsReconsent: false, + createdAt: '2', }) }) }) diff --git a/extensions/general/skatteverket/__tests__/skattekonto-drift-email.test.ts b/extensions/general/skatteverket/__tests__/skattekonto-drift-email.test.ts index d4724471..6d61c7f3 100644 --- a/extensions/general/skatteverket/__tests__/skattekonto-drift-email.test.ts +++ b/extensions/general/skatteverket/__tests__/skattekonto-drift-email.test.ts @@ -9,7 +9,6 @@ */ import { describe, it, expect, vi, beforeEach } from 'vitest' import type { SupabaseClient } from '@supabase/supabase-js' -import type { ExtensionContext } from '@/lib/extensions/types' import type { EventPayload } from '@/lib/events/types' const { warnRecorder } = vi.hoisted(() => ({ warnRecorder: vi.fn() })) @@ -33,6 +32,17 @@ vi.mock('@/lib/email/service', () => ({ getEmailService: () => ({ isConfigured: mockIsConfigured, sendEmail: mockSendEmail }), })) +// The handler builds its own SERVICE-ROLE client: the only emitter is the +// nightly cron, where the registry-built ctx is an anonymous (or absent) +// client that RLS would turn into "no members, no recipient". +const { serviceClientHolder } = vi.hoisted(() => ({ + serviceClientHolder: { current: null as unknown }, +})) +vi.mock('@/lib/supabase/server', () => ({ + createClient: vi.fn(), + createServiceClient: vi.fn(() => serviceClientHolder.current), +})) + import { handleSkattekontoDriftDetected } from '../lib/skattekonto-drift-email' interface QueryRecord { @@ -41,26 +51,30 @@ interface QueryRecord { filters: Record } -type MemberRow = { user_id: string; profiles: { email: string } } +type MemberRow = { user_id: string; email: string } -const OWNER: MemberRow = { user_id: 'user-1', profiles: { email: 'owner@example.com' } } -const ACCOUNTANT: MemberRow = { user_id: 'user-2', profiles: { email: 'revisor@byra.se' } } +const OWNER: MemberRow = { user_id: 'user-1', email: 'owner@example.com' } +const ACCOUNTANT: MemberRow = { user_id: 'user-2', email: 'revisor@byra.se' } /** * Hand-rolled mock (instead of createQueuedMockSupabase) because the * assertions need the selected COLUMN list per table: the bug this covers was * a select of a column that does not exist on company_settings. + * + * Recipient resolution is the two-step lookup from + * lib/notifications/member-email: company_members yields user ids, profiles + * yields their emails via an .in() read. */ function makeSupabase( opts: { members?: MemberRow[] | null membersError?: { message: string } | null + profilesError?: { message: string } | null settings?: Record | null settingsError?: { message: string } | null - profile?: { email: string } | null - profileError?: { message: string } | null } = {}, ) { + const members = opts.members === undefined ? [OWNER] : opts.members ?? [] const queries: QueryRecord[] = [] const from = (table: string) => { const record: QueryRecord = { table, filters: {} } @@ -68,7 +82,7 @@ function makeSupabase( const result = () => { if (table === 'company_members') { return { - data: opts.members === undefined ? [OWNER] : opts.members, + data: opts.membersError ? null : members.map((m) => ({ user_id: m.user_id })), error: opts.membersError ?? null, } } @@ -86,8 +100,10 @@ function makeSupabase( } if (table === 'profiles') { return { - data: opts.profile === undefined ? { email: OWNER.profiles.email } : opts.profile, - error: opts.profileError ?? null, + data: opts.profilesError + ? null + : members.map((m) => ({ id: m.user_id, email: m.email })), + error: opts.profilesError ?? null, } } return { data: null, error: null } @@ -102,7 +118,17 @@ function makeSupabase( record.filters[key] = value return builder }, - maybeSingle: async () => result(), + in: (key: string, value: unknown) => { + record.filters[key] = value + return builder + }, + order: () => builder, + range: () => builder, + maybeSingle: async () => { + const r = result() + const rows = r.data as Array> | null + return { data: Array.isArray(rows) ? rows[0] ?? null : rows, error: r.error } + }, then: (resolve: (v: unknown) => void) => resolve(result()), }) return builder @@ -110,13 +136,9 @@ function makeSupabase( return { supabase: { from } as unknown as SupabaseClient, queries } } -function makeCtx(supabase: SupabaseClient): ExtensionContext { - return { - userId: 'user-1', - companyId: 'company-1', - extensionId: 'skatteverket', - supabase, - } as unknown as ExtensionContext +/** Point the mocked createServiceClient at this test's supabase stub. */ +function useServiceClient(supabase: SupabaseClient): void { + serviceClientHolder.current = supabase } const payload: EventPayload<'skattekonto.drift_detected'> = { @@ -149,22 +171,24 @@ describe('handleSkattekontoDriftDetected recipient resolution', () => { it('sends to the configured tax contact when it belongs to an active member', async () => { const { supabase } = makeSupabase({ members: [OWNER, ACCOUNTANT], - settings: { tax_contact_email: ACCOUNTANT.profiles.email }, + settings: { tax_contact_email: ACCOUNTANT.email }, }) - await handleSkattekontoDriftDetected(payload, makeCtx(supabase)) + useServiceClient(supabase) + await handleSkattekontoDriftDetected(payload) expect(mockSendEmail).toHaveBeenCalledTimes(1) - expect(sentTo()).toBe(ACCOUNTANT.profiles.email) + expect(sentTo()).toBe(ACCOUNTANT.email) }) it('reads tax_contact_email: the column the settings UI actually writes', async () => { const { supabase, queries } = makeSupabase({ members: [OWNER, ACCOUNTANT], - settings: { tax_contact_email: ACCOUNTANT.profiles.email }, + settings: { tax_contact_email: ACCOUNTANT.email }, }) - await handleSkattekontoDriftDetected(payload, makeCtx(supabase)) + useServiceClient(supabase) + await handleSkattekontoDriftDetected(payload) const settingsQuery = queries.find((q) => q.table === 'company_settings') // Exact match, not `toContain`: 'tax_contact_email' contains the phantom @@ -179,7 +203,8 @@ describe('handleSkattekontoDriftDetected recipient resolution', () => { settings: { tax_contact_email: 'Revisor@Byra.se' }, }) - await handleSkattekontoDriftDetected(payload, makeCtx(supabase)) + useServiceClient(supabase) + await handleSkattekontoDriftDetected(payload) expect(sentTo()).toBe('Revisor@Byra.se') }) @@ -187,9 +212,10 @@ describe('handleSkattekontoDriftDetected recipient resolution', () => { it('falls back to the syncing user when no tax contact is configured', async () => { const { supabase } = makeSupabase({ members: [OWNER], settings: null }) - await handleSkattekontoDriftDetected(payload, makeCtx(supabase)) + useServiceClient(supabase) + await handleSkattekontoDriftDetected(payload) - expect(sentTo()).toBe(OWNER.profiles.email) + expect(sentTo()).toBe(OWNER.email) }) it('refuses a tax contact that is not an active member, and says so', async () => { @@ -198,9 +224,10 @@ describe('handleSkattekontoDriftDetected recipient resolution', () => { settings: { tax_contact_email: 'ex-admin@example.com' }, }) - await handleSkattekontoDriftDetected(payload, makeCtx(supabase)) + useServiceClient(supabase) + await handleSkattekontoDriftDetected(payload) - expect(sentTo()).toBe(OWNER.profiles.email) + expect(sentTo()).toBe(OWNER.email) expect(warnedWith('not an active member')).toBe(true) }) @@ -211,11 +238,12 @@ describe('handleSkattekontoDriftDetected recipient resolution', () => { settingsError: { message: 'column company_settings.tax_contact_email does not exist' }, }) - await handleSkattekontoDriftDetected(payload, makeCtx(supabase)) + useServiceClient(supabase) + await handleSkattekontoDriftDetected(payload) expect(warnedWith('could not read tax contact email')).toBe(true) // Still delivers to the documented fallback rather than dropping the alert. - expect(sentTo()).toBe(OWNER.profiles.email) + expect(sentTo()).toBe(OWNER.email) }) it('does not silently swallow a member lookup error', async () => { @@ -224,30 +252,32 @@ describe('handleSkattekontoDriftDetected recipient resolution', () => { membersError: { message: 'permission denied for table company_members' }, }) - await handleSkattekontoDriftDetected(payload, makeCtx(supabase)) + useServiceClient(supabase) + await handleSkattekontoDriftDetected(payload) expect(warnedWith('could not read company members')).toBe(true) expect(mockSendEmail).not.toHaveBeenCalled() }) - it('does not silently swallow a profile lookup error on the fallback path', async () => { + it('does not silently swallow a member-email lookup error', async () => { const { supabase } = makeSupabase({ members: [OWNER], settings: null, - profile: null, - profileError: { message: 'timeout' }, + profilesError: { message: 'timeout' }, }) - await handleSkattekontoDriftDetected(payload, makeCtx(supabase)) + useServiceClient(supabase) + await handleSkattekontoDriftDetected(payload) - expect(warnedWith('could not read syncing user profile')).toBe(true) + expect(warnedWith('could not read member emails')).toBe(true) expect(mockSendEmail).not.toHaveBeenCalled() }) it('sends nothing when the company has no members at all', async () => { const { supabase } = makeSupabase({ members: [] }) - await handleSkattekontoDriftDetected(payload, makeCtx(supabase)) + useServiceClient(supabase) + await handleSkattekontoDriftDetected(payload) expect(mockSendEmail).not.toHaveBeenCalled() expect(warnedWith('no authorised recipient')).toBe(true) @@ -257,7 +287,8 @@ describe('handleSkattekontoDriftDetected recipient resolution', () => { mockIsConfigured.mockReturnValue(false) const { supabase, queries } = makeSupabase({ members: [OWNER, ACCOUNTANT] }) - await handleSkattekontoDriftDetected(payload, makeCtx(supabase)) + useServiceClient(supabase) + await handleSkattekontoDriftDetected(payload) expect(mockSendEmail).not.toHaveBeenCalled() expect(queries).toHaveLength(0) @@ -266,10 +297,11 @@ describe('handleSkattekontoDriftDetected recipient resolution', () => { it('keeps the drift figures out of the email body', async () => { const { supabase } = makeSupabase({ members: [OWNER, ACCOUNTANT], - settings: { tax_contact_email: ACCOUNTANT.profiles.email }, + settings: { tax_contact_email: ACCOUNTANT.email }, }) - await handleSkattekontoDriftDetected(payload, makeCtx(supabase)) + useServiceClient(supabase) + await handleSkattekontoDriftDetected(payload) const body = `${mockSendEmail.mock.calls[0][0].text}${mockSendEmail.mock.calls[0][0].html}` expect(body).not.toContain('1250') diff --git a/extensions/general/skatteverket/index.ts b/extensions/general/skatteverket/index.ts index c466aa38..f245c319 100644 --- a/extensions/general/skatteverket/index.ts +++ b/extensions/general/skatteverket/index.ts @@ -55,7 +55,6 @@ import { SkattekontoBookingError, } from './lib/skattekonto-booking' import { handleSkattekontoDriftDetected } from './lib/skattekonto-drift-email' -import { handleSkattekontoConnectionExpired } from './lib/connection-expired-notification' import { findMatchCandidates, findMatchSuggestionsBulk, @@ -196,7 +195,8 @@ function readAuthFailureResponse(reason: 'no_token' | 'needs_reconsent'): NextRe if (reason === 'needs_reconsent') { return NextResponse.json( { - error: 'Anslutningen mot Skatteverket behöver förnyas. Anslut igen med BankID.', + error: + 'Anslutningen mot Skatteverket har gått ut. Skatteverkets inloggning gäller bara ca 1 timme, så detta är normalt. Anslut igen med BankID.', code: 'SESSION_EXPIRED', }, { status: 401 }, @@ -2483,15 +2483,15 @@ export const skatteverketExtension: Extension = { }, ], + // skattekonto.connection.expired is still emitted (needs_reconsent flagging, + // UI banner, agent briefing) but has no email consumer: with SKV's 65-minute + // personal sessions a per-episode expiry mail is one mail per connect, which + // trains users to ignore it. See DECISIONS.md 2026-08-25. eventHandlers: [ { eventType: 'skattekonto.drift_detected', handler: handleSkattekontoDriftDetected, }, - { - eventType: 'skattekonto.connection.expired', - handler: handleSkattekontoConnectionExpired, - }, ], // Registry-resolved commit services for the MCP submit tools. The core diff --git a/extensions/general/skatteverket/lib/connection-expired-notification.ts b/extensions/general/skatteverket/lib/connection-expired-notification.ts deleted file mode 100644 index c4f49191..00000000 --- a/extensions/general/skatteverket/lib/connection-expired-notification.ts +++ /dev/null @@ -1,246 +0,0 @@ -/** - * Connection-expired notification (email). - * - * Handler for `skattekonto.connection.expired`: emitted by syncSkattekonto - * when a terminal auth error (REFRESH_EXHAUSTED / SESSION_EXPIRED / - * TOKEN_CORRUPTED) kills the personal SKV token. Before this handler the - * event was emitted but unconsumed: the needs_reconsent state was only - * visible on the settings panel, which users have no reason to revisit, so - * a dead connection silently meant "skattekonto never syncs again". Prod - * accumulated ~70 companies in exactly that state. - * - * Email is the delivery channel (same rationale as kvittens-notification: - * push-notifications is a separate, optional extension and cross-extension - * imports are not allowed). The recipient is the token OWNER: only they can - * redo the BankID consent, so a company contact address would be actionable - * for the wrong person. The owner must still be an active member of the - * company. - * - * Deduplication: one email per consent episode. The reference key is derived - * from (userId, token.created_at): a reconnect creates a new token row with - * a new created_at, so a later expiry notifies again, while the nightly cron - * re-observing the same dead token stays silent. Claim-first through - * notification_log under type 'skv_connection_expired', made atomic by the - * partial unique index in migration 20260720090000 (mirrors the kvittens - * pattern). - * - * Best-effort by contract: a notification failure must never fail the sync - * (or cron) that observed the expiry. The body carries no financial data. - */ -import { createHash } from 'crypto' -import type { SupabaseClient } from '@supabase/supabase-js' -import type { EventPayload } from '@/lib/events/types' -import { createServiceClient } from '@/lib/supabase/server' -import { getEmailService } from '@/lib/email/service' -import { createLogger } from '@/lib/logger' - -const log = createLogger('skv-connection-expired-notification') - -export async function handleSkattekontoConnectionExpired( - payload: EventPayload<'skattekonto.connection.expired'>, -): Promise { - // Service-role client, NOT the registry-built ctx: the primary emitter is - // the nightly cron, whose request carries no user cookies, so the ctx the - // registry lazily builds there is an anonymous client that RLS turns into - // "no token, no member, nothing to do". Same rationale as the - // document-extraction handler. - const supabase = createServiceClient() - await sendConnectionExpiredNotification(supabase, { - companyId: payload.companyId, - userId: payload.userId, - }) -} - -export interface ConnectionExpiredInput { - companyId: string - /** The token-owning user: the only person who can redo the BankID consent. */ - userId: string -} - -export async function sendConnectionExpiredNotification( - supabase: SupabaseClient, - input: ConnectionExpiredInput, -): Promise<{ sent: boolean; reason?: string }> { - try { - const email = getEmailService() - if (!email.isConfigured()) return { sent: false, reason: 'email_not_configured' } - - // Episode key: the token row's created_at. No token row means nothing to - // reconnect (already disconnected): skip. Rows are per (user, company): - // without the company filter a multi-company operator's second row made - // maybeSingle() error out and the email was never sent. - const { data: token } = await supabase - .from('skatteverket_tokens') - .select('created_at') - .eq('user_id', input.userId) - .eq('company_id', input.companyId) - .maybeSingle() - const tokenCreatedAt = (token as { created_at?: string | null } | null)?.created_at - if (!tokenCreatedAt) return { sent: false, reason: 'no_token' } - - const referenceUuid = toReferenceUuid( - `skv-connection-expired|${input.userId}|${tokenCreatedAt}`, - ) - - // Dedup fast path: cheap read that skips the work below on re-observed - // expiries. NOT the enforcement: the claim insert further down is. - const { data: already } = await supabase - .from('notification_log') - .select('id') - .eq('user_id', input.userId) - .eq('notification_type', 'skv_connection_expired') - .eq('reference_id', referenceUuid) - .maybeSingle() - if (already) return { sent: false, reason: 'duplicate' } - - const recipient = await resolveMemberEmail(supabase, input.companyId, input.userId) - if (!recipient) { - log.info('no authorised recipient for connection-expired email', { - companyId: input.companyId, - }) - return { sent: false, reason: 'no_recipient' } - } - - // Claim before sending: the partial unique index on notification_log - // (user_id, reference_id) where notification_type = - // 'skv_connection_expired' makes this atomic. Of two overlapping - // emitters exactly one wins the insert; the loser gets a unique - // violation and skips the send. - const { error: claimError } = await supabase.from('notification_log').insert({ - user_id: input.userId, - company_id: input.companyId, - notification_type: 'skv_connection_expired', - reference_id: referenceUuid, - days_before: 0, - delivery_status: 'sent', - }) - if (claimError) { - if (claimError.code === '23505') return { sent: false, reason: 'duplicate' } - // Without a claim we cannot guarantee single delivery: skip the send - // (fail closed on the never-twice guarantee) and let a later - // observation retry. - log.warn('connection-expired claim insert failed', { - companyId: input.companyId, - error: claimError.message, - }) - return { sent: false, reason: 'claim_failed' } - } - - const appUrl = (process.env.NEXT_PUBLIC_APP_URL || 'https://gnubok.se').replace(/\/$/, '') - const settingsLink = `${appUrl}/settings/tax` - - const subject = 'Anslutningen till Skatteverket behöver förnyas' - const text = [ - 'Din anslutning till Skatteverket har gått ut. Hämtningen av skattekontots saldo och transaktioner är pausad tills du ansluter igen.', - '', - 'Så här återansluter du:', - `1. Gå till Inställningar > Skatt i Accounted: ${settingsLink}`, - '2. Klicka på "Anslut igen" och legitimera dig med BankID.', - '3. Godkänn alla behörigheter på Skatteverkets samtyckessida.', - '', - 'När du anslutit hämtas skattekontot direkt.', - ].join('\n') - const html = [ - '

Din anslutning till Skatteverket har gått ut. Hämtningen av skattekontots saldo och transaktioner är pausad tills du ansluter igen.

', - '

Så här återansluter du:

', - '
    ', - `
  1. Gå till Inställningar > Skatt i Accounted.
    2. `, - '
  2. Klicka på "Anslut igen" och legitimera dig med BankID.
    3. ', - '
  3. Godkänn alla behörigheter på Skatteverkets samtyckessida.
    4. ', - '
', - '

När du anslutit hämtas skattekontot direkt.

', - ].join('') - - let result: Awaited> - try { - result = await email.sendEmail({ to: recipient, subject, text, html }) - } catch (sendErr) { - // Release the claim so a later observation can retry the send. - await releaseClaim(supabase, input.userId, referenceUuid) - throw sendErr - } - if (!result.success) { - log.warn('connection-expired email send failed', { - companyId: input.companyId, - error: result.error, - }) - await releaseClaim(supabase, input.userId, referenceUuid) - return { sent: false, reason: 'send_failed' } - } - - return { sent: true } - } catch (err) { - log.warn('connection-expired notification failed', { - companyId: input.companyId, - error: err instanceof Error ? err.message : String(err), - }) - return { sent: false, reason: 'error' } - } -} - -/** - * notification_log.reference_id is a uuid column; the episode key is a - * composite string. Map it to a deterministic uuid-shaped SHA-256 digest so - * the same episode always resolves to the same claim row (same approach as - * kvittens-notification). - */ -function toReferenceUuid(referenceKey: string): string { - const hex = createHash('sha256').update(referenceKey).digest('hex') - return `${hex.slice(0, 8)}-${hex.slice(8, 12)}-${hex.slice(12, 16)}-${hex.slice(16, 20)}-${hex.slice(20, 32)}` -} - -/** Remove a claim whose email never went out, so a later run can retry. */ -async function releaseClaim( - supabase: SupabaseClient, - userId: string, - referenceUuid: string, -): Promise { - try { - await supabase - .from('notification_log') - .delete() - .eq('user_id', userId) - .eq('notification_type', 'skv_connection_expired') - .eq('reference_id', referenceUuid) - } catch (err) { - // A stuck claim only suppresses a retry of this one email: log and move on. - log.warn('failed to release connection-expired claim', { - userId, - referenceUuid, - error: err instanceof Error ? err.message : String(err), - }) - } -} - -/** - * The recipient must still be an active member of the company (mirrors the - * kvittens rule): a token owner who has since been removed from the company - * must not receive connection nudges for it. - */ -async function resolveMemberEmail( - supabase: SupabaseClient, - companyId: string, - userId: string, -): Promise { - const { data: member } = await supabase - .from('company_members') - .select('user_id, profiles!inner(email)') - .eq('company_id', companyId) - .eq('user_id', userId) - .maybeSingle() - if (!member) return null - - type ProfileRef = { email?: string | null } | { email?: string | null }[] | null - const profiles = (member as { profiles: ProfileRef }).profiles - const profile = Array.isArray(profiles) ? profiles[0] : profiles - return profile?.email ?? null -} - -function escapeHtml(input: string): string { - return input - .replace(/&/g, '&') - .replace(//g, '>') - .replace(/"/g, '"') - .replace(/'/g, ''') -} diff --git a/extensions/general/skatteverket/lib/declaration-status.ts b/extensions/general/skatteverket/lib/declaration-status.ts index e321ec43..1f85b2d6 100644 --- a/extensions/general/skatteverket/lib/declaration-status.ts +++ b/extensions/general/skatteverket/lib/declaration-status.ts @@ -91,8 +91,8 @@ export async function fetchVatDeclarationStatus( http_status: 401, error: resolved.reason === 'needs_reconsent' - ? 'Anslutningen mot Skatteverket behöver förnyas. Anslut igen med BankID.' - : 'Inte ansluten till Skatteverket.', + ? 'Anslutningen mot Skatteverket behöver förnyas: Skatteverkets personliga inloggning gäller bara ca 1 timme, så detta är normalt. Be användaren ansluta igen med BankID under Inställningar → Skatteverket. Bara en person kan göra det; försök inte igen förrän användaren bekräftat.' + : 'Inte ansluten till Skatteverket. Be användaren ansluta med BankID under Inställningar → Skatteverket.', } } diff --git a/extensions/general/skatteverket/lib/kvittens-notification.ts b/extensions/general/skatteverket/lib/kvittens-notification.ts index cba3fd2e..95bc83c0 100644 --- a/extensions/general/skatteverket/lib/kvittens-notification.ts +++ b/extensions/general/skatteverket/lib/kvittens-notification.ts @@ -26,6 +26,7 @@ import { createHash } from 'crypto' import type { SupabaseClient } from '@supabase/supabase-js' import { getEmailService } from '@/lib/email/service' import { createLogger } from '@/lib/logger' +import { resolveMemberEmail } from '@/lib/notifications/member-email' const log = createLogger('skv-kvittens-notification') @@ -172,30 +173,6 @@ async function releaseClaim( } } -/** - * The recipient must still be an active member of the company (mirrors the - * drift-email rule): a token owner who has since been removed from the - * company must not receive filing confirmations for it. - */ -async function resolveMemberEmail( - supabase: SupabaseClient, - companyId: string, - userId: string -): Promise { - const { data: member } = await supabase - .from('company_members') - .select('user_id, profiles!inner(email)') - .eq('company_id', companyId) - .eq('user_id', userId) - .maybeSingle() - if (!member) return null - - type ProfileRef = { email?: string | null } | { email?: string | null }[] | null - const profiles = (member as { profiles: ProfileRef }).profiles - const profile = Array.isArray(profiles) ? profiles[0] : profiles - return profile?.email ?? null -} - function escapeHtml(input: string): string { return input .replace(/&/g, '&') diff --git a/extensions/general/skatteverket/lib/resolve-auth.ts b/extensions/general/skatteverket/lib/resolve-auth.ts index 7c6ef7ff..3575532c 100644 --- a/extensions/general/skatteverket/lib/resolve-auth.ts +++ b/extensions/general/skatteverket/lib/resolve-auth.ts @@ -105,6 +105,12 @@ export async function resolveReadAuth( export interface CompanyTokenUser { userId: string needsReconsent: boolean + /** + * When the serving row was issued (last consent or refresh; storeTokens is + * DELETE + INSERT). Personal SKV sessions live ~65 minutes from this time, + * so consumers (the agent briefing) can reason about likely expiry. + */ + createdAt: string | null } /** @@ -141,8 +147,11 @@ export async function findCompanyTokenUser( return null } - const rows = ((data ?? []) as Array<{ user_id: string | null; status: string | null }>).filter( - (row): row is { user_id: string; status: string | null } => typeof row.user_id === 'string' + const rows = ( + (data ?? []) as Array<{ user_id: string | null; status: string | null; created_at: string | null }> + ).filter( + (row): row is { user_id: string; status: string | null; created_at: string | null } => + typeof row.user_id === 'string' ) if (rows.length === 0) return null @@ -154,5 +163,6 @@ export async function findCompanyTokenUser( return { userId: pick.user_id, needsReconsent: pick.status === 'needs_reconsent', + createdAt: pick.created_at ?? null, } } diff --git a/extensions/general/skatteverket/lib/skattekonto-drift-email.ts b/extensions/general/skatteverket/lib/skattekonto-drift-email.ts index 3f253313..2f4fadbe 100644 --- a/extensions/general/skatteverket/lib/skattekonto-drift-email.ts +++ b/extensions/general/skatteverket/lib/skattekonto-drift-email.ts @@ -1,5 +1,8 @@ +import type { SupabaseClient } from '@supabase/supabase-js' import { getEmailService } from '@/lib/email/service' import { createLogger } from '@/lib/logger' +import { resolveMemberEmails } from '@/lib/notifications/member-email' +import { createServiceClient } from '@/lib/supabase/server' import { formatDate } from '@/lib/utils' import type { ExtensionContext } from '@/lib/extensions/types' import type { EventPayload } from '@/lib/events/types' @@ -14,6 +17,12 @@ const log = createLogger('skattekonto-drift-email') * (the dashboard SkattekontoDriftTile) so a misdelivered mail doesn't leak * financial figures. * + * Service-role client, NOT the registry-built ctx: the only emitter is the + * nightly cron, whose request carries no user cookies, so the ctx the + * registry lazily builds there is an anonymous client (or undefined) that + * RLS turns into "no members, no recipient, no mail". Same rationale as the + * document-extraction handler and the retired connection-expired handler. + * * Recipient resolution is restricted to active members of the company. A * stale company_settings.tax_contact_email that no longer corresponds to a * member is never used. Falls back to the syncing user only if they're @@ -24,15 +33,8 @@ const log = createLogger('skattekonto-drift-email') */ export async function handleSkattekontoDriftDetected( payload: EventPayload<'skattekonto.drift_detected'>, - ctx?: ExtensionContext, + _ctx?: ExtensionContext, ): Promise { - if (!ctx) { - log.warn('drift event fired without ctx: cannot resolve recipient', { - companyId: payload.companyId, - }) - return - } - const email = getEmailService() if (!email.isConfigured()) { log.info('email service not configured: skipping drift alert', { @@ -41,7 +43,8 @@ export async function handleSkattekontoDriftDetected( return } - const recipient = await resolveAuthorisedRecipient(ctx, payload.userId) + const supabase = createServiceClient() + const recipient = await resolveAuthorisedRecipient(supabase, payload.companyId, payload.userId) if (!recipient) { log.warn('no authorised recipient resolved for drift alert', { companyId: payload.companyId, @@ -120,39 +123,24 @@ export async function handleSkattekontoDriftDetected( * whoever happened to trigger the sync. */ async function resolveAuthorisedRecipient( - ctx: ExtensionContext, + supabase: SupabaseClient, + companyId: string, userId: string, ): Promise { // 1. Build the set of active member emails for this company. We accept - // only addresses that appear here. - const { data: members, error: membersError } = await ctx.supabase - .from('company_members') - .select('user_id, profiles!inner(email)') - .eq('company_id', ctx.companyId) - - if (membersError) { - // Without the allowlist every candidate is rejected below, so a failed - // lookup silently cancels the alert. Say so out loud. - log.warn('could not read company members for drift alert', { - companyId: ctx.companyId, - error: membersError.message, - }) - } - - type MemberRow = { user_id: string; profiles: { email?: string | null } | { email?: string | null }[] | null } + // only addresses that appear here. A failed lookup resolves to an empty + // map, cancelling the alert: the helper logs it out loud. + const memberEmails = await resolveMemberEmails(supabase, companyId) const allowedEmails = new Set() - for (const m of (members ?? []) as MemberRow[]) { - const profile = Array.isArray(m.profiles) ? m.profiles[0] : m.profiles - if (profile?.email) allowedEmails.add(profile.email.toLowerCase()) - } + for (const email of memberEmails.values()) allowedEmails.add(email.toLowerCase()) if (allowedEmails.size === 0) return null // 2. Prefer the configured tax contact email IF it matches an active member. - const { data: settings, error: settingsError } = await ctx.supabase + const { data: settings, error: settingsError } = await supabase .from('company_settings') .select('tax_contact_email') - .eq('company_id', ctx.companyId) + .eq('company_id', companyId) .maybeSingle() if (settingsError) { @@ -160,7 +148,7 @@ async function resolveAuthorisedRecipient( // trace is how a company silently stops getting alerts where it asked // for them. log.warn('could not read tax contact email: falling back to syncing user', { - companyId: ctx.companyId, + companyId, error: settingsError.message, }) } @@ -171,24 +159,13 @@ async function resolveAuthorisedRecipient( return contactEmail } log.warn('configured tax contact is not an active member: falling back to syncing user', { - companyId: ctx.companyId, + companyId, }) } - // 3. Fall back to the syncing user's email if they're still a member. - const { data: profile, error: profileError } = await ctx.supabase - .from('profiles') - .select('email') - .eq('id', userId) - .maybeSingle() - if (profileError) { - log.warn('could not read syncing user profile for drift alert', { - companyId: ctx.companyId, - userId, - error: profileError.message, - }) - } - const userEmail = (profile as { email?: string | null } | null)?.email + // 3. Fall back to the syncing user's email: present in the map only while + // they are still a member. + const userEmail = memberEmails.get(userId) if (userEmail && allowedEmails.has(userEmail.toLowerCase())) { return userEmail } diff --git a/lib/errors/structured-errors.ts b/lib/errors/structured-errors.ts index b942f50a..c40dcbce 100644 --- a/lib/errors/structured-errors.ts +++ b/lib/errors/structured-errors.ts @@ -3473,14 +3473,20 @@ const SKATTEVERKET: Record = { message_sv: 'Skatteverket-integrationen är inte aktiverad i denna miljö.', message_en: 'The Skatteverket integration is not enabled in this environment.', }, + // One code covers both never-connected and expired: splitting it would + // ripple through every consumer, and the declaration-status path already + // differentiates in its message (DECISIONS.md 2026-08-25). The copy is + // agent-directive on purpose: only a person can run the BankID flow, so + // the agent must hand the task to the user instead of retrying. SKATTEVERKET_NOT_CONNECTED: { httpStatus: 401, message_sv: - 'Anslutningen till Skatteverket saknas eller har gått ut. Anslut med BankID under Inställningar → Skatteverket.', - message_en: 'No valid Skatteverket connection. Reconnect with BankID before retrying.', + 'Anslutningen till Skatteverket saknas eller har gått ut. Om företaget varit anslutet tidigare är detta normalt: Skatteverkets personliga inloggning gäller bara ca 1 timme. Be användaren ansluta (igen) med BankID under Inställningar → Skatteverket.', + message_en: + 'The Skatteverket connection is missing or has expired. If the company was connected before this is expected: Skatteverket personal sessions last only about 1 hour. Tell the user to connect (or reconnect) with BankID under Inställningar → Skatteverket in Accounted. Only a person can do this; do not retry until they confirm they have reconnected.', remediation: { description: - 'Connect (or reconnect) to Skatteverket with BankID under Settings → Skatteverket, then retry.', + 'A person must connect (or reconnect) to Skatteverket with BankID under Inställningar → Skatteverket. Personal Skatteverket sessions expire after about 1 hour by SKV design, so an expired session is normal, not a fault. Do not retry until the user confirms they have reconnected.', }, }, SKATTEVERKET_ACCESS_DENIED: { diff --git a/lib/notifications/__tests__/member-email.test.ts b/lib/notifications/__tests__/member-email.test.ts new file mode 100644 index 00000000..086e981b --- /dev/null +++ b/lib/notifications/__tests__/member-email.test.ts @@ -0,0 +1,199 @@ +/** + * Recipient resolution for notification emails. + * + * The regression this guards: the four notification senders used the + * PostgREST embed company_members → profiles!inner(email), but no FK links + * those tables, so the embed 400'd and every recipient resolved to null. + * The helper resolves membership and email as two separate queries. + */ +import { describe, it, expect, vi, beforeEach } from 'vitest' +import type { SupabaseClient } from '@supabase/supabase-js' + +const { warnRecorder } = vi.hoisted(() => ({ warnRecorder: vi.fn() })) + +vi.mock('@/lib/logger', () => ({ + createLogger: () => ({ + info: vi.fn(), + warn: warnRecorder, + error: vi.fn(), + child() { + return this + }, + }), +})) + +import { resolveMemberEmail, resolveMemberEmails } from '../member-email' + +interface QueryRecord { + table: string + columns?: string + filters: Record +} + +function makeSupabase(opts: { + memberRows?: Array<{ user_id: string | null }> | null + membersError?: { message: string } | null + profileRows?: Array<{ id: string; email: string | null }> | null + profilesError?: { message: string } | null +}) { + const queries: QueryRecord[] = [] + const from = (table: string) => { + const record: QueryRecord = { table, filters: {} } + queries.push(record) + const result = () => { + if (table === 'company_members') { + return { + data: opts.membersError ? null : opts.memberRows ?? [], + error: opts.membersError ?? null, + } + } + if (table === 'profiles') { + return { + data: opts.profilesError ? null : opts.profileRows ?? [], + error: opts.profilesError ?? null, + } + } + return { data: null, error: null } + } + const builder: Record = {} + Object.assign(builder, { + select: (columns: string) => { + record.columns = columns + return builder + }, + eq: (key: string, value: unknown) => { + record.filters[key] = value + return builder + }, + in: (key: string, value: unknown) => { + record.filters[key] = value + return builder + }, + order: () => builder, + range: () => builder, + maybeSingle: async () => { + const r = result() + const rows = r.data as Array> | null + return { data: rows?.[0] ?? null, error: r.error } + }, + then: (resolve: (v: unknown) => void) => resolve(result()), + }) + return builder + } + return { supabase: { from } as unknown as SupabaseClient, queries } +} + +function warnedWith(fragment: string): boolean { + return warnRecorder.mock.calls.some( + (call) => typeof call[0] === 'string' && call[0].includes(fragment), + ) +} + +beforeEach(() => { + vi.clearAllMocks() +}) + +describe('resolveMemberEmail', () => { + it('resolves an active member to their profile email via two queries (no embed)', async () => { + const { supabase, queries } = makeSupabase({ + memberRows: [{ user_id: 'user-1' }], + profileRows: [{ id: 'user-1', email: 'user@example.com' }], + }) + + const email = await resolveMemberEmail(supabase, 'company-1', 'user-1') + + expect(email).toBe('user@example.com') + const memberQuery = queries.find((q) => q.table === 'company_members') + // The embed column list is exactly what broke: assert it is gone. + expect(memberQuery?.columns).toBe('user_id') + expect(memberQuery?.filters).toMatchObject({ company_id: 'company-1', user_id: 'user-1' }) + const profileQuery = queries.find((q) => q.table === 'profiles') + expect(profileQuery?.columns).toBe('email') + expect(profileQuery?.filters).toMatchObject({ id: 'user-1' }) + }) + + it('returns null for a user who is no longer a member (no profile query at all)', async () => { + const { supabase, queries } = makeSupabase({ + memberRows: [], + profileRows: [{ id: 'user-1', email: 'user@example.com' }], + }) + + expect(await resolveMemberEmail(supabase, 'company-1', 'user-1')).toBeNull() + expect(queries.some((q) => q.table === 'profiles')).toBe(false) + }) + + it('returns null when the profile has no email', async () => { + const { supabase } = makeSupabase({ + memberRows: [{ user_id: 'user-1' }], + profileRows: [{ id: 'user-1', email: null }], + }) + expect(await resolveMemberEmail(supabase, 'company-1', 'user-1')).toBeNull() + }) + + it('logs and returns null (never throws) on a member query error', async () => { + const { supabase } = makeSupabase({ + membersError: { message: 'permission denied for table company_members' }, + }) + expect(await resolveMemberEmail(supabase, 'company-1', 'user-1')).toBeNull() + expect(warnedWith('could not read company members')).toBe(true) + }) + + it('logs and returns null (never throws) on a profile query error', async () => { + const { supabase } = makeSupabase({ + memberRows: [{ user_id: 'user-1' }], + profilesError: { message: 'timeout' }, + }) + expect(await resolveMemberEmail(supabase, 'company-1', 'user-1')).toBeNull() + expect(warnedWith('could not read profile email')).toBe(true) + }) +}) + +describe('resolveMemberEmails', () => { + it('maps every member with a profile email, keyed by user id', async () => { + const { supabase, queries } = makeSupabase({ + memberRows: [{ user_id: 'user-1' }, { user_id: 'user-2' }, { user_id: 'user-3' }], + profileRows: [ + { id: 'user-1', email: 'owner@example.com' }, + { id: 'user-2', email: 'revisor@byra.se' }, + { id: 'user-3', email: null }, + ], + }) + + const emails = await resolveMemberEmails(supabase, 'company-1') + + expect(emails.get('user-1')).toBe('owner@example.com') + expect(emails.get('user-2')).toBe('revisor@byra.se') + expect(emails.has('user-3')).toBe(false) + const memberQuery = queries.find((q) => q.table === 'company_members') + expect(memberQuery?.columns).toBe('user_id') + const profileQuery = queries.find((q) => q.table === 'profiles') + expect(profileQuery?.columns).toBe('id, email') + expect(profileQuery?.filters).toMatchObject({ id: ['user-1', 'user-2', 'user-3'] }) + }) + + it('returns an empty map for a company with no members (no profile query)', async () => { + const { supabase, queries } = makeSupabase({ memberRows: [] }) + const emails = await resolveMemberEmails(supabase, 'company-1') + expect(emails.size).toBe(0) + expect(queries.some((q) => q.table === 'profiles')).toBe(false) + }) + + it('logs and returns an empty map on a member query error', async () => { + const { supabase } = makeSupabase({ + membersError: { message: 'permission denied for table company_members' }, + }) + const emails = await resolveMemberEmails(supabase, 'company-1') + expect(emails.size).toBe(0) + expect(warnedWith('could not read company members')).toBe(true) + }) + + it('logs and returns an empty map on a profiles query error', async () => { + const { supabase } = makeSupabase({ + memberRows: [{ user_id: 'user-1' }], + profilesError: { message: 'timeout' }, + }) + const emails = await resolveMemberEmails(supabase, 'company-1') + expect(emails.size).toBe(0) + expect(warnedWith('could not read member emails')).toBe(true) + }) +}) diff --git a/lib/notifications/member-email.ts b/lib/notifications/member-email.ts new file mode 100644 index 00000000..7c5ab312 --- /dev/null +++ b/lib/notifications/member-email.ts @@ -0,0 +1,139 @@ +/** + * Recipient resolution for notification emails. + * + * Every outbound notification restricts its recipient to active members of + * the company: a token owner, schedule owner, or configured contact who has + * since been removed must never receive company mail (the bare existence of + * some notifications is sensitive financial signal). + * + * The lookup is deliberately two queries. `company_members.user_id` + * references auth.users, not public.profiles, so the PostgREST embed + * `profiles!inner(email)` has no foreign-key relationship to traverse and + * fails the whole query with a 400. That embed shipped in four notification + * senders and silently killed all of them: the recipient resolved to null + * and every mail was skipped as "no recipient". Adding the FK instead would + * mean a migration on a core tenancy table for zero functional gain + * (see DECISIONS.md 2026-08-25). + * + * Both functions are best-effort and never throw: a failed lookup logs and + * resolves to "no recipient", because notification delivery must never fail + * the sync/cron/reconciliation that triggered it. Logging alone is not the + * safety net (the embed bug WAS logged by one caller and nobody read it): + * callers verify delivery end to end after deploy. + * + * SERVICE-ROLE CLIENT REQUIRED. Under an RLS user client these lookups + * silently degrade: company_members is readable company-wide, but the + * profiles SELECT policy is own-row-only, so resolveMemberEmails would + * return at most the caller's own email and resolveMemberEmail(other user) + * always null. Every current caller is a service-role cron path; keep it + * that way or widen the profiles policy first. + */ +import type { SupabaseClient } from '@supabase/supabase-js' +import { createLogger } from '@/lib/logger' +import { fetchAllRows } from '@/lib/supabase/fetch-all' + +const log = createLogger('member-email') + +/** + * Resolve one user's email, only if they are still an active member of the + * company. Returns null (never throws) when the user is not a member, has no + * profile email, or a query fails. + */ +export async function resolveMemberEmail( + supabase: SupabaseClient, + companyId: string, + userId: string +): Promise { + const { data: member, error: memberError } = await supabase + .from('company_members') + .select('user_id') + .eq('company_id', companyId) + .eq('user_id', userId) + .maybeSingle() + if (memberError) { + log.warn('could not read company members for notification recipient', { + companyId, + userId, + error: memberError.message, + }) + return null + } + if (!member) return null + + const { data: profile, error: profileError } = await supabase + .from('profiles') + .select('email') + .eq('id', userId) + .maybeSingle() + if (profileError) { + log.warn('could not read profile email for notification recipient', { + companyId, + userId, + error: profileError.message, + }) + return null + } + return (profile as { email?: string | null } | null)?.email ?? null +} + +/** + * Resolve every active member's email for a company, as a map of + * user_id → email. Used by senders that route to a configured contact + * address but must verify it against the member allowlist (skattekonto + * drift alert). Returns an empty map (never throws) on any query failure, + * which callers treat as "no authorised recipient". + */ +export async function resolveMemberEmails( + supabase: SupabaseClient, + companyId: string +): Promise> { + const emails = new Map() + + // fetchAllRows with stable ordering: PostgREST silently caps unpaged + // reads at 1000 rows, which would drop members from the allowlist. + let members: Array<{ user_id: string | null }> + try { + members = await fetchAllRows(({ from, to }) => + supabase + .from('company_members') + .select('user_id') + .eq('company_id', companyId) + .order('user_id', { ascending: true }) + .range(from, to) + ) + } catch (err) { + log.warn('could not read company members for notification allowlist', { + companyId, + error: err instanceof Error ? err.message : String(err), + }) + return emails + } + + const userIds = members + .map((m) => m.user_id) + .filter((id): id is string => typeof id === 'string') + if (userIds.length === 0) return emails + + let profiles: Array<{ id: string; email: string | null }> + try { + profiles = await fetchAllRows(({ from, to }) => + supabase + .from('profiles') + .select('id, email') + .in('id', userIds) + .order('id', { ascending: true }) + .range(from, to) + ) + } catch (err) { + log.warn('could not read member emails for notification allowlist', { + companyId, + error: err instanceof Error ? err.message : String(err), + }) + return emails + } + + for (const row of profiles) { + if (row.email) emails.set(row.id, row.email) + } + return emails +} diff --git a/skills/accounted-api/references/periods.md b/skills/accounted-api/references/periods.md index 7cf07bf5..401ccdf0 100644 --- a/skills/accounted-api/references/periods.md +++ b/skills/accounted-api/references/periods.md @@ -514,7 +514,7 @@ Fetches the momsdeklaration for one period as Skatteverket has it on file: `subm **Do not use for:** Computing the declaration from the books (use the VAT report), or filing: submission is a separate BankID-signed flow. **Pitfalls:** -- This is a live Skatteverket read: it fails with SKATTEVERKET_NOT_CONNECTED (401) until someone in the company has connected with BankID under Installningar, and the response reflects SKV's state, not the books. +- This is a live Skatteverket read: it fails with SKATTEVERKET_NOT_CONNECTED (401) when the company has neither a member's BankID connection (made under Installningar) nor a verified ombud grant, and the response reflects SKV's state, not the books. Personal BankID sessions expire after ~1 hour by design, so an expired connection is normal: ask the user to reconnect; only a person can, so do not retry until they confirm. - submitted=null and decided=null with HTTP 200 means "nothing on file for the period": it is not an error. - A submitted declaration can lack a beslut for days: poll decided separately rather than assuming both appear together. - redovisningsperiod is SKV's YYYYMM format (the period's LAST month): quarterly period 1 is 03, not 01.