History

Desktop can't attach to CLI daemon because it might be foreground; revert to old behavior

Griffin Evans committed 4 months ago

Commit bab14421 parent ae8fb9f1 file · +14 −105Browse files

Unified · Split

apps/desktop/src/main/sidecar.ts+14−105
@@ -5,9 +5,9 @@ · 4 unmodified lines
55 * In prod: spawns the bundled CLI binary in foreground daemon mode.
66 *
77 * Either way, the child receives EXECUTOR_PORT/EXECUTOR_HOST/EXECUTOR_AUTH_TOKEN.
8− * The dev sidecar and packaged CLI child announce structured stdout sentinels
9− * (`EXECUTOR_READY:<port>` or `EXECUTOR_ATTACHED`). Human-readable log text is
10− * never part of the desktop startup contract.
8+ * The dev sidecar and packaged CLI child announce the structured stdout sentinel
9+ * `EXECUTOR_READY:<port>`. Human-readable log text is never part of the desktop
10+ * startup contract.
1111 */
1212
1313 import { spawn, type ChildProcess } from "node:child_process";
@@ -36,9 +36,6 @@ const sidecarLog = log.scope("sidecar"); · 22 unmodified lines
3636 // sidecar can't grow it unbounded over a long session.
3737 const STDERR_TAIL_LIMIT = 8 * 1024;
3838 const READY_SENTINEL = "EXECUTOR_READY";
39−const ATTACHED_SENTINEL = "EXECUTOR_ATTACHED";
40−const ATTACH_OWNER_TIMEOUT_MS = 15_000;
41−const ATTACH_OWNER_POLL_MS = 150;
4239
4340 // Children deliberately stopped via stopSidecar (quit, restart, update) —
4441 // their exits are expected and must not be reported as crashes.
@@ -227,12 +224,6 @@ export async function startSidecar(options: StartOptions = {}): Promise<SidecarC · 182 unmodified lines
227224 const dataDir = scopeDir;
228225 mkdirSync(dataDir, { recursive: true });
229226
230− // Fast path: a reachable local server already owns this data dir (a CLI
231− // daemon, another desktop sidecar, or a supervised service). Attach to it
232− // instead of spawning a child that would only lose the DB ownership race.
233− const attached = await attachToReachableLocalServer(dataDir);
234− if (attached) return attached;
235−
236227 // The stable bearer token from auth.json (shared with the CLI). The main
237228 // process holds it so it can inject the header into the webview; the child
238229 // validates against the same value. Always present — auth is unconditional.
@@ -253,8 +244,8 @@ export async function startSidecar(options: StartOptions = {}): Promise<SidecarC · 14 unmodified lines
253244
254245 // No process-level startup lock: the dev sidecar child opens the DB through
255246 // openOwnedLocalDatabase, whose ownership lock is the real gate. If the child
256− // loses the race it exits and onExit attaches to the winner. server.json is an
257− // attach hint only.
247+ // loses the race, startup fails as before; only the packaged supervised boot
248+ // path attaches to an existing daemon.
258249 const webBaseUrl = `http://${hostname}:${settings.port}`;
259250 const child = spawn(command, args, {
260251 cwd,
@@ -284,7 +275,6 @@ export async function startSidecar(options: StartOptions = {}): Promise<SidecarC · 23 unmodified lines
284275 let stdoutControlBuffer = "";
285276 let resolved = false;
286277 let rejected = false;
287− let attachedOwnerResolution: Promise<boolean> | null = null;
288278
289279 const logStdoutLine = makeLineSplitter((line) => sidecarLog.info(line));
290280 const logStderrLine = makeLineSplitter((line) => sidecarLog.error(line));
@@ -296,25 +286,6 @@ export async function startSidecar(options: StartOptions = {}): Promise<SidecarC · 5 unmodified lines
296286 rejectStart(err);
297287 };
298288
299− const resolveAttachedOwner = async (): Promise<boolean> => {
300− if (resolved || rejected) return true;
301− if (!attachedOwnerResolution) {
302− attachedOwnerResolution = waitForReachableLocalServer(dataDir)
303− .then((attached) => {
304− if (resolved || rejected) return true;
305− if (!attached) return false;
306− resolved = true;
307− expectedExits.add(child);
308− resolveStart(attached);
309− return true;
310− })
311− .finally(() => {
312− attachedOwnerResolution = null;
313− });
314− }
315− return attachedOwnerResolution;
316− };
317−
318289 const onStdout = (chunk: Buffer) => {
319290 const text = chunk.toString("utf8");
320291 process.stdout.write(`[executor-server] ${text}`);
@@ -326,10 +297,6 @@ export async function startSidecar(options: StartOptions = {}): Promise<SidecarC · 5 unmodified lines
326297 .map((line) => line.trim())
327298 .filter((line) => line.length > 0);
328299 const readyLine = stdoutLines.find((line) => line.startsWith(`${READY_SENTINEL}:`));
329− if (stdoutLines.includes(ATTACHED_SENTINEL) && !resolved) {
330− void resolveAttachedOwner();
331− return;
332− }
333300 if (readyLine && !resolved) {
334301 if (!child.pid) {
335302 reject(
@@ -388,18 +355,15 @@ export async function startSidecar(options: StartOptions = {}): Promise<SidecarC · 52 unmodified lines
388355 return;
389356 }
390357 if (rejected) return;
391− void (async () => {
392− if (await resolveAttachedOwner()) return;
393− // Detect bind failure — the Node listener prints either "EADDRINUSE" or
394− // "address already in use" on stderr before exiting non-zero.
395− if (/EADDRINUSE|address already in use/i.test(stderrBuffer)) {
396− reject(new SidecarPortInUseError(settings.port));
397− return;
398− }
399− const message = `Sidecar exited before ready (code=${code} signal=${signal}). Stderr:\n${stderrBuffer}`;
400− // oxlint-disable-next-line executor/no-error-constructor -- boundary: sidecar boot failure surfaces here as a rejected start promise
401− reject(new Error(message));
402− })();
358+ // Detect bind failure — the Node listener prints either "EADDRINUSE" or
359+ // "address already in use" on stderr before exiting non-zero.
360+ if (/EADDRINUSE|address already in use/i.test(stderrBuffer)) {
361+ reject(new SidecarPortInUseError(settings.port));
362+ return;
363+ }
364+ const message = `Sidecar exited before ready (code=${code} signal=${signal}). Stderr:\n${stderrBuffer}`;
365+ // oxlint-disable-next-line executor/no-error-constructor -- boundary: sidecar boot failure surfaces here as a rejected start promise
366+ reject(new Error(message));
403367 };
404368
405369 child.stdout?.on("data", onStdout);
@@ -431,61 +395,6 @@ const isDaemonReachable = async (origin: string): Promise<boolean> => { · 25 unmodified lines
431395 return false;
432396 };
433397
434−const waitForReachableLocalServer = async (
435− dataDir: string,
436− options: { readonly requireCliDaemon?: boolean } = {},
437−): Promise<SidecarConnection | null> => {
438− const deadline = Date.now() + ATTACH_OWNER_TIMEOUT_MS;
439− while (Date.now() <= deadline) {
440− const attached = await attachToReachableLocalServer(dataDir, options);
441− if (attached) return attached;
442− const remaining = deadline - Date.now();
443− if (remaining <= 0) return null;
444− await delay(Math.min(ATTACH_OWNER_POLL_MS, remaining));
445− }
446− return null;
447−};
448−
449−const attachToReachableLocalServer = async (
450− dataDir: string,
451− options: { readonly requireCliDaemon?: boolean } = {},
452−): Promise<SidecarConnection | null> => {
453− const manifest = readManifest(dataDir);
454− if (!manifest) return null;
455− if (options.requireCliDaemon && manifest.kind !== "cli-daemon") return null;
456− if (!isPidAlive(manifest.pid)) {
457− removeManifestIfOwnedBy(dataDir, manifest.pid);
458− return null;
459− }
460−
461− const origin = manifest.connection.origin;
462− const auth = manifest.connection.auth;
463− const authToken = auth && auth.kind === "bearer" ? auth.token : "";
464− if (!(await isDaemonReachable(origin))) {
465− sidecarLog.warn(
466− `local Executor ${manifest.kind} at ${origin} (pid ${manifest.pid}) did not answer the health probe; keeping its manifest because the process is still alive`,
467− );
468− return null;
469− }
470−
471− const url = new URL(origin);
472− sidecarLog.info(
473− `attaching to local Executor ${manifest.kind} at ${origin} (pid ${manifest.pid})`,
474− );
475− return {
476− baseUrl: origin,
477− hostname: url.hostname,
478− port: Number.parseInt(url.port, 10) || (url.protocol === "https:" ? 443 : 80),
479− username: SERVER_SETTINGS_USERNAME,
480− authToken,
481− child: null,
482− supervisedDaemon: manifest.kind === "cli-daemon",
483− ownerVersion: manifest.owner.version,
484− ownerClient: manifest.owner.client,
485− ownerExecutablePath: manifest.owner.executablePath,
486− };
487−};
488−
489398 /**
490399 * Attach to an already-running OS-supervised daemon instead of spawning our own
491400 * sidecar. Reads `server.json`, confirms the endpoint answers, and returns a