diff --git a/docs-site/src/content/docs/guides/chatgpt-desktop.md b/docs-site/src/content/docs/guides/chatgpt-desktop.md index 761862d2ea..887493f193 100644 --- a/docs-site/src/content/docs/guides/chatgpt-desktop.md +++ b/docs-site/src/content/docs/guides/chatgpt-desktop.md @@ -95,6 +95,13 @@ This integration installs no certificate, network listener, PAC, or background watcher. It does not log the app's messages or environment. Status reports whether the running ChatGPT bundle process carries the expected launcher override. +If app discovery or process inspection fails, `status` reports `unknown` rather +than `not running`. `launch` and `restore` refuse to reopen an app whose exit +cannot be confirmed; restore keeps the launcher. Resolve the inspection error +and retry. Launch checks before writing its launcher, but a later inspection can +still fail after that atomic update; the commands do not guarantee an all-or-nothing +restart transaction. + ## Failure behavior and known limits When the platform is not macOS, the OpenCodex runtime is missing, or the filter diff --git a/src/cli/chatgpt-command.ts b/src/cli/chatgpt-command.ts index f870e66ff7..3006f44638 100644 --- a/src/cli/chatgpt-command.ts +++ b/src/cli/chatgpt-command.ts @@ -20,7 +20,7 @@ const USAGE = `Usage (experimental, macOS only): function run(command: string, args: string[]) { const result = spawnSync(command, args, { encoding: "utf8", timeout: 5000 }); - return { ok: result.status === 0, output: `${result.stdout ?? ""}${result.stderr ?? ""}`.trim() }; + return { ok: result.status === 0, status: result.status, output: `${result.stdout ?? ""}${result.stderr ?? ""}`.trim() }; } /** @@ -42,11 +42,12 @@ function chatgptDesktopIssueInFile(): string | null { * desktop restart adapter does. "ChatGPT" is a display name another app can share, so quitting, * relaunching and the binary path all key on this verified bundle rather than on the name. */ -function discoverApp(): DesktopAppInstall | null { +function discoverApp(): DesktopAppInstall | null | undefined { try { return darwinDesktopAppAdapter.discover(darwinDefaultExec); } catch { - return null; + // An inspection error is not evidence that the app is uninstalled. + return undefined; } } @@ -55,14 +56,20 @@ function discoverApp(): DesktopAppInstall | null { * `-a` keeps ancestors in the match: when ocx runs inside a ChatGPT/Codex session the app is * one of this process's ancestors, and plain `pgrep -x` would report it as not running. */ -function appState(install: DesktopAppInstall, launcher: string): { running: boolean; shim: boolean } { +function appState(install: DesktopAppInstall, launcher: string): { running: boolean | null; shim: boolean | null } { const shell = join(install.root, "Contents", "MacOS", "ChatGPT"); // Only this user's processes: another account's ChatGPT can neither be quit nor relaunched here. const uid = typeof process.getuid === "function" ? process.getuid() : undefined; const pids = run("pgrep", [...(uid === undefined ? [] : ["-U", String(uid)]), "-a", "-x", "ChatGPT"]); - if (!pids.ok) return { running: false, shim: false }; - for (const pid of pids.output.split(/\s+/).filter(value => /^\d+$/.test(value))) { + // pgrep exit 1 means no match; timeout, signal, or command errors are unknown. + if (!pids.ok) return pids.status === 1 + ? { running: false, shim: false } : { running: null, shim: null }; + const candidates = pids.output.split(/\s+/).filter(value => /^\d+$/.test(value)); + if (candidates.length === 0) return { running: null, shim: null }; + let unreadable = false; + for (const pid of candidates) { const command = run("ps", ["eww", "-o", "command=", "-p", pid]); + if (!command.ok || !command.output) unreadable = true; if (command.ok && (command.output === shell || command.output.startsWith(`${shell} `))) { const marker = `CODEX_CLI_PATH=${launcher}`; const start = command.output.indexOf(marker); @@ -70,20 +77,24 @@ function appState(install: DesktopAppInstall, launcher: string): { running: bool && (start + marker.length === command.output.length || command.output[start + marker.length] === " ") }; } } - return { running: false, shim: false }; + return unreadable ? { running: null, shim: null } : { running: false, shim: false }; } /** Adapted from #5947: open ignores new launch settings until the previous app exits. */ async function quitApp(install: DesktopAppInstall, launcher: string): Promise { - if (!appState(install, launcher).running) return true; + const initial = appState(install, launcher).running; + if (initial === false) return true; + if (initial === null) return false; for (let attempt = 0; attempt < 3; attempt++) { run("/usr/bin/osascript", ["-e", `quit app id "${install.id}"`]); for (let poll = 0; poll < 20; poll++) { - if (!appState(install, launcher).running) return true; + const running = appState(install, launcher).running; + if (running === false) return true; + if (running === null) return false; await Bun.sleep(250); } } - return !appState(install, launcher).running; + return appState(install, launcher).running === false; } export async function handleChatgptCommand(args: string[], platform: NodeJS.Platform = process.platform): Promise { @@ -105,16 +116,21 @@ export async function handleChatgptCommand(args: string[], platform: NodeJS.Plat const launcher = chatgptShimLauncherPath(); const install = discoverApp(); if (sub === "status") { - const app = install ? appState(install, launcher) : { running: false, shim: false }; + const app = install ? appState(install, launcher) + : install === undefined ? { running: null, shim: null } : { running: false, shim: false }; const configIssue = config.chatgptDesktop?.appServerShim === true ? null : chatgptDesktopIssueInFile(); const flag = config.chatgptDesktop?.appServerShim === true ? "on" : configIssue ? `off (config.json ${configIssue}; the whole chatgptDesktop block is ignored)` : "off"; console.log(`app-server shim (experimental): ${flag} launcher: ${existsSync(launcher) ? "present" : "absent"} -app: ${install ? (app.running ? "running" : "not running") : "not installed"} -CODEX_CLI_PATH launcher: ${app.shim ? "yes" : "no"}`); +app: ${install === undefined ? "unknown (app discovery failed)" : install ? (app.running === null ? "unknown (process inspection failed)" : app.running ? "running" : "not running") : "not installed"} +CODEX_CLI_PATH launcher: ${app.shim === null ? "unknown" : app.shim ? "yes" : "no"}`); return 0; } + if (install === undefined) { + console.error("ChatGPT app discovery failed; no launcher was changed and no app was opened. Retry after resolving discovery."); + return 1; + } if (!install) { if (sub === "restore") rmSync(launcher, { force: true }); console.error("ChatGPT (com.openai.codex) was not found; install or open it once, then retry."); @@ -143,9 +159,14 @@ CODEX_CLI_PATH launcher: ${app.shim ? "yes" : "no"}`); console.error(`Refusing to ${sub === "launch" ? "launch the shim" : "restore ChatGPT"}: ${untrusted}.`); return 1; } + // Preflight before changing the launcher. quitApp rechecks at the action boundary. + if (appState(install, launcher).running === null) { + console.error("ChatGPT process inspection failed; no launcher was changed and no app was opened. Retry after resolving inspection."); + return 1; + } if (binary) writeChatgptShimLauncher(undefined, binary); if (!(await quitApp(install, launcher))) { - console.error("ChatGPT did not quit; quit it manually and retry."); + console.error("ChatGPT exit could not be confirmed; no new app was opened. Check the running app and retry."); return 1; } // Remove an inherited override too: restore must launch without CODEX_CLI_PATH. diff --git a/structure/clients/chatgpt-desktop.md b/structure/clients/chatgpt-desktop.md index daabf923d9..dca30b1d07 100644 --- a/structure/clients/chatgpt-desktop.md +++ b/structure/clients/chatgpt-desktop.md @@ -61,7 +61,12 @@ app-server binary. It relaunches without that override and removes the launcher `com.openai.codex` bundle is found it removes the launcher, relaunches nothing and exits 1. Status reports the experimental flag, launcher presence, and the verified bundle process's override -without printing its environment. Other platforms reject all three operations. +without printing its environment. Discovery exceptions and failed process inspection +are unknown rather than evidence of an absent app. Launch preflights process inspection +before writing the launcher; both operations refuse to open when exit cannot be confirmed. +Restore keeps the launcher on those failures. A later probe may fail after launch has +atomically updated it; this is not a transaction spanning the launcher and app restart. +Other platforms reject all three operations. The launcher and its executable paths are local code-execution inputs. This integration installs no network listener, CA, PAC or background watcher, and diff --git a/tests/clients/desktop-app-server-shim-launcher.test.ts b/tests/clients/desktop-app-server-shim-launcher.test.ts index 0f89aeea61..db53fc7fd0 100644 --- a/tests/clients/desktop-app-server-shim-launcher.test.ts +++ b/tests/clients/desktop-app-server-shim-launcher.test.ts @@ -186,7 +186,7 @@ describe("restore validates the discovered app before any quit or open", () => { expect(result.status).toBe(0); return { ...JSON.parse(result.stdout.trim().split("\n").at(-1)!), stdout: result.stdout, stderr: result.stderr } as { code: number; calls: Array<{ command: string; args: string[]; override?: string }>; - launcherExists: boolean; root: string; shell: string; binary: string; stdout: string; stderr: string; + launcherExists: boolean; launcherContent: string | null; root: string; shell: string; binary: string; stdout: string; stderr: string; }; } finally { removeTreeWithRetry(dir); } }; @@ -199,6 +199,37 @@ describe("restore validates the discovered app before any quit or open", () => { expect(result.calls.filter(call => ["pgrep", "ps", "/usr/bin/osascript", "/usr/bin/open"].includes(call.command))).toEqual([]); }); } + for (const sub of ["launch", "restore"]) { + for (const scenario of [ + { pgrepFails: true }, { pgrepFails: true, pgrepTimeout: true }, + { psFails: true, running: true }, { invalidPgrep: true }, { discoveryFails: true }, + ]) { + test(`${sub} preserves the launcher and refuses unknown app state ${JSON.stringify(scenario)}`, () => { + const result = run({ sub, flag: true, ...scenario }); + expect(result.code).toBe(1); + expect(result.launcherContent).toBe("preserve until restore succeeds"); + expect(result.calls.some(call => ["/usr/bin/osascript", "/usr/bin/open"].includes(call.command))).toBe(false); + expect(result.stderr).not.toContain("private"); + }); + } + } + test("an inspection failure after quit does not authorize open or launcher removal", () => { + const result = run({ running: true, probeFailsAfterQuit: true }); + expect(result.code).toBe(1); + expect(result.launcherContent).toBe("preserve until restore succeeds"); + expect(result.calls.some(call => call.command === "/usr/bin/osascript")).toBe(true); + expect(result.calls.some(call => call.command === "/usr/bin/open")).toBe(false); + }); + for (const scenario of [{ pgrepFails: true }, { discoveryFails: true }, { psFails: true, running: true }]) { + test(`status reports unknown without mutation ${JSON.stringify(scenario)}`, () => { + const result = run({ sub: "status", ...scenario }); + expect(result.code).toBe(0); + expect(result.stdout).toContain("app: unknown"); + expect(result.stdout).toContain("CODEX_CLI_PATH launcher: unknown"); + expect(result.launcherContent).toBe("preserve until restore succeeds"); + expect(result.stdout).not.toContain("private"); + }); + } test("valid restore works with the flag off and no bundled app-server", () => { const result = run({ flag: false, missingBinary: true, running: true }); expect(result.code).toBe(0); diff --git a/tests/helpers/desktop-app-server-shim-command-child.ts b/tests/helpers/desktop-app-server-shim-command-child.ts index 1143cb5dde..72362b026b 100644 --- a/tests/helpers/desktop-app-server-shim-command-child.ts +++ b/tests/helpers/desktop-app-server-shim-command-child.ts @@ -19,6 +19,7 @@ for (let parent = dirname(root); ; parent = dirname(parent)) { } const calls: Array<{ command: string; args: string[]; override?: string }> = []; let running = scenario.running === true; +let pgrepCalls = 0; const uid = typeof process.getuid === "function" ? process.getuid() : -1; mock.module("node:fs", () => ({ ...fs, lstatSync(path: string, ...args: unknown[]) { @@ -48,8 +49,15 @@ mock.module("node:child_process", () => ({ ...childProcess, spawnSync(command: s status: scenario.trust === "unsigned" ? 1 : 0, stdout: args[0] === "--verify" ? "" : `TeamIdentifier=${scenario.trust === "foreign-team" ? "OTHERTEAM" : "2DC432GLL2"}\n`, stderr: "", }; - if (command === "pgrep") return { status: running ? 0 : 1, stdout: running ? "123" : "", stderr: "" }; - if (command === "ps") return { status: 0, stdout: shell, stderr: "" }; + if (command === "pgrep") { + pgrepCalls++; + if (scenario.pgrepFails || (scenario.probeFailsAfterQuit && pgrepCalls > 2)) return { + status: scenario.pgrepTimeout ? null : 2, stdout: "", stderr: "private fixture diagnostic", + }; + if (scenario.invalidPgrep) return { status: 0, stdout: "not-a-pid", stderr: "" }; + return { status: running ? 0 : 1, stdout: running ? "123" : "", stderr: "" }; + } + if (command === "ps") return { status: scenario.psFails ? 1 : 0, stdout: scenario.psFails ? "" : shell, stderr: "" }; if (command === "/usr/bin/osascript") running = false; if (command === "/usr/bin/open" && scenario.openFails) return { status: 1, stdout: "", stderr: "fixture open failed" }; return { status: 0, stdout: "", stderr: "" }; @@ -60,7 +68,10 @@ mock.module("../../src/config", () => ({ loadConfig: () => ({ chatgptDesktop: { mock.module("../../src/config/diagnostics", () => ({ readConfigFileSnapshot: () => ({ raw: scenario.configRaw }) })); mock.module("../../src/codex/desktop-app/darwin", () => ({ darwinDefaultExec: () => "", - darwinDesktopAppAdapter: { discover: () => scenario.noInstall ? null : { id: "com.openai.codex", root, relaunch: "com.openai.codex" } }, + darwinDesktopAppAdapter: { discover: () => { + if (scenario.discoveryFails) throw new Error("private discovery diagnostic"); + return scenario.noInstall ? null : { id: "com.openai.codex", root, relaunch: "com.openai.codex" }; + } }, })); mock.module("../../src/chatgpt/app-server-shim/launcher", () => ({ chatgptShimLauncherPath: () => launcher, @@ -69,4 +80,4 @@ mock.module("../../src/chatgpt/app-server-shim/launcher", () => ({ })); const { handleChatgptCommand } = await import("../../src/cli/chatgpt-command"); const code = await handleChatgptCommand([scenario.sub ?? "restore"], "darwin"); -console.log(JSON.stringify({ code, calls, launcherExists: fs.existsSync(launcher), root, shell, binary })); +console.log(JSON.stringify({ code, calls, launcherExists: fs.existsSync(launcher), launcherContent: fs.existsSync(launcher) ? fs.readFileSync(launcher, "utf8") : null, root, shell, binary }));