diff --git a/devlog/_plan/260911_l4_service_cli/020_wp2_stop_refusal.md b/devlog/_plan/260911_l4_service_cli/020_wp2_stop_refusal.md new file mode 100644 index 0000000000..06962ff39b --- /dev/null +++ b/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. diff --git a/src/cli/index.ts b/src/cli/index.ts index d59f9e3257..2589f1eb5d 100755 --- a/src/cli/index.ts +++ b/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) { diff --git a/src/lib/process-control.ts b/src/lib/process-control.ts index 49da43987c..165d21aeaa 100644 --- a/src/lib/process-control.ts +++ b/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 { + 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 = {}; 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 { 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") { diff --git a/tests/lib/process-control-graceful.test.ts b/tests/lib/process-control-graceful.test.ts index 9fad73d38b..9f1f2fe7fc 100644 --- a/tests/lib/process-control-graceful.test.ts +++ b/tests/lib/process-control-graceful.test.ts @@ -1,5 +1,5 @@ import { describe, expect, test } from "bun:test"; -import { gracefulStopHost, lastStopRefusalMessage, stopProxyGracefully } from "../../src/lib/process-control"; +import { gracefulStopHost, lastStopRefusalCode, lastStopRefusalMessage, ProxyOwnershipRefusedError, refusalNextStep, stopProxy, stopProxyGracefully } from "../../src/lib/process-control"; function okResponse(): Response { return new Response(JSON.stringify({ success: true, sharedTeardown: "performed" }), { status: 200 }); @@ -198,4 +198,153 @@ describe("409 refusal reporting", () => { expect(result).toBe("refused"); expect(lastStopRefusalMessage()).toBeNull(); }); + + test("the refusal code is captured alongside the message", async () => { + // The message alone cannot drive the fallback: a refusal that arrives with an empty or + // unparseable body still has to name a cause, and #4169 showed what happens when the + // fallback guesses one — the operator re-checks CODEX_HOME for a refusal the scheduler + // wrapper issued. + await stopProxyGracefully(7, { + readRuntime: () => ({ port: 10100 }), + fetchFn: (async () => new Response( + JSON.stringify({ success: false, code: "respawnable_service", message: "wrapper owns it" }), + { status: 409, headers: { "content-type": "application/json" } }, + )) as typeof fetch, + waitExit: () => true, + env: {}, + }); + expect(lastStopRefusalCode()).toBe("respawnable_service"); + + await stopProxyGracefully(7, { + readRuntime: () => ({ port: 10100 }), + fetchFn: (async () => new Response("not json", { status: 409 })) as typeof fetch, + waitExit: () => true, + env: {}, + }); + expect(lastStopRefusalCode()).toBeNull(); + }); + + test("a refusal without a message falls back by code, never to an ownership claim", async () => { + const refusalFor = async (code: string | null): Promise => { + const body = code === null ? "not json" : JSON.stringify({ success: false, code }); + try { + await stopProxy(process.pid, { + readRuntime: () => ({ port: 10100 }), + fetchFn: (async () => new Response(body, { + status: 409, + headers: { "content-type": "application/json" }, + })) as typeof fetch, + waitExit: () => { throw new Error("must not wait for a refused stop"); }, + env: {}, + }); + } catch (err) { + if (err instanceof ProxyOwnershipRefusedError) return err.message; + throw err; + } + throw new Error("stopProxy must throw on a refusal"); + }; + + const respawnable = await refusalFor("respawnable_service"); + expect(respawnable).toContain("respawn"); + // Not `ocx stop`: the only callers of stopProxy are `ocx stop` and the service + // manager's own cleanup, so recommending it here is the #4169 loop. The fallback names + // the cause and refusalNextStep names the command. + expect(respawnable).not.toContain("ocx stop"); + + const selfUnload = await refusalFor("self_unload_service"); + expect(selfUnload).toContain("installed service itself"); + expect(selfUnload).not.toContain("ocx stop"); + + const unknownState = await refusalFor("service_state_unknown"); + expect(unknownState).toContain("could not be read"); + expect(unknownState).not.toContain("ocx stop"); + + const noBody = await refusalFor(null); + expect(noBody).toContain("sent no reason"); + expect(noBody).not.toContain("ocx stop"); + + // None of them may assert the cause that #4169 was filed for. + for (const message of [respawnable, selfUnload, unknownState, noBody]) { + expect(message).not.toContain("CODEX_HOME"); + expect(message).not.toContain("OPENCODEX_HOME"); + } + }); + + test("the refusal carries its code to the caller that has to report it", async () => { + // The reporting caller acts on the cause. Re-parsing the prose is not an option: the + // message is the server's, and the server's message is exactly what recommends the + // command that already failed. + let thrown: unknown; + try { + await stopProxy(process.pid, { + readRuntime: () => ({ port: 10100 }), + fetchFn: (async () => new Response( + JSON.stringify({ success: false, code: "respawnable_service", message: "wrapper owns it" }), + { status: 409, headers: { "content-type": "application/json" } }, + )) as typeof fetch, + waitExit: () => { throw new Error("must not wait for a refused stop"); }, + env: {}, + }); + } catch (err) { + thrown = err; + } + expect(thrown).toBeInstanceOf(ProxyOwnershipRefusedError); + expect((thrown as ProxyOwnershipRefusedError).code).toBe("respawnable_service"); + // The server's own message still wins, unchanged. + expect((thrown as ProxyOwnershipRefusedError).message).toBe("wrapper owns it"); + }); + + test("no next step sends the operator back to the command that just refused", () => { + // #4169's loop: POST /api/stop answers "the stop must be run by `ocx stop`", and + // `ocx stop` reprints it. Whatever the cause, the next step has to be something else, + // because the service manager was already asked to stop before this point. + for (const code of ["respawnable_service", "self_unload_service", "service_state_unknown", null]) { + const step = refusalNextStep(code); + // Naming `ocx stop` in order to rule it out is the point; recommending it is the loop. + expect(step).not.toMatch(/Run `ocx stop`/); + expect(step).toContain("ocx service status"); + } + // The two service causes say why repeating the stop is not the missing step, since the + // server's message printed just above them recommends exactly that. + expect(refusalNextStep("respawnable_service")).toContain("already asked the service manager"); + expect(refusalNextStep("self_unload_service")).toContain("already asked the service manager"); + }); + + test("concurrent refusals each keep their own cause", async () => { + // Reading the reason from module state lets one stop publish its refusal and a second + // overwrite it before the first continuation consumes it. Starting both together is + // what actually reproduces that: verified against the pre-fix global handoff, where + // this schedule fails with the first call throwing the second's cause + // ("...it is the installed service itself..." for the respawnable_service stop). + // A schedule that lets one call finish entirely before resuming the other does NOT + // discriminate — the parked call republishes its own globals last and passes either way. + const refusalOf = (code: string) => async (): Promise => { + try { + await stopProxy(process.pid, { + readRuntime: () => ({ port: 10100 }), + fetchFn: (async () => new Response(JSON.stringify({ success: false, code }), { + status: 409, + headers: { "content-type": "application/json" }, + })) as typeof fetch, + waitExit: () => { throw new Error("must not wait for a refused stop"); }, + env: {}, + }); + } catch (err) { + if (err instanceof ProxyOwnershipRefusedError) return err.message; + throw err; + } + throw new Error("stopProxy must throw on a refusal"); + }; + + // Repeated because the interleaving is scheduler-dependent; the pre-fix code fails on + // the first iteration, but a single run would be a weak guard against reintroduction. + for (let i = 0; i < 20; i++) { + const [respawnable, selfUnload] = await Promise.all([ + refusalOf("respawnable_service")(), + refusalOf("self_unload_service")(), + ]); + expect(respawnable).toContain("respawn"); + expect(selfUnload).toContain("installed service itself"); + } + }); }); diff --git a/tests/providers/xai/grok-lifecycle.test.ts b/tests/providers/xai/grok-lifecycle.test.ts index 88212768af..e4d69b599f 100644 --- a/tests/providers/xai/grok-lifecycle.test.ts +++ b/tests/providers/xai/grok-lifecycle.test.ts @@ -150,8 +150,14 @@ describe("Grok fence lifecycle wiring", () => { // shared teardown must be skipped at both call sites, exactly like the service-manager path. const ownershipRefusals = stopFn.match(/err instanceof ProxyOwnershipRefusedError[\s\S]{0,200}?ownershipBlocked = true;/g); expect(ownershipRefusals).toHaveLength(2); - expect(stopFn.match(/Skipping shared teardown \(native Codex restore, Grok config\): the foreign proxy is still running\./g)).toHaveLength(2); + expect(stopFn.match(/Skipping shared teardown \(native Codex restore, Grok config\): the refusing proxy is still running\./g)).toHaveLength(2); expect(PROCESS_CONTROL_SOURCE).toContain("throw new ProxyOwnershipRefusedError("); + + // Both sites also print what is actually left to do. The refusal itself is written for + // an API client, so it recommends `ocx stop` — the command doing the printing — which + // is the loop #4169 reports. Echoing the server's message alone reproduces it. + expect(stopFn.match(/console\.error\(` \$\{refusalNextStep\(err\.code\)\}`\);/g)).toHaveLength(2); + expect(PROCESS_CONTROL_SOURCE).toContain("export function refusalNextStep("); }); test("handleStop returns its outcome while both restart surfaces share the in-place lifecycle", () => { @@ -507,10 +513,12 @@ describe("POST /api/stop teardown", () => { }); test("a 409 does not escalate to a forced kill", () => { - // Escalating would run the daemon's cleanup and strip shared config while the foreign - // service keeps the proxy alive — the exact hole the ownership gate exists to close. + // Escalating would run the daemon's cleanup and strip shared config while the refusing + // service keeps the proxy alive — the exact hole the refusal gate exists to close. // The 409 branch may capture the server's reason first (#4023 added a second refusal - // cause), but it must still return "refused" without falling through to !res.ok. + // cause, #4169 the code that names it), but it must still yield "refused" without + // falling through to !res.ok. Matched loosely so a wrapped return (`done("refused")`) + // still satisfies the invariant this guards, which is ordering, not spelling. const stopGracefully = sliceFn( PROCESS_CONTROL_SOURCE, "export async function stopProxyGracefully(", @@ -518,9 +526,12 @@ describe("POST /api/stop teardown", () => { ); const four09At = stopGracefully.indexOf("res.status === 409"); expect(four09At).toBeGreaterThan(-1); - expect(stopGracefully.slice(four09At)).toContain('return "refused"'); - expect(stopGracefully.indexOf('return "refused"', four09At)) - .toBeLessThan(stopGracefully.indexOf("if (!res.ok) return false;", four09At)); + const refusedReturn = /return (?:done\()?"refused"/; + const okFallthrough = /if \(!res\.ok\) return (?:done\()?false/; + const afterFour09 = stopGracefully.slice(four09At); + expect(afterFour09).toMatch(refusedReturn); + expect(afterFour09.search(refusedReturn)) + .toBeLessThan(afterFour09.search(okFallthrough)); const stopProxyFn = sliceFn(PROCESS_CONTROL_SOURCE, "export async function stopProxy(", "export function killProxy("); const refusedAt = stopProxyFn.indexOf('graceful === "refused"');