* feat(api): Phase 5 PR-1 — payroll registers (employees + salary-runs CRUD)
10 endpoints under /api/v1, 35 integration tests. Mirrors Phase 4 PR-1 size
and review profile. No engine interaction; no period-lock checks. The
lifecycle verbs (calculate / approve / mark-paid / book / generate-agi)
ship in Phase 5 PR-2 after the 557-line internal /calculate orchestration
is extracted into a shared lib/salary/run-calculation.ts helper.
Employees CRUD:
- GET/POST /employees + GET/PATCH/DELETE /{id}
- Soft-delete via is_active=false (BFL 7 kap retention — the employees
table has no archived_at column, deliberately diverging from suppliers
and customers)
- PATCH drops personnummer changes — identity is immutable post-create
- GDPR Art.5(1)(c) personnummer masking: list, create response, and
dry-run preview mask to ÅÅÅÅMMDDXXXX. Detail endpoint (deliberate
drill-in) returns the full value. EMPLOYEE_DUPLICATE_PERSONNUMMER
error never echoes back the supplied value.
- Mask helper extracted to lib/api/v1/mask-personnummer.ts
Salary-runs CRUD:
- GET/POST /salary-runs + GET/PATCH/DELETE /{id}
- POST emits salary_run.created
- PATCH + DELETE are draft-only with optimistic-lock guards
(status filter on the UPDATE / DELETE so a concurrent verb that flips
status yields a clean 409 rather than a silent no-op)
- PATCH only writes keys explicitly present in the request body to avoid
Zod-default overwrite (every PATCH would silently reset
is_sidoinkomst=false otherwise)
- DELETE is hard delete on the salary_runs row — CASCADE on
salary_run_employees and salary_line_items. Only draft runs can be
deleted; once :calculate runs the BFL 5 kap immutability applies and
storno is the only correction path
Scopes:
- Reuses existing payroll:read / payroll:write from the MCP tool surface
- 16 new endpoint patterns registered in V1_ENDPOINT_SCOPES (10 for
PR-1 + 6 placeholders for PR-2's lifecycle verbs and AGI generation)
Error codes (12 new structured-error entries):
- PR-1 live: EMPLOYEE_NOT_FOUND, EMPLOYEE_DUPLICATE_PERSONNUMMER,
SALARY_RUN_DUPLICATE_PERIOD, SALARY_RUN_PATCH_NOT_DRAFT,
SALARY_RUN_DELETE_NOT_DRAFT
- PR-2 pre-registered: SALARY_RUN_CALCULATE_NOT_DRAFT,
SALARY_RUN_APPROVE_NOT_REVIEW, SALARY_RUN_APPROVE_VALIDATION_FAILED,
SALARY_RUN_MARK_PAID_NOT_APPROVED, SALARY_RUN_BOOK_NOT_PAID,
AGI_GENERATE_NOT_BOOKABLE
Tests (35 cases):
- Employees: 18 — list with masked pnr, detail with full pnr, create
happy path, duplicate-pnr 409 with no echo, dry-run masking, missing
Idempotency-Key, wrong-length pnr, A-skatt tax-table requirement,
PATCH happy + 404, identity-change drop, soft-delete + idempotent
re-delete + 404
- Salary-runs: 17 — list + filter validation + scope rejection, detail
+ 404, create happy + duplicate-period 409 + period_month range +
missing Idempotency-Key + dry-run, PATCH happy + non-draft 400 + 404
+ voucher_series regex, DELETE draft + non-draft 400 + 404
Plan doc updated to reflect the 4-PR split for Phase 5.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* refactor(api): address PR-479 review — disambiguate 23505, mask PATCH responses, return 400 on personnummer-in-PATCH
Triage of PR-479 review bots:
- **Greptile P1 (`ensureInitialized()` missing on salary-runs/route.ts)** —
FALSE POSITIVE. The v1 wrapper at `lib/api/v1/with-api-v1.ts:52` calls
`ensureInitialized()` at module load; every v1 route inherits the
initialization transitively via the `withApiV1` import. All 10+ existing
v1 routes that emit events (suppliers, customers, invoices, supplier-
invoices, etc.) follow the same pattern. The wrapper file's own comment
documents the centralization. No fix needed; Greptile is applying the
CLAUDE.md rule literally without checking the wrapper.
- **Greptile P2 (23505 constraint disambiguation)** — FIXED.
Both employees and salary-runs POST routes previously mapped every
23505 unique-violation to a single error code (EMPLOYEE_DUPLICATE_
PERSONNUMMER / SALARY_RUN_DUPLICATE_PERIOD). A future migration adding
another unique index (e.g. employees(company_id, email)) would have
produced misleading errors. Now check `error.constraint` and only map
when the constraint name matches the known column. Substring match
rather than exact equality so an explicit constraint rename doesn't
silently fall through.
Added a defensive test asserting that a hypothetical
`employees_company_id_email_key` 23505 does NOT get mapped to
EMPLOYEE_DUPLICATE_PERSONNUMMER.
- **GDPR Art.5(1)(c) — PATCH response + dry-run preview masking** — FIXED.
Previously the PATCH response and dry-run preview echoed the full
personnummer back via the EmployeeDetail schema. Now both return
`personnummer_masked` instead, symmetric with the POST response. Added
`EmployeeWriteResponse` schema (EmployeeDetail.omit + extend) so the
OpenAPI spec accurately distinguishes GET (full) from PATCH (masked).
Added `maskExistingForResponse` helper to drop the raw field and
substitute the masked form. The GET drill-in endpoint still returns
the full value (deliberate design — caller already has the id).
- **SOC 2 PI1.3 — silent personnummer drop on PATCH** — FIXED.
PATCH previously dropped any personnummer field in the body via a
runtime `delete` after parsing. Caller saw no signal that the
intent was rejected. Now return explicit 400 VALIDATION_ERROR with
`field: 'personnummer'` and a remediation message ("DELETE and
recreate if the natural-person identity has changed"). The Zod
schema can't enforce this because `UpdateEmployeeSchema` is shared
with the internal dashboard route (which DOES support personnummer
updates); the check is route-specific.
- **ISO A.5.34 — real-format personnummer in docs/tests** — FIXED.
Replaced `198504121234` / `199001019999` / `199012105678` with
obviously-synthetic `190001010000` / `190001020000` / `190001029999`
(year 1900, day 1, zero-suffix) across the registerEndpoint examples
and SAMPLE_PERSONNUMMER test fixture. Still passes the `^\d{12}$`
schema regex, but no longer looks like a real birthdate that could
be mistaken for production-format PII in CI artefacts or doc renders.
Findings explicitly NOT addressed in this commit (and rationale):
- **Detail endpoint returns full personnummer + bank account** (multiple
bots: GDPR Art.5(1)(c), ISO A.8.11, SOC 2 CC6.1). INTENTIONAL design.
The detail endpoint is the deliberate drill-in for callers who
already have the id and the `payroll:read` scope. Matches the
dashboard's internal /api/salary/employees/[id] behavior. Splitting
into a separate `payroll:admin` scope is a CC6.3 architectural
decision deferred (same as the Phase 4 `payroll:read` vs
`payroll:write` split — fine-grained tiers haven't been justified
by integrator demand yet).
- **calculation_params shape (Art.5(1)(b) / CC2.1)** — DEFERRED to
Phase 5 PR-2. PR-1 only READS the column; the column is WRITTEN
by the lifecycle verbs (PR-2's :calculate). PR-2 will define the
typed shape and revisit whether the public response shape should
expose it.
- **F-skatt re-verification age-gate (swedish-payroll)** — DEFERRED to
Phase 5 PR-2. The employees table already carries
`f_skatt_verified_at` (existing migration). PR-2's :calculate is
the correct enforcement point.
- **Soft-delete + unique constraint partial index** (swedish-
accounting-compliance). VALID concern for genuine rehires. Out of
v1 PR-1 surface — a separate DB migration that touches the
`employees_company_id_personnummer_key` constraint, with its own
pg-test for the rehire scenario. Tracked.
- **semestertillagg_rate vs vacation_rule consistency** (swedish-
payroll). Engine-layer concern. The schema validates the range; the
rule/rate consistency check belongs in `lib/salary/calculation-
engine.ts` next to the actual accrual math. Tracked for the engine
audit alongside Phase 5 PR-2.
- **voucher_series default 'A' vs convention 'N'** (swedish-payroll).
Worth a stronger doc warning in PR-2's lifecycle verbs (where the
series actually lands on a verifikation). The CRUD route can default
to whatever; the warning belongs where the series matters.
- **personnummer_last4 column** (Art.25). Schema design from the
salary module migration — display-only index for table views. Out
of v1 scope.
- **Bank account at-rest encryption (CC6.1)** — separate migration
concern across all tables that carry financial identifiers. Out of
v1 scope.
Test count: 37 (up from 35). All type-checks clean. Full v1 suite green
(232 tests).
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* refactor(api): address PR-479 review round 2 — proto-pollution defense + salary-run JE-orphan guard
Triage of bot re-run on c0d168be:
- **Compliance Swarm V4.5 (prototype pollution in PATCH rawKeys)** — FIXED.
`Object.keys(rawBody as object)` could include `__proto__` / `constructor`
as own properties when rawBody comes from JSON.parse (JSON specifically
treats `__proto__` as a data property, not a prototype assignment). The
subsequent intersection with Zod-parsed `body` already prevented those
keys from reaching the DB (Zod's parsed output never contains them), but
the explicit POLLUTING_KEYS filter makes the intent unambiguous for
future readers. Defense in depth.
- **Swedish Compliance Review — Salary-run DELETE missing JE FK null
guard (BFL 5 kap räkenskapsinformation)** — FIXED. The DELETE chain
previously gated only on `status='draft'`. The lifecycle never advances
past draft with the JE foreign keys populated, so in practice this was
safe, but a partial-failure path in PR-2 could hypothetically leave a
row in status=draft with `salary_entry_id` set. The .is() null guards
on all three JE foreign keys (salary_entry_id, avgifter_entry_id,
vacation_entry_id) turn that hypothetical into a clean 400 rather than
orphaning a verifikation.
Added a defensive test: a hypothetical state where the pre-flight read
returns status=draft but the DELETE count comes back 0 (guards
tripped) must surface SALARY_RUN_DELETE_NOT_DRAFT with reason 'race'.
Findings on this round explicitly NOT addressed:
- **V16.1.1 + Art.5(1)(f) on app/api/bookkeeping/journal-entries/[id]/
commit/route.ts** — NOT MY FILES. Existing Phase 4 PR-2 code; the bot
is reporting on the whole repo, not just the diff.
- **V2.2 PostgREST .or() injection (recurring)** — Known false positive.
Same escaping pattern as suppliers + customers since Phase 2. The
documented architectural floor per the plan doc.
- **Art.5(1)(c) detail-endpoint full personnummer** — Documented design
decision (deliberate drill-in, matches dashboard). Same as the
previous round.
- **Art.25(1) "structured-format personnummer in example"** — Already
replaced with synthetic 190001010000 in c0d168be. Bot is now
suggesting a non-numeric placeholder (e.g. 'YYYYMMDDXXXX'). Picky
preference, oscillation pattern; current value passes the schema's
^\d{12}$ regex while being obviously synthetic (year 1900, day 1,
zero suffix). No change.
- **Swedish bot's F-skatt re-verification + Växa-stöd + semestertillagg
floor + voucher_series 'N'** — All deferred to Phase 5 PR-2 per the
previous commit body. The lifecycle verbs are where these belong.
Test count: 38 (+1 for the JE-orphan guard test). 233 total v1 tests
green. Type-check clean.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* refactor(api): address PR-479 review round 3 — symmetrize PATCH defenses + tighten docs/BFL wording
Compliance Swarm dropped 18 → 15 findings on the round-2 commit; the floor
is narrowing. This commit addresses the remaining actionable items.
- **V2.3 / PI1.3 — salary-run PATCH missing POLLUTING_KEYS filter** — FIXED.
Same defense as employees PATCH (round 2). Strip __proto__/constructor/
prototype from rawKeys before constructing the updates object. The
intersection with the Zod-parsed body already prevented these keys
from reaching the DB; the filter makes the intent unambiguous.
- **V4.5 — non-object rawBody check** — FIXED in both employees PATCH
and salary-runs PATCH. After JSON.parse, require typeof === 'object',
not null, not Array.isArray. Zod would catch a non-object body
downstream, but the rawKeys Object.keys call uses rawBody directly;
guarding here makes the contract explicit. (An array body would pass
`typeof === 'object'` and produce numeric-string keys.)
- **A.5.34 — request-example personnummer too realistic** — FIXED. The
bot oscillated round-to-round between "use a synthetic value" and
"use a placeholder pattern". Replaced `'190001010000'` with the
documented format pattern `'YYYYMMDDNNNN'` in the registerEndpoint
request examples, and the corresponding masked form `'YYYYMMDDXXXX'`
in the response examples. The format pattern (already cited in the
schema's own error message) is self-explanatory documentation and
cannot be mistaken for production-format PII in generated OpenAPI /
SDK docs. Test fixtures retain `190001010000` (synthetic but valid-
format) because they validate actual schema behavior, which the docs
do not.
- **Swedish bot — BFL 7 kap comment slightly overstates the law** —
FIXED. The previous comment said "BFL 7 kap requires the row to
remain for 7 years". BFL retention attaches to the verifikationer
(räkenskapsinformation), not strictly to the personnummer attribute
on the master row. Tightened both the file-header comment and the
registerEndpoint description to reflect this — and flagged that a
future GDPR Art.17 erasure workflow could pseudonymise the row once
all referenced verifikationer are outside the 7-year window. The
practical outcome (soft-delete only via v1) is unchanged.
Findings on this round explicitly NOT addressed:
- **V14.2 / V16.1.1 / Art.5(1)(f) on app/api/bookkeeping/journal-
entries/[id]/commit/route.ts** — NOT MY FILES (Phase 4 PR-2 surface).
- **V16.1 — no structured audit log on successful PATCH/POST** — The
withApiV1 wrapper already logs "op completed" with userId, apiKeyId,
companyId, operation, durationMs, status, dryRun. Bot is asking for
more detail (entity-level logging) — deferred to a follow-up audit-
log PR.
- **Art.5(1)(c) / A.8.11 / CC6.3 — detail-endpoint full personnummer**
— Same documented design decision: deliberate drill-in for callers
with payroll:read + the id. Mirrors the dashboard. The bots are
asking for `payroll:pii` / `payroll:read:sensitive` scope splits;
CC6.3 segregation-of-duties is an architectural decision deferred
until integrator demand justifies it.
- **C1.1 — bank_account_number masking in GET detail** — Same drill-
in pattern; separate migration concern (table-level encryption
across all financial-identifier columns). Out of v1 PR-1 scope.
- **Art.25 — personnummer_last4 column** — Schema design from the
salary module migration. Display-only index. Out of v1 scope.
- **Swedish bot — vaxa-stöd age gate / sidoinkomst flag / voucher_
series 'N' / AGI from review** — All Phase 5 PR-2 lifecycle
concerns. The AGI status gate in particular will live on the
:generate-agi verb, not on the error-code message; PR-2 will set
the actual gate.
- **Swedish bot — GDPR Art.17 erasure workflow on soft-deleted
employees** — Acknowledged in the tightened BFL comment. Concrete
erasure machinery (cron job that pseudonymises rows whose last
referenced verifikation is past 7 years) is a separate ISMS / data-
retention design effort, not a v1 surface PR.
Test count: 38 (unchanged). 233 total v1 tests green. Type-check clean.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Jakob Wennberg
1f89a71962