Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
10 changes: 5 additions & 5 deletions docs/05-command-surface.md
Original file line number Diff line number Diff line change
Expand Up @@ -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 "<question>"
Expand All @@ -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

Expand Down
10 changes: 5 additions & 5 deletions docs/08-browser-auth-gate.md
Original file line number Diff line number Diff line change
Expand Up @@ -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`
Expand Down
7 changes: 6 additions & 1 deletion docs/windows-work.md
Original file line number Diff line number Diff line change
Expand Up @@ -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\<you>\.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\<id>-<hash>\browser-profile`.
- The first run migrates an inactive legacy profile from
`C:\Users\<you>\.agents\skills\ask-pro\`; active profiles and collisions fail
Expand All @@ -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.
Expand Down
14 changes: 7 additions & 7 deletions skills/ask-pro/SKILL.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
14 changes: 13 additions & 1 deletion src/ask-pro/atomicWrite.ts
Original file line number Diff line number Diff line change
@@ -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<void> {
const temporaryPath = path.join(
Expand All @@ -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);
}
Expand Down
59 changes: 31 additions & 28 deletions src/browser/chromeLifecycle.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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}`);
Expand Down Expand Up @@ -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,
Expand All @@ -849,14 +849,17 @@ async function launchWithCustomHost({
host: string | null;
requestedPort?: number;
}): Promise<LaunchedChrome & { host?: string }> {
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<void>; port?: number };
Expand Down
22 changes: 22 additions & 0 deletions tests/ask-pro/session.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);
},
);
});
9 changes: 9 additions & 0 deletions tests/browser/chromeLifecycle.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import { describe, expect, test, vi } from "vitest";
import {
buildChromeLaunchFlags,
buildChromeFlags,
preserveChromeProcessLifetime,
restoreChromeWindowByPid,
shouldLaunchChromeMinimized,
} from "../../src/browser/chromeLifecycle.js";
Expand Down Expand Up @@ -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"));

Expand Down
Loading