From c13fd80d4b43233690efce7109ec3cc03fb4331f Mon Sep 17 00:00:00 2001 From: Rami Djebari <238472321+ramioca@users.noreply.github.com> Date: Wed, 9 Sep 2026 13:29:19 +0200 Subject: [PATCH 1/3] desktop: apply chain switches without a restart; Hermes needs none Switching the payment chain wrote ~/.blockrun/.chain and then asked the user to "restart the gateway", but the proxy that needed restarting is the child process Desktop itself launched, which the user cannot restart short of quitting the app. The router resolves the chain once at startup and talks to a different gateway per chain, so there is no in-process switch to call. Desktop now restarts the proxy it owns after the CLI succeeds, waits for the new one to pass the ownership check, and confirms through /health that it signs on the requested chain before reporting it active. A proxy Desktop did not launch is left alone and keeps the old restart message. The Hermes card demanded a restart of the Hermes process after every change. Hermes re-reads config.yaml whenever its mtime changes, looks the provider key up in ~/.hermes/.env at call time, and Hermes Desktop starts a fresh `hermes --tui` process per chat, so nothing needs the process restarted. The adapter now reports the change as live; only an already-open session keeps its model until /model. Co-Authored-By: Claude Fable 5.1 --- apps/desktop/electron/adapters/hermes.ts | 10 ++- apps/desktop/electron/core/manager.ts | 54 +++++++++++- apps/desktop/electron/core/supervisor.ts | 31 +++++++ apps/desktop/src/api.ts | 2 +- apps/desktop/tests/manager.test.ts | 107 ++++++++++++++++++++++- apps/desktop/tests/supervisor.test.ts | 32 +++++++ 6 files changed, 229 insertions(+), 7 deletions(-) diff --git a/apps/desktop/electron/adapters/hermes.ts b/apps/desktop/electron/adapters/hermes.ts index ecf60d8d..66c34499 100644 --- a/apps/desktop/electron/adapters/hermes.ts +++ b/apps/desktop/electron/adapters/hermes.ts @@ -10,7 +10,11 @@ export class HermesAdapter implements AgentAdapter { readonly id = "hermes" as const; readonly name = "Hermes"; readonly description = "Hermes model-provider plugin backed by the local ClawRouter proxy."; - readonly activation = "restart-agent" as const; + // Hermes re-reads config.yaml whenever it changes and looks the provider key + // up in ~/.hermes/.env at call time, and Hermes Desktop starts a fresh + // `hermes --tui` process per chat. Nothing here needs the Hermes process + // restarted; only an already-open session keeps its model until /model. + readonly activation = "immediate" as const; managedPaths(context: AdapterContext): string[] { return [ @@ -30,10 +34,10 @@ export class HermesAdapter implements AgentAdapter { installed: Boolean(hermes), configured: hasHermesConfig(config), activation: this.activation, - restartRequired: hasHermesConfig(config), + restartRequired: false, details: hermes ? hasHermesConfig(config) - ? ["Restart the current Hermes process after changing this connection."] + ? ["New Hermes chats use ClawRouter; an open session switches with /model."] : [] : ["Install Hermes before enabling the provider."], }); diff --git a/apps/desktop/electron/core/manager.ts b/apps/desktop/electron/core/manager.ts index bad0985d..ff6bdf96 100644 --- a/apps/desktop/electron/core/manager.ts +++ b/apps/desktop/electron/core/manager.ts @@ -169,11 +169,48 @@ export class ClawRouterManager { message: (result.stderr || result.stdout).trim() || `Could not switch to ${chain}.`, }; } + const label = chain === "solana" ? "Solana" : "Base"; + // The proxy reads the chain once at startup, so apply the switch by + // restarting the proxy Desktop launched instead of asking the user to + // restart something they never started. A proxy Desktop does not own is + // left alone and still needs its own restart. + let restarted = false; + try { + restarted = await this.supervisor.restartProxy(); + } catch (error) { + return { + ok: false, + chain, + restartRequired: true, + message: `${label} selected, but the local proxy did not come back: ${ + error instanceof Error ? error.message : String(error) + }. Restart ClawRouter Desktop to apply it.`, + }; + } + if (!restarted) { + return { + ok: true, + chain, + restartRequired: true, + message: `${label} selected. Restart the ClawRouter/OpenClaw gateway to apply it.`, + }; + } + const active = await this.activePaymentChain(); + if (active !== chain) { + return { + ok: false, + chain, + restartRequired: true, + message: `${label} selected, but the restarted proxy reports ${ + active ?? "no payment chain" + }. Check the proxy log in ClawRouter Desktop.`, + }; + } return { ok: true, chain, - restartRequired: true, - message: `${chain === "solana" ? "Solana" : "Base"} selected. Restart the ClawRouter/OpenClaw gateway to apply it.`, + restartRequired: false, + message: `${label} is active. ClawRouter restarted its local proxy on the new chain.`, }; } catch (error) { return { @@ -185,6 +222,16 @@ export class ClawRouterManager { } } + /** The chain the running proxy signs on, from its own /health report. */ + private async activePaymentChain(): Promise { + const root = this.context.proxyBaseUrl.replace(/\/v1\/?$/, ""); + const health = await fetchJson>( + `${root}/health?full=true`, + this.context.fetch, + ); + return paymentChainOrUndefined(health.value?.paymentChain); + } + async createOnramp(amount: number): Promise { try { const command = await ensureNpmPackage(this.context, "@blockrun/clawrouter", "clawrouter", { @@ -620,6 +667,9 @@ function activationMessage( if (adapter.id === "pi" && action === "connect") { return `${changed} Open /model (or press Ctrl+L) in a running Pi session to refresh it now; no restart is needed.`; } + if (adapter.id === "hermes") { + return `${changed} New Hermes chats pick it up right away; run /model in an open session to switch it; no restart is needed.`; + } return `${changed} The change is active now; no restart is needed.`; } if (adapter.activation === "restart-gateway") { diff --git a/apps/desktop/electron/core/supervisor.ts b/apps/desktop/electron/core/supervisor.ts index cb9301e8..8992b556 100644 --- a/apps/desktop/electron/core/supervisor.ts +++ b/apps/desktop/electron/core/supervisor.ts @@ -8,6 +8,9 @@ import type { AdapterContext, CommandRunner } from "./types.js"; type ServiceName = "proxy" | "codex-bridge"; +/** How long a managed proxy gets to exit on SIGTERM before it is SIGKILLed. */ +const STOP_GRACE_MS = 5_000; + export class ServiceSupervisor { private readonly children = new Map(); @@ -98,6 +101,23 @@ export class ServiceSupervisor { ); } + /** + * Restart the proxy Desktop itself launched so it re-reads the payment chain + * in ~/.blockrun/.chain. Resolves false when the proxy on 8402 is not one of + * Desktop's children (an OpenClaw gateway, a terminal instance): Desktop must + * not kill a process it does not own, so that one still needs its own restart. + * The Codex bridge keeps running; it reaches the proxy by URL and only sees a + * brief outage. + */ + async restartProxy(): Promise { + const current = this.liveChild("proxy"); + if (!current) return false; + await stopChild(current, STOP_GRACE_MS); + this.children.delete("proxy"); + await this.ensureProxy(); + return true; + } + async stopAll(): Promise { for (const child of this.children.values()) { if (!child.killed) child.kill("SIGTERM"); @@ -199,6 +219,17 @@ async function waitForOwned( throw new Error(`Service did not become healthy: ${url}`); } +async function stopChild(child: ChildProcess, graceMs: number): Promise { + if (child.exitCode !== null) return; + const exited = new Promise((resolve) => child.once("exit", () => resolve())); + child.kill("SIGTERM"); + const escalate = setTimeout(() => { + if (child.exitCode === null) child.kill("SIGKILL"); + }, graceMs); + await exited; + clearTimeout(escalate); +} + async function isPortOpen(port: number): Promise { return new Promise((resolve) => { const socket = createConnection({ host: "127.0.0.1", port }); diff --git a/apps/desktop/src/api.ts b/apps/desktop/src/api.ts index e1ec5675..39741b6b 100644 --- a/apps/desktop/src/api.ts +++ b/apps/desktop/src/api.ts @@ -63,7 +63,7 @@ const demoAgents: AgentStatus[] = [ configured: false, proxyReachable: true, health: "needs-attention", - activation: "restart-agent", + activation: "immediate", restartRequired: false, removalMode: "disconnect", details: [], diff --git a/apps/desktop/tests/manager.test.ts b/apps/desktop/tests/manager.test.ts index 28bf7979..db26f6fc 100644 --- a/apps/desktop/tests/manager.test.ts +++ b/apps/desktop/tests/manager.test.ts @@ -100,7 +100,7 @@ describe("ClawRouterManager adapter flow", () => { const expectations = { openclaw: "Restart the OpenClaw gateway", codex: "Restart Codex", - hermes: "Restart Hermes", + hermes: "no restart is needed", dsh: "no restart is needed", pi: "no restart is needed", } as const; @@ -155,6 +155,99 @@ describe("ClawRouterManager adapter flow", () => { } }); + it("applies a chain switch by restarting the proxy Desktop owns", async () => { + const home = await mkdtemp(join(tmpdir(), "clawrouter-chain-owned-")); + await stagePinnedClawRouter(home); + const commands: string[][] = []; + const manager = fixtureManager( + home, + async () => false, + async (_command, args) => { + commands.push(args); + return { code: 0, stdout: "", stderr: "" }; + }, + { paymentChain: "solana" }, + ); + const restarts: number[] = []; + manager.supervisor.restartProxy = async () => { + restarts.push(Date.now()); + return true; + }; + + const result = await manager.switchPaymentChain("solana"); + + expect(result).toMatchObject({ ok: true, chain: "solana", restartRequired: false }); + expect(result.message).toContain("Solana is active"); + expect(restarts).toHaveLength(1); + expect(commands.some((args) => args.join(" ").endsWith("chain solana"))).toBe(true); + }); + + it("still asks for a gateway restart when the proxy is not Desktop's to restart", async () => { + const home = await mkdtemp(join(tmpdir(), "clawrouter-chain-foreign-")); + await stagePinnedClawRouter(home); + const manager = fixtureManager(home, async () => false); + manager.supervisor.restartProxy = async () => false; + + const result = await manager.switchPaymentChain("solana"); + + expect(result).toMatchObject({ ok: true, chain: "solana", restartRequired: true }); + expect(result.message).toContain("Restart the ClawRouter/OpenClaw gateway"); + }); + + it("reports a proxy that failed to come back after the chain switch", async () => { + const home = await mkdtemp(join(tmpdir(), "clawrouter-chain-failed-")); + await stagePinnedClawRouter(home); + const manager = fixtureManager(home, async () => false); + manager.supervisor.restartProxy = async () => { + throw new Error("Service did not become healthy"); + }; + + const result = await manager.switchPaymentChain("base"); + + expect(result).toMatchObject({ ok: false, chain: "base", restartRequired: true }); + expect(result.message).toContain("Service did not become healthy"); + }); + + it("does not claim the new chain is active when the restarted proxy reports another", async () => { + const home = await mkdtemp(join(tmpdir(), "clawrouter-chain-mismatch-")); + await stagePinnedClawRouter(home); + const manager = fixtureManager(home, async () => false, undefined, { paymentChain: "base" }); + manager.supervisor.restartProxy = async () => true; + + const result = await manager.switchPaymentChain("solana"); + + expect(result).toMatchObject({ ok: false, chain: "solana", restartRequired: true }); + expect(result.message).toContain("reports base"); + }); + + it("does not switch or restart anything when the chain command fails", async () => { + const home = await mkdtemp(join(tmpdir(), "clawrouter-chain-cli-failed-")); + await stagePinnedClawRouter(home); + const manager = fixtureManager( + home, + async () => false, + async () => ({ + code: 1, + stdout: "", + stderr: "no Solana wallet", + }), + ); + let restarted = false; + manager.supervisor.restartProxy = async () => { + restarted = true; + return true; + }; + + const result = await manager.switchPaymentChain("solana"); + + expect(result).toMatchObject({ + ok: false, + restartRequired: false, + message: "no Solana wallet", + }); + expect(restarted).toBe(false); + }); + it("keeps a reported zero wallet balance available instead of treating it as missing", async () => { const home = await mkdtemp(join(tmpdir(), "clawrouter-wallet-")); const manager = fixtureManager(home, async () => false, undefined, { balance: "$0.00" }); @@ -582,6 +675,18 @@ describe("ClawRouterManager adapter flow", () => { }); }); +/** Stage the pinned ClawRouter CLI where ensureNpmPackage looks for it. */ +async function stagePinnedClawRouter(home: string): Promise { + const runtime = join(home, ".clawrouter-desktop", "runtime", "node_modules"); + const binary = join(runtime, ".bin", "clawrouter"); + const manifest = join(runtime, "@blockrun", "clawrouter", "package.json"); + await mkdir(dirname(binary), { recursive: true }); + await mkdir(dirname(manifest), { recursive: true }); + await writeFile(binary, "#!/bin/sh\n", { mode: 0o755 }); + await writeFile(manifest, JSON.stringify({ version: CLAWROUTER_PACKAGE_VERSION })); + return binary; +} + function fixtureManager( homeDir: string, exists: (command: string) => Promise, diff --git a/apps/desktop/tests/supervisor.test.ts b/apps/desktop/tests/supervisor.test.ts index 698781a0..a39ab48f 100644 --- a/apps/desktop/tests/supervisor.test.ts +++ b/apps/desktop/tests/supervisor.test.ts @@ -1,3 +1,4 @@ +import { spawn, type ChildProcess } from "node:child_process"; import { mkdtemp } from "node:fs/promises"; import { createHmac } from "node:crypto"; import { tmpdir } from "node:os"; @@ -52,6 +53,37 @@ describe("ServiceSupervisor ownership", () => { await expect(supervisor.ensureProxy()).rejects.toThrow("unverified or outdated"); }); + it("refuses to restart a proxy Desktop did not launch", async () => { + const supervisor = new ServiceSupervisor( + await context(async () => response({ status: "ok", wallet: "0xabc" })), + async () => true, + ); + + await expect(supervisor.restartProxy()).resolves.toBe(false); + }); + + it("stops the proxy it launched and brings a fresh one up", async () => { + const supervisor = new ServiceSupervisor( + await context(async () => response({ status: "ok", wallet: "0xabc" })), + async () => false, + ); + // A real child standing in for the proxy: idles until it is signalled. + const child = spawn(process.execPath, ["-e", "setInterval(() => {}, 1000)"], { + stdio: "ignore", + }); + (supervisor as unknown as { children: Map }).children.set("proxy", child); + let relaunched = 0; + supervisor.ensureProxy = async () => { + relaunched += 1; + expect(child.exitCode ?? child.signalCode).not.toBeNull(); + }; + + await expect(supervisor.restartProxy()).resolves.toBe(true); + + expect(relaunched).toBe(1); + expect(child.signalCode).toBe("SIGTERM"); + }); + it("rejects a shape-compatible Codex bridge that Desktop did not start", async () => { const supervisor = new ServiceSupervisor( await context(async () => response({ data: [] })), From e270c94bdc1648e770db815c9b15036f7e3ebbfe Mon Sep 17 00:00:00 2001 From: Rami Djebari <238472321+ramioca@users.noreply.github.com> Date: Wed, 9 Sep 2026 16:11:06 +0200 Subject: [PATCH 2/3] desktop: stop any listener the proxy child leaves behind before relaunching Review pointed out that stopping only the direct child is not enough: were the child a wrapper, the listener on 8402 would be a descendant that outlives it, and ensureProxy() would adopt the stale proxy (it still proves the Desktop token) instead of starting one on the new chain. The supervisor now collects the listeners that descend from the child before signalling, stops them with the child, waits for the port to actually close before relaunching, escalates to SIGKILL after the grace period, and fails loudly if the port is still held rather than letting a stale proxy be re-adopted. Regression test spawns a real wrapper whose grandchild would otherwise survive. Co-Authored-By: Claude Fable 5.1 --- apps/desktop/electron/core/supervisor.ts | 64 ++++++++++++++++++++++-- apps/desktop/tests/supervisor.test.ts | 58 +++++++++++++++++++++ 2 files changed, 117 insertions(+), 5 deletions(-) diff --git a/apps/desktop/electron/core/supervisor.ts b/apps/desktop/electron/core/supervisor.ts index 8992b556..d3f38c52 100644 --- a/apps/desktop/electron/core/supervisor.ts +++ b/apps/desktop/electron/core/supervisor.ts @@ -10,6 +10,7 @@ type ServiceName = "proxy" | "codex-bridge"; /** How long a managed proxy gets to exit on SIGTERM before it is SIGKILLed. */ const STOP_GRACE_MS = 5_000; +const PROXY_PORT = 8402; export class ServiceSupervisor { private readonly children = new Map(); @@ -112,12 +113,43 @@ export class ServiceSupervisor { async restartProxy(): Promise { const current = this.liveChild("proxy"); if (!current) return false; - await stopChild(current, STOP_GRACE_MS); + await this.stopProxy(current); this.children.delete("proxy"); await this.ensureProxy(); return true; } + /** + * Stop the proxy child and anything of its that still listens on the proxy + * port. The child is normally the listener itself, but were it a wrapper the + * listener would be a descendant that outlives it, and ensureProxy() would + * then adopt the stale proxy (it still proves the Desktop token) instead of + * starting one on the new chain. So the port must be closed before relaunch. + */ + private async stopProxy(child: ChildProcess): Promise { + const owned = child.pid + ? await listenersOwnedBy(child.pid, PROXY_PORT, this.context.runCommand) + : []; + const descendants = owned.filter((pid) => pid !== child.pid); + await stopChild(child, STOP_GRACE_MS); + for (const pid of descendants) signal(pid, "SIGTERM"); + if (await this.waitForPortClosed(PROXY_PORT, STOP_GRACE_MS)) return; + for (const pid of descendants) signal(pid, "SIGKILL"); + if (await this.waitForPortClosed(PROXY_PORT, 1_000)) return; + throw new Error( + `The previous proxy is still listening on port ${PROXY_PORT}. Restart ClawRouter Desktop to apply the change.`, + ); + } + + private async waitForPortClosed(port: number, timeoutMs: number): Promise { + const started = Date.now(); + while (await this.portOpen(port)) { + if (Date.now() - started >= timeoutMs) return false; + await new Promise((resolve) => setTimeout(resolve, 100)); + } + return true; + } + async stopAll(): Promise { for (const child of this.children.values()) { if (!child.killed) child.kill("SIGTERM"); @@ -160,14 +192,28 @@ export async function listenerBelongsToProcess( port: number, runCommand: CommandRunner, ): Promise { + return (await listenersOwnedBy(ownerPid, port, runCommand)).length > 0; +} + +/** PIDs listening on `port` that are `ownerPid` itself or descend from it. */ +export async function listenersOwnedBy( + ownerPid: number, + port: number, + runCommand: CommandRunner, +): Promise { const listeners = await runCommand("lsof", ["-nP", `-iTCP:${port}`, "-sTCP:LISTEN", "-t"], { timeoutMs: 3_000, }); - if (listeners.code !== 0) return false; + if (listeners.code !== 0) return []; + const owned: number[] = []; for (const line of listeners.stdout.split(/\r?\n/)) { - let pid = Number.parseInt(line.trim(), 10); + const listener = Number.parseInt(line.trim(), 10); + let pid = listener; for (let depth = 0; Number.isInteger(pid) && pid > 1 && depth < 12; depth += 1) { - if (pid === ownerPid) return true; + if (pid === ownerPid) { + owned.push(listener); + break; + } const parent = await runCommand("ps", ["-o", "ppid=", "-p", String(pid)], { timeoutMs: 3_000, }); @@ -177,7 +223,15 @@ export async function listenerBelongsToProcess( pid = next; } } - return false; + return owned; +} + +function signal(pid: number, name: "SIGTERM" | "SIGKILL"): void { + try { + process.kill(pid, name); + } catch { + // Already gone, or not ours to signal; the port check decides what happens next. + } } async function isModelService(url: string, fetcher: typeof fetch): Promise { diff --git a/apps/desktop/tests/supervisor.test.ts b/apps/desktop/tests/supervisor.test.ts index a39ab48f..44f4fe11 100644 --- a/apps/desktop/tests/supervisor.test.ts +++ b/apps/desktop/tests/supervisor.test.ts @@ -84,6 +84,64 @@ describe("ServiceSupervisor ownership", () => { expect(child.signalCode).toBe("SIGTERM"); }); + it("also stops a listener a wrapper child left behind before relaunching", async () => { + // A wrapper that spawns the real "proxy" as a grandchild, reports its pid, + // and idles. SIGTERM on the wrapper alone would orphan the grandchild. + const wrapper = spawn( + process.execPath, + [ + "-e", + 'const c = require("node:child_process").spawn(process.execPath, ["-e", "setInterval(() => {}, 1000)"], { stdio: "ignore" }); process.stdout.write(c.pid + "\\n"); setInterval(() => {}, 1000);', + ], + { stdio: ["ignore", "pipe", "ignore"] }, + ); + const grandchild = Number( + await new Promise((resolve) => + wrapper.stdout!.once("data", (chunk) => resolve(String(chunk).trim())), + ), + ); + const alive = (pid: number) => { + try { + process.kill(pid, 0); + return true; + } catch { + return false; + } + }; + const base = await context(async () => response({ status: "ok", wallet: "0xabc" })); + const supervisor = new ServiceSupervisor( + { + ...base, + // Fake lsof/ps: the grandchild holds the port and its parent is the wrapper. + runCommand: async (command, args) => { + if (command === "lsof") + return { code: 0, stdout: alive(grandchild) ? `${grandchild}\n` : "", stderr: "" }; + if (command === "ps") { + const pid = Number(args.at(-1)); + return { code: 0, stdout: pid === grandchild ? `${wrapper.pid}\n` : "1\n", stderr: "" }; + } + return { code: 1, stdout: "", stderr: "unexpected command" }; + }, + }, + async () => alive(grandchild), + ); + (supervisor as unknown as { children: Map }).children.set( + "proxy", + wrapper, + ); + let relaunched = 0; + supervisor.ensureProxy = async () => { + relaunched += 1; + expect(alive(grandchild)).toBe(false); + }; + + await expect(supervisor.restartProxy()).resolves.toBe(true); + + expect(relaunched).toBe(1); + expect(wrapper.signalCode).toBe("SIGTERM"); + expect(alive(grandchild)).toBe(false); + }); + it("rejects a shape-compatible Codex bridge that Desktop did not start", async () => { const supervisor = new ServiceSupervisor( await context(async () => response({ data: [] })), From 998874e26cb2b1a69c30dfe89f3a955381b1dd51 Mon Sep 17 00:00:00 2001 From: 1bcMax Date: Wed, 9 Sep 2026 22:35:22 -0400 Subject: [PATCH 3/3] desktop: a signal-killed proxy child is dead, not running MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three defects in the new restart path, all reachable from one chain switch. `stopChild()` gated on `child.exitCode !== null`. A process killed by a signal keeps `exitCode === null` and reports the signal in `signalCode`, which is exactly how this file stops things, so the guard read a dead child as running, sent SIGTERM at a closed handle and then awaited an "exit" that had already fired. `restartProxy()` never settled and the chain switch hung with no way back. `hasExited()` now answers the question, and the wait is bounded so a child that outlives SIGKILL hands the decision to stopProxy's port check instead of stalling. The same predicate was wrong in `liveChild()`, `childOwnsPort()` and `start()`. `stopProxy()` escalated to SIGKILL against `descendants`, a pid list read before the parent died. Five seconds of grace is long enough for a descendant that obeyed SIGTERM to have had its pid recycled, and SIGKILL is not a signal to send at a pid nobody has re-confirmed. It now re-reads the port (`listenersOnPort`, factored out of `listenersOwnedBy`) and kills only what is still holding it. `restartProxy()` skipped `children.delete("proxy")` when `stopProxy()` threw, so a child it had already signalled stayed in the map and the next restart told the user Desktop does not own a proxy it started. Tests: all three fail on the previous code — the first as a 5 s timeout, which is the hang itself. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_015mUab3xrLHNpYqVHJLgJHL --- apps/desktop/electron/core/supervisor.ts | 70 ++++++++++++++++++------ apps/desktop/tests/supervisor.test.ts | 65 +++++++++++++++++++++- 2 files changed, 118 insertions(+), 17 deletions(-) diff --git a/apps/desktop/electron/core/supervisor.ts b/apps/desktop/electron/core/supervisor.ts index d3f38c52..bcec1f12 100644 --- a/apps/desktop/electron/core/supervisor.ts +++ b/apps/desktop/electron/core/supervisor.ts @@ -113,8 +113,14 @@ export class ServiceSupervisor { async restartProxy(): Promise { const current = this.liveChild("proxy"); if (!current) return false; - await this.stopProxy(current); - this.children.delete("proxy"); + try { + await this.stopProxy(current); + } finally { + // stopProxy throws when the port stays open, but the child is already + // signalled by then. Keeping it in the map makes the next restartProxy() + // resolve false and tell the user Desktop does not own a proxy it started. + this.children.delete("proxy"); + } await this.ensureProxy(); return true; } @@ -134,7 +140,12 @@ export class ServiceSupervisor { await stopChild(child, STOP_GRACE_MS); for (const pid of descendants) signal(pid, "SIGTERM"); if (await this.waitForPortClosed(PROXY_PORT, STOP_GRACE_MS)) return; - for (const pid of descendants) signal(pid, "SIGKILL"); + // Re-read the port before escalating. `descendants` was captured before the + // parent died; a descendant that exited on SIGTERM can have had its pid + // recycled inside the grace window, and SIGKILL is not a signal to send at a + // pid we have not just re-confirmed is the thing holding the port. + const holding = await listenersOnPort(PROXY_PORT, this.context.runCommand); + for (const pid of descendants) if (holding.includes(pid)) signal(pid, "SIGKILL"); if (await this.waitForPortClosed(PROXY_PORT, 1_000)) return; throw new Error( `The previous proxy is still listening on port ${PROXY_PORT}. Restart ClawRouter Desktop to apply the change.`, @@ -164,7 +175,7 @@ export class ServiceSupervisor { env: NodeJS.ProcessEnv, ): Promise { const current = this.children.get(name); - if (current && current.exitCode === null && !current.killed) return current; + if (current && !hasExited(current) && !current.killed) return current; const child = spawn(command, args, { env: await withEmbeddedNode(this.context.stateDir, env), stdio: ["ignore", "pipe", "pipe"], @@ -178,11 +189,11 @@ export class ServiceSupervisor { private liveChild(name: ServiceName): ChildProcess | undefined { const child = this.children.get(name); - return child && child.exitCode === null && !child.killed ? child : undefined; + return child && !hasExited(child) && !child.killed ? child : undefined; } private async childOwnsPort(child: ChildProcess, port: number): Promise { - if (!child.pid || child.exitCode !== null || child.killed) return false; + if (!child.pid || hasExited(child) || child.killed) return false; return listenerBelongsToProcess(child.pid, port, this.context.runCommand); } } @@ -201,13 +212,8 @@ export async function listenersOwnedBy( port: number, runCommand: CommandRunner, ): Promise { - const listeners = await runCommand("lsof", ["-nP", `-iTCP:${port}`, "-sTCP:LISTEN", "-t"], { - timeoutMs: 3_000, - }); - if (listeners.code !== 0) return []; const owned: number[] = []; - for (const line of listeners.stdout.split(/\r?\n/)) { - const listener = Number.parseInt(line.trim(), 10); + for (const listener of await listenersOnPort(port, runCommand)) { let pid = listener; for (let depth = 0; Number.isInteger(pid) && pid > 1 && depth < 12; depth += 1) { if (pid === ownerPid) { @@ -226,6 +232,27 @@ export async function listenersOwnedBy( return owned; } +/** PIDs currently listening on `port`, whoever owns them. */ +export async function listenersOnPort(port: number, runCommand: CommandRunner): Promise { + const listeners = await runCommand("lsof", ["-nP", `-iTCP:${port}`, "-sTCP:LISTEN", "-t"], { + timeoutMs: 3_000, + }); + if (listeners.code !== 0) return []; + return listeners.stdout + .split(/\r?\n/) + .map((line) => Number.parseInt(line.trim(), 10)) + .filter((pid) => Number.isInteger(pid) && pid > 0); +} + +/** + * Whether the child has actually exited. `exitCode` alone is not the test: it + * stays null for a process killed by a signal, which is exactly how this file + * stops things, so `exitCode === null` reads a dead child as still running. + */ +function hasExited(child: ChildProcess): boolean { + return child.exitCode !== null || child.signalCode !== null; +} + function signal(pid: number, name: "SIGTERM" | "SIGKILL"): void { try { process.kill(pid, name); @@ -274,14 +301,25 @@ async function waitForOwned( } async function stopChild(child: ChildProcess, graceMs: number): Promise { - if (child.exitCode !== null) return; + if (hasExited(child)) return; const exited = new Promise((resolve) => child.once("exit", () => resolve())); child.kill("SIGTERM"); const escalate = setTimeout(() => { - if (child.exitCode === null) child.kill("SIGKILL"); + if (!hasExited(child)) child.kill("SIGKILL"); }, graceMs); - await exited; - clearTimeout(escalate); + // A child that outlives SIGKILL (uninterruptible I/O, a stopped process) must + // not hang the chain switch with no way back: stop waiting and let stopProxy's + // port check decide, which is the same evidence it uses for the descendants. + let bail: ReturnType | undefined; + const timedOut = new Promise((resolve) => { + bail = setTimeout(resolve, graceMs * 2); + }); + try { + await Promise.race([exited, timedOut]); + } finally { + clearTimeout(escalate); + if (bail) clearTimeout(bail); + } } async function isPortOpen(port: number): Promise { diff --git a/apps/desktop/tests/supervisor.test.ts b/apps/desktop/tests/supervisor.test.ts index 44f4fe11..c143dacc 100644 --- a/apps/desktop/tests/supervisor.test.ts +++ b/apps/desktop/tests/supervisor.test.ts @@ -5,7 +5,11 @@ import { tmpdir } from "node:os"; import { join } from "node:path"; import { describe, expect, it } from "vitest"; -import { listenerBelongsToProcess, ServiceSupervisor } from "../electron/core/supervisor.js"; +import { + listenerBelongsToProcess, + listenersOnPort, + ServiceSupervisor, +} from "../electron/core/supervisor.js"; import { ensureServiceToken, verifyClawRouter } from "../electron/core/service-auth.js"; import type { AdapterContext } from "../electron/core/types.js"; @@ -142,6 +146,65 @@ describe("ServiceSupervisor ownership", () => { expect(alive(grandchild)).toBe(false); }); + it("does not hang on a tracked proxy something else already killed", async () => { + // A child killed by a signal keeps exitCode === null and killed === false, so + // reading liveness off exitCode alone treats a dead process as running and + // then waits for an "exit" that already fired. This must resolve, not hang. + const supervisor = new ServiceSupervisor( + await context(async () => response({ status: "ok", wallet: "0xabc" })), + async () => false, + ); + const child = spawn(process.execPath, ["-e", "setInterval(() => {}, 1000)"], { + stdio: "ignore", + }); + await new Promise((resolve) => child.once("spawn", () => resolve())); + process.kill(child.pid!, "SIGTERM"); // an outside kill, not child.kill() + await new Promise((resolve) => child.once("exit", () => resolve())); + expect(child.exitCode).toBeNull(); + expect(child.signalCode).toBe("SIGTERM"); + expect(child.killed).toBe(false); + (supervisor as unknown as { children: Map }).children.set("proxy", child); + let relaunched = 0; + supervisor.ensureProxy = async () => { + relaunched += 1; + }; + + await expect(supervisor.restartProxy()).resolves.toBe(false); + + expect(relaunched).toBe(0); + }, 5_000); + + it("clears the tracked proxy when the port refuses to close", async () => { + // stopProxy throws here; the child is already signalled, so it must not stay + // in the map claiming Desktop still owns a live proxy. + const supervisor = new ServiceSupervisor( + await context(async () => response({ status: "ok", wallet: "0xabc" })), + async () => true, // the port never closes + ); + const child = spawn(process.execPath, ["-e", "setInterval(() => {}, 1000)"], { + stdio: "ignore", + }); + const children = (supervisor as unknown as { children: Map }).children; + children.set("proxy", child); + + await expect(supervisor.restartProxy()).rejects.toThrow("still listening on port 8402"); + + expect(children.has("proxy")).toBe(false); + expect(child.signalCode).toBe("SIGTERM"); + }, 20_000); + + it("reads the listeners on a port without an ownership filter", async () => { + const pids = await listenersOnPort(8402, async () => ({ + code: 0, + stdout: "4242\n\n4343\nnot-a-pid\n", + stderr: "", + })); + expect(pids).toEqual([4242, 4343]); + await expect( + listenersOnPort(8402, async () => ({ code: 1, stdout: "", stderr: "" })), + ).resolves.toEqual([]); + }); + it("rejects a shape-compatible Codex bridge that Desktop did not start", async () => { const supervisor = new ServiceSupervisor( await context(async () => response({ data: [] })),