Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
32 changes: 28 additions & 4 deletions modules/agent-box.nix

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

16 changes: 14 additions & 2 deletions modules/src/lib/session-capacity.py
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
23 changes: 23 additions & 0 deletions tests/test-session-capacity.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand Down