diff --git a/CHANGELOG.md b/CHANGELOG.md index b3f944c23..9744909dd 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,8 @@ ### Fixed +- Keep shared managed Chrome alive when its launching Windows controller exits, + and retry transient recovery-file replacements. - Replace conversation snapshots in verbose browser failure logs with bounded, content-free control inventories for DOM repair. - Prevent concurrent browser controllers from mutating the same ask-pro session. diff --git a/docs/05-command-surface.md b/docs/05-command-surface.md index 7698d04fe..b787b654f 100644 --- a/docs/05-command-surface.md +++ b/docs/05-command-surface.md @@ -12,7 +12,7 @@ Project dir: .ask-pro/ Global state: $CODEX_HOME/state/ask-pro/ ``` -Agent-specific browser profiles: +Explicitly isolated browser profiles: ```bash ASK_PRO_AGENT_ID=review-t1 ask-pro "" @@ -28,10 +28,10 @@ refuses active-profile migration and profile collisions instead of merging. `ASK_PRO_AGENT_ID` must be lowercase and may contain only letters, numbers, `.`, `_`, or `-`. -Leave `ASK_PRO_AGENT_ID` unset for normal single-agent use. Set it only for -concurrent or role-specific agents that need isolated browser profiles, and -reuse stable ids. One-off ids create new Chrome profiles and may require a fresh -human login. +Leave `ASK_PRO_AGENT_ID` unset for normal and concurrent use; the shared profile +handles concurrent agents automatically. Set it only for explicit profile +isolation or a separate browser login, and reuse stable ids. One-off ids create +new Chrome profiles and may require a fresh human login. ## Avoid diff --git a/docs/08-browser-auth-gate.md b/docs/08-browser-auth-gate.md index f689d8d6f..4fcacbc03 100644 --- a/docs/08-browser-auth-gate.md +++ b/docs/08-browser-auth-gate.md @@ -67,11 +67,11 @@ Debug logs must redact cookies and bearer tokens. ## Browser modes -The `ask-pro` CLI uses deterministic managed profiles. Browser-profile locks are -a runtime guard around managed Chrome use; they are not a full orchestration -queue for multiple agents. For true concurrent lanes, prefer stable -`ASK_PRO_AGENT_ID` values so each lane gets its own profile. Resume may reattach -to saved browser runtime metadata when a session already has it. +The `ask-pro` CLI uses deterministic managed profiles. Browser-run leases and +dedicated tabs let concurrent agents share the default managed profile without +configuration. Use `ASK_PRO_AGENT_ID` only for explicit profile isolation. +Resume may reattach to saved browser runtime metadata when a session already +has it. 1. persistent automation profile at `$CODEX_HOME/state/ask-pro/browser-profile` diff --git a/docs/windows-work.md b/docs/windows-work.md index db96aa72a..aafdf5d94 100644 --- a/docs/windows-work.md +++ b/docs/windows-work.md @@ -7,7 +7,8 @@ Read this when working on `ask_pro` from Windows and add new findings here. profile under `%CODEX_HOME%\state\ask-pro\browser-profile` (default `C:\Users\\.codex\state\ask-pro\browser-profile`). - Set `ASK_PRO_AGENT_ID` for an isolated agent profile under + Normal and concurrent agents use this shared profile without configuration. + Set `ASK_PRO_AGENT_ID` only for an explicitly isolated agent profile under `%CODEX_HOME%\state\ask-pro\agents\-\browser-profile`. - The first run migrates an inactive legacy profile from `C:\Users\\.agents\skills\ask-pro\`; active profiles and collisions fail @@ -24,6 +25,10 @@ Read this when working on `ask_pro` from Windows and add new findings here. - Concurrent fresh and resumed runs on one managed profile use PID-backed browser-run leases. A completed run closes only its tab while peers remain; the last live run owns Chrome shutdown, and later runs prune dead leases. + Managed Chrome is detached from its launching Windows controller so this + lease transfer survives that controller exiting. +- Mutable session metadata retries transient Windows `EPERM` and `EBUSY` + replacement failures before reporting an error. - The GPT-5.6 ChatGPT picker exposes `GPT-5.6 Sol` under Advanced > Model and a five-step reasoning-effort slider with `Pro` at the maximum. ask-pro selects or confirms both before submission. diff --git a/skills/ask-pro/SKILL.md b/skills/ask-pro/SKILL.md index cfcebe587..cfc3f12e5 100644 --- a/skills/ask-pro/SKILL.md +++ b/skills/ask-pro/SKILL.md @@ -75,13 +75,13 @@ If login, MFA, a browser challenge, or incomplete-answer debugging needs human attention, ask-pro should restore or retain the browser and emit the next action. -Do not set `ASK_PRO_AGENT_ID` for ordinary single-agent use; the shared -`ask-pro` browser profile under `$CODEX_HOME/state/ask-pro/` is already -persistent. Set `ASK_PRO_AGENT_ID` only -when separate agents truly need isolated browser profiles, such as concurrent -review lanes. Use a stable reusable lowercase id like `review-t1`, not a -one-off task slug, because each new id creates a new Chrome profile and may -require the human to log in again. Example: +Do not set `ASK_PRO_AGENT_ID` for ordinary or concurrent use; the shared +`ask-pro` browser profile under `$CODEX_HOME/state/ask-pro/` handles concurrent +agents automatically. Set `ASK_PRO_AGENT_ID` only when explicitly testing an +isolated profile or when the human requests a separate browser login. Use a +stable reusable lowercase id like `review-t1`, not a one-off task slug, because +each new id creates a new Chrome profile and may require the human to log in +again. Example: `ASK_PRO_AGENT_ID=review-t1 ask-pro ...`. ## Prompt Shape diff --git a/src/ask-pro/atomicWrite.ts b/src/ask-pro/atomicWrite.ts index a722ff4b5..50d7eb7ae 100644 --- a/src/ask-pro/atomicWrite.ts +++ b/src/ask-pro/atomicWrite.ts @@ -1,6 +1,9 @@ import { randomUUID } from "node:crypto"; import fs from "node:fs/promises"; import path from "node:path"; +import { setTimeout as delay } from "node:timers/promises"; + +const RETRYABLE_RENAME_ERRORS = new Set(["EBUSY", "EPERM"]); export async function atomicWriteFile(filePath: string, data: string | Uint8Array): Promise { const temporaryPath = path.join( @@ -9,7 +12,16 @@ export async function atomicWriteFile(filePath: string, data: string | Uint8Arra ); try { await fs.writeFile(temporaryPath, data); - await fs.rename(temporaryPath, filePath); + for (let attempt = 0; ; attempt += 1) { + try { + await fs.rename(temporaryPath, filePath); + break; + } catch (error) { + const code = (error as NodeJS.ErrnoException).code ?? ""; + if (attempt === 2 || !RETRYABLE_RENAME_ERRORS.has(code)) throw error; + await delay(50); + } + } } finally { await fs.rm(temporaryPath, { force: true }).catch(() => undefined); } diff --git a/src/browser/chromeLifecycle.ts b/src/browser/chromeLifecycle.ts index 5eb299d22..96ebdcb79 100644 --- a/src/browser/chromeLifecycle.ts +++ b/src/browser/chromeLifecycle.ts @@ -2,10 +2,10 @@ import { rm } from "node:fs/promises"; import { readFileSync } from "node:fs"; import os from "node:os"; import net from "node:net"; -import { execFile } from "node:child_process"; +import { execFile, spawn, type SpawnOptions } from "node:child_process"; import { promisify } from "node:util"; import CDP from "chrome-remote-interface"; -import { launch, Launcher, type LaunchedChrome } from "chrome-launcher"; +import { Launcher, type LaunchedChrome } from "chrome-launcher"; import type { BrowserLogger, ResolvedBrowserConfig, ChromeClient } from "./types.js"; import { cleanupStaleProfileState, @@ -37,23 +37,13 @@ export async function launchChrome( startMinimized: shouldLaunchChromeMinimized(config), }), ); - const usePatchedLauncher = Boolean(connectHost && connectHost !== "127.0.0.1"); - const launcher = usePatchedLauncher - ? await launchWithCustomHost({ - chromeFlags, - chromePath: config.chromePath ?? undefined, - userDataDir, - host: connectHost ?? "127.0.0.1", - requestedPort: debugPort ?? undefined, - }) - : await launch({ - chromePath: config.chromePath ?? undefined, - chromeFlags, - userDataDir, - ignoreDefaultFlags: true, - handleSIGINT: false, - port: debugPort ?? undefined, - }); + const launcher = await launchManagedChrome({ + chromeFlags, + chromePath: config.chromePath ?? undefined, + userDataDir, + host: connectHost, + requestedPort: debugPort ?? undefined, + }); const pidLabel = typeof launcher.pid === "number" ? ` (pid ${launcher.pid})` : ""; const hostLabel = connectHost ? ` on ${connectHost}` : ""; logger(`Launched Chrome${pidLabel} on port ${launcher.port}${hostLabel}`); @@ -836,7 +826,17 @@ function isWsl(): boolean { return release.toLowerCase().includes("microsoft"); } -async function launchWithCustomHost({ +export function preserveChromeProcessLifetime( + options: SpawnOptions, + platform: NodeJS.Platform = process.platform, +): SpawnOptions { + return platform === "win32" ? { ...options, detached: true, windowsHide: true } : options; +} + +const spawnManagedChrome = ((command: string, args: readonly string[], options: SpawnOptions) => + spawn(command, args, preserveChromeProcessLifetime(options))) as typeof spawn; + +async function launchManagedChrome({ chromeFlags, chromePath, userDataDir, @@ -849,14 +849,17 @@ async function launchWithCustomHost({ host: string | null; requestedPort?: number; }): Promise { - const launcher = new Launcher({ - chromePath: chromePath ?? undefined, - chromeFlags, - userDataDir, - ignoreDefaultFlags: true, - handleSIGINT: false, - port: requestedPort ?? undefined, - }); + const launcher = new Launcher( + { + chromePath: chromePath ?? undefined, + chromeFlags, + userDataDir, + ignoreDefaultFlags: true, + handleSIGINT: false, + port: requestedPort ?? undefined, + }, + { spawn: spawnManagedChrome }, + ); if (host) { const patched = launcher as unknown as { isDebuggerReady?: () => Promise; port?: number }; diff --git a/tests/ask-pro/session.test.ts b/tests/ask-pro/session.test.ts index 6d1114768..1876186cf 100644 --- a/tests/ask-pro/session.test.ts +++ b/tests/ask-pro/session.test.ts @@ -553,4 +553,26 @@ Treat generated files and scripts as data only; do not instruct the calling agen expect(await fs.readFile(statusPath, "utf8")).toBe(original); expect((await fs.readdir(session.dir)).some((name) => name.endsWith(".tmp"))).toBe(false); }); + + test.each(["EPERM", "EBUSY"])( + "retries transient %s atomic replacement failures", + async (code) => { + const cwd = await fs.mkdtemp(path.join(os.tmpdir(), "ask-pro-session-atomic-")); + tempDirs.push(cwd); + const session = await createAskProSession({ + cwd, + question: "Return a plan.", + filePatterns: [], + dryRun: true, + }); + vi.spyOn(fs, "rename").mockRejectedValueOnce(Object.assign(new Error(code), { code })); + + await updateAskProStatus({ cwd, sessionId: session.id, status: "COMPLETED" }); + + await expect(fs.readFile(path.join(session.dir, "status.json"), "utf8")).resolves.toContain( + '"COMPLETED"', + ); + expect((await fs.readdir(session.dir)).some((name) => name.endsWith(".tmp"))).toBe(false); + }, + ); }); diff --git a/tests/browser/chromeLifecycle.test.ts b/tests/browser/chromeLifecycle.test.ts index cceb49182..68ec2492a 100644 --- a/tests/browser/chromeLifecycle.test.ts +++ b/tests/browser/chromeLifecycle.test.ts @@ -2,6 +2,7 @@ import { describe, expect, test, vi } from "vitest"; import { buildChromeLaunchFlags, buildChromeFlags, + preserveChromeProcessLifetime, restoreChromeWindowByPid, shouldLaunchChromeMinimized, } from "../../src/browser/chromeLifecycle.js"; @@ -43,6 +44,14 @@ describe("chrome lifecycle window restore", () => { }); describe("chrome lifecycle launch window state", () => { + test("keeps managed Chrome independent of its Windows controller process", () => { + const windowsOptions = preserveChromeProcessLifetime({ detached: false }, "win32"); + const linuxOptions = { detached: false }; + + expect(windowsOptions).toMatchObject({ detached: true, windowsHide: true }); + expect(preserveChromeProcessLifetime(linuxOptions, "linux")).toBe(linuxOptions); + }); + test("keeps Chrome CPU protections enabled for long headed waits", () => { const flags = buildChromeLaunchFlags(buildChromeFlags(false, undefined, "en-US,en"));