fix(desktop): attach to a serving daemon instead of spawning a doomed child (#373)
* fix(desktop): attach to a serving daemon instead of spawning a doomed child Launching the Electron app while a standalone `ao daemon` already owns the port made the Electron-spawned child daemon log "daemon already running … refusing to start" and exit 1, instead of attaching to the running daemon. `inspectExistingDaemon` only attaches when ~/.ao/running.json agrees with a live daemon, so any run-file divergence (missing/stale/unparseable file, dead PID, or a /healthz pid mismatch) made it return null — and there was no independent port probe before spawn(), so Electron spawned into an occupied port and the Go bind guard correctly refused. Add a defensive direct probe of http://127.0.0.1:<expectedPort>/healthz in startDaemonInner, after the run-file check and before spawn(): if a genuine daemon answers, attach to it (the same "ready" DaemonStatus shape the run-file path returns) instead of spawning. The expected port is resolved the same way startup does (AO_PORT or the default). The attach-or-spawn decision is extracted into a pure, dependency-injected module (shared/daemon-attach.ts) so it can be exercised directly; main.ts keeps ownership of fs reads, process signals, fetch, and the path identity check. Covered by 27 tests, including real loopback-server cases that reproduce the issue scenario end to end. Fixes #367 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * chore: format with prettier [skip ci] * fix(desktop): enforce readiness + identity checks on the port-probe attach path Address review feedback on #367: the new direct port probe attached to any service-matching daemon as soon as /healthz returned ok, without the /readyz and foreign-binary identity checks the run-file path enforces. That reopened the mismatch daemonIdentityError was built to prevent — Electron could silently drive a different/older AO build serving the port — and could mark a still-starting daemon "ready". Extract the shared post-handshake tail (readinessStatus) and run it from both paths, anchoring on the PID /healthz reports for the port probe. A serving daemon that is not ready, or whose binary the identity check refuses, now yields the same "error" DaemonStatus instead of attaching — strictly safer than spawning, which would only collide on the occupied port and die. resolveDaemonFromPort now takes the same identityError dependency resolveDaemonFromRunFile does; main.ts passes daemonIdentityError(launch, …). Adds port-path tests for not-ready, foreign-binary identity (unit), and a real-server foreign-binary scenario (e2e). 30 tests in daemon-attach. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * chore: format with prettier [skip ci] * docs(desktop): note why the port probe uses the expected (not hardcoded) port Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com> Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
This commit is contained in:
Pritom Mazumdar
GitHub
parent
348fd414d1
commit
cbd2a1baba
|
|
@ -21,6 +21,13 @@ import { pathToFileURL } from "node:url";
|
|||
import { type DaemonLaunchSpec, resolveDaemonLaunch } from "./shared/daemon-launch";
|
||||
import { createListenPortScanner, defaultRunFilePath, parseRunFile } from "./shared/daemon-discovery";
|
||||
import type { DaemonStatus } from "./shared/daemon-status";
|
||||
import {
|
||||
type DaemonProbe,
|
||||
expectedDaemonPort,
|
||||
parseDaemonProbe,
|
||||
resolveDaemonFromPort,
|
||||
resolveDaemonFromRunFile,
|
||||
} from "./shared/daemon-attach";
|
||||
import { DEFAULT_POSTHOG_HOST, DEFAULT_POSTHOG_PROJECT_KEY } from "./shared/posthog-config";
|
||||
import { buildTelemetryBootstrap } from "./shared/telemetry";
|
||||
import { createBrowserViewHost, type BrowserViewHost } from "./main/browser-view-host";
|
||||
|
|
@ -196,15 +203,6 @@ const RUN_FILE_POLL_MS = 300;
|
|||
// clock reading and ours race within normal scheduling jitter.
|
||||
const RUN_FILE_FRESHNESS_SKEW_MS = 2_000;
|
||||
const DAEMON_PROBE_TIMEOUT_MS = 2_000;
|
||||
const DAEMON_SERVICE_NAME = "agent-orchestrator-daemon";
|
||||
|
||||
type DaemonProbe = {
|
||||
status: string;
|
||||
service: string;
|
||||
pid: number;
|
||||
executablePath?: string;
|
||||
workingDirectory?: string;
|
||||
};
|
||||
|
||||
function runFilePath(): string | null {
|
||||
if (process.env.AO_RUN_FILE) return process.env.AO_RUN_FILE;
|
||||
|
|
@ -252,17 +250,7 @@ async function readDaemonProbe(port: number, endpoint: "healthz" | "readyz"): Pr
|
|||
try {
|
||||
const response = await net.fetch(`http://127.0.0.1:${port}/${endpoint}`, { signal: controller.signal });
|
||||
if (!response.ok) return null;
|
||||
const body = (await response.json()) as Partial<DaemonProbe>;
|
||||
if (body.status !== (endpoint === "healthz" ? "ok" : "ready")) return null;
|
||||
if (body.service !== DAEMON_SERVICE_NAME) return null;
|
||||
if (typeof body.pid !== "number" || !Number.isInteger(body.pid)) return null;
|
||||
return {
|
||||
status: body.status,
|
||||
service: body.service,
|
||||
pid: body.pid,
|
||||
executablePath: typeof body.executablePath === "string" ? body.executablePath : undefined,
|
||||
workingDirectory: typeof body.workingDirectory === "string" ? body.workingDirectory : undefined,
|
||||
};
|
||||
return parseDaemonProbe(endpoint, await response.json());
|
||||
} catch {
|
||||
return null;
|
||||
} finally {
|
||||
|
|
@ -297,49 +285,20 @@ function daemonIdentityError(launch: DaemonLaunchSpec, probe: DaemonProbe): stri
|
|||
|
||||
async function inspectExistingDaemon(launch: DaemonLaunchSpec): Promise<DaemonStatus | null> {
|
||||
const handshakePath = runFilePath();
|
||||
if (!handshakePath) return null;
|
||||
let contents: string;
|
||||
try {
|
||||
contents = await readFile(handshakePath, "utf8");
|
||||
} catch {
|
||||
return null;
|
||||
let runFileContents: string | null = null;
|
||||
if (handshakePath) {
|
||||
try {
|
||||
runFileContents = await readFile(handshakePath, "utf8");
|
||||
} catch {
|
||||
runFileContents = null;
|
||||
}
|
||||
}
|
||||
const info = parseRunFile(contents);
|
||||
if (!info || !processAlive(info.pid)) return null;
|
||||
|
||||
const health = await readDaemonProbe(info.port, "healthz");
|
||||
if (!health || health.pid !== info.pid) return null;
|
||||
const ready = await readDaemonProbe(info.port, "readyz");
|
||||
if (!ready || ready.pid !== info.pid) {
|
||||
return {
|
||||
state: "error",
|
||||
port: info.port,
|
||||
pid: info.pid,
|
||||
executablePath: health.executablePath,
|
||||
workingDirectory: health.workingDirectory,
|
||||
message: "An AO daemon is already running, but it is not ready yet.",
|
||||
};
|
||||
}
|
||||
|
||||
const identityError = daemonIdentityError(launch, ready);
|
||||
if (identityError) {
|
||||
return {
|
||||
state: "error",
|
||||
port: info.port,
|
||||
pid: info.pid,
|
||||
executablePath: ready.executablePath,
|
||||
workingDirectory: ready.workingDirectory,
|
||||
message: identityError,
|
||||
};
|
||||
}
|
||||
|
||||
return {
|
||||
state: "ready",
|
||||
port: info.port,
|
||||
pid: info.pid,
|
||||
executablePath: ready.executablePath,
|
||||
workingDirectory: ready.workingDirectory,
|
||||
};
|
||||
return resolveDaemonFromRunFile({
|
||||
runFileContents,
|
||||
isProcessAlive: processAlive,
|
||||
probe: readDaemonProbe,
|
||||
identityError: (probe) => daemonIdentityError(launch, probe),
|
||||
});
|
||||
}
|
||||
|
||||
async function refreshDaemonStatus(): Promise<DaemonStatus> {
|
||||
|
|
@ -412,6 +371,27 @@ async function startDaemonInner(startEpoch: number): Promise<DaemonStatus> {
|
|||
return daemonStatus;
|
||||
}
|
||||
|
||||
// Defensive: inspectExistingDaemon only attaches when the run-file agrees with
|
||||
// a live daemon. Any divergence (missing/stale/unparseable run-file, dead PID,
|
||||
// health.pid mismatch) makes it return null — yet a daemon may still be serving
|
||||
// the port. Spawning then would just make the Go child refuse and exit 1. Probe
|
||||
// the expected port directly, independent of the run-file, and attach if a
|
||||
// daemon answers. The expected port (AO_PORT or the default) is exactly the
|
||||
// port the Go child would bind and collide on — probing a hardcoded 3001 would
|
||||
// miss an AO_PORT override.
|
||||
const directDaemon = await resolveDaemonFromPort({
|
||||
expectedPort: expectedDaemonPort(process.env),
|
||||
probe: readDaemonProbe,
|
||||
identityError: (probe) => daemonIdentityError(launch, probe),
|
||||
});
|
||||
if (startEpoch !== daemonStartEpoch) {
|
||||
return daemonStatus;
|
||||
}
|
||||
if (directDaemon) {
|
||||
setDaemonStatus(directDaemon);
|
||||
return daemonStatus;
|
||||
}
|
||||
|
||||
if (launch.source === "bundled" && !existsSync(launch.command)) {
|
||||
setDaemonStatus({
|
||||
state: "error",
|
||||
|
|
|
|||
|
|
@ -0,0 +1,494 @@
|
|||
import { createServer, type Server } from "node:http";
|
||||
import { afterEach, describe, expect, it, vi } from "vitest";
|
||||
import {
|
||||
DAEMON_SERVICE_NAME,
|
||||
DEFAULT_DAEMON_PORT,
|
||||
type DaemonProbe,
|
||||
type DaemonProber,
|
||||
expectedDaemonPort,
|
||||
parseDaemonProbe,
|
||||
resolveDaemonFromPort,
|
||||
resolveDaemonFromRunFile,
|
||||
} from "./daemon-attach";
|
||||
|
||||
// A run-file the daemon would write: pid+port+timestamp, as JSON.
|
||||
function runFile(pid: number, port: number): string {
|
||||
return JSON.stringify({ pid, port, startedAt: "2026-06-10T16:15:04Z" });
|
||||
}
|
||||
|
||||
// A probe map keyed by `${port}:${endpoint}` → fake an HTTP probe deterministically.
|
||||
function fakeProbe(responses: Record<string, DaemonProbe | null>): DaemonProber {
|
||||
return (port, endpoint) => Promise.resolve(responses[`${port}:${endpoint}`] ?? null);
|
||||
}
|
||||
|
||||
const ALIVE = () => true;
|
||||
const DEAD = () => false;
|
||||
const NO_IDENTITY_ERROR = () => null;
|
||||
|
||||
describe("expectedDaemonPort", () => {
|
||||
it("defaults to 3001 when AO_PORT is unset or empty", () => {
|
||||
expect(expectedDaemonPort({})).toBe(3001);
|
||||
expect(expectedDaemonPort({ AO_PORT: "" })).toBe(3001);
|
||||
expect(DEFAULT_DAEMON_PORT).toBe(3001);
|
||||
});
|
||||
|
||||
it("honors a valid AO_PORT override", () => {
|
||||
expect(expectedDaemonPort({ AO_PORT: "3037" })).toBe(3037);
|
||||
expect(expectedDaemonPort({ AO_PORT: "1" })).toBe(1);
|
||||
expect(expectedDaemonPort({ AO_PORT: "65535" })).toBe(65535);
|
||||
});
|
||||
|
||||
it("falls back to the default for an out-of-range or non-integer AO_PORT", () => {
|
||||
expect(expectedDaemonPort({ AO_PORT: "0" })).toBe(3001);
|
||||
expect(expectedDaemonPort({ AO_PORT: "70000" })).toBe(3001);
|
||||
expect(expectedDaemonPort({ AO_PORT: "3001.5" })).toBe(3001);
|
||||
expect(expectedDaemonPort({ AO_PORT: "not-a-number" })).toBe(3001);
|
||||
expect(expectedDaemonPort({ AO_PORT: "-1" })).toBe(3001);
|
||||
});
|
||||
});
|
||||
|
||||
describe("parseDaemonProbe", () => {
|
||||
const healthBody = { status: "ok", service: DAEMON_SERVICE_NAME, pid: 4242 };
|
||||
const readyBody = { status: "ready", service: DAEMON_SERVICE_NAME, pid: 4242 };
|
||||
|
||||
it("accepts a well-formed healthz body", () => {
|
||||
expect(parseDaemonProbe("healthz", healthBody)).toEqual({
|
||||
status: "ok",
|
||||
service: DAEMON_SERVICE_NAME,
|
||||
pid: 4242,
|
||||
executablePath: undefined,
|
||||
workingDirectory: undefined,
|
||||
});
|
||||
});
|
||||
|
||||
it("accepts a well-formed readyz body and carries identity fields", () => {
|
||||
expect(parseDaemonProbe("readyz", { ...readyBody, executablePath: "/bin/ao", workingDirectory: "/work" })).toEqual({
|
||||
status: "ready",
|
||||
service: DAEMON_SERVICE_NAME,
|
||||
pid: 4242,
|
||||
executablePath: "/bin/ao",
|
||||
workingDirectory: "/work",
|
||||
});
|
||||
});
|
||||
|
||||
it("rejects a status that does not match the endpoint", () => {
|
||||
expect(parseDaemonProbe("healthz", readyBody)).toBeNull();
|
||||
expect(parseDaemonProbe("readyz", healthBody)).toBeNull();
|
||||
});
|
||||
|
||||
it("rejects a foreign or missing service", () => {
|
||||
expect(parseDaemonProbe("healthz", { ...healthBody, service: "something-else" })).toBeNull();
|
||||
expect(parseDaemonProbe("healthz", { status: "ok", pid: 1 })).toBeNull();
|
||||
});
|
||||
|
||||
it("rejects a missing or non-integer pid", () => {
|
||||
expect(parseDaemonProbe("healthz", { status: "ok", service: DAEMON_SERVICE_NAME })).toBeNull();
|
||||
expect(parseDaemonProbe("healthz", { ...healthBody, pid: 1.5 })).toBeNull();
|
||||
expect(parseDaemonProbe("healthz", { ...healthBody, pid: "4242" })).toBeNull();
|
||||
});
|
||||
|
||||
it("rejects non-object bodies", () => {
|
||||
expect(parseDaemonProbe("healthz", null)).toBeNull();
|
||||
expect(parseDaemonProbe("healthz", "ok")).toBeNull();
|
||||
expect(parseDaemonProbe("healthz", 200)).toBeNull();
|
||||
});
|
||||
|
||||
it("drops identity fields that are not strings", () => {
|
||||
const probe = parseDaemonProbe("healthz", { ...healthBody, executablePath: 5, workingDirectory: {} });
|
||||
expect(probe?.executablePath).toBeUndefined();
|
||||
expect(probe?.workingDirectory).toBeUndefined();
|
||||
});
|
||||
});
|
||||
|
||||
describe("resolveDaemonFromRunFile", () => {
|
||||
it("returns null when there is no run-file (caller falls through to port probe)", async () => {
|
||||
const probe = vi.fn<DaemonProber>();
|
||||
const result = await resolveDaemonFromRunFile({
|
||||
runFileContents: null,
|
||||
isProcessAlive: ALIVE,
|
||||
probe,
|
||||
identityError: NO_IDENTITY_ERROR,
|
||||
});
|
||||
expect(result).toBeNull();
|
||||
expect(probe).not.toHaveBeenCalled(); // never probes without a parseable run-file
|
||||
});
|
||||
|
||||
it("returns null for an unparseable run-file", async () => {
|
||||
const result = await resolveDaemonFromRunFile({
|
||||
runFileContents: "{not json",
|
||||
isProcessAlive: ALIVE,
|
||||
probe: fakeProbe({}),
|
||||
identityError: NO_IDENTITY_ERROR,
|
||||
});
|
||||
expect(result).toBeNull();
|
||||
});
|
||||
|
||||
it("returns null when the recorded pid is not alive (#367 divergence)", async () => {
|
||||
const result = await resolveDaemonFromRunFile({
|
||||
runFileContents: runFile(4242, 3001),
|
||||
isProcessAlive: DEAD,
|
||||
probe: fakeProbe({
|
||||
"3001:healthz": { status: "ok", service: DAEMON_SERVICE_NAME, pid: 4242 },
|
||||
}),
|
||||
identityError: NO_IDENTITY_ERROR,
|
||||
});
|
||||
expect(result).toBeNull();
|
||||
});
|
||||
|
||||
it("returns null when the health probe fails (#367 divergence)", async () => {
|
||||
const result = await resolveDaemonFromRunFile({
|
||||
runFileContents: runFile(4242, 3001),
|
||||
isProcessAlive: ALIVE,
|
||||
probe: fakeProbe({ "3001:healthz": null }),
|
||||
identityError: NO_IDENTITY_ERROR,
|
||||
});
|
||||
expect(result).toBeNull();
|
||||
});
|
||||
|
||||
it("returns null when health.pid disagrees with the run-file pid (#367 divergence)", async () => {
|
||||
const result = await resolveDaemonFromRunFile({
|
||||
runFileContents: runFile(4242, 3001),
|
||||
isProcessAlive: ALIVE,
|
||||
probe: fakeProbe({
|
||||
"3001:healthz": { status: "ok", service: DAEMON_SERVICE_NAME, pid: 9999 },
|
||||
}),
|
||||
identityError: NO_IDENTITY_ERROR,
|
||||
});
|
||||
expect(result).toBeNull();
|
||||
});
|
||||
|
||||
it("reports an error (not a spawn) when the daemon is up but not ready", async () => {
|
||||
const result = await resolveDaemonFromRunFile({
|
||||
runFileContents: runFile(4242, 3001),
|
||||
isProcessAlive: ALIVE,
|
||||
probe: fakeProbe({
|
||||
"3001:healthz": { status: "ok", service: DAEMON_SERVICE_NAME, pid: 4242 },
|
||||
"3001:readyz": null,
|
||||
}),
|
||||
identityError: NO_IDENTITY_ERROR,
|
||||
});
|
||||
expect(result).toEqual({
|
||||
state: "error",
|
||||
port: 3001,
|
||||
pid: 4242,
|
||||
executablePath: undefined,
|
||||
workingDirectory: undefined,
|
||||
message: "An AO daemon is already running, but it is not ready yet.",
|
||||
});
|
||||
});
|
||||
|
||||
it("surfaces a foreign-daemon identity error", async () => {
|
||||
const result = await resolveDaemonFromRunFile({
|
||||
runFileContents: runFile(4242, 3001),
|
||||
isProcessAlive: ALIVE,
|
||||
probe: fakeProbe({
|
||||
"3001:healthz": { status: "ok", service: DAEMON_SERVICE_NAME, pid: 4242 },
|
||||
"3001:readyz": { status: "ready", service: DAEMON_SERVICE_NAME, pid: 4242, workingDirectory: "/other" },
|
||||
}),
|
||||
identityError: () => "Another AO daemon is already running from /other.",
|
||||
});
|
||||
expect(result).toMatchObject({
|
||||
state: "error",
|
||||
pid: 4242,
|
||||
port: 3001,
|
||||
message: "Another AO daemon is already running from /other.",
|
||||
});
|
||||
});
|
||||
|
||||
it("attaches (ready) when the run-file, liveness, health, readiness, and identity all agree", async () => {
|
||||
const result = await resolveDaemonFromRunFile({
|
||||
runFileContents: runFile(4242, 3037),
|
||||
isProcessAlive: ALIVE,
|
||||
probe: fakeProbe({
|
||||
"3037:healthz": { status: "ok", service: DAEMON_SERVICE_NAME, pid: 4242 },
|
||||
"3037:readyz": {
|
||||
status: "ready",
|
||||
service: DAEMON_SERVICE_NAME,
|
||||
pid: 4242,
|
||||
executablePath: "/bin/ao",
|
||||
workingDirectory: "/work/backend",
|
||||
},
|
||||
}),
|
||||
identityError: NO_IDENTITY_ERROR,
|
||||
});
|
||||
expect(result).toEqual({
|
||||
state: "ready",
|
||||
port: 3037,
|
||||
pid: 4242,
|
||||
executablePath: "/bin/ao",
|
||||
workingDirectory: "/work/backend",
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
describe("resolveDaemonFromPort", () => {
|
||||
it("returns null when nothing valid answers the port (caller spawns)", async () => {
|
||||
const result = await resolveDaemonFromPort({
|
||||
expectedPort: 3001,
|
||||
probe: fakeProbe({}),
|
||||
identityError: NO_IDENTITY_ERROR,
|
||||
});
|
||||
expect(result).toBeNull();
|
||||
});
|
||||
|
||||
it("attaches (ready) when a daemon answers /healthz and /readyz on the expected port", async () => {
|
||||
const result = await resolveDaemonFromPort({
|
||||
expectedPort: 3001,
|
||||
probe: fakeProbe({
|
||||
"3001:healthz": { status: "ok", service: DAEMON_SERVICE_NAME, pid: 777 },
|
||||
"3001:readyz": {
|
||||
status: "ready",
|
||||
service: DAEMON_SERVICE_NAME,
|
||||
pid: 777,
|
||||
executablePath: "/bin/ao",
|
||||
workingDirectory: "/work",
|
||||
},
|
||||
}),
|
||||
identityError: NO_IDENTITY_ERROR,
|
||||
});
|
||||
expect(result).toEqual({
|
||||
state: "ready",
|
||||
port: 3001,
|
||||
pid: 777,
|
||||
executablePath: "/bin/ao",
|
||||
workingDirectory: "/work",
|
||||
});
|
||||
});
|
||||
|
||||
it("reports an error (not a spawn) when the serving daemon is not ready yet", async () => {
|
||||
const result = await resolveDaemonFromPort({
|
||||
expectedPort: 3001,
|
||||
probe: fakeProbe({
|
||||
"3001:healthz": { status: "ok", service: DAEMON_SERVICE_NAME, pid: 777 },
|
||||
"3001:readyz": null,
|
||||
}),
|
||||
identityError: NO_IDENTITY_ERROR,
|
||||
});
|
||||
expect(result).toEqual({
|
||||
state: "error",
|
||||
port: 3001,
|
||||
pid: 777,
|
||||
executablePath: undefined,
|
||||
workingDirectory: undefined,
|
||||
message: "An AO daemon is already running, but it is not ready yet.",
|
||||
});
|
||||
});
|
||||
|
||||
it("reports an identity error (not a silent attach) for a foreign daemon binary on the port", async () => {
|
||||
const result = await resolveDaemonFromPort({
|
||||
expectedPort: 3001,
|
||||
probe: fakeProbe({
|
||||
"3001:healthz": { status: "ok", service: DAEMON_SERVICE_NAME, pid: 777 },
|
||||
"3001:readyz": { status: "ready", service: DAEMON_SERVICE_NAME, pid: 777, executablePath: "/old/ao" },
|
||||
}),
|
||||
identityError: (probe) =>
|
||||
probe.executablePath === "/new/ao"
|
||||
? null
|
||||
: `Another AO daemon is already running from ${probe.executablePath}.`,
|
||||
});
|
||||
expect(result).toMatchObject({
|
||||
state: "error",
|
||||
port: 3001,
|
||||
pid: 777,
|
||||
message: "Another AO daemon is already running from /old/ao.",
|
||||
});
|
||||
});
|
||||
|
||||
it("probes exactly the expected port", async () => {
|
||||
const probe = vi.fn<DaemonProber>().mockResolvedValue(null);
|
||||
await resolveDaemonFromPort({ expectedPort: 4317, probe, identityError: NO_IDENTITY_ERROR });
|
||||
expect(probe).toHaveBeenCalledWith(4317, "healthz");
|
||||
});
|
||||
});
|
||||
|
||||
// End-to-end against a REAL loopback HTTP server, exercising the actual network
|
||||
// probe (Node's global fetch, mirroring main.ts's readDaemonProbe) and the real
|
||||
// startup decision order: run-file first, then the #367 port-probe backstop.
|
||||
describe("end-to-end against a real daemon server", () => {
|
||||
const servers: Server[] = [];
|
||||
|
||||
afterEach(async () => {
|
||||
await Promise.all(servers.splice(0).map((s) => new Promise<void>((r) => s.close(() => r()))));
|
||||
});
|
||||
|
||||
// Stand up a server on an ephemeral port. `service` lets us simulate a foreign
|
||||
// (non-AO) server squatting on the port.
|
||||
function startServer(opts: {
|
||||
pid: number;
|
||||
service?: string;
|
||||
executablePath?: string;
|
||||
workingDirectory?: string;
|
||||
}): Promise<number> {
|
||||
const service = opts.service ?? DAEMON_SERVICE_NAME;
|
||||
const server = createServer((req, res) => {
|
||||
const url = req.url ?? "";
|
||||
const base = {
|
||||
service,
|
||||
pid: opts.pid,
|
||||
executablePath: opts.executablePath,
|
||||
workingDirectory: opts.workingDirectory,
|
||||
};
|
||||
if (url === "/healthz") {
|
||||
res.writeHead(200, { "content-type": "application/json" });
|
||||
res.end(JSON.stringify({ status: "ok", ...base }));
|
||||
return;
|
||||
}
|
||||
if (url === "/readyz") {
|
||||
res.writeHead(200, { "content-type": "application/json" });
|
||||
res.end(JSON.stringify({ status: "ready", ...base }));
|
||||
return;
|
||||
}
|
||||
res.writeHead(404);
|
||||
res.end();
|
||||
});
|
||||
servers.push(server);
|
||||
return new Promise((resolve) => {
|
||||
server.listen(0, "127.0.0.1", () => {
|
||||
const addr = server.address();
|
||||
resolve(typeof addr === "object" && addr ? addr.port : 0);
|
||||
});
|
||||
});
|
||||
}
|
||||
|
||||
// Faithful copy of main.ts's readDaemonProbe, but with global fetch in place of
|
||||
// electron's net.fetch — so this test exercises the real HTTP round-trip + parse.
|
||||
const realProbe: DaemonProber = async (port, endpoint) => {
|
||||
const controller = new AbortController();
|
||||
const timer = setTimeout(() => controller.abort(), 2_000);
|
||||
try {
|
||||
const response = await fetch(`http://127.0.0.1:${port}/${endpoint}`, { signal: controller.signal });
|
||||
if (!response.ok) return null;
|
||||
return parseDaemonProbe(endpoint, await response.json());
|
||||
} catch {
|
||||
return null;
|
||||
} finally {
|
||||
clearTimeout(timer);
|
||||
}
|
||||
};
|
||||
|
||||
// The real startup decision: attach via run-file if possible, else fall back to
|
||||
// the direct port probe (the #367 fix), else null → caller spawns.
|
||||
async function startupDecision(opts: {
|
||||
runFileContents: string | null;
|
||||
isProcessAlive: (pid: number) => boolean;
|
||||
expectedPort: number;
|
||||
identityError?: (probe: DaemonProbe) => string | null;
|
||||
}) {
|
||||
const identityError = opts.identityError ?? NO_IDENTITY_ERROR;
|
||||
const fromRunFile = await resolveDaemonFromRunFile({
|
||||
runFileContents: opts.runFileContents,
|
||||
isProcessAlive: opts.isProcessAlive,
|
||||
probe: realProbe,
|
||||
identityError,
|
||||
});
|
||||
if (fromRunFile) return fromRunFile;
|
||||
return resolveDaemonFromPort({ expectedPort: opts.expectedPort, probe: realProbe, identityError });
|
||||
}
|
||||
|
||||
it("attaches to a genuinely serving daemon via the direct port probe", async () => {
|
||||
const port = await startServer({ pid: 555 });
|
||||
const result = await resolveDaemonFromPort({
|
||||
expectedPort: port,
|
||||
probe: realProbe,
|
||||
identityError: NO_IDENTITY_ERROR,
|
||||
});
|
||||
expect(result).toEqual({
|
||||
state: "ready",
|
||||
port,
|
||||
pid: 555,
|
||||
executablePath: undefined,
|
||||
workingDirectory: undefined,
|
||||
});
|
||||
});
|
||||
|
||||
it("returns null when the port is closed (caller spawns)", async () => {
|
||||
const port = await startServer({ pid: 1 });
|
||||
// Close the only server so the port is now refused.
|
||||
await Promise.all(servers.splice(0).map((s) => new Promise<void>((r) => s.close(() => r()))));
|
||||
const result = await resolveDaemonFromPort({
|
||||
expectedPort: port,
|
||||
probe: realProbe,
|
||||
identityError: NO_IDENTITY_ERROR,
|
||||
});
|
||||
expect(result).toBeNull();
|
||||
});
|
||||
|
||||
it("does NOT attach to a foreign (non-AO) server squatting on the port", async () => {
|
||||
const port = await startServer({ pid: 1, service: "some-other-service" });
|
||||
const result = await resolveDaemonFromPort({
|
||||
expectedPort: port,
|
||||
probe: realProbe,
|
||||
identityError: NO_IDENTITY_ERROR,
|
||||
});
|
||||
expect(result).toBeNull();
|
||||
});
|
||||
|
||||
// A foreign AO daemon (correct service, wrong binary) serving the port. The
|
||||
// identity check must surface an error rather than silently attach — the same
|
||||
// guard the run-file path enforces, now enforced on the port-probe path too.
|
||||
it("surfaces an identity error for a foreign AO binary serving the port (does not silently attach)", async () => {
|
||||
const port = await startServer({ pid: 909, executablePath: "/old/build/ao", workingDirectory: "/old/build" });
|
||||
const result = await startupDecision({
|
||||
runFileContents: null, // run-file diverged, so we reach the port probe
|
||||
isProcessAlive: ALIVE,
|
||||
expectedPort: port,
|
||||
identityError: (probe) =>
|
||||
probe.executablePath === "/expected/ao"
|
||||
? null
|
||||
: `Another AO daemon is already running from ${probe.executablePath}.`,
|
||||
});
|
||||
expect(result).toMatchObject({
|
||||
state: "error",
|
||||
port,
|
||||
pid: 909,
|
||||
message: "Another AO daemon is already running from /old/build/ao.",
|
||||
});
|
||||
});
|
||||
|
||||
// THE #367 SCENARIO: a standalone `ao daemon` is serving the port, but the
|
||||
// run-file diverges — here it names a DEAD pid (e.g. a stale handshake from a
|
||||
// crashed launch). Pre-fix this fell through to spawn() and the Go child
|
||||
// refused with exit 1. Post-fix the port probe attaches instead.
|
||||
it("attaches when a daemon serves the port but the run-file names a dead pid", async () => {
|
||||
const port = await startServer({ pid: 6060, executablePath: "/bin/ao" });
|
||||
const result = await startupDecision({
|
||||
runFileContents: runFile(4242, port), // stale pid 4242 ...
|
||||
isProcessAlive: DEAD, // ... which is no longer alive
|
||||
expectedPort: port,
|
||||
});
|
||||
expect(result).toEqual({
|
||||
state: "ready",
|
||||
port,
|
||||
pid: 6060, // attached to the daemon actually serving the port
|
||||
executablePath: "/bin/ao",
|
||||
workingDirectory: undefined,
|
||||
});
|
||||
});
|
||||
|
||||
// #367 VARIANT: run-file pid is alive, but /healthz reports a different pid
|
||||
// (run-file points at the wrong process). Old code returned null → spawn.
|
||||
it("attaches when the run-file pid disagrees with the daemon's reported pid", async () => {
|
||||
const port = await startServer({ pid: 8080 });
|
||||
const result = await startupDecision({
|
||||
runFileContents: runFile(4242, port),
|
||||
isProcessAlive: ALIVE,
|
||||
expectedPort: port,
|
||||
});
|
||||
expect(result).toMatchObject({ state: "ready", port, pid: 8080 });
|
||||
});
|
||||
|
||||
it("still attaches via the run-file path when everything agrees (no regression)", async () => {
|
||||
const port = await startServer({
|
||||
pid: 4242,
|
||||
executablePath: "/work/backend/ao",
|
||||
workingDirectory: "/work/backend",
|
||||
});
|
||||
const result = await startupDecision({
|
||||
runFileContents: runFile(4242, port),
|
||||
isProcessAlive: ALIVE,
|
||||
expectedPort: port,
|
||||
identityError: (probe) => (probe.pid === 4242 ? null : "wrong daemon"),
|
||||
});
|
||||
expect(result).toMatchObject({ state: "ready", port, pid: 4242 });
|
||||
});
|
||||
});
|
||||
|
|
@ -0,0 +1,172 @@
|
|||
// Deciding whether to ATTACH to an already-running daemon or SPAWN a fresh one.
|
||||
//
|
||||
// Two independent signals are consulted, in order:
|
||||
// 1. the running.json handshake file (whatever the last daemon wrote), and
|
||||
// 2. a direct probe of the expected port, independent of the run-file.
|
||||
//
|
||||
// (2) is the defensive backstop for issue #367: a standalone `ao daemon` may be
|
||||
// serving the port while running.json is missing, stale, unparseable, names a
|
||||
// dead PID, or reports a PID that disagrees with /healthz. In every one of those
|
||||
// cases the run-file check yields null; without the port probe the supervisor
|
||||
// would spawn a child daemon that the Go bind guard then refuses ("daemon
|
||||
// already running … refusing to start") and exits 1.
|
||||
//
|
||||
// These functions are kept side-effect free and dependency-injected (no node:*
|
||||
// or electron imports — the vite-plugin-electron-renderer polyfill breaks node:*
|
||||
// under vitest, see daemon-discovery.ts) so they can be exercised directly; the
|
||||
// Electron main process owns the real fs reads, process signals, fetch, and
|
||||
// path identity check.
|
||||
|
||||
import type { DaemonStatus } from "./daemon-status";
|
||||
import { parseRunFile } from "./daemon-discovery";
|
||||
|
||||
// The daemon's default bind port (backend/internal/config). AO_PORT overrides it.
|
||||
export const DEFAULT_DAEMON_PORT = 3001;
|
||||
// The `service` field every genuine AO daemon stamps on its health payloads.
|
||||
export const DAEMON_SERVICE_NAME = "agent-orchestrator-daemon";
|
||||
|
||||
export type DaemonProbe = {
|
||||
status: string;
|
||||
service: string;
|
||||
pid: number;
|
||||
executablePath?: string;
|
||||
workingDirectory?: string;
|
||||
};
|
||||
|
||||
/** A /healthz|/readyz probe of a loopback port; resolves null when nothing valid answers. */
|
||||
export type DaemonProber = (port: number, endpoint: "healthz" | "readyz") => Promise<DaemonProbe | null>;
|
||||
|
||||
/**
|
||||
* The port a freshly spawned daemon is expected to bind: AO_PORT when set and
|
||||
* valid, otherwise the daemon's default. Used to probe for an already-serving
|
||||
* daemon before spawning a child that would only refuse and exit.
|
||||
*/
|
||||
export function expectedDaemonPort(env: Record<string, string | undefined>): number {
|
||||
const configured = env.AO_PORT ? Number(env.AO_PORT) : NaN;
|
||||
return Number.isInteger(configured) && configured >= 1 && configured <= 65535 ? configured : DEFAULT_DAEMON_PORT;
|
||||
}
|
||||
|
||||
/**
|
||||
* Validate a /healthz or /readyz JSON body against the daemon contract. Returns
|
||||
* the typed probe, or null when the body is the wrong shape, status, or service
|
||||
* (e.g. some unrelated server happens to occupy the port).
|
||||
*/
|
||||
export function parseDaemonProbe(endpoint: "healthz" | "readyz", body: unknown): DaemonProbe | null {
|
||||
if (typeof body !== "object" || body === null) return null;
|
||||
const candidate = body as Partial<DaemonProbe>;
|
||||
if (candidate.status !== (endpoint === "healthz" ? "ok" : "ready")) return null;
|
||||
if (candidate.service !== DAEMON_SERVICE_NAME) return null;
|
||||
if (typeof candidate.pid !== "number" || !Number.isInteger(candidate.pid)) return null;
|
||||
return {
|
||||
status: candidate.status,
|
||||
service: candidate.service,
|
||||
pid: candidate.pid,
|
||||
executablePath: typeof candidate.executablePath === "string" ? candidate.executablePath : undefined,
|
||||
workingDirectory: typeof candidate.workingDirectory === "string" ? candidate.workingDirectory : undefined,
|
||||
};
|
||||
}
|
||||
|
||||
export type RunFileResolveDeps = {
|
||||
/** running.json contents, or null when the file has no path or could not be read. */
|
||||
runFileContents: string | null;
|
||||
isProcessAlive: (pid: number) => boolean;
|
||||
probe: DaemonProber;
|
||||
/** Foreign-daemon check (dev/bundled identity); returns a message, or null when it is ours. */
|
||||
identityError: (probe: DaemonProbe) => string | null;
|
||||
};
|
||||
|
||||
/**
|
||||
* Attach decision driven by the running.json handshake. Returns:
|
||||
* - a "ready" status → attach to the recorded daemon,
|
||||
* - an "error" status → a daemon is up but unusable (not ready / foreign);
|
||||
* surface it rather than spawn,
|
||||
* - null → the run-file is absent/stale/inconsistent; the caller
|
||||
* should fall through to {@link resolveDaemonFromPort}.
|
||||
*/
|
||||
export async function resolveDaemonFromRunFile(deps: RunFileResolveDeps): Promise<DaemonStatus | null> {
|
||||
const { runFileContents, isProcessAlive, probe, identityError } = deps;
|
||||
if (runFileContents === null) return null;
|
||||
const info = parseRunFile(runFileContents);
|
||||
if (!info || !isProcessAlive(info.pid)) return null;
|
||||
|
||||
const health = await probe(info.port, "healthz");
|
||||
// The recorded PID must match the live daemon; otherwise the run-file points
|
||||
// at the wrong process — return null so the caller falls through to the port
|
||||
// probe rather than trusting a stale handshake.
|
||||
if (!health || health.pid !== info.pid) return null;
|
||||
return readinessStatus(info.port, info.pid, health, probe, identityError);
|
||||
}
|
||||
|
||||
export type PortProbeResolveDeps = {
|
||||
expectedPort: number;
|
||||
probe: DaemonProber;
|
||||
/** Foreign-daemon check (dev/bundled identity); returns a message, or null when it is ours. */
|
||||
identityError: (probe: DaemonProbe) => string | null;
|
||||
};
|
||||
|
||||
/**
|
||||
* Attach decision driven by a direct /healthz probe of the expected port,
|
||||
* independent of the run-file (issue #367 backstop). Returns:
|
||||
* - a "ready" status → attach to the daemon serving the port,
|
||||
* - an "error" status → a daemon serves the port but is unusable (not ready, or
|
||||
* a foreign binary the identity check refuses); surface it
|
||||
* rather than spawn (spawning would only collide and die),
|
||||
* - null → nothing genuine answers the port; the caller should spawn.
|
||||
*
|
||||
* This mirrors {@link resolveDaemonFromRunFile}'s post-handshake validation
|
||||
* (/readyz + identity), anchoring on the PID /healthz reports instead of the
|
||||
* run-file's, so attaching via the port is no laxer than attaching via the file.
|
||||
*/
|
||||
export async function resolveDaemonFromPort(deps: PortProbeResolveDeps): Promise<DaemonStatus | null> {
|
||||
const { expectedPort, probe, identityError } = deps;
|
||||
const health = await probe(expectedPort, "healthz");
|
||||
if (!health) return null;
|
||||
return readinessStatus(expectedPort, health.pid, health, probe, identityError);
|
||||
}
|
||||
|
||||
/**
|
||||
* Shared tail of both attach paths: given a daemon confirmed serving /healthz on
|
||||
* `port` with PID `pid`, confirm it is ready and is the daemon we expect, and
|
||||
* build the resulting DaemonStatus. Returns an "error" status (never null) — by
|
||||
* here a daemon is definitely occupying the port, so spawning is never the right
|
||||
* move.
|
||||
*/
|
||||
async function readinessStatus(
|
||||
port: number,
|
||||
pid: number,
|
||||
health: DaemonProbe,
|
||||
probe: DaemonProber,
|
||||
identityError: (probe: DaemonProbe) => string | null,
|
||||
): Promise<DaemonStatus> {
|
||||
const ready = await probe(port, "readyz");
|
||||
if (!ready || ready.pid !== pid) {
|
||||
return {
|
||||
state: "error",
|
||||
port,
|
||||
pid,
|
||||
executablePath: health.executablePath,
|
||||
workingDirectory: health.workingDirectory,
|
||||
message: "An AO daemon is already running, but it is not ready yet.",
|
||||
};
|
||||
}
|
||||
|
||||
const message = identityError(ready);
|
||||
if (message) {
|
||||
return {
|
||||
state: "error",
|
||||
port,
|
||||
pid,
|
||||
executablePath: ready.executablePath,
|
||||
workingDirectory: ready.workingDirectory,
|
||||
message,
|
||||
};
|
||||
}
|
||||
|
||||
return {
|
||||
state: "ready",
|
||||
port,
|
||||
pid,
|
||||
executablePath: ready.executablePath,
|
||||
workingDirectory: ready.workingDirectory,
|
||||
};
|
||||
}
|
||||
Loading…
Reference in New Issue