From 5307bcab12193a14e02916bbd969a77e8c28868e Mon Sep 17 00:00:00 2001 From: axisrow Date: Mon, 20 Jul 2026 01:32:01 +0800 Subject: [PATCH 1/2] feat(desktop): AO_KEEP_DAEMON to keep the daemon alive after the app closes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Opt-in env var: when set, the desktop app spawns its daemon without the OS-native supervisor link and skips the orphan-cleanup kill on exit, so the daemon persists across app quit and stops only on an explicit `ao stop`. Default (unset) is byte-for-byte unchanged — the daemon self-stops shortly after the app quits. The persistent-daemon mode is also a prerequisite for a detached / remote topology (a headless daemon in a container, attached from the desktop over an SSH tunnel): without it, closing the desktop window arms-then-fires the supervisor watchdog and kills every running agent session in the container. Mechanism: - daemonEnv() stamps AO_OWNER="persistent" under AO_KEEP_DAEMON (else "app"). The daemon writes AO_OWNER verbatim into running.json; the attach path reads that durable record (not this Electron process's env, which differs across launches) and skips the supervisor re-link for "persistent". A daemon spawned keep-alive stays persistent even when the app is later reopened without AO_KEEP_DAEMON. shouldLinkOnAttach("persistent") === false is a regression test. - Under the flag the daemon's stdio is redirected to ~/.ao/daemon.log (stdin ignored) and the child is unref()'d, so Electron-owned pipes no longer kill the kept-alive daemon on the next stderr log write. The parent's log fd is closed on the spawn-throw path and after a successful spawn so each keep-alive start/stop cycle does not accumulate descriptors. - The port-bound supervisor link and the stdio port-scanners are skipped under the flag; port discovery falls back to the running.json handshake. - keepDaemonAlive uses an explicit allowlist (1/true/yes/on); off/no and any unrecognized string keep the default self-stop behavior. - The process.on("exit") orphan-cleanup kill is skipped under the flag. Rebased clean onto current main (5532fa1b); the keyboard-shortcut refactor (PR #2795) that landed in the meantime is left untouched. Closes #2230 Co-Authored-By: Claude --- backend/internal/runfile/runfile.go | 10 +- docs/cli/README.md | 15 +-- frontend/src/main.ts | 147 +++++++++++++++++------- frontend/src/main/daemon-owner.test.ts | 45 +++++++- frontend/src/main/daemon-owner.ts | 24 +++- frontend/src/shared/daemon-discovery.ts | 7 +- 6 files changed, 193 insertions(+), 55 deletions(-) diff --git a/backend/internal/runfile/runfile.go b/backend/internal/runfile/runfile.go index f2383044df..57cc231872 100644 --- a/backend/internal/runfile/runfile.go +++ b/backend/internal/runfile/runfile.go @@ -24,10 +24,12 @@ type Info struct { Port int `json:"port"` // StartedAt is when the daemon came up (RFC 3339). StartedAt time.Time `json:"startedAt"` - // Owner is "app" when the desktop Electron app spawned this daemon; empty - // for a headless `ao start` daemon. Used by the app to decide whether to - // hold a supervisor link on attach (app-owned: re-link; headless: skip so - // the daemon stays persistent across app quit). + // Owner records how this daemon was spawned, so the app can decide whether + // to hold a supervisor link on attach from the daemon's own durable record + // rather than the current process env. "app" = normal desktop-spawned daemon + // (re-link on attach); "persistent" = spawned under AO_KEEP_DAEMON, stays + // alive across app quit and is never re-linked; empty = headless `ao start` + // daemon, stays persistent across app quit. Owner string `json:"owner,omitempty"` } diff --git a/docs/cli/README.md b/docs/cli/README.md index a94952ff65..df0f09dcb4 100644 --- a/docs/cli/README.md +++ b/docs/cli/README.md @@ -90,13 +90,14 @@ commands yet. The CLI and daemon share the same environment-driven config: -| Var | Default | Purpose | -| --------------------- | -------------------- | ---------------------- | -| `AO_PORT` | `3001` | Loopback daemon port. | -| `AO_RUN_FILE` | `~/.ao/running.json` | PID/port handshake. | -| `AO_DATA_DIR` | `~/.ao/data` | SQLite data directory. | -| `AO_REQUEST_TIMEOUT` | `60s` | REST request timeout. | -| `AO_SHUTDOWN_TIMEOUT` | `10s` | Graceful shutdown cap. | +| Var | Default | Purpose | +| --------------------- | -------------------- | ---------------------------------------------------------------------------------------------- | +| `AO_PORT` | `3001` | Loopback daemon port. | +| `AO_RUN_FILE` | `~/.ao/running.json` | PID/port handshake. | +| `AO_DATA_DIR` | `~/.ao/data` | SQLite data directory. | +| `AO_REQUEST_TIMEOUT` | `60s` | REST request timeout. | +| `AO_SHUTDOWN_TIMEOUT` | `10s` | Graceful shutdown cap. | +| `AO_KEEP_DAEMON` | unset (off) | Keep the desktop app's daemon running after the window closes; stop only via `ao stop`. (fork) | The daemon always binds `127.0.0.1`. diff --git a/frontend/src/main.ts b/frontend/src/main.ts index 340fd358bf..b519379cbb 100644 --- a/frontend/src/main.ts +++ b/frontend/src/main.ts @@ -28,8 +28,8 @@ import { type UpdateSettings, type UpdateStatus, } from "./main/update-settings"; -import { execFile, spawn, type ChildProcessWithoutNullStreams } from "node:child_process"; -import { existsSync } from "node:fs"; +import { execFile, spawn, type ChildProcess } from "node:child_process"; +import { closeSync, existsSync, openSync } from "node:fs"; import { mkdir, readdir, readFile, rm, stat, writeFile } from "node:fs/promises"; import os from "node:os"; import path from "node:path"; @@ -53,7 +53,7 @@ import { DEFAULT_POSTHOG_HOST, DEFAULT_POSTHOG_PROJECT_KEY } from "./shared/post import { buildTelemetryBootstrap } from "./shared/telemetry"; import { createBrowserViewHost, type BrowserViewHost } from "./main/browser-view-host"; import { connectSupervisor, type SupervisorLinkHandle } from "./main/supervisor-link"; -import { shouldLinkOnAttach } from "./main/daemon-owner"; +import { keepDaemonAlive, shouldLinkOnAttach } from "./main/daemon-owner"; import { readMigrationState, updateMigration, writeAppStateMarker, type MigrationState } from "./main/app-state"; import { isAllowedAppExternalURL, openAllowedAppExternalURL } from "./main/external-open"; @@ -93,8 +93,8 @@ if (process.platform === "win32") { app.setPath("userData", path.join(os.homedir(), ".ao", "electron")); let mainWindow: BrowserWindow | null = null; -let daemonProcess: ChildProcessWithoutNullStreams | null = null; -let daemonStoppingProcess: ChildProcessWithoutNullStreams | null = null; +let daemonProcess: ChildProcess | null = null; +let daemonStoppingProcess: ChildProcess | null = null; let daemonStartPromise: Promise | null = null; let daemonStartEpoch = 0; let daemonStatus: DaemonStatus = { state: "stopped" }; @@ -465,10 +465,14 @@ function ensureShellEnv(): Promise { } function daemonEnv(): NodeJS.ProcessEnv { - // AO_OWNER=app marks this daemon as app-spawned so the app can re-link the - // supervisor on attach (headless `ao start` daemons get no AO_OWNER and stay - // unlinked, preserving their persistence across app quit). - const ownerTag = { AO_OWNER: "app" }; + // AO_OWNER is the daemon's durable spawn-mode record: the daemon writes it + // into running.json and the attach path reads it to decide the supervisor + // link from the daemon's own state (not this Electron process's env, which + // differs across launches). A keep-alive daemon is "persistent" (never + // re-linked, survives app quit); a normal app-owned daemon is "app"; + // headless `ao start` sets none (stays unlinked, persistent by default). + const AO_OWNER = keepDaemonAlive(process.env) ? "persistent" : "app"; + const ownerTag = { AO_OWNER }; // In dev mode, inject isolation defaults so the dev daemon never collides with // the installed app. User-set env vars take priority (checked first). const devExtras: Record = {}; @@ -815,14 +819,60 @@ async function startDaemonInner(startEpoch: number): Promise { // runs the command through /bin/sh, a plain kill() would only signal the shell // wrapper and orphan the real daemon (which keeps holding the port). Killing // the whole group via killDaemon() reaches the daemon and any PTY children. - const child = spawn(launch.command, launch.args, { - cwd: launch.cwd, - env: daemonEnv(), - shell: launch.shell, - detached: true, - // Hide the daemon's console on a Windows GUI launch (no flashing terminal). - windowsHide: true, - }); + // + // AO_KEEP_DAEMON: the daemon must survive this app, so it cannot inherit + // Electron-owned stdout/stderr pipes — when Electron exits, the pipe read + // ends close and the daemon's next stderr log write (SIGPIPE/EPIPE) kills it, + // defeating the keep-alive. Redirect stdio to ~/.ao/daemon.log and unref the + // child so the parent does not wait on it. Port discovery then relies on the + // running.json handshake (the log pipe scan is skipped). + const keep = keepDaemonAlive(process.env); + let keepDaemonLogFd: number | undefined; + let stdio: "pipe" | "ignore" | ["ignore", number, number] = "pipe"; + if (keep) { + const logPath = path.join(os.homedir(), ".ao", "daemon.log"); + try { + keepDaemonLogFd = openSync(logPath, "a"); + stdio = ["ignore", keepDaemonLogFd, keepDaemonLogFd]; + } catch { + keepDaemonLogFd = undefined; + stdio = "ignore"; + } + } + let child: ChildProcess; + try { + child = spawn(launch.command, launch.args, { + cwd: launch.cwd, + env: daemonEnv(), + shell: launch.shell, + detached: true, + // Hide the daemon's console on a Windows GUI launch (no flashing terminal). + windowsHide: true, + stdio, + }); + } catch (err) { + // spawn can throw synchronously for invalid args; don't leak the log fd. + if (keepDaemonLogFd !== undefined) { + try { + closeSync(keepDaemonLogFd); + } catch { + // best-effort — the throw below is the real error + } + } + throw err; + } + // The child inherited its own copy of the log fd via stdio at spawn time; + // close the parent's copy now so each keep-alive start/stop cycle does not + // accumulate open descriptors until the process limit. + if (keepDaemonLogFd !== undefined) { + try { + closeSync(keepDaemonLogFd); + } catch { + // best-effort — the child still holds its inherited copy + } + keepDaemonLogFd = undefined; + } + if (keep) child.unref(); daemonProcess = child; // Discover the port the daemon ACTUALLY bound rather than trusting AO_PORT: @@ -847,31 +897,46 @@ async function startDaemonInner(startEpoch: number): Promise { stopDiscovery(); setDaemonStatus({ state: "ready", port }); - // Establish the OS-native liveness link unconditionally: this callback fires - // only on the spawn path (we own this daemon). Holding the connection keeps - // the daemon alive; when Electron exits for any reason, the OS closes the fd - // and the daemon detects EOF, then self-stops after its ~5s grace period. - // The attach paths link only when the daemon is app-owned (see - // establishSupervisorLink + shouldLinkOnAttach); headless `ao start` daemons - // stay unlinked so they remain persistent across app quit. - establishSupervisorLink(); + // Establish the OS-native liveness link on the spawn path (we own this + // daemon). Holding the connection keeps the daemon alive; when Electron + // exits for any reason, the OS closes the fd and the daemon detects EOF, + // then self-stops after its ~5s grace period. The attach paths link only + // when the daemon is app-owned (see establishSupervisorLink + + // shouldLinkOnAttach); headless `ao start` daemons stay unlinked so they + // remain persistent across app quit. + // + // AO_KEEP_DAEMON opts out of the link entirely: the daemon is spawned but + // survives the window closing, stopping only on an explicit `ao start`. + // Reuse the `keep` captured at spawn rather than re-reading process.env + // here: the flag is a property of this spawn, not a value that should be + // able to flip between spawn and port-confirmation. (The process.on("exit") + // orphan-cleanup below re-reads process.env because this `keep` is scoped + // to the spawn function — AO_KEEP_DAEMON is set once at startup and never + // mutated, so both reads agree.) + if (!keep) { + establishSupervisorLink(); + } }; // One scanner per stream: each keeps its own partial-line buffer. - const scanStdout = createListenPortScanner(reportBoundPort); - const scanStderr = createListenPortScanner(reportBoundPort); + // Skipped under AO_KEEP_DAEMON: stdio is redirected to a log file (no pipes + // to scan), so port discovery falls back to the running.json handshake below. + if (!keep) { + const scanStdout = createListenPortScanner(reportBoundPort); + const scanStderr = createListenPortScanner(reportBoundPort); - child.stdout.on("data", (chunk: Buffer) => { - const text = chunk.toString("utf8"); - console.log(text.trimEnd()); - scanStdout(text); - }); + child.stdout?.on("data", (chunk: Buffer) => { + const text = chunk.toString("utf8"); + console.log(text.trimEnd()); + scanStdout(text); + }); - child.stderr.on("data", (chunk: Buffer) => { - const text = chunk.toString("utf8"); - console.error(text.trimEnd()); - scanStderr(text); - }); + child.stderr?.on("data", (chunk: Buffer) => { + const text = chunk.toString("utf8"); + console.error(text.trimEnd()); + scanStderr(text); + }); + } const handshakePath = runFilePath(); if (handshakePath) { @@ -940,7 +1005,7 @@ async function startDaemonInner(startEpoch: number): Promise { // behind the /bin/sh wrapper (and any PTY children it forked), not just the // shell. Falls back to a direct kill if the group signal can't be delivered // (e.g. the process already exited). -function killDaemon(child: ChildProcessWithoutNullStreams): void { +function killDaemon(child: ChildProcess): void { if (child.pid === undefined) return; try { process.kill(-child.pid, "SIGTERM"); @@ -1411,8 +1476,12 @@ app.on("before-quit", () => { // exits without tearing down sessions, which survive for the next boot to adopt. // When the link IS connected we do nothing here and rely on the OS closing the // fd on exit, which covers crash and SIGKILL uniformly. +// +// AO_KEEP_DAEMON opts out entirely: the daemon is deliberately spawned without a +// supervisor link so it persists across app quit, so this orphan-cleanup kill +// must be skipped — otherwise it would defeat the whole point on quit. process.on("exit", () => { - if (daemonProcess && !supervisorLink?.connected) { + if (daemonProcess && !supervisorLink?.connected && !keepDaemonAlive(process.env)) { killDaemon(daemonProcess); } }); diff --git a/frontend/src/main/daemon-owner.test.ts b/frontend/src/main/daemon-owner.test.ts index 806dc4ba05..674b88c123 100644 --- a/frontend/src/main/daemon-owner.test.ts +++ b/frontend/src/main/daemon-owner.test.ts @@ -1,6 +1,6 @@ // @vitest-environment node import { describe, it, expect } from "vitest"; -import { shouldLinkOnAttach } from "./daemon-owner"; +import { keepDaemonAlive, shouldLinkOnAttach } from "./daemon-owner"; describe("shouldLinkOnAttach", () => { it('returns true when owner is "app"', () => { @@ -18,4 +18,47 @@ describe("shouldLinkOnAttach", () => { it('returns false when owner is "cli"', () => { expect(shouldLinkOnAttach("cli")).toBe(false); }); + + // Cross-launch regression (PR #2231 review): a daemon spawned with + // AO_KEEP_DAEMON is stamped owner:"persistent" in running.json. A LATER + // launch of the app — which may have AO_KEEP_DAEMON unset — must NOT + // re-establish the supervisor link from that durable owner, or closing the + // second instance would kill the supposedly-persistent daemon. The decision + // is read only from the daemon's record, never from the current process env. + it("does NOT re-link a persistent daemon on attach, even when AO_KEEP_DAEMON is unset now", () => { + expect(shouldLinkOnAttach("persistent")).toBe(false); + }); +}); + +describe("keepDaemonAlive", () => { + it("returns false when AO_KEEP_DAEMON is unset", () => { + expect(keepDaemonAlive({})).toBe(false); + }); + + it("returns false when AO_KEEP_DAEMON is empty", () => { + expect(keepDaemonAlive({ AO_KEEP_DAEMON: "" })).toBe(false); + }); + + it.each(["1", "true", "TRUE", "yes", "on", "ON", "Yes"])("returns true for truthy value %j", (value) => { + expect(keepDaemonAlive({ AO_KEEP_DAEMON: value })).toBe(true); + }); + + // Explicit allowlist (PR #2231 review): conventional falsy values and any + // unrecognized string must NOT retain the daemon — "off"/"no" previously + // fell through the old "anything but 0/false" check and kept it alive. + it.each(["0", "false", "FALSE", "off", "OFF", "no", "No"])("returns false for conventional off value %j", (value) => { + expect(keepDaemonAlive({ AO_KEEP_DAEMON: value })).toBe(false); + }); + + it.each(["2", "random", "yep", "disable"])( + "returns false for unrecognized value %j (allowlist, not truthiness)", + (value) => { + expect(keepDaemonAlive({ AO_KEEP_DAEMON: value })).toBe(false); + }, + ); + + it("trims surrounding whitespace before evaluating", () => { + expect(keepDaemonAlive({ AO_KEEP_DAEMON: " 0 " })).toBe(false); + expect(keepDaemonAlive({ AO_KEEP_DAEMON: " 1 " })).toBe(true); + }); }); diff --git a/frontend/src/main/daemon-owner.ts b/frontend/src/main/daemon-owner.ts index 771060165e..973984b3b0 100644 --- a/frontend/src/main/daemon-owner.ts +++ b/frontend/src/main/daemon-owner.ts @@ -1,7 +1,27 @@ +/** + * Whether the user opted the app's own daemon out of the app-lifetime link via + * the AO_KEEP_DAEMON env var. When set to an explicit truthy value ("1", + * "true", "yes", "on"), the app spawns the daemon but does NOT hold the + * supervisor link, so the daemon survives the window closing and stops only on + * an explicit `ao stop`. Any other value — including conventional falsy ones + * like "0"/"false"/"off"/"no" and unrecognized strings — is treated as unset, so + * the default desktop behavior holds: the daemon self-stops shortly after the + * app quits. An allowlist (rather than "anything but 0/false") keeps "off"/"no" + * from unexpectedly retaining the daemon. + */ +export function keepDaemonAlive(env: { AO_KEEP_DAEMON?: string }): boolean { + const raw = env.AO_KEEP_DAEMON?.trim().toLowerCase(); + return raw === "1" || raw === "true" || raw === "yes" || raw === "on"; +} + /** * Whether the app should hold a supervisor link to a daemon it ATTACHED to - * (did not spawn). Only re-link app-owned daemons (owner === "app"); leave - * headless `ao start` daemons (owner unset or empty) unlinked so they stay + * (did not spawn). The decision is read from the daemon's durable owner record + * in running.json — NOT the current Electron process env, which can differ + * across launches (a cross-launch regression: a daemon spawned keep-alive must + * stay unlinked even when the app is later reopened without AO_KEEP_DAEMON). + * Only a normal app-owned daemon ("app") is linked; a keep-alive daemon + * ("persistent") and headless `ao start` daemons (owner unset/empty) stay * persistent across app quit. */ export function shouldLinkOnAttach(owner: string | undefined): boolean { diff --git a/frontend/src/shared/daemon-discovery.ts b/frontend/src/shared/daemon-discovery.ts index 9e9f608429..a70f804959 100644 --- a/frontend/src/shared/daemon-discovery.ts +++ b/frontend/src/shared/daemon-discovery.ts @@ -67,8 +67,11 @@ export type RunFileInfo = { /** startedAt in epoch ms; 0 when missing/unparseable. */ startedAtMs: number; /** - * Daemon ownership tag. "app" when the desktop app spawned this daemon; - * undefined/empty for a headless `ao start` daemon. + * Daemon ownership tag — read from running.json so the attach-path link + * decision uses the daemon's durable record, not the current process env. + * "app" = desktop-spawned (re-link on attach); "persistent" = spawned under + * AO_KEEP_DAEMON (stays alive across app quit, never re-linked); + * undefined/empty = headless `ao start` daemon. */ owner?: string; }; From 8d24a94c09933935f59e20f116c550c3a9c09dfa Mon Sep 17 00:00:00 2001 From: axisrow Date: Mon, 20 Jul 2026 01:48:16 +0800 Subject: [PATCH 2/2] chore: apply non-blocking review nitpicks before merge MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Local review (Claude subagent + Codex companion), cycle 1. No critical findings in scope; two non-blocking cleanups applied: - Warn when the keep-daemon log redirect fails (openSync throws). Previously the catch silently fell back to stdio "ignore", leaving a long-lived keep-alive daemon with zero log output and no signal. Now mirrors the supervisor-link-skip warning. - Fix a doc-comment typo: "stopping only on an explicit `ao stop`" (was `ao start`). Deferred (out of scope, follow-up): the auto-update + persistent-daemon interaction Codex flagged — a keep-alive daemon survives quitAndInstall, so the relaunched UI can attach to an old bundled backend. This is the same P1 raised on the combined PR and is best handled as its own follow-up (version negotiation or kill-on-update); it does not affect self-build (where the auto-updater is disabled) and is inherent to persistent mode, not a bug in this PR's opt-in env-var. Co-Authored-By: Claude --- frontend/src/main.ts | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/frontend/src/main.ts b/frontend/src/main.ts index b519379cbb..ff5a05c74c 100644 --- a/frontend/src/main.ts +++ b/frontend/src/main.ts @@ -835,6 +835,10 @@ async function startDaemonInner(startEpoch: number): Promise { keepDaemonLogFd = openSync(logPath, "a"); stdio = ["ignore", keepDaemonLogFd, keepDaemonLogFd]; } catch { + // Log redirect failed (e.g. ~/.ao not creatable, permission denied): + // fall back to "ignore" so the daemon still runs, but warn — otherwise + // a long-lived keep-alive daemon would run with zero log output. + console.warn(`AO: keep-daemon log redirect failed; daemon will run with stdio disabled: ${logPath}`); keepDaemonLogFd = undefined; stdio = "ignore"; } @@ -906,7 +910,7 @@ async function startDaemonInner(startEpoch: number): Promise { // remain persistent across app quit. // // AO_KEEP_DAEMON opts out of the link entirely: the daemon is spawned but - // survives the window closing, stopping only on an explicit `ao start`. + // survives the window closing, stopping only on an explicit `ao stop`. // Reuse the `keep` captured at spawn rather than re-reading process.env // here: the flag is a property of this spawn, not a value that should be // able to flip between spawn and port-confirmation. (The process.on("exit")