fix: resuming an auto-paused contract no longer discards the backlog
Found by tracing what happens when a back-dated backfill contract cannot fetch a historical rate. The failure handling itself was fine — period 0 fails, the backlog halts so nothing settles out of order, five ledger rows record the reason, no money moves, and the contract auto-pauses once the retry budget is spent. The recovery was not. The operator fixes the cause (switches to a stated rate, or to current), clicks Resume, and periods_done jumps 0 -> 6: every unpaid payday silently written off, contract back to looking healthy, employee never paid. The confirm dialog even asserted the missed paydays "are written off" — true of one kind of pause and a lie about the other. Two features colliding. "Do not backfill a deliberate pause" is right when the operator paused: the pause *was* the decision not to pay. It is wrong when payroll paused, because nobody decided anything — the money is still owed and the operator has just removed whatever blocked it. Contracts now carry `paused_reason`, set only when payroll pauses them and cleared by a deliberate pause. Resume infers from it, and an explicit `catch_up` still overrides either way. The console asks a different question for each, quoting the reason, and flags a payroll-paused contract in the table so the distinction is visible before anyone clicks. Verified end to end: five failing ticks leave periods_done at 0 and pause with "period 0 (2026-08-01) failed 5 times: no historical EUR rate available for 2026-08-01"; resuming after switching to a manual rate keeps the position at 0, and the next tick settles all seven owed periods. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018jy52j9GRZ6XKa1Zt21LLj
This commit is contained in:
parent
1584eb337f
commit
e77f431d47
8 changed files with 168 additions and 22 deletions
|
|
@ -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
|
||||
|
||||
|
|
|
|||
|
|
@ -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 '';"
|
||||
)
|
||||
|
|
|
|||
|
|
@ -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.
|
||||
|
|
|
|||
33
services.py
33
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)
|
||||
|
|
|
|||
|
|
@ -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'))
|
||||
},
|
||||
|
||||
|
|
|
|||
|
|
@ -59,7 +59,18 @@
|
|||
<template v-slot:body-cell-status="props">
|
||||
<q-td :props="props">
|
||||
<q-chip dense square :color="statusColour(props.row.status)"
|
||||
text-color="white" :label="props.row.status"></q-chip>
|
||||
text-color="white" :label="props.row.status">
|
||||
<q-tooltip v-if="props.row.paused_reason">
|
||||
${ props.row.paused_reason }
|
||||
</q-tooltip>
|
||||
</q-chip>
|
||||
<q-icon v-if="props.row.paused_reason" name="error_outline"
|
||||
color="negative" size="xs" class="q-ml-xs">
|
||||
<q-tooltip>
|
||||
Paused by payroll, not by you — the unpaid periods are
|
||||
still owed and resuming will settle them.
|
||||
</q-tooltip>
|
||||
</q-icon>
|
||||
</q-td>
|
||||
</template>
|
||||
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
|
|
|||
16
views_api.py
16
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)
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue