fix(stop): name the real refusal cause instead of asserting ownership

devlog/_plan/260911_l4_service_cli/020_wp2_stop_refusal.md

@@ -0,0 +1,87 @@
+# wp2 — #4169: every stop refusal is reported as a CODEX_HOME ownership mismatch
+
+Work-phase 2 of the L4 lane, stacked on wp1. Carried source: PR #4170 by `yeongjunyoo`,
+head `4d72ef0103`, three commits on base `c15a98caa9`. All four of its files are inside this
+lane's ownership, so nothing was dropped.
+
+## What the carry does
+
+`POST /api/stop` refuses for three distinct reasons. `stopProxy` preferred the server's own
+message but fell back, when none was readable, to one hardcoded sentence naming a fourth cause
+the server never reports. #4170 captures the refusal `code`, selects the fallback wording from
+it, and returns the refusal per attempt instead of publishing it to module state — the last of
+those fixes a real interleaving bug where two overlapping stops could swap causes.
+
+## What the carry left open
+
+#4169's **Expected** section asks for two things:
+
+1. a `respawnable_service` or `service_state_unknown` refusal must not be described as an
+   ownership mismatch, and
+2. *"the recommended next command should not be the command the operator just ran."*
+
+The carry does (1). Subagent Heisenberg traced (2) and returned `LOOP_REAL`:
+
+- `ocx stop` reaches `POST /api/stop` through `dispatch.stop` → `handleStop` →
+  `stopWithDeferral` → `stopProxy` → `stopProxyGracefully`. Stopping the service manager
+  first does not skip it.
+- The server answers with *"the stop must be run by `ocx stop`"* because that refusal is
+  written for an API client, and `management-api.ts` emits `respawnable_service` precisely
+  when the CLI's teardown receipt was **not** honoured, so it cannot tell the two apart.
+- `handleStop` prints `err.message` verbatim, so the operator is told to run the command they
+  are already running.
+- The carried fallback for `respawnable_service` also ends in "Run `ocx stop`", making the
+  empty-body path a tighter loop than the one being fixed.
+
+There is no header, flag, query parameter or route that marks a CLI-originated stop, so the
+server cannot word the refusal differently. The correction belongs to the CLI, which is the one
+caller that knows which it is.
+
+## What this work-phase adds
+
+- The refusal `code` travels on `ProxyOwnershipRefusedError`. The reporting caller acts on the
+  cause; re-parsing the prose is not an option, because the prose is the server's.
+- `refusalFallbackMessage` names the cause only.
+- A new exported `refusalNextStep(code)` owns the command, and `handleStop` prints it under the
+  refusal at both call sites. The only callers of `stopProxy` are `ocx stop` and the service
+  manager's own cleanup, and both have already asked the service manager to stop by then, so no
+  branch answers with the command that just failed.
+
+`ProxyOwnershipRefusedError` keeps its name. The issue suggests renaming it and the name does
+overstate what it carries, but the carrying author deliberately deferred that as a separate
+wider change and it is not part of the issue's Expected behaviour. Recorded, not decided
+unilaterally.
+
+## Audit
+
+Subagent Confucius reviewed the staged diff adversarially and returned `BLOCKERS_FOUND` with one
+item: the new next-step test banned the literal `` `ocx stop` ``, which the production wording
+contains **in order to rule it out**, so the test failed against its own implementation. Folded:
+the assertion now bans a recommendation (`/Run \`ocx stop\`/`) rather than a mention. Its second
+observation — that no test pinned the CLI wiring — is folded as a source oracle over the two
+`refusalNextStep(err.code)` print sites.
+
+Re-checked deterministically afterwards: the four source-oracle counts in
+`grok-lifecycle.test.ts` are all exactly 2, and no `refusalNextStep` branch matches
+`/Run \`ocx stop\`/`.
+
+## CI repair carried into this phase
+
+Hosted CI on wp1's head failed on Linux and macOS with one test:
+`codex-cli-update-launcher-policy` asserts `bin/ocx.mjs` contains
+`"!codexCliUpdateInspection && isNodeModulesInstall()"` as an adjacent string. The carry gates
+the #1849 boot probe on the npm layout, inserting `installMethod === "npm"` between those two
+clauses. The invariant the oracle protects is intact and still evaluated first; only the
+adjacency changed. The oracle now locates the guard wrapping the `bootRestoreProbe` call and
+asserts both clauses are in it.
+
+`tests/codex-integration/codex-cli-update-launcher-policy.test.ts` is not in the packet's
+keep-set. It is an oracle over `bin/ocx.mjs`, which this lane owns, and the round already
+granted L2 the same thing for the same reason: a lane that changes a file owns the oracle
+asserting it, or the change cannot land at all. Reported rather than assumed.
+
+## Not run
+
+`bun test`, `bun run test`, `bun run test:changed`, `bun run typecheck`, `bun run build:gui`
+and `bun install` are NOT RUN by operator instruction. Hosted CI on the exact pushed head is
+the only product evidence this round accepts.

src/cli/index.ts

@@ -68,7 +68,7 @@ import { findLiveProxy, probeHostname, type LiveProxy } from "../server/proxy-li
 import { createReadinessGate } from "../server/readiness";
 import { runReady, type ReadyArgs } from "./ready";
 import { runCli } from "./root";
-import { isProcessAlive, ProxyOwnershipRefusedError, stopProxy } from "../lib/process-control";
+import { isProcessAlive, ProxyOwnershipRefusedError, refusalNextStep, stopProxy } from "../lib/process-control";
 import { loadServiceTokenFromFile } from "../lib/service-secrets";
 import { assertNotAdminToken, diagnoseService, isServiceOwnershipError, proxyStillLiveAfterStop, serviceCommand, serviceEnvironmentOwnedHere, serviceStartableFromTray, serviceStatusSummary, stopServiceIfInstalledDetailed, uninstallServiceIfInstalled, uninstallServiceDetailed } from "../service";
 import { formatStartupRoutingDetail, startupHealthSummary } from "../codex/autostart-health";
@@ -952,7 +952,12 @@ async function handleStop() {
       if (detail) console.error(`   ${detail}`);
       if (err instanceof ProxyOwnershipRefusedError) {
         ownershipBlocked = true;
-        console.error("   Skipping shared teardown (native Codex restore, Grok config): the foreign proxy is still running.");
+        // Every refusal `POST /api/stop` produces is written for an API client, so it
+        // recommends `ocx stop` — the command printing it. Following that advice returns
+        // the operator to this exact message, which is the loop #4169 was filed for. The
+        // service manager was already asked to stop above, so name what is actually left.
+        console.error(`   ${refusalNextStep(err.code)}`);
+        console.error("   Skipping shared teardown (native Codex restore, Grok config): the refusing proxy is still running.");
       }
     }
   } else {
@@ -979,7 +984,9 @@ async function handleStop() {
         if (detail) console.error(`   ${detail}`);
         if (err instanceof ProxyOwnershipRefusedError) {
           ownershipBlocked = true;
-          console.error("   Skipping shared teardown (native Codex restore, Grok config): the foreign proxy is still running.");
+          // Same loop as the tracked-pid path above: the refusal recommends this command.
+          console.error(`   ${refusalNextStep(err.code)}`);
+          console.error("   Skipping shared teardown (native Codex restore, Grok config): the refusing proxy is still running.");
         }
       }
     } else if (live) {

src/lib/process-control.ts

@@ -80,17 +80,96 @@ export type GracefulStopResult = boolean | "refused" | "teardown-unconfirmed";
  */
 let lastRefusalMessage: string | null = null;
 
+/**
+ * The server's machine-readable reason for the most recent 409, captured alongside the
+ * message so a refusal that arrives without a body still names the right cause. Without it
+ * the fallback has to guess, and guessing "ownership" sent operators to re-check
+ * CODEX_HOME for a refusal the scheduler wrapper had issued (#4169).
+ */
+let lastRefusalCode: string | null = null;
+
 /** The server's explanation for the most recent 409, or `null` when it sent none. */
 export function lastStopRefusalMessage(): string | null {
   return lastRefusalMessage;
 }
 
+/** The server's `code` for the most recent 409, or `null` when it sent none. */
+export function lastStopRefusalCode(): string | null {
+  return lastRefusalCode;
+}
+
+/**
+ * Wording for a refusal whose body carried no message. Each branch mirrors a refusal the
+ * management API can return from `POST /api/stop`; the default stays cause-neutral because
+ * naming the wrong cause is worse than naming none — it costs the operator the time they
+ * spend acting on it.
+ *
+ * These name the cause only. The command belongs to {@link refusalNextStep}, because the
+ * only callers of `stopProxy` are `ocx stop` and the service manager's own cleanup, and a
+ * message that told either of them to run `ocx stop` would be the #4169 loop again.
+ */
+function refusalFallbackMessage(code: string | null): string {
+  switch (code) {
+    case "respawnable_service":
+      return "The running proxy refused to stop: a service manager that can respawn it owns "
+        + "the process.";
+    case "self_unload_service":
+      return "The running proxy refused to stop: it is the installed service itself, so "
+        + "stopping the manager from inside it would end the process before native Codex is "
+        + "restored.";
+    case "service_state_unknown":
+      return "The running proxy refused to stop: the service manager state could not be read, "
+        + "so it cannot tell whether a wrapper would respawn it.";
+    default:
+      return "The running proxy refused to stop and sent no reason.";
+  }
+}
+
+/**
+ * What is actually left to do when `ocx stop` is the command that received the refusal.
+ *
+ * Every refusal `POST /api/stop` produces is written for an API client, so it recommends
+ * `ocx stop` — which is the command already running when the CLI prints it. That is the
+ * loop #4169 reports: the endpoint points at `ocx stop`, `ocx stop` repeats the endpoint,
+ * and neither names the wrapper that is refusing. `ocx stop` has already asked the service
+ * manager to stop by the time this is reached, so the remaining question is always what the
+ * service manager is doing, and no branch may answer with the command that just failed.
+ */
+export function refusalNextStep(code: string | null): string {
+  switch (code) {
+    case "respawnable_service":
+      return "This stop already asked the service manager to stop, so running `ocx stop` "
+        + "again is not the missing step. Run `ocx service status` to see whether a wrapper "
+        + "is still installed and able to respawn the proxy.";
+    case "self_unload_service":
+      return "This stop already asked the service manager to stop, so running `ocx stop` "
+        + "again is not the missing step. Run `ocx service status` to see whether the service "
+        + "is still registered.";
+    case "service_state_unknown":
+      return "Run `ocx service status` to see the query error, repair the service manager "
+        + "access, then retry.";
+    default:
+      return "Run `ocx service status` to inspect the service state.";
+  }
+}
+
 /**
  * A proxy declined shutdown (HTTP 409). There is more than one reason it can say no — a
  * scheduler wrapper under another home, or the proxy being the installed service itself
  * (#4023) — so the server's own message is carried through rather than guessed at.
+ *
+ * The refusal's `code` travels on the error because the reporting caller has to act on the
+ * cause, not re-parse prose: the message is the server's, and it recommends a command the
+ * CLI has already run.
  */
-export class ProxyOwnershipRefusedError extends Error {}
+export class ProxyOwnershipRefusedError extends Error {
+  readonly code: string | null;
+
+  constructor(message: string, code: string | null = null) {
+    super(message);
+    this.code = code;
+  }
+}
 
 /**
  * Ask a running proxy to stop itself via the management API (`POST /api/stop`), which
@@ -104,9 +183,28 @@ export class ProxyOwnershipRefusedError extends Error {}
  * attest the process exit code or completion of every drain/shutdown hook.
  */
 export async function stopProxyGracefully(pid: number, io: GracefulStopIo = {}): Promise<GracefulStopResult> {
+  return (await stopProxyGracefullyDetailed(pid, io)).result;
+}
+
+/**
+ * The refusal a single stop attempt received, carried back to that attempt's caller.
+ *
+ * Module-scoped state cannot do this job: two overlapping stops race, and the first would
+ * report the second's cause. The exported accessors stay as observational state for callers
+ * that only want the last refusal, but the error text is built from this per-call value.
+ */
+type StopRefusal = { message: string | null; code: string | null };
+
+async function stopProxyGracefullyDetailed(
+  pid: number,
+  io: GracefulStopIo = {},
+): Promise<{ result: GracefulStopResult; refusal: StopRefusal }> {
+  const refusal: StopRefusal = { message: null, code: null };
+  const done = (result: GracefulStopResult): { result: GracefulStopResult; refusal: StopRefusal } =>
+    ({ result, refusal });
   const readRuntime = io.readRuntime ?? readRuntimePort;
   const runtime = io.runtimeEndpoint ?? readRuntime(pid);
-  if (!runtime?.port) return false;
+  if (!runtime?.port) return done(false);
   const env = io.env ?? process.env;
   const headers: Record<string, string> = {};
   const token = configuredAdminToken(env.OPENCODEX_HOME?.trim() || undefined, env as NodeJS.ProcessEnv);
@@ -128,20 +226,31 @@ export async function stopProxyGracefully(pid: number, io: GracefulStopIo = {}):
       // longer than a health poll so we prefer drain over taskkill /F.
       signal: AbortSignal.timeout(io.exitTimeoutMs ? Math.min(io.exitTimeoutMs, 10_000) : 10_000),
     });
-    // 409 is the proxy REFUSING to stop (a service installed under another home owns it and
-    // would respawn it anyway). That is a policy answer, not a dead endpoint — escalating to
-    // SIGTERM here would run the daemon's cleanup and strip shared config out from under the
+    // 409 is the proxy REFUSING to stop. There is more than one reason it can say no — a
+    // respawning service manager, the proxy being the installed service itself, or an
+    // unreadable scheduler state — so both the message and the code are captured rather
+    // than assumed. That is a policy answer, not a dead endpoint — escalating to SIGTERM
+    // here would run the daemon's cleanup and strip shared config out from under the
     // still-running service. Report the refusal instead of forcing.
     if (res.status === 409) {
-      lastRefusalMessage = await res.json()
+      const parsed = await res.json()
         .then(body => {
-          const message = (body as { message?: unknown } | null)?.message;
-          return typeof message === "string" && message.trim() ? message.trim() : null;
+          const record = body as { message?: unknown; code?: unknown } | null;
+          const message = record?.message;
+          const code = record?.code;
+          return {
+            message: typeof message === "string" && message.trim() ? message.trim() : null,
+            code: typeof code === "string" && code.trim() ? code.trim() : null,
+          };
         })
-        .catch(() => null);
-      return "refused";
+        .catch(() => ({ message: null, code: null }));
+      refusal.message = parsed.message;
+      refusal.code = parsed.code;
+      lastRefusalMessage = parsed.message;
+      lastRefusalCode = parsed.code;
+      return done("refused");
     }
-    if (!res.ok) return false;
+    if (!res.ok) return done(false);
     const body: unknown = await res.json().catch(() => null);
     const expectedTeardown = io.deferSharedTeardownNonce ? "deferred" : "performed";
     sharedTeardownConfirmed = body !== null
@@ -150,14 +259,14 @@ export async function stopProxyGracefully(pid: number, io: GracefulStopIo = {}):
       && "success" in body && body.success === true
       && "sharedTeardown" in body && body.sharedTeardown === expectedTeardown;
   } catch {
-    return false;
+    return done(false);
   }
   const waitExit = io.waitExit ?? waitForExit;
   // Honor the server's own drain window: /api/stop answers 200 first, then drains for
   // config.shutdownTimeoutMs. Waiting less than that hard-kills mid-drain.
   const exitTimeoutMs = io.exitTimeoutMs ?? drainDeadlineMs();
-  if (!waitExit(pid, exitTimeoutMs)) return false;
-  return sharedTeardownConfirmed ? true : "teardown-unconfirmed";
+  if (!waitExit(pid, exitTimeoutMs)) return done(false);
+  return done(sharedTeardownConfirmed ? true : "teardown-unconfirmed");
 }
 
 function drainDeadlineMs(): number {
@@ -172,14 +281,15 @@ function drainDeadlineMs(): number {
 export async function stopProxy(pid: number, io: GracefulStopIo = {}): Promise<boolean> {
   if (!isProcessAlive(pid)) return false;
   const runtime = io.runtimeEndpoint ?? readRuntimePort(pid);
-  const graceful = await stopProxyGracefully(pid, io);
+  const { result: graceful, refusal } = await stopProxyGracefullyDetailed(pid, io);
   if (graceful === "refused") {
-    // The proxy refused on purpose (foreign service owns it). Forcing would strip shared
-    // config while that service keeps the proxy alive.
+    // The proxy refused on purpose. Forcing would strip shared config while whatever owns
+    // the process keeps it alive. The server's own message is preferred; the fallback is
+    // selected from its code so an empty body still names the right cause. Both come from
+    // THIS attempt, so an overlapping stop cannot lend it the wrong reason.
     throw new ProxyOwnershipRefusedError(
-      lastRefusalMessage
-      ?? "The running proxy refused to stop: a service installed under a different "
-        + "CODEX_HOME/OPENCODEX_HOME owns it. Run the stop from that home.",
+      refusal.message ?? refusalFallbackMessage(refusal.code),
+      refusal.code,
     );
   }
   if (graceful === "teardown-unconfirmed") {