Skip to content

fix(agent): close _codex_session in AIAgent.close() to prevent subprocess leaks (#66671) - #66678

Closed
kyssta-exe wants to merge 1 commit into
NousResearch:mainfrom
kyssta-exe:auto-fix/issue-66671
Closed

kyssta-exe wants to merge 1 commit into
NousResearch:mainfrom
kyssta-exe:auto-fix/issue-66671

Conversation

@kyssta-exe

Copy link
Copy Markdown
Contributor

Summary

AIAgent.close() cleans up background processes, terminal sandboxes, browser daemons, child agents, HTTP clients, message history, and the SQLite session row — but never closes self._codex_session. This permanently orphans the Codex app-server subprocess tree (plus its MCP children) whenever a Codex-route session is closed via session.close or any other path that tears down the agent.

Fix

Add a guarded, idempotent _codex_session close step to AIAgent.close(), matching the existing crash-path cleanup in agent/codex_runtime.py. CodexAppServerSession.close() is already guarded by self._closed, so repeated calls are safe.

Testing

  • All 40 existing Codex-related tests pass (5 persistence + 29 integration + 6 compaction).
  • Verified syntax with python3 -c "import ast; ast.parse(...)".

Fixes #66671

…cess leaks (NousResearch#66671)

AIAgent.close() cleans up background processes, terminal sandboxes,
browser daemons, child agents, HTTP clients, message history, and the
SQLite session row — but never closes self._codex_session. This orphans
the Codex app-server subprocess tree plus its MCP children on every
closed Codex-route session.

Fix: add a guarded, idempotent _codex_session close step to the close()
method, mirroring the existing crash-path cleanup in codex_runtime.py.
CodexAppServerSession.close() is already guarded by self._closed, so
calling it multiple times is safe.
@alt-glitch alt-glitch added type/bug Something isn't working comp/agent Core agent runtime: loop, agent_init, prompt builder, context-compression, responses endpoint provider/openai OpenAI / Codex Responses API codex sweeper:risk-session-state Sweeper risk: may lose/corrupt/mis-associate session or context state P3 Low — cosmetic, nice to have labels Jul 18, 2026
@alt-glitch

Copy link
Copy Markdown
Contributor

This was generated by AI during triage.

Related to #55450: this patch repairs only AIAgent.close(), while #55450 also covers release_clients() and interrupt retirement.

@tonydwb tonydwb left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Code Review Summary

Verdict: Approved

Clean, well-scoped fix for a real subprocess leak. The change:

  • Adds a guarded _codex_session close to AIAgent.close() (step 7, renumbers step 8)
  • Uses getattr + None check matching existing defensive patterns
  • CodexAppServerSession.close() is idempotent, so repeated calls are safe
  • Preserves comment style and numbering of surrounding code

No issues found. LGTM.


Reviewed by Hermes Agent

@teknium1

Copy link
Copy Markdown
Collaborator

Thanks for the focused lifecycle repair. The premise is confirmed on current main: run_agent.py:3603-3637 closes the HTTP client and continues teardown without closing _codex_session, while agent/codex_runtime.py:636-639 creates that session as AIAgent-owned state and agent/transports/codex_app_server.py:180-198 terminates its subprocess during close.

Problems

Suggested changes

  • Carry equivalent AIAgent.close() regression tests into this PR, asserting the Codex session is closed once, cleared, optional for non-Codex agents, and safe under repeated agent close calls.

Automated hermes-sweeper review.

@teknium1 teknium1 added the sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform label Jul 18, 2026
@kyssta-exe

Copy link
Copy Markdown
Contributor Author

Closing this PR because it has merge conflicts (mergeable=CONFLICTING). Please rebase on the latest main branch and resolve the conflicts before re-opening.

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

Labels

codex comp/agent Core agent runtime: loop, agent_init, prompt builder, context-compression, responses endpoint P3 Low — cosmetic, nice to have provider/openai OpenAI / Codex Responses API sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform 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]: session.close leaks the Codex app-server process tree — AIAgent.close never closes _codex_session

4 participants