Skip to content

emrg: a boss call that delivers nothing does not consume the re-ask window - #2502

Merged
argszero merged 1 commit into
masterfrom
fix/a-timeout-does-not-consume-the-window
Oct 22, 2026
Merged

argszero merged 1 commit into
masterfrom
fix/a-timeout-does-not-consume-the-window

Conversation

@how2how2how2-arch

Copy link
Copy Markdown
Collaborator

Closes #2499

A boss call 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_child stamps 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_tool holds the bound, and it cancels that turn on timeout_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 ("Raise timeout_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_child beside the owner session), and is called from the timeout_s branch and the CancelledError branch of _run_boss_tool. Session.set_boss_asked_at now accepts None to clear, and the key leaves meta.json with it — the state a session that was never asked is in, since _save_meta_with_title rebuilds 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:

Arm Before the fix After
test_a_call_that_timed_out_does_not_consume_the_window — timeout, then retry must start a turn red — assert 0 == 1, the retry started no turn green
test_a_call_cancelled_with_its_round_does_not_consume_the_window — the round dies, the clock must not be left on disk red — boss_asked_at was 1792692812.11, not None green
test_a_call_that_answered_still_consumes_the_window — the control: an answered call still refuses the second green green

The 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 the timeout_s branch (occurrence asserted, and the count verified 3 → 2), arm (a) failed; restored, grep -c reads 3 and git status is 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

  • CI is pending on this head. It runs test (ubuntu-latest) and test-windows (windows-2025); there is no macOS leg and none is claimed. The logic is platform-free Python.
  • The four-hour figure is unchanged and unreviewed here; this change is only about when the clock stands.
  • A turn cancelled because its own round was cancelled now also gives the window back. That is the same defect by the same argument, and it is measured — but it is a behaviour change beyond the timeout case named in the issue, so it is called out rather than slipped in.

@argszero argszero left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ 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 argszero left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ 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 argszero left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ 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 → base cc121c7a, final tree e696de0fba3c,
    suite OK: 5677 passed, 35 skipped in 231.77s.
  • check-merge-landing-diff.py 2502 → the landing changes exactly emrg/server/daemon.py
    (+38 -4), emrg/session.py (+12 -3) and tests/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.

@argszero
argszero merged commit 21d76a8 into master Oct 22, 2026
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

emrg: a boss call that times out still locks the owner out for four hours

2 participants