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..bcec1f12 100644 --- a/apps/desktop/electron/core/supervisor.ts +++ b/apps/desktop/electron/core/supervisor.ts @@ -8,6 +8,10 @@ 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; +const PROXY_PORT = 8402; + export class ServiceSupervisor { private readonly children = new Map(); @@ -98,6 +102,65 @@ 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; + 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; + } + + /** + * 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; + // 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.`, + ); + } + + 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"); @@ -112,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"], @@ -126,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); } } @@ -140,14 +203,23 @@ export async function listenerBelongsToProcess( 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; - for (const line of listeners.stdout.split(/\r?\n/)) { - let pid = Number.parseInt(line.trim(), 10); + 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 owned: number[] = []; + 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) return true; + if (pid === ownerPid) { + owned.push(listener); + break; + } const parent = await runCommand("ps", ["-o", "ppid=", "-p", String(pid)], { timeoutMs: 3_000, }); @@ -157,7 +229,36 @@ export async function listenerBelongsToProcess( pid = next; } } - return false; + 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); + } catch { + // Already gone, or not ours to signal; the port check decides what happens next. + } } async function isModelService(url: string, fetcher: typeof fetch): Promise { @@ -199,6 +300,28 @@ async function waitForOwned( throw new Error(`Service did not become healthy: ${url}`); } +async function stopChild(child: ChildProcess, graceMs: number): Promise { + if (hasExited(child)) return; + const exited = new Promise((resolve) => child.once("exit", () => resolve())); + child.kill("SIGTERM"); + const escalate = setTimeout(() => { + if (!hasExited(child)) child.kill("SIGKILL"); + }, graceMs); + // 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 { 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..c143dacc 100644 --- a/apps/desktop/tests/supervisor.test.ts +++ b/apps/desktop/tests/supervisor.test.ts @@ -1,10 +1,15 @@ +import { spawn, type ChildProcess } from "node:child_process"; import { mkdtemp } from "node:fs/promises"; import { createHmac } from "node:crypto"; 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"; @@ -52,6 +57,154 @@ 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("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("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: [] })),