fix(salary): surface employee-save failures and a typed 503 for the missing encryption key (#1996) (#2009)

* fix(salary): surface employee-save failures in the dialog and type the missing encryption key (#1996)

Pressing Spara in "Ny anställd" could fail without any feedback: a thrown
fetch or a non-JSON 5xx body escaped handleSubmit before setSaving(false)
ran, leaving the button stuck on "Sparar..." and the dialog silent. Even
when the toast did fire, the Radix modal aria-hides the root-layout
Toaster, so assistive tech (and the E2E driver that found this) heard
nothing, and the requestId support needs was never shown anywhere.

- NewEmployeeDialog: fetch + parse run in a never-throwing helper, saving
  is released in finally, the body is parsed with json().catch(() => null)
  so an HTML/plain-text error page still maps through the HTTP-status map,
  and the failure is rendered inline (role="alert" in the footer) with
  "Ärende-id: <requestId>" next to the single destructive toast.
- personnummer.ts: the production "key missing" throw now carries the
  registry code PERSONNUMMER_ENCRYPTION_NOT_CONFIGURED, and the SALARY
  registry gains a 503 entry naming PERSONNUMMER_ENCRYPTION_KEY with a
  "contact support" message and a remediation hint. withRouteContext
  emits the typed envelope automatically instead of INTERNAL_ERROR 500,
  which read as transient and invited retries that can never succeed.
- Tests for the route (401, 400, 503 with requestId and no insert), the
  key guard, the registry entry, errorResponse dispatch on a coded Error,
  and getErrorMessage locale handling of the new envelope.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015nAd8XJ2RPCmG2eKoLBdna

* fix(salary): address review findings (#1996)

- NewEmployeeDialog: fall back to the X-Request-Id response header when the
  body carries no error.requestId. The route hand-builds its 409 (duplicate
  personnummer) and generic insert-failure 500 bodies as flat strings, so the
  inline "Ärende-id" line was hidden for exactly the DB-failure class the
  issue names; withRouteContext sets the header on every response.
- Route tests pin that the 409 and 500 insert-error arms carry X-Request-Id.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015nAd8XJ2RPCmG2eKoLBdna

---------

Co-authored-by: Jakob Wennberg <311770904+jakobwennberg-oss@users.noreply.github.com>
Co-authored-by: Claude Fable 5 <noreply@anthropic.com>

lib/errors/__tests__/get-error-message.test.ts

@@ -411,6 +411,28 @@ describe('getErrorMessage: API response body vs new Error(body.error)', () => {
     )
   })
 
+  // #1996: the envelope a missing PERSONNUMMER_ENCRYPTION_KEY now produces.
+  it('a typed configuration-gap envelope yields the registry text per locale', () => {
+    const configGap = {
+      error: {
+        code: 'PERSONNUMMER_ENCRYPTION_NOT_CONFIGURED',
+        message:
+          'Lönemodulen är inte konfigurerad: krypteringsnyckeln för personnummer (PERSONNUMMER_ENCRYPTION_KEY) saknas i driftmiljön. Kontakta supporten.',
+        message_en:
+          'Payroll is not configured: the personal-number encryption key (PERSONNUMMER_ENCRYPTION_KEY) is missing from the deployment environment. Contact support.',
+        requestId: 'req_00000000-0000-4000-8000-000000000001',
+      },
+    }
+    const sv = getErrorMessage(configGap, { context: 'salary', statusCode: 503, locale: 'sv' })
+    expect(sv).toBe(configGap.error.message)
+    const en = getErrorMessage(configGap, { context: 'salary', statusCode: 503, locale: 'en' })
+    expect(en).toBe(configGap.error.message_en)
+    // Neither locale falls back to the generic 503 "temporarily unavailable"
+    // text: this failure is permanent until an operator sets the variable.
+    expect(sv).not.toMatch(/tillfälligt/i)
+    expect(en).not.toMatch(/temporarily/i)
+  })
+
   it('the same call handles an unparseable body via statusCode', () => {
     // `await response.json().catch(() => null)` on an HTML 403 page.
     expect(getErrorMessage(null, { statusCode: 403 })).toBe(

lib/errors/__tests__/structured-errors.test.ts

@@ -58,6 +58,18 @@ describe('structured-errors registry', () => {
     }
   })
 
+  it('registers PERSONNUMMER_ENCRYPTION_NOT_CONFIGURED as a 503 configuration gap (#1996)', () => {
+    const entry = getErrorEntry('PERSONNUMMER_ENCRYPTION_NOT_CONFIGURED')
+    expect(entry).toBeDefined()
+    expect(entry?.httpStatus).toBe(503)
+    expect(entry?.message_sv).toContain('PERSONNUMMER_ENCRYPTION_KEY')
+    expect(entry?.message_sv).toMatch(/Kontakta supporten/)
+    expect(entry?.message_en).toContain('PERSONNUMMER_ENCRYPTION_KEY')
+    expect(entry?.remediation?.description).toContain('PERSONNUMMER_ENCRYPTION_KEY')
+    // Retrying without the variable fails identically: never mark it transient.
+    expect(entry?.retryable).toBeFalsy()
+  })
+
   it('listErrorCodes returns at least the bookkeeping + generic + provider codes', () => {
     const codes = listErrorCodes()
     expect(codes.length).toBeGreaterThan(20)
@@ -73,6 +85,23 @@ describe('structured-errors registry', () => {
 })
 
 describe('errorResponse', () => {
+  it('maps a plain Error carrying a registry code to that code, status and requestId', async () => {
+    // The shape lib/salary/personnummer.ts throws when the key is unset in
+    // production: an Error with a `code` own-property, no class hierarchy.
+    const err = Object.assign(new Error('PERSONNUMMER_ENCRYPTION_KEY is required in production'), {
+      code: 'PERSONNUMMER_ENCRYPTION_NOT_CONFIGURED',
+    })
+    const res = errorResponse(err, noopLogger, { requestId: 'req_1996' })
+    expect(res.status).toBe(503)
+    expect(res.headers.get('X-Request-Id')).toBe('req_1996')
+    const body = await readEnvelope(res)
+    expect(body.error.code).toBe('PERSONNUMMER_ENCRYPTION_NOT_CONFIGURED')
+    expect(body.error.message).toMatch(/PERSONNUMMER_ENCRYPTION_KEY/)
+    expect(body.error.requestId).toBe('req_1996')
+    // The raw English Error.message must not replace the registry message.
+    expect(body.error.message).not.toBe(err.message)
+  })
+
   it('maps BookkeepingError to its code + structured details + Swedish message', async () => {
     const err = new JournalEntryNotBalancedError(100, 90)
     const res = errorResponse(err, noopLogger, { requestId: 'req_1' })

lib/errors/structured-errors.ts

@@ -2841,6 +2841,24 @@ const SALARY: Record<string, StructuredErrorEntry> = {
     message_sv: 'En anställd med samma personnummer finns redan.',
     message_en: 'An employee with that personnummer already exists.',
   },
+  // A production deployment without PERSONNUMMER_ENCRYPTION_KEY: every
+  // employee create (and every decrypt-on-read) throws before touching the
+  // database. Deliberately not INTERNAL_ERROR: it is a configuration gap, not
+  // transient, and retrying never helps, so the user should hear "contact
+  // support" rather than "try again later". 503 like
+  // INVOICE_SEND_EMAIL_NOT_CONFIGURED: the service is unavailable until an
+  // operator sets the variable. #1996
+  PERSONNUMMER_ENCRYPTION_NOT_CONFIGURED: {
+    httpStatus: 503,
+    message_sv:
+      'Lönemodulen är inte konfigurerad: krypteringsnyckeln för personnummer (PERSONNUMMER_ENCRYPTION_KEY) saknas i driftmiljön. Kontakta supporten.',
+    message_en:
+      'Payroll is not configured: the personal-number encryption key (PERSONNUMMER_ENCRYPTION_KEY) is missing from the deployment environment. Contact support.',
+    remediation: {
+      description:
+        'Set PERSONNUMMER_ENCRYPTION_KEY in the deployment environment and redeploy. Retrying the request without it will fail identically.',
+    },
+  },
   SALARY_RUN_DUPLICATE_PERIOD: {
     httpStatus: 409,
     message_sv: 'En lönekörning för perioden finns redan.',

lib/salary/__tests__/personnummer.test.ts

@@ -1,7 +1,8 @@
 import { readFileSync } from 'node:fs'
 import { fileURLToPath } from 'node:url'
-import { describe, it, expect } from 'vitest'
+import { afterEach, describe, it, expect, vi } from 'vitest'
 import {
+  PERSONNUMMER_ENCRYPTION_NOT_CONFIGURED,
   validatePersonnummer,
   extractLast4,
   extractBirthDate,
@@ -307,6 +308,47 @@ describe('encryption roundtrip', () => {
   })
 })
 
+describe('encryption key configuration guard (#1996)', () => {
+  afterEach(() => {
+    vi.unstubAllEnvs()
+  })
+
+  it('throws a coded error in production when PERSONNUMMER_ENCRYPTION_KEY is unset', () => {
+    vi.stubEnv('NODE_ENV', 'production')
+    vi.stubEnv('PERSONNUMMER_ENCRYPTION_KEY', '')
+
+    let thrown: unknown
+    try {
+      encryptPersonnummer('199001019802')
+    } catch (err) {
+      thrown = err
+    }
+    expect(thrown).toBeInstanceOf(Error)
+    // The code is what withRouteContext -> errorResponse() dispatches on, so
+    // the route answers 503 with the registry message instead of a generic 500.
+    expect((thrown as { code?: unknown }).code).toBe(PERSONNUMMER_ENCRYPTION_NOT_CONFIGURED)
+    expect(PERSONNUMMER_ENCRYPTION_NOT_CONFIGURED).toBe('PERSONNUMMER_ENCRYPTION_NOT_CONFIGURED')
+  })
+
+  it('decrypt is guarded the same way (genuine ciphertext, no key, production)', () => {
+    const ciphertext = encryptPersonnummer('199001019802')
+    vi.stubEnv('NODE_ENV', 'production')
+    vi.stubEnv('PERSONNUMMER_ENCRYPTION_KEY', '')
+
+    expect(() => decryptPersonnummer(ciphertext)).toThrow(
+      expect.objectContaining({ code: PERSONNUMMER_ENCRYPTION_NOT_CONFIGURED }),
+    )
+  })
+
+  it('falls back to the deterministic dev key outside production', () => {
+    vi.stubEnv('NODE_ENV', 'test')
+    vi.stubEnv('PERSONNUMMER_ENCRYPTION_KEY', '')
+
+    const encrypted = encryptPersonnummer('199001019802')
+    expect(decryptPersonnummer(encrypted)).toBe('199001019802')
+  })
+})
+
 describe('decryptPersonnummer tolerance for unencrypted rows', () => {
   it('passes a raw 12-digit personnummer through unchanged (no crash)', () => {
     // A row stored unencrypted (pre-fix v1 create, or a seed) would otherwise

lib/salary/personnummer.ts

@@ -7,6 +7,16 @@ const TAG_LENGTH = 16
 
 const logger = createLogger('salary/personnummer')
 
+/**
+ * Registry code (lib/errors/structured-errors.ts) for a production deployment
+ * that never set PERSONNUMMER_ENCRYPTION_KEY. Carried on the thrown Error so
+ * withRouteContext -> errorResponse() answers with the typed 503 envelope
+ * ("krypteringsnyckel saknas, kontakta supporten") instead of the generic
+ * INTERNAL_ERROR 500, which reads as a transient failure and invites retries
+ * that can never succeed. #1996
+ */
+export const PERSONNUMMER_ENCRYPTION_NOT_CONFIGURED = 'PERSONNUMMER_ENCRYPTION_NOT_CONFIGURED'
+
 /**
  * Get the encryption key from environment.
  * Falls back to a dev-only key for local development.
@@ -15,7 +25,9 @@ function getEncryptionKey(): Buffer {
   const envKey = process.env.PERSONNUMMER_ENCRYPTION_KEY
   if (!envKey) {
     if (process.env.NODE_ENV === 'production') {
-      throw new Error('PERSONNUMMER_ENCRYPTION_KEY is required in production')
+      throw Object.assign(new Error('PERSONNUMMER_ENCRYPTION_KEY is required in production'), {
+        code: PERSONNUMMER_ENCRYPTION_NOT_CONFIGURED,
+      })
     }
     // Dev-only deterministic key (NOT safe for production)
     return scryptSync('dev-only-key', 'gnubok-dev-salt', 32)