fix(assistant): the salary fact follows the ledger, never the column default (#2290)

* fix(assistant): the salary fact follows the ledger, never the column default The in-app assistant told a payroll-running aktiebolag in every answer that it "betalar inte löner" (support case 2026-09-04). company_settings. pays_salaries is NOT NULL DEFAULT false and only the Skatt settings form writes it, so for every company that never opened that form the flag reads false whatever the ledger says, and lib/agent/ask/snapshot.ts asserted that default as a fact. - Trigger salary_runs_booked_marks_employer (20260904191000): a booked salary run sets pays_salaries = true and fills a never-attested employer_registered, at the one place every writer (dashboard, MCP, v1, seeders) passes through. Backfill for the 12 companies on prod already booking payroll with the flag at its default (8 of them also lacked the employer flag, and with it their AGI deadline reminders). An explicit employer_registered = false stays the user's answer. - The assistant snapshot applies the composer's employee-facts doctrine: positive evidence (active employees, the flag, an attested employer registration) yields the fact, only an attested negative yields the negative, the default yields nothing. It also names Inställningar > Skatt / > Bokföring so the model can point at the page. - The composer's KÄNDA FAKTA no longer prints "Betalar ut lön: nej" from the same default. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01S62AGZwsMoBc8x8obBDVqE * fix(migration): NOT EXISTS instead of NOT IN for the reset-source exclusion A NULL source_company_id in the subquery would make NOT IN never true and silently skip the whole backfill. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01S62AGZwsMoBc8x8obBDVqE --------- Co-authored-by: Jakob Wennberg <311770904+jakobwennberg-oss@users.noreply.github.com> Co-authored-by: Claude Fable 5.1 <noreply@anthropic.com>
2026-09-05 09:49:21 +02:00
parent e684606b23
commit 0bd3c27fba
6 changed files with 278 additions and 16 deletions
@@ -1583,6 +1583,7 @@ One line per decision: `[YYYY-MM-DD] <decision>: <why>`. Appended by agents and
 [2026-09-04] Supplier-invoice list overflow (#2262) fixed per page, not in a shared list component: none exists (every list hand-writes the overflow-x-auto wrapper) and the three overflow reports had three different causes, so the column budget went into .claude/rules/design.md instead. Fakturadatum was dropped rather than Kvar or the action column (förfaller is the payer's date and the default order, the customer list has no invoice-date column either); the always-visible sort icon from #2091 was kept although it was the proximate regression.
 [2026-09-04] Parties: the model reads a counterpart only on demand (picker, review list), never when the queue builds: a five-hundred-row queue would cost five hundred calls nobody asked for and a rebuild would repeat them; the reading is a 'model' fact and a search query, never a hard key. The review list ticks rows with exactly one active SCB match but writes nothing until a person approves: an exact legal name plus one active hit is high precision, auto-attaching would still be the system choosing. Exact legal-form names ("Visma Spcs AB", not "Visma") group keys and attach to existing parties: registered company names are unique in Sweden, so this is a key in all but form; the "name never merges" rule keeps applying to fuzzy and form-less names.
 [2026-09-04] reset_fiscal_year's next_year_dependency no longer counts an opening-balance verifikat in the following year as reliance (migration 20260904163000 redefines fiscal_year_reset_snapshot; the block now fires only when the following year is locked, closed or has its own closing entry). Why: the old check (opening_balance_entry_id / opening_balances_set on the next period) fired for the dominant migration shape, import the first year with its own #IB and later backfill the year before it, so a backfilled year could never be reset (Aisen & Adison AB, 2026-09-03), while a next year WITHOUT an IB, whose balansrapport really rolls from this year's books, was allowed: the check was inverted relative to actual reliance. An IB in the next year is its own verifikat with its own underlag and survives the reset untouched; the one IB that IS derived from this year's books, the bokslut-generated one, is still refused via this year's closing_entry_id (year_end_state). The preview now returns next_period {name, has_opening_balances} and the dialog says the following year's IB stays as it is, instead of a blocker. Rejected alternative: stornoing the next year's IB inside the reset (it would destroy a correct migration boundary and re-create the #1022 dead end). Sibling fix in the same change: CreatePeriodDialog now derives the name from the dates the user types until the name is hand-edited, which is how a 2022-07-01..2023-12-31 year got saved as "Räkenskapsår 2027" (the seed suggestion is always the next forward year).
+[2026-09-04] company_settings.pays_salaries (and a never-attested employer_registered) now follow the ledger: trigger salary_runs_booked_marks_employer (20260904191000) sets them when a salary run is booked, with a backfill for the 12 companies already booking payroll with the flag at its default. Support case 2026-09-04: the assistant told a payroll-running aktiebolag in every answer that it "betalar inte löner" because lib/agent/ask/snapshot.ts asserted the column default (NOT NULL DEFAULT false, only written by the Skatt settings form) as a fact. The snapshot and the composer's KÄNDA FAKTA now apply the employee-facts doctrine (positive evidence or an attested negative, never the default) and the snapshot names Inställningar > Skatt / > Bokföring so the model can point at the page. Chose a trigger over calling a helper from the booking code because there are three booking writers (lib/salary/book-run.ts, the v1 route's own copy, seeders) and 20260717151000 already treated in-app payroll as employer evidence once as a one-off backfill; the trigger makes that rule continuous. An explicit employer_registered = false stays the user's answer.
 [2026-09-04] Supplier credit notes rest at 'credited' from creation (one row builder in lib/supplier-invoices/credit-note.ts for the dashboard route, the MCP executor and the v1 API) and a CHECK (supplier_invoices_credit_note_not_payable, 20260904190000) keeps every writer out of the payable states; the worklist attest count also excludes is_credit_note. Support case 2026-09-04: a credit note was inserted at 'registered' (the attest entry state) while the detail page rightly had no attest button for it, so Att göra showed "1 leverantörsfaktura att attestera" that nobody could clear; 14 such rows on prod plus one MCP-approved credit note, all backfilled by the migration (one immutable reset-source row skipped, hence NOT VALID). Chose the DB CHECK over fixing only the three insert literals because the invariant is "a credit note is never a payable", not "this literal says registered"; chose 'credited' over 'approved'+approved_at (would surface Markera betald and the Att betala tab) and over 'paid' (the customer side forbids paid credit notes by CHECK); the provider importers already used 'credited'. Also: the GET route now hydrates credited_original with a second scoped query because PostgREST cannot pick a direction for a self-referencing embed hint and returned the one-to-many side (an empty array), rendered as "Krediterar: Ankomst #" with no number.
 [2026-09-04] Underlag attach on a folder-picked Fortnox export (Loftux, 50 of 50 files refused with UNDERLAG_REF_MISMATCH): the multipart filename is reduced to its basename at the route boundary (lib/documents/upload-file-name.ts), rather than teaching the voucher-ref parser to strip directories or adding a client-supplied file_name field. Chrome writes webkitRelativePath as the multipart filename for folder selections, so the attach check saw "2026/06/Leverantörsfakturor/A166_x.pdf" while the preview had resolved File.name "A166_x.pdf"; the two endpoints received the same file under two names and the guard compared them. Stripping inside the parser would turn a typed manual ref "2024/01/31" into voucher 31 (the manual box shares the parser), and a second client-supplied name is no more trustworthy than the first, so the boundary is the only level that fixes the class.
 [2026-09-04] Connector-hop failures (timeout, error envelope, wire-contract mismatch) are transient in every sync path: the row keeps its status and the user message says no renewal is needed, same as AspspUnavailableError (#2202), and the cron now treats AspspUnavailableError the same way instead of parking it in 'error'. Why: on 2026-09-04 the Connect service answered a shape the client rejects and the cron flipped four canary companies to 'error' with SYNC_FAILED_MESSAGE, so users re-authorized consents that were fine. The Zod issues are logged (field paths) because a bare 'unexpected shape' left the failure undiagnosable. Rejected: a new 'degraded' connection status (one more state every filter and the probe would have to learn; the health probe already catches a dead session on the same run) and removing the canary companies from the env (hides the contract bug instead of exposing its field paths).
@@ -8,13 +8,28 @@ vi.mock('@/lib/deadlines/status-engine', () => ({

 import { buildAssistantSnapshot } from '../snapshot'

-function supabaseWith(settings: Record<string, unknown> | null): SupabaseClient {
-  const chain = {
-    select: () => chain,
-    eq: () => chain,
-    maybeSingle: async () => ({ data: settings, error: null }),
-  }
-  return { from: () => chain } as unknown as SupabaseClient
+/**
+ * company_settings answers maybeSingle(); employees answers the awaited head
+ * count (the builder chain is thenable, like the real PostgREST builder).
+ */
+function supabaseWith(
+  settings: Record<string, unknown> | null,
+  activeEmployees: number | null = null,
+): SupabaseClient {
+  return {
+    from: (table: string) => {
+      const chain = {
+        select: () => chain,
+        eq: () => chain,
+        maybeSingle: async () => ({ data: table === 'company_settings' ? settings : null, error: null }),
+        then: (onFulfilled: (v: unknown) => unknown) =>
+          Promise.resolve({ count: table === 'employees' ? activeEmployees : null, error: null }).then(
+            onFulfilled,
+          ),
+      }
+      return chain
+    },
+  } as unknown as SupabaseClient
 }

 beforeEach(() => {
@@ -45,7 +60,60 @@ describe('buildAssistantSnapshot', () => {
    )
    expect(snap).toContain('ej momsregistrerad')
    expect(snap).toContain('bokföringsmetod: kontantmetod')
-    expect(snap).toContain('betalar inte löner')
+  })
+
+  describe('the salary fact', () => {
+    // pays_salaries is NOT NULL DEFAULT false and only the Skatt settings form
+    // writes it, so false is what every company that never opened that form
+    // reads. Claiming "betalar inte löner" from it told a payroll-running
+    // aktiebolag in every answer that it had no salaries.
+    it('claims nothing when the flag is merely at its column default', async () => {
+      const snap = await buildAssistantSnapshot(
+        supabaseWith({ vat_registered: true, pays_salaries: false, employer_registered: null }, 0),
+        'c1',
+      )
+      expect(snap).not.toContain('betalar inte löner')
+      expect(snap).not.toContain('betalar löner')
+      expect(snap).toContain('Status: momsregistrerad.')
+    })
+
+    it('derives it from active employees when the flag was never set', async () => {
+      const snap = await buildAssistantSnapshot(
+        supabaseWith({ vat_registered: true, pays_salaries: false, employer_registered: null }, 1),
+        'c1',
+      )
+      expect(snap).toContain('betalar löner (1 anställd)')
+    })
+
+    it('pluralises the headcount', async () => {
+      const snap = await buildAssistantSnapshot(
+        supabaseWith({ vat_registered: true, pays_salaries: false }, 3),
+        'c1',
+      )
+      expect(snap).toContain('betalar löner (3 anställda)')
+    })
+
+    it('accepts an attested employer registration as the positive fact', async () => {
+      const snap = await buildAssistantSnapshot(
+        supabaseWith({ vat_registered: true, pays_salaries: false, employer_registered: true }, 0),
+        'c1',
+      )
+      expect(snap).toContain('betalar löner')
+    })
+
+    it('states the negative only from an attested employer_registered = false', async () => {
+      const snap = await buildAssistantSnapshot(
+        supabaseWith({ vat_registered: true, pays_salaries: false, employer_registered: false }, 0),
+        'c1',
+      )
+      expect(snap).toContain('betalar inte löner')
+    })
+  })
+
+  it('tells the model where the profile values are edited', async () => {
+    const snap = await buildAssistantSnapshot(supabaseWith({ vat_registered: true }), 'c1')
+    expect(snap).toContain('Inställningar > Skatt')
+    expect(snap).toContain('Inställningar > Bokföring')
  })

  it('lists deadlines that need attention (overdue first, capped)', async () => {
@@ -1,5 +1,10 @@
 import type { SupabaseClient } from '@supabase/supabase-js'
 import { getDeadlinesNeedingAttention } from '@/lib/deadlines/status-engine'
+import {
+  loadActiveEmployeeCount,
+  resolveEmployeeFacts,
+  type EmployeeVerdict,
+} from '@/lib/agent/composer/employee-facts'

 /**
 * A compact, always-on grounding block for the single-call assistant.
@@ -24,6 +29,30 @@ function accountingMethodLabel(method: string | null | undefined): string | null
  return method
 }

+/**
+ * The salary part of the status line, or null when nothing is known.
+ *
+ * company_settings.pays_salaries is NOT NULL DEFAULT false and its only
+ * writer is the Skatt settings form, so `false` is what every company that
+ * never opened that form reads, whatever its ledger says. Stating "betalar
+ * inte löner" from that default told a payroll-running aktiebolag in every
+ * answer that it had no salaries (support case 2026-09-04). The same doctrine
+ * as the composer (lib/agent/composer/employee-facts.ts): positive evidence
+ * (active employees, the flag, an attested employer registration) yields the
+ * fact, only an attested negative yields the negative, and the default
+ * yields nothing.
+ */
+function salaryPart(verdict: EmployeeVerdict): string | null {
+  switch (verdict.kind) {
+    case 'count':
+      return `betalar löner (${verdict.count} ${verdict.count === 1 ? 'anställd' : 'anställda'})`
+    case 'employer':
+      return verdict.isEmployer ? 'betalar löner' : 'betalar inte löner'
+    default:
+      return null
+  }
+}
+
 export async function buildAssistantSnapshot(
  supabase: SupabaseClient,
  companyId: string,
@@ -31,16 +60,20 @@ export async function buildAssistantSnapshot(
  const lines: string[] = []

  try {
-    const { data } = await supabase
-      .from('company_settings')
-      .select('vat_registered, moms_period, accounting_method, pays_salaries')
-      .eq('company_id', companyId)
-      .maybeSingle()
+    const [{ data }, activeEmployees] = await Promise.all([
+      supabase
+        .from('company_settings')
+        .select('vat_registered, moms_period, accounting_method, pays_salaries, employer_registered')
+        .eq('company_id', companyId)
+        .maybeSingle(),
+      loadActiveEmployeeCount(supabase, companyId),
+    ])
    const row = data as {
      vat_registered?: boolean | null
      moms_period?: string | null
      accounting_method?: string | null
      pays_salaries?: boolean | null
+      employer_registered?: boolean | null
    } | null
    if (row) {
      const parts: string[] = []
@@ -51,8 +84,21 @@ export async function buildAssistantSnapshot(
      )
      const method = accountingMethodLabel(row.accounting_method)
      if (method) parts.push(`bokföringsmetod: ${method}`)
-      parts.push(row.pays_salaries ? 'betalar löner' : 'betalar inte löner')
+      const salary = salaryPart(
+        resolveEmployeeFacts({
+          activeEmployees,
+          ticEmployeeRange: null,
+          employerRegistered: row.employer_registered ?? null,
+          paysSalaries: row.pays_salaries ?? null,
+        }),
+      )
+      if (salary) parts.push(salary)
      lines.push(`Status: ${parts.join(', ')}.`)
+      // So the model can point the user at the right page instead of
+      // "inställningarna" in general when a value here is wrong.
+      lines.push(
+        'Grunduppgifterna ovan ändras under Inställningar > Skatt (moms, löner) och Inställningar > Bokföring (bokföringsmetod).',
+      )
    }
  } catch {
    // best-effort: skip the status line
@@ -480,8 +480,12 @@ export function buildKnownFacts(inputs: ComposerInputs): string[] {
    if (s.vat_registered != null) {
      out.push(`Momsregistrerad: ${s.vat_registered ? 'ja' : 'nej'}`)
    }
-    if (s.pays_salaries != null) {
-      out.push(`Betalar ut lön: ${s.pays_salaries ? 'ja' : 'nej'}`)
+    // Only a true is a fact: the column is NOT NULL DEFAULT false, so false is
+    // what every company that never opened the Skatt settings reads
+    // (employee-facts.ts). An attested negative surfaces through
+    // employeeKnownFact below; the default must not be read as "nej".
+    if (s.pays_salaries === true) {
+      out.push('Betalar ut lön: ja')
    }
    if (s.employer_registered != null) {
      out.push(`Arbetsgivarregistrerad: ${s.employer_registered ? 'ja' : 'nej'}`)
@@ -0,0 +1,62 @@
+-- A booked salary run is evidence that the company pays salaries (support
+-- case 2026-09-04: the assistant told a payroll-running aktiebolag in every
+-- answer that it "betalar inte löner").
+--
+-- company_settings.pays_salaries is NOT NULL DEFAULT false and its only
+-- writer is the Skatt settings form, so for every company that never opened
+-- that form the flag reads false regardless of what the ledger says, while
+-- the app itself books the salary verifikat. The flag feeds the assistant's
+-- profile block, the composer, the MCP company resource, the Personal menu
+-- for enskild firma and (as the fallback for employer_registered) the AGI
+-- deadline reminders, so the stale default is visible in five places.
+--
+-- 20260717151000 already treated in-app payroll as employer evidence once,
+-- as a one-off backfill of employer_registered from salary_runs. This makes
+-- the same rule continuous and extends it to pays_salaries, at the one place
+-- every writer (dashboard, MCP executor, v1 API, seeders) passes through:
+-- the run flipping to 'booked'.
+--
+-- employer_registered is only filled when NULL (never attested): an explicit
+-- false is the user's own answer and stays theirs. SECURITY DEFINER because
+-- company_settings_update requires company admin, while any member with
+-- write access can book payroll; the function touches only the booking
+-- company's own row, and Postgres refuses to call a trigger function
+-- directly, so it is not reachable through the API.
+-- pg-test: covered-by tests/pg/salary-run-booked-marks-employer.pg.test.ts
+
+CREATE OR REPLACE FUNCTION public.mark_company_pays_salaries_on_booked_run()
+RETURNS trigger
+LANGUAGE plpgsql
+SECURITY DEFINER
+SET search_path = public
+AS $$
+BEGIN
+  UPDATE public.company_settings
+  SET pays_salaries = true,
+      employer_registered = COALESCE(employer_registered, true)
+  WHERE company_id = NEW.company_id
+    AND (pays_salaries = false OR employer_registered IS NULL);
+  RETURN NEW;
+END;
+$$;
+
+DROP TRIGGER IF EXISTS salary_runs_booked_marks_employer ON public.salary_runs;
+CREATE TRIGGER salary_runs_booked_marks_employer
+  AFTER INSERT OR UPDATE OF status ON public.salary_runs
+  FOR EACH ROW
+  WHEN (NEW.status = 'booked')
+  EXECUTE FUNCTION public.mark_company_pays_salaries_on_booked_run();
+
+-- Backfill: companies that already booked payroll in the app. Archived
+-- migration-reset sources are skipped: block_migration_reset_source_mutation()
+-- makes their rows immutable and would abort the migration.
+UPDATE public.company_settings cs
+SET pays_salaries = true,
+    employer_registered = COALESCE(cs.employer_registered, true)
+FROM (SELECT DISTINCT company_id FROM public.salary_runs WHERE status = 'booked') sr
+WHERE sr.company_id = cs.company_id
+  AND (cs.pays_salaries = false OR cs.employer_registered IS NULL)
+  AND NOT EXISTS (
+    SELECT 1 FROM public.company_migration_resets r
+    WHERE r.source_company_id = cs.company_id
+  );
@@ -0,0 +1,81 @@
+/**
+ * pg-real tests for 20260904191000_salary_run_booked_marks_employer.sql.
+ *
+ * Booking a salary run is evidence that the company pays salaries: the
+ * trigger flips company_settings.pays_salaries and fills a never-attested
+ * employer_registered, but never overrides an explicit employer answer.
+ */
+import { randomUUID } from 'node:crypto'
+import { describe, expect, it } from 'vitest'
+import { getPool } from '@/tests/pg/setup'
+import { insertAuthUser, insertCompany } from '@/tests/pg/fixtures'
+
+async function seed(settings: { paysSalaries?: boolean; employerRegistered?: boolean | null } = {}) {
+  const userId = await insertAuthUser()
+  const companyId = await insertCompany({ createdBy: userId })
+  await getPool().query(
+    `INSERT INTO public.company_settings (user_id, company_id, pays_salaries, employer_registered)
+     VALUES ($1, $2, $3, $4)`,
+    [userId, companyId, settings.paysSalaries ?? false, settings.employerRegistered ?? null],
+  )
+  return { userId, companyId }
+}
+
+async function insertRun(companyId: string, userId: string, status: string, month = 6): Promise<string> {
+  const runId = randomUUID()
+  await getPool().query(
+    `INSERT INTO public.salary_runs (id, company_id, user_id, period_year, period_month, payment_date, status)
+     VALUES ($1, $2, $3, 2026, $4, '2026-06-25', $5)`,
+    [runId, companyId, userId, month, status],
+  )
+  return runId
+}
+
+async function readFlags(companyId: string) {
+  const { rows } = await getPool().query<{ pays_salaries: boolean; employer_registered: boolean | null }>(
+    `SELECT pays_salaries, employer_registered FROM public.company_settings WHERE company_id = $1`,
+    [companyId],
+  )
+  return rows[0]!
+}
+
+describe('salary_runs_booked_marks_employer trigger', () => {
+  it('leaves the flags at their defaults while the run is a draft', async () => {
+    const { companyId, userId } = await seed()
+    await insertRun(companyId, userId, 'draft')
+    expect(await readFlags(companyId)).toEqual({ pays_salaries: false, employer_registered: null })
+  })
+
+  it('marks the company as paying salaries when a run is booked', async () => {
+    const { companyId, userId } = await seed()
+    const runId = await insertRun(companyId, userId, 'draft')
+    await getPool().query(`UPDATE public.salary_runs SET status = 'booked' WHERE id = $1`, [runId])
+    expect(await readFlags(companyId)).toEqual({ pays_salaries: true, employer_registered: true })
+  })
+
+  it('also fires for a run inserted directly as booked', async () => {
+    const { companyId, userId } = await seed()
+    await insertRun(companyId, userId, 'booked')
+    expect(await readFlags(companyId)).toEqual({ pays_salaries: true, employer_registered: true })
+  })
+
+  it('never overrides an explicitly attested employer_registered = false', async () => {
+    const { companyId, userId } = await seed({ employerRegistered: false })
+    await insertRun(companyId, userId, 'booked')
+    expect(await readFlags(companyId)).toEqual({ pays_salaries: true, employer_registered: false })
+  })
+
+  it('is a no-op for a company whose flags are already set', async () => {
+    const { companyId, userId } = await seed({ paysSalaries: true, employerRegistered: true })
+    const before = await getPool().query<{ updated_at: string }>(
+      `SELECT updated_at FROM public.company_settings WHERE company_id = $1`,
+      [companyId],
+    )
+    await insertRun(companyId, userId, 'booked')
+    const after = await getPool().query<{ updated_at: string }>(
+      `SELECT updated_at FROM public.company_settings WHERE company_id = $1`,
+      [companyId],
+    )
+    expect(after.rows[0]!.updated_at).toEqual(before.rows[0]!.updated_at)
+  })
+})