[CrysisDeu]

Problem / Motivation

Goal: Stop Codex background sessions (title gen, navigation, folders, suggestions) from accumulating MCP server processes that survive after the session closes.

Codex starts a per-session MCP fleet (~44 processes) from mcp_servers entries in CODEX_HOME/config.toml for every session/new. session/close evicts the ACP session from codex-acp's internal map but does not terminate those MCP children. Background sessions (kirocrew-lite) are high-churn — many per conversation at a rate the user never controls — so the fleet grows without bound until the background runtime hits its 6-hour age ceiling and killpg reclaims the whole group.

Reporter hugoncosta (codex-acp 1.11.0) observed ~855 processes / 21 GiB after 4 hours of a recurring dashboard session, cleared only by restart.

Why it matters

Any operator who selects the Codex backend and has MCP servers configured in ~/.codex/config.toml will hit this in a long-running dashboard session. The accumulation causes OOM, which kills the gateway and interrupts all active sessions.

Not a goal

• RFC docs(rfc): decide how leaked agent runtimes are reclaimed #15748 step B (persisted spawn record for AcpRuntime): durability for the reconciler after a gateway OOM restart.
• RFC docs(rfc): decide how leaked agent runtimes are reclaimed #15748 step C (default reclaim on non-cgroup Linux): automatic cleanup of any remaining orphaned processes.
• Stamping AcpClient._spawn with KIROCREW_SPAWN_INSTANCE / KIROCREW_SPAWN_HOME.
• Foreground codex sessions: they accumulate at the rate a person opens chats and retain full CODEX_HOME access unchanged.
• Project-level .codex/config.toml overrides: names are read once from the user-level CODEX_HOME/config.toml at spawn time; servers added by a project config are a known residual (see code comment).

Intended? / Repro

Intended? No. On main, CodexHarness.apply_spawn_env (src/kiro_crew/acp/harness/codex.py:354) passes the operator's CODEX_HOME through unchanged for every runtime, including the background agent kirocrew-lite (src/kiro_crew/session_parts/policy.py:150). Nothing on main limits the MCP fleet a background session/new starts, and the module docstring itself names the MCP fleet in CODEX_HOME as the memory lever. No RFC or maintainer comment says background sessions should start the user's MCP servers; RFC #15748 treats the leaked processes as a defect to reclaim.

Repro. From the reporter on #13324 (codex-acp 1.11.0): a recurring dashboard session on the Codex backend with MCP servers in ~/.codex/config.toml grew to ~855 processes / 21 GiB in 4 hours, cleared only by a restart. The background runtime opens a new codex session per title/navigation/suggestion turn, each starts the configured MCP servers, and session/close does not reap them. This host has no codex-acp install, so the live before/after count below is still pending (see Manual verification).

What changed (motivation → approach → change)

At background-runtime spawn, resolve_spawn reads CODEX_HOME/config.toml via tomllib (stdlib ≥ 3.11; pyproject.toml requires ≥ 3.12) through asyncio.to_thread, so a slow home directory never blocks the event loop. It enumerates the mcp_servers keys and stores a CODEX_CONFIG JSON override in self._bg_codex_config. A missing, unreadable or malformed file yields no override. apply_spawn_env — which runs after agent.env is merged in — injects this as CODEX_CONFIG only if no CODEX_CONFIG already exists in the assembled env. Any existing CODEX_CONFIG (from os.environ, agent.env, or cron extra_env) takes precedence; the override is then skipped and a warning is logged saying the MCP servers will start for every background session.

CODEX_CONFIG is documented in the codex-acp README as "JSON object merged into the Codex session config". McpServerConfig.enabled is documented in codex-rs/config/src/mcp_types.rs line 234: "When false, Codex skips initializing this MCP server." The TOML deep-merge semantics are verified from codex-rs/config/src/merge.rs.

Implementation follows the opencode.py / pi.py pattern: compute in resolve_spawn, consume in apply_spawn_env via self._bg_codex_config. No changes to SpawnPlan, launch.py precedence, or CODEX_HOME.

Backwards compatibility

Compatible: no change to foreground sessions, SpawnPlan, launch.py, or CODEX_HOME. Background sessions that previously had MCP servers from codex's own config will no longer have them unless the operator sets CODEX_CONFIG (in which case the operator's value wins).

Removes nothing: no public API or config key is removed.

Tests

Added to test/test_codex_sqlite_home.py:

• test_read_mcp_server_names_section_headers — standard [mcp_servers.<name>] headers extracted.
• test_read_mcp_server_names_dotted_key — dotted top-level keys extracted.
• test_read_mcp_server_names_dotted_in_name — a quoted server name containing a dot stays one name.
• test_read_mcp_server_names_malformed_toml — malformed TOML → [].
• test_read_mcp_server_names_empty_config / _missing_file — empty/absent → [].
• test_background_spawn_env_sets_codex_config_with_servers — after resolve_spawn + apply_spawn_env, env carries CODEX_CONFIG with enabled:false per server.
• test_background_spawn_env_does_not_overwrite_existing_codex_config — operator CODEX_CONFIG preserved and the skip is logged.
• test_foreground_apply_spawn_env_does_not_set_codex_config — foreground env unchanged.

Reader: test/test_codex_sqlite_home.py

Manual verification

Required live check (pending owner; Design Review will clear once the result is posted):

1. Add two entries to ~/.codex/config.toml:

   [mcp_servers.srv1]
   command = "npx"
   args = ["-y", "@modelcontextprotocol/server-filesystem", "/tmp"]
   
   [mcp_servers.srv2]
   command = "python3"
   args = ["-m", "http.server", "9999"]

2. Start a fresh Kiro Crew dashboard session on the Codex backend and open one chat (so a background runtime spawns).
3. Before: count MCP children of the codex-acp process: ps --ppid $(pgrep -f codex-acp) -o pid= or pgrep -P $(pgrep -f codex-acp).
4. Trigger several background turns (rename the chat, navigate between chats, ask for suggestions).
5. After: run the same ps/pgrep command. Expected: process count stays at or near 0 (no new MCP children); title generation and navigation still work.

Screenshots / video

N/A — no user-visible UI change.

Related Issues

Fixes #13324
Refs #15748

Pattern harvest

Rule candidate: agents-md
Pattern: to suppress a shared-process ACP backend's own per-session resource spawn, use the backend's documented config-override env var (CODEX_CONFIG) with per-entry enabled:false; compute the value off-loop in resolve_spawn, store in self, inject in apply_spawn_env after agent.env is assembled (following opencode.py/pi.py).

Checklist

• At most two commits (one is the norm)
• Existing tests pass and new tests added
• Self-review completed
• Documentation updated
• No secrets, credentials, or internal references in the diff

[github-actions]

UX Review (Opus 5.5) — ⏭️ skipped

Revision 48b195481735cf2ed66e307fa9a1cd698bf6b21f touches no user-facing surface (no changes under website/ or committed screenshots), so the UX review was skipped. Does not block merge.

[github-actions]

First Principles Review (Opus 5.5) — ✅ PASS

Premise-level review of 48b195481735cf2ed66e307fa9a1cd698bf6b21f — why this exists and whether the shipped surface is the smallest honest version. Updated in place on each push. A BLOCK verdict blocks PR readiness; PASS does not.

First-Principles-Verdict: PASS

Nothing blocks. The one open premise is that codex-acp merges CODEX_CONFIG the way the author says, and the live check for that is still marked pending.

Notes

• Item 2 — undeclared: the description says it "enumerates the mcp_servers keys", but the diff also writes command = "true" / url = "http://127.0.0.1/" placeholders into each entry. Whether every background thread/start keeps parsing depends on codex's TryFrom<RawMcpServerConfig> rule. The tests check that rule only against _codex_accepts, a copy of it written in the test file. If the copy is wrong, Codex background turns fail for operators with MCP servers configured. That degrades the product rather than stopping it, which is why this is a note and not a blocker. Run step 5 of Manual verification before merge.
• Item 1 — symptom-level, cause out of scope: the root cause is codex-acp's session/close not reaping MCP children. Foreground codex sessions keep leaking, and the description says so under ## Not a goal.

What this change ships

Inventory (5 items) — 3 justified

Intent: stop Codex background helpers (titles, navigation, suggestions) from piling up MCP processes until the gateway runs out of memory. FIX.

1. On Codex, background kirocrew-lite sessions no longer start the operator's configured MCP servers — justified
2. Each disable entry carries a placeholder command/url so it still parses after config edits — undeclared, rests on an unverified codex parse rule
3. An operator-set CODEX_CONFIG takes precedence, and the skipped override is logged as a warning — justified
4. Foreground Codex sessions are unchanged — justified
5. sqlite_home_set local hoisted in resolve_spawn — rides along, harmless

[FIRST-PRINCIPLES-REVIEWED] 48b1954

Description read

[DESCRIPTION-READ] af051d4beba0d5698ab719fd78d274114ae2fc58047b8840ba547115fc46480e

That sha256 names the PR description this verdict read. Recompute it with .github/scripts/pr-description-capture.sh; if it differs, the description changed after this verdict was formed and any finding drawn from the description is unproven.

[github-actions]

Design Review (Opus 5.5) — ✅ PASS

Design-level review of 48b195481735cf2ed66e307fa9a1cd698bf6b21f — updated in place on each push. A BLOCK verdict blocks PR readiness; PASS does not.

Design-Verdict: PASS

No design problem blocks this: a spawn-time CODEX_CONFIG override, scoped to the background agent, with operator precedence and no persisted state, is a proportionate fix that one revert undoes.

Notes

• The fix is a no-op if the background session/new carries a non-empty mcpServers array: runtime_session_create.py can add control-plane, stub or label elements even though kirocrew-lite's spec has mcpServers: {}. Your own comment says such an array "replaces the whole mcp_servers table". The pending live process count is the evidence that rules this out; post it before merge.
• A server that switches between command and url under the same name after spawn makes codex reject the entry ("invalid transport") on every background thread/start until the runtime recycles. That breaks title generation and navigation for the Codex backend, not just the cleanup. Rebuilding the override from a re-read when the runtime respawns would remove this, without adding a watcher.

[DESIGN-REVIEWED] 48b1954

Description read

[DESCRIPTION-READ] e8371e59fe7a283a43b37117f0800c33ffa0ce4b2f0dfb87a44b215b76d68310

That sha256 names everything this verdict read: the PR description, plus the 1 evidence file(s) the workflow collected from it and handed this review. It covers that evidence because the media strip replaces every attachment URL with the same placeholder, so an edit swapping one attachment for another would leave the description's captured text identical while what this review judged changed. Recompute it with .github/scripts/pr-description-capture.sh over the same inputs; if it differs, the description or that evidence changed after this verdict was formed and any finding drawn from either is unproven.

[github-actions]

GPT 6.1 Review — ✅ no blocking findings

GPT 6.1 completed its review of 48b195481735cf2ed66e307fa9a1cd698bf6b21f and found no blocking issues.

This comment is updated in place on each push.

Review details

No findings.
[GPT-REVIEWED] 48b1954

False positive or not applicable? A repository writer can comment:
/ai-review override gpt 48b195481735cf2ed66e307fa9a1cd698bf6b21f false-positive: <reason, 20 to 500 characters>

[github-actions]

Opus 5.5 Review — ✅ no blocking findings

Reviewed 48b195481735cf2ed66e307fa9a1cd698bf6b21f — this comment is updated in place on each push.

Review details

No findings.

[OPUS-REVIEWED] 48b1954

Verdict parsed from the review's SHA-scoped output markers for commit 48b195481735cf2ed66e307fa9a1cd698bf6b21f.

False positive or not applicable? A repository writer can comment:
/ai-review override fable 48b195481735cf2ed66e307fa9a1cd698bf6b21f false-positive: <reason, 20 to 500 characters>

[Pearcekieser]

Live evidence for the open question in the Design and First Principles reviews, measured with codex-acp main (2.2.2, bundled Codex 0.160.1) against a local fake model provider and 2 recording MCP servers in a throwaway CODEX_HOME.

On thread/start, config: {"mcp_servers": {"<name>": {"enabled": false}}} starts 0 servers, and so does the dotted mcp_servers.<name>.enabled form. {"mcp_servers": {}} changes nothing and still starts 2. A name that no config layer defines fails the whole thread/start with invalid transport in mcp_servers.<name>, so the names must come from the config codex actually loads.

This PR does not reach codex-acp's title threads. CODEX_CONFIG is merged only in createSessionConfig, and TitleGenerator calls threadStart({cwd, ephemeral: true}) with no config. Every background session's first turn therefore still starts one title thread with the full config.toml fleet. That half is fixed upstream: agentclientprotocol/codex-acp#629 releases the thread, and agentclientprotocol/codex-acp#661 disables its MCP servers. With both, 3 sessions gave 0 title-thread MCP starts.

[CrysisDeu]

GPT F1 (blocking, no-blocking-call-on-event-loop, codex.py config read on the event loop): fixed on head 807250a. resolve_spawn now calls await asyncio.to_thread(_read_codex_mcp_server_names, ...), and the wrong "already on the spawn worker" comment is corrected. The non-blocking finding is fixed too: the docstring and comment no longer claim a regex fallback, malformed TOML returns [] explicitly, and a test covers it. The same head also fixes the failing dotted-name test (now a quoted key) and the black failures.

[CrysisDeu]

Design Review BLOCK (provider behaviour unverified), status on head 807250a: still open. It needs a live codex-acp 1.11.0 run, which the code cannot provide. This host has no codex-acp install, so the 5-step check in the description has to be run by the owner on a machine with Codex configured.

The Watch item "skip is silent" is addressed on this head: when CODEX_CONFIG is already set, apply_spawn_env logs a warning that the MCP servers will start for every background session. A test asserts the warning. The upstream-issue Watch item is unchanged.

[iamwhatever]

Hold at 807250a until the provider premise is verified.

The code does what it says: a background Codex spawn reads the mcp_servers names from CODEX_HOME/config.toml off the event loop and injects CODEX_CONFIG with enabled: false for each, unless the operator already set one (codex.py#L350, L429). Foreground sessions are unchanged, checks are green and the branch merges cleanly.

What is unresolved: the Design Review blocker, which the author's latest reply says is still open. If codex-acp ignores CODEX_CONFIG, the leak continues; if it rejects an entry that only carries enabled, every background spawn fails and title generation, navigation and suggestions stop on that backend. The live check in the thread covers codex-acp 2.2.2 and supports the premise there. It does not cover 1.11.0, the version in #13324.

What would clear it: the Manual verification steps run on codex-acp 1.11.x (or a stated minimum version that is known to work), with before/after MCP child counts and title generation still working.

Non-blocking notes: the leak still reaches sub-agent sessions, pooled reuse and the entitlement probe, and title threads stay covered only upstream; naming those in Not a goal would help. Background sessions losing the operator's MCP servers is better written as a Breaking: line.

— crew-pr-reviewer (r138)

[bolichen97]

[operator: bolichen97] Requesting changes for one breaking case on the codex background runtime.

Stale server names make every background session fail (src/kiro_crew/acp/harness/codex.py, the _read_codex_mcp_server_names call in resolve_spawn and the CODEX_CONFIG injection in apply_spawn_env).
The server names are read once, when the background runtime spawns, and frozen into CODEX_CONFIG. codex re-reads config.toml on every session/new, and the runtime can live for hours. If the user removes or renames an [mcp_servers.<name>] entry in that window, the frozen {"<name>": {"enabled": false}} no longer merges onto a real entry. It becomes a server entry with neither command nor url. codex's TryFrom<RawMcpServerConfig> rejects that with invalid transport (codex-rs/config/src/mcp_types.rs), so the config fails to load. Every background session/new (titles, suggestions, navigation) then fails until the runtime recycles. On main the same edit is harmless.
A related case: the names come from ctx.environ's CODEX_HOME, while agent.env is merged later and can point the child at a different CODEX_HOME, which yields the same orphan entries.

Possible fixes: use a global "skip MCP startup" switch if codex or codex-acp has one; or recycle the background runtime when config.toml changes; or prove (with a test or a live run) that codex tolerates an override for a missing server. Please add a test for the "server removed after spawn" case either way.

Non-blocking:

• When CODEX_CONFIG is already set, the fix is skipped with only a warning. JSON-merging the disable map into the existing value would keep it working.
• codex-acp replaces mcp_servers when session/new carries mcpServers. If the background agent ever mounts one, the override is lost. Worth a comment or test.
• import tomllib can move to module scope.
• The PR body's manual before/after process count is still pending; a live run would confirm enabled: false via CODEX_CONFIG actually stops the fleet.

The rest looks right: the override deep-merges over config.toml without touching the user's file or auth, it is scoped to the background agent only, and an existing CODEX_CONFIG is preserved.

AI-assisted, approved by bolichen97

[CrysisDeu]

@bolichen97 thanks, the stale-name case is real. I checked it against the adapter: @agentclientprotocol/codex-acp 2.5.0 reads CODEX_CONFIG once in startAcpServer and merges it into every thread/start config in createSessionConfig, and an orphan entry fails with invalid transport (TryFrom<RawMcpServerConfig> in codex-rs mcp_types.rs, also seen live in Pearcekieser's run above).

Fixed on head 48b1954 (rebased on main, second commit; re-pushed once because CI did not start on 56f5f36):

• Each disable entry now also carries an inert placeholder for the transport key the server had at spawn: {"enabled": false, "command": "true"} for stdio, {"enabled": false, "url": "http://127.0.0.1/"} for HTTP. If the server is removed or renamed after spawn, or the names came from a CODEX_HOME that agent.env later replaces, the orphan still parses as a disabled server instead of failing every background session/new. Over a live entry it only replaces the command or URL of a server that enabled = false already stops. A server with neither key is skipped, since codex rejects it on main too.
• New test test_override_still_parses_after_its_server_is_removed: it builds the override from a config with one stdio and one HTTP server, then checks codex's transport rule on the merged entry with the server present and with it removed.
• import tomllib is at module scope now.

Why not the other fixes: codex has no global "skip MCP startup" switch (no such key in the config schema), and recycling the background runtime on a config.toml change would add a new staleness seam to the shared runtime, which is a bigger change than this bug needs.

Residuals, noted in the code comment: a server renamed after spawn starts its fleet under the new name until the runtime recycles (same as main). A server that switches between command and url under the same name conflicts with the placeholder until the runtime recycles. A session/new that carries mcpServers replaces the whole mcp_servers table in createSessionConfig, which drops the override for that session; the comment says so.

Not changed: merging into an existing CODEX_CONFIG (still skip + warning) and the live before/after process count. This host has no codex install, so the live check is still open.

[iamwhatever]

HOLD at 48b195481735cf2ed66e307fa9a1cd698bf6b21f: active change request.

@bolichen97's change request at 807250a (stale server names make every background session fail) is still active. The author's reply describes a fix on this head: each disable entry carries an inert command/url so an orphaned entry still parses (codex.py#L178-L214). The reviewer has not yet re-reviewed it. That fix is tested only against _codex_accepts, a copy of codex's transport rule in the test file (test_codex_sqlite_home.py#L199), not against a real codex.

The live check from our r138 hold is still open. Both current-head lanes name it as the evidence for two unanswered notes: that the placeholders really parse, and that a background session/new carrying a non-empty mcpServers does not cancel the override. I cannot settle either from the code. If the copy of the rule is wrong, every background Codex turn fails for operators with MCP servers configured.

What would clear it:

1. @bolichen97 re-reviews or dismisses the change request.
2. The Manual verification run on a real codex-acp: MCP child count before and after, with title generation still working, and one run with a server removed from config.toml after spawn.
3. A reply to the Design note about a server switching between command and url, or an issue for it.

The rest is sound: the override is limited to the background agent, an existing CODEX_CONFIG wins with a logged warning, and CI is green.

— crew-pr-reviewer (r146)

[bolichen97]

Approving 48b1954. I read the full diff at this head.

• A background codex runtime now gets a CODEX_CONFIG override with one enabled = false entry per mcp_servers server in config.toml, so it does not start the user's whole MCP fleet.
• Each entry keeps a placeholder for the transport key the server had (command or url), so codex still parses it if the server is removed or renamed after the spawn. A server with neither key is left out, as codex already rejects it.
• config.toml is read with stdlib tomllib; a missing, unreadable or malformed file yields no override, so nothing changes for that spawn.
• The new tests cover the override and its edge cases.

No regex, glob or case-blind matching is added. All 97 checks pass, and the Design, First Principles, Opus and GPT lanes pass on this head.