History

[codex] Fix desktop black screen fallback (#1051)

* Fix desktop black screen fallback * Add desktop black screen changeset

Rhys Sullivan committed 4 months ago

Commit cfda0ac1 parent af96bae6 files · +355 −18Browse files

Unified · Split

6 files
  1. .changeset/desktop-black-screen-fallback.md +8 −0
  2. apps/desktop/src/main/index.ts +83 −17
  3. apps/desktop/src/main/supervised-connection.test.ts +79 −0
  4. apps/desktop/src/main/supervised-connection.ts +22 −0
  5. e2e/desktop-packaged/supervised-regressions.test.ts +135 −1
  6. e2e/notes/testing-on-mac.md +28 −0
.changeset/desktop-black-screen-fallback.md+8−0
@@ -0,0 +1,8 @@
1+---
2+"executor": patch
3+---
4+
5+Fix desktop startup so a failed supervised-daemon replacement no longer leaves
6+the app on a black window. The desktop now re-checks the daemon after install
7+failures, falls back to a managed sidecar when the stale daemon disappears, and
8+surfaces startup recovery instead of leaving a failed renderer visible.
apps/desktop/src/main/index.ts+83−17
@@ -35,6 +35,7 @@ import { · 34 unmodified lines
3535 reportAProblem,
3636 } from "./diagnostics";
3737 import { sidecarCrashHtml } from "./crash-screen";
38+import { replaceSupervisedDaemonForDesktop } from "./supervised-connection";
3839 import {
3940 bundledExecutorPath,
4041 installSupervisedService,
@@ -88,6 +89,11 @@ const liveMainWindow = (): BrowserWindow | null => { · 47 unmodified lines
8889 return window;
8990 };
9091
92+const destroyWindow = (window: BrowserWindow) => {
93+ if (mainWindow === window) mainWindow = null;
94+ if (!window.isDestroyed()) window.destroy();
95+};
96+
9197 const focusMainWindow = () => {
9298 const window = liveMainWindow();
9399 if (!window) {
@@ -208,17 +214,21 @@ const ensureSupervisedConnection = async (): Promise<SidecarConnection | null> = · 114 unmodified lines
208214 if (attached) {
209215 if (!shouldReplaceDaemonForDesktop(attached)) return attached;
210216 const settings = getServerSettings();
211− // oxlint-disable-next-line executor/no-try-catch-or-throw -- boundary: desktop launch should attach to the running daemon if automatic upgrade fails
212− try {
213− await installSupervisedService({
214− port: settings.port,
215− dataDir: DESKTOP_DATA_DIR,
216− });
217− return (await waitForSupervisedAttach(30_000, { port: settings.port })) ?? attached;
218− } catch (error) {
219− log.warn("Failed to replace older supervised daemon; attaching to the running daemon", error);
220− return attached;
221− }
217+ return replaceSupervisedDaemonForDesktop(attached, {
218+ install: () =>
219+ installSupervisedService({
220+ port: settings.port,
221+ dataDir: DESKTOP_DATA_DIR,
222+ }),
223+ waitForAttach: () => waitForSupervisedAttach(30_000, { port: settings.port }),
224+ attach: attachToSupervisedDaemon,
225+ onInstallFailure: (error) => {
226+ log.warn(
227+ "Failed to replace older supervised daemon; re-checking before falling back",
228+ error,
229+ );
230+ },
231+ });
222232 }
223233
224234 const status = await supervisedServiceStatus();
@@ -452,7 +462,18 @@ const createWindow = async (conn: SidecarConnection) => { · 227 unmodified lines
452462 return { action: "deny" };
453463 });
454464
455− await window.loadURL(webUrlForConnection(conn));
465+ // A supervised daemon can pass the health probe and still disappear before
466+ // navigation begins. Treat that as a failed connection instead of leaving the
467+ // user with a visible BrowserWindow that only shows the black background.
468+ // oxlint-disable-next-line executor/no-try-catch-or-throw -- boundary: Electron navigation rejects when the sidecar vanishes during startup
469+ try {
470+ await window.loadURL(webUrlForConnection(conn));
471+ } catch (error) {
472+ log.error("Failed to load Executor web UI", error);
473+ destroyWindow(window);
474+ // oxlint-disable-next-line executor/no-try-catch-or-throw -- boundary: caller decides whether to fall back or surface startup failure
475+ throw error;
476+ }
456477 };
457478
458479 const showPortInUseDialog = async (port: number) => {
@@ -852,10 +873,18 @@ const boot = async () => { · 393 unmodified lines
852873 const supervised = await ensureSupervisedConnection();
853874 if (supervised) {
854875 connection = supervised;
855− await createWindow(supervised); // installs the bearer-auth header itself
856− armSupervisedMonitor();
857− void runUpdateCheck({ alertOnFail: false });
858− return;
876+ // createWindow installs the bearer-auth header itself.
877+ // oxlint-disable-next-line executor/no-try-catch-or-throw -- boundary: supervised attach can race with daemon shutdown; fall back to managed spawn
878+ try {
879+ await createWindow(supervised);
880+ armSupervisedMonitor();
881+ void runUpdateCheck({ alertOnFail: false });
882+ return;
883+ } catch (error) {
884+ log.warn("Failed to load supervised daemon; falling back to managed sidecar", error);
885+ stopSupervisedMonitor();
886+ connection = null;
887+ }
859888 }
860889 }
861890 connection = await startWithCurrentSettings();
@@ -878,7 +907,44 @@ const boot = async () => { · 16 unmodified lines
878907 app.quit();
879908 return;
880909 }
881− await createWindow(connection);
910+ // oxlint-disable-next-line executor/no-try-catch-or-throw -- boundary: renderer navigation failure must show the startup recovery dialog, not a black window
911+ try {
912+ await createWindow(connection);
913+ } catch (error) {
914+ log.error("Failed to load managed sidecar web UI", error);
915+ const failedConnection = connection;
916+ connection = null;
917+ await stopConnection(failedConnection);
918+
919+ const retryAfterReset = await handleFatalSidecarFailure(error);
920+ if (!retryAfterReset) {
921+ app.quit();
922+ return;
923+ }
924+
925+ lastSidecarStartError = null;
926+ connection = await startWithCurrentSettings();
927+ if (!connection && lastSidecarStartError != null) {
928+ await handleFatalSidecarFailure(lastSidecarStartError);
929+ }
930+ if (!connection) {
931+ app.quit();
932+ return;
933+ }
934+
935+ // oxlint-disable-next-line executor/no-try-catch-or-throw -- boundary: one post-reset retry before giving the user the final startup failure
936+ try {
937+ await createWindow(connection);
938+ } catch (retryError) {
939+ log.error("Failed to load managed sidecar web UI after reset", retryError);
940+ const failedRetryConnection = connection;
941+ connection = null;
942+ await stopConnection(failedRetryConnection);
943+ await handleFatalSidecarFailure(retryError);
944+ app.quit();
945+ return;
946+ }
947+ }
882948 // Check at boot. If an update is available, autoDownload pulls it and
883949 // the 'update-downloaded' handler fires the install dialog. Silent on
884950 // no-update / failure so we don't bother users on every launch.
apps/desktop/src/main/supervised-connection.test.ts+79−0
@@ -0,0 +1,79 @@
1+import { describe, expect, it } from "@effect/vitest";
2+import { Effect } from "effect";
3+import type { SidecarConnection } from "./sidecar";
4+import { replaceSupervisedDaemonForDesktop } from "./supervised-connection";
5+
6+const connection = (baseUrl: string): SidecarConnection => ({
7+ baseUrl,
8+ hostname: "localhost",
9+ port: 4789,
10+ username: "executor",
11+ authToken: "token",
12+ child: null,
13+ supervisedDaemon: true,
14+ ownerVersion: "1.5.12",
15+ ownerClient: "cli",
16+ ownerExecutablePath: "/tmp/old-executor",
17+});
18+
19+const rejectedInstall = (error: unknown): Promise<void> => Effect.runPromise(Effect.fail(error));
20+
21+describe("replaceSupervisedDaemonForDesktop", () => {
22+ it("uses the replacement daemon after install succeeds", async () => {
23+ const stale = connection("http://localhost:4789");
24+ const fresh = connection("http://localhost:55334");
25+ let installCalls = 0;
26+ let waitCalls = 0;
27+ let attachCalls = 0;
28+
29+ const result = await replaceSupervisedDaemonForDesktop(stale, {
30+ install: async () => {
31+ installCalls += 1;
32+ },
33+ waitForAttach: async () => {
34+ waitCalls += 1;
35+ return fresh;
36+ },
37+ attach: async () => {
38+ attachCalls += 1;
39+ return stale;
40+ },
41+ });
42+
43+ expect(result).toBe(fresh);
44+ expect(installCalls).toBe(1);
45+ expect(waitCalls).toBe(1);
46+ expect(attachCalls).toBe(0);
47+ });
48+
49+ it("re-probes after install fails instead of returning the stale daemon", async () => {
50+ const stale = connection("http://localhost:4789");
51+ const installFailure = { _tag: "InstallFailure" };
52+ let loggedError: unknown;
53+ let loggedConnection: SidecarConnection | null = null;
54+ let waitCalls = 0;
55+ let attachCalls = 0;
56+
57+ const result = await replaceSupervisedDaemonForDesktop(stale, {
58+ install: () => rejectedInstall(installFailure),
59+ waitForAttach: async () => {
60+ waitCalls += 1;
61+ return stale;
62+ },
63+ attach: async () => {
64+ attachCalls += 1;
65+ return null;
66+ },
67+ onInstallFailure: (error, connection) => {
68+ loggedError = error;
69+ loggedConnection = connection;
70+ },
71+ });
72+
73+ expect(result).toBeNull();
74+ expect(waitCalls).toBe(0);
75+ expect(attachCalls).toBe(1);
76+ expect(loggedError).toBe(installFailure);
77+ expect(loggedConnection).toBe(stale);
78+ });
79+});
apps/desktop/src/main/supervised-connection.ts+22−0
@@ -0,0 +1,22 @@
1+/* oxlint-disable executor/no-try-catch-or-throw -- boundary: Electron startup must convert service-install failures into a fresh daemon probe */
2+import type { SidecarConnection } from "./sidecar";
3+
4+export interface ReplaceSupervisedDaemonInput {
5+ readonly install: () => Promise<void>;
6+ readonly waitForAttach: () => Promise<SidecarConnection | null>;
7+ readonly attach: () => Promise<SidecarConnection | null>;
8+ readonly onInstallFailure?: (error: unknown, staleConnection: SidecarConnection) => void;
9+}
10+
11+export const replaceSupervisedDaemonForDesktop = async (
12+ staleConnection: SidecarConnection,
13+ input: ReplaceSupervisedDaemonInput,
14+): Promise<SidecarConnection | null> => {
15+ try {
16+ await input.install();
17+ return (await input.waitForAttach()) ?? (await input.attach());
18+ } catch (error) {
19+ input.onInstallFailure?.(error, staleConnection);
20+ return await input.attach();
21+ }
22+};
e2e/desktop-packaged/supervised-regressions.test.ts+135−1
@@ -660,6 +660,15 @@ if (!guiAvailable() || !packagedSingleInstanceAvailable()) { · 659 unmodified lines
660660 }),
661661 );
662662
663+ scenario(
664+ "Desktop packaged supervised attach · install failure falls back instead of black-screening",
665+ { timeout: 300_000 },
666+ Effect.gen(function* () {
667+ const runDir = yield* RunDir;
668+ yield* Effect.promise(() => runInstallFailureFallsBackToManagedSidecar(runDir));
669+ }),
670+ );
671+
663672 scenario(
664673 "Desktop packaged supervised settings · changing the port moves the active daemon",
665674 { timeout: 300_000 },
@@ -685,6 +694,9 @@ const writeCliDaemonManifest = (input: { · 19 unmodified lines
685694 readonly origin: string;
686695 readonly displayName: string;
687696 readonly token: string;
697+ readonly ownerVersion?: string | null;
698+ readonly ownerClient?: "cli" | "desktop";
699+ readonly ownerExecutablePath?: string | null;
688700 }): void => {
689701 mkdirSync(input.controlDir, { recursive: true });
690702 writeFileSync(
@@ -701,12 +713,134 @@ const writeCliDaemonManifest = (input: { · 10 unmodified lines
701713 displayName: input.displayName,
702714 auth: { kind: "bearer", token: input.token },
703715 }),
704− owner: { client: "cli", version: null, executablePath: null },
716+ owner: {
717+ client: input.ownerClient ?? "cli",
718+ version: input.ownerVersion ?? null,
719+ executablePath: input.ownerExecutablePath ?? null,
720+ },
705721 }),
706722 { mode: 0o600 },
707723 );
708724 };
709725
726+const shellSingleQuote = (value: string): string => `'${value.replaceAll("'", "'\"'\"'")}'`;
727+
728+const withFailingBundledInstall = async <T>(run: () => Promise<T>): Promise<T> => {
729+ const { executor } = requireBundle();
730+ const original = readFileSync(executor);
731+ const mode = statSync(executor).mode & 0o777;
732+ const backup = `${executor}.e2e-real`;
733+ writeFileSync(backup, original, { mode });
734+ chmodSync(backup, mode);
735+ writeFileSync(
736+ executor,
737+ [
738+ "#!/bin/sh",
739+ 'if [ "$1" = "install" ]; then',
740+ ' echo "launchctl bootstrap failed (exit 5): Bootstrap failed: 5: Input/output error" >&2',
741+ " exit 1",
742+ "fi",
743+ `exec ${shellSingleQuote(backup)} "$@"`,
744+ "",
745+ ].join("\n"),
746+ { mode: 0o755 },
747+ );
748+
749+ try {
750+ return await run();
751+ } finally {
752+ writeFileSync(executor, original, { mode });
753+ chmodSync(executor, mode);
754+ rmSync(backup, { force: true });
755+ }
756+};
757+
758+const runInstallFailureFallsBackToManagedSidecar = async (runDir: string) => {
759+ const home = mkdtempSync(join(tmpdir(), "executor-pkg-install-failure-fallback-"));
760+ const dataDir = join(home, ".executor");
761+ const controlDir = join(dataDir, "server-control");
762+ const token = "install-failure-fallback-token";
763+ const launchdSnapshot = captureLaunchdService();
764+ const oldPort = await freePort();
765+ const requests: Array<{ readonly url: string; readonly authorization: string | null }> = [];
766+ let resolveHealthProbe!: () => void;
767+ const sawHealthProbe = new Promise<void>((resolve) => {
768+ resolveHealthProbe = resolve;
769+ });
770+ let app: PackagedApp | undefined;
771+ let serverOpen = false;
772+
773+ const server = createServer((req: IncomingMessage, res) => {
774+ const url = req.url ?? "/";
775+ requests.push({
776+ url,
777+ authorization: req.headers.authorization ?? null,
778+ });
779+ if (url.startsWith("/api/health")) {
780+ res.writeHead(200, {
781+ "content-type": "text/plain",
782+ connection: "close",
783+ });
784+ res.end("ok", () => {
785+ resolveHealthProbe();
786+ void closeServer();
787+ });
788+ return;
789+ }
790+ res.writeHead(200, { "content-type": "text/html" });
791+ res.end("<!doctype html><title>Stale Executor</title><body>Stale daemon</body>");
792+ });
793+ const closeServer = async (): Promise<void> => {
794+ if (!serverOpen) return;
795+ serverOpen = false;
796+ await new Promise<void>((resolve) => server.close(() => resolve()));
797+ };
798+
799+ try {
800+ await new Promise<void>((resolve) =>
801+ server.listen(oldPort, "127.0.0.1", () => {
802+ serverOpen = true;
803+ resolve();
804+ }),
805+ );
806+ writeCliDaemonManifest({
807+ controlDir,
808+ dataDir,
809+ origin: `http://127.0.0.1:${oldPort}`,
810+ displayName: "Older daemon that disappears",
811+ token,
812+ ownerVersion: "0.0.0",
813+ });
814+
815+ await withFailingBundledInstall(async () => {
816+ app = await launchPackaged(home);
817+ const page = app.cdp;
818+ await sawHealthProbe;
819+ await page.waitForText("Settings", 120_000);
820+ await page.screenshot(join(runDir, "01-fell-back-to-managed-sidecar.png"));
821+
822+ const connection = await page.evaluate<{ readonly origin: string } | null>(
823+ "window.executor.getServerConnection()",
824+ );
825+ expect(
826+ new URL(connection!.origin).port,
827+ "desktop should not keep rendering from the stale daemon port after install failure",
828+ ).not.toBe(String(oldPort));
829+ expect(
830+ requests
831+ .filter((request) => request.url.startsWith("/api/health"))
832+ .map((request) => request.authorization),
833+ "the stale-daemon health probe must not disclose the saved bearer",
834+ ).not.toContain(`Bearer ${token}`);
835+ });
836+ } finally {
837+ await closePackaged(app);
838+ await restoreLaunchdService(launchdSnapshot);
839+ await closeServer();
840+ rmSync(home, { recursive: true, force: true });
841+ }
842+};
843+
710844 const runSlowLiveDaemonProbe = async (runDir: string) => {
711845 const home = mkdtempSync(join(tmpdir(), "executor-pkg-slow-live-daemon-"));
712846 const dataDir = join(home, ".executor");
e2e/notes/testing-on-mac.md+28−0
@@ -0,0 +1,28 @@
1+# Testing on macOS
2+
3+Packaged desktop e2e tests need a real GUI session. Check `launchctl managername`;
4+it should report `Aqua`. SSH-only sessions usually land in the Background
5+launchd domain, where `open` can fail with `OSLaunchdErrorDomain Code=125` even
6+after the VM is reachable over SSH.
7+
8+For VM runs, log in through VNC before launching the app. SSH is still useful
9+for staging files, reading logs, and running probes, but GUI app launch and
10+window assertions are more reliable after a console user is active.
11+
12+[trycua/cua](https://github.com/trycua/cua)'s Lume CLI is a useful way to clone,
13+boot, SSH into, VNC into, stop, and delete local macOS VMs programmatically.
14+
15+The packaged desktop app has a single-instance lock on macOS. If
16+`/Applications/Executor.app` is already running on the host, prefer an isolated
17+VM or close that app before running `vitest run --project desktop-packaged`.
18+
19+Useful evidence locations:
20+
21+- Main process log: `~/Library/Logs/Executor/main.log`
22+- Supervised daemon manifest: `~/.executor/server-control/server.json`
23+- Electron CDP port: `~/Library/Application Support/Executor/DevToolsActivePort`
24+
25+When debugging black-window reports, verify both pixels and renderer state. VNC
26+can occasionally show a black framebuffer while CDP proves the renderer DOM is
27+healthy, so capture a CDP screenshot or inspect `document.body.innerText` before
28+calling a UI blank.