Skip to content

fix(cron): close codex app-server session when a job run finishes - #62112

Open
Shaance wants to merge 2 commits into
NousResearch:mainfrom
Shaance:fix/cron-codex-app-server-leak
Open

Shaance wants to merge 2 commits into
NousResearch:mainfrom
Shaance:fix/cron-codex-app-server-leak

Conversation

@Shaance

@Shaance Shaance commented Jul 10, 2026 •

Copy link
Copy Markdown

What does this PR do?

Fixes a process leak when model.openai_runtime: codex_app_server is active: every cron run inside the long-lived gateway process could leave behind one codex app-server subprocess and its MCP children.

The fix now follows the cron agent's existing lifecycle boundary:

  • agent/codex_runtime.py provides an idempotent, exception-safe close_codex_session(agent) helper.
  • cron/scheduler.py invokes that helper from _teardown_cron_agent(), so cleanup happens on both inline and delivery-deferred teardown paths.
  • agent/transports/codex_app_server_session.py retains stable client/thread references during run_turn(), allowing inactivity-timeout teardown to close the transport concurrently without a NoneType race.
  • A closed transport racing turn/interrupt is treated as non-fatal because the turn is already stopping.

Related Issue

Fixes #62101

Why

Cron constructs a fresh AIAgent per run. The Codex runtime lazily attaches a CodexAppServerSession, but AIAgent.close() does not currently own that session. In a one-shot CLI process, parent exit masks the problem; in the long-lived gateway, the client reader threads keep the subprocess and its pipes alive indefinitely.

Cleanup originally landed immediately after ThreadPoolExecutor.shutdown(wait=False). Review correctly identified that the worker can still be inside run_turn() at that point. This revision moves cleanup to the centralized cron teardown path and makes the active turn safe when close happens concurrently.

Validation

  • pytest -o addopts='' tests/agent/test_codex_runtime_close.py tests/agent/transports/test_codex_app_server_session.py tests/cron/test_run_one_job.py -q — 79 passed
  • pytest -o addopts='' tests/cron/test_cron_inactivity_timeout.py tests/cron/test_codex_execution_paths.py -q — 13 passed
  • python -m py_compile for all changed production and test modules — passed
  • Mutation check: temporarily restoring the unsafe self._client.is_alive() dereference made the new concurrency regression fail with the original AttributeError; restoring the fix returned the suite to green.

The focused regressions cover:

  • session close and reference clearing
  • no-op cleanup without a session
  • cleanup despite a session close error
  • close racing an active run_turn()
  • close racing turn/interrupt
  • centralized scheduler teardown, including when agent.close() fails

Manual verification

With provider: openai-codex and openai_runtime: codex_app_server, a triggered cron run returns the gateway's Codex child-process count to baseline after delivery/teardown instead of leaking one app-server per run.

Checklist

  • Bug fix is scoped to Codex app-server cron lifecycle cleanup
  • Cleanup uses the existing centralized cron teardown path
  • Concurrent-close behavior has regression coverage
  • Focused agent/session/cron tests pass
  • Broader cron inactivity and Codex execution-path tests pass
  • Tested on macOS arm64 with Python 3.11

Cron jobs build a fresh AIAgent per run; on the codex_app_server runtime
the agent lazily spawns a codex app-server subprocess that nothing ever
closed. One-shot CLI runs mask this (process exit closes the child's
stdin), but cron runs inside the long-lived gateway process, and the
client's reader threads pin it against GC — so every scheduled run
leaked one app-server process, indefinitely.

Add close_codex_session() and call it from the cron runner's finally.
On the inactivity-timeout path this also kills the hung subprocess,
matching the intended caught-and-killed semantics.

Fixes NousResearch#62101

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01GBqupVzebanvnWrdYZXu1K
@alt-glitch alt-glitch added type/bug Something isn't working P3 Low — cosmetic, nice to have comp/agent Core agent runtime: loop, agent_init, prompt builder, context-compression, responses endpoint comp/cron Cron scheduler and job management codex duplicate This issue or pull request already exists labels Jul 10, 2026
@alt-glitch

Copy link
Copy Markdown
Contributor

This was generated by AI during triage.

Duplicate of #62105 — same fix for #62101 (cron per-run codex app-server subprocess leak): both add an idempotent close_codex_session(agent) helper in agent/codex_runtime.py and call it from cron run_job()'s finally block. #62105 was created first (14:01:10Z vs 14:11:28Z), so it is canonical. Related: #62101 (target issue), #55450 (broader run_agent.py session close, same family).

@teknium1 teknium1 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for tracing the missing cron-owned Codex session cleanup; current main does leave _codex_session outside AIAgent.close() (run_agent.py:3463-3544) while cron creates a fresh agent per run (cron/scheduler.py:3044-3075).

Problems

  • The proposed cron/scheduler.py:3144 close can run while the worker is still active: the pool is stopped with wait=False (cron/scheduler.py:3133-3134) and agent.interrupt() follows later (cron/scheduler.py:3157-3159). CodexAppServerSession.close() clears self._client (agent/transports/codex_app_server_session.py:300-310), but the active run_turn() loop reads self._client.is_alive() (agent/transports/codex_app_server_session.py:455-465). This is a close-vs-turn race.

Suggested changes

  • Make run_turn() safe against concurrent close, then route cleanup through _teardown_cron_agent() (cron/scheduler.py:3324-3346), which already covers both inline and deferred cron teardown.
  • Add a scheduler-path regression; the added tests currently validate only the standalone helper.

Automated hermes-sweeper review.

Comment thread cron/scheduler.py Outdated
@teknium1 teknium1 added sweeper:risk-session-state Sweeper risk: may lose/corrupt/mis-associate session or context state sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform labels Jul 11, 2026

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/sessions Session lifecycle, resume, persistence, history codex comp/agent Core agent runtime: loop, agent_init, prompt builder, context-compression, responses endpoint comp/cron Cron scheduler and job management duplicate This issue or pull request already exists P3 Low — cosmetic, nice to have sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades sweeper:risk-session-state Sweeper risk: may lose/corrupt/mis-associate session or context state type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: cron jobs leak one codex app-server subprocess per run inside the gateway (codex_app_server runtime)

3 participants