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 <noreply@anthropic.com>
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 <noreply@anthropic.com>
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 <noreply@anthropic.com>
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 <noreply@anthropic.com>
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 <noreply@anthropic.com>
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 <noreply@anthropic.com>
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 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_018zGah8Yy49esAwpnKGxiGy

---------

Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
Mattsson
2026-08-30 21:43:23 +02:00
committed by GitHub
co-authored by Claude Fable 5
parent e92b5a59b2
commit f43a6653f1
21 changed files with 1293 additions and 14 deletions
@@ -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
+79 -2
View File
@@ -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<string, unknown> = {}
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 })
},
@@ -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 })
},
)
@@ -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 () => {