diff --git a/docs/operations.md b/docs/operations.md index 1bc1c8b..dfcc626 100644 --- a/docs/operations.md +++ b/docs/operations.md @@ -138,9 +138,25 @@ Retries are bounded. After `MAX_PERIOD_ATTEMPTS` (5) failures on the *same* period, the contract is **paused** and the operator has to act. Pausing rather than abandoning the period is the point: a payday that cannot be funded is a fact somebody needs to see, and silently dropping it is the one -outcome payroll must never produce. Because pausing does not advance the -position, resuming after topping up the source wallet retries that same -payday. +outcome payroll must never produce. + +### Two kinds of pause, and why resume must tell them apart + +| paused by | `paused_reason` | resume does | +|---|---|---| +| the operator | empty | **writes off** the missed paydays | +| payroll | names the period and cause | **keeps** them; they settle next tick | + +These mean opposite things. An operator pause *is* the decision not to pay +those periods. An automatic pause means payroll could not pay them and +nobody decided anything — the money is still owed, and the operator has just +fixed whatever blocked it. + +Sharing one path between the two destroys money silently: fix the cause, +click resume, and the backlog vanishes while the contract looks healthy. +`resume` therefore infers from `paused_reason`, and `catch_up` overrides it. +The console asks a different question for each and flags a payroll-paused +contract in the table with its reason. ## The payout ledger diff --git a/migrations.py b/migrations.py index b82e7d1..8bf2ba3 100644 --- a/migrations.py +++ b/migrations.py @@ -112,3 +112,17 @@ async def m003_pricing_mode(db): await db.execute( "ALTER TABLE payroll.payouts ADD COLUMN rate_source TEXT NOT NULL DEFAULT '';" ) + + +async def m004_paused_reason(db): + """Tell an automatic pause apart from a deliberate one. + + Without this they share a recovery path, and resuming a contract that + payroll paused because it could not price a backlog silently writes that + backlog off — the operator fixes the cause, clicks resume, and the money + owed quietly disappears while the contract looks healthy again. + """ + await db.execute( + "ALTER TABLE payroll.contracts " + "ADD COLUMN paused_reason TEXT NOT NULL DEFAULT '';" + ) diff --git a/models.py b/models.py index 524224f..7e24d0a 100644 --- a/models.py +++ b/models.py @@ -159,6 +159,12 @@ class Contract(CreateContract): employee_username: str = "" status: ContractStatus = ContractStatus.active + # Why the contract is paused, when payroll paused it rather than the + # operator. Empty for a deliberate pause. The distinction matters on + # resume: a deliberate pause means "do not pay those paydays", while an + # automatic one means "could not pay them yet" — opposite intentions + # that must not share a recovery path. + paused_reason: str = "" # Periods *consumed* — paid or deliberately skipped. A failed payout does # NOT advance this, which is what makes the next tick retry it rather # than silently drop a payday. diff --git a/services.py b/services.py index e3cf8f2..b8c990e 100644 --- a/services.py +++ b/services.py @@ -555,6 +555,10 @@ def _maybe_pause_after_repeated_failure(contract: Contract, payout: Payout) -> N if payout.attempt < MAX_PERIOD_ATTEMPTS: return contract.status = ContractStatus.paused + contract.paused_reason = ( + f"period {payout.period_index} ({payout.payday}) failed " + f"{payout.attempt} times: {payout.detail}" + ) logger.error( f"payroll: contract {contract.id} paused after {payout.attempt} failed " f"attempts at period {payout.period_index} ({payout.payday}): " @@ -651,29 +655,42 @@ def pause(contract: Contract) -> Contract: f"Only an active contract can be paused (is {contract.status.value})." ) contract.status = ContractStatus.paused + # A deliberate pause carries no reason, which is what makes resume treat + # the missed paydays as written off rather than still owed. + contract.paused_reason = "" return contract def resume( - contract: Contract, today: date | None = None, catch_up: bool = False + contract: Contract, today: date | None = None, catch_up: bool | None = None ) -> Contract: """Put a paused contract back to work. - By default the paydays that fell during the pause are *not* paid: a - pause is a decision not to pay them, and resuming into a surprise - multi-period transfer is the opposite of what an operator asking to - resume expects. The contract's position is fast-forwarded to the next - payday on or after today instead. + What happens to the paydays that fell during the pause depends on who + paused it, because the two cases mean opposite things: - `catch_up=True` opts into paying them, for the case where the pause was - an operational hold rather than a decision about the money. + * **The operator paused it.** Those paydays are written off — the pause + *was* the decision not to pay them, and resuming into a surprise + multi-period transfer is the opposite of what "resume" implies. The + position fast-forwards to the next payday on or after today. + * **Payroll paused it** — a period exhausted its retries, because the + wallet was empty or the date could not be priced. Nobody decided + anything, the money is still owed, and the operator has just fixed + whatever blocked it. The backlog is kept and settles on the next tick. + + Sharing one path between those two silently destroys money that is owed, + so the inference is on `paused_reason`. `catch_up` overrides it. """ if contract.status != ContractStatus.paused: raise LifecycleError( f"Only a paused contract can be resumed (is {contract.status.value})." ) + if catch_up is None: + catch_up = bool(contract.paused_reason) + contract.status = ContractStatus.active + contract.paused_reason = "" if not catch_up: today = today or _today() skipped_to = fast_forward_index(contract, today) diff --git a/static/js/index.js b/static/js/index.js index 8f3b912..b268c7f 100644 --- a/static/js/index.js +++ b/static/js/index.js @@ -436,11 +436,19 @@ window.app = Vue.createApp({ }, resumeContract(contract) { + // Who paused it decides what happens to the missed paydays, so the + // question has to be asked differently for each. Telling an operator + // that a backlog payroll could not pay is "written off" would be + // false, and acting on it would destroy money that is still owed. + const auto = !!contract.paused_reason + const message = auto + ? `Payroll paused this contract: ${contract.paused_reason}. ` + + 'Resuming keeps the unpaid periods — they settle on the next tick. ' + + 'Fix the cause first, or they will just fail again.' + : 'Resume this contract? The paydays missed while it was paused ' + + 'are written off — they are not paid retroactively.' LNbits.utils - .confirmDialog( - 'Resume this contract? The paydays missed while it was paused ' + - 'are written off — they are not paid retroactively.' - ) + .confirmDialog(message) .onOk(() => this.transition(contract, 'resume')) }, diff --git a/templates/payroll/index.html b/templates/payroll/index.html index 4a5ab2e..ea00a95 100644 --- a/templates/payroll/index.html +++ b/templates/payroll/index.html @@ -59,7 +59,18 @@ diff --git a/tests/test_lifecycle.py b/tests/test_lifecycle.py index c8ccd57..d45d7f4 100644 --- a/tests/test_lifecycle.py +++ b/tests/test_lifecycle.py @@ -105,3 +105,73 @@ def test_cancel_from_a_live_status(status): def test_terminal_contracts_cannot_be_cancelled_again(status): with pytest.raises(LifecycleError): cancel(make_contract(status=status)) + + +# --- resuming an automatic pause ------------------------------------------- + +# Payroll pausing a contract because it could not pay, and an operator +# pausing one because they chose not to, mean opposite things. Sharing a +# recovery path silently writes off money that is still owed. + + +def _auto_paused(**kw): + contract = make_contract(status=ContractStatus.paused, **kw) + contract.paused_reason = "period 0 (2026-01-15) failed 5 times: no rate" + return contract + + +def test_resuming_an_auto_paused_contract_keeps_the_backlog(): + """The operator fixed the cause; those paydays are still owed.""" + contract = _auto_paused(start_date="2026-01-15", periods_done=0) + + resume(contract, today=date(2026, 4, 10)) + + assert contract.status == ContractStatus.active + assert contract.periods_done == 0 # nothing written off + assert contract.paused_reason == "" + + +def test_resuming_an_operator_pause_still_writes_off(): + contract = make_contract( + start_date="2026-01-15", periods_done=0, status=ContractStatus.paused + ) + + resume(contract, today=date(2026, 4, 10)) + + assert contract.periods_done == 3 + + +def test_an_explicit_choice_overrides_the_inference(): + contract = _auto_paused(start_date="2026-01-15", periods_done=0) + resume(contract, today=date(2026, 4, 10), catch_up=False) + assert contract.periods_done == 3 + + +def test_pausing_by_hand_clears_an_earlier_automatic_reason(): + """Otherwise a contract that once auto-paused would keep catching up + forever, even after a deliberate pause.""" + contract = _auto_paused(periods_done=2) + contract.status = ContractStatus.active + pause(contract) + assert contract.paused_reason == "" + + +def test_the_pause_reason_names_the_period_and_cause(): + from ..models import Payout, PayoutStatus + from ..services import MAX_PERIOD_ATTEMPTS, _maybe_pause_after_repeated_failure + + contract = make_contract() + payout = Payout( + id="p1", + contract_id=contract.id, + period_index=3, + payday="2026-04-15", + status=PayoutStatus.failed, + attempt=MAX_PERIOD_ATTEMPTS, + detail="no historical EUR rate available for 2026-04-15", + ) + _maybe_pause_after_repeated_failure(contract, payout) + + assert contract.status == ContractStatus.paused + assert "2026-04-15" in contract.paused_reason + assert "no historical EUR rate" in contract.paused_reason diff --git a/views_api.py b/views_api.py index 2d82d6d..580a5a8 100644 --- a/views_api.py +++ b/views_api.py @@ -214,14 +214,18 @@ async def api_pause_contract(contract_id: str) -> Contract: @payroll_api_router.post("/api/v1/contracts/{contract_id}/resume") -async def api_resume_contract(contract_id: str, catch_up: bool = False) -> Contract: +async def api_resume_contract( + contract_id: str, catch_up: bool | None = None +) -> Contract: """Put a paused contract back to work. - Paydays missed during the pause are skipped by default — a pause is a - decision not to pay them, and resuming into an unannounced multi-period - transfer is the opposite of what "resume" implies. `catch_up=true` pays - them, for a pause that was an operational hold rather than a decision - about the money. + What happens to the paydays missed during the pause depends on who + paused it. An operator pause *was* the decision not to pay them, so they + are written off. A pause payroll applied itself — a period that + exhausted its retries — means the money is still owed and the operator + has just fixed whatever blocked it, so the backlog is kept. + + Pass `catch_up` explicitly to override that inference. """ return await _transition( contract_id, lambda c: services.resume(c, catch_up=catch_up)