feat(salary): agent path to set this month's per-run salary (#2015)

* feat(salary): agent path to set this month's per-run salary Agents could not do variable owner pay: the only per-run edit tool, gnubok_update_payslip_line, edits the display-only Grundlon line that every recalculation rebuilds from salary_run_employees.monthly_salary, so the fixed employee salary silently won (user-reported). - lib/salary/run-employees.ts: setRunEmployeeSalary() shared service (draft gate, roundOre, 0 = nollkorning, display-line refresh); the cookie route PATCH now delegates to it (behavior unchanged) - MCP: gnubok_set_run_salary staged tool (search catalog: tools/list budget at zero headroom), op type set_run_salary (medium risk), commitSetRunSalary executor, payroll:write scope, payroll_month loadout + payroll-monthly skill step; update_payslip_line description now warns that recalc rebuilds base salary lines - v1 REST: PATCH /salary-runs/{id}/employees/{employeeId} accepting monthly_salary (draft only, dry-run, idempotency key) - Migration pair (NOT VALID + VALIDATE) adds set_run_salary to the pending_operations op-type CHECK; base list verified against prod live - Tests: service, staged tool, executor, cookie route, v1 route; spec snapshot updated Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MP37pE3zk667nP6S766iJG * fix(salary): harden set_run_salary per skeptic + CI findings - Clear calculation_breakdown when the per-run salary changes so the existing book preflights force a recalculation: a run can no longer be booked with gross/tax derived from the old salary (skeptic R1) - Enforce SALARY_OVERRIDE_MAX (10 MSEK) in the shared service and the v1 body schema: closes the unbounded/1e307-overflow path that wrote Infinity -> NULL -> 500 (skeptic R2) - Promote gnubok_set_run_salary to the default catalog: a search-only WRITE is uncallable on Claude.ai (update_customer lesson) while three surfaces pointed agents at it; payload ceiling bumped 63.8K -> 64.4K with a ledger entry, read-demotion left as its own change (skeptic R3) - Granskning label type_set_run_salary in vocabulary.ts + sv/en (R4) - Display-line refresh is fire-and-forget again (write already committed; matches pre-refactor route behavior) and DB error details carry the SQLSTATE code for Swedish error mapping - v1 risk metadata aligned to 'medium'; NOT_DRAFT message now covers salary edits, not just roster changes - npm run apiskill:generate committed (CI apiskill:check failure) Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MP37pE3zk667nP6S766iJG * chore(migrations): rename set_run_salary pair past main's newest versions origin/main gained 20260828120000 and 20260828154800 after this branch staged 20260828110000/1; out-of-order versions are skipped at merge, so the pair moves to 20260828160000/1 (byte-identical SQL, reference in the VALIDATE header updated). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MP37pE3zk667nP6S766iJG * chore: retrigger Supabase preview after migration-version repair The preview branch tracked 20260828110000/1 before the rename to 20260828160000/1; the orphan rows are deleted from the preview branch's schema_migrations (preview only, prod never saw those versions) and this empty commit re-runs the tasks. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MP37pE3zk667nP6S766iJG --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
2026-08-28 18:14:31 +02:00
parent a4ceaafa4f
commit e8aa0670ca
28 changed files with 1090 additions and 59 deletions
@@ -8,7 +8,7 @@
 import { beforeEach, describe, expect, it, vi } from 'vitest'
 import type { SupabaseClient } from '@supabase/supabase-js'
 import { createQueuedMockSupabase } from '@/tests/helpers'
-import { addEmployeeToRun, removeEmployeeFromRun } from '@/lib/salary/run-employees'
+import { addEmployeeToRun, removeEmployeeFromRun, setRunEmployeeSalary } from '@/lib/salary/run-employees'

 const COMPANY_ID = 'aaaaaaaa-aaaa-4aaa-8aaa-aaaaaaaaaaaa'
 const RUN_ID = 'cccccccc-cccc-4ccc-8ccc-cccccccccccc'
@@ -217,3 +217,164 @@ describe('removeEmployeeFromRun', () => {
    expect(fromCalls).toEqual(['salary_runs', 'salary_run_employees'])
  })
 })
+
+describe('setRunEmployeeSalary', () => {
+  const SRE_ROW = {
+    id: SRE_ID,
+    employee_id: EMPLOYEE_ID,
+    salary_type: 'monthly',
+    employment_degree: 80,
+    monthly_salary: 35000,
+  }
+
+  it('rejects a negative salary without touching the DB', async () => {
+    const result = await setRunEmployeeSalary(supabase, {
+      companyId: COMPANY_ID,
+      salaryRunId: RUN_ID,
+      employeeId: EMPLOYEE_ID,
+      monthlySalary: -1,
+    })
+    expect(result.ok).toBe(false)
+    if (!result.ok) expect(result.code).toBe('VALIDATION_ERROR')
+    expect((mock.supabase.from as ReturnType<typeof vi.fn>).mock.calls.length).toBe(0)
+  })
+
+  it('rejects a salary above SALARY_OVERRIDE_MAX without touching the DB', async () => {
+    const result = await setRunEmployeeSalary(supabase, {
+      companyId: COMPANY_ID,
+      salaryRunId: RUN_ID,
+      employeeId: EMPLOYEE_ID,
+      monthlySalary: 10_000_001,
+    })
+    expect(result.ok).toBe(false)
+    if (!result.ok) expect(result.code).toBe('VALIDATION_ERROR')
+    expect((mock.supabase.from as ReturnType<typeof vi.fn>).mock.calls.length).toBe(0)
+  })
+
+  it('rejects an overflow-scale salary (1e307) instead of writing Infinity', async () => {
+    const result = await setRunEmployeeSalary(supabase, {
+      companyId: COMPANY_ID,
+      salaryRunId: RUN_ID,
+      employeeId: EMPLOYEE_ID,
+      monthlySalary: 1e307,
+    })
+    expect(result.ok).toBe(false)
+    if (!result.ok) expect(result.code).toBe('VALIDATION_ERROR')
+  })
+
+  it('returns SALARY_RUN_NOT_FOUND for a missing run', async () => {
+    mock.enqueue({ data: null })
+    const result = await setRunEmployeeSalary(supabase, {
+      companyId: COMPANY_ID,
+      salaryRunId: RUN_ID,
+      employeeId: EMPLOYEE_ID,
+      monthlySalary: 45000,
+    })
+    expect(result).toEqual({ ok: false, code: 'SALARY_RUN_NOT_FOUND' })
+  })
+
+  it('gates on draft status', async () => {
+    mock.enqueue({ data: { id: RUN_ID, status: 'review' } })
+    const result = await setRunEmployeeSalary(supabase, {
+      companyId: COMPANY_ID,
+      salaryRunId: RUN_ID,
+      employeeId: EMPLOYEE_ID,
+      monthlySalary: 45000,
+    })
+    expect(result.ok).toBe(false)
+    if (!result.ok) expect(result.code).toBe('SALARY_RUN_EMPLOYEES_NOT_DRAFT')
+  })
+
+  it('returns SALARY_RUN_EMPLOYEE_NOT_FOUND when the employee is not on the run', async () => {
+    mock.enqueue({ data: { id: RUN_ID, status: 'draft' } })
+    mock.enqueue({ data: null })
+    const result = await setRunEmployeeSalary(supabase, {
+      companyId: COMPANY_ID,
+      salaryRunId: RUN_ID,
+      employeeId: EMPLOYEE_ID,
+      monthlySalary: 45000,
+    })
+    expect(result).toEqual({ ok: false, code: 'SALARY_RUN_EMPLOYEE_NOT_FOUND' })
+  })
+
+  it('updates the per-run salary and refreshes the display line (happy path)', async () => {
+    mock.enqueue({ data: { id: RUN_ID, status: 'draft' } })
+    mock.enqueue({ data: SRE_ROW })
+    mock.enqueue({ data: null }) // sre update
+    mock.enqueue({ data: null }) // line update
+
+    const result = await setRunEmployeeSalary(supabase, {
+      companyId: COMPANY_ID,
+      salaryRunId: RUN_ID,
+      employeeId: EMPLOYEE_ID,
+      monthlySalary: 45000.005,
+    })
+
+    expect(result.ok).toBe(true)
+    if (result.ok) {
+      expect(result.data.previous_monthly_salary).toBe(35000)
+      expect(result.data.monthly_salary).toBe(45000.01) // roundOre applied
+      expect(result.data.salary_run_employee_id).toBe(SRE_ID)
+    }
+    const fromCalls = (mock.supabase.from as ReturnType<typeof vi.fn>).mock.calls.map((c) => c[0])
+    expect(fromCalls).toEqual([
+      'salary_runs',
+      'salary_run_employees',
+      'salary_run_employees',
+      'salary_line_items',
+    ])
+  })
+
+  it('accepts 0 as an intentional nollkörning', async () => {
+    mock.enqueue({ data: { id: RUN_ID, status: 'draft' } })
+    mock.enqueue({ data: SRE_ROW })
+    mock.enqueue({ data: null })
+    mock.enqueue({ data: null })
+
+    const result = await setRunEmployeeSalary(supabase, {
+      companyId: COMPANY_ID,
+      salaryRunId: RUN_ID,
+      employeeId: EMPLOYEE_ID,
+      monthlySalary: 0,
+    })
+    expect(result.ok).toBe(true)
+    if (result.ok) expect(result.data.monthly_salary).toBe(0)
+  })
+
+  it('skips the display-line refresh for hourly employees', async () => {
+    mock.enqueue({ data: { id: RUN_ID, status: 'draft' } })
+    mock.enqueue({ data: { ...SRE_ROW, salary_type: 'hourly' } })
+    mock.enqueue({ data: null }) // sre update only
+
+    const result = await setRunEmployeeSalary(supabase, {
+      companyId: COMPANY_ID,
+      salaryRunId: RUN_ID,
+      employeeId: EMPLOYEE_ID,
+      monthlySalary: 45000,
+    })
+    expect(result.ok).toBe(true)
+    const fromCalls = (mock.supabase.from as ReturnType<typeof vi.fn>).mock.calls.map((c) => c[0])
+    expect(fromCalls).toEqual(['salary_runs', 'salary_run_employees', 'salary_run_employees'])
+  })
+
+  it('dry-run resolves old and new salary without writing', async () => {
+    mock.enqueue({ data: { id: RUN_ID, status: 'draft' } })
+    mock.enqueue({ data: SRE_ROW })
+
+    const result = await setRunEmployeeSalary(supabase, {
+      companyId: COMPANY_ID,
+      salaryRunId: RUN_ID,
+      employeeId: EMPLOYEE_ID,
+      monthlySalary: 45000,
+      dryRun: true,
+    })
+
+    expect(result.ok).toBe(true)
+    if (result.ok) {
+      expect(result.data.previous_monthly_salary).toBe(35000)
+      expect(result.data.monthly_salary).toBe(45000)
+    }
+    const fromCalls = (mock.supabase.from as ReturnType<typeof vi.fn>).mock.calls.map((c) => c[0])
+    expect(fromCalls).toEqual(['salary_runs', 'salary_run_employees'])
+  })
+})
@@ -13,6 +13,7 @@
 import type { SupabaseClient } from '@supabase/supabase-js'
 import { getLineItemAccount } from '@/lib/salary/account-mapping'
 import { roundOre } from '@/lib/money'
+import { SALARY_OVERRIDE_MAX } from '@/lib/api/schemas'
 import type { SalaryLineItemType } from '@/types'

 export type RunEmployeeResult<T> =
@@ -184,6 +185,130 @@ export async function addEmployeeToRun(
  return { ok: true, data: sre as unknown as SalaryRunEmployeeRow }
 }

+export interface SetRunSalaryData {
+  salary_run_employee_id: string
+  employee_id: string
+  salary_type: string
+  employment_degree: number
+  previous_monthly_salary: number
+  monthly_salary: number
+}
+
+/**
+ * Set THIS RUN's base salary for one employee (salary_run_employees.monthly_salary),
+ * leaving the employee master record untouched. Draft runs only. 0 is valid
+ * (an intentional nollkörning). The calculation engine reads this per-run value,
+ * so the displayed 'Grundlön' line refresh here is display-only; a recalculation
+ * derives gross from the column, never from the line.
+ */
+export async function setRunEmployeeSalary(
+  supabase: SupabaseClient,
+  args: {
+    companyId: string
+    salaryRunId: string
+    employeeId: string
+    monthlySalary: number
+    /** Validate + resolve only; return the would-be change without writing. */
+    dryRun?: boolean
+  },
+): Promise<RunEmployeeResult<SetRunSalaryData>> {
+  // Single enforcement point for the salary bound: the cookie route's Zod
+  // schema, the v1 body schema and the MCP tool all funnel through here. The
+  // cap also keeps roundOre far away from Infinity (1e307 * 100 overflows).
+  if (
+    !Number.isFinite(args.monthlySalary) ||
+    args.monthlySalary < 0 ||
+    args.monthlySalary > SALARY_OVERRIDE_MAX
+  ) {
+    return {
+      ok: false,
+      code: 'VALIDATION_ERROR',
+      details: { field: 'monthly_salary', max: SALARY_OVERRIDE_MAX },
+    }
+  }
+  const gate = await assertRunDraftForRoster(supabase, args.companyId, args.salaryRunId)
+  if (!gate.ok) return gate
+
+  const monthly = roundOre(args.monthlySalary)
+
+  const { data: sre, error: sreError } = await supabase
+    .from('salary_run_employees')
+    .select('id, employee_id, salary_type, employment_degree, monthly_salary')
+    .eq('salary_run_id', args.salaryRunId)
+    .eq('employee_id', args.employeeId)
+    .eq('company_id', args.companyId)
+    .maybeSingle()
+
+  if (sreError) {
+    return {
+      ok: false,
+      code: 'INTERNAL_ERROR',
+      details: { message: sreError.message, code: sreError.code },
+    }
+  }
+  if (!sre) {
+    return { ok: false, code: 'SALARY_RUN_EMPLOYEE_NOT_FOUND' }
+  }
+
+  const row = sre as {
+    id: string
+    employee_id: string
+    salary_type: string
+    employment_degree: number
+    monthly_salary: number
+  }
+
+  const data: SetRunSalaryData = {
+    salary_run_employee_id: row.id,
+    employee_id: row.employee_id,
+    salary_type: row.salary_type,
+    employment_degree: row.employment_degree,
+    previous_monthly_salary: row.monthly_salary,
+    monthly_salary: monthly,
+  }
+
+  if (args.dryRun) {
+    return { ok: true, data }
+  }
+
+  // Clearing calculation_breakdown is what makes a stale booking impossible:
+  // both book preflights (MCP gnubok_book_salary_run and the v1/UI path via
+  // advanceAndBookSalaryRun) refuse roster rows without a breakdown, so a
+  // salary change after a calculation forces a recalculation before booking
+  // instead of silently booking gross/tax derived from the old salary.
+  const { error: updError } = await supabase
+    .from('salary_run_employees')
+    .update({ monthly_salary: monthly, calculation_breakdown: null })
+    .eq('id', row.id)
+    .eq('company_id', args.companyId)
+
+  if (updError) {
+    return {
+      ok: false,
+      code: 'INTERNAL_ERROR',
+      details: { message: updError.message, code: updError.code },
+    }
+  }
+
+  // Keep the displayed 'Grundlön' line consistent before the next calculation.
+  // Display-only: the engine recomputes baseSalary from monthly_salary at calc
+  // time, and the next calculation rewrites this row anyway, so a failure here
+  // must not fail the request: the salary write above has already committed,
+  // and reporting failure for an applied change is worse than a briefly stale
+  // display row (matches the pre-refactor route behavior).
+  if (row.salary_type === 'monthly') {
+    const baseAmount = roundOre(monthly * (row.employment_degree / 100))
+    await supabase
+      .from('salary_line_items')
+      .update({ amount: baseAmount })
+      .eq('salary_run_employee_id', row.id)
+      .eq('company_id', args.companyId)
+      .eq('item_type', 'monthly_salary')
+  }
+
+  return { ok: true, data }
+}
+
 export async function removeEmployeeFromRun(
  supabase: SupabaseClient,
  args: {