Skip to content

fix(codex): stop usage polls from overlapping app-server marketplace clones - #27

Merged
bernardopg merged 4 commits into
mainfrom
fix/codex-poll-marketplace-clones
Sep 8, 2026
Merged

fix(codex): stop usage polls from overlapping app-server marketplace clones#27
bernardopg merged 4 commits into
mainfrom
fix/codex-poll-marketplace-clones

Conversation

@bernardopg

Copy link
Copy Markdown
Owner

Closes #25.

Reproduction

The reporter's numbers (1,443 staging checkouts, ~118 GiB) reproduce mechanically. On this dev machine the same accumulation was already present: 14,298 git-* temp dirs + 27 marketplace-upgrade-* staging clones ≈ 3.1 GiB, with mtimes matching the widget's 120s poll cadence minute for minute (17:17→17:23 today), stopping exactly when the shell stopped.

A controlled reproduction makes it deterministic: a scratch CODEX_HOME with a marketplace backed by a local git repo, advancing HEAD between runs:

  • new upstream HEAD → +2 leaked git-* temp dirs per invocation
  • unchanged HEAD → +0 (explains the quiet periods)
  • 3 concurrent invocations on the same new HEAD → +5–6 (no internal single-flight in Codex; matches the reporter's 6→12 observation)

The chain: every adapter invocation spawned a fresh codex app-server; every such startup runs a marketplace refresh; the adapter's own timeout/teardown interrupts it, orphaning the git child; Codex then never records the upgraded revision, so the next poll starts over — unbounded growth, multiplied by concurrency and by the adapter's own retry loop.

Fix (adapter-side)

  • Single-flight: flock on the cache dir serializes backend launches; an invocation arriving mid-refresh serves the cached snapshot, or waits bounded (CODEX_LOCK_WAIT, default 8s) before a structured error.
  • Freshness gate: snapshots younger than CODEX_FRESH_TTL (default 60s) answer from cache with zero launches — widget reload bursts (popout, settings, IPC reload) no longer spawn anything.
  • Daemon/proxy backend: codex app-server daemon start (idempotent) is attempted first; polls then go through codex app-server proxy, so steady-state usage never starts a backend — and never pays its startup marketplace refresh. npm/brew/distro installs without the daemon subcommand fall back to a direct spawn; CODEX_APP_SERVER_MODE=spawn forces that path explicitly.
  • Graceful teardown: the backend exits by itself when stdin closes (verified); the adapter waits briefly for that self-exit before the SIGTERM/SIGKILL ladder, so in-flight startup work is not orphaned mid-clone by the poll that started it.

Verified: same 3-concurrent/changed-HEAD scenario that leaked +5–6 now leaks +2 — one serialized launch; the other two callers served the cache.

Cleanup of already-leaked dirs stays a documented manual step in docs/providers.md — deleting Codex's files from the adapter would be its own hazard. The remaining leak per actual launch (when an upgrade is interrupted) and the missing staging cleanup are Codex-side; the reporter's upstream links (#30620, #36093, #38770, #39421) already track them.

Testing

New tests/test-codex-usage.sh drives the adapter against a fake codex and covers: daemon preference (no bare spawn), spawn fallback when the daemon subcommand fails, explicit spawn mode, the freshness gate (zero backend calls), the in-flight lock (serve cache / structured error), and valid usage in every path.

r and others added 3 commits September 8, 2026 17:54
…clones

Every get-codex-usage invocation spawned a fresh `codex app-server`, and
every such startup runs a marketplace refresh round. When that round is
interrupted — which our own poll timeout guarantees — its git child is
orphaned mid-clone, Codex never records the upgraded revision, and the
next poll starts the whole round over. The reporter accumulated 1,443
staging checkouts (~118 GiB) this way; this dev machine had 14,298
leftover `git-*` temp dirs plus 27 staging clones (~3.1 GiB) whose
mtimes matched the widget's 120s poll cadence minute for minute.

A controlled reproduction (scratch CODEX_HOME with a marketplace backed
by a local git repo) shows two leaked temp dirs per invocation whenever
upstream differs from the recorded revision, multiplying under
concurrency; the same three concurrent calls now leak exactly one
backend's worth, because:

- the launch is single-flighted with a lock; overlapping callers serve
  the cached snapshot or wait bounded before a structured error
- snapshots younger than 60s answer from cache outright, so reload
  bursts (popout, settings, IPC reload) launch nothing
- on installs that have it, `codex app-server daemon start` (idempotent)
  is preferred and each poll is forwarded through `codex app-server
  proxy`, so steady-state usage never starts a backend at all; npm,
  brew and distro installs fall back to a direct spawn per refresh
- the backend is allowed to exit by itself on stdin close before
  escalating to SIGTERM/SIGKILL, giving in-flight startup work a chance
  to finish or abort cleanly instead of being orphaned

`CODEX_APP_SERVER_MODE=spawn` forces the spawn path; `CODEX_FRESH_TTL`
and `CODEX_LOCK_WAIT` tune the gate and the lock wait. Cleanup of
already-leaked dirs stays a documented manual step — deleting Codex's
files from the adapter would be its own hazard.

tests/test-codex-usage.sh drives the adapter against a fake codex and
covers daemon preference, the spawn fallback, the freshness gate, the
in-flight lock, and explicit spawn mode.
@github-actions github-actions Bot added documentation Improvements or additions to documentation area:providers Provider adapters (providers/) area:ci CI, workflows, tooling labels Sep 8, 2026
Three fixture interactions changed with the single-flight work:

- OUT and WEEKLY scenarios shared one implicit cache dir; consecutive
  fixtures now collide because a snapshot younger than CODEX_FRESH_TTL
  is served without launching the backend. Each scenario gets its own
  XDG_CACHE_HOME.

- The backend-failing scenario asserted two counted attempts before the
  cached fallback. With a fresh snapshot the gate serves cache before
  any backend call, so the fixture ages the snapshot past the gate
  first, which is exactly the path it means to exercise.

- The aging write must keep cached_at an integer; the adapter now also
  truncates float timestamps defensively.

Also redirects the daemon probe's stdin from /dev/null and bounds it
with timeout, so a probe can never inherit (and block on) a caller's
pipe.
@bernardopg
bernardopg merged commit 0094a53 into main Sep 8, 2026
10 checks passed
@bernardopg
bernardopg deleted the fix/codex-poll-marketplace-clones branch September 8, 2026 21:39
bernardopg added a commit that referenced this pull request Sep 8, 2026
Two 2 MiB random blobs (blob3.bin, blob4.bin) from an unrelated scratch
repository were accidentally committed on main and carried into PR #27
by a misplaced cd inside a reproduction loop. This restores the tree;
they remain in history.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:ci CI, workflows, tooling area:providers Provider adapters (providers/) documentation Improvements or additions to documentation

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[bug] Codex usage polling triggers concurrent marketplace clones and disk growth

1 participant