fix(payroll): require jamkning valid_to on every write path (#2058) (#2240)

* fix(payroll): require jamkning valid_to on every write path (#2058) A jamkningsbeslut saved through the v1 API or MCP with a percentage and a start date but no end date was stored and returned 200, yet the engine (isJamkningValid) never applies a beslut without both dates: the payslip and the AGI carried the table tax while the caller believed the beslut was live. One shared validator (lib/salary/jamkning-rules.ts) now requires both dates whenever a percentage is set and checks their ordering. Every write path runs it: CreateEmployeeSchema and UpdateEmployeeSchema, the web POST and PATCH routes, the v1 PATCH route (its private copy is deleted), the MCP create and update executors in employee-commands, and the MCP update tool preflights the merged row at staging time so the agent sees the error before approval. The update paths keep the existing touched gate, so legacy rows stored without valid_to stay editable in unrelated ways. The MCP tool descriptions state that both dates are required for the beslut to apply. scripts/list-incomplete-jamkning.ts lists the existing rows (percentage set, valid_to null) per company, read-only; setting an end date or clearing the beslut is decided per company since either changes the next payslip. Declined: defaulting valid_to to 31 December of the from-year. It matches most beslut but silently changes withholding on rows that today do nothing. Closes #2058 Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0161fHpCX3rnWtidwwdGfCdB * fix(payroll): keep the jamkning PR inside the type and tools/list budgets CI on the first push failed on two ratchets this PR itself tripped: - Typecheck ratchet: the three staging tests added here reused the untyped 'agent_chat' actor literal the file already carried, which raised that file's error count above its baseline. They now pass { type: 'user' }. - tools/list payload budget: the first jamkning field descriptions on gnubok_create_employee and gnubok_update_employee pushed the projected catalog to 60 113 tokens against the 60 000 ceiling. The percentage fields keep a one-line "needs both dates or never applied" note; the date fields drop theirs. Also acts on the compliance swarm's GDPR Art.32 note: the read-only lister no longer selects employee names at all (the employee id is what the per-company decision needs), so the script touches no PII. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0161fHpCX3rnWtidwwdGfCdB * docs(mcp): say the jamkning percentage is rejected without both dates CodeRabbit on #2240: "never applied" described the pre-fix engine behaviour; the contract now is that a create or update with a percentage and a missing date is rejected before staging. Same length, so the tools/list payload budget is unchanged. The concurrency finding is tracked in #2256 instead of this PR. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> * fix(mcp): keep tools/list under budget after proforma landed on main After merging main (#2254 proforma fields) the projected tools/list measured 60 010 tokens against the 60 000 ceiling with this PR's two jamkning field notes. Per the budget test's own rule, demote a read tool instead of bumping the ceiling: gnubok_list_arsredovisning_versions goes search-only. Versions exist only once a report is rendered for signing or filing, which is the same switched-off iXBRL path as its sibling gnubok_get_arsredovisning_filing_status, already search-only since 2026-09-02. Still reachable via gnubok_call_tool. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> --------- Co-authored-by: Claude Fable 5.1 <noreply@anthropic.com>
2026-09-03 20:22:41 +02:00
parent 22b98e0a3b
commit b07efcafd4
17 changed files with 706 additions and 117 deletions
@@ -220,6 +220,7 @@ describe('personnummer contract on /api/salary/employees/[id]', () => {
 */
 describe('jämkning on PATCH /api/salary/employees/[id]', () => {
  const JAMKNING_START_REQUIRED = 'Jämkningens startdatum måste anges när jämkningsprocent sätts'
+  const JAMKNING_END_REQUIRED = 'Jämkningens slutdatum måste anges när jämkningsprocent sätts'
  const JAMKNING_ORDER = 'Jämkningens slutdatum måste vara efter startdatumet'

  function useRow(existing: Record<string, unknown>) {
@@ -259,6 +260,20 @@ describe('jämkning on PATCH /api/salary/employees/[id]', () => {
    expect(captured.updates).toBeNull()
  })

+  it('400 when a percentage is set with a start date but no end date (#2058)', async () => {
+    const captured = useRow({ ...EXISTING_ROW, jamkning_percentage: null, jamkning_valid_from: null, jamkning_valid_to: null })
+
+    const response = await PATCH(
+      patchRequest({ jamkning_percentage: 20, jamkning_valid_from: '2026-01-01', jamkning_valid_to: null }),
+      params,
+    )
+    const { status, body } = await parseJsonResponse<{ error: string }>(response)
+
+    expect(status).toBe(400)
+    expect(body.error).toContain(JAMKNING_END_REQUIRED)
+    expect(captured.updates).toBeNull()
+  })
+
  it('400 when the end date precedes the start date within the body', async () => {
    const captured = useRow({ ...EXISTING_ROW })

@@ -7,6 +7,7 @@ import { getCompanyEntityType } from '@/lib/company/context'
 import { encryptPersonnummer, extractLast4, maskEmployeeForResponse, validatePersonnummer } from '@/lib/salary/personnummer'
 import { isEmploymentTypeAllowedForEntity, EF_OWNER_EMPLOYMENT_ERROR } from '@/lib/salary/employment-rules'
 import { validateEmployeeBankAccount } from '@/lib/salary/payment/bank-account'
+import { touchesJamkning, validateJamkning } from '@/lib/salary/jamkning-rules'
 import { getErrorMessage as getUserErrorMessage } from '@/lib/errors/get-error-message'

 ensureInitialized()
@@ -72,28 +73,15 @@ export const PATCH = withRouteContext<{ params: Promise<{ id: string }> }>(
    if (merged.f_skatt_status === 'a_skatt' && !merged.is_sidoinkomst && !merged.tax_table_number) {
      mergedErrors.push('Skattetabell krävs för A-skatt anställda')
    }
-    // Merged-state jämkning check (same rule as the v1 route and
-    // employee-commands): a non-null percentage needs a start date, and the
-    // dates must be ordered, but the schema can only see the body. Only run
-    // when the PATCH touches a jamkning field: a legacy row with inconsistent
-    // jamkning_* state must not block unrelated updates (fixing it requires
-    // touching those very fields). `body` is the parsed patch: absent keys are
-    // absent, explicit nulls survive.
-    const jamkningTouched =
-      'jamkning_percentage' in body ||
-      'jamkning_valid_from' in body ||
-      'jamkning_valid_to' in body
-    if (jamkningTouched) {
-      if (merged.jamkning_percentage != null && !merged.jamkning_valid_from) {
-        mergedErrors.push('Jämkningens startdatum måste anges när jämkningsprocent sätts')
-      }
-      if (
-        merged.jamkning_valid_from &&
-        merged.jamkning_valid_to &&
-        merged.jamkning_valid_to < merged.jamkning_valid_from
-      ) {
-        mergedErrors.push('Jämkningens slutdatum måste vara efter startdatumet')
-      }
+    // Merged-state jämkning check through the shared validator (same rule as
+    // the v1 route and employee-commands): a non-null percentage needs both
+    // dates, and the dates must be ordered, but the schema can only see the
+    // body. Only run when the PATCH touches a jamkning field: a legacy row
+    // with inconsistent jamkning_* state must not block unrelated updates
+    // (fixing it requires touching those very fields). `body` is the parsed
+    // patch: absent keys are absent, explicit nulls survive. #2058
+    if (touchesJamkning(body)) {
+      for (const issue of validateJamkning(merged)) mergedErrors.push(issue.message)
    }
    if (mergedErrors.length > 0) {
      return NextResponse.json({ error: mergedErrors.join('. ') }, { status: 400 })
@@ -219,6 +219,19 @@ describe('POST /api/salary/employees', () => {
    })
  })

+  it('returns 400 on a jämkning percentage without an end date, without inserting (#2058)', async () => {
+    const { supabase, insert } = supabaseWithInsert({ id: 'emp-new', personnummer: encryptPersonnummer(NEW_PNR) })
+    authed(supabase)
+
+    const res = await POST(
+      postRequest({ ...CREATE_BASE, jamkning_percentage: 12.5, jamkning_valid_from: '2026-01-01' }),
+      params,
+    )
+
+    expect(res.status).toBe(400)
+    expect(insert).not.toHaveBeenCalled()
+  })
+
  it('inserts null jämkning fields when the body omits them', async () => {
    const { supabase, insert } = supabaseWithInsert({ id: 'emp-new', personnummer: encryptPersonnummer(NEW_PNR) })
    authed(supabase)
@@ -27,6 +27,7 @@ import { readV1JsonBody } from '@/lib/api/v1/body'
 import { UpdateEmployeeSchema } from '@/lib/api/schemas'
 import { maskPersonnummer } from '@/lib/api/v1/mask-personnummer'
 import { decryptPersonnummer } from '@/lib/salary/personnummer'
+import { JAMKNING_ORDER, touchesJamkning, validateJamkning, type JamkningFields } from '@/lib/salary/jamkning-rules'

 const EmploymentType = z.enum(['employee', 'company_owner', 'board_member'])
 const SalaryType = z.enum(['monthly', 'hourly'])
@@ -341,46 +342,25 @@ export const PATCH = withApiV1<{ params: Promise<{ companyId: string; id: string
      })
    }

-    // Merged-state jämkning check (same pattern as växa-stöd): a non-null
-    // percentage needs a start date, but the schema can only see the body.
-    // Also validate merged date ordering when only one of the dates is
-    // updated. Setting jamkning_percentage to null clears the beslut and
-    // skips these checks. Only run when the PATCH touches a jamkning field:
-    // a legacy row with inconsistent jamkning_* state must not block
-    // unrelated updates (fixing it requires touching those very fields).
-    const jamkningTouched =
-      'jamkning_percentage' in updates ||
-      'jamkning_valid_from' in updates ||
-      'jamkning_valid_to' in updates
-    if (jamkningTouched) {
-      const mergedJamkningPct =
-        'jamkning_percentage' in updates
-          ? (updates.jamkning_percentage as number | null)
-          : ((existing as Record<string, unknown>).jamkning_percentage as number | null)
-      const mergedJamkningFrom =
-        'jamkning_valid_from' in updates
-          ? (updates.jamkning_valid_from as string | null)
-          : ((existing as Record<string, unknown>).jamkning_valid_from as string | null)
-      const mergedJamkningTo =
-        'jamkning_valid_to' in updates
-          ? (updates.jamkning_valid_to as string | null)
-          : ((existing as Record<string, unknown>).jamkning_valid_to as string | null)
-      if (mergedJamkningPct !== null && mergedJamkningPct !== undefined && !mergedJamkningFrom) {
+    // Merged-state jämkning check through the shared validator (same pattern
+    // as växa-stöd): a non-null percentage needs both dates, but the schema
+    // can only see the body. Setting jamkning_percentage to null clears the
+    // beslut and skips these checks. Only run when the PATCH touches a
+    // jamkning field: a legacy row with inconsistent jamkning_* state must
+    // not block unrelated updates (fixing it requires touching those very
+    // fields). #2058
+    if (touchesJamkning(updates)) {
+      const mergedJamkning = { ...(existing as Record<string, unknown>), ...updates } as JamkningFields
+      const [issue] = validateJamkning(mergedJamkning)
+      if (issue) {
        return v1ErrorResponseFromCode('VALIDATION_ERROR', ctx.log, {
          requestId: ctx.requestId,
          details: {
-            field: 'jamkning_valid_from',
+            field: issue.field,
            message:
-              'Jämkningens startdatum måste anges när jämkningsprocent sätts. Skicka även `jamkning_valid_from` i samma PATCH.',
-          },
-        })
-      }
-      if (mergedJamkningFrom && mergedJamkningTo && mergedJamkningTo < mergedJamkningFrom) {
-        return v1ErrorResponseFromCode('VALIDATION_ERROR', ctx.log, {
-          requestId: ctx.requestId,
-          details: {
-            field: 'jamkning_valid_to',
-            message: 'Jämkningens slutdatum måste vara efter startdatumet.',
+              issue.message === JAMKNING_ORDER
+                ? `${issue.message}.`
+                : `${issue.message}. Skicka även \`${issue.field}\` i samma PATCH.`,
          },
        })
      }
@@ -353,6 +353,60 @@ describe('POST /api/v1/companies/:companyId/employees', () => {
    expect(JSON.stringify(body)).not.toContain(SAMPLE_PERSONNUMMER)
  })

+  it('rejects a jämkning percentage without an end date (#2058)', async () => {
+    // The engine applies a beslut only when BOTH dates are set; the API used
+    // to accept this shape and store an inert beslut.
+    mockServiceClient.mockReturnValue(
+      makeFlexibleSupabase({
+        company_members: { data: { company_id: COMPANY_ID, role: 'owner' }, error: null },
+        employees: { data: SAMPLE_EMPLOYEE, error: null },
+        idempotency_keys: { data: null, error: null },
+      }),
+    )
+
+    const res = await createEmployee(
+      makeRequest(`https://x.test/api/v1/companies/${COMPANY_ID}/employees`, {
+        method: 'POST',
+        body: JSON.stringify({
+          ...validBody,
+          jamkning_percentage: 15,
+          jamkning_valid_from: '2026-01-01',
+        }),
+      }),
+      companyParams(COMPANY_ID),
+    )
+
+    expect(res.status).toBe(400)
+    const body = await res.json()
+    expect(body.error.code).toBe('VALIDATION_ERROR')
+    expect(JSON.stringify(body.error)).toContain('jamkning_valid_to')
+  })
+
+  it('accepts a complete jämkning beslut on create', async () => {
+    mockServiceClient.mockReturnValue(
+      makeFlexibleSupabase({
+        company_members: { data: { company_id: COMPANY_ID, role: 'owner' }, error: null },
+        employees: { data: SAMPLE_EMPLOYEE, error: null },
+        idempotency_keys: { data: null, error: null },
+      }),
+    )
+
+    const res = await createEmployee(
+      makeRequest(`https://x.test/api/v1/companies/${COMPANY_ID}/employees`, {
+        method: 'POST',
+        body: JSON.stringify({
+          ...validBody,
+          jamkning_percentage: 15,
+          jamkning_valid_from: '2026-01-01',
+          jamkning_valid_to: '2026-12-31',
+        }),
+      }),
+      companyParams(COMPANY_ID),
+    )
+
+    expect(res.status).toBe(201)
+  })
+
  it('returns 409 EMPLOYEE_DUPLICATE_PERSONNUMMER on 23505 (and does not echo the personnummer)', async () => {
    mockServiceClient.mockReturnValue(
      makeFlexibleSupabase({
@@ -799,6 +853,87 @@ describe('PATCH /api/v1/companies/:companyId/employees/:id', () => {
    expect(body.error.details.field).toBe('jamkning_valid_from')
  })

+  it('rejects a jämkning percentage without an end date (merged state, #2058)', async () => {
+    // Percentage + start date only: the engine would never apply it, so the
+    // route must refuse rather than store an inert beslut.
+    mockServiceClient.mockReturnValue(
+      makeFlexibleSupabase({
+        company_members: { data: { company_id: COMPANY_ID, role: 'owner' }, error: null },
+        employees: { data: SAMPLE_EMPLOYEE, error: null },
+        idempotency_keys: { data: null, error: null },
+      }),
+    )
+
+    const res = await updateEmployee(
+      makeRequest(`https://x.test/api/v1/companies/${COMPANY_ID}/employees/${EMPLOYEE_ID}`, {
+        method: 'PATCH',
+        body: JSON.stringify({ jamkning_percentage: 15, jamkning_valid_from: '2026-01-01' }),
+      }),
+      detailParams(COMPANY_ID, EMPLOYEE_ID),
+    )
+
+    expect(res.status).toBe(400)
+    const body = await res.json()
+    expect(body.error.code).toBe('VALIDATION_ERROR')
+    expect(body.error.details.field).toBe('jamkning_valid_to')
+    expect(body.error.details.message).toContain('slutdatum')
+  })
+
+  it('rejects clearing only the end date of a stored beslut', async () => {
+    const withJamkning = {
+      ...SAMPLE_EMPLOYEE,
+      jamkning_percentage: 15,
+      jamkning_valid_from: '2026-01-01',
+      jamkning_valid_to: '2026-12-31',
+    }
+    mockServiceClient.mockReturnValue(
+      makeFlexibleSupabase({
+        company_members: { data: { company_id: COMPANY_ID, role: 'owner' }, error: null },
+        employees: { data: withJamkning, error: null },
+        idempotency_keys: { data: null, error: null },
+      }),
+    )
+
+    const res = await updateEmployee(
+      makeRequest(`https://x.test/api/v1/companies/${COMPANY_ID}/employees/${EMPLOYEE_ID}`, {
+        method: 'PATCH',
+        body: JSON.stringify({ jamkning_valid_to: null }),
+      }),
+      detailParams(COMPANY_ID, EMPLOYEE_ID),
+    )
+
+    expect(res.status).toBe(400)
+    const body = await res.json()
+    expect(body.error.details.field).toBe('jamkning_valid_to')
+  })
+
+  it('leaves a legacy row without valid_to editable in unrelated ways (touched gate)', async () => {
+    const legacy = {
+      ...SAMPLE_EMPLOYEE,
+      jamkning_percentage: 15,
+      jamkning_valid_from: '2026-01-01',
+      jamkning_valid_to: null,
+    }
+    const updated = { ...legacy, monthly_salary: 38000 }
+    mockServiceClient.mockReturnValue(
+      makeFlexibleSupabase({
+        company_members: { data: { company_id: COMPANY_ID, role: 'owner' }, error: null },
+        employees: [{ data: legacy, error: null }, { data: updated, error: null }],
+        idempotency_keys: { data: null, error: null },
+      }),
+    )
+
+    const res = await updateEmployee(
+      makeRequest(`https://x.test/api/v1/companies/${COMPANY_ID}/employees/${EMPLOYEE_ID}`, {
+        method: 'PATCH',
+        body: JSON.stringify({ monthly_salary: 38000 }),
+      }),
+      detailParams(COMPANY_ID, EMPLOYEE_ID),
+    )
+
+    expect(res.status).toBe(200)
+  })
+
  it('rejects jamkning_valid_to before jamkning_valid_from', async () => {
    mockServiceClient.mockReturnValue(
      makeFlexibleSupabase({