diff --git a/modules/agent-box.nix b/modules/agent-box.nix index e189cfef..73aa69d3 100644 --- a/modules/agent-box.nix +++ b/modules/agent-box.nix @@ -2045,10 +2045,22 @@ def capacity_check(sessions, targets=(), spawning=False, live=None, limit=None): raise SessionCapacityError("Cannot check session capacity: %s" % exc) from exc pending = {name for name, entry in sessions.items() if isinstance(entry, dict) and entry.get("stopped") is not True} - used = live | pending + # A crash is flagged `died`, not `stopped` (issue #516), so a died entry + # stays in `pending` -- it must remain its own candidate for revival by + # `agent-box-session restart`, or a stale flag on a session that already + # respawned fine (a race the pane epilogue and the supervisor both write + # `died`/`stopped` into) can never be admitted again to clear it. But its + # pane is a post-mortem shell doing no real work, so unlike a genuinely + # running session it must not cost anyone ELSE a slot: before this fix a + # died session's pane counted as real, running capacity forever, and + # enough of them stalled every OTHER pending session too, with nothing to + # clear it but `agent-box-session rm` (issue #523). + died = {name for name, entry in sessions.items() + if isinstance(entry, dict) and entry.get("died") is not None} + used = (live | pending) - died targets = set(targets) if spawning: - available = max(0, limit - len(live)) + available = max(0, limit - len(live - died)) admitted = live | set(sorted(pending - live)[:available]) allowed = targets <= admitted else: @@ -2127,10 +2139,22 @@ def capacity_check(sessions, targets=(), spawning=False, live=None, limit=None): raise SessionCapacityError("Cannot check session capacity: %s" % exc) from exc pending = {name for name, entry in sessions.items() if isinstance(entry, dict) and entry.get("stopped") is not True} - used = live | pending + # A crash is flagged `died`, not `stopped` (issue #516), so a died entry + # stays in `pending` -- it must remain its own candidate for revival by + # `agent-box-session restart`, or a stale flag on a session that already + # respawned fine (a race the pane epilogue and the supervisor both write + # `died`/`stopped` into) can never be admitted again to clear it. But its + # pane is a post-mortem shell doing no real work, so unlike a genuinely + # running session it must not cost anyone ELSE a slot: before this fix a + # died session's pane counted as real, running capacity forever, and + # enough of them stalled every OTHER pending session too, with nothing to + # clear it but `agent-box-session rm` (issue #523). + died = {name for name, entry in sessions.items() + if isinstance(entry, dict) and entry.get("died") is not None} + used = (live | pending) - died targets = set(targets) if spawning: - available = max(0, limit - len(live)) + available = max(0, limit - len(live - died)) admitted = live | set(sorted(pending - live)[:available]) allowed = targets <= admitted else: diff --git a/modules/src/lib/session-capacity.py b/modules/src/lib/session-capacity.py index 892cb466..872f0324 100644 --- a/modules/src/lib/session-capacity.py +++ b/modules/src/lib/session-capacity.py @@ -59,10 +59,22 @@ def capacity_check(sessions, targets=(), spawning=False, live=None, limit=None): raise SessionCapacityError("Cannot check session capacity: %s" % exc) from exc pending = {name for name, entry in sessions.items() if isinstance(entry, dict) and entry.get("stopped") is not True} - used = live | pending + # A crash is flagged `died`, not `stopped` (issue #516), so a died entry + # stays in `pending` -- it must remain its own candidate for revival by + # `agent-box-session restart`, or a stale flag on a session that already + # respawned fine (a race the pane epilogue and the supervisor both write + # `died`/`stopped` into) can never be admitted again to clear it. But its + # pane is a post-mortem shell doing no real work, so unlike a genuinely + # running session it must not cost anyone ELSE a slot: before this fix a + # died session's pane counted as real, running capacity forever, and + # enough of them stalled every OTHER pending session too, with nothing to + # clear it but `agent-box-session rm` (issue #523). + died = {name for name, entry in sessions.items() + if isinstance(entry, dict) and entry.get("died") is not None} + used = (live | pending) - died targets = set(targets) if spawning: - available = max(0, limit - len(live)) + available = max(0, limit - len(live - died)) admitted = live | set(sorted(pending - live)[:available]) allowed = targets <= admitted else: diff --git a/tests/golden/vm/payloads/agent-box-session-capacity/bin/agent-box-session-capacity b/tests/golden/vm/payloads/agent-box-session-capacity/bin/agent-box-session-capacity index 8da2b056..94d7a730 100644 --- a/tests/golden/vm/payloads/agent-box-session-capacity/bin/agent-box-session-capacity +++ b/tests/golden/vm/payloads/agent-box-session-capacity/bin/agent-box-session-capacity @@ -60,10 +60,22 @@ def capacity_check(sessions, targets=(), spawning=False, live=None, limit=None): raise SessionCapacityError("Cannot check session capacity: %s" % exc) from exc pending = {name for name, entry in sessions.items() if isinstance(entry, dict) and entry.get("stopped") is not True} - used = live | pending + # A crash is flagged `died`, not `stopped` (issue #516), so a died entry + # stays in `pending` -- it must remain its own candidate for revival by + # `agent-box-session restart`, or a stale flag on a session that already + # respawned fine (a race the pane epilogue and the supervisor both write + # `died`/`stopped` into) can never be admitted again to clear it. But its + # pane is a post-mortem shell doing no real work, so unlike a genuinely + # running session it must not cost anyone ELSE a slot: before this fix a + # died session's pane counted as real, running capacity forever, and + # enough of them stalled every OTHER pending session too, with nothing to + # clear it but `agent-box-session rm` (issue #523). + died = {name for name, entry in sessions.items() + if isinstance(entry, dict) and entry.get("died") is not None} + used = (live | pending) - died targets = set(targets) if spawning: - available = max(0, limit - len(live)) + available = max(0, limit - len(live - died)) admitted = live | set(sorted(pending - live)[:available]) allowed = targets <= admitted else: diff --git a/tests/golden/web/payloads/agent-box-settings/bin/agent-box-settings b/tests/golden/web/payloads/agent-box-settings/bin/agent-box-settings index a633be01..f9b1a9ee 100644 --- a/tests/golden/web/payloads/agent-box-settings/bin/agent-box-settings +++ b/tests/golden/web/payloads/agent-box-settings/bin/agent-box-settings @@ -391,10 +391,22 @@ def capacity_check(sessions, targets=(), spawning=False, live=None, limit=None): raise SessionCapacityError("Cannot check session capacity: %s" % exc) from exc pending = {name for name, entry in sessions.items() if isinstance(entry, dict) and entry.get("stopped") is not True} - used = live | pending + # A crash is flagged `died`, not `stopped` (issue #516), so a died entry + # stays in `pending` -- it must remain its own candidate for revival by + # `agent-box-session restart`, or a stale flag on a session that already + # respawned fine (a race the pane epilogue and the supervisor both write + # `died`/`stopped` into) can never be admitted again to clear it. But its + # pane is a post-mortem shell doing no real work, so unlike a genuinely + # running session it must not cost anyone ELSE a slot: before this fix a + # died session's pane counted as real, running capacity forever, and + # enough of them stalled every OTHER pending session too, with nothing to + # clear it but `agent-box-session rm` (issue #523). + died = {name for name, entry in sessions.items() + if isinstance(entry, dict) and entry.get("died") is not None} + used = (live | pending) - died targets = set(targets) if spawning: - available = max(0, limit - len(live)) + available = max(0, limit - len(live - died)) admitted = live | set(sorted(pending - live)[:available]) allowed = targets <= admitted else: diff --git a/tests/test-session-capacity.py b/tests/test-session-capacity.py index 3503690d..a04631c1 100644 --- a/tests/test-session-capacity.py +++ b/tests/test-session-capacity.py @@ -37,6 +37,29 @@ def test_stopped_is_free_only_after_pane_exits(self): with self.assertRaises(capacity.SessionCapacityError): capacity.capacity_check(sessions, ["new"], live={"stopped"}, limit=1) + def test_died_never_holds_a_slot_even_with_its_pane_still_up(self): + # A crash is flagged `died`, not `stopped` (issue #516), so its pane + # lingers as a post-mortem shell tmux still reports, and its entry + # never gets `stopped` either -- before this fix that meant a died + # session held its slot for good (issue #523), unlike a stopped one + # which frees its slot once the pane actually exits. + sessions = {"crashed": {"died": 1}} + capacity.capacity_check(sessions, ["new"], live=set(), limit=1) + capacity.capacity_check(sessions, ["new"], live={"crashed"}, limit=1) + + def test_died_entry_remains_its_own_revival_candidate(self): + # A died entry must still admit ITSELF: the supervisor's own spawn + # decision for the name being revived has to see it in `pending`, or + # `agent-box-session restart` on a died session can never re-admit it + # to clear the stale flag -- a real deadlock hit in CI (issue #523): + # excluding a died name from `pending` too, not just from counting + # against everyone else, made `agent-box-session restart` on a died + # session hang forever, because the one name being spawned was never + # a candidate for its own slot. + sessions = {"revived": {"died": 99}} + capacity.capacity_check(sessions, ["revived"], spawning=True, + live=set(), limit=1) + def test_restart_does_not_need_a_second_slot(self): capacity.capacity_check({"a": {}, "b": {}}, ["a"], live={"a"}, limit=1)