fix(sessions): a died hook-* session never holds a capacity slot (#523) - #682
Conversation
capacity_check() counted a `died` entry as `pending` (its `stopped` flag is never set, issue #516) and its lingering post-mortem-shell pane as `live`, so a crashed hook-* session held its slot forever -- only `agent-box-session rm` ever freed it. Four such corpses made every standing watch inert with nothing actually running, the same shape as the pre-#280 bug this admission policy was built to fix. Exclude any session the registry flags `died` from both `live` and `pending`, in the single shared session-capacity.py the CLI, supervisor, webhook spawner and settings daemon all admit through since #668. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0188bYSAQvRmVjNDtsgznirR
|
Warning Review limit reachedNext included review available in 41 minutes. View limit detailsLimit details: You’ve used the included review currently available. You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Review configuration: ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (5)
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (5)
Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review. 📝 WalkthroughWalkthroughThe change updates capacity checks to exclude crashed sessions from live and pending accounting. It applies the logic to standing-watch and spawning paths, updates generated payloads, and adds regression coverage for crashed sessions with live panes. ChangesSession capacity accounting
Priority: ➖ Normal Estimated code review effort: 3 (Moderate) | ~20 minutes Change: Bug fix · Severity of issue fixed: Medium Suggested reviewers: Merge Risk: ⚪ Minimal · up to Crashed sessions no longer retain capacity solely because a post-mortem pane remains present, and the updated paths are covered consistently. 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Full details: Docstring CoverageExplanation Docstring coverage is 25.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 4 functions across 2 files. (3 skipped: 3 unsupported.) ✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
writePython3Bin's flakeIgnore replaces pycodestyle's defaults, so W503 (normally off) became a build error on the `and` line break split across two continuation lines in capacity_check()'s `pending` comprehension -- caught by CI's native-checks build, not by anything runnable without Nix. Rewritten as three `if` clauses (implicitly ANDed in a comprehension), which sidesteps the rule instead of choosing a side of it. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0188bYSAQvRmVjNDtsgznirR
CI's VM (sessions) lane caught what the native tests couldn't: excluding a `died` entry from `pending` (not just from counting against everyone else) made it structurally impossible for the supervisor to ever re-admit that SAME name. `agent-box-session restart` on a died session kills its old pane and relies on the supervisor's own spawn admission to start a fresh one and clear the flag -- but with the died name absent from `pending`, it could never again appear in `admitted`, so the restart hung forever (CI: "a spawn clears a stale died flag" timed out at 120s). Keep `pending` unfiltered (a died entry stays a valid candidate for its own restart), and instead exclude `died` names only from `used` (the total charged against the limit) and from the `live` count `available` subtracts -- so a died session's corpse still doesn't cost anyone ELSE a slot, but remains admissible for itself. Added a regression test at the unit level (`test_died_entry_remains_its_own_revival_candidate`) so this deadlock shape doesn't need a 12-minute VM run to catch again. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0188bYSAQvRmVjNDtsgznirR
1 participant
Summary
capacity_check()inmodules/src/lib/session-capacity.pycounted a crashed (died, issue The web UI reports a died session as Running, so nobody notices #516) session as bothpending(its registry entry never getsstopped) andlive(its post-mortem-shell pane still shows up intmux list-sessions) - so it held its capacity slot forever. Onlyagent-box-session rmever freed it, and enough corpses stalled every standing watch on the box with nothing actually running - the same shape as the pre-The hook-session cap counts registry keys, so finished sessions permanently consume dispatch capacity #280 bug this admission policy exists to fix.diedfrom bothliveandpending. This is the single shared choke point PR fix(sessions): share capacity admission across CLI, UI and webhooks #668 routed the CLI, the supervisor, the webhook spawner and the settings daemon's auto-start through, so the fix covers all four callers at once.webhook-spawn.sh(which had its own tmux-probe cap logic); by the time I went to push, fix(sessions): share capacity admission across CLI, UI and webhooks #668 had just replaced that with the sharedsession-capacity.pyadmission library, so I re-based the fix onto the new code before pushing.Test plan
python3 tests/test-session-capacity.py- addedtest_died_never_holds_a_slot_even_with_its_pane_still_up, confirmed it fails without the fix and passes with it; all 13 tests passpython3 tests/test_agentbox.py- 149 passed, no driftpython3 bin/assemble-module.py --check-modules/agent-box.nixregenerated and up to datepython3 scripts/check_backend_parity.py- OK, no undeclared divergencepython3 tests/test-assemble-module.py- OKpython3 scripts/check_vendor.py- OKtests/golden/vm/payloads/agent-box-session-capacity/...andtests/golden/web/payloads/agent-box-settings/...) to match, verified byte-for-byte againstbin/assemble-module.py --resolvenix build .#checks.x86_64-linux.sessionsand friends - relying on CI for thoseCloses #523
🤖 Generated with Claude Code