From f43a6653f1cd57c37cc0c34f491fda9706a83d60 Mon Sep 17 00:00:00 2001 From: Mattsson <111893710+mattssonn@users.noreply.github.com> Date: Sun, 30 Aug 2026 21:43:23 +0200 Subject: [PATCH] feat(salary): update_salary_run MCP tool and editable draft payment date (#2041) * feat(salary): update_salary_run MCP tool and editable draft payment date payment_date drives the booking entry date but was only editable via the v1 PATCH. Close the gap on both remaining surfaces: - New staged MCP write tool gnubok_update_salary_run (search-only catalog; tools/list budget is at zero headroom) accepting the exact v1 PATCH field set: payment_date, voucher_series, notes. Draft-only with the same optimistic lock semantics, via a new shared service lib/salary/update-run.ts used by both the staging preflight and the commit executor. - Run header UI: payment date on a draft run is now an inline date input (prefilled, committed on blur/Enter, snaps back on failure), saved through the existing internal PATCH. Read-only once not draft. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_018zGah8Yy49esAwpnKGxiGy * fix(salary): op-type migration, calc invalidation on date change, scanner compliance Consolidated CI + review fix pass for #2041: - pg-real: add 'update_salary_run' to pending_operations_operation_type_check (wholesale re-create, NOT VALID + VALIDATE pair, mirroring 20260828160000/1). - Swedish accounting review: a payment_date change on a draft run now clears every roster row's calculation_breakdown (shared service and internal PATCH alike), so both book preflights refuse the run until a recalculation has run against the new date; skatteavdrag and the AGI redovisningsperiod follow the payment month. Staging preview exposes invalidates_calculation and the next hint states the clearing. - no-phantom-columns: literal select strings in update-run.ts; ceiling +1 with a documented reason for the inherent patch-shaped UPDATE payload. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_018zGah8Yy49esAwpnKGxiGy * fix(salary): close skeptic findings on payment_date editing Skeptic round 1 refuted two paths; both closed: - Retry idempotency (correctness): the calculation_breakdown clear was gated on new-date-differs-from-stored, so a retry after a partial failure (header committed, clear failed) compared against the already updated date and skipped the clear forever, leaving a stale calculation bookable. The clear is now gated on payment_date being SUPPLIED, on all three surfaces (shared service, internal PATCH, v1 PATCH: the v1 route previously had no clear at all and bypassed the invariant). - Kontantprincipen (compliance): AGI derives its redovisningsperiod from period_year/period_month while the verifikat books on payment_date, so a cross-month payment_date change could book salary in one month and declare it in another. All three edit surfaces now refuse a payment_date outside the run's period month with the new structured error SALARY_RUN_PAYMENT_DATE_OUTSIDE_PERIOD; the UI date input is min/max-bounded to the period month. - The internal PATCH update is now optimistic-locked on status='draft' (races return 400 instead of silently writing), matching the v1 PATCH and the shared service, and the clear cannot fire for a run that left draft. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_018zGah8Yy49esAwpnKGxiGy * fix(salary): carry book_skattekonto op types through the constraint re-create The sibling migration 20260830130000 (merged from main) re-created pending_operations_operation_type_check with book_skattekonto_row and book_skattekonto_rows. This branch's 20260830150000 sorts after it and re-creates the constraint wholesale, so its list must be that migration's superset or the two values would be silently revoked at apply time. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_018zGah8Yy49esAwpnKGxiGy * fix(salary): value-validate internal PATCH and grandfather out-of-period dates Two skeptic follow-ups: - The internal PATCH now validates values, not just keys: JSON body must be an object, payment_date must be ISO (shared ISO_DATE_RE), voucher_series a single A-Z letter, notes a string of max 2000 chars or null: the same rules as the v1 UpdateSalaryRunSchema, so nothing unvalidated can reach the DB through the whitelist. - Creation does not (yet) couple payment_date to the period month, so a legally created out-of-period date must stay correctable. All three edit surfaces now allow day adjustments within the run's CURRENT payment month as well as the period month (grandfather clause); no move can introduce a new wrong month. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_018zGah8Yy49esAwpnKGxiGy * fix(salary): resolve migration version collision with delete_draft_invoice Main's delete_draft_invoice PR landed on the same 20260830150000/150001 versions and also re-creates pending_operations_operation_type_check. Rename this branch's pair to 20260830160000/160001 (applies last) and carry delete_draft_invoice through the wholesale re-create so nothing is silently revoked. Final list = sibling's list + update_salary_run. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_018zGah8Yy49esAwpnKGxiGy * docs(salary): regenerate accounted-api skill for the new PATCH pitfalls apiskill:check byte-compares the generated skill against the registry; the two pitfalls added to the v1 salary-runs PATCH endpoint made references/salary-runs.md stale and failed Core Build. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_018zGah8Yy49esAwpnKGxiGy --------- Co-authored-by: Claude Fable 5 --- app/(dashboard)/salary/runs/[id]/page.tsx | 41 +++ .../salary/runs/[id]/__tests__/route.test.ts | 175 +++++++++++- app/api/salary/runs/[id]/route.ts | 81 +++++- .../[companyId]/salary-runs/[id]/route.ts | 42 +++ .../salary-runs/__tests__/route.test.ts | 69 ++++- components/salary/run/RunHeader.tsx | 57 +++- .../__tests__/payroll-staged-tools.test.ts | 137 ++++++++++ extensions/general/mcp-server/server.ts | 95 +++++++ lib/auth/scope-catalog.ts | 1 + lib/errors/structured-errors.ts | 10 + .../__tests__/payroll-executors.test.ts | 169 ++++++++++++ lib/pending-operations/commit.ts | 46 ++++ lib/pending-operations/risk-tiers.ts | 6 + lib/salary/update-run.ts | 250 ++++++++++++++++++ messages/en.json | 2 + messages/sv.json | 2 + .../accounted-api/references/salary-runs.md | 2 + ...nding_operations_add_update_salary_run.sql | 104 ++++++++ ...e_pending_operations_update_salary_run.sql | 6 + tests/schema/no-phantom-columns.test.ts | 8 +- types/index.ts | 4 + 21 files changed, 1293 insertions(+), 14 deletions(-) create mode 100644 lib/salary/update-run.ts create mode 100644 supabase/migrations/20260830160000_pending_operations_add_update_salary_run.sql create mode 100644 supabase/migrations/20260830160001_validate_pending_operations_update_salary_run.sql diff --git a/app/(dashboard)/salary/runs/[id]/page.tsx b/app/(dashboard)/salary/runs/[id]/page.tsx index b6809de1..9d03ced5 100644 --- a/app/(dashboard)/salary/runs/[id]/page.tsx +++ b/app/(dashboard)/salary/runs/[id]/page.tsx @@ -238,6 +238,46 @@ export default function SalaryRunPage({ params }: { params: Promise<{ id: string } } + // Draft-only payment-date edit from the header meta line, via the same + // internal PATCH the run has always supported. Returns success so the + // header can snap its input back to the server value on failure. + async function handleUpdatePaymentDate(date: string): Promise { + setActionLoading('payment_date') + try { + const res = await fetch(`/api/salary/runs/${id}`, { + method: 'PATCH', + headers: { 'Content-Type': 'application/json' }, + body: JSON.stringify({ payment_date: date }), + }) + if (res.ok) { + // Optimistic: the PATCH returns the updated run row. + const payload = await res.json().catch(() => null) + if (payload?.data) { + setRun(prev => (prev ? { ...prev, ...payload.data } : prev)) + } + toast({ title: t('toast_payment_date_updated') }) + loadRun() // background reconcile - not awaited + return true + } + const result = await res.json().catch(() => ({})) + toast({ + title: t('toast_status_failed'), + description: getErrorMessage(result, { context: 'salary', statusCode: res.status }), + variant: 'destructive', + }) + return false + } catch (err) { + toast({ + title: t('toast_status_failed'), + description: err instanceof Error ? getErrorMessage(err) : t('unknown_error'), + variant: 'destructive', + }) + return false + } finally { + setActionLoading(null) + } + } + // Approval is an authorization step. Missing bank details are an *overridable* // block (SALARY_APPROVE_BANK_DETAILS_MISSING) - rather than dead-ending on a // 400 toast, we surface a confirm dialog and re-approve with ?force=true when @@ -790,6 +830,7 @@ export default function SalaryRunPage({ params }: { params: Promise<{ id: string onDownloadAgi={handleDownloadAgi} onDelete={handleDelete} onCorrect={handleCorrect} + onUpdatePaymentDate={handleUpdatePaymentDate} /> {/* The one attention sentence on the page: an empty declaration will be diff --git a/app/api/salary/runs/[id]/__tests__/route.test.ts b/app/api/salary/runs/[id]/__tests__/route.test.ts index 433299d0..d6a99fa3 100644 --- a/app/api/salary/runs/[id]/__tests__/route.test.ts +++ b/app/api/salary/runs/[id]/__tests__/route.test.ts @@ -28,7 +28,7 @@ vi.mock('@/lib/auth/require-write', () => ({ requireWritePermission: vi.fn().mockResolvedValue({ ok: true }), })) -import { DELETE, GET } from '../route' +import { DELETE, GET, PATCH } from '../route' import { requireAuth } from '@/lib/auth/require-auth' import { encryptPersonnummer } from '@/lib/salary/personnummer' @@ -120,6 +120,179 @@ describe('DELETE /api/salary/runs/[id]', () => { }) }) +// ── PATCH (draft-only header edit) ─────────────────────────────────────────── + +describe('PATCH /api/salary/runs/[id]', () => { + beforeEach(() => { + vi.clearAllMocks() + }) + + function authorize(supabase: unknown) { + vi.mocked(requireAuth).mockResolvedValue({ + user: mockUser as never, + supabase: supabase as never, + error: null, + }) + } + + const DRAFT_RUN = { + id: 'run-1', + status: 'draft', + payment_date: '2026-07-25', + period_year: 2026, + period_month: 7, + } + + it('returns 400 when the run is not a draft', async () => { + const { supabase, enqueueMany } = createQueuedMockSupabase() + authorize(supabase) + + enqueueMany([ + { data: { ...DRAFT_RUN, status: 'review' } }, // lookup + ]) + + const request = createMockRequest('/api/salary/runs/run-1', { + method: 'PATCH', + body: { payment_date: '2026-07-23' }, + }) + const response = await PATCH(request, createMockRouteParams({ id: 'run-1' })) + const { status, body } = await parseJsonResponse<{ error: string }>(response) + + expect(status).toBe(400) + expect(body.error).toContain('utkast') + }) + + it('updates payment_date on a draft and clears roster calculation_breakdown', async () => { + const { supabase, enqueueMany, findCall } = createQueuedMockSupabase() + authorize(supabase) + + enqueueMany([ + { data: DRAFT_RUN }, // lookup + { data: { ...DRAFT_RUN, payment_date: '2026-07-23' } }, // update + { data: null }, // roster calculation_breakdown clear + ]) + + const request = createMockRequest('/api/salary/runs/run-1', { + method: 'PATCH', + body: { payment_date: '2026-07-23' }, + }) + const response = await PATCH(request, createMockRouteParams({ id: 'run-1' })) + const { status, body } = await parseJsonResponse<{ data: { payment_date: string } }>(response) + + expect(status).toBe(200) + expect(body.data.payment_date).toBe('2026-07-23') + // The new date invalidates any existing calculation: skatteavdrag and the + // AGI redovisningsperiod follow the payment month, and the book preflights + // refuse roster rows without a breakdown until a recalculation runs. + expect(findCall('salary_run_employees', 'update')).toEqual([ + { calculation_breakdown: null }, + ]) + }) + + it('does not clear calculations when only notes change', async () => { + const { supabase, enqueueMany, findCall } = createQueuedMockSupabase() + authorize(supabase) + + enqueueMany([ + { data: DRAFT_RUN }, // lookup + { data: { ...DRAFT_RUN, notes: 'Julbonus' } }, // update + ]) + + const request = createMockRequest('/api/salary/runs/run-1', { + method: 'PATCH', + body: { notes: 'Julbonus' }, + }) + const response = await PATCH(request, createMockRouteParams({ id: 'run-1' })) + const { status } = await parseJsonResponse(response) + + expect(status).toBe(200) + expect(findCall('salary_run_employees', 'update')).toBeUndefined() + }) + + it('rejects a payment_date outside the run period month (kontantprincipen)', async () => { + const { supabase, enqueueMany, findCall } = createQueuedMockSupabase() + authorize(supabase) + + enqueueMany([ + { data: DRAFT_RUN }, // lookup: period 2026-07 + ]) + + const request = createMockRequest('/api/salary/runs/run-1', { + method: 'PATCH', + body: { payment_date: '2026-08-01' }, + }) + const response = await PATCH(request, createMockRouteParams({ id: 'run-1' })) + const { status, body } = await parseJsonResponse<{ error: string }>(response) + + expect(status).toBe(400) + expect(body.error).toContain('period') + // Refused before any write. + expect(findCall('salary_runs', 'update')).toBeUndefined() + }) + + it('grandfathers day adjustments when the current date is already outside the period', async () => { + const { supabase, enqueueMany, findCall } = createQueuedMockSupabase() + authorize(supabase) + + enqueueMany([ + { data: { ...DRAFT_RUN, payment_date: '2026-08-05' } }, // period 2026-07, paid in August + { data: { ...DRAFT_RUN, payment_date: '2026-08-07' } }, // update + { data: null }, // roster calculation_breakdown clear + ]) + + const request = createMockRequest('/api/salary/runs/run-1', { + method: 'PATCH', + body: { payment_date: '2026-08-07' }, + }) + const response = await PATCH(request, createMockRouteParams({ id: 'run-1' })) + const { status } = await parseJsonResponse(response) + + expect(status).toBe(200) + expect(findCall('salary_run_employees', 'update')).toEqual([{ calculation_breakdown: null }]) + }) + + it('rejects an invalid voucher_series value', async () => { + const { supabase, enqueueMany, findCall } = createQueuedMockSupabase() + authorize(supabase) + + enqueueMany([ + { data: DRAFT_RUN }, // lookup + ]) + + const request = createMockRequest('/api/salary/runs/run-1', { + method: 'PATCH', + body: { voucher_series: 'AB' }, + }) + const response = await PATCH(request, createMockRouteParams({ id: 'run-1' })) + const { status } = await parseJsonResponse<{ error: string }>(response) + + expect(status).toBe(400) + expect(findCall('salary_runs', 'update')).toBeUndefined() + }) + + it('returns 400 when the run advances past draft between fetch and update (race)', async () => { + const { supabase, enqueueMany, findCall } = createQueuedMockSupabase() + authorize(supabase) + + enqueueMany([ + { data: DRAFT_RUN }, // lookup still sees draft + { data: null }, // status-locked update matches no row + ]) + + const request = createMockRequest('/api/salary/runs/run-1', { + method: 'PATCH', + body: { payment_date: '2026-07-23' }, + }) + const response = await PATCH(request, createMockRouteParams({ id: 'run-1' })) + const { status, body } = await parseJsonResponse<{ error: string }>(response) + + expect(status).toBe(400) + expect(body.error).toContain('utkast') + // The clear must not run when the locked update matched nothing. + expect(findCall('salary_run_employees', 'update')).toBeUndefined() + }) +}) + // ── GET detail additions (previous_run, corrected_by_run_id, deliveries) ──── // // GET consumes queue entries in from() order. The run loads first; the rest diff --git a/app/api/salary/runs/[id]/route.ts b/app/api/salary/runs/[id]/route.ts index 2aa2f43f..c29d3f67 100644 --- a/app/api/salary/runs/[id]/route.ts +++ b/app/api/salary/runs/[id]/route.ts @@ -4,6 +4,7 @@ import { withRouteContext } from '@/lib/api/with-route-context' import { formatRedovisare } from '@/lib/skatteverket/format' import { maskEmployeeForResponse } from '@/lib/salary/personnummer' import { getErrorMessage as getUserErrorMessage } from '@/lib/errors/get-error-message' +import { ISO_DATE_RE } from '@/lib/invariants' ensureInitialized() @@ -175,7 +176,7 @@ export const PATCH = withRouteContext<{ params: Promise<{ id: string }> }>( // Only allow updates on draft runs const { data: run, error: fetchError } = await supabase .from('salary_runs') - .select('id, status') + .select('id, status, payment_date, period_year, period_month') .eq('id', id) .eq('company_id', companyId) .single() @@ -189,6 +190,11 @@ export const PATCH = withRouteContext<{ params: Promise<{ id: string }> }>( } const body = await request.json() + if (typeof body !== 'object' || body === null || Array.isArray(body)) { + return NextResponse.json({ error: 'Ogiltig förfrågan' }, { status: 400 }) + } + // Whitelist keys AND values: only these three fields, with the same value + // rules as the v1 UpdateSalaryRunSchema, ever reach the DB. const allowedFields = ['payment_date', 'voucher_series', 'notes'] const updates: Record = {} for (const field of allowedFields) { @@ -196,18 +202,89 @@ export const PATCH = withRouteContext<{ params: Promise<{ id: string }> }>( updates[field] = body[field] } } + if ( + updates.payment_date !== undefined && + (typeof updates.payment_date !== 'string' || !ISO_DATE_RE.test(updates.payment_date)) + ) { + return NextResponse.json({ error: 'Ogiltigt datum (ÅÅÅÅ-MM-DD)' }, { status: 400 }) + } + if ( + updates.voucher_series !== undefined && + (typeof updates.voucher_series !== 'string' || !/^[A-Z]$/.test(updates.voucher_series)) + ) { + return NextResponse.json( + { error: 'Verifikationsserie måste vara en bokstav A-Z' }, + { status: 400 }, + ) + } + if ( + updates.notes !== undefined && + updates.notes !== null && + (typeof updates.notes !== 'string' || updates.notes.length > 2000) + ) { + return NextResponse.json({ error: 'Anteckningen får vara högst 2000 tecken' }, { status: 400 }) + } + // Kontantprincipen guard (SFL 26 kap): the AGI derives its + // redovisningsperiod from period_year/period_month while the verifikat + // books on payment_date, so a payment date outside the run's period month + // would post the entries in one month and declare them in another. Same + // rule as lib/salary/update-run.ts and the v1 PATCH, including the + // grandfather clause: a run created with an out-of-period payment date + // may still be day-adjusted within that same month. + if (typeof updates.payment_date === 'string') { + const periodPrefix = `${run.period_year}-${String(run.period_month).padStart(2, '0')}` + const newMonth = updates.payment_date.slice(0, 7) + const currentMonth = String(run.payment_date).slice(0, 7) + if (newMonth !== periodPrefix && newMonth !== currentMonth) { + return NextResponse.json( + { + error: + 'Utbetalningsdagen måste ligga i lönekörningens period: AGI redovisas per utbetalningsmånad.', + }, + { status: 400 }, + ) + } + } + + // Optimistic lock on status='draft': a concurrent step advancing the run + // (Beräkna → Till granskning in another tab) between the fetch above and + // this write must fail the write, not slip a frozen-field edit through + // (same guard as the v1 PATCH and lib/salary/update-run.ts). const { data: updated, error } = await supabase .from('salary_runs') .update(updates) .eq('id', id) .eq('company_id', companyId) + .eq('status', 'draft') .select() - .single() + .maybeSingle() if (error) { return NextResponse.json({ error: getUserErrorMessage(error) }, { status: 500 }) } + if (!updated) { + // Race: the run advanced past draft between fetch and update. + return NextResponse.json({ error: 'Kan bara redigera utkast' }, { status: 400 }) + } + + // A supplied payment_date invalidates any existing calculation: + // skatteavdrag follows the payment date, so clearing calculation_breakdown + // makes both book preflights refuse the roster until a recalculation has + // run against the new date (same invariant as setRunEmployeeSalary in + // lib/salary/run-employees.ts and lib/salary/update-run.ts). Gated on + // SUPPLIED, not on changed, so a retry after a partial failure re-clears + // instead of comparing against the already-updated date and skipping. + if (updates.payment_date !== undefined) { + const { error: clearError } = await supabase + .from('salary_run_employees') + .update({ calculation_breakdown: null }) + .eq('salary_run_id', id) + .eq('company_id', companyId) + if (clearError) { + return NextResponse.json({ error: getUserErrorMessage(clearError) }, { status: 500 }) + } + } return NextResponse.json({ data: updated }) }, diff --git a/app/api/v1/companies/[companyId]/salary-runs/[id]/route.ts b/app/api/v1/companies/[companyId]/salary-runs/[id]/route.ts index 46a17f39..951a0c68 100644 --- a/app/api/v1/companies/[companyId]/salary-runs/[id]/route.ts +++ b/app/api/v1/companies/[companyId]/salary-runs/[id]/route.ts @@ -149,6 +149,8 @@ registerEndpoint({ pitfalls: [ 'Returns 400 SALARY_RUN_PATCH_NOT_DRAFT if status !== "draft".', 'period_year + period_month are immutable post-create.', + 'payment_date must stay within the run\'s period month (400 SALARY_RUN_PAYMENT_DATE_OUTSIDE_PERIOD otherwise): the AGI is declared per payment month. A run whose current payment date already sits outside the period month may still be day-adjusted within that same month.', + 'Supplying payment_date clears every roster row\'s calculation_breakdown, so an already-calculated run must be recalculated before :approve/:book.', ], example: { request: { payment_date: '2026-05-23' }, @@ -248,6 +250,28 @@ export const PATCH = withApiV1<{ params: Promise<{ companyId: string; id: string return ok(existing, { requestId: ctx.requestId }) } + // Kontantprincipen guard (SFL 26 kap): the AGI derives its + // redovisningsperiod from period_year/period_month while the verifikat + // books on payment_date, so a payment date outside the run's period month + // would post the entries in one month and declare them in another. Same + // rule as lib/salary/update-run.ts and the internal dashboard PATCH, + // including the grandfather clause: a run created with an out-of-period + // payment date may still be day-adjusted within that same month, since + // creation does not (yet) enforce the coupling. No move can introduce a + // NEW wrong month. + if (typeof updates.payment_date === 'string') { + const ex = existing as { period_year: number; period_month: number; payment_date: string } + const periodPrefix = `${ex.period_year}-${String(ex.period_month).padStart(2, '0')}` + const newMonth = updates.payment_date.slice(0, 7) + const currentMonth = ex.payment_date.slice(0, 7) + if (newMonth !== periodPrefix && newMonth !== currentMonth) { + return v1ErrorResponseFromCode('SALARY_RUN_PAYMENT_DATE_OUTSIDE_PERIOD', ctx.log, { + requestId: ctx.requestId, + details: { period: periodPrefix, payment_date: updates.payment_date }, + }) + } + } + if (ctx.dryRun) { const merged = { ...(existing as object), ...updates } return dryRunPreview(merged, { requestId: ctx.requestId, log: ctx.log }) @@ -276,6 +300,24 @@ export const PATCH = withApiV1<{ params: Promise<{ companyId: string; id: string }) } + // A supplied payment_date invalidates any existing calculation: + // skatteavdrag follows the payment date, so clearing calculation_breakdown + // makes both book preflights refuse the roster until a recalculation has + // run against the new date (same invariant as setRunEmployeeSalary in + // lib/salary/run-employees.ts and lib/salary/update-run.ts). Gated on + // SUPPLIED, not on changed, so a retry after a partial failure re-clears + // instead of comparing against the already-updated date and skipping. + if (updates.payment_date !== undefined) { + const { error: clearError } = await ctx.supabase + .from('salary_run_employees') + .update({ calculation_breakdown: null }) + .eq('salary_run_id', idParse.data) + .eq('company_id', ctx.companyId!) + if (clearError) { + return v1ErrorResponse(clearError, ctx.log, { requestId: ctx.requestId }) + } + } + return ok(data, { requestId: ctx.requestId }) }, ) diff --git a/app/api/v1/companies/[companyId]/salary-runs/__tests__/route.test.ts b/app/api/v1/companies/[companyId]/salary-runs/__tests__/route.test.ts index 3eac8749..8c78876f 100644 --- a/app/api/v1/companies/[companyId]/salary-runs/__tests__/route.test.ts +++ b/app/api/v1/companies/[companyId]/salary-runs/__tests__/route.test.ts @@ -357,15 +357,15 @@ describe('POST /api/v1/companies/:companyId/salary-runs', () => { }) describe('PATCH /api/v1/companies/:companyId/salary-runs/:id', () => { - it('updates a draft salary run', async () => { + it('updates a draft salary run and clears roster calculation_breakdown', async () => { const updated = { ...SAMPLE_RUN, payment_date: '2026-05-23' } - mockServiceClient.mockReturnValue( - makeFlexibleSupabase({ - company_members: { data: { company_id: COMPANY_ID, role: 'owner' }, error: null }, - salary_runs: [{ data: SAMPLE_RUN, error: null }, { data: updated, error: null }], - idempotency_keys: { data: null, error: null }, - }), - ) + const flex = makeFlexibleSupabase({ + company_members: { data: { company_id: COMPANY_ID, role: 'owner' }, error: null }, + salary_runs: [{ data: SAMPLE_RUN, error: null }, { data: updated, error: null }], + salary_run_employees: { data: null, error: null }, + idempotency_keys: { data: null, error: null }, + }) + mockServiceClient.mockReturnValue(flex) const res = await updateSalaryRun( makeRequest(`https://x.test/api/v1/companies/${COMPANY_ID}/salary-runs/${RUN_ID}`, { @@ -378,6 +378,59 @@ describe('PATCH /api/v1/companies/:companyId/salary-runs/:id', () => { expect(res.status).toBe(200) const body = await res.json() expect(body.data.payment_date).toBe('2026-05-23') + // A supplied payment_date invalidates any existing calculation so the + // book preflights force a recalculation against the new date. + expect(flex.from).toHaveBeenCalledWith('salary_run_employees') + }) + + it('returns 400 SALARY_RUN_PAYMENT_DATE_OUTSIDE_PERIOD for a cross-month payment_date', async () => { + // Period 2026-05: moving the payment into June would book the verifikat + // in June while the AGI still declares 202605 (kontantprincipen). + const flex = makeFlexibleSupabase({ + company_members: { data: { company_id: COMPANY_ID, role: 'owner' }, error: null }, + salary_runs: { data: SAMPLE_RUN, error: null }, + idempotency_keys: { data: null, error: null }, + }) + mockServiceClient.mockReturnValue(flex) + + const res = await updateSalaryRun( + makeRequest(`https://x.test/api/v1/companies/${COMPANY_ID}/salary-runs/${RUN_ID}`, { + method: 'PATCH', + body: JSON.stringify({ payment_date: '2026-06-05' }), + }), + detailParams(COMPANY_ID, RUN_ID), + ) + + expect(res.status).toBe(400) + const body = await res.json() + expect(body.error.code).toBe('SALARY_RUN_PAYMENT_DATE_OUTSIDE_PERIOD') + expect(flex.from).not.toHaveBeenCalledWith('salary_run_employees') + }) + + it('grandfathers day adjustments when the current date is already outside the period', async () => { + // Creation does not enforce the period coupling, so a legally created + // out-of-period date must stay correctable within its own month. + const grandfathered = { ...SAMPLE_RUN, payment_date: '2026-06-05' } + const updated = { ...SAMPLE_RUN, payment_date: '2026-06-07' } + const flex = makeFlexibleSupabase({ + company_members: { data: { company_id: COMPANY_ID, role: 'owner' }, error: null }, + salary_runs: [{ data: grandfathered, error: null }, { data: updated, error: null }], + salary_run_employees: { data: null, error: null }, + idempotency_keys: { data: null, error: null }, + }) + mockServiceClient.mockReturnValue(flex) + + const res = await updateSalaryRun( + makeRequest(`https://x.test/api/v1/companies/${COMPANY_ID}/salary-runs/${RUN_ID}`, { + method: 'PATCH', + body: JSON.stringify({ payment_date: '2026-06-07' }), + }), + detailParams(COMPANY_ID, RUN_ID), + ) + + expect(res.status).toBe(200) + const body = await res.json() + expect(body.data.payment_date).toBe('2026-06-07') }) it('returns 400 SALARY_RUN_PATCH_NOT_DRAFT for non-draft status', async () => { diff --git a/components/salary/run/RunHeader.tsx b/components/salary/run/RunHeader.tsx index a33b088c..329a9828 100644 --- a/components/salary/run/RunHeader.tsx +++ b/components/salary/run/RunHeader.tsx @@ -5,6 +5,7 @@ import Link from 'next/link' import { useTranslations } from 'next-intl' import { Badge } from '@/components/ui/badge' import { Button } from '@/components/ui/button' +import { Input } from '@/components/ui/input' import { Dialog, DialogContent, @@ -78,6 +79,10 @@ interface RunHeaderProps { onDownloadAgi: () => void onDelete: () => void onCorrect: () => void + // Draft-only: payment_date is frozen once the run leaves draft (it becomes + // the booking entry date), so the header only offers the editor there. + // Resolves false when the save failed, so the input can snap back. + onUpdatePaymentDate: (date: string) => Promise } export function RunHeader({ @@ -95,6 +100,7 @@ export function RunHeader({ onDownloadAgi, onDelete, onCorrect, + onUpdatePaymentDate, }: RunHeaderProps) { const t = useTranslations('salary_run') const tSalary = useTranslations('salary') @@ -135,9 +141,56 @@ export function RunHeader({ actionLoading === 'bulk_payslip' || actionLoading === 'agi-download' + // Editable while draft: prefilled with the effective value, committed on + // blur/Enter so half-typed dates never fire a PATCH. The draft state resyncs + // whenever the server value changes (save reconcile, status transitions), + // via the adjust-state-during-render pattern rather than an effect. + const paymentDateEditable = canWrite && run.status === 'draft' + const [paymentDateDraft, setPaymentDateDraft] = useState(run.payment_date) + const [lastServerPaymentDate, setLastServerPaymentDate] = useState(run.payment_date) + if (run.payment_date !== lastServerPaymentDate) { + setLastServerPaymentDate(run.payment_date) + setPaymentDateDraft(run.payment_date) + } + + async function commitPaymentDate() { + if (!paymentDateDraft || paymentDateDraft === run.payment_date) { + // Empty or unchanged: snap back to the server value instead of saving. + setPaymentDateDraft(run.payment_date) + return + } + const saved = await onUpdatePaymentDate(paymentDateDraft) + if (!saved) setPaymentDateDraft(run.payment_date) + } + + // The payment date must stay within the run's period month: the AGI is + // declared per payment month (kontantprincipen), so the API refuses dates + // outside it. min/max keeps the native picker inside the month; typed + // values outside it still get the server's 400 toast. + const periodMonthPrefix = `${run.period_year}-${String(run.period_month).padStart(2, '0')}` + const periodMonthLastDay = new Date(run.period_year, run.period_month, 0).getDate() + const metaParts: React.ReactNode[] = [ - - {t('payment_date_label')} {formatDate(run.payment_date)} + + {t('payment_date_label')}{' '} + {paymentDateEditable ? ( + setPaymentDateDraft(e.target.value)} + onBlur={commitPaymentDate} + onKeyDown={(e) => { + if (e.key === 'Enter') e.currentTarget.blur() + }} + disabled={busy} + aria-label={t('payment_date_edit_aria')} + className="h-8 w-fit px-2 py-0 text-sm tabular-nums" + /> + ) : ( + {formatDate(run.payment_date)} + )} , {t('header_employees', { count: employeeCount })}, ] diff --git a/extensions/general/mcp-server/__tests__/payroll-staged-tools.test.ts b/extensions/general/mcp-server/__tests__/payroll-staged-tools.test.ts index 1f2cbd57..f4ce3e6a 100644 --- a/extensions/general/mcp-server/__tests__/payroll-staged-tools.test.ts +++ b/extensions/general/mcp-server/__tests__/payroll-staged-tools.test.ts @@ -20,6 +20,7 @@ import { decryptPersonnummer } from '@/lib/salary/personnummer' const updatePayslipLine = tools.find((t) => t.name === 'gnubok_update_payslip_line')! const setRunSalary = tools.find((t) => t.name === 'gnubok_set_run_salary')! +const updateSalaryRun = tools.find((t) => t.name === 'gnubok_update_salary_run')! const registerAbsence = tools.find((t) => t.name === 'gnubok_register_absence')! const bookSalaryRun = tools.find((t) => t.name === 'gnubok_book_salary_run')! const deleteAbsence = tools.find((t) => t.name === 'gnubok_delete_absence')! @@ -197,6 +198,142 @@ describe('gnubok_set_run_salary', () => { }) }) +describe('gnubok_update_salary_run', () => { + const RUN_ROW = { + id: 'run-1', + status: 'draft', + period_year: 2026, + period_month: 3, + payment_date: '2026-03-25', + voucher_series: 'L', + notes: null, + } + + it('stages a payment_date change with old/new preview and a recalculate next-hint', async () => { + const { supabase, enqueue } = createQueuedMockSupabase() + enqueue({ data: RUN_ROW }) // service draft gate + current values (dry-run preflight) + enqueue({ data: null }) // resolvePeriodStatusForDate: company_settings + enqueue({ data: null }) // resolvePeriodStatusForDate: fiscal_periods + enqueue({ data: { id: 'op-1' }, error: null }) // pending_operations insert + + const result = (await updateSalaryRun.execute( + { salary_run_id: 'run-1', payment_date: '2026-03-23' }, + 'company-1', 'user-1', supabase as never, { type: 'user' }, + )) as { + staged: boolean + risk_level: string + message: string + preview: Record + next?: { tool: string } + } + + expect(result.staged).toBe(true) + expect(result.risk_level).toBe('low') + expect((result.preview.previous as Record).payment_date).toBe('2026-03-25') + expect(result.preview.new_payment_date).toBe('2026-03-23') + expect(result.preview.salary_run_id).toBe('run-1') + expect(result.preview.invalidates_calculation).toBe(true) + expect(result.next?.tool).toBe('gnubok_calculate_salary_run') + }) + + it('stages a notes-only change without the recalculate next-hint', async () => { + const { supabase, enqueue } = createQueuedMockSupabase() + enqueue({ data: RUN_ROW }) // service draft gate (dry-run preflight) + enqueue({ data: null }) // resolvePeriodStatusForDate: company_settings + enqueue({ data: null }) // resolvePeriodStatusForDate: fiscal_periods + enqueue({ data: { id: 'op-1' }, error: null }) // pending_operations insert + + const result = (await updateSalaryRun.execute( + { salary_run_id: 'run-1', notes: 'Extra utbetalning i mars' }, + 'company-1', 'user-1', supabase as never, { type: 'user' }, + )) as { staged: boolean; preview: Record; next?: { tool: string } } + + expect(result.staged).toBe(true) + expect(result.preview.new_notes).toBe('Extra utbetalning i mars') + expect(result.preview.invalidates_calculation).toBe(false) + // Payment date untouched: no recalculation hint. + expect(result.next).toBeUndefined() + }) + + it('throws when the run has advanced past draft (preflight)', async () => { + const { supabase, enqueue } = createQueuedMockSupabase() + enqueue({ data: { ...RUN_ROW, status: 'review' } }) + + await expect( + updateSalaryRun.execute( + { salary_run_id: 'run-1', payment_date: '2026-03-23' }, + 'company-1', 'user-1', supabase as never, { type: 'user' }, + ), + ).rejects.toThrow(/SALARY_RUN_PATCH_NOT_DRAFT/) + }) + + it('grandfathers day adjustments for a run whose current date is already outside the period', async () => { + // Creation does not enforce the period coupling, so a legally created + // out-of-period date must stay correctable within its own month. + const { supabase, enqueue } = createQueuedMockSupabase() + enqueue({ data: { ...RUN_ROW, payment_date: '2026-04-05' } }) // period 2026-03, paid in April + enqueue({ data: null }) // resolvePeriodStatusForDate: company_settings + enqueue({ data: null }) // resolvePeriodStatusForDate: fiscal_periods + enqueue({ data: { id: 'op-1' }, error: null }) // pending_operations insert + + const result = (await updateSalaryRun.execute( + { salary_run_id: 'run-1', payment_date: '2026-04-07' }, + 'company-1', 'user-1', supabase as never, { type: 'user' }, + )) as { staged: boolean; preview: Record } + + expect(result.staged).toBe(true) + expect(result.preview.new_payment_date).toBe('2026-04-07') + }) + + it('rejects a payment_date outside the run period month (kontantprincipen)', async () => { + const { supabase, enqueue } = createQueuedMockSupabase() + enqueue({ data: RUN_ROW }) // draft gate passes; period is 2026-03 + + await expect( + updateSalaryRun.execute( + { salary_run_id: 'run-1', payment_date: '2026-04-05' }, + 'company-1', 'user-1', supabase as never, { type: 'user' }, + ), + ).rejects.toThrow(/SALARY_RUN_PAYMENT_DATE_OUTSIDE_PERIOD/) + }) + + it('throws SALARY_RUN_NOT_FOUND for a foreign or unknown run', async () => { + const { supabase, enqueue } = createQueuedMockSupabase() + // company_id-scoped lookup misses: another tenant's run id looks identical + // to a nonexistent one. + enqueue({ data: null }) + + await expect( + updateSalaryRun.execute( + { salary_run_id: 'run-foreign', payment_date: '2026-03-23' }, + 'company-1', 'user-1', supabase as never, { type: 'user' }, + ), + ).rejects.toThrow(/SALARY_RUN_NOT_FOUND/) + }) + + it('rejects a malformed payment_date before touching the run', async () => { + const { supabase } = createQueuedMockSupabase() + + await expect( + updateSalaryRun.execute( + { salary_run_id: 'run-1', payment_date: '23/03/2026' }, + 'company-1', 'user-1', supabase as never, { type: 'user' }, + ), + ).rejects.toThrow(/VALIDATION_ERROR/) + }) + + it('rejects a call with no updatable field', async () => { + const { supabase } = createQueuedMockSupabase() + + await expect( + updateSalaryRun.execute( + { salary_run_id: 'run-1' }, + 'company-1', 'user-1', supabase as never, { type: 'user' }, + ), + ).rejects.toThrow(/At least one of payment_date, voucher_series, notes/) + }) +}) + describe('gnubok_register_absence', () => { it('stages with day-count preview and dateForPeriodCheck', async () => { const { supabase, enqueue } = createQueuedMockSupabase() diff --git a/extensions/general/mcp-server/server.ts b/extensions/general/mcp-server/server.ts index 28351a5d..c1734eb9 100644 --- a/extensions/general/mcp-server/server.ts +++ b/extensions/general/mcp-server/server.ts @@ -14745,6 +14745,101 @@ export const tools: McpTool[] = [ ) }, }, + { + name: 'gnubok_update_salary_run', + title: 'Update Salary Run', + description: 'Stage an update of a DRAFT salary run\'s payment_date, voucher_series or notes; frozen past draft (payment_date becomes the booking entry date). Commit via gnubok_approve_pending_operation; recalculate a calculated run after a payment_date change.', + // Search-only: tools/list budget is at zero headroom (see the ledger in + // payload-size.bench.test.ts) and the payroll_month loadout plus + // gnubok_get_salary_run's status field route agents here when needed. + catalogVisibility: 'search', + inputSchema: { + type: 'object', + additionalProperties: false, + properties: { + salary_run_id: { type: 'string', description: 'UUID of the salary run (must be draft)' }, + payment_date: { type: 'string', description: 'New payment date (YYYY-MM-DD); the date the salary verifikat will be booked on. Must stay within the run\'s period month (AGI is declared per payment month); supplying it clears the run\'s calculation.' }, + voucher_series: { type: 'string', description: 'Voucher series letter (single A-Z)' }, + notes: { type: ['string', 'null'], description: 'Free-text note on the run (max 2000 chars); null clears it' }, + }, + required: ['salary_run_id'], + }, + outputSchema: STAGED_OPERATION_SCHEMA, + annotations: { readOnlyHint: false, destructiveHint: false, idempotentHint: false, openWorldHint: false }, + async execute(args, companyId, userId, supabase, actor) { + const { salary_run_id, payment_date, voucher_series, notes } = args as { + salary_run_id: string; payment_date?: string; voucher_series?: string; notes?: string | null + } + if (!salary_run_id) { + throw new Error('salary_run_id is required') + } + const patch: Record = {} + if (payment_date !== undefined) patch.payment_date = payment_date + if (voucher_series !== undefined) patch.voucher_series = voucher_series + if (notes !== undefined) patch.notes = notes + if (Object.keys(patch).length === 0) { + throw new Error('At least one of payment_date, voucher_series, notes is required') + } + + // Preflight via the shared service in dry-run: verifies draft status + // (exactly as the v1 PATCH does) and yields old/new values for the + // preview. No writes here: the commit path re-runs the service for real. + const { updateDraftSalaryRun } = await import('@/lib/salary/update-run') + const preflight = await updateDraftSalaryRun(supabase, { + companyId, + salaryRunId: salary_run_id, + patch: patch as never, + dryRun: true, + }) + if (!preflight.ok) { + throw new Error(`Cannot update salary run: ${preflight.code}`) + } + const d = preflight.data + + const changeParts: string[] = [] + if (d.changes.payment_date !== undefined) { + changeParts.push(`utbetalningsdag ${d.previous.payment_date} → ${d.payment_date}`) + } + if (d.changes.voucher_series !== undefined) { + changeParts.push(`serie ${d.previous.voucher_series} → ${d.voucher_series}`) + } + if (d.changes.notes !== undefined) { + changeParts.push('anteckning uppdaterad') + } + const periodLabel = `${d.period_year}-${String(d.period_month).padStart(2, '0')}` + + return stagePendingOperation( + supabase, companyId, userId, 'update_salary_run', + `Uppdatera lönekörning ${periodLabel}: ${changeParts.join(', ')}`, + { salary_run_id, patch }, + { + salary_run_id, + period_year: d.period_year, + period_month: d.period_month, + previous: d.previous, + new_payment_date: d.payment_date, + new_voucher_series: d.voucher_series, + new_notes: d.notes, + changes: d.changes, + // A payment_date change clears every roster row's + // calculation_breakdown at commit (skatteavdrag and the AGI + // redovisningsperiod follow the payment month), so booking is + // refused until a recalculation has run against the new date. + invalidates_calculation: d.changes.payment_date !== undefined, + }, + actor, + d.changes.payment_date !== undefined + ? { + description: 'Approval clears the run\'s calculation (tax follows the payment month): re-run gnubok_calculate_salary_run before booking.', + tool: 'gnubok_calculate_salary_run', + } + : undefined, + // Period gate on the date the verifikat would land on: the new + // payment_date when it changes, else the current one. + { dateForPeriodCheck: d.payment_date }, + ) + }, + }, { name: 'gnubok_register_absence', keywords: ['frånvaro', 'sjukanmälan', 'semester', 'vab'], diff --git a/lib/auth/scope-catalog.ts b/lib/auth/scope-catalog.ts index 2340ea12..8c1910ce 100644 --- a/lib/auth/scope-catalog.ts +++ b/lib/auth/scope-catalog.ts @@ -324,6 +324,7 @@ export const TOOL_SCOPE_MAP: Record = { gnubok_list_absence: 'payroll:read', gnubok_update_payslip_line: 'payroll:write', gnubok_set_run_salary: 'payroll:write', + gnubok_update_salary_run: 'payroll:write', gnubok_register_absence: 'payroll:write', gnubok_delete_absence: 'payroll:write', gnubok_create_employee: 'payroll:write', diff --git a/lib/errors/structured-errors.ts b/lib/errors/structured-errors.ts index 8cdd2664..92e65a9f 100644 --- a/lib/errors/structured-errors.ts +++ b/lib/errors/structured-errors.ts @@ -2901,6 +2901,16 @@ const SALARY: Record = { message_sv: 'Endast utkast (draft) kan uppdateras.', message_en: 'Only draft salary runs can be patched.', }, + // Kontantprincipen guard: AGI derives its redovisningsperiod from the run's + // period_year/period_month while the verifikat books on payment_date, so a + // payment date outside the period month would declare the salary in the + // wrong period (SFL 26 kap). For a payment that truly lands in another + // month, the run itself belongs in that period. + SALARY_RUN_PAYMENT_DATE_OUTSIDE_PERIOD: { + httpStatus: 400, + message_sv: 'Utbetalningsdagen måste ligga i lönekörningens period: AGI redovisas per utbetalningsmånad. Skapa en lönekörning för rätt period i stället.', + message_en: 'The payment date must fall within the salary run\'s period month: the AGI is declared per payment month. Create a salary run for the correct period instead.', + }, SALARY_RUN_DELETE_NOT_DRAFT: { httpStatus: 400, message_sv: 'Endast utkast (draft) kan raderas.', diff --git a/lib/pending-operations/__tests__/payroll-executors.test.ts b/lib/pending-operations/__tests__/payroll-executors.test.ts index 524d6814..5f68f83d 100644 --- a/lib/pending-operations/__tests__/payroll-executors.test.ts +++ b/lib/pending-operations/__tests__/payroll-executors.test.ts @@ -195,6 +195,175 @@ describe('commitPendingOperation: set_run_salary', () => { }) }) +describe('commitPendingOperation: update_salary_run', () => { + const RUN_ROW = { + id: 'run-1', + status: 'draft', + period_year: 2026, + period_month: 3, + payment_date: '2026-03-25', + voucher_series: 'L', + notes: null, + } + + it('applies the header patch through the shared service (happy path)', async () => { + const { supabase, enqueue } = createQueuedMockSupabase() + enqueue({ data: { id: 'op-1' }, error: null }) // CAS claim + enqueue({ data: RUN_ROW }) // service draft gate + current values + enqueue({ data: { ...RUN_ROW, payment_date: '2026-03-23' } }) // optimistic-locked update + enqueue({ data: null, error: null }) // roster calculation_breakdown clear + enqueue({ data: null, error: null }) // finalize + + const op = makePendingOp({ + operation_type: 'update_salary_run', + params: { salary_run_id: 'run-1', patch: { payment_date: '2026-03-23' } }, + }) + const result = await commitPendingOperation(supabase as never, 'user-1', 'company-1', op) + + expect(result.status).toBe('committed') + expect(result.data).toMatchObject({ + salary_run_id: 'run-1', + payment_date: '2026-03-23', + changes: { payment_date: '2026-03-23' }, + }) + }) + + it('re-clears breakdowns when retried with the already-applied date (partial-failure retry)', async () => { + // After a partial failure (header committed, clear failed) a retry sees + // the new date as current. The clear must run anyway: it is gated on the + // date being SUPPLIED, never on it differing from the stored value. + const { supabase, enqueue, findCall } = createQueuedMockSupabase() + enqueue({ data: { id: 'op-1' }, error: null }) // CAS claim + enqueue({ data: { ...RUN_ROW, payment_date: '2026-03-23' } }) // run already carries the new date + enqueue({ data: { ...RUN_ROW, payment_date: '2026-03-23' } }) // header update no-ops successfully + enqueue({ data: null, error: null }) // roster calculation_breakdown clear + enqueue({ data: null, error: null }) // finalize + + const op = makePendingOp({ + operation_type: 'update_salary_run', + params: { salary_run_id: 'run-1', patch: { payment_date: '2026-03-23' } }, + }) + const result = await commitPendingOperation(supabase as never, 'user-1', 'company-1', op) + + expect(result.status).toBe('committed') + expect(findCall('salary_run_employees', 'update')).toEqual([{ calculation_breakdown: null }]) + }) + + it('rejects a payment_date outside the run period month with 400', async () => { + const { supabase, enqueue } = createQueuedMockSupabase() + enqueue({ data: { id: 'op-1' }, error: null }) // CAS claim + enqueue({ data: RUN_ROW }) // draft gate passes; period is 2026-03 + enqueue({ data: null, error: null }) // finalize (rejected) + + const op = makePendingOp({ + operation_type: 'update_salary_run', + params: { salary_run_id: 'run-1', patch: { payment_date: '2026-04-05' } }, + }) + const result = await commitPendingOperation(supabase as never, 'user-1', 'company-1', op) + + expect(result.status).not.toBe('committed') + expect(result.http_status).toBe(400) + }) + + it('fails when the calculation-invalidation clear errors (guard is not best-effort)', async () => { + const { supabase, enqueue } = createQueuedMockSupabase() + enqueue({ data: { id: 'op-1' }, error: null }) // CAS claim + enqueue({ data: RUN_ROW }) // service draft gate + current values + enqueue({ data: { ...RUN_ROW, payment_date: '2026-03-23' } }) // optimistic-locked update + enqueue({ data: null, error: { message: 'connection reset' } }) // breakdown clear fails + enqueue({ data: null, error: null }) // finalize (failed) + + const op = makePendingOp({ + operation_type: 'update_salary_run', + params: { salary_run_id: 'run-1', patch: { payment_date: '2026-03-23' } }, + }) + const result = await commitPendingOperation(supabase as never, 'user-1', 'company-1', op) + + expect(result.status).toBe('failed') + }) + + it('fails cleanly when the run advanced between staging and approval', async () => { + const { supabase, enqueue } = createQueuedMockSupabase() + enqueue({ data: { id: 'op-1' }, error: null }) // CAS claim + enqueue({ data: { ...RUN_ROW, status: 'review' } }) // draft gate trips + enqueue({ data: null, error: null }) // finalize (failed) + + const op = makePendingOp({ + operation_type: 'update_salary_run', + params: { salary_run_id: 'run-1', patch: { payment_date: '2026-03-23' } }, + }) + const result = await commitPendingOperation(supabase as never, 'user-1', 'company-1', op) + + expect(result.status).toBe('failed') + expect(result.error).toMatch(/utkast/) + expect(result.http_status).toBe(400) + }) + + it('fails cleanly when the optimistic lock loses the race (update matches no row)', async () => { + const { supabase, enqueue } = createQueuedMockSupabase() + enqueue({ data: { id: 'op-1' }, error: null }) // CAS claim + enqueue({ data: RUN_ROW }) // draft gate passes + enqueue({ data: null }) // status flipped mid-flight: locked update hits nothing + enqueue({ data: null, error: null }) // finalize (failed) + + const op = makePendingOp({ + operation_type: 'update_salary_run', + params: { salary_run_id: 'run-1', patch: { payment_date: '2026-03-23' } }, + }) + const result = await commitPendingOperation(supabase as never, 'user-1', 'company-1', op) + + expect(result.status).toBe('failed') + expect(result.error).toMatch(/utkast/) + }) + + it('auto-rejects with 404 when the run does not exist for this company', async () => { + const { supabase, enqueue } = createQueuedMockSupabase() + enqueue({ data: { id: 'op-1' }, error: null }) // CAS claim + enqueue({ data: null }) // company-scoped lookup misses + enqueue({ data: null, error: null }) // finalize (rejected) + + const op = makePendingOp({ + operation_type: 'update_salary_run', + params: { salary_run_id: 'run-foreign', patch: { payment_date: '2026-03-23' } }, + }) + const result = await commitPendingOperation(supabase as never, 'user-1', 'company-1', op) + + // 404s are auto-rejected by the dispatcher (the run is gone; re-stage). + expect(result.status).toBe('rejected') + expect(result.http_status).toBe(404) + }) + + it('rejects a missing patch with 400', async () => { + const { supabase, enqueue } = createQueuedMockSupabase() + enqueue({ data: { id: 'op-1' }, error: null }) // CAS claim + enqueue({ data: null, error: null }) // finalize (failed) + + const op = makePendingOp({ + operation_type: 'update_salary_run', + params: { salary_run_id: 'run-1' }, + }) + const result = await commitPendingOperation(supabase as never, 'user-1', 'company-1', op) + + expect(result.status).toBe('failed') + expect(result.http_status).toBe(400) + }) + + it('rejects an invalid voucher_series with 400 before any write', async () => { + const { supabase, enqueue } = createQueuedMockSupabase() + enqueue({ data: { id: 'op-1' }, error: null }) // CAS claim + enqueue({ data: null, error: null }) // finalize (failed): validation trips pre-query + + const op = makePendingOp({ + operation_type: 'update_salary_run', + params: { salary_run_id: 'run-1', patch: { voucher_series: 'AB' } }, + }) + const result = await commitPendingOperation(supabase as never, 'user-1', 'company-1', op) + + expect(result.status).toBe('failed') + expect(result.http_status).toBe(400) + }) +}) + describe('commitPendingOperation: register_absence', () => { it('upserts the expanded range through the shared service (happy path)', async () => { const { supabase, enqueue } = createQueuedMockSupabase() diff --git a/lib/pending-operations/commit.ts b/lib/pending-operations/commit.ts index 28167c14..bb61bb18 100644 --- a/lib/pending-operations/commit.ts +++ b/lib/pending-operations/commit.ts @@ -5581,6 +5581,49 @@ async function commitSetRunSalary( } } +async function commitUpdateSalaryRun( + supabase: SupabaseClient, + companyId: string, + params: Record +): Promise { + const salaryRunId = params.salary_run_id as string + const patch = params.patch as Record | null | undefined + if (!salaryRunId || !patch || typeof patch !== 'object' || Array.isArray(patch)) { + return { error: 'salary_run_id and patch are required', status: 400 } + } + + try { + const { updateDraftSalaryRun } = await import('@/lib/salary/update-run') + const { getErrorEntry } = await import('@/lib/errors/structured-errors') + const result = await updateDraftSalaryRun(supabase, { + companyId, + salaryRunId, + patch: patch as never, + }) + if (!result.ok) { + const entry = getErrorEntry(result.code) + return { + error: entry?.message_sv ?? `Kunde inte uppdatera lönekörningen: ${result.code}`, + status: entry?.httpStatus ?? 500, + } + } + return { + data: { + salary_run_id: result.data.salary_run_id, + payment_date: result.data.payment_date, + voucher_series: result.data.voucher_series, + notes: result.data.notes, + changes: result.data.changes, + }, + } + } catch (err) { + return { + error: err instanceof Error ? err.message : 'Failed to update salary run', + status: 500, + } + } +} + async function commitCreateEmployee( supabase: SupabaseClient, userId: string, @@ -6729,6 +6772,9 @@ async function commitPendingOperationInner( case 'set_run_salary': result = await commitSetRunSalary(supabase, companyId, pendingOp.params) break + case 'update_salary_run': + result = await commitUpdateSalaryRun(supabase, companyId, pendingOp.params) + break case 'register_absence': result = await commitRegisterAbsence(supabase, companyId, pendingOp.params) break diff --git a/lib/pending-operations/risk-tiers.ts b/lib/pending-operations/risk-tiers.ts index b293f6c2..b16903af 100644 --- a/lib/pending-operations/risk-tiers.ts +++ b/lib/pending-operations/risk-tiers.ts @@ -162,6 +162,12 @@ export const OPERATION_RISK_TIERS: Record = { // Draft-only edit of one employee's per-run base salary; no booking impact // until the run is calculated and booked (both separately staged). set_run_salary: 'medium', + // Draft-only header edit (payment_date / voucher_series / notes): freely + // re-editable while draft and changes no pay outcome, matching the v1 + // PATCH's risk: 'low'. A payment_date change clears the roster's + // calculation_breakdown so a stale calculation cannot be booked, and the + // booking that makes payment_date matter is separately staged at 'high'. + update_salary_run: 'low', // Absence rows drive sjuklön math and the statutory AGI Frånvarouppgift. // Reversible via delete, but not audit-free: medium. register_absence: 'medium', diff --git a/lib/salary/update-run.ts b/lib/salary/update-run.ts new file mode 100644 index 00000000..19b08508 --- /dev/null +++ b/lib/salary/update-run.ts @@ -0,0 +1,250 @@ +/** + * Shared draft-run header update: payment_date / voucher_series / notes. + * + * Single source of truth for the MCP staging tool (gnubok_update_salary_run) + * and its commit executor. Mirrors the v1 PATCH semantics + * (app/api/v1/companies/[companyId]/salary-runs/[id]): draft-only, with the + * update optimistic-locked on status='draft' so a concurrent :calculate that + * advances the run between pre-flight and write yields a clean + * SALARY_RUN_PATCH_NOT_DRAFT instead of a silently-accepted update. The field + * set is exactly what the v1 PATCH accepts; this module must not grow fields + * the v1 surface does not have. + * + * payment_date is the date the booking entries will carry + * (lib/salary/salary-entries.ts), which is why it freezes past draft. + * + * Result-object convention mirrors lib/salary/run-employees.ts. + */ + +import type { SupabaseClient } from '@supabase/supabase-js' +import { ISO_DATE_RE } from '@/lib/invariants' + +export type UpdateRunResult = + | { ok: true; data: T } + | { ok: false; code: string; details?: Record } + +/** The v1-PATCH-supported draft fields. Nothing else is updatable here. */ +export interface SalaryRunHeaderPatch { + payment_date?: string + voucher_series?: string + notes?: string | null +} + +export interface SalaryRunHeaderValues { + payment_date: string + voucher_series: string + notes: string | null +} + +export interface UpdateSalaryRunData { + salary_run_id: string + period_year: number + period_month: number + status: string + previous: SalaryRunHeaderValues + /** Effective values after the patch (merged for dry-run, read back after a write). */ + payment_date: string + voucher_series: string + notes: string | null + changes: SalaryRunHeaderPatch +} + +const VOUCHER_SERIES_RE = /^[A-Z]$/ +const NOTES_MAX = 2000 + +/** Same field rules as the v1 UpdateSalaryRunSchema (Zod). */ +function validatePatch(patch: SalaryRunHeaderPatch): UpdateRunResult { + const changes: SalaryRunHeaderPatch = {} + if (patch.payment_date !== undefined) { + if (typeof patch.payment_date !== 'string' || !ISO_DATE_RE.test(patch.payment_date)) { + return { + ok: false, + code: 'VALIDATION_ERROR', + details: { field: 'payment_date', message: 'Expected YYYY-MM-DD date format' }, + } + } + changes.payment_date = patch.payment_date + } + if (patch.voucher_series !== undefined) { + if (typeof patch.voucher_series !== 'string' || !VOUCHER_SERIES_RE.test(patch.voucher_series)) { + return { + ok: false, + code: 'VALIDATION_ERROR', + details: { field: 'voucher_series', message: 'Verifikationsserie måste vara en bokstav A-Z' }, + } + } + changes.voucher_series = patch.voucher_series + } + if (patch.notes !== undefined) { + if (patch.notes !== null && (typeof patch.notes !== 'string' || patch.notes.length > NOTES_MAX)) { + return { + ok: false, + code: 'VALIDATION_ERROR', + details: { field: 'notes', message: `Max ${NOTES_MAX} tecken` }, + } + } + changes.notes = patch.notes + } + if (Object.keys(changes).length === 0) { + return { + ok: false, + code: 'VALIDATION_ERROR', + details: { message: 'At least one of payment_date, voucher_series, notes is required' }, + } + } + return { ok: true, data: changes } +} + +export async function updateDraftSalaryRun( + supabase: SupabaseClient, + args: { + companyId: string + salaryRunId: string + patch: SalaryRunHeaderPatch + /** Validate + resolve only; return the would-be merged row without writing. */ + dryRun?: boolean + }, +): Promise> { + const validated = validatePatch(args.patch) + if (!validated.ok) return validated + const changes = validated.data + + const { data: run, error } = await supabase + .from('salary_runs') + .select('id, status, period_year, period_month, payment_date, voucher_series, notes') + .eq('id', args.salaryRunId) + .eq('company_id', args.companyId) + .maybeSingle() + + if (error) { + return { ok: false, code: 'INTERNAL_ERROR', details: { message: error.message } } + } + if (!run) { + return { ok: false, code: 'SALARY_RUN_NOT_FOUND' } + } + + const row = run as { + id: string + status: string + period_year: number + period_month: number + payment_date: string + voucher_series: string + notes: string | null + } + + if (row.status !== 'draft') { + return { + ok: false, + code: 'SALARY_RUN_PATCH_NOT_DRAFT', + details: { current_status: row.status }, + } + } + + // Kontantprincipen guard (SFL 26 kap): the AGI derives its + // redovisningsperiod from period_year/period_month while the verifikat + // books on payment_date. A payment date outside the run's period month + // would post the entries in one month and declare them in another, so it + // is refused; a payment truly landing in another month belongs to a run + // for that period. Grandfather clause: creation does not (yet) enforce + // this coupling, so a run whose CURRENT payment date already sits outside + // the period month may still be day-adjusted within that same month + // (otherwise a legal create state would be uncorrectable). No move can + // introduce a NEW wrong month. + if (changes.payment_date !== undefined) { + const periodPrefix = `${row.period_year}-${String(row.period_month).padStart(2, '0')}` + const newMonth = changes.payment_date.slice(0, 7) + const currentMonth = row.payment_date.slice(0, 7) + if (newMonth !== periodPrefix && newMonth !== currentMonth) { + return { + ok: false, + code: 'SALARY_RUN_PAYMENT_DATE_OUTSIDE_PERIOD', + details: { period: periodPrefix, payment_date: changes.payment_date }, + } + } + } + + const previous: SalaryRunHeaderValues = { + payment_date: row.payment_date, + voucher_series: row.voucher_series, + notes: row.notes, + } + + if (args.dryRun) { + return { + ok: true, + data: { + salary_run_id: row.id, + period_year: row.period_year, + period_month: row.period_month, + status: row.status, + previous, + payment_date: changes.payment_date ?? previous.payment_date, + voucher_series: changes.voucher_series ?? previous.voucher_series, + notes: changes.notes !== undefined ? changes.notes : previous.notes, + changes, + }, + } + } + + // Optimistic lock on status='draft': a concurrent :calculate flipping the + // run to review between the read above and this write must fail the write, + // not let a frozen field slip through (same guard as the v1 PATCH). + const { data: updated, error: updError } = await supabase + .from('salary_runs') + .update(changes) + .eq('id', args.salaryRunId) + .eq('company_id', args.companyId) + .eq('status', 'draft') + .select('id, status, period_year, period_month, payment_date, voucher_series, notes') + .maybeSingle() + + if (updError) { + return { ok: false, code: 'INTERNAL_ERROR', details: { message: updError.message } } + } + if (!updated) { + return { + ok: false, + code: 'SALARY_RUN_PATCH_NOT_DRAFT', + details: { reason: 'race' }, + } + } + + // A supplied payment_date invalidates any existing calculation: + // skatteavdrag follows the payment date, so clearing calculation_breakdown + // makes both book preflights refuse the roster until a recalculation has + // run against the new date (same invariant as setRunEmployeeSalary in + // lib/salary/run-employees.ts). Unlike the display-line refresh there, + // this clear IS the compliance guard, so a failure surfaces as an error. + // Gated on SUPPLIED, not on changed: after a partial failure (header + // committed, clear failed) a retry re-reads the run and sees the new date + // as current, so a changed-only gate would skip the clear forever and + // leave a stale calculation bookable. Clearing on an equal date merely + // forces a redundant recalculation, which is the safe direction. + if (changes.payment_date !== undefined) { + const { error: clearError } = await supabase + .from('salary_run_employees') + .update({ calculation_breakdown: null }) + .eq('salary_run_id', args.salaryRunId) + .eq('company_id', args.companyId) + if (clearError) { + return { ok: false, code: 'INTERNAL_ERROR', details: { message: clearError.message } } + } + } + + const after = updated as typeof row + return { + ok: true, + data: { + salary_run_id: after.id, + period_year: after.period_year, + period_month: after.period_month, + status: after.status, + previous, + payment_date: after.payment_date, + voucher_series: after.voucher_series, + notes: after.notes, + changes, + }, + } +} diff --git a/messages/en.json b/messages/en.json index 241a5516..9b6ffe87 100644 --- a/messages/en.json +++ b/messages/en.json @@ -7185,6 +7185,8 @@ "title": "Payroll run {period}", "back_to_salary": "Back to payroll", "payment_date_label": "Payment", + "payment_date_edit_aria": "Change payment date", + "toast_payment_date_updated": "Payment date updated", "header_employees": "{count, plural, one {# employee} other {# employees}}", "not_found": "Payroll run not found", "more_actions": "More actions", diff --git a/messages/sv.json b/messages/sv.json index cde0251f..656fda43 100644 --- a/messages/sv.json +++ b/messages/sv.json @@ -7185,6 +7185,8 @@ "title": "Lönekörning {period}", "back_to_salary": "Tillbaka till löner", "payment_date_label": "Utbetalning", + "payment_date_edit_aria": "Ändra utbetalningsdag", + "toast_payment_date_updated": "Utbetalningsdag uppdaterad", "header_employees": "{count, plural, one {# anställd} other {# anställda}}", "not_found": "Lönekörning hittades inte", "more_actions": "Fler åtgärder", diff --git a/skills/accounted-api/references/salary-runs.md b/skills/accounted-api/references/salary-runs.md index 77fc614b..5d95731c 100644 --- a/skills/accounted-api/references/salary-runs.md +++ b/skills/accounted-api/references/salary-runs.md @@ -262,6 +262,8 @@ Updates payment_date, voucher_series, or notes on a draft salary run. ONLY allow **Pitfalls:** - Returns 400 SALARY_RUN_PATCH_NOT_DRAFT if status !== "draft". - period_year + period_month are immutable post-create. +- payment_date must stay within the run's period month (400 SALARY_RUN_PAYMENT_DATE_OUTSIDE_PERIOD otherwise): the AGI is declared per payment month. A run whose current payment date already sits outside the period month may still be day-adjusted within that same month. +- Supplying payment_date clears every roster row's calculation_breakdown, so an already-calculated run must be recalculated before :approve/:book. | Parameter | In | Type | Required | Notes | |---|---|---|---|---| diff --git a/supabase/migrations/20260830160000_pending_operations_add_update_salary_run.sql b/supabase/migrations/20260830160000_pending_operations_add_update_salary_run.sql new file mode 100644 index 00000000..cd08b600 --- /dev/null +++ b/supabase/migrations/20260830160000_pending_operations_add_update_salary_run.sql @@ -0,0 +1,104 @@ +-- Add 'update_salary_run' to the pending_operations operation_type CHECK +-- constraint. +-- +-- gnubok_update_salary_run (MCP) stages a draft salary run's header edit +-- (payment_date / voucher_series / notes; the exact field set the v1 PATCH +-- accepts, draft runs only). The user approves it in Granskning and +-- commitUpdateSalaryRun in lib/pending-operations/commit.ts applies it via +-- lib/salary/update-run.ts, which optimistic-locks on status='draft'. +-- payment_date is the date the booking entries will carry +-- (lib/salary/salary-entries.ts), which is why it freezes past draft. +-- Risk 'low': freely re-editable while draft and changes no pay outcome; +-- the booking that makes payment_date matter is separately staged at 'high'. +-- +-- NOTE on the value list: this constraint is re-created wholesale (the +-- established pattern here), so the list below is every value of the +-- constraint as left by 20260830150000 (delete_draft_invoice, which built on +-- 20260830130000's book_skattekonto_row / book_skattekonto_rows) PLUS the +-- new value. Dropping any existing value here would silently revoke it. +-- +-- NOT VALID + separate VALIDATE migration (paired file, same pattern as +-- 20260830150000 / 20260830150001). +-- +-- pg-test: tests/pg/pending-operations-op-type-audit.pg.test.ts asserts every +-- op type staged in server.ts or tiered in risk-tiers.ts is accepted here. +ALTER TABLE public.pending_operations + DROP CONSTRAINT IF EXISTS pending_operations_operation_type_check; + +ALTER TABLE public.pending_operations + ADD CONSTRAINT pending_operations_operation_type_check + CHECK (operation_type IN ( + 'categorize_transaction', + 'create_customer', + 'create_invoice', + 'mark_invoice_paid', + 'send_invoice', + 'mark_invoice_sent', + 'match_transaction_invoice', + 'close_period', + 'lock_period', + 'unlock_period', + 'set_opening_balances', + 'run_year_end', + 'post_kontantmetod_cutoff', + 'run_currency_revaluation', + 'import_sie', + 'explain_voucher_gap', + 'uncategorize_transaction', + 'approve_supplier_invoice', + 'credit_supplier_invoice', + 'credit_invoice', + 'convert_invoice', + 'delete_draft_invoice', + 'create_transaction', + 'attach_document_to_transaction', + 'create_voucher', + 'correct_entry', + 'reverse_entry', + 'create_supplier', + 'create_supplier_invoice_from_inbox', + 'post_annual_depreciation', + 'link_invoice_voucher', + 'undo_sie_import', + 'match_batch_allocate', + 'bulk_book_transactions', + 'create_salary_run', + 'generate_agi', + 'link_transaction_journal_entry', + 'link_supplier_invoice_voucher', + 'submit_vat_declaration', + 'submit_agi', + 'create_article', + 'update_article', + 'bulk_book_inbox_items', + 'create_dimension_value', + 'retag_line_dimensions', + 'link_document_to_voucher', + 'update_payslip_line', + 'set_run_salary', + 'update_salary_run', + 'register_absence', + 'create_employee', + 'update_employee', + 'set_employee_opening_balances', + 'vacation_year_close', + 'create_account', + 'update_account', + 'set_voucher_note', + 'book_salary_run', + 'delete_absence', + 'update_company_settings', + 'update_customer', + 'update_invoice', + 'create_recurring_schedule', + 'update_recurring_schedule', + 'log_mileage_trip', + 'book_mileage_period', + 'link_documents_to_vouchers', + 'reconciliation_match', + 'reconciliation_unmatch', + 'reconciliation_signoff', + 'reconciliation_residual', + 'book_skattekonto_row', + 'book_skattekonto_rows' + )) NOT VALID; diff --git a/supabase/migrations/20260830160001_validate_pending_operations_update_salary_run.sql b/supabase/migrations/20260830160001_validate_pending_operations_update_salary_run.sql new file mode 100644 index 00000000..9804a804 --- /dev/null +++ b/supabase/migrations/20260830160001_validate_pending_operations_update_salary_run.sql @@ -0,0 +1,6 @@ +-- Validate the operation type CHECK re-added in 20260830160000. +-- This separate transaction avoids a full-table scan while the preceding +-- migration holds its stronger table lock. + +ALTER TABLE public.pending_operations + VALIDATE CONSTRAINT pending_operations_operation_type_check; diff --git a/tests/schema/no-phantom-columns.test.ts b/tests/schema/no-phantom-columns.test.ts index 7a0c148d..a8ce9e69 100644 --- a/tests/schema/no-phantom-columns.test.ts +++ b/tests/schema/no-phantom-columns.test.ts @@ -124,8 +124,14 @@ const KNOWN_STALE_ON_CONFLICT: Record = {} * 39 spread-payload, 5 dynamic-column, 2 computed key); ceiling re-baselined * with the usual headroom. The later catch-up merge of #1954 (byte-exact SIE * upload) brought the merged count to 386. + * + * 2026-08-30 +1: lib/salary/update-run.ts applies the draft salary-run header + * patch (payment_date / voucher_series / notes) as a partial UPDATE payload, + * same patch-shape rationale as webshop-orders ingest: one literal per key + * combination is not viable. The field set is pinned by validatePatch and + * covered by payroll-executors.test.ts; both selects around it are literals. */ -const UNRESOLVED_CEILING = 388 +const UNRESOLVED_CEILING = 389 /** * Floor on statically resolved column references. Guards the guard: if a change diff --git a/types/index.ts b/types/index.ts index 6bdcd025..c99c4f6c 100644 --- a/types/index.ts +++ b/types/index.ts @@ -2594,6 +2594,10 @@ export type PendingOperationType = // monthly_salary, draft only). The per-run column is what the engine reads; // the employee master's fixed salary stays untouched (variable owner pay). | 'set_run_salary' + // Draft-only salary-run header edit (payment_date / voucher_series / + // notes), same field set as the v1 PATCH; payment_date is the future + // booking entry date. + | 'update_salary_run' | 'register_absence' | 'create_employee' | 'update_employee'