Found during the operational end-to-end review (#218) and pinned rather than fixed, because fixing it needs a decision about which clock is authoritative rather than a patch.
The inconsistency
POST /api/work/{id}/retry and POST /api/work/{id}/block compare a stored lease_until against time.time().
GET /api/holds compares against queue.now() — the queue's own clock — and says why in its docstring.
Both readings are of the same field, written by the same code, and they disagree about what "now" is.
Why it matters less than it looks, and still matters
In production they are identical: the queue's clock is the wall clock unless something injected another. So nothing is broken today, and this is not urgent.
It matters because the queue's clock is injectable precisely so that lease behaviour can be tested and simulated, and two of the routes silently opt out of that. A test that advances the queue's clock to expire a lease will find /api/holds agrees and /retry does not — so the API behaves differently under test than the queue does, which is exactly backwards from what an injectable clock is for.
The E2E suite in #218 does not paper over this: its fixture starts the queue clock at the wall clock so both readings coincide, and says so rather than hiding the divergence.
The decision behind it
queue.now() is presumably right — one clock, owned by the component that owns the leases. But that should be stated once and applied everywhere rather than assumed here, and it is worth checking whether any other module reads time.time() against queue state.
Blind spots
- Only these two routes were found.
api.py has not been swept for other direct time.time() comparisons against queue fields.
- Whether anything outside tests ever injects a different clock is unknown; if nothing does, this is documentation rather than a defect.
Found during the operational end-to-end review (#218) and pinned rather than fixed, because fixing it needs a decision about which clock is authoritative rather than a patch.
The inconsistency
POST /api/work/{id}/retryandPOST /api/work/{id}/blockcompare a storedlease_untilagainsttime.time().GET /api/holdscompares againstqueue.now()— the queue's own clock — and says why in its docstring.Both readings are of the same field, written by the same code, and they disagree about what "now" is.
Why it matters less than it looks, and still matters
In production they are identical: the queue's clock is the wall clock unless something injected another. So nothing is broken today, and this is not urgent.
It matters because the queue's clock is injectable precisely so that lease behaviour can be tested and simulated, and two of the routes silently opt out of that. A test that advances the queue's clock to expire a lease will find
/api/holdsagrees and/retrydoes not — so the API behaves differently under test than the queue does, which is exactly backwards from what an injectable clock is for.The E2E suite in #218 does not paper over this: its fixture starts the queue clock at the wall clock so both readings coincide, and says so rather than hiding the divergence.
The decision behind it
queue.now()is presumably right — one clock, owned by the component that owns the leases. But that should be stated once and applied everywhere rather than assumed here, and it is worth checking whether any other module readstime.time()against queue state.Blind spots
api.pyhas not been swept for other directtime.time()comparisons against queue fields.