fix(codex): match the shim's launcher backups in --restart-codex

devlog/_plan/260829_bugpr_lane_h_residual_issues/140_pr2884_shim_backup_matcher.md

@@ -0,0 +1,94 @@
+# 140 — #2884: match the shim's launcher backups, and only those
+
+Contributor PR #2884 reported a real defect with `ps` output from an affected host. This
+completes it. The plan below is the second version: an independent audit rejected the
+first, and the rejection was correct.
+
+## Scope
+
+IN: `src/codex/app-server-processes.ts` (`isCodexExecutableToken`,
+`WINDOWS_CODEX_BASENAME_CANDIDATE_RE`), `tests/codex-app-server-processes.test.ts`.
+
+OUT: the shim itself, and `.ps1` process-shape support beyond admitting the basename —
+proving a PowerShell launcher's real command line needs a Win32 reproduction.
+
+## The defect
+
+`backupPathFor` (`src/codex/shim.ts`) renames the original launcher when the autostart
+shim installs, inserting `.opencodex-real` before the extension. On a shimmed host the
+running process is
+
+```text
+/home/ubuntu/.local/bin/codex.opencodex-real -c features.code_mode_host=true app-server --listen unix://
+```
+
+`isCodexExecutableToken` admitted `codex`, `codex.exe`, `codex.cmd` and target triples,
+so `ocx sync --restart-codex` matched nothing, reported zero processes stopped, and left
+app-servers alive holding stale in-memory model catalogs.
+
+## What the first plan got wrong
+
+It claimed `backupPathFor` also produces target-triple backups such as
+`codex-x86_64-unknown-linux-gnu.opencodex-real`, and proposed stripping an optional
+`.opencodex-real` segment before the existing checks so any stem would match.
+
+Both halves were wrong.
+
+**The triple case is unreachable.** Unix discovery accepts only a PATH entry named
+`codex`. Windows discovery refuses a real `codex.exe` outright and targets `codex.cmd`,
+`codex.ps1` and the extensionless Git-Bash launcher. Nothing hands a triple-named binary
+to `backupPathFor`.
+
+**The normalisation would have been unsafe.** Stripping the suffix before
+`CODEX_TARGET_TRIPLE_BASENAME_RE` turns `codex-report-generator-worker.opencodex-real`
+into a syntactically valid triple, making an unrelated process a kill target. In a code
+path whose job is sending SIGTERM, widening the matcher to be tidy is the wrong trade.
+
+## The fix
+
+An exact basename set, kept separate from the triple pattern rather than folded into it.
+
+`.ps1` is added because `findWindowsCodexTargets` shims `codex.ps1` alongside
+`codex.cmd`, and #2884 omitted it.
+
+`.opencodex-real.exe` is deliberately NOT matched. Both #2884 and my first draft included
+it, reasoning that matching a name nothing produces is free breadth. A review round
+rejected that and was right: this set decides what receives SIGTERM, and Windows
+installation refuses to rename a native `codex.exe`, so the backup cannot exist. Breadth
+in a matcher is not free when the matcher's output is a signal.
+
+The Windows prefilter's optional suffix goes where `backupPathFor` writes it — after the
+stem, before the extension. #2884 placed it before the triple, which admits
+`codex.opencodex-real-x86_64-pc-windows-msvc.exe`: a name nothing produces, paying
+GetOwner for it. The regex source is embedded into PowerShell, so every addition stays
+within plain character classes that .NET reads identically.
+
+## A pre-existing kill-target bug, found on the way
+
+The same review round found that `codex -- app-server` matched, and still matched before
+any of this work: `--` was consumed by the option-skipping loop like any other
+`-`-prefixed token. But `--` ends option parsing, so the word after it is a prompt for
+the interactive TUI. `codex -- app-server` opens a session whose first prompt word is
+"app-server", and `--restart-codex` sent SIGTERM to it.
+
+That is not #2884's defect and it is not caused by the backup names — it applies to every
+launcher spelling. It is fixed here because the shim-backup change widens which processes
+reach this scanner, and shipping a broader matcher over a known false positive would be
+the wrong order. The scanner now stops at `--`.
+
+## Verification
+
+Named mutations, each observed red:
+
+- Backups not admitted at all — the reported command line fails.
+- Reversed suffix/triple ordering — the prefilter negative assertion fails.
+- `--` treated as an ordinary option again — the TUI-prompt case fails.
+- `.opencodex-real.exe` readmitted — the impossible-backup negative fails.
+
+Positive coverage uses the exact command line from the report. Negative coverage holds
+the line the fix is at risk of crossing: a subcommand, an argument position, and
+`codex-report-generator-worker.opencodex-real`, which must never match.
+
+One limit worth stating: `.ps1` is basename admission only. A real PowerShell launcher
+runs as `powershell.exe -File <path>`, which this scanner does not match, and proving
+that shape needs a Win32 reproduction rather than another synthetic token test.

src/codex/app-server-processes.ts

@@ -44,9 +44,14 @@ const CODEX_TARGET_TRIPLE_BODY = "[a-z0-9_]+-[a-z0-9_]+-[a-z0-9_]+(?:-[a-z0-9_]+
  * `"C:\Program Files\...\codex.exe" app-server` still reach GetOwner.
  * Also admits official target-triple basenames such as
  * `codex-x86_64-pc-windows-msvc.exe`.
+ *
+ * The optional `.opencodex-real` sits where `backupPathFor` actually puts it — after
+ * the stem and BEFORE the extension — and deliberately not before the triple. Written
+ * the other way it admits `codex.opencodex-real-x86_64-pc-windows-msvc.exe`, a name
+ * nothing produces, and pays GetOwner for it.
  */
 export const WINDOWS_CODEX_BASENAME_CANDIDATE_RE = new RegExp(
-  `(^|[/\\\\\\s'"=])codex(-${CODEX_TARGET_TRIPLE_BODY})?([.]exe|[.]cmd)?['"]?(\\s|$)`,
+  `(^|[/\\\\\\s'"=])codex(-${CODEX_TARGET_TRIPLE_BODY})?([.]opencodex-real)?([.]exe|[.]cmd|[.]ps1)?['"]?(\\s|$)`,
   "i",
 );
 
@@ -57,6 +62,38 @@ const CODEX_TARGET_TRIPLE_BASENAME_RE = new RegExp(
   `^codex-${CODEX_TARGET_TRIPLE_BODY}(?:\\.exe|\\.cmd)?$`,
 );
 
+/**
+ * Launcher basenames a Codex app-server can be started through, including the
+ * `.opencodex-real` backups the autostart shim creates.
+ *
+ * When the shim installs, `backupPathFor` (`src/codex/shim.ts`) renames the original
+ * launcher by inserting `.opencodex-real` before its extension, so a shimmed host runs
+ * `~/.local/bin/codex.opencodex-real app-server`. Reported by a contributor (#2884) with
+ * `ps` output from an affected host: `--restart-codex` matched nothing and left
+ * app-servers alive holding stale in-memory catalogs.
+ *
+ * An EXACT set, kept separate from the target-triple pattern above rather than folded
+ * into it by stripping the suffix first. That shortcut is unsafe: normalising
+ * `codex-report-generator-worker.opencodex-real` yields a syntactically valid triple
+ * and would make an unrelated process a kill target. A triple binary cannot be a shim
+ * target anyway — Unix discovery accepts only a PATH entry named `codex`, and Windows
+ * refuses a real `codex.exe` outright — so the combination is unreachable, not merely
+ * unlisted.
+ *
+ * `.ps1` and `.cmd` are here because `findWindowsCodexTargets` shims both, and the
+ * extensionless form because Unix discovery and the Git-Bash launcher use it. There is
+ * deliberately no `.opencodex-real.exe`: Windows installation REFUSES to rename a native
+ * `codex.exe`, so that backup cannot exist. Matching it looked like free breadth until a
+ * review round put it plainly — this set decides what receives SIGTERM, and a name no
+ * installation can produce only widens what a coincidence can hit.
+ */
+const CODEX_LAUNCHER_BASENAMES = new Set([
+  "codex", "codex.exe", "codex.cmd",
+  "codex.opencodex-real",
+  "codex.opencodex-real.cmd",
+  "codex.opencodex-real.ps1",
+]);
+
 /** True when a Windows CommandLine is worth paying GetOwner for (current-user scoped later). */
 export function isWindowsCodexCandidateCommandLine(commandLine: string): boolean {
   return WINDOWS_CODEX_BASENAME_CANDIDATE_RE.test(commandLine)
@@ -158,7 +195,7 @@ function tokenBasename(token: string): string {
 
 function isCodexExecutableToken(token: string): boolean {
   const base = tokenBasename(token);
-  return base === "codex" || base === "codex.exe" || base === "codex.cmd"
+  return CODEX_LAUNCHER_BASENAMES.has(base)
     || CODEX_TARGET_TRIPLE_BASENAME_RE.test(base);
 }
 
@@ -282,6 +319,10 @@ export function isCodexAppServerCommandLine(commandLine: string, executable?: st
   let i = 1;
   while (i < tokens.length) {
     const token = tokens[i]!;
+    // `--` ends option parsing, so what follows is a prompt for the interactive TUI, not
+    // a subcommand. `codex -- app-server` starts a session whose first prompt word is
+    // "app-server"; treating it as a match sends SIGTERM to somebody's live session.
+    if (token === "--") return false;
     if (token.startsWith("-")) {
       i = advancePastCodexGlobalOption(tokens, i);
       continue;
@@ -418,9 +459,9 @@ function windowsSnapshotPowerShellCommand(): string {
   // Newlines keep -Command as a real script (space-joined statements need ';').
   // Double-quoted format string so `t expands to a real tab.
   // Codex candidates only: basename token codex / codex.exe / codex.cmd /
-  // official target-triple binaries (optional closing quote after the
-  // basename), or code-mode-host — not incidental substrings like a repo
-  // path with "opencodex".
+  // codex.ps1, their .opencodex-real shim backups, official target-triple
+  // binaries (optional closing quote after the basename), or code-mode-host —
+  // not incidental substrings like a repo path with "opencodex".
   const basenameMatch = powerShellSingleQuotedIgnoreCaseMatch(WINDOWS_CODEX_BASENAME_CANDIDATE_RE.source);
   const codeModeMatch = powerShellSingleQuotedIgnoreCaseMatch(WINDOWS_CODEX_CODE_MODE_HOST_CANDIDATE_RE.source);
   return [

tests/codex-app-server-processes.test.ts

@@ -397,6 +397,50 @@ describe("Codex app-server process matching (#476)", () => {
     expect(isCodexAppServerCommandLine("node /opt/codex-code-mode-host --session 1")).toBe(true);
   });
 
+  /**
+   * Reported by a contributor in #2884 with `ps` output from an affected host: once the
+   * autostart shim renames the original launcher to `codex.opencodex-real`,
+   * `--restart-codex` matched nothing and left app-servers alive on stale catalogs.
+   */
+  test("matches the .opencodex-real launcher backups the shim creates", () => {
+    expect(isCodexAppServerCommandLine("/home/ubuntu/.local/bin/codex.opencodex-real app-server proxy")).toBe(true);
+    // The exact command line from the report.
+    expect(isCodexAppServerCommandLine(
+      "/home/ubuntu/.local/bin/codex.opencodex-real -c features.code_mode_host=true app-server --listen unix://",
+    )).toBe(true);
+    expect(isCodexAppServerCommandLine("\"C:\\Program Files\\nodejs\\codex.opencodex-real.cmd\" app-server")).toBe(true);
+    // findWindowsCodexTargets shims codex.ps1 alongside codex.cmd, so its backup runs too.
+    expect(isCodexAppServerCommandLine("\"C:\\Program Files\\nodejs\\codex.opencodex-real.ps1\" app-server")).toBe(true);
+    expect(isCodexAppServerCommandLine("node /usr/local/bin/codex.opencodex-real app-server proxy")).toBe(true);
+
+    // Still narrow: the suffix does not turn a subcommand or an argument into a match.
+    expect(isCodexAppServerCommandLine("codex.opencodex-real exec 'hello'")).toBe(false);
+    expect(isCodexAppServerCommandLine("node worker.js codex.opencodex-real app-server")).toBe(false);
+    // A backup name must not be normalised into the target-triple pattern. Stripping the
+    // suffix before that test would make this unrelated binary a kill target.
+    expect(isCodexAppServerCommandLine("/opt/tools/codex-report-generator-worker.opencodex-real app-server")).toBe(false);
+    // No shim installation can produce a .exe backup: Windows refuses to rename a native
+    // codex.exe. Matching a name nothing writes only widens what SIGTERM can reach.
+    expect(isCodexAppServerCommandLine("C:\\tools\\codex.opencodex-real.exe app-server")).toBe(false);
+  });
+
+  /**
+   * `--` ends option parsing, so the next word is a TUI prompt rather than a subcommand.
+   * `codex -- app-server` opens an interactive session whose first prompt word happens to
+   * be "app-server"; matching it sent SIGTERM to a live session. Predates the shim-backup
+   * work and applies to every launcher name.
+   */
+  test("a prompt after -- is not the app-server subcommand", () => {
+    expect(isCodexAppServerCommandLine("codex -- app-server")).toBe(false);
+    expect(isCodexAppServerCommandLine("/usr/local/bin/codex -- app-server --listen unix://")).toBe(false);
+    expect(isCodexAppServerCommandLine("codex.opencodex-real -- app-server")).toBe(false);
+    expect(isCodexAppServerCommandLine("codex -c features.x=true -- app-server")).toBe(false);
+    expect(isCodexAppServerCommandLine("node /usr/local/bin/codex -- app-server")).toBe(false);
+    // The real invocations still match: a global option before the subcommand is ordinary.
+    expect(isCodexAppServerCommandLine("codex app-server")).toBe(true);
+    expect(isCodexAppServerCommandLine("codex -c features.x=true app-server")).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
@@ -496,6 +540,22 @@ describe("Codex app-server process matching (#476)", () => {
     expect(isWindowsCodexCandidateCommandLine(
       "node C:\\Users\\a\\opencodex\\src\\cli\\index.ts start",
     )).toBe(false);
+    // Shim backups reach GetOwner, in the shape backupPathFor actually writes: the
+    // suffix goes after the stem and before the extension.
+    expect(isWindowsCodexCandidateCommandLine(
+      "\"C:\\Program Files\\nodejs\\codex.opencodex-real.cmd\" app-server",
+    )).toBe(true);
+    expect(isWindowsCodexCandidateCommandLine(
+      "\"C:\\Program Files\\nodejs\\codex.opencodex-real.ps1\" app-server",
+    )).toBe(true);
+    expect(isWindowsCodexCandidateCommandLine(
+      "C:\\Users\\a\\.local\\bin\\codex.opencodex-real app-server",
+    )).toBe(true);
+    // The reverse ordering is a name nothing produces. Admitting it would pay GetOwner
+    // on a process that cannot be a shim backup.
+    expect(isWindowsCodexCandidateCommandLine(
+      "C:\\x\\codex.opencodex-real-x86_64-pc-windows-msvc.exe app-server",
+    )).toBe(false);
     expect(isWindowsCodexCandidateCommandLine("opencodex app-server")).toBe(false);
     expect(isWindowsCodexCandidateCommandLine("hermes-codex-bridge-mcp")).toBe(false);
     expect(isWindowsCodexCandidateCommandLine("hermes-codex-x86_64-pc-windows-msvc.exe")).toBe(false);