aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authoramankumarsingh77 <amankumarsingh7702@gmail.com>2026-07-26 20:51:43 +0530
committeramankumarsingh77 <amankumarsingh7702@gmail.com>2026-07-26 20:51:43 +0530
commit7d2256e584709e4b7c27ab83b61950f00def7046 (patch)
tree642775bd745cb59423e762f4df8ff6ea65520ac4
parent083012ef3ede192505a953b7779d0c916dcaa98f (diff)
fix(profile): resolve replaced binaries and surface real launch failures
Two failures made a profile launch report the wrong cause. Linux appends " (deleted)" to /proc/<pid>/exe once the running binary is unlinked, which is what a google-chrome package upgrade does while the browser stays open. detectRunningBrowser passed that literal string on as the executable path, so every launch tried to spawn "/opt/google/chrome/chrome (deleted)" and hit ENOENT. Executable paths are now validated against disk, with the marker stripped and argv[0] used as a fallback; a binary genuinely named "… (deleted)" still wins. spawn() reports a missing or non-executable binary asynchronously via an 'error' event rather than throwing, and that event was discarded — so openProfileWindow returned success, no window opened, and the caller blamed the browser 15s later with a sentinel timeout. The launch now awaits 'spawn'/'error', returns the real cause, and removes its handshake page on the failure path. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
-rw-r--r--src/profile/browser-process.ts62
-rw-r--r--src/profile/launch.ts34
-rw-r--r--test/profile/browser-process-test.ts71
-rw-r--r--test/profile/launch-test.ts95
4 files changed, 244 insertions, 18 deletions
diff --git a/src/profile/browser-process.ts b/src/profile/browser-process.ts
index c0f6178..77bc05c 100644
--- a/src/profile/browser-process.ts
+++ b/src/profile/browser-process.ts
@@ -92,6 +92,47 @@ export type BrowserCandidate = {
readonly explicitUserDataDir?: string;
};
+/**
+ * Linux marks a replaced binary in `/proc/<pid>/exe`: once the file behind a
+ * running process is unlinked — which is exactly what a `google-chrome` package
+ * upgrade does while the browser keeps running — the symlink reads
+ * "/opt/google/chrome/chrome (deleted)". Spawning that string fails with
+ * ENOENT, so the marker has to come off.
+ *
+ * It is stripped only as a fallback, and only when the stripped path exists: a
+ * binary genuinely named "… (deleted)" must still win.
+ */
+const DELETED_SUFFIX = " (deleted)";
+
+/** The path without Linux's deleted-binary marker, or undefined if unmarked. */
+export const stripDeletedSuffix = (path: string): string | undefined =>
+ path.endsWith(DELETED_SUFFIX) && path.length > DELETED_SUFFIX.length
+ ? path.slice(0, -DELETED_SUFFIX.length)
+ : undefined;
+
+const exists = async (path: string): Promise<boolean> => {
+ try {
+ await access(path);
+ return true;
+ } catch {
+ return false;
+ }
+};
+
+/**
+ * A spawnable executable path for a raw readlink/argv value: the value itself
+ * when it exists, else the same value with the deleted-binary marker removed.
+ * Undefined when neither form is on disk — a path we cannot spawn must never be
+ * handed on as if it were usable.
+ */
+export const spawnableExePath = async (raw: string): Promise<string | undefined> => {
+ if (raw.length === 0) return undefined;
+ if (await exists(raw)) return raw;
+ const stripped = stripDeletedSuffix(raw);
+ if (stripped && (await exists(stripped))) return stripped;
+ return undefined;
+};
+
/** Path comparison that tolerates separator and case differences per platform. */
const samePath = (a: string, b: string): boolean => {
const norm = (p: string): string => {
@@ -163,14 +204,19 @@ const collectLinux = async (): Promise<ReadonlyArray<BrowserCandidate>> => {
continue;
}
if (!cmdline || hasChildProcessType(cmdline)) continue;
- let exePath: string | undefined;
+ let link: string | undefined;
try {
- exePath = await readlink(`/proc/${pid}/exe`);
+ link = await readlink(`/proc/${pid}/exe`);
} catch {
- // /proc/<pid>/exe is unreadable for processes we don't own; argv[0] is
- // the next best thing.
- exePath = cmdline.split(" ")[0];
+ // /proc/<pid>/exe is unreadable for processes we don't own.
}
+ // argv[0] backs up the symlink in both directions: unreadable for a process
+ // we don't own, and pointing at an unlinked binary after an in-place
+ // upgrade. Chrome's browser process rewrites its argv to the bare
+ // executable path, so on Linux argv[0] is the live install location.
+ const exePath =
+ (link ? await spawnableExePath(link) : undefined) ??
+ (await spawnableExePath(cmdline.split(" ")[0] ?? ""));
const explicit = parseUserDataDirFlag(cmdline);
candidates.push({
...(exePath ? { exePath } : {}),
@@ -354,7 +400,11 @@ export const isBrowserRunning = async (): Promise<boolean> => (await detectRunni
*/
export const resolveBrowserExecutable = async (running?: RunningBrowser): Promise<string | undefined> => {
const detected = running ?? (await detectRunningBrowser());
- if (detected.exePath) return detected.exePath;
+ // Re-check spawnability here too: this function is the single gate every
+ // launch passes through, and a caller may hand us a RunningBrowser collected
+ // before the binary was replaced on disk.
+ const usable = detected.exePath ? await spawnableExePath(detected.exePath) : undefined;
+ if (usable) return usable;
if (process.platform !== "win32") return undefined;
const fromRegistry = await winRegistryExecutable();
diff --git a/src/profile/launch.ts b/src/profile/launch.ts
index fd93d3d..0088b6d 100644
--- a/src/profile/launch.ts
+++ b/src/profile/launch.ts
@@ -69,29 +69,47 @@ export const openProfileWindow = async (
if (req.explicitUserDataDir) args.push(`--user-data-dir=${req.explicitUserDataDir}`);
args.push(`--profile-directory=${req.profileDir}`, "--new-window", sentinelUrl);
- let child: ChildProcess | null = null;
+ let child: ChildProcess;
try {
// No shell: arguments are passed as an array, so profile names and paths
// containing spaces survive intact on every platform.
child = spawn(req.exePath, args, { detached: true, stdio: "ignore" });
- child.unref();
- // The launcher process exits as soon as the running browser takes over the
- // command line; an error here means the binary itself could not be started.
- child.on("error", () => {});
} catch (e) {
await rm(file, { force: true }).catch(() => {});
return err(`could not launch ${req.exePath}: ${e instanceof Error ? e.message : String(e)}`);
}
+ // spawn() reports a missing or non-executable binary ASYNCHRONOUSLY, as an
+ // 'error' event — it does not throw. Ignoring that event made an ENOENT
+ // indistinguishable from a browser that simply never opened the window, so
+ // the caller's 15s sentinel poll reported a timeout for what was really a bad
+ // executable path. Node guarantees exactly one of 'spawn' or 'error', so
+ // waiting for it costs nothing and turns the failure into a named cause.
+ const spawnFailure = await new Promise<string | undefined>((resolve) => {
+ child.once("spawn", () => resolve(undefined));
+ child.once("error", (e: Error) => resolve(e.message));
+ });
+ // A later 'error' event with no listener would throw; keep one attached for
+ // the lifetime of the detached launcher.
+ child.on("error", () => {});
+ child.unref();
+
+ if (spawnFailure) {
+ await rm(file, { force: true }).catch(() => {});
+ return err(`could not launch ${req.exePath}: ${spawnFailure}`);
+ }
+
+ let live: ChildProcess | null = child;
+
return ok({
sentinelUrl,
cleanup: async () => {
await rm(file, { force: true }).catch(() => {});
},
kill: () => {
- if (child) {
- try { child.kill("SIGTERM"); } catch {}
- child = null;
+ if (live) {
+ try { live.kill("SIGTERM"); } catch {}
+ live = null;
}
},
});
diff --git a/test/profile/browser-process-test.ts b/test/profile/browser-process-test.ts
index 29982e5..29b3ddf 100644
--- a/test/profile/browser-process-test.ts
+++ b/test/profile/browser-process-test.ts
@@ -9,9 +9,16 @@
*
* Run: npx tsx test/profile/browser-process-test.ts
*/
-import { homedir } from "node:os";
+import { chmodSync, mkdtempSync, rmSync, writeFileSync } from "node:fs";
+import { homedir, tmpdir } from "node:os";
import { join } from "node:path";
-import { parseUserDataDirFlag, rankBrowserCandidate } from "../../src/profile/browser-process";
+import {
+ parseUserDataDirFlag,
+ rankBrowserCandidate,
+ resolveBrowserExecutable,
+ spawnableExePath,
+ stripDeletedSuffix,
+} from "../../src/profile/browser-process";
let passed = 0;
let failed = 0;
@@ -25,7 +32,7 @@ const check = (cond: boolean, label: string): void => {
}
};
-function main(): void {
+async function main(): Promise<void> {
// B1: --user-data-dir parsing
{
check(
@@ -101,8 +108,64 @@ function main(): void {
);
}
+ // B6: Linux's deleted-binary marker. An in-place `google-chrome` upgrade
+ // unlinks the running binary, after which /proc/<pid>/exe reads
+ // "…/chrome (deleted)" — a path spawn() rejects with ENOENT.
+ {
+ check(
+ stripDeletedSuffix("/opt/google/chrome/chrome (deleted)") === "/opt/google/chrome/chrome",
+ "B6: marker stripped from a replaced binary's path",
+ );
+ check(stripDeletedSuffix("/opt/google/chrome/chrome") === undefined, "B6: unmarked path left alone");
+ check(stripDeletedSuffix(" (deleted)") === undefined, "B6: marker alone is not a path");
+ }
+
+ // B7: only a path that is actually on disk may be handed on as spawnable.
+ {
+ const root = mkdtempSync(join(tmpdir(), "pi-exe-"));
+ try {
+ const real = join(root, "chrome");
+ writeFileSync(real, "#!/bin/sh\n");
+ chmodSync(real, 0o755);
+
+ check(await spawnableExePath(real) === real, "B7: an existing path is returned unchanged");
+ check(
+ await spawnableExePath(`${real} (deleted)`) === real,
+ "B7: a replaced binary resolves to the live install path",
+ );
+ check(await spawnableExePath(join(root, "absent")) === undefined, "B7: a missing path yields undefined");
+ check(await spawnableExePath("") === undefined, "B7: an empty path yields undefined");
+
+ // The launch gate must never pass on a path it cannot spawn — that is
+ // what produced a 15s "couldn't open a window" timeout instead of a
+ // named error.
+ check(
+ await resolveBrowserExecutable({ running: true, exePath: `${real} (deleted)` }) === real,
+ "B7: resolveBrowserExecutable repairs a replaced-binary path",
+ );
+ if (process.platform !== "win32") {
+ check(
+ await resolveBrowserExecutable({ running: true, exePath: join(root, "absent") }) === undefined,
+ "B7: an unspawnable path is not passed on as usable",
+ );
+ }
+
+ // A binary genuinely named "… (deleted)" must win over the stripped form.
+ // Created last, so the checks above see only the ordinary case.
+ const literal = join(root, "chrome (deleted)");
+ writeFileSync(literal, "#!/bin/sh\n");
+ chmodSync(literal, 0o755);
+ check(
+ await spawnableExePath(literal) === literal,
+ "B7: a real file named '… (deleted)' is preferred over stripping",
+ );
+ } finally {
+ rmSync(root, { recursive: true, force: true });
+ }
+ }
+
console.log(`\n${passed} passed, ${failed} failed`);
process.exit(failed === 0 ? 0 : 1);
}
-main();
+void main();
diff --git a/test/profile/launch-test.ts b/test/profile/launch-test.ts
new file mode 100644
index 0000000..0061273
--- /dev/null
+++ b/test/profile/launch-test.ts
@@ -0,0 +1,95 @@
+/**
+ * Unit tests for openProfileWindow's failure reporting — no browser required.
+ *
+ * spawn() reports a missing or non-executable binary asynchronously, as an
+ * 'error' event, not by throwing. When that event was ignored, a bad executable
+ * path returned `ok` and the caller's 15s sentinel poll blamed the browser:
+ * "couldn't open a window in <profile> automatically". These tests pin the
+ * failure to its real cause instead.
+ *
+ * Run: npx tsx test/profile/launch-test.ts
+ */
+import { existsSync, mkdtempSync, readdirSync, rmSync, writeFileSync } from "node:fs";
+import { tmpdir } from "node:os";
+import { join } from "node:path";
+import { fileURLToPath } from "node:url";
+import { openProfileWindow } from "../../src/profile/launch";
+
+let passed = 0;
+let failed = 0;
+const check = (cond: boolean, label: string): void => {
+ if (cond) {
+ passed++;
+ console.log(` ✓ ${label}`);
+ } else {
+ failed++;
+ console.error(` ✗ ${label}`);
+ }
+};
+
+async function main(): Promise<void> {
+ const root = mkdtempSync(join(tmpdir(), "pi-launch-"));
+ try {
+ // L1: a nonexistent binary — the shape of an in-place browser upgrade,
+ // where /proc/<pid>/exe still names the unlinked file.
+ {
+ const missing = join(root, "chrome (deleted)");
+ const r = await openProfileWindow({ exePath: missing, profileDir: "Default" });
+ check(!r.success, "L1: a missing executable fails the launch");
+ if (!r.success) {
+ check(r.error.includes(missing), "L1: the error names the path that could not be launched");
+ check(r.error.includes("ENOENT"), "L1: the error carries spawn's own cause");
+ }
+ }
+
+ // L2: a file that exists but is not executable — same async 'error' path,
+ // different errno (EACCES). Skipped where the check is meaningless.
+ if (process.platform !== "win32" && process.getuid?.() !== 0) {
+ const notExec = join(root, "not-executable");
+ writeFileSync(notExec, "#!/bin/sh\n", { mode: 0o644 });
+ const r = await openProfileWindow({ exePath: notExec, profileDir: "Default" });
+ check(!r.success, "L2: a non-executable file fails the launch");
+ }
+
+ // L3: a failed launch leaves no sentinel file behind. The caller only calls
+ // cleanup() on the success path, so the failure path owns its own tidying.
+ {
+ const before = process.env["TMPDIR"];
+ process.env["TMPDIR"] = root;
+ try {
+ const r = await openProfileWindow({ exePath: join(root, "absent"), profileDir: "Default" });
+ check(!r.success, "L3: launch failed as set up");
+ const leftovers = readdirSync(root).filter((n) => n.startsWith("pi-harness-"));
+ check(leftovers.length === 0, "L3: no handshake page is left on disk after a failed launch");
+ } finally {
+ if (before === undefined) delete process.env["TMPDIR"];
+ else process.env["TMPDIR"] = before;
+ }
+ }
+
+ // L4: a launcher that starts succeeds, and the sentinel it advertises is a
+ // real, readable file — the page the browser is asked to open.
+ {
+ const stub = join(root, "stub-browser");
+ writeFileSync(stub, "#!/bin/sh\nexit 0\n", { mode: 0o755 });
+ const r = await openProfileWindow({ exePath: stub, profileDir: "Default" });
+ check(r.success, "L4: a startable binary yields a handle");
+ if (r.success) {
+ const sentinel = fileURLToPath(r.data.sentinelUrl);
+ check(existsSync(sentinel), "L4: the sentinel page exists before the browser is polled");
+ await r.data.cleanup();
+ check(!existsSync(sentinel), "L4: cleanup removes the sentinel page");
+ r.data.kill();
+ r.data.kill();
+ check(true, "L4: kill is safe to call twice");
+ }
+ }
+ } finally {
+ rmSync(root, { recursive: true, force: true });
+ }
+
+ console.log(`\n${passed} passed, ${failed} failed`);
+ process.exit(failed === 0 ? 0 : 1);
+}
+
+void main();