fix(coding-agent): preserved late browser rejections
Logged late user continuation failures in cmux runs and delayed worker rejection folding until request-interception cleanup completed. This closes both windows where missing awaits could be silently dropped.
This commit is contained in:
@@ -1397,9 +1397,13 @@ export async function runCmuxCode(tab: CmuxTab, opts: RunCmuxCodeOptions): Promi
|
||||
let runActive = true;
|
||||
let hasFloatingFailure = false;
|
||||
const recordFloatingFailure = (reason: unknown): void => {
|
||||
if (!runActive || hasFloatingFailure || postmortem.isExpectedCleanupError(reason)) return;
|
||||
hasFloatingFailure = true;
|
||||
if (hasFloatingFailure || postmortem.isExpectedCleanupError(reason)) return;
|
||||
const message = reason instanceof Error ? reason.message : String(reason);
|
||||
if (!runActive) {
|
||||
logger.warn("Unhandled rejection after browser run ended", { runId, error: message });
|
||||
return;
|
||||
}
|
||||
hasFloatingFailure = true;
|
||||
const error = new Error(`Unhandled rejection (missing await?): ${message}`, { cause: reason });
|
||||
if (reason instanceof Error) error.name = reason.name;
|
||||
rejectFloatingFailure(error);
|
||||
|
||||
@@ -1140,13 +1140,13 @@ export class WorkerCore {
|
||||
failure = { error };
|
||||
} finally {
|
||||
await Bun.sleep(0);
|
||||
failure = this.#foldFloatingRejections(active, failure);
|
||||
runAc.abort(postmortem.markExpectedCleanupError(new ToolAbortError("Browser run ended")));
|
||||
try {
|
||||
await runPage?.cleanup();
|
||||
} catch (error) {
|
||||
failure = { error };
|
||||
}
|
||||
failure = this.#foldFloatingRejections(active, failure);
|
||||
if (this.#active?.id === msg.id) this.#active = null;
|
||||
}
|
||||
if (failure) {
|
||||
|
||||
@@ -42,6 +42,7 @@ import {
|
||||
runInTab,
|
||||
} from "@oh-my-pi/pi-coding-agent/tools/browser/tab-supervisor";
|
||||
import type { ToolSession } from "@oh-my-pi/pi-coding-agent/tools/index";
|
||||
import * as logger from "@oh-my-pi/pi-utils/logger";
|
||||
|
||||
function makeKind(socketSuffix: string): CmuxKind {
|
||||
return {
|
||||
@@ -286,6 +287,55 @@ describe("browser tab-supervisor — cmux tab close mid-run (#4499)", () => {
|
||||
}
|
||||
});
|
||||
|
||||
it("logs a user continuation rejection after its cmux run ends", async () => {
|
||||
spyOn(CmuxSocketClient.prototype, "connect").mockResolvedValue(undefined);
|
||||
spyOn(CmuxSocketClient.prototype, "close").mockImplementation(() => undefined);
|
||||
spyOn(CmuxSocketClient.prototype, "request").mockImplementation(
|
||||
async (method: string): Promise<Record<string, unknown>> => {
|
||||
switch (method) {
|
||||
case "browser.open_split":
|
||||
return { surface_id: "surface-late-rejection", url: "about:blank" };
|
||||
case "browser.url.get":
|
||||
return { url: "about:blank" };
|
||||
case "browser.snapshot":
|
||||
return { page: { html: "" } };
|
||||
case "browser.eval":
|
||||
return { value: "" };
|
||||
default:
|
||||
return {};
|
||||
}
|
||||
},
|
||||
);
|
||||
const warn = spyOn(logger, "warn").mockImplementation(() => {});
|
||||
const browser = await acquireBrowser(makeKind("late-rejection"), { cwd: "/tmp" });
|
||||
await acquireTab("late-rejection", browser, {
|
||||
timeoutMs: 5_000,
|
||||
ownerSessionId: "session-late-rejection",
|
||||
});
|
||||
|
||||
const result = await runInTab("late-rejection", {
|
||||
code: `
|
||||
const continuationStarted = Promise.withResolvers();
|
||||
void tab.title().then(async () => {
|
||||
continuationStarted.resolve();
|
||||
await Bun.sleep(50);
|
||||
throw new Error("late cmux continuation failed");
|
||||
});
|
||||
await continuationStarted.promise;
|
||||
return "completed";
|
||||
`,
|
||||
timeoutMs: 5_000,
|
||||
session: makeSession("/tmp"),
|
||||
});
|
||||
expect(result.returnValue).toBe("completed");
|
||||
|
||||
await Bun.sleep(150);
|
||||
expect(warn).toHaveBeenCalledWith("Unhandled rejection after browser run ended", {
|
||||
runId: expect.any(String),
|
||||
error: "late cmux continuation failed",
|
||||
});
|
||||
});
|
||||
|
||||
it("ignores the daemon screenshot path when no screenshot directory is configured", async () => {
|
||||
spyOn(CmuxSocketClient.prototype, "connect").mockResolvedValue(undefined);
|
||||
spyOn(CmuxSocketClient.prototype, "close").mockImplementation(() => undefined);
|
||||
|
||||
@@ -308,6 +308,45 @@ describe.skipIf(!CHROMIUM_AVAILABLE)("browser tab evaluation", () => {
|
||||
}
|
||||
}, 30_000);
|
||||
|
||||
it("folds a user continuation rejection that settles during cleanup", async () => {
|
||||
const tool = new BrowserTool(makeSession());
|
||||
const name = `cleanup-continuation-rejection-${process.pid}`;
|
||||
|
||||
try {
|
||||
await tool.execute("open", {
|
||||
action: "open",
|
||||
name,
|
||||
url: "data:text/html,<h1>ready</h1>",
|
||||
});
|
||||
let failure = "";
|
||||
try {
|
||||
await tool.execute("run", {
|
||||
action: "run",
|
||||
name,
|
||||
code: `
|
||||
await page.setRequestInterception(true);
|
||||
page.setRequestInterception = async () => {
|
||||
await Bun.sleep(50);
|
||||
};
|
||||
const continuationStarted = Promise.withResolvers();
|
||||
void tab.title().then(async () => {
|
||||
continuationStarted.resolve();
|
||||
await Bun.sleep(10);
|
||||
throw new Error("cleanup continuation failed");
|
||||
});
|
||||
await continuationStarted.promise;
|
||||
return "incorrect success";
|
||||
`,
|
||||
});
|
||||
} catch (error) {
|
||||
failure = error instanceof Error ? error.message : String(error);
|
||||
}
|
||||
expect(failure).toContain("Unhandled rejection (missing await?): cleanup continuation failed");
|
||||
} finally {
|
||||
await tool.execute("close", { action: "close", name, kill: true });
|
||||
}
|
||||
}, 30_000);
|
||||
|
||||
it("logs a user continuation rejection after its browser run ends", async () => {
|
||||
const warn = vi.spyOn(logger, "warn").mockImplementation(() => {});
|
||||
const tool = new BrowserTool(makeSession());
|
||||
|
||||
Reference in New Issue
Block a user