fix(issue-50): the waiter and its hold are pinned to one attempt and session; an unsignalled pending record lives by its ack budget (review 55219be1 R1, R2)
R1: wait-open-jobs takes --dispatch-token (the token the wrapper read for this attempt) and keeps it, with the session it asked about, for its whole life; every write (signal, admission, the hold's renewal) is a guarded engine merge that re-reads the record under the registry lock and refuses once the record is not that attempt's pending one in that session. A record redispatched, moved or closed by someone else makes the waiter answer `superseded`: the bash wrapper exits 5 without a close, the record is its new owner's. hold-admission is given the waiter's pinned identity instead of reading a token from disk, so a hold that starts late holds nothing. R2: a pending record that never carried a waiter's signal (create --onboard registers and injects directly and waits ack_timeout_sec for `started`) is live for that budget from created_at, not for the 30 s signal window; the window applies only once a dispatcher has signalled. Tests: u1 rows for the two pending lifetimes, u3b asserts the guard, u3f/u3g/u3h (cross-session redispatch, in-place attempt change, token changed before the waiter started) with the real waiter subprocess, u4e/u4f (late hold, running hold released by a redispatch), s1 pins the token argument and the no-close branch; real herdr C10 (onboarding valid before and after 30 s blocks the session), C11 (expired onboarding is stale), C12 (redispatched waiter leaves its record alone, exit 5). Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
This commit is contained in:
GodopuandClaude Fable 5.1 committed
1 parent
9cbe08e8de
commit
30c25f193c
5 files changed
+394
-104
No files matched your search
+197
-38
@@ -1,11 +1,12 @@
|
||||
"""Issue #50 C: the delegate-job wrapper waits by the registry before it injects.
|
||||
|
||||
- U: the decisions in lib_py.delegation.wrapper - the open-job table (status x
|
||||
signal age x session spelling), FIFO release among live pending waiters,
|
||||
the admission persisted on the released record (review aa1bff92 F1) and
|
||||
held alive after the answer (F2), the waiter's own signal written through
|
||||
the registry, the bound, a dead wrapper, and finalize-end's `registry_wait`
|
||||
phase closing as session_busy.
|
||||
signal age x ack budget x session spelling), FIFO release among live
|
||||
pending waiters, the admission persisted on the released record (review
|
||||
aa1bff92 F1) and held alive after the answer (F2), the waiter and its hold
|
||||
pinned to one attempt and session (review 55219be1 R1), the waiter's own
|
||||
signal written through the registry, the bound, a dead wrapper, and
|
||||
finalize-end's `registry_wait` phase closing as session_busy.
|
||||
- H: against a real herdr 0.9.3 server (one-off session, fake claude in the
|
||||
pane, the real publish path through publish_event.py and the suite's local
|
||||
broker): see test_issue50_dispatch_herdr.py.
|
||||
@@ -83,24 +84,26 @@ def _ns(reg, job, session="herdr:s", wait_s=0, start_time=None, timeout=3600, wa
|
||||
# U1 - the open-job table
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
@pytest.mark.parametrize("status,age,signal,want", [
|
||||
("running", 10, None, "live"), # an event idle_timeout_sec (120) ago at most
|
||||
("running", 121, None, "stale"),
|
||||
("running", None, None, "stale"), # running with no events.log at all
|
||||
("pending", 10, None, "live"), # created within the signal window (30)
|
||||
("pending", 30, None, "live"),
|
||||
("pending", 31, None, "stale"),
|
||||
("pending", 181, 5, "live"), # old record, fresh dispatch_wait_at
|
||||
("pending", 181, 31, "stale"),
|
||||
("pending", 10, 31, "stale"), # the signal, once present, is what counts
|
||||
("pending", 1, 2, "live"), # ack_timeout_sec (1 here) is not the window
|
||||
("completed", 10, None, None),
|
||||
("error", 10, None, None),
|
||||
("cancelled", 10, None, None),
|
||||
@pytest.mark.parametrize("status,age,signal,ack,want", [
|
||||
("running", 10, None, 1, "live"), # an event idle_timeout_sec (120) ago at most
|
||||
("running", 121, None, 1, "stale"),
|
||||
("running", None, None, 1, "stale"), # running with no events.log at all
|
||||
("pending", 10, None, 180, "live"), # never signalled: created within its ack budget
|
||||
("pending", 31, None, 180, "live"), # ... past the signal window too (review 55219be1 R2:
|
||||
("pending", 180, None, 180, "live"), # create --onboard waits that long for `started`)
|
||||
("pending", 181, None, 180, "stale"),
|
||||
("pending", 10, None, 1, "stale"), # never signalled, past its ack budget: a dead producer
|
||||
("pending", 181, 5, 1, "live"), # old record, fresh dispatch_wait_at
|
||||
("pending", 181, 31, 180, "stale"), # signalled: the window (30), not the ack budget
|
||||
("pending", 10, 31, 180, "stale"), # the signal, once present, is what counts
|
||||
("pending", 1, 2, 1, "live"), # ack_timeout_sec (1 here) is not the signalled window
|
||||
("completed", 10, None, 1, None),
|
||||
("error", 10, None, 1, None),
|
||||
("cancelled", 10, None, 1, None),
|
||||
])
|
||||
def test_u1_open_jobs_table(tmp_path, status, age, signal, want):
|
||||
def test_u1_open_jobs_table(tmp_path, status, age, signal, ack, want):
|
||||
NOW = int(time.time()) # the running check reads the real clock (recent_disk_heartbeat)
|
||||
other = _register(tmp_path, status=status, ack=1)
|
||||
other = _register(tmp_path, status=status, ack=ack)
|
||||
if status == "running" and age is not None:
|
||||
_event(tmp_path, other, NOW - age)
|
||||
if status == "pending":
|
||||
@@ -115,8 +118,11 @@ def test_u1_open_jobs_table(tmp_path, status, age, signal, want):
|
||||
elif want == "stale":
|
||||
assert got == (set(), {other}), got
|
||||
assert stale[0][1].startswith(status + ", no ")
|
||||
if status == "pending":
|
||||
assert stale[0][1].endswith(f"for {WINDOW}s" if signal else f"no started within {ack}s")
|
||||
else:
|
||||
assert got == (set(), set())
|
||||
assert decide.DEFAULT_ACK_TIMEOUT_SEC == registry.DEFAULT_ACK_TIMEOUT_SEC
|
||||
|
||||
|
||||
@pytest.mark.parametrize("token_match,signal,want", [(True, 5, "admitted"), (False, 5, "pending"), (True, 31, "stale")])
|
||||
@@ -304,13 +310,13 @@ def test_u3e_the_admission_is_rechecked_under_the_registry_lock(tmp_path, monkey
|
||||
|
||||
|
||||
def test_u3c_a_terminal_self_record_cannot_carry_the_signal(tmp_path):
|
||||
# why the loop path resets its record to pending before it waits (s1):
|
||||
# the engine refuses a merge into a terminal record, so the waiter would
|
||||
# stay invisible to every other waiter's FIFO
|
||||
# why the loop path resets its record to pending before it waits (s1): a
|
||||
# terminal record is not this attempt's pending one, the waiter ends at
|
||||
# once without a write (superseded) and is in no other waiter's FIFO
|
||||
a = _register(tmp_path, status="running")
|
||||
_event(tmp_path, a, time.time())
|
||||
me = _register(tmp_path, status="completed")
|
||||
assert decide.wait_open_jobs(_ns(tmp_path, me, wait_s=1)) == f"busy {a}"
|
||||
assert decide.wait_open_jobs(_ns(tmp_path, me, wait_s=1)) == "superseded"
|
||||
after = json.loads((tmp_path / f"{me}.json").read_text())
|
||||
assert after["status"] == "completed" and "dispatch_wait_at" not in after
|
||||
later = _register(tmp_path)
|
||||
@@ -318,15 +324,114 @@ def test_u3c_a_terminal_self_record_cannot_carry_the_signal(tmp_path):
|
||||
assert me not in {j for _c, j, _s in live}
|
||||
|
||||
|
||||
def test_u3b_the_signal_is_a_registry_merge(tmp_path, monkeypatch):
|
||||
def test_u3b_the_signal_is_a_registry_merge_guarded_by_the_pinned_identity(tmp_path, monkeypatch):
|
||||
from lib_py.delegation import engine
|
||||
me = _register(tmp_path)
|
||||
tok = _rec(tmp_path, me)["dispatch_token"]
|
||||
seen = {}
|
||||
monkeypatch.setattr(mqtt_common, "update_job_status",
|
||||
lambda job_id, registry_dir, _source="unknown", **fields: seen.update(
|
||||
job=job_id, reg=registry_dir, source=_source, fields=fields))
|
||||
decide._signal_dispatch_wait(str(tmp_path), me)
|
||||
assert seen["job"] == me and seen["reg"] == str(tmp_path) and seen["source"] == "dispatch_wait"
|
||||
assert set(seen["fields"]) == {"dispatch_wait_at"}
|
||||
|
||||
def transition(job_id, request, registry_dir=None, source="unknown", guard=None, fields=None, **kw):
|
||||
seen.update(job=job_id, request=request, reg=registry_dir, source=source, guard=guard, fields=fields)
|
||||
return argparse.Namespace(committed=True)
|
||||
|
||||
monkeypatch.setattr(engine, "transition", transition)
|
||||
assert decide._signal_dispatch_wait(str(tmp_path), me, tok, "herdr:s") is True
|
||||
assert seen["job"] == me and seen["request"] == "merge" and seen["reg"] == str(tmp_path)
|
||||
assert seen["source"] == "dispatch_wait" and set(seen["fields"]) == {"dispatch_wait_at"}
|
||||
# the guard, run under the registry lock, is the waiter's identity (review 55219be1 R1)
|
||||
mine = {"status": "pending", "dispatch_token": tok, "agent_session": "herdr:s"}
|
||||
assert seen["guard"](mine) and seen["guard"](dict(mine, agent_session="s"))
|
||||
assert not seen["guard"](dict(mine, dispatch_token="another-attempt"))
|
||||
assert not seen["guard"](dict(mine, agent_session="herdr:t"))
|
||||
assert not seen["guard"](dict(mine, status="running"))
|
||||
# the hold's renewal also requires its admission to stand
|
||||
decide._signal_dispatch_wait(str(tmp_path), me, tok, "herdr:s", admitted=True)
|
||||
assert not seen["guard"](mine) and seen["guard"](dict(mine, dispatch_admitted_token=tok))
|
||||
assert not seen["guard"](dict(mine, dispatch_admitted_token="another-attempt"))
|
||||
|
||||
|
||||
def _waiter(reg, job, session="herdr:s", wait_s="20", token=None):
|
||||
cmd = [sys.executable, "-m", "lib_py.delegation.wrapper", "wait-open-jobs", "--job", job, "--agent-session",
|
||||
session, "--registry-dir", str(reg), "--wait-s", wait_s, "--timeout", "3600", "--watch-pid", str(os.getpid())]
|
||||
if token:
|
||||
cmd += ["--dispatch-token", token]
|
||||
return subprocess.Popen(cmd, cwd=str(SKILLS), env=dict(os.environ, PYTHONPATH=str(SKILLS)), stdout=subprocess.PIPE,
|
||||
stderr=subprocess.PIPE, text=True)
|
||||
|
||||
|
||||
def _redispatch(reg, jid, *extra):
|
||||
res = subprocess.run([sys.executable, str(REGISTRY_PY), "--registry-dir", str(reg), "update", "--job", jid,
|
||||
"--status", "pending", "--attempt-id", "2", *extra], capture_output=True, text=True, timeout=60)
|
||||
assert res.returncode == 0, res.stdout + res.stderr
|
||||
|
||||
|
||||
def test_u3f_a_waiter_redispatched_to_another_session_is_superseded_and_admits_nothing(tmp_path):
|
||||
# review 55219be1 R1: A runs in S, C runs in T, B waits behind A in S; B is
|
||||
# redispatched (attempt 2) to T; A ends. The attempt-1 waiter must not
|
||||
# admit attempt 2 against S's occupancy: it ends, writes nothing
|
||||
a = _register(tmp_path, status="running")
|
||||
_event(tmp_path, a, time.time())
|
||||
c = _register(tmp_path, session="herdr:t", status="running")
|
||||
_event(tmp_path, c, time.time())
|
||||
b = _register(tmp_path)
|
||||
old = _rec(tmp_path, b)["dispatch_token"]
|
||||
proc = _waiter(tmp_path, b, token=old)
|
||||
try:
|
||||
deadline = time.time() + 5
|
||||
while time.time() < deadline and not _rec(tmp_path, b).get("dispatch_wait_at"):
|
||||
time.sleep(0.05)
|
||||
_redispatch(tmp_path, b, "--agent-session", "herdr:t")
|
||||
moved = _rec(tmp_path, b)
|
||||
assert moved["dispatch_token"] != old and moved["agent_session"] == "herdr:t"
|
||||
_edit(tmp_path, a, status="completed")
|
||||
out, err = proc.communicate(timeout=10)
|
||||
assert out.strip() == "superseded", (out, err)
|
||||
rec = _rec(tmp_path, b)
|
||||
assert "dispatch_admitted_token" not in rec and rec["status"] == "pending" and rec["attempt_id"] == 2
|
||||
assert rec["dispatch_wait_at"] == moved["dispatch_wait_at"] and rec["transition_seq"] == moved["transition_seq"], \
|
||||
"nothing was renewed after the redispatch"
|
||||
assert _rec(tmp_path, c)["status"] == "running"
|
||||
assert decide.wait_open_jobs(_ns(tmp_path, b, "herdr:t")) == f"busy {c}", "T is what attempt 2 is judged by"
|
||||
finally:
|
||||
if proc.poll() is None:
|
||||
proc.kill()
|
||||
proc.communicate()
|
||||
|
||||
|
||||
def test_u3g_a_waiter_whose_attempt_changed_in_place_leaves_the_admission_to_the_new_dispatcher(tmp_path):
|
||||
a = _register(tmp_path, status="running")
|
||||
_event(tmp_path, a, time.time())
|
||||
b = _register(tmp_path)
|
||||
old = _rec(tmp_path, b)["dispatch_token"]
|
||||
proc = _waiter(tmp_path, b, token=old)
|
||||
try:
|
||||
deadline = time.time() + 5
|
||||
while time.time() < deadline and not _rec(tmp_path, b).get("dispatch_wait_at"):
|
||||
time.sleep(0.05)
|
||||
_redispatch(tmp_path, b)
|
||||
_edit(tmp_path, a, status="completed")
|
||||
out, err = proc.communicate(timeout=10)
|
||||
assert out.strip() == "superseded", (out, err)
|
||||
assert "dispatch_admitted_token" not in _rec(tmp_path, b)
|
||||
new = _rec(tmp_path, b)["dispatch_token"]
|
||||
assert decide.wait_open_jobs(_ns(tmp_path, b)) == "clear", "attempt 2's own dispatcher admits it"
|
||||
assert _rec(tmp_path, b)["dispatch_admitted_token"] == new != old
|
||||
finally:
|
||||
if proc.poll() is None:
|
||||
proc.kill()
|
||||
proc.communicate()
|
||||
|
||||
|
||||
def test_u3h_the_pinned_token_is_the_wrappers_not_the_records(tmp_path):
|
||||
# the record's token changed before the waiter started (a redispatch raced
|
||||
# the wrapper): the waiter given the old token is superseded at once
|
||||
me = _register(tmp_path)
|
||||
old = _rec(tmp_path, me)["dispatch_token"]
|
||||
_redispatch(tmp_path, me)
|
||||
proc = _waiter(tmp_path, me, token=old)
|
||||
out, err = proc.communicate(timeout=10)
|
||||
assert out.strip() == "superseded" and "dispatch_admitted_token" not in _rec(tmp_path, me), (out, err)
|
||||
assert decide.wait_open_jobs(_ns(tmp_path, me)) == "clear" # the current token, read once at the start
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
@@ -345,7 +450,8 @@ def test_u4_open_jobs_prints_live_and_stale(tmp_path):
|
||||
me = _register(tmp_path)
|
||||
res = _cli(tmp_path, "open-jobs", "--job", me, "--agent-session", "herdr:s", "--registry-dir", str(tmp_path))
|
||||
assert res.returncode == 0, res.stderr
|
||||
assert f"{a} running" in res.stdout and f"{b} stale pending, no dispatch signal for {WINDOW}s" in res.stdout
|
||||
assert f"{a} running" in res.stdout
|
||||
assert f"{b} stale pending, no dispatch signal and no started within 180s" in res.stdout
|
||||
|
||||
|
||||
def _hold_pids(jid):
|
||||
@@ -378,19 +484,68 @@ def test_u4c_the_wait_holds_its_admission_until_the_record_leaves_pending(tmp_pa
|
||||
os.kill(pid, 9)
|
||||
|
||||
|
||||
def _hold(reg, jid, token, *extra, session="herdr:s"):
|
||||
return _cli(reg, "hold-admission", "--job", jid, "--agent-session", session, "--dispatch-token", token,
|
||||
"--registry-dir", str(reg), "--timeout", "3600", *extra)
|
||||
|
||||
|
||||
def test_u4d_the_hold_ends_with_its_wrapper_and_without_an_admission(tmp_path):
|
||||
me = _register(tmp_path)
|
||||
res = _cli(tmp_path, "hold-admission", "--job", me, "--registry-dir", str(tmp_path), "--timeout", "3600")
|
||||
tok = _rec(tmp_path, me)["dispatch_token"]
|
||||
res = _hold(tmp_path, me, tok)
|
||||
assert res.stdout.strip() == "released", res.stderr # never admitted: nothing to hold
|
||||
assert decide.wait_open_jobs(_ns(tmp_path, me)) == "clear"
|
||||
gone = subprocess.Popen(["true"])
|
||||
gone.wait()
|
||||
t0 = time.time()
|
||||
res = _cli(tmp_path, "hold-admission", "--job", me, "--registry-dir", str(tmp_path), "--timeout", "3600",
|
||||
"--watch-pid", str(gone.pid))
|
||||
res = _hold(tmp_path, me, tok, "--watch-pid", str(gone.pid))
|
||||
assert res.stdout == "" and "is gone" in res.stderr and time.time() - t0 < 10, res.stdout + res.stderr
|
||||
|
||||
|
||||
def test_u4e_a_hold_that_starts_late_holds_only_the_attempt_it_was_given(tmp_path):
|
||||
# review 55219be1 R1: the token changed (or the record moved) between the
|
||||
# admission and the hold's start - the hold is released without a write
|
||||
me = _register(tmp_path)
|
||||
assert decide.wait_open_jobs(_ns(tmp_path, me)) == "clear"
|
||||
admitted = _rec(tmp_path, me)
|
||||
tok = admitted["dispatch_token"]
|
||||
res = _hold(tmp_path, me, tok, session="herdr:t") # not the session it admitted in
|
||||
assert res.stdout.strip() == "released" and _rec(tmp_path, me) == admitted, res.stderr
|
||||
_redispatch(tmp_path, me)
|
||||
moved = _rec(tmp_path, me)
|
||||
assert moved["dispatch_token"] != tok
|
||||
res = _hold(tmp_path, me, tok)
|
||||
assert res.stdout.strip() == "released" and _rec(tmp_path, me) == moved, res.stderr
|
||||
assert "dispatch_admitted_token" in moved and moved["dispatch_admitted_token"] == tok, \
|
||||
"attempt 1's admission is left as it is - and names no current token (u3d)"
|
||||
|
||||
|
||||
def test_u4f_a_running_hold_is_released_by_a_redispatch_and_renews_nothing_after_it(tmp_path):
|
||||
me = _register(tmp_path)
|
||||
assert decide.wait_open_jobs(_ns(tmp_path, me)) == "clear"
|
||||
tok = _rec(tmp_path, me)["dispatch_token"]
|
||||
first = _rec(tmp_path, me)["dispatch_wait_at"]
|
||||
proc = subprocess.Popen([sys.executable, "-m", "lib_py.delegation.wrapper", "hold-admission", "--job", me,
|
||||
"--agent-session", "herdr:s", "--dispatch-token", tok, "--registry-dir", str(tmp_path),
|
||||
"--timeout", "3600", "--watch-pid", str(os.getpid())],
|
||||
cwd=str(SKILLS), env=dict(os.environ, PYTHONPATH=str(SKILLS)), stdout=subprocess.PIPE,
|
||||
stderr=subprocess.PIPE, text=True)
|
||||
try:
|
||||
deadline = time.time() + decide.DISPATCH_SIGNAL_EVERY + 5
|
||||
while time.time() < deadline and _rec(tmp_path, me)["dispatch_wait_at"] == first:
|
||||
time.sleep(0.5)
|
||||
assert _rec(tmp_path, me)["dispatch_wait_at"] != first, "it renewed while the admission stood"
|
||||
_redispatch(tmp_path, me)
|
||||
moved = _rec(tmp_path, me)
|
||||
out, err = proc.communicate(timeout=10)
|
||||
assert out.strip() == "released", (out, err)
|
||||
assert _rec(tmp_path, me) == moved, "nothing written after the redispatch"
|
||||
finally:
|
||||
if proc.poll() is None:
|
||||
proc.kill()
|
||||
proc.communicate()
|
||||
|
||||
|
||||
def test_u4b_wait_open_jobs_stops_with_its_wrapper(tmp_path):
|
||||
a = _register(tmp_path, status="running")
|
||||
_event(tmp_path, a, time.time())
|
||||
@@ -436,8 +591,9 @@ def test_u5_finalize_end_registry_wait_is_a_session_busy_cancel(tmp_path):
|
||||
|
||||
def test_s1_both_dispatch_paths_wait_by_the_registry_before_the_idle_wait():
|
||||
src = WRAPPER.read_text()
|
||||
assert src.count('_registry_wait "$JOB_ID" "$AGENT_SESSION"') == 1
|
||||
assert src.count('_registry_wait "$JOB_ID" "$current_session"') == 1
|
||||
# ... each with the token it read for this attempt, pinned (review 55219be1 R1)
|
||||
assert src.count('_registry_wait "$JOB_ID" "$AGENT_SESSION" "$_disp_tok"') == 1
|
||||
assert src.count('_registry_wait "$JOB_ID" "$current_session" "$_disp_tok"') == 1
|
||||
direct = src.index('_registry_wait "$JOB_ID" "$AGENT_SESSION"')
|
||||
assert direct < src.index('wait_for_agent_idle "$AGENT_SESSION"')
|
||||
loop = src.index('_registry_wait "$JOB_ID" "$current_session"')
|
||||
@@ -454,3 +610,6 @@ def test_s1_both_dispatch_paths_wait_by_the_registry_before_the_idle_wait():
|
||||
assert "wait-open-jobs" in body and 'finalize_unfinished "$job_id" "error" "$SUB_RC_ERROR" "registry_wait"' in body
|
||||
assert "exit 5" in body and "mam_int_knob MAM_DISPATCH_WAIT_S 600 0" in body
|
||||
assert "--hold" in body, "the admission is held alive after the answer (u4c)"
|
||||
assert '${tok:+--dispatch-token "$tok"}' in body
|
||||
superseded = body.split("superseded)")[1].split(";;")[0]
|
||||
assert "exit 5" in superseded and "finalize_unfinished" not in superseded, "left to its owner: no close"
|
||||
Reference in new issue
Block a user