Skip to main content

50. Decouple the billing lock from payroll's locked status

Date: 2026-07-13

Status

Proposed. Pending human sign-off + security-reviewer. Supersedes the entry-locking behaviour introduced by migration 0094 (send_lock_all_payable). Also supersedes the interim UI relabel in PR #92 (fix/timeapproval-lock-reason), which is folded into this work.

Context

This is fundamentally about the payroll workflow. time_entries.status = 'locked' is read by two independent domains that must not interfere:

  • Payrollclose_and_lock_payroll_batch locks a period's entries when a pay run is finalized. In the Approve Time grid, locked means "this is committed to a pay run, edit it there."
  • Billingsend_invoices (migration 0094) also sets status = 'locked' when a facility invoice is sent, to freeze the invoiced basis.

Overloading one status across two domains produced real, observed breakage:

  1. Payroll UI lied. A contractor whose hours were invoiced to a facility (never in a pay run) showed 🔒 locked in Approve Time with "unlock in Calculate / Process & Pay" — screens where there is nothing to unlock. From payroll's point of view, whether a facility has been invoiced is irrelevant to approving/paying the contractor.

  2. Payroll clobbered billing's lock. discard_payroll_batch and the close/lock path operate on entries by date + assignment, with no awareness of the billing invoice_id. A prepare → discard cycle on a payroll batch silently flipped billing-locked entries back to approved without clearing invoice_id — leaving them in an impossible "billed but editable, still tied to a live invoice" state. Observed on the seed tenant: a single discard-driven write moved 43 entries across 9 contractors out of locked while invoice_id stayed set.

  3. The lock was inconsistent across employees. Two contractors on the same sent invoice ended up in different lock states purely from unrelated payroll churn — confusing and untrustworthy.

Reconciliation of the seed tenant (a0000000-…-0001), by transition bucket:

BucketMeaningEntriesContractors
Apayroll-locked (in a prepared batch) — keep locked1029
Bbilling-locked, no pay run — unlock, keep invoice_id383
Cinvoiced + editable (already the target state)439

Decision

Billing never uses status = 'locked'. locked is reserved exclusively for payroll pay-run locks.

  1. send_invoices stops flipping status. It marks coverage by setting invoice_id on the covered entries and leaves status untouched. Invoiced hours stay approved/submitted/etc. — fully visible and editable in the payroll approval grid.
  2. void_invoice only detaches the invoice — clears invoice_id, never touches status. (It can no longer accidentally unlock a payroll-locked entry that also happens to be invoiced.)
  3. Editing an invoiced entry is allowed, with a warning. The Approve Time grid marks a row whose entries carry invoice_id as "Invoiced on INV-####" and warns, on edit/approve/delete, that changing invoiced hours may require reissuing that invoice. It does not block. The reissue path already exists (void + regenerate).
  4. Drift surfacing is the safety net — and this migration must repair it. Because edits to invoiced hours are no longer prevented, the invoice-drift surfacing from migration 0096 is now load-bearing. But 0096 keys drift on status in (submitted/pending_approval/approved) = "billable but not locked", assuming a sent invoice's own entries are locked. Decoupling deletes that invariant, so every sent invoice would read as permanently drifted. The 0096 invoice_drift predicate must be rewritten to key on invoice_id inside the same 0097 migration (see §(d)); shipping (a)/(c) without it turns the safety net into an always-on alarm. Note 0096 only catches the under-billed direction (hours added after send); the over-billed direction (hours reduced or deleted after send) has no drift backstop and relies entirely on the edit/delete warning + guard below.
  5. status_before_lock becomes vestigial (it existed only to let billing restore status on void). Leave the column in place, marked deprecated; do not drop it in this change.

Payroll semantics are unchanged: PAYABLE_ENTRY_STATUSES still pays submitted, pending_approval, approved, and locked. Invoiced-but-editable entries are approved → already payable; no change to what gets paid.

Migration plan — 0097_decouple_billing_lock

Not yet applied. Requires the companion app-code changes below to ship together.

(a) send_invoices — replace the lock UPDATE (0094:162-173) with a marker-only UPDATE:

-- Mark which entries this invoice covers (for the "invoiced — editing may need reissue" warning and
-- drift surfacing). NO status change: billing must not touch payroll's 'locked' state (ADR-0050).
-- Mark the FULL priced basis — including 'locked' (payroll-locked hours are still invoiced) — and only
-- entries not already attributed to another invoice, so a send can never steal a marker (H2).
update public.time_entries te
set invoice_id = v_id, updated_at = now()
where te.company_id = p_company_id
and te.status in ('approved','submitted','pending_approval','locked')
and te.invoice_id is null
and te.entry_date between v_ps and v_pe
and te.assignment_id in (
select li.assignment_id from public.invoice_line_items li
where li.invoice_id = v_id and li.assignment_id is not null
);
get diagnostics v_covered = row_count; -- audit metadata: rename locked_entries → covered_entries

Keep everything else in the function (the unapproved-hours acknowledge gate, recipients, email, audit).

(b) void_invoice — replace the restore UPDATE (0094:58-62) with detach-only:

-- Billing no longer owns 'status' (ADR-0050): just detach the invoice marker. A payroll-locked entry
-- that was also invoiced stays 'locked'.
update public.time_entries
set invoice_id = null, status_before_lock = null, updated_at = now()
where company_id = p_company_id and invoice_id = p_invoice_id;

(c) One-time data reconciliation — bucket B (unlock billing-only locks, all tenants). Wrap in a transaction that snapshots bucket A/B before and after and aborts if bucket A moves — a billing lock whose date coincides with an unrelated payroll batch window would otherwise stay mis-bucketed (M2):

begin;
-- snapshot A/B counts here (see reconcile-billing-locks.sql), assert A unchanged after the UPDATE.
update public.time_entries te
set status = coalesce(te.status_before_lock, 'approved'),
status_before_lock = null
where te.status = 'locked'
and te.invoice_id is not null
and not exists ( -- exclude real payroll locks
select 1 from public.payroll_batches b
where b.company_id = te.company_id and b.status in ('prepared','funded','closed')
and te.entry_date between b.period_start and b.period_end
);
-- invoice_id intentionally preserved. Bucket A untouched (payroll). Bucket C already correct.
commit;

The guard is one-directional-safe: it can only fail to unlock (never wrongly unlock a payroll lock), and coalesce(status_before_lock,'approved') prevents laundering never-approved hours into approved.

(d) Rewrite the invoice_drift predicate (0096) — REQUIRED, same migration (B1). Re-key drift on attribution instead of lock state, so a sent invoice's own basis is not counted as drift:

-- was: te.status in ('submitted','pending_approval','approved') -- "billable but not locked"
-- now: billable hours in this invoice's period+assignment NOT attributed to this invoice
and te.status in ('submitted','pending_approval','approved','locked')
and te.invoice_id is distinct from i.id

Run scripts/sql/reconcile-billing-locks.sql (read-only) before and after to confirm bucket B → 0 and bucket A count is unchanged.

Follow-up app-code changes (same PR train)

  • loadTimeApproval / TimeApprovalGrid: locked = payroll only. Surface invoice_id-marked rows as an editable "Invoiced (INV-####)" row with the reissue warning. Replaces PR #92's lockReason.
  • manualTimeEntry (:92) currently blocks editing both locked and approved. Relax only the approved arm for invoiced-editable, keep the locked arm intact (M1 — must not accidentally unblock payroll-locked edits), and add the confirm-with-consequence reissue warning.
  • Delete paths are the sole over-bill defense (drift can't catch reductions — H1): deleteUnlockedTime (timeApproval.ts:115) and deleteTimeEntryById (:174) today guard only status != 'locked' with no invoice_id awareness, so they would silently delete invoiced hours. Add an invoice_id guard: block, or confirm-with-consequence + reissue nudge.
  • The invoice_drift predicate rewrite is in migration §(d), not a "confirm" — it must ship in 0097.

L1 resolution — the last two non-payroll locked writers (later PR)

After 0097, two paths still wrote status='locked' for a non-payroll reason — re-introducing the overload the ADR removed from billing, and (post-0097) flipping invoiced-approved rows to locked so the grid mislabels them as pay-run locks:

  1. offboarding.ts — locked a departing contractor's hours through their end date.
  2. lockEndedAssignments cron (services/cron.ts + api/cron/lock-ended) — nightly "auto-lock on end date." This one is the primary/automated writer; offboarding's inline lock was largely redundant with it, and it would re-lock offboarded hours after offboarding stopped.

Resolution: neither persists a freeze anymore. PAYABLE_ENTRY_STATUSES already includes locked, so a lock never gated pay — it only made rows read-only. That freeze is now derived live from assignment state (assignmentIsFrozen in services/assignments.ts: active = false OR end_date < today) and enforced at the owner edit/delete/grid boundaries, so locked is finally payroll-only. Benefits: covers offboardFacility (ends assignments without archiving the contractor) uniformly; reactivating a contractor/facility lifts the freeze with nothing to unlock; settlement (approve / invoice / close pay run for the final week) stays open. The lock-ended cron + route were deleted.

Consequences

  • Good: billing and payroll stop corrupting each other's state; the payroll grid tells the truth; invoiced hours are correctable (with a nudge to reissue) instead of dead-ended.
  • Trade-off: the invoiced basis can drift after send. Mitigated by the reissue warning + 0096 drift surfacing. This is a deliberate accept (operator-in-the-loop over hard lock).
  • Not covered here: the separate discard_payroll_batch narrow-unlock asymmetry — moot for billing once decoupled, but revisit if payroll-only orphans ever appear.