authorgravatar for me@t3.ggTheo Browne <me@t3.gg> 2026-10-01 02:10:50-07:00
committergravatar for noreply@github.comGitHub <noreply@github.com> 2026-10-01 02:10:50-07:00
log5cc99e1c23980d7995a13c47f969b47cb68ed1be
treeb099901bb6d8aa16442b3fff8efcc0712ca9fee9
parent148e6deea046658639aae9fef5b349781cec39d1
signature Signed by PGP key B5690EEEBB952194

fix(desktop): agent clicks in the browser no longer pop Save dialogs (#14573)

Co-authored-by: Claude Opus 5.5 (1M context) <noreply@anthropic.com>

2 files changed, 157 insertions(+), 3 deletions(-)

apps/desktop/src/preview/Manager.test.ts+119-1
......@@ -186,6 +186,7 @@ const {
186186 fromId,
187187 getFocusedWebContents,
188188 mkdir,
189 previewSession,
189190 showItemInFolder,
190191 webviewSend,
191192 writeFile,
......@@ -200,6 +201,7 @@ const {
200201 fromId: vi.fn<(_id?: number) => Electron.WebContents | null>((_id?: number) => null),
201202 getFocusedWebContents: vi.fn(() => null),
202203 mkdir: vi.fn((_path: string) => undefined),
204 previewSession: { on: vi.fn() },
203205 showItemInFolder: vi.fn(),
204206 webviewSend: vi.fn(),
205207 writeFile: vi.fn((_path: string, _data: Uint8Array) => undefined),
......@@ -236,7 +238,7 @@ const browserSessionLayer = Layer.succeed(
236238 BrowserSession.BrowserSession.of({
237239 getPartition: () => Effect.succeed("persist:t3code-preview-test"),
238240 isPartition: (partition) => partition.startsWith("persist:t3code-preview-"),
239 getSession: () => Effect.die("unexpected getSession"),
241 getSession: () => Effect.succeed(previewSession as unknown as Electron.Session),
240242 clearCookies: () => Effect.void,
241243 clearCache: () => Effect.void,
242244 }),
......@@ -547,6 +549,7 @@ describe("PreviewManager", () => {
547549 getFocusedWebContents.mockReset();
548550 getFocusedWebContents.mockReturnValue(null);
549551 mkdir.mockClear();
552 previewSession.on.mockClear();
550553 writeFile.mockClear();
551554 showItemInFolder.mockClear();
552555 clipboardItemConstructor.mockClear();
......@@ -4536,6 +4539,121 @@ describe("PreviewManager", () => {
45364539 }),
45374540 ),
45384541 );
4542 effectIt.effect("saves downloads from agent-driven pages without a Save dialog", () =>
4543 withManager((manager) =>
4544 Effect.gen(function* () {
4545 let humanInput: ((event: unknown, signal?: unknown) => void) | undefined;
4546 const wc = makeTestPreviewWebContents(vi.fn());
4547 Object.assign(wc, {
4548 isDevToolsOpened: () => false,
4549 loadURL: vi.fn(async () => undefined),
4550 reload: vi.fn(),
4551 });
4552 Object.assign(wc.ipc, {
4553 on: vi.fn((channel: string, listener: typeof humanInput) => {
4554 if (channel === "preview:human-input") humanInput = listener;
4555 }),
4556 });
4557 let holdEvaluate = false;
4558 let releaseEvaluate: (() => void) | undefined;
4559 Object.assign(wc.debugger, {
4560 sendCommand: vi.fn(async (method: string, params?: { expression?: string }) => {
4561 if (method !== "Runtime.evaluate") return undefined;
4562 if (params?.expression?.includes("matched"))
4563 return { result: { value: { matched: true } } };
4564 if (holdEvaluate) {
4565 holdEvaluate = false;
4566 await new Promise<void>((resolve) => {
4567 releaseEvaluate = resolve;
4568 });
4569 }
4570 return { result: { value: 42 } };
4571 }),
4572 });
4573 fromId.mockReturnValue(wc);
4574 const takeovers = [yield* Deferred.make<void>(), yield* Deferred.make<void>()];
4575 let takeoverCount = 0;
4576 let controller = "none";
4577 yield* manager.subscribeStateChanges((_tabId, state) => {
4578 const takeover =
4579 controller !== "human" && state.controller === "human"
4580 ? takeovers[takeoverCount++]
4581 : undefined;
4582 controller = state.controller;
4583 return takeover ? Deferred.succeed(takeover, undefined).pipe(Effect.asVoid) : Effect.void;
4584 });
4585 yield* manager.getBrowserSession();
4586 yield* manager.getBrowserSession();
4587 const installs = previewSession.on.mock.calls.filter(
4588 ([event]) => event === "will-download",
4589 );
4590 expect(installs).toHaveLength(1);
4591 const willDownload = installs[0]![1] as (
4592 event: unknown,
4593 item: {
4594 getFilename: () => string;
4595 getStartTime: () => number;
4596 setSavePath: (path: string) => void;
4597 },
4598 source: Electron.WebContents,
4599 ) => void;
4600 const download = () => {
4601 const setSavePath = vi.fn();
4602 willDownload(
4603 {},
4604 { getFilename: () => "chart.png", getStartTime: () => 1_790_844_530.5, setSavePath },
4605 wc,
4606 );
4607 return setSavePath;
4608 };
4609 yield* manager.createTab("tab_1");
4610 yield* manager.registerWebview("tab_1", 42);
4611
4612 expect(download()).not.toHaveBeenCalled();
4613
4614 yield* manager.automationEvaluate("tab_1", { expression: "42" });
4615 const startedAt = (1_790_844_530_500).toString(36);
4616 expect(download()).toHaveBeenCalledWith(
4617 `/tmp/t3/dev/browser-artifacts/browser-download-${startedAt}-0-chart.png`,
4618 );
4619 // Same name, same millisecond: still a separate file.
4620 expect(download()).toHaveBeenCalledWith(
4621 `/tmp/t3/dev/browser-artifacts/browser-download-${startedAt}-1-chart.png`,
4622 );
4623
4624 humanInput?.({}, { kind: "pointer", x: 10, y: 10, button: 0 });
4625 yield* Deferred.await(takeovers[0]!);
4626 expect(download()).not.toHaveBeenCalled();
4627
4628 // An action still waiting for the page when the human takes over must
4629 // not mark it as agent-driven again.
4630 holdEvaluate = true;
4631 const running = yield* manager
4632 .automationEvaluate("tab_1", { expression: "42" })
4633 .pipe(Effect.forkChild({ startImmediately: true }));
4634 yield* settle(() => releaseEvaluate !== undefined);
4635 const queued = yield* manager
4636 .automationEvaluate("tab_1", { expression: "42" })
4637 .pipe(Effect.forkChild({ startImmediately: true }));
4638 humanInput?.({}, { kind: "pointer", x: 20, y: 20, button: 0 });
4639 yield* Deferred.await(takeovers[1]!);
4640 releaseEvaluate?.();
4641 yield* Fiber.await(running);
4642 expect(Exit.isFailure(yield* Fiber.await(queued))).toBe(true);
4643 expect(download()).not.toHaveBeenCalled();
4644
4645 // URL-bar navigation hands the page back to the human.
4646 yield* manager.automationEvaluate("tab_1", { expression: "42" });
4647 expect(download()).toHaveBeenCalled();
4648 yield* manager.navigate("tab_1", "https://example.com/report.csv");
4649 expect(download()).not.toHaveBeenCalled();
4650
4651 // Reading the page does not make it agent-driven.
4652 yield* manager.automationWaitFor("tab_1", { text: "Example" });
4653 expect(download()).not.toHaveBeenCalled();
4654 }),
4655 ),
4656 );
45394657});
45404658
45414659describe("PreviewOperationError", () => {
apps/desktop/src/preview/Manager.ts+38-2
......@@ -520,6 +520,9 @@ interface ExpectedAgentInput {
520520 */
521521const POPUP_PROTOCOLS = new Set(["http:", "https:"]);
522522
523/** Control actions that only read the page, so they do not make it agent-driven. */
524const READ_ONLY_CONTROL_ACTIONS = new Set(["snapshot", "waitFor"]);
525
523526const isPopupUrl = (rawUrl: string): boolean => {
524527 try {
525528 return POPUP_PROTOCOLS.has(new URL(rawUrl).protocol);
......@@ -685,6 +688,10 @@ const makeNativeOperations = Effect.fn("PreviewManager.makeOperations")(function
685688 // (electron#44618) and now always rejects with NotAllowedError.
686689 let pendingRecording: PendingRecording | null = null;
687690 const displayMediaHandlerSessions = new WeakSet<Session>();
691 const downloadHandlerSessions = new WeakSet<Session>();
692 let downloadCount = 0;
693 // Preview pages whose latest input came from an agent action, not the human.
694 const agentDrivenWebContents = new WeakSet<Electron.WebContents>();
688695 let frameCaptureWindowOpen = true;
689696 let currentMainWindow: BrowserWindow | undefined;
690697 let mainWindowCleanupFiber: Fiber.Fiber<void, never> | undefined;
......@@ -1487,7 +1494,6 @@ const makeNativeOperations = Effect.fn("PreviewManager.makeOperations")(function
14871494 const epoch = (yield* Ref.get(controlEpochRef)).get(tabId) ?? 0;
14881495 const control = yield* ensureControlSession(wc);
14891496 const execute = Effect.fn("PreviewManager.executeControlAction")(function* () {
1490 yield* update(tabId, { controller: "agent" });
14911497 const checkControl = Effect.gen(function* () {
14921498 const currentEpoch = (yield* Ref.get(controlEpochRef)).get(tabId) ?? 0;
14931499 if (currentEpoch !== epoch) {
......@@ -1498,6 +1504,11 @@ const makeNativeOperations = Effect.fn("PreviewManager.makeOperations")(function
14981504 });
14991505 }
15001506 });
1507 // A human who took over while this action waited for the permit owns
1508 // the page; marking it agent-driven would hide their Save dialog.
1509 yield* checkControl;
1510 if (!READ_ONLY_CONTROL_ACTIONS.has(action)) agentDrivenWebContents.add(wc);
1511 yield* update(tabId, { controller: "agent" });
15011512 const send: SendCommand = Effect.fn("PreviewManager.sendCommand")(
15021513 function* (method, commandParams, sessionId) {
15031514 yield* checkControl;
......@@ -1922,6 +1933,7 @@ const makeNativeOperations = Effect.fn("PreviewManager.makeOperations")(function
19221933 if (isPreviewInputSignal(rawSignal) && (yield* consumeExpectedAgentInput(tabId, rawSignal))) {
19231934 return;
19241935 }
1936 agentDrivenWebContents.delete(wc);
19251937 yield* Ref.update(controlEpochRef, (epochs) =>
19261938 replaceMap(epochs, (copy) => {
19271939 copy.set(tabId, (epochs.get(tabId) ?? 0) + 1);
......@@ -2476,6 +2488,9 @@ const makeNativeOperations = Effect.fn("PreviewManager.makeOperations")(function
24762488 );
24772489 return;
24782490 }
2491 // URL-bar navigation hands the page back to the human. The agent's own
2492 // navigation shares this path; its next action marks the page again.
2493 agentDrivenWebContents.delete(wc);
24792494 if (wc.getURL() === url) {
24802495 yield* attempt({ operation: "navigate.reload", tabId, webContentsId: wc.id }, () =>
24812496 wc.reload(),
......@@ -3500,6 +3515,24 @@ const makeNativeOperations = Effect.fn("PreviewManager.makeOperations")(function
35003515 });
35013516 };
35023517
3518 // Installed once per session. Electron opens a native Save dialog for a
3519 // download with no save path, so every export an agent clicked while testing
3520 // a page put a modal over the whole app. Downloads from a page the agent drove
3521 // last go to the artifact directory instead. Downloads the human starts keep
3522 // the Save dialog.
3523 const installDownloadHandler = (session: Session) => {
3524 if (downloadHandlerSessions.has(session)) return;
3525 downloadHandlerSessions.add(session);
3526 session.on("will-download", (_event, item, source) => {
3527 if (!agentDrivenWebContents.has(source)) return;
3528 // The start time keeps names unique across restarts; the count keeps two
3529 // same-name downloads in one millisecond from overwriting each other.
3530 const id = `${Math.round(item.getStartTime() * 1000).toString(36)}-${(downloadCount++).toString(36)}`;
3531 const fileName = `browser-download-${id}-${path.basename(item.getFilename())}`;
3532 item.setSavePath(path.join(resolvedArtifactDirectory, fileName));
3533 });
3534 };
3535
35033536 const startRecording = Effect.fn("PreviewManager.startRecording")(function* (
35043537 tabId: string,
35053538 options: RecordingInputOptions = DEFAULT_RECORDING_INPUT_OPTIONS,
......@@ -4624,6 +4657,7 @@ const makeNativeOperations = Effect.fn("PreviewManager.makeOperations")(function
46244657 goBack,
46254658 goForward,
46264659 hardReload,
4660 installDownloadHandler,
46274661 navigate,
46284662 openPictureInPicture,
46294663 openDevTools,
......@@ -5075,13 +5109,15 @@ export const make = Effect.gen(function* PreviewManagerMake() {
50755109 setMainWindow: operations.setMainWindow,
50765110 getBrowserSession: Effect.fn("PreviewManager.getBrowserSession")(
50775111 function* (scope, persistent, namespace) {
5078 return yield* browserSession
5112 const session = yield* browserSession
50795113 .getSession(scope, persistent, namespace)
50805114 .pipe(
50815115 Effect.mapError(
50825116 (cause) => new PreviewOperationError({ operation: "getBrowserSession", cause }),
50835117 ),
50845118 );
5119 operations.installDownloadHandler(session);
5120 return session;
50855121 },
50865122 ),
50875123 isBrowserPartition: browserSession.isPartition,