Skip to content

Commit 749f186

Browse files
committed
fix(desktop): attach to a daemon that owns the data dir instead of wedging
The desktop app and a standalone `executor daemon run` both default to owning ~/.executor, and only one process can own it. boot() attaches to an existing daemon before spawning, but the restart and boot-fallback paths only spawned: when a foreign cli-daemon already owned the scope, the spawn died on the scope lock ("already running ... owns the current data directory") and was treated as fatal, wedging the app on the sidecar crash screen until the daemon was stopped by hand. startWithCurrentSettings now adopts a healthy cli-daemon that already owns the data dir instead of failing, polling briefly to ride out an owner that is alive but still finishing startup (the health-probe vs scope-lock race). Adopted daemons are watched by the supervised monitor, since they have no child process for onUnexpectedSidecarExit to observe. Also adds EXECUTOR_TEST_SKIP_BACKGROUND_SERVICE so the packaged app can boot headlessly, a deterministic dev e2e regression test, and a headless Linux+Xvfb harness that reproduces the wedge against the real packaged bundle.
1 parent ac5d095 commit 749f186

7 files changed

Lines changed: 677 additions & 0 deletions

File tree

.oxlintrc.jsonc

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -153,6 +153,7 @@
153153
"vendor/",
154154
"emulators/",
155155
"e2e/runs/",
156+
"e2e/desktop-packaged/linux-vm/repro.mjs",
156157
"node_modules/",
157158
"packages/core/fumadb/",
158159
"packages/core/sdk/src/vendor/json-schema-to-typescript/",

apps/desktop/src/main/index.ts

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -231,6 +231,13 @@ const ensureSupervisedConnection = async (): Promise<SidecarConnection | null> =
231231
});
232232
}
233233

234+
// Headless/e2e: the first-run background-service prompt and the systemd/
235+
// launchd install both need a user (and a real session bus) present. With
236+
// this set, fall straight through to managed-spawn after the attach attempt
237+
// so the packaged app can be driven without a window server. Mirrors
238+
// EXECUTOR_TEST_AUTO_CONFIRM_RESET in reset-state.ts.
239+
if (process.env.EXECUTOR_TEST_SKIP_BACKGROUND_SERVICE === "1") return null;
240+
234241
const status = await supervisedServiceStatus();
235242
if (!status.supported) return null;
236243

@@ -491,6 +498,17 @@ const showPortInUseDialog = async (port: number) => {
491498
// user-facing dialog instead of letting the app vanish without a window.
492499
let lastSidecarStartError: unknown = null;
493500

501+
// A spawn aborts with this when another local server already owns ~/.executor:
502+
// the bundled `executor daemon run` (packaged) or assertNoOtherLocalServerOwner
503+
// (dev) both phrase it the same way. Treated as fatal historically, which
504+
// wedged the app into the crash screen whenever a CLI `executor daemon run`
505+
// (or a daemon a prior run left behind) held the data dir.
506+
const isScopeOwnershipConflict = (error: unknown): boolean => {
507+
// oxlint-disable-next-line executor/no-instanceof-error, executor/no-unknown-error-message -- boundary: spawn-conflict failures arrive as plain Node errors from startSidecar; classified by message text
508+
const message = error instanceof Error ? error.message : String(error);
509+
return /already running|owns the current data directory/i.test(message);
510+
};
511+
494512
const startWithCurrentSettings = async (): Promise<SidecarConnection | null> => {
495513
// oxlint-disable-next-line executor/no-try-catch-or-throw -- boundary: bind failures surface as a user-facing dialog
496514
try {
@@ -501,6 +519,21 @@ const startWithCurrentSettings = async (): Promise<SidecarConnection | null> =>
501519
await showPortInUseDialog(error.port);
502520
return null;
503521
}
522+
// The data dir is already owned by another local daemon. If it's a healthy
523+
// cli-daemon, adopt it instead of failing — the same handoff boot() does up
524+
// front, applied here so the fallback and restart paths recover from a
525+
// mid-session daemon takeover. Poll briefly to ride out an owner that's
526+
// alive but still finishing its own startup (the health-probe vs.
527+
// scope-lock race that left the app wedged on the crash screen).
528+
if (isScopeOwnershipConflict(error)) {
529+
const adopted = await waitForSupervisedAttach(10_000);
530+
if (adopted) {
531+
log.info(
532+
"Adopted the local daemon that owns the data dir instead of spawning a second one",
533+
);
534+
return adopted;
535+
}
536+
}
504537
lastSidecarStartError = error;
505538
log.error("Failed to start executor sidecar", error);
506539
return null;
@@ -537,6 +570,12 @@ const restartSidecarAndReload = async (): Promise<DesktopServerConnection> => {
537570
throw new Error("Sidecar failed to restart — see Settings");
538571
}
539572
connection = next;
573+
// A restart can adopt a daemon that already owns the data dir (the
574+
// attach-on-conflict fallback). An adopted daemon has no child process, so
575+
// onUnexpectedSidecarExit can't see it die — watch it with the supervised
576+
// monitor instead. A freshly spawned sidecar keeps the child-exit path.
577+
if (next.supervisedDaemon) armSupervisedMonitor();
578+
else stopSupervisedMonitor();
540579
installBearerAuthHeader(next.baseUrl, next.authToken);
541580
const window = liveMainWindow();
542581
if (window) await window.loadURL(webUrlForConnection(next));
@@ -910,6 +949,11 @@ const boot = async () => {
910949
// oxlint-disable-next-line executor/no-try-catch-or-throw -- boundary: renderer navigation failure must show the startup recovery dialog, not a black window
911950
try {
912951
await createWindow(connection);
952+
// The fallback can adopt a daemon that owns the data dir instead of
953+
// spawning (attach-on-conflict). Like the primary supervised path, watch an
954+
// adopted daemon with the supervised monitor — it has no child process for
955+
// onUnexpectedSidecarExit to observe.
956+
if (connection.supervisedDaemon) armSupervisedMonitor();
913957
} catch (error) {
914958
log.error("Failed to load managed sidecar web UI", error);
915959
const failedConnection = connection;
Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,26 @@
1+
# Headless Linux box for reproducing the desktop daemon attach/spawn wedge
2+
# against the REAL packaged app (no macOS, no Aqua, no host GUI). The linux
3+
# bundle is built on the host (electron-builder --linux --arm64) and mounted at
4+
# /app; this image only provides Xvfb plus Electron's runtime shared libraries
5+
# and a Node to run the CDP repro driver.
6+
FROM node:22-bookworm-slim
7+
8+
# Electron 41 runtime deps + Xvfb. Kept to the libs Chromium actually dlopen()s
9+
# at startup so the image stays small and the failure surface is just "missing
10+
# lib" (loud) rather than a silent renderer crash.
11+
RUN apt-get update && apt-get install -y --no-install-recommends \
12+
xvfb xauth \
13+
libnss3 libnspr4 \
14+
libatk1.0-0 libatk-bridge2.0-0 libatspi2.0-0 \
15+
libgtk-3-0 libgbm1 libdrm2 \
16+
libxcomposite1 libxdamage1 libxfixes3 libxrandr2 libxkbcommon0 \
17+
libasound2 libcups2 libpango-1.0-0 libcairo2 \
18+
libx11-6 libxcb1 libxext6 libxi6 libxtst6 libglib2.0-0 \
19+
ca-certificates \
20+
&& rm -rf /var/lib/apt/lists/*
21+
22+
COPY repro.mjs /repro/repro.mjs
23+
24+
# xvfb-run gives Chromium a virtual display; --no-sandbox is required because the
25+
# container runs as root and we are not setuid-installing chrome-sandbox.
26+
ENTRYPOINT ["xvfb-run", "-a", "-s", "-screen 0 1280x800x24", "node", "/repro/repro.mjs"]
Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,56 @@
1+
# Headless Linux reproduction: desktop daemon attach/spawn wedge
2+
3+
Reproduces, against the REAL packaged desktop app (no macOS, no host GUI), the
4+
production incident where the app wedges on the sidecar crash screen because a
5+
standalone `executor daemon run` (the CLI daemon) already owns `~/.executor`.
6+
7+
See `../../../notes/desktop-daemon-ownership-wedge.md` for the root cause. The
8+
fix lives in `apps/desktop/src/main/index.ts` (`startWithCurrentSettings`):
9+
when a spawn aborts because the data dir is already owned, the app attaches to
10+
the running cli-daemon instead of failing.
11+
12+
## What the driver does (`repro.mjs`)
13+
14+
Replays the exact incident over the Chrome DevTools Protocol:
15+
16+
1. Cold boot with no daemon -> the app spawns its own managed sidecar.
17+
2. SIGKILL that sidecar -> the in-window crash screen appears.
18+
3. Start a separate `executor daemon run` that takes over `~/.executor`.
19+
4. Click "Restart server".
20+
21+
Exit 0 = the app recovered by ATTACHING to the cli-daemon (fixed). Non-zero =
22+
it re-spawned into the scope lock and stayed wedged (the bug), or a harness
23+
failure. A watchdog (`REPRO_WATCHDOG_MS`, default 240s) bails loudly rather than
24+
hanging.
25+
26+
## Requirements
27+
28+
- Docker / OrbStack with linux/arm64 (Apple Silicon runs arm64 Linux natively;
29+
on x64 build the bundle for linux-x64 and pass `--platform linux/amd64`).
30+
31+
## Run
32+
33+
Build the linux bundle on the host, then run it in the container:
34+
35+
```sh
36+
# from apps/desktop — build the bundled CLI sidecar + the linux app bundle
37+
BUN_TARGET=linux-arm64 bun ./scripts/build-sidecar.ts
38+
bunx --bun electron-vite build
39+
bunx --bun electron-builder --linux --arm64 --config electron-builder.e2e.config.ts
40+
41+
# from this dir — build the image and run the repro
42+
cd ../../e2e/desktop-packaged/linux-vm
43+
docker build --platform linux/arm64 -t executor-linux-repro .
44+
docker run --rm --init --platform linux/arm64 \
45+
-v "$PWD/../../../apps/desktop/dist/linux-arm64-unpacked":/app:ro \
46+
executor-linux-repro
47+
```
48+
49+
Always run with `--init` so Electron's detached children are reaped and the
50+
container exits cleanly.
51+
52+
## Proving the bug (RED)
53+
54+
Build the bundle from a commit BEFORE the fix and run the same image against it:
55+
the driver reports `FAIL: app stayed wedged on the crash screen after restart`
56+
and exits non-zero. Against the fixed bundle it prints `PASS`.

0 commit comments

Comments
 (0)