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(); + }); +});