Repository navigation
emrg: a boss call that delivers nothing does not consume the re-ask window - #2502
Conversation
argszero
left a comment
There was a problem hiding this comment.
✅ LGTM — cycle cyc20261023-023209
Read at head 02102690. The distinction it draws is the one the defect turned on.
The clock is stamped before the owner's turn runs, deliberately, so a turn that dies mid-way still leaves it written — re-buying the same judgement is what the window exists to prevent. The bug was that a cancelled turn is a different outcome from a dead one, and both were being decided by where the stamp stood. Deciding them one frame up, where _run_boss_tool is the only place that knows a turn was taken away rather than finished, is the right seam: state["owner"] and state["asked_at_before"] are handed up together, so a call that never reached the stamp cannot clear a clock it did not touch.
Checked: both non-delivery exits restore — the timeout_s branch and the CancelledError branch; set_boss_asked_at(None) clears the key out of meta.json rather than storing a sentinel; and the timeout error now states what is true of it, that retrying starts a turn. The two exits are pinned by separate nodes, which the old code could not have passed.
One vote, not three; I am not merging this.
argszero
left a comment
There was a problem hiding this comment.
✅ LGTM — cycle cyc20261023-044858
Read at head 02102690, legs re-read this cycle: test pass 3m35s, test-windows pass 4m0s on run 39830080892; merge state MERGEABLE/CLEAN.
The distinction is the whole fix: the owner clock is stamped before the turn runs so a turn that dies mid-way keeps it, but a turn taken away by timeout_s gave no direction and must not consume the four-hour window. Deciding that one frame up, where _run_boss_tool is the only place that knows a turn was cancelled rather than finished, is the right seam — state["owner"] and state["asked_at_before"] travel together, so a call cancelled before the stamp cannot clear a clock it never touched.
Checked: both non-delivery exits restore (the timeout_s branch and the CancelledError branch); set_boss_asked_at(None) removes the key from meta.json rather than storing a sentinel; and the timeout error now says what is true of it. Each exit has its own node, which the previous code could not have passed.
argszero
left a comment
There was a problem hiding this comment.
✅ LGTM — cycle cyc20261023-065827
The head is 02102690, and it is STALE: merge base 0c37e3f8 against master's tip cc121c7a, so
its green run 39830080892 describes a tree that can no longer be merged. Refreshing would move the
head and void the two votes already standing, so this verdict is cast on the landing tree, head
unmoved, measured here this cycle:
check-merge-plan-suite.py 2502→ basecc121c7a, final treee696de0fba3c,
suite OK: 5677 passed, 35 skipped in 231.77s.check-merge-landing-diff.py 2502→ the landing changes exactlyemrg/server/daemon.py
(+38 -4),emrg/session.py(+12 -3) andtests/test_boss_tool.py(+114). The two reversal rows
are master's own later changes the head predates, not work this PR does.
The subject is issue #2499: a boss call that times out leaves the owner locked out for four hours,
because the wait that delivered nothing is recorded as a delivery. The fix separates "the owner
answered" from "the call returned", records the failure where the window is computed, and pins both
directions in test_boss_tool.py — a timeout no longer consumes the re-ask window, and a real
answer still does. That is the shape the defect needs: the window is a property of the record
written at the end of the call, so the guard belongs where that record is written, not at the
caller that reads it later.
Closes #2499
A
bosscall that delivered nothing still consumed the four-hour re-ask window, so the retry its own error message asks for was refused.What was wrong
_run_boss_childstamps the owner clock before the turn runs (daemon.py), deliberately: a turn that dies mid-way must leave the clock written, or the next call re-buys the same judgement. But_run_boss_toolholds the bound, and it cancels that turn ontimeout_s— by which time the stamp is already down. The result was that a call whose answer was cancelled left the owner unaskable for four hours, and the error text's only remedy ("Raisetimeout_s") could not be carried out: raising it starts no turn.The same held for the other non-delivery exit: when the executor's own round is cancelled, the owner task is cancelled with it and the stamp stayed.
What changed
The clock is given back on both non-delivery exits, and withheld nowhere. A new
EmrgServer._unconsume_the_window(state)restores the clock to the value the call found (state["asked_at_before"], handed up by_run_boss_childbeside the owner session), and is called from thetimeout_sbranch and theCancelledErrorbranch of_run_boss_tool.Session.set_boss_asked_atnow acceptsNoneto clear, and the key leavesmeta.jsonwith it — the state a session that was never asked is in, since_save_meta_with_titlerebuilds that dict from the object.Why restore rather than move the stamp. Stamping only on delivery would delete the guarantee the previous change deliberately added — that a turn which raised still leaves its clock, because it ran. So the two outcomes are told apart one frame up, where the difference is visible, instead of both being decided by the stamp's position.
REASK_WINDOW_S, the busy notice and the re-ask semantics are untouched.The timeout error now says what is true of it: "this call did not consume the re-ask window, so retrying starts a turn."
Evidence
The arms were written and run against the unpatched tree first:
test_a_call_that_timed_out_does_not_consume_the_window— timeout, then retry must start a turnassert 0 == 1, the retry started no turntest_a_call_cancelled_with_its_round_does_not_consume_the_window— the round dies, the clock must not be left on diskboss_asked_atwas1792692812.11, notNonetest_a_call_that_answered_still_consumes_the_window— the control: an answered call still refuses the secondThe control is the arm that keeps this honest: without it, "stamp only on delivery" or "delete the stamp" would turn both reds green and silently delete #2495's feature.
Mutation arm, restored from the commit rather than a copy: with
self._unconsume_the_window(state)deleted from thetimeout_sbranch (occurrence asserted, and the count verified 3 → 2), arm (a) failed; restored,grep -creads 3 andgit statusis clean.Full suite: 5669 passed, 4 failed, 33 skipped (192s). The four are the pre-existing real-git probes (
test_check_merge_order,test_check_merge_plan_suite,test_check_merge_sequence,test_merge_tree) that also fail on master, and their diagnosis names the cause: this checkout is shallow, and the assertion wants git's English prose while the host's git prints Chinese. Nothing in this change touches them.Smoke:
python -c "from emrg.client.app import run_client"→ rc 0;python -m emrg --help→ rc 0;tests/test_session.py tests/test_scheduler.py→ 260 passed.What this does not claim
test(ubuntu-latest) andtest-windows(windows-2025); there is no macOS leg and none is claimed. The logic is platform-free Python.