From 9c854bcfcf481d00f057654dd8bc2d37280bcb00 Mon Sep 17 00:00:00 2001
From: Sylvester Kaczmarek
 <16242628+sylvesterkaczmarek@users.noreply.github.com>
Date: Mon, 7 Sep 2026 08:54:13 +0100
Subject: [PATCH] fix(broker): do not signal stale persisted pids

---
 .../codex/scripts/lib/broker-lifecycle.mjs    |  9 ++---
 .../codex/scripts/session-lifecycle-hook.mjs  |  2 -
 tests/broker-lifecycle.test.mjs               | 30 +++++++++++++++
 tests/runtime.test.mjs                        | 38 +++++++++++++++++++
 4 files changed, 72 insertions(+), 7 deletions(-)
 create mode 100644 tests/broker-lifecycle.test.mjs

diff --git a/plugins/codex/scripts/lib/broker-lifecycle.mjs b/plugins/codex/scripts/lib/broker-lifecycle.mjs
index ef763819c..329ddca4b 100644
--- a/plugins/codex/scripts/lib/broker-lifecycle.mjs
+++ b/plugins/codex/scripts/lib/broker-lifecycle.mjs
@@ -122,7 +122,6 @@ export async function ensureBrokerSession(cwd, options = {}) {
       pidFile: existing.pidFile ?? null,
       logFile: existing.logFile ?? null,
       sessionDir: existing.sessionDir ?? null,
-      pid: existing.pid ?? null,
       killProcess: options.killProcess ?? null
     });
     clearBrokerSession(cwd);
@@ -153,7 +152,7 @@ export async function ensureBrokerSession(cwd, options = {}) {
       pidFile,
       logFile,
       sessionDir,
-      pid: child.pid ?? null,
+      ownedPid: child.pid ?? null,
       killProcess: options.killProcess ?? null
     });
     return null;
@@ -170,10 +169,10 @@ export async function ensureBrokerSession(cwd, options = {}) {
   return session;
 }
 
-export function teardownBrokerSession({ endpoint = null, pidFile, logFile, sessionDir = null, pid = null, killProcess = null }) {
-  if (Number.isFinite(pid) && killProcess) {
+export function teardownBrokerSession({ endpoint = null, pidFile, logFile, sessionDir = null, ownedPid = null, killProcess = null }) {
+  if (Number.isFinite(ownedPid) && killProcess) {
     try {
-      killProcess(pid);
+      killProcess(ownedPid);
     } catch {
       // Ignore missing or already-exited broker processes.
     }
diff --git a/plugins/codex/scripts/session-lifecycle-hook.mjs b/plugins/codex/scripts/session-lifecycle-hook.mjs
index 778571e6c..bea1a04c0 100644
--- a/plugins/codex/scripts/session-lifecycle-hook.mjs
+++ b/plugins/codex/scripts/session-lifecycle-hook.mjs
@@ -95,7 +95,6 @@ async function handleSessionEnd(input) {
   const pidFile = brokerSession?.pidFile ?? null;
   const logFile = brokerSession?.logFile ?? null;
   const sessionDir = brokerSession?.sessionDir ?? null;
-  const pid = brokerSession?.pid ?? null;
 
   if (brokerEndpoint) {
     await sendBrokerShutdown(brokerEndpoint);
@@ -107,7 +106,6 @@ async function handleSessionEnd(input) {
     pidFile,
     logFile,
     sessionDir,
-    pid,
     killProcess: terminateProcessTree
   });
   clearBrokerSession(cwd);
diff --git a/tests/broker-lifecycle.test.mjs b/tests/broker-lifecycle.test.mjs
new file mode 100644
index 000000000..0b3dd9abc
--- /dev/null
+++ b/tests/broker-lifecycle.test.mjs
@@ -0,0 +1,30 @@
+import test from "node:test";
+import assert from "node:assert/strict";
+
+import { teardownBrokerSession } from "../plugins/codex/scripts/lib/broker-lifecycle.mjs";
+
+test("teardownBrokerSession does not treat persisted pid metadata as process ownership", () => {
+  const killed = [];
+
+  teardownBrokerSession({
+    pid: 1234,
+    killProcess(pid) {
+      killed.push(pid);
+    }
+  });
+
+  assert.deepEqual(killed, []);
+});
+
+test("teardownBrokerSession may terminate a process explicitly owned by the caller", () => {
+  const killed = [];
+
+  teardownBrokerSession({
+    ownedPid: 1234,
+    killProcess(pid) {
+      killed.push(pid);
+    }
+  });
+
+  assert.deepEqual(killed, [1234]);
+});
diff --git a/tests/runtime.test.mjs b/tests/runtime.test.mjs
index 8f276835b..274ad3150 100644
--- a/tests/runtime.test.mjs
+++ b/tests/runtime.test.mjs
@@ -1801,6 +1801,44 @@ test("cancel sends turn interrupt to the shared app-server before killing a brok
   assert.equal(cleanup.status, 0, cleanup.stderr);
 });
 
+test("session end does not signal a stale broker pid", (t) => {
+  const repo = makeTempDir();
+  initGitRepo(repo);
+
+  const sleeper = spawn(process.execPath, ["-e", "setInterval(() => {}, 1000)"], {
+    detached: true,
+    stdio: "ignore"
+  });
+  sleeper.unref();
+  t.after(() => {
+    try {
+      process.kill(-sleeper.pid, "SIGTERM");
+    } catch {
+      // Ignore an already-exited process.
+    }
+  });
+
+  saveBrokerSession(repo, {
+    endpoint: null,
+    pid: sleeper.pid,
+    pidFile: null,
+    logFile: null,
+    sessionDir: null
+  });
+
+  const result = run("node", [SESSION_HOOK, "SessionEnd"], {
+    cwd: repo,
+    input: JSON.stringify({
+      hook_event_name: "SessionEnd",
+      cwd: repo
+    })
+  });
+
+  assert.equal(result.status, 0, result.stderr);
+  assert.doesNotThrow(() => process.kill(sleeper.pid, 0));
+  assert.equal(loadBrokerSession(repo), null);
+});
+
 test("session end fully cleans up jobs for the ending session", async (t) => {
   const repo = makeTempDir();
   initGitRepo(repo);