From aee59f50d01ab54ba93d2742f1814ee5bda9efe5 Mon Sep 17 00:00:00 2001 From: yawbtng <154343001+yawbtng@users.noreply.github.com> Date: Thu, 2 Jul 2026 14:06:27 -0700 Subject: [PATCH] fix(cli): surface no-match failures in browse click/fill MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit browse click and browse fill returned { clicked: true } / { filled: true } and exited 0 even when the selector matched no element, because they run through act() — which reports a missing element via success:false rather than throwing. The sibling commands select/upload already error via deepLocator, so click/fill were inconsistent outliers that silently reported success on a no-op. Check the act() result and throw its message when success is false, so a no-match click/fill exits non-zero like the rest of the element commands. Adds unit coverage for both the failure (throws) and success paths, and that a failed fill never presses Enter. --- .changeset/fix-cli-click-fill-nomatch.md | 5 ++ .../cli/src/lib/driver/commands/elements.ts | 10 ++- .../tests/element-command-failures.test.ts | 79 +++++++++++++++++++ 3 files changed, 92 insertions(+), 2 deletions(-) create mode 100644 .changeset/fix-cli-click-fill-nomatch.md create mode 100644 packages/cli/tests/element-command-failures.test.ts diff --git a/.changeset/fix-cli-click-fill-nomatch.md b/.changeset/fix-cli-click-fill-nomatch.md new file mode 100644 index 000000000..728f6dfe4 --- /dev/null +++ b/.changeset/fix-cli-click-fill-nomatch.md @@ -0,0 +1,5 @@ +--- +"browse": patch +--- + +`browse click` and `browse fill` now exit non-zero and report the error when their selector matches no element, instead of printing `{ "clicked": true }` / `{ "filled": true }` and exiting 0. Both commands run through `act()`, which reports a missing element via `success:false` rather than throwing, so the failure was previously swallowed — unlike `select`/`upload`, which already error via `deepLocator`. This makes the false-positive success visible to scripts and agents. diff --git a/packages/cli/src/lib/driver/commands/elements.ts b/packages/cli/src/lib/driver/commands/elements.ts index cf5dc448c..b8632cb11 100644 --- a/packages/cli/src/lib/driver/commands/elements.ts +++ b/packages/cli/src/lib/driver/commands/elements.ts @@ -8,12 +8,17 @@ export const elementsHandlers: DriverCommandHandlers = { .object({ selector: z.string().min(1) }) .parse(params); const stagehand = await manager.stagehandInstance(); - await stagehand.act({ + // act() reports a failed action (e.g. a selector that matches nothing) by + // returning success:false rather than throwing, so surface it as an error + // instead of reporting { clicked: true } — matching select/upload, which + // throw via deepLocator when the element is missing. + const result = await stagehand.act({ arguments: [], description: "click element", method: "click", selector: manager.resolveSelector(selector), } as never); + if (!result.success) throw new Error(result.message); return { clicked: true }; }, @@ -26,12 +31,13 @@ export const elementsHandlers: DriverCommandHandlers = { }) .parse(params); const stagehand = await manager.stagehandInstance(); - await stagehand.act({ + const result = await stagehand.act({ arguments: [value], description: "fill element", method: "fill", selector: manager.resolveSelector(selector), } as never); + if (!result.success) throw new Error(result.message); if (pressEnter) { const page = await manager.activePage(); await page.keyPress("Enter"); diff --git a/packages/cli/tests/element-command-failures.test.ts b/packages/cli/tests/element-command-failures.test.ts new file mode 100644 index 000000000..7a9e44dca --- /dev/null +++ b/packages/cli/tests/element-command-failures.test.ts @@ -0,0 +1,79 @@ +import { describe, expect, it, vi } from "vitest"; + +import { elementsHandlers } from "../src/lib/driver/commands/elements.js"; + +/** + * Regression coverage: `browse click` / `browse fill` used to return + * { clicked: true } / { filled: true } (exit 0) even when the selector matched + * no element, because act() reports failure via success:false instead of + * throwing. The sibling commands (select/upload) already throw via deepLocator. + * These tests lock in that click/fill now surface the failure. + */ +type ClickManager = Parameters< + NonNullable<(typeof elementsHandlers)["click"]> +>[0]; + +function managerWithActResult(actResult: { + success: boolean; + message: string; +}) { + const act = vi.fn().mockResolvedValue(actResult); + const keyPress = vi.fn(); + const manager = { + stagehandInstance: async () => ({ act }), + resolveSelector: (selector: string) => selector, + activePage: async () => ({ keyPress }), + } as unknown as ClickManager; + return { manager, act, keyPress }; +} + +const NO_MATCH = { + success: false, + message: "Could not find an element for the given xPath(s): #missing", +}; + +describe("browse click/fill surface act failures instead of reporting success", () => { + it("click throws when the underlying act reports failure (no-match selector)", async () => { + const { manager } = managerWithActResult(NO_MATCH); + await expect( + elementsHandlers.click!(manager, { selector: "#missing" }), + ).rejects.toThrow("Could not find an element"); + }); + + it("click resolves { clicked: true } when the action succeeds", async () => { + const { manager } = managerWithActResult({ success: true, message: "" }); + await expect( + elementsHandlers.click!(manager, { selector: "#ok" }), + ).resolves.toEqual({ clicked: true }); + }); + + it("fill throws when the underlying act reports failure (no-match selector)", async () => { + const { manager } = managerWithActResult(NO_MATCH); + await expect( + elementsHandlers.fill!(manager, { selector: "#missing", value: "hi" }), + ).rejects.toThrow("Could not find an element"); + }); + + it("fill resolves and does not press Enter when the action succeeds", async () => { + const { manager, keyPress } = managerWithActResult({ + success: true, + message: "", + }); + await expect( + elementsHandlers.fill!(manager, { selector: "#ok", value: "hi" }), + ).resolves.toEqual({ filled: true, pressedEnter: false }); + expect(keyPress).not.toHaveBeenCalled(); + }); + + it("fill does not press Enter when the action failed", async () => { + const { manager, keyPress } = managerWithActResult(NO_MATCH); + await expect( + elementsHandlers.fill!(manager, { + selector: "#missing", + value: "hi", + pressEnter: true, + }), + ).rejects.toThrow("Could not find an element"); + expect(keyPress).not.toHaveBeenCalled(); + }); +});