History

Fix local service convergence

Rhys Sullivan committed 5 months ago

Commit 76b8a331 parent 1547bab33 files · +2172 −998Browse files

Showing the changes to apps/cli/src (5 of 33 files). Show all

Unified · Split

5 files
  1. apps/cli/src/daemon.test.ts +131 −0
  2. apps/cli/src/daemon.ts +49 −0
  3. apps/cli/src/main.ts +123 −29
  4. apps/cli/src/service.test.ts +17 −4
  5. apps/cli/src/service.ts +37 −16
apps/cli/src/daemon.test.ts+131−0
@@ -7,6 +7,7 @@ import { · 6 unmodified lines
77 canAutoStartLocalDaemonForHost,
88 isDevCliEntrypoint,
99 isExecutorServerReachable,
10+ planServiceInstall,
1011 } from "./daemon";
1112
1213 describe("isDevCliEntrypoint", () => {
@@ -93,3 +94,133 @@ describe("isExecutorServerReachable", () => { · 80 unmodified lines
9394 }),
9495 );
9596 });
97+
98+describe("planServiceInstall", () => {
99+ it("no-ops when the supervised service already runs this version on the requested port", () => {
100+ expect(
101+ planServiceInstall({
102+ registered: true,
103+ running: true,
104+ activeKind: "cli-daemon",
105+ activeVersion: "1.5.11",
106+ activePort: 4789,
107+ requestedPort: 4789,
108+ currentVersion: "1.5.11",
109+ }),
110+ ).toBe("noop");
111+ });
112+
113+ it("reinstalls when the supervised service runs an older version", () => {
114+ expect(
115+ planServiceInstall({
116+ registered: true,
117+ running: true,
118+ activeKind: "cli-daemon",
119+ activeVersion: "1.5.10",
120+ activePort: 4789,
121+ requestedPort: 4789,
122+ currentVersion: "1.5.11",
123+ }),
124+ ).toBe("reinstall");
125+ });
126+
127+ it("reinstalls when the version matches but the requested service port changed", () => {
128+ expect(
129+ planServiceInstall({
130+ registered: true,
131+ running: true,
132+ activeKind: "cli-daemon",
133+ activeVersion: "1.5.11",
134+ activePort: 4789,
135+ requestedPort: 5790,
136+ currentVersion: "1.5.11",
137+ }),
138+ ).toBe("reinstall");
139+ });
140+
141+ it("reinstalls when the version and port match but the service points at another executable", () => {
142+ expect(
143+ planServiceInstall({
144+ registered: true,
145+ running: true,
146+ activeKind: "cli-daemon",
147+ activeVersion: "1.5.11",
148+ activeExecutablePath:
149+ "/Applications/Executor.app/Contents/Resources/sidecar/executor-sidecar",
150+ activePort: 4789,
151+ requestedPort: 4789,
152+ currentVersion: "1.5.11",
153+ currentExecutablePath: "/Applications/Executor.app/Contents/Resources/executor/executor",
154+ }),
155+ ).toBe("reinstall");
156+ });
157+
158+ it("takes over when a detached CLI daemon owns the manifest while the service is running elsewhere", () => {
159+ expect(
160+ planServiceInstall({
161+ registered: true,
162+ running: true,
163+ activeKind: "cli-daemon",
164+ activePid: 2002,
165+ servicePid: 1001,
166+ activeVersion: "1.5.11",
167+ activePort: 4788,
168+ requestedPort: 55334,
169+ currentVersion: "1.5.11",
170+ }),
171+ ).toBe("takeover-then-install");
172+ });
173+
174+ it("reinstalls when the supervised service is up but manifest details are unavailable", () => {
175+ expect(
176+ planServiceInstall({
177+ registered: true,
178+ running: true,
179+ activeKind: null,
180+ activeVersion: null,
181+ activePort: null,
182+ requestedPort: 4789,
183+ currentVersion: "1.5.11",
184+ }),
185+ ).toBe("reinstall");
186+ });
187+
188+ it("takes over when another local server kind owns the data directory", () => {
189+ expect(
190+ planServiceInstall({
191+ registered: true,
192+ running: true,
193+ activeKind: "foreground",
194+ activeVersion: "1.5.11",
195+ activePort: 4789,
196+ requestedPort: 4789,
197+ currentVersion: "1.5.11",
198+ }),
199+ ).toBe("takeover-then-install");
200+ expect(
201+ planServiceInstall({
202+ registered: false,
203+ running: false,
204+ activeKind: "desktop-sidecar",
205+ activeVersion: "1.5.10",
206+ activePort: 4789,
207+ requestedPort: 4789,
208+ currentVersion: "1.5.11",
209+ }),
210+ ).toBe("takeover-then-install");
211+ });
212+
213+ it("takes over on a fresh install path before writing the service", () => {
214+ expect(
215+ planServiceInstall({
216+ registered: false,
217+ running: false,
218+ activeKind: null,
219+ activeVersion: null,
220+ activePort: null,
221+ requestedPort: 4789,
222+ currentVersion: "1.5.11",
223+ }),
224+ ).toBe("takeover-then-install");
225+ });
226+});
apps/cli/src/daemon.ts+49−0
@@ -282,3 +282,52 @@ export const chooseDaemonPort = (input: { · 281 unmodified lines
282282 }
283283 return fallbackPort;
284284 });
285+
286+// ---------------------------------------------------------------------------
287+// Service-install planning (pure)
288+// ---------------------------------------------------------------------------
289+
290+export type ServiceInstallPlan = "noop" | "reinstall" | "takeover-then-install";
291+
292+export const planServiceInstall = (input: {
293+ readonly registered: boolean;
294+ readonly running: boolean;
295+ readonly activeKind: "cli-daemon" | "desktop-sidecar" | "foreground" | null;
296+ readonly activePid?: number | null;
297+ readonly servicePid?: number | null;
298+ readonly activeVersion: string | null;
299+ readonly activeExecutablePath?: string | null;
300+ readonly activePort: number | null;
301+ readonly requestedPort: number;
302+ readonly currentVersion: string;
303+ readonly currentExecutablePath?: string | null;
304+}): ServiceInstallPlan => {
305+ if (input.activeKind !== null && input.activeKind !== "cli-daemon") {
306+ return "takeover-then-install";
307+ }
308+
309+ if (input.registered && input.running) {
310+ if (
311+ input.activeKind === "cli-daemon" &&
312+ input.activePid !== undefined &&
313+ input.activePid !== null &&
314+ input.servicePid !== undefined &&
315+ input.servicePid !== null &&
316+ input.activePid !== input.servicePid
317+ ) {
318+ return "takeover-then-install";
319+ }
320+
321+ const executableMatches =
322+ !input.activeExecutablePath ||
323+ !input.currentExecutablePath ||
324+ input.activeExecutablePath === input.currentExecutablePath;
325+ return input.activeVersion === input.currentVersion &&
326+ input.activePort === input.requestedPort &&
327+ executableMatches
328+ ? "noop"
329+ : "reinstall";
330+ }
331+
332+ return "takeover-then-install";
333+};
apps/cli/src/main.ts+123−29
@@ -36,6 +36,25 @@ if (typeof Bun !== "undefined" && (await Bun.file(wasmOnDisk).exists())) { · 35 unmodified lines
3636 setQuickJSModule(mod);
3737 }
3838
39+const sentryDsn = process.env.EXECUTOR_SENTRY_DSN;
40+if (sentryDsn) {
41+ const Sentry = await import("@sentry/bun");
42+ Sentry.init({
43+ dsn: sentryDsn,
44+ release: process.env.EXECUTOR_SENTRY_RELEASE,
45+ environment: process.env.EXECUTOR_SENTRY_ENVIRONMENT ?? "production",
46+ tracesSampleRate: 0,
47+ initialScope: {
48+ tags: {
49+ process: "daemon",
50+ platform: process.platform,
51+ arch: process.arch,
52+ ...(process.env.EXECUTOR_RUN_ID ? { runId: process.env.EXECUTOR_RUN_ID } : {}),
53+ },
54+ },
55+ });
56+}
57+
3958 import { Argument as Args, Command, Flag as Options } from "effect/unstable/cli";
4059 import { BunRuntime, BunServices } from "@effect/platform-bun";
4160 import { HttpApiClient } from "effect/unstable/httpapi";
@@ -71,6 +90,7 @@ import { · 29 unmodified lines
7190 isExecutorServerReachable,
7291 isDevCliEntrypoint,
7392 parseDaemonBaseUrl,
93+ planServiceInstall,
7494 spawnDetached,
7595 waitForReachable,
7696 waitForUnreachable,
@@ -100,7 +120,6 @@ import { · 23 unmodified lines
100120 acquireLocalServerStartLock,
101121 readLocalServerManifest,
102122 releaseLocalServerStartLock,
103− removeLocalServerManifest,
104123 removeLocalServerManifestIfOwnedBy,
105124 resolveExecutorDataDir,
106125 writeLocalServerManifest,
@@ -260,7 +279,7 @@ const makeLocalServerManifest = (input: { · 153 unmodified lines
260279 scopeDir: currentScopeDirForManifest(),
261280 connection: input.connection,
262281 owner: {
263− client: "cli",
282+ client: process.env.EXECUTOR_CLIENT === "desktop" ? "desktop" : "cli",
264283 version: CLI_VERSION,
265284 executablePath: isDevMode ? (script ?? null) : process.execPath,
266285 },
@@ -286,6 +305,41 @@ const assertNoOtherActiveLocalServer = (): Effect.Effect< · 19 unmodified lines
286305 );
287306 });
288307
308+const takeOverActiveLocalServer = (): Effect.Effect<
309+ ExecutorLocalServerManifest | null,
310+ Error,
311+ FileSystem.FileSystem | PlatformPath.Path
312+> =>
313+ Effect.gen(function* () {
314+ const manifest = yield* readLocalServerManifest();
315+ if (!manifest) return null;
316+
317+ if (!isPidAlive(manifest.pid) || manifest.pid === process.pid) {
318+ yield* removeLocalServerManifestIfOwnedBy({ pid: manifest.pid }).pipe(Effect.ignore);
319+ return null;
320+ }
321+
322+ yield* terminatePid(manifest.pid).pipe(Effect.ignore);
323+ const stopped = yield* waitForUnreachable({
324+ check: isServerReachable(manifest.connection.origin),
325+ timeoutMs: DAEMON_STOP_TIMEOUT_MS,
326+ intervalMs: DAEMON_BOOT_POLL_MS,
327+ });
328+ if (!stopped) {
329+ return yield* Effect.fail(
330+ new Error(
331+ [
332+ `The existing Executor ${manifest.kind} at ${manifest.connection.origin} (pid ${manifest.pid}) did not stop within ${DAEMON_STOP_TIMEOUT_MS / 1000}s.`,
333+ "Stop it manually and re-run.",
334+ ].join("\n"),
335+ ),
336+ );
337+ }
338+
339+ yield* removeLocalServerManifestIfOwnedBy({ pid: manifest.pid }).pipe(Effect.ignore);
340+ return manifest;
341+ });
342+
289343 const publishLocalServerManifest = (input: {
290344 readonly kind: ExecutorLocalServerKind;
291345 readonly connection: ExecutorServerConnection;
@@ -897,7 +951,7 @@ const runDaemonSession = (input: { · 605 unmodified lines
897951 // and crash-loop under KeepAlive. (Found by a real reboot test with
898952 // integration data in the DB.)
899953 if (process.env.EXECUTOR_SUPERVISED) {
900− yield* removeLocalServerManifest().pipe(Effect.ignore);
954+ yield* takeOverActiveLocalServer().pipe(Effect.ignore);
901955 } else {
902956 yield* assertNoOtherActiveLocalServer();
903957 }
@@ -907,15 +961,34 @@ const runDaemonSession = (input: { · 3 unmodified lines
907961 if (existing) {
908962 const existingUrl = daemonBaseUrl(existing.hostname, existing.port);
909963 if (isPidAlive(existing.pid) && (yield* isServerReachable(existingUrl))) {
910− return yield* Effect.fail(
911− new Error(
912− [
913− `A daemon is already running for scope ${scopeId} on ${daemonHost}.`,
914− `Existing daemon: ${existingUrl} (pid ${existing.pid}).`,
915− `Stop it first: ${cliPrefix} daemon stop`,
916− ].join("\n"),
917− ),
918− );
964+ if (process.env.EXECUTOR_SUPERVISED) {
965+ yield* terminatePid(existing.pid).pipe(Effect.ignore);
966+ const stopped = yield* waitForUnreachable({
967+ check: isServerReachable(existingUrl),
968+ timeoutMs: DAEMON_STOP_TIMEOUT_MS,
969+ intervalMs: DAEMON_BOOT_POLL_MS,
970+ });
971+ if (!stopped) {
972+ return yield* Effect.fail(
973+ new Error(
974+ [
975+ `The existing daemon for scope ${scopeId} at ${existingUrl} (pid ${existing.pid}) did not stop within ${DAEMON_STOP_TIMEOUT_MS / 1000}s.`,
976+ "Stop it manually and re-run.",
977+ ].join("\n"),
978+ ),
979+ );
980+ }
981+ } else {
982+ return yield* Effect.fail(
983+ new Error(
984+ [
985+ `A daemon is already running for scope ${scopeId} on ${daemonHost}.`,
986+ `Existing daemon: ${existingUrl} (pid ${existing.pid}).`,
987+ `Stop it first: ${cliPrefix} daemon stop`,
988+ ].join("\n"),
989+ ),
990+ );
991+ }
919992 }
920993 yield* cleanupPointer({ hostname: existing.hostname, scopeId, port: existing.port });
921994 }
@@ -2125,6 +2198,17 @@ const mcpCommand = Command.make( · 1203 unmodified lines
21252198
21262199 const supervisedServiceOrigin = (port: number): string => `http://127.0.0.1:${port}`;
21272200
2201+const portFromOrigin = (origin: string): number | null => {
2202+ try {
2203+ const url = new URL(origin);
2204+ if (!url.port) return url.protocol === "https:" ? 443 : 80;
2205+ const port = Number.parseInt(url.port, 10);
2206+ return Number.isInteger(port) && port > 0 ? port : null;
2207+ } catch {
2208+ return null;
2209+ }
2210+};
2211+
21282212 const servicePortOption = () =>
21292213 Options.integer("port")
21302214 .pipe(Options.withDefault(DEFAULT_SERVICE_PORT))
@@ -2164,26 +2248,36 @@ const installService = (port: number, commandName: string) => · 33 unmodified lines
21642248 return;
21652249 }
21662250
2167− // Don't fight an already-running local server against the same data dir
2168− // (a desktop sidecar, a foreground `executor web`, or an existing daemon).
2169− const active = yield* readActiveLocalServerManifest();
2170− if (active) {
2171− const status = yield* backend.status();
2172− if (status.registered && status.running && active.kind === "cli-daemon") {
2251+ const status = yield* backend.status();
2252+ const active = yield* readActiveLocalServerManifest().pipe(Effect.orElseSucceed(() => null));
2253+ const plan = planServiceInstall({
2254+ registered: status.registered,
2255+ running: status.running,
2256+ activeKind: active?.kind ?? null,
2257+ activePid: active?.pid ?? null,
2258+ servicePid: status.pid,
2259+ activeVersion: active?.owner.version ?? null,
2260+ activeExecutablePath: active?.owner.executablePath ?? null,
2261+ activePort: active ? portFromOrigin(active.connection.origin) : null,
2262+ requestedPort: port,
2263+ currentVersion: CLI_VERSION,
2264+ currentExecutablePath: process.execPath,
2265+ });
2266+
2267+ if (plan === "noop") {
2268+ const where = active ? ` at ${active.connection.origin} (pid ${active.pid})` : "";
2269+ console.log(`Executor background service is already running${where}.`);
2270+ console.log(`Open it in your browser, already signed in, with: ${cliPrefix} web`);
2271+ return;
2272+ }
2273+
2274+ if (plan === "takeover-then-install") {
2275+ const replaced = yield* takeOverActiveLocalServer();
2276+ if (replaced) {
21732277 console.log(
2174− `Executor background service is already running at ${active.connection.origin} (pid ${active.pid}).`,
2278+ `Replacing running Executor ${replaced.kind} at ${replaced.connection.origin} (pid ${replaced.pid})...`,
21752279 );
2176− console.log(`Open it in your browser, already signed in, with: ${cliPrefix} web`);
2177− return;
21782280 }
2179− return yield* Effect.fail(
2180− new Error(
2181− [
2182− `A local Executor ${active.kind} is already running at ${active.connection.origin} (pid ${active.pid}).`,
2183− `Stop it first (quit the desktop app, or \`${cliPrefix} daemon stop\`), then re-run \`${command}\`.`,
2184− ].join("\n"),
2185− ),
2186− );
21872281 }
21882282
21892283 const path = yield* PlatformPath.Path;
apps/cli/src/service.test.ts+17−4
@@ -13,12 +13,14 @@ describe("service unit generation", () => { · 12 unmodified lines
1313 const launchdInput = {
1414 label: "sh.executor.daemon",
1515 programArguments: [
16− "/Applications/Executor.app/Contents/Resources/sidecar/executor-sidecar",
16+ "/Applications/Executor.app/Contents/Resources/executor/executor",
1717 "daemon",
1818 "run",
1919 "--foreground",
2020 "--port",
2121 "4789",
22+ "--hostname",
23+ "127.0.0.1",
2224 ],
2325 environment: {
2426 EXECUTOR_SUPERVISED: "1",
@@ -65,13 +67,24 @@ describe("service unit generation", () => { · 40 unmodified lines
6567
6668 it("renders a systemd --user unit with crash-only restart", () => {
6769 const unit = generateSystemdUnit({
68− execStart: ["/usr/local/bin/executor", "daemon", "run", "--foreground", "--port", "4789"],
70+ execStart: [
71+ "/usr/local/bin/executor",
72+ "daemon",
73+ "run",
74+ "--foreground",
75+ "--port",
76+ "4789",
77+ "--hostname",
78+ "127.0.0.1",
79+ ],
6980 environment: { EXECUTOR_SUPERVISED: "1", EXECUTOR_DATA_DIR: "/home/x/.executor" },
7081 workingDirectory: "/home/x/.executor",
7182 stdoutPath: "/home/x/.executor/logs/daemon.log",
7283 stderrPath: "/home/x/.executor/logs/daemon.error.log",
7384 });
74− expect(unit).toContain("ExecStart=/usr/local/bin/executor daemon run --foreground --port 4789");
85+ expect(unit).toContain(
86+ "ExecStart=/usr/local/bin/executor daemon run --foreground --port 4789 --hostname 127.0.0.1",
87+ );
7588 expect(unit).toContain("Restart=on-failure");
7689 expect(unit).toContain("WantedBy=default.target");
7790 expect(unit).toContain("Environment=EXECUTOR_SUPERVISED=1");
@@ -92,7 +105,7 @@ describe("service unit generation", () => { · 14 unmodified lines
92105 expect(wrapper).toContain('set "EXECUTOR_SUPERVISED=1"');
93106 expect(wrapper).toContain('set "EXECUTOR_DATA_DIR=C:\\Users\\x\\.executor"');
94107 expect(wrapper).toContain(
95− '"C:\\Program Files\\Executor\\executor.exe" daemon run --foreground --port 4789',
108+ '"C:\\Program Files\\Executor\\executor.exe" daemon run --foreground --port 4789 --hostname 127.0.0.1',
96109 );
97110 expect(wrapper).toContain('1>> "C:\\Users\\x\\.executor\\logs\\daemon.log"');
98111 // No secret in the wrapper — the daemon reads the bearer from auth.json at boot.
apps/cli/src/service.ts+37−16
@@ -138,27 +138,48 @@ const serviceProgramArguments = (descriptor: ServiceDescriptor): ReadonlyArray<s · 137 unmodified lines
138138 "--foreground",
139139 "--port",
140140 String(descriptor.port),
141+ "--hostname",
142+ "127.0.0.1",
141143 ];
142144
143145 const serviceEnvironment = (
144146 descriptor: ServiceDescriptor,
145147 dataDir: string,
146−): Record<string, string> => ({
147− // Marks the process as OS-supervised so the daemon resolves its bearer token
148− // from the durable 0600 auth.json (the secret is never in the unit itself).
149− EXECUTOR_SUPERVISED: "1",
150− // Pin the data dir explicitly: launchd/systemd give a minimal environment and
151− // we never want the daemon to fall back to a different home than the user's.
152− EXECUTOR_DATA_DIR: dataDir,
153− // Stamp the installing version so `service status` can flag drift after an
154− // upgrade where the unit still points at an older binary path.
155− EXECUTOR_SERVICE_VERSION: descriptor.version,
156− // A launchd/systemd unit starts with a bare PATH — without the user's PATH
157− // the daemon can't find pyenv/nvm/volta/Homebrew tools that integrations may
158− // shell out to. `service install` runs from the user's shell, so its own
159− // PATH is the right one to bake in. (Reference: opencode shell-env capture.)
160− ...(process.env.PATH ? { PATH: process.env.PATH } : {}),
161−});
148+): Record<string, string> => {
149+ const passThroughKeys = [
150+ "EXECUTOR_CLIENT",
151+ "EXECUTOR_SENTRY_DSN",
152+ "EXECUTOR_SENTRY_RELEASE",
153+ "EXECUTOR_SENTRY_ENVIRONMENT",
154+ "EXECUTOR_RUN_ID",
155+ ] as const;
156+ const passThrough = Object.fromEntries(
157+ passThroughKeys.flatMap((key) => {
158+ const value = process.env[key];
159+ return value ? [[key, value] as const] : [];
160+ }),
161+ );
162+
163+ return {
164+ // Marks the process as OS-supervised so the daemon resolves its bearer token
165+ // from the durable 0600 auth.json (the secret is never in the unit itself).
166+ EXECUTOR_SUPERVISED: "1",
167+ // Pin the data/scope dirs explicitly: launchd/systemd give a minimal
168+ // environment and we never want the daemon to fall back to a different home
169+ // or cwd than the user's singleton local service.
170+ EXECUTOR_DATA_DIR: dataDir,
171+ EXECUTOR_SCOPE_DIR: process.env.EXECUTOR_SCOPE_DIR ?? dataDir,
172+ // Stamp the installing version so `service status` can flag drift after an
173+ // upgrade where the unit still points at an older binary path.
174+ EXECUTOR_SERVICE_VERSION: descriptor.version,
175+ // A launchd/systemd unit starts with a bare PATH — without the user's PATH
176+ // the daemon can't find pyenv/nvm/volta/Homebrew tools that integrations may
177+ // shell out to. `service install` runs from the user's shell, so its own
178+ // PATH is the right one to bake in. (Reference: opencode shell-env capture.)
179+ ...(process.env.PATH ? { PATH: process.env.PATH } : {}),
180+ ...passThrough,
181+ };
182+};
162183
163184 // ---------------------------------------------------------------------------
164185 // macOS — launchd LaunchAgent (fully built)