Skip to content

fix(gateway): stop leaking Codex app-server sessions - #73429

Open
HaraldTR wants to merge 4 commits into
NousResearch:mainfrom
HaraldTR:fix/codex-session-cleanup
Open

fix(gateway): stop leaking Codex app-server sessions#73429
HaraldTR wants to merge 4 commits into
NousResearch:mainfrom
HaraldTR:fix/codex-session-cleanup

Conversation

@HaraldTR

@HaraldTR HaraldTR commented Jul 28, 2026

Copy link
Copy Markdown

What does this PR do?

Closes and detaches an agent-owned Codex app-server session whenever AIAgent releases its clients or performs full cleanup, and evicts one-shot webhook agents as soon as their delivery finishes.

Long-running gateways create short-lived agents for cron and webhook work. Evicted agents already released their LLM clients, child agents, and tool resources, but _codex_session was omitted from both cleanup paths. Each eviction could therefore retain a Codex app-server and its MCP subprocesses until the gateway itself restarted.

Webhook deliveries have unique session keys, but their completed agents also remained in the general agent cache. That cache permits 128 entries and can defer idle eviction for finite-reset sessions, so webhook runtimes could consume memory long before normal cache pressure cleaned them up.

The lifecycle helper detaches the session before best-effort close, making cleanup idempotent and preventing a close failure from retaining the session reference. The webhook completion hook now evicts its unique one-shot cache key after ending the persisted session; interactive chat caching is unchanged.

Related Issue

N/A — no matching issue or PR was found.

Type of Change

  • 🐛 Bug fix (non-breaking change that fixes an issue)
  • ✅ Tests (adding or improving test coverage)

Changes Made

  • Add an idempotent _close_codex_session() lifecycle helper in run_agent.py.
  • Invoke it from both soft client release and full agent close.
  • Cover both paths, repeated cleanup, and a raising close() implementation.
  • Evict each unique webhook delivery agent after its existing webhook_complete session close.
  • Cover successful, failed, and async-SessionDB webhook completion paths.

How to Test

  1. Run the Codex test matrix: all tests/**/test_*codex*.py files.
  2. Run gateway cache, shutdown, webhook, and cron lifecycle tests.
  3. In a long-running gateway using codex_app_server, complete repeated cron and webhook runs and verify no codex app-server or MCP child remains in the gateway cgroup after each one-shot run.

Checklist

Code

  • I've read the Contributing Guide.
  • My commit messages follow Conventional Commits.
  • I searched open and closed issues and PRs for duplicates.
  • My PR contains only changes related to this fix.
  • I've run pytest tests/ -q and all tests pass. The complete suite has three pre-existing macOS ACP failures on untouched main: ACP edit-approval tests classify pytest's /private/var/... temp directory as a sensitive system path.
  • I've added regression tests for the bug.
  • I've tested on macOS 26.5.2 and in a live NixOS gateway.

Documentation & Housekeeping

  • Documentation update: N/A; this is internal lifecycle cleanup with a docstring and comments.
  • cli-config.yaml.example: N/A; no config changes.
  • CONTRIBUTING.md / AGENTS.md: N/A; no architecture or workflow changes.
  • Cross-platform impact considered: cleanup uses the existing platform-neutral session API.
  • Tool descriptions/schemas: N/A; no tool behavior changes.

Test Results

  • 802 passed — every Codex-named test file across agent, transport, CLI, cron, tools, plugins, and TUI.
  • 268 passed — gateway webhook completion/cache/shutdown and cron shutdown lifecycle tests.
  • Ruff, Windows-footgun, and whitespace checks passed.
  • ACP subset: 348 passed, 3 failed; the same three failures reproduce on untouched upstream main.
  • Live NixOS acceptance: two consecutive real scheduled runs and two distinct accepted webhook deliveries completed; the gateway cgroup returned to the gateway process only after every run, with no retained Codex or MCP child.

@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 P3 Low — cosmetic, nice to have sweeper:risk-session-state Sweeper risk: may lose/corrupt/mis-associate session or context state needs-decision Awaiting maintainer decision before any implementation labels Jul 28, 2026
@alt-glitch

Copy link
Copy Markdown
Collaborator

This was generated by AI during triage.

Related to #55450 and #66678. This patch covers both cleanup paths like #55450, but detaches _codex_session before attempting close(), so a close failure cannot retain the reference. Maintainers should choose the lifecycle implementation to consolidate.

@HaraldTR HaraldTR changed the title fix(codex-runtime): close sessions on agent cleanup fix(gateway): stop leaking Codex app-server sessions Jul 28, 2026
@HaraldTR

Copy link
Copy Markdown
Author

Confirmed related to #55450 and #66678. The additional production finding here is that dynamic webhook deliveries have unique one-shot session keys but remain in the general agent cache (cap 128, with finite-reset sessions potentially retained until expiry), so lifecycle cleanup is not invoked soon enough to prevent OOM. This PR adds webhook completion eviction as the second half of the fix, plus detach-before-close semantics and regression coverage for both layers. I am happy to consolidate whichever lifecycle implementation maintainers prefer; the webhook eviction should remain paired with one of them.

@teknium1

Copy link
Copy Markdown
Contributor

Thanks for covering both the agent-owned Codex runtime and one-shot webhook cache lifecycle.

Suggested changes

  • The webhook tests currently verify only that the adapter calls the fake runner's eviction method. Please add a focused regression test for the production cache behavior: GatewayRunner._evict_cached_agent() pops the entry and asynchronously calls release_clients() (gateway/run.py:22242-22310). This is the resource-release guarantee the webhook half of the fix relies on.

The implementation ordering is sound: the gateway handler clears its running-agent slot in gateway/run.py:14943-14965 before BasePlatformAdapter invokes on_processing_complete at gateway/platforms/base.py:6269-6273, so eviction is not skipped as an active turn.

Automated hermes-sweeper review.

@teknium1 teknium1 added sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform area/sessions Session lifecycle, resume, persistence, history labels Jul 30, 2026
@alt-glitch alt-glitch added comp/gateway Gateway runner, session dispatch, delivery platform/webhook Webhook / API server sweeper:risk-message-delivery Sweeper risk: may drop, duplicate, misroute, or suppress messages labels Jul 31, 2026
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/gateway Gateway runner, session dispatch, delivery needs-decision Awaiting maintainer decision before any implementation P3 Low — cosmetic, nice to have platform/webhook Webhook / API server provider/openai OpenAI / Codex Responses API sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform sweeper:risk-message-delivery Sweeper risk: may drop, duplicate, misroute, or suppress messages 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.

3 participants