Repository navigation
fix(cron): compute the session-title fallback lazily inside its recovery block - #146940
princejain756 wants to merge 1 commit into
Conversation
…ery block The lineage fallback in _finalize_cron_session was evaluated eagerly in the for-loop tuple, outside the inner try. When _BoundedCronSessionDB was already wedged (e.g. after a title-write timeout), get_next_title_in_lineage raised there, escaped the except handler, and aborted run_job's finally before defer_agent_teardown ran — leaking the ephemeral agent's httpx clients, terminal sandboxes and MCP stdio children per fired job (the EMFILE class the block below explicitly guards against). The fallbacks are now lambdas evaluated inside the per-iteration try, so a raising lineage lookup falls through to the bare id-stamped title and the rest of finalization always completes. Fixes NousResearch#146855
alt-glitch
added
area/sessions
|
Verified at head
|
|
Reproduced the bug on Neighbouring cron session/title tests ( One thing you should know before a maintainer picks this up: it's a duplicate. #147138 (nikolas-sapa) fixes the same three lines. I ran your test against their branch: The only behavioural difference I can find: #147138 appends a final |
|
Thanks @whyyagswhy and @wsiczek for running the repro on main. Good catch on #147138 touching the same lines too. Totally fine with whichever version maintainers prefer. |
Summary
Fixes #146855 - when a cron job finishes,
_finalize_cron_sessionpersists a title before tearing the session down. The recovery fallback was built eagerly in the for-loop tuple, outside the inner try: aget_next_title_in_lineagecall on an already-wedged_BoundedCronSessionDBraised there, escaped the except handler, and abortedrun_job's finally beforedefer_agent_teardownran. Per fired job that leaks the ephemeral agent's httpx clients, terminal sandboxes and MCP stdio children (the EMFILE class the block below explicitly guards against), plus a bogus failure notice.The two fallbacks are now lambdas evaluated inside the per-iteration try, so a raising lineage lookup is caught like any other title failure and falls through to the bare id-stamped title. Finalization can no longer be interrupted by the title path.
Test plan
scripts/run_tests.sh tests/cron/test_finalize_cron_session_title.py- new regression test: all title paths + the lineage lookup raise,_finalize_cron_sessionmust complete (proven red on unfixed code -RuntimeError: session finalization disabledescapes; green after)scripts/run_tests.sh tests/cron/- 1482 pass, 0 failpython scripts/check- 13 checks ok (line-neutral change inside the over-cap file)