From f583ae9b6b9fea9dec2b0c380871d4e629ee97aa Mon Sep 17 00:00:00 2001 From: jun Date: Wed, 26 Aug 2026 23:14:21 +0900 Subject: [PATCH] fix(codex): restart the npm wrapper that supervises the app-server MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit On Linux `ocx sync --restart-codex` kept reporting a survivor: Codex app-server PID(s) still running after SIGTERM: 1782906. An npm-installed Codex runs as a PAIR - `node /usr/local/bin/codex app-server` and the vendored native binary that wrapper spawns. Running listCodexAppServerProcesses() live on the affected host returned five matches and every one was a child; not a single `node …/codex app-server` supervisor was in the list. SIGTERM went to the child while its parent kept holding the socket, which is precisely what the message said. isCodeModeHostProcess already accepted an interpreter-then-target pair. The plain app-server path never got the same treatment, and that asymmetry was the whole defect: isInterpreterToken existed and was called exactly once. Deliberately narrow. The codex-shaped token has to be immediately next - skipping interpreter flags to reach it would read the VALUE of `node --require codex app-server worker.js` as the entrypoint. Dropping only the interpreter and re-running the ordinary scan is what keeps `node exec 'hi'` unmatched, exactly like `codex exec 'hi'`. Not touched: the Unix SIGTERM-without-SIGKILL policy. A restart click does not consent to a hard kill, and the survivors are reported instead; widening the matcher fixes the miss without moving that boundary. The regression was driven red against the old matcher first. MSGEOF && git log --oneline -1 --- .../000_repro_and_root_cause.md | 128 ++++++++++++++++++ src/codex/app-server-processes.ts | 17 +++ tests/codex-app-server-processes.test.ts | 24 ++++ 3 files changed, 169 insertions(+) create mode 100644 devlog/_plan/260826_restart_codex_linux_survivor/000_repro_and_root_cause.md diff --git a/devlog/_plan/260826_restart_codex_linux_survivor/000_repro_and_root_cause.md b/devlog/_plan/260826_restart_codex_linux_survivor/000_repro_and_root_cause.md new file mode 100644 index 00000000000..d4be777cf3c --- /dev/null +++ b/devlog/_plan/260826_restart_codex_linux_survivor/000_repro_and_root_cause.md @@ -0,0 +1,128 @@ +# 000 — `--restart-codex` leaves an app-server alive on Linux + +## The report + +``` +Stopping Codex app-server process(es): 1782906, 1783602, 1783863 (active turns may be interrupted). +Stopped Codex app-server PID(s): 1783602, 1783863 +Codex app-server PID(s) still running after SIGTERM: 1782906. Stop them manually if the model list stays stale. +``` + +Host: `cli-jaw-server` (reachable as `clisu-oracle`), aarch64 Ubuntu, Codex 0.142.5 +installed through npm, opencodex v2.32.1-preview.20260825 from a source checkout. + +## What is actually running there + +`ps -eo pid,ppid,args` on the host, trimmed to the app-server tree: + +``` +1884131 1 node /usr/local/bin/codex -c features.code_mode_host=true app-server --listen unix:// +1884188 1884131 …/codex-linux-arm64/vendor/aarch64-unknown-linux-musl/bin/codex -c … app-server --listen unix:// +1884276 1884271 node /usr/local/bin/codex app-server proxy +1884814 1884276 …/codex-linux-arm64/vendor/…/bin/codex app-server proxy +1933835 1933830 node /usr/local/bin/codex app-server proxy +1933985 1933835 …/codex-linux-arm64/vendor/…/bin/codex app-server proxy +``` + +Every app-server is a **pair**: an npm wrapper running under `node`, and the vendored +native binary it spawns as its child. + +## The defect, measured rather than inferred + +Running the shipped matcher live on that host: + +``` +$ bun -e 'import {listCodexAppServerProcesses} from "./src/codex/app-server-processes.ts"; …' +MATCHED: 5 + 1884188 …/codex-linux-arm64/vendor/aarch64-unknown-linux-musl/bin/codex … + 1884814 …/codex-linux-arm64/vendor/…/bin/codex app-server proxy + 1933985 …/codex-linux-arm64/vendor/…/bin/codex app-server proxy + 1945058 … + 1945383 … +``` + +Five matches, **all of them children**. Not one `node /usr/local/bin/codex app-server` +parent is in the list. So `--restart-codex` signals the child and leaves its supervisor +untouched; the supervisor is still holding the socket, and the user is told a PID +survived SIGTERM. + +The cause is in `isCodexAppServerCommandLine` (`src/codex/app-server-processes.ts:249`): + +```ts +if (isCodeModeHostProcess(tokens)) return true; // handles `node /codex-code-mode-host` +if (!isCodexExecutableToken(tokens[0]!)) return false; // <- `node` fails here +``` + +`isCodeModeHostProcess` already accepts an interpreter-then-target pair +(`isInterpreterToken(tokens[0]) && isCodeModeHostToken(tokens[1])`, line 240). The plain +app-server path never got the same treatment. `isInterpreterToken` exists and is used +exactly once. **The asymmetry is the bug** — not the SIGTERM policy, which is working as +designed. + +## Scope boundary + +IN + +- `isCodexAppServerCommandLine`: accept ` … app-server`. +- A regression in `tests/codex-app-server-processes.test.ts`, driven red first. +- This devlog unit. + +OUT + +- **Escalating to SIGKILL on Unix.** The code says so at line 1027 and it is right: a + restart click does not consent to a hard kill, and survivors are reported instead. + Widening the matcher fixes the miss without touching that consent boundary. +- Killing by process *tree*. The parent and child are matched independently once the + matcher is correct; a `/T`-style sweep is Windows-only behavior for a different reason. +- The `lidge-gemma` 502 and the `cursor` catalog-drop lines in the same log. Different + subsystems, not this defect. + +## The change + +```ts +// after: if (isCodeModeHostProcess(tokens)) return true; +if (isInterpreterToken(tokens[0]!) && tokens.length > 1 && isCodexExecutableToken(tokens[1]!)) { + tokens = tokens.slice(1); +} +if (!isCodexExecutableToken(tokens[0]!)) return false; +``` + +Dropping the interpreter token and re-running the existing scan is what keeps the +subcommand walk (`advancePastCodexGlobalOption`, then require `app-server`) intact — so +`node /usr/local/bin/codex exec 'hello'` stays unmatched for the same reason +`codex exec 'hello'` does. + +## The guard that must not regress + +`tests/codex-app-server-processes.test.ts:426` asserts: + +```ts +expect(isCodexAppServerCommandLine("node worker.js codex app-server")).toBe(false); +``` + +That must stay false. The difference is **position**: `worker.js` is not a codex +executable, so `tokens[1]` fails `isCodexExecutableToken` and the pair never forms. +Only `node app-server` matches. The same reasoning protects +line 428's `node worker.js codex-code-mode-host`. + +## Accept criteria + +| # | Criterion | Evidence | +|---|---|---| +| 1 | The npm wrapper shape matches | `isCodexAppServerCommandLine("node /usr/local/bin/codex app-server proxy")` is true | +| 2 | The later-argument guard still rejects | `node worker.js codex app-server` stays false | +| 3 | The regression is protective | it fails against the current matcher before the fix lands | +| 4 | Nothing else in the file regresses | `bun test ./tests/codex-app-server-processes.test.ts` green | +| 5 | Types hold | `bun x tsc --noEmit` clean | +| 6 | Real host, real processes | live probe on `clisu-oracle` lists the `node …/codex app-server` parents | +| 7 | The reported symptom is gone | `ocx sync --restart-codex` there reports no survivors | + +## Loop spec + +- Archetype: spec-satisfaction repair. The verifier is the regression plus the live probe. +- Stop condition: criteria 1-7 hold and the PR is open with CI green. +- Write scope: one source file, one test file, this devlog unit, one branch, one PR. + No writes to `dev` or `main`; no merge. +- Escalation: if the parents turn out to be excluded deliberately, that is `NOOP` with + evidence rather than a forced change. + diff --git a/src/codex/app-server-processes.ts b/src/codex/app-server-processes.ts index 62a149e42fa..fbce4282786 100644 --- a/src/codex/app-server-processes.ts +++ b/src/codex/app-server-processes.ts @@ -260,6 +260,23 @@ export function isCodexAppServerCommandLine(commandLine: string, executable?: st } if (tokens.length === 0) return false; if (isCodeModeHostProcess(tokens)) return true; + // An npm-installed Codex runs as a PAIR: `node /usr/local/bin/codex app-server` and the + // vendored native binary that wrapper spawns. Only the child used to match, so + // `--restart-codex` signalled the child while its supervisor kept holding the socket - + // which is what "PID(s) still running after SIGTERM" was reporting on Linux. + // + // The codex-shaped token must be IMMEDIATELY next. Skipping interpreter flags to reach + // it looks tempting and is wrong: interpreter options take values, so a generic skip + // reads the value of `node --require codex app-server worker.js` as the entrypoint. + // Supporting flags needs a real Node/Bun/Deno entrypoint parser, not a loop over + // hyphens; the observed wrappers put the path first, so this stays narrow on purpose. + // + // Dropping only the interpreter and re-running the ordinary scan is what preserves the + // subcommand discipline below: `node exec 'hi'` stays unmatched exactly like + // `codex exec 'hi'` does. + if (isInterpreterToken(tokens[0]!) && tokens.length > 1 && isCodexExecutableToken(tokens[1]!)) { + tokens = tokens.slice(1); + } if (!isCodexExecutableToken(tokens[0]!)) return false; let i = 1; diff --git a/tests/codex-app-server-processes.test.ts b/tests/codex-app-server-processes.test.ts index 31872fae4bb..e06b566aee0 100644 --- a/tests/codex-app-server-processes.test.ts +++ b/tests/codex-app-server-processes.test.ts @@ -375,6 +375,30 @@ describe("Codex app-server process matching (#476)", () => { expect(isCodexAppServerCommandLine("node /opt/codex-code-mode-host --session 1")).toBe(true); }); + + test("matches the npm wrapper that supervises the native app-server", () => { + // The shape that made `ocx sync --restart-codex` report a survivor on Linux. An + // npm-installed Codex runs as a PAIR: `node /usr/local/bin/codex app-server` and + // the vendored native binary it spawns. Only the child matched, so SIGTERM went to + // the child while its supervisor kept the socket. Both halves have to match. + expect(isCodexAppServerCommandLine("node /usr/local/bin/codex app-server proxy")).toBe(true); + expect(isCodexAppServerCommandLine( + "node /usr/local/bin/codex -c features.code_mode_host=true app-server --listen unix://", + )).toBe(true); + expect(isCodexAppServerCommandLine("bun /usr/local/bin/codex app-server")).toBe(true); + + // The interpreter pair only forms when the codex-shaped token is IMMEDIATELY next. + // `worker.js` is not codex-shaped, so this stays the unrelated process it always was. + expect(isCodexAppServerCommandLine("node worker.js codex app-server")).toBe(false); + // Subcommand discipline survives the slice: `exec` is not `app-server`. + expect(isCodexAppServerCommandLine("node /usr/local/bin/codex exec 'hello'")).toBe(false); + // `run` is an npm-script indirection, not the executable. + expect(isCodexAppServerCommandLine("bun run codex app-server")).toBe(false); + // Interpreter FLAGS are deliberately unsupported: skipping them generically would + // mistake an option VALUE for the entrypoint (`node --require codex app-server x.js`). + expect(isCodexAppServerCommandLine("node --inspect /usr/local/bin/codex app-server")).toBe(false); + }); + test("matches official platform-baked Codex target-triple basenames", () => { expect(isCodexAppServerCommandLine( "/opt/codex/codex-x86_64-unknown-linux-musl app-server --listen unix:///tmp/c.sock",