diff --git a/apps/extension/src/tools/__tests__/fill.test.ts b/apps/extension/src/tools/__tests__/fill.test.ts index e256c56..53adbf5 100644 --- a/apps/extension/src/tools/__tests__/fill.test.ts +++ b/apps/extension/src/tools/__tests__/fill.test.ts @@ -11,7 +11,6 @@ interface ScriptParams { async function setup(markup = '') { document.body.innerHTML = markup; const element = document.body.firstElementChild as HTMLInputElement; - vi.spyOn(document, "hasFocus").mockReturnValue(true); const manager = new SessionManager({ agentWindow: { create: async () => 100, @@ -67,6 +66,13 @@ async function setup(markup = '') { case "Input.insertText": await insert((params as { text: string }).text); return {}; + case "Input.dispatchKeyEvent": + if ((params as { commands?: string[] }).commands?.includes("moveToEndOfDocument")) { + if (typeof element.selectionStart === "number") { + element.setSelectionRange(element.value.length, element.value.length); + } + } + return {}; case "Runtime.releaseObject": return release(); default: @@ -106,7 +112,10 @@ describe("fill result verification", () => { "disabled", ])("preserves a %s field before rejecting it", async (attribute) => { const h = await setup(``); - expect(await h.fill()).toMatchObject({ code: "cdp_failed", message: /not editable/ }); + expect(await h.fill()).toMatchObject({ + code: "invalid_params", + data: { reason: "target_not_fillable" }, + }); expect(h.element.value).toBe("old"); expect(h.insert).not.toHaveBeenCalled(); expect(h.release).toHaveBeenCalledOnce(); @@ -121,26 +130,87 @@ describe("fill result verification", () => { ])("rejects an unsupported %s input before clearing", async (type) => { const h = await setup(``); const before = h.element.value; - expect(await h.fill()).toMatchObject({ code: "cdp_failed", message: /input type/ }); + expect(await h.fill()).toMatchObject({ + code: "invalid_params", + data: { reason: "target_not_fillable" }, + }); expect(h.element.value).toBe(before); expect(h.insert).not.toHaveBeenCalled(); }); it("rejects an overlong value before clearing", async () => { const h = await setup(''); - expect(await h.fill()).toMatchObject({ code: "cdp_failed", message: /maxlength/ }); + expect(await h.fill()).toMatchObject({ + code: "invalid_params", + message: /maxlength/, + data: { reason: "fill_value_invalid" }, + }); expect(h.element.value).toBe("old"); expect(h.insert).not.toHaveBeenCalled(); }); - it("does not type into a field that gained focus during clearing", async () => { + it.each([ + true, + false, + ])("classifies a DOM.focus rejection on a disabled=%s target", async (disabled) => { + const h = await setup(``); + const send = h.send.getMockImplementation()!; + h.send.mockImplementation(async (tabId, method, params) => { + if (method === "DOM.focus") throw new Error("Element is not focusable"); + return send(tabId, method, params); + }); + expect(await h.fill()).toMatchObject({ + code: disabled ? "invalid_params" : "cdp_failed", + data: { reason: disabled ? "target_not_fillable" : "fill_failed" }, + }); + expect(h.element.value).toBe("old"); + expect(h.insert).not.toHaveBeenCalled(); + expect(h.release).toHaveBeenCalledOnce(); + }); + + it("fills a background document without requiring foreground browser focus", async () => { + const h = await setup(); + vi.spyOn(document, "hasFocus").mockReturnValue(false); + expect(await h.fill()).toMatchObject({ value_length: 5 }); + expect(h.element.value).toBe("hello"); + }); + + it("restores focus once after clearing without typing into another field", async () => { const h = await setup(); const other = document.createElement("input"); document.body.append(other); h.element.addEventListener("input", () => other.focus()); - expect(await h.fill()).toMatchObject({ code: "cdp_failed", message: /lost focus/ }); + expect(await h.fill()).toMatchObject({ value_length: 5 }); + expect(h.element.value).toBe("hello"); + expect(other.value).toBe(""); + expect(h.send.mock.calls.filter(([, method]) => method === "DOM.focus")).toHaveLength(2); + }); + + it.each([ + "focus", + "value", + ])("rechecks %s after restoring focus, without retrying again", async (action) => { + const h = await setup(); + const other = document.createElement("input"); + document.body.append(other); + h.element.addEventListener( + "input", + () => { + other.focus(); + h.element.addEventListener("focus", () => { + if (action === "focus") queueMicrotask(() => other.focus()); + else h.element.value = "changed"; + }); + }, + { once: true }, + ); + expect(await h.fill()).toMatchObject({ + code: "cdp_failed", + data: { reason: action === "focus" ? "fill_focus_lost" : "fill_target_changed" }, + }); expect(other.value).toBe(""); expect(h.insert).not.toHaveBeenCalled(); + expect(h.send.mock.calls.filter(([, method]) => method === "DOM.focus")).toHaveLength(2); }); it.each([ @@ -159,14 +229,20 @@ describe("fill result verification", () => { h.element.value = "restored"; }); }); - expect(await h.fill()).toMatchObject({ code: "cdp_failed" }); + expect(await h.fill()).toMatchObject({ + code: "cdp_failed", + data: { reason: "fill_target_changed" }, + }); expect(h.insert).not.toHaveBeenCalled(); }); it("fails when insertText is acknowledged without changing the value", async () => { const h = await setup(); h.insert.mockImplementation(async () => {}); - expect(await h.fill()).toMatchObject({ code: "cdp_failed", message: /expected value/ }); + expect(await h.fill()).toMatchObject({ + code: "cdp_failed", + data: { reason: "fill_value_mismatch" }, + }); expect(h.release).toHaveBeenCalledOnce(); }); @@ -179,7 +255,10 @@ describe("fill result verification", () => { }, { once: true }, ); - expect(await h.fill()).toMatchObject({ code: "cdp_failed", message: /expected value/ }); + expect(await h.fill()).toMatchObject({ + code: "cdp_failed", + data: { reason: "fill_value_mismatch" }, + }); expect(h.element.value).toBe("hel"); }); @@ -212,34 +291,39 @@ describe("fill result verification", () => { it("does not report success for a target replaced by a change handler", async () => { const h = await setup(); h.element.addEventListener("change", () => h.element.replaceWith(h.element.cloneNode())); - expect(await h.fill()).toMatchObject({ code: "cdp_failed", message: /expected value/ }); + expect(await h.fill()).toMatchObject({ + code: "cdp_failed", + data: { reason: "fill_target_changed" }, + }); }); it.each([ - 1, 2, 3, 4, + 1, 2, 3, 4, 5, ])("rejects script exceptions at phase %s and releases the target", async (phase) => { const h = await setup(); + vi.spyOn(document, "hasFocus").mockReturnValue(false); const run = h.script.getMockImplementation()!; let count = 0; h.script.mockImplementation(async (params) => ++count === phase ? { exceptionDetails: { text: "page secret" } } : run(params), ); const result = await h.fill(); - expect(result).toMatchObject({ code: "cdp_failed" }); + expect(result).toMatchObject({ code: "cdp_failed", data: { reason: "fill_failed" } }); expect(JSON.stringify(result)).not.toContain("page secret"); - if (phase <= 2) expect(h.insert).not.toHaveBeenCalled(); + if (phase <= 3) expect(h.insert).not.toHaveBeenCalled(); expect(h.release).toHaveBeenCalledOnce(); }); - it.each([1, 2, 4])("rejects missing results at phase %s", async (phase) => { + it.each([1, 2, 3, 5])("rejects missing results at phase %s", async (phase) => { const h = await setup(); + vi.spyOn(document, "hasFocus").mockReturnValue(false); const run = h.script.getMockImplementation()!; let count = 0; h.script.mockImplementation(async (params) => ++count === phase ? { result: { type: "undefined" } } : run(params), ); expect(await h.fill()).toMatchObject({ code: "cdp_failed" }); - if (phase <= 2) expect(h.insert).not.toHaveBeenCalled(); + if (phase <= 3) expect(h.insert).not.toHaveBeenCalled(); }); it.each(["input", "change"])("checks state after nested microtasks from %s", async (event) => { @@ -270,7 +354,7 @@ describe("fill result verification", () => { const h = await setup(); h.element.focus(); h.element.setSelectionRange(3, 3); - expect(await h.fill("!", false)).toMatchObject({ value_length: 1 }); + expect(await h.fill("!", false)).toMatchObject({ value_length: 4 }); expect(h.element.value).toBe("old!"); }); @@ -314,18 +398,100 @@ describe("fill result verification", () => { expect(await h.fill("one\n")).toMatchObject({ value_length: 4 }); }); - it("rejects a non-append selection before typing", async () => { + it.each([ + [0, 0], + [1, 1], + [0, 3], + ])("appends after moving selection %s:%s to the end", async (start, end) => { const h = await setup(); h.element.focus(); - h.element.setSelectionRange(1, 1); - expect(await h.fill("!", false)).toMatchObject({ code: "cdp_failed" }); - expect(h.element.value).toBe("old"); + h.element.setSelectionRange(start, end); + expect(await h.fill("🙂", false)).toMatchObject({ value_length: 5 }); + expect(h.element.value).toBe("old🙂"); + }); + + it.each([ + "number", + "email", + ])("appends to %s without setting an unsupported selection API", async (type) => { + const h = await setup(``); + vi.spyOn(h.element, "selectionStart", "get").mockReturnValue(null); + vi.spyOn(h.element, "selectionEnd", "get").mockReturnValue(null); + const selection = vi.spyOn(h.element, "setSelectionRange").mockImplementation(() => { + throw new DOMException("unsupported", "InvalidStateError"); + }); + expect(await h.fill("3", false)).toMatchObject({ value_length: 3 }); + expect(h.element.value).toBe("123"); + expect(selection).not.toHaveBeenCalled(); + }); + + it("rejects invalid number text before clearing", async () => { + const h = await setup(''); + expect(await h.fill("abc")).toMatchObject({ + code: "invalid_params", + data: { reason: "fill_value_invalid" }, + }); + expect(h.element.value).toBe("12"); expect(h.insert).not.toHaveBeenCalled(); }); + it("reports page formatting as unconfirmed without retrying the write", async () => { + const h = await setup(); + h.element.addEventListener("change", () => { + h.element.value = h.element.value.toUpperCase(); + }); + expect(await h.fill()).toMatchObject({ + code: "cdp_failed", + data: { reason: "fill_value_mismatch" }, + }); + expect(h.element.value).toBe("HELLO"); + expect(h.insert).toHaveBeenCalledOnce(); + }); + + it.each([ + ["focus", false], + ["focus", true], + ["value", false], + ["value", true], + ["cancel", false], + ["cancel", true], + ["throw", false], + ["throw", true], + ] as const)("stops when positioning triggers %s (clear=%s) and releases the key", async (action, clearBefore) => { + const h = await setup(); + vi.spyOn(document, "hasFocus").mockReturnValue(false); + const controller = new AbortController(); + const other = document.createElement("input"); + document.body.append(other); + const send = h.send.getMockImplementation()!; + h.send.mockImplementation(async (tabId, method, params) => { + const reply = await send(tabId, method, params); + if ( + method === "Input.dispatchKeyEvent" && + (params as { type: string }).type === "rawKeyDown" + ) { + if (action === "focus") other.focus(); + if (action === "value") h.element.value = "changed"; + if (action === "cancel") controller.abort(); + if (action === "throw") throw new Error("dispatch failed"); + } + return reply; + }); + const result = await h.fill("!", clearBefore, controller.signal); + expect(result).toMatchObject({ code: action === "cancel" ? "cancelled" : "cdp_failed" }); + expect(h.insert).not.toHaveBeenCalled(); + expect(other.value).toBe(""); + expect(h.send).toHaveBeenCalledWith( + 4, + "Input.dispatchKeyEvent", + expect.objectContaining({ type: "keyUp" }), + ); + expect(h.release).toHaveBeenCalledOnce(); + }); + it.each([true, false])("verifies an empty request with clear_before=%s", async (clearBefore) => { const h = await setup(); - expect(await h.fill("", clearBefore)).toMatchObject({ value_length: 0 }); + expect(await h.fill("", clearBefore)).toMatchObject({ value_length: clearBefore ? 0 : 3 }); expect(h.element.value).toBe(clearBefore ? "" : "old"); expect(h.insert).not.toHaveBeenCalled(); }); diff --git a/apps/extension/src/tools/__tests__/interaction.test.ts b/apps/extension/src/tools/__tests__/interaction.test.ts index 7d552b2..7cb1d4d 100644 --- a/apps/extension/src/tools/__tests__/interaction.test.ts +++ b/apps/extension/src/tools/__tests__/interaction.test.ts @@ -541,13 +541,16 @@ describe("handleHover", () => { }); function successfulFillScript(params: unknown) { - const args = (params as { arguments?: Array<{ value: unknown }> }).arguments ?? []; + const script = params as { arguments?: Array<{ value: unknown }>; functionDeclaration: string }; + const args = script.arguments ?? []; return { result: { value: args.length === 2 - ? { before: "", expected: args[0].value, valueLength: String(args[0].value).length } - : true, + ? { before: "", expected: args[0].value } + : script.functionDeclaration.startsWith("function(expected)") + ? { connected: true, matches: true, valueLength: String(args[0].value).length } + : "ready", }, }; } @@ -613,7 +616,7 @@ describe("handleFill", () => { expect(res.used_ref).toBe("e1"); const insert = fake.sent.find((c) => c.method === "Input.insertText"); expect(insert?.params).toEqual({ text: "hello" }); - // Preparation, readiness, notifications, and final verification. + // Foreground replacement needs no extra caret-positioning round trip. const callFns = fake.sent.filter((c) => c.method === "Runtime.callFunctionOn"); expect(callFns).toHaveLength(4); }); @@ -634,6 +637,7 @@ describe("handleFill", () => { if (args.length === 2) expect(args[1].value).toBe(false); return successfulFillScript(p); }, + "Input.dispatchKeyEvent": () => ({}), "Runtime.releaseObject": () => ({}), "Input.insertText": () => ({}), }); diff --git a/apps/extension/src/tools/interaction.ts b/apps/extension/src/tools/interaction.ts index 5a48349..9e36c2d 100644 --- a/apps/extension/src/tools/interaction.ts +++ b/apps/extension/src/tools/interaction.ts @@ -609,11 +609,28 @@ type SelectMutationResult = // tool.fill // --------------------------------------------------------------------------- -interface FillPreparation { - before?: string; - expected?: string; - valueLength?: number; - error?: string; +type FillFailureReason = + | "target_not_fillable" + | "fill_value_invalid" + | "fill_target_changed" + | "fill_focus_lost" + | "fill_value_mismatch" + | "fill_failed"; + +type FillPreparation = + | { before: string; expected: string } + | { reason: FillFailureReason; error: string }; + +type FillReadiness = "ready" | "background" | "fill_target_changed" | "fill_focus_lost"; + +function fillError(reason: FillFailureReason, message: string): RpcError { + return rpcError( + reason === "target_not_fillable" || reason === "fill_value_invalid" + ? "invalid_params" + : "cdp_failed", + reason, + message, + ); } interface FillScriptReply { @@ -690,12 +707,8 @@ export async function handleFill( if (throwIfAborted(deps.signal)) { return { code: "cancelled", message: "fill aborted" }; } - await nodeCdp.send(target.tabId, "DOM.focus", { backendNodeId: node.backendNodeId }); } catch (err) { - return { - code: "cdp_failed", - message: err instanceof Error ? err.message : String(err), - }; + return fillError("fill_failed", err instanceof Error ? err.message : String(err)); } if (throwIfAborted(deps.signal)) { @@ -708,6 +721,31 @@ export async function handleFill( const clearBefore = params.clear_before ?? true; try { + if (throwIfAborted(deps.signal)) { + return { code: "cancelled", message: "fill aborted" }; + } + try { + await nodeCdp.send(target.tabId, "DOM.focus", { backendNodeId: node.backendNodeId }); + } catch (error) { + // Chrome rejects DOM.focus for disabled controls, including inherited + // fieldset state. Classify the live target before treating this as a + // browser failure; the normal path needs no extra round trip. + const editable = await nodeCdp.send>( + target.tabId, + "Runtime.callFunctionOn", + { objectId, functionDeclaration: FILL_EDITABLE_FUNCTION, returnByValue: true }, + ); + if (throwIfAborted(deps.signal)) { + return { code: "cancelled", message: "fill aborted" }; + } + if (!editable.exceptionDetails && editable.result?.value === false) { + return fillError( + "target_not_fillable", + "fill target is not editable or its input type is unsupported", + ); + } + throw error; + } if (throwIfAborted(deps.signal)) { return { code: "cancelled", message: "fill aborted" }; } @@ -721,14 +759,9 @@ export async function handleFill( functionDeclaration: `function(value, clearBefore) { const tag = this.tagName.toLowerCase(); const native = tag === 'input' || tag === 'textarea'; - if (!(${FILL_EDITABLE_FUNCTION}).call(this)) return { error: 'fill target is not editable or its input type is unsupported' }; - const focused = () => this.getRootNode().activeElement === this && this.ownerDocument.hasFocus(); - if (!focused()) return { error: 'fill target does not have focus' }; + if (!(${FILL_EDITABLE_FUNCTION}).call(this)) return { reason: 'target_not_fillable', error: 'fill target is not editable or its input type is unsupported' }; + if (this.getRootNode().activeElement !== this) return { reason: 'fill_focus_lost', error: 'fill target does not have focus' }; const before = clearBefore ? '' : (${FILL_VALUE_FUNCTION}).call(this); - if (!clearBefore && value !== '' && native && typeof this.selectionStart === 'number' && - (this.selectionStart !== before.length || this.selectionEnd !== before.length)) { - return { error: 'fill append requires the caret at the end of the field' }; - } let expected = before + value; if (native) { // Use the browser's own value sanitization (e.g. textarea @@ -742,11 +775,11 @@ export async function handleFill( if (tag === 'input') expected = expected.replace(/\\r\\n|\\r|\\n/g, ' '); normalizer.value = expected; if (expected !== '' && normalizer.value === '' && this.type === 'number') { - return { error: 'fill value is not valid for a number input' }; + return { reason: 'fill_value_invalid', error: 'fill value is not valid for a number input' }; } expected = normalizer.value; if (this.type !== 'number' && this.maxLength >= 0 && expected.length > this.maxLength) { - return { error: 'fill value exceeds the target maxlength' }; + return { reason: 'fill_value_invalid', error: 'fill value exceeds the target maxlength' }; } } else { expected = expected.replace(/\\r\\n?/g, '\\n'); @@ -760,7 +793,7 @@ export async function handleFill( } this.dispatchEvent(new Event('input', { bubbles: true })); } - return { before, expected, valueLength: Math.max(0, expected.length - before.length) }; + return { before, expected }; }`, arguments: [{ value: params.value }, { value: clearBefore }], returnByValue: true, @@ -770,44 +803,95 @@ export async function handleFill( return { code: "cancelled", message: "fill aborted" }; } if (prepared.exceptionDetails) { - return { code: "cdp_failed", message: "fill preparation script failed" }; + return fillError("fill_failed", "fill preparation script failed"); } const preparation = prepared.result?.value; - if ( - preparation?.error || - typeof preparation?.before !== "string" || - typeof preparation?.expected !== "string" || - typeof preparation.valueLength !== "number" - ) { - return { - code: "cdp_failed", - message: preparation?.error ?? "fill preparation returned an unexpected result", - }; + if (preparation && "error" in preparation) { + return fillError(preparation.reason, preparation.error); + } + if (typeof preparation?.before !== "string" || typeof preparation?.expected !== "string") { + return fillError("fill_failed", "fill preparation returned an unexpected result"); } // A separate call runs after the clearing event's microtasks drain. - // Confirm the target before sending input to the focused element. - const ready = await nodeCdp.send>( - target.tabId, - "Runtime.callFunctionOn", - { - objectId, - functionDeclaration: `function(before) { - return (${FILL_EDITABLE_FUNCTION}).call(this) && - this.getRootNode().activeElement === this && this.ownerDocument.hasFocus() && - (${FILL_VALUE_FUNCTION}).call(this) === before; - }`, - arguments: [{ value: preparation.before }], - returnByValue: true, - }, - ); + // DOM focus works in background tabs. Use document.hasFocus() only to + // detect deferred focus events, never to reject background input. + const checkReady = async (): Promise => { + const reply = await nodeCdp.send>( + target.tabId, + "Runtime.callFunctionOn", + { + objectId, + functionDeclaration: `function(before) { + if (!(${FILL_EDITABLE_FUNCTION}).call(this) || + (${FILL_VALUE_FUNCTION}).call(this) !== before) return 'fill_target_changed'; + if (this.getRootNode().activeElement !== this) return 'fill_focus_lost'; + return this.ownerDocument.hasFocus() ? 'ready' : 'background'; + }`, + arguments: [{ value: preparation.before }], + returnByValue: true, + }, + ); + const state = reply.result?.value; + if ( + reply.exceptionDetails || + (state !== "ready" && + state !== "background" && + state !== "fill_target_changed" && + state !== "fill_focus_lost") + ) { + return fillError( + "fill_failed", + "fill readiness script failed or returned an unexpected result", + ); + } + return state; + }; + let ready = await checkReady(); if (throwIfAborted(deps.signal)) { return { code: "cancelled", message: "fill aborted" }; } - if (ready.exceptionDetails || ready.result?.value !== true) { - return { - code: "cdp_failed", - message: "fill target changed, lost focus, or could not be checked before typing", - }; + // Restore focus once only if clearing left this same target editable + // and unchanged. Recheck after focus handlers have run. + if (clearBefore && ready === "fill_focus_lost") { + await nodeCdp.send(target.tabId, "DOM.focus", { backendNodeId: node.backendNodeId }); + if (throwIfAborted(deps.signal)) { + return { code: "cancelled", message: "fill aborted" }; + } + ready = await checkReady(); + } + if (throwIfAborted(deps.signal)) { + return { code: "cancelled", message: "fill aborted" }; + } + if (typeof ready !== "string") return ready; + if (ready !== "ready" && ready !== "background") { + return fillError(ready, "fill target changed or lost focus before typing"); + } + if (params.value !== "" && (!clearBefore || ready === "background")) { + // Native editing commands also support number/email inputs, whose + // selection APIs cannot set a caret, and multiline contenteditables. + // Also do this for background replacement: CDP input focuses the + // renderer, delivering deferred focus events before it inserts text. + // Use a command-only event so this does not invoke an End shortcut. + try { + await deps.cdp.send(target.tabId, "Input.dispatchKeyEvent", { + type: "rawKeyDown", + commands: ["moveToEndOfDocument"], + }); + } finally { + await deps.cdp.send(target.tabId, "Input.dispatchKeyEvent", { type: "keyUp" }); + } + if (throwIfAborted(deps.signal)) { + return { code: "cancelled", message: "fill aborted" }; + } + // Focus and key handlers can change the target or move focus too. + ready = await checkReady(); + if (throwIfAborted(deps.signal)) { + return { code: "cancelled", message: "fill aborted" }; + } + if (typeof ready !== "string") return ready; + if (ready !== "ready" && ready !== "background") { + return fillError(ready, "fill target changed or lost focus while positioning the caret"); + } } // CDP `Input.insertText` handles IME / multi-byte input out of the // box, much more reliably than per-key `dispatchKeyEvent`. @@ -835,42 +919,51 @@ export async function handleFill( return { code: "cancelled", message: "fill aborted" }; } if (notified.exceptionDetails) { - return { code: "cdp_failed", message: "fill notification script failed" }; + return fillError("fill_failed", "fill notification script failed"); } - const verified = await nodeCdp.send>( - target.tabId, - "Runtime.callFunctionOn", - { - objectId, - functionDeclaration: `function(expected) { - return this.isConnected && (${FILL_VALUE_FUNCTION}).call(this) === expected; + const verified = await nodeCdp.send< + FillScriptReply<{ connected: boolean; matches: boolean; valueLength: number }> + >(target.tabId, "Runtime.callFunctionOn", { + objectId, + functionDeclaration: `function(expected) { + const value = (${FILL_VALUE_FUNCTION}).call(this); + return { connected: this.isConnected, matches: value === expected, valueLength: value.length }; }`, - arguments: [{ value: preparation.expected }], - returnByValue: true, - }, - ); + arguments: [{ value: preparation.expected }], + returnByValue: true, + }); if (throwIfAborted(deps.signal)) { return { code: "cancelled", message: "fill aborted" }; } - if (verified.exceptionDetails || verified.result?.value !== true) { - return { - code: "cdp_failed", - message: verified.exceptionDetails - ? "fill verification script failed" - : "fill could not verify the expected value; observe the page before retrying", - }; + const verification = verified.result?.value; + if ( + verified.exceptionDetails || + typeof verification?.connected !== "boolean" || + typeof verification?.matches !== "boolean" || + typeof verification?.valueLength !== "number" + ) { + return fillError( + "fill_failed", + "fill verification script failed or returned an unexpected result", + ); + } + if (!verification.connected) { + return fillError("fill_target_changed", "fill target was removed or replaced during input"); + } + if (!verification.matches) { + return fillError( + "fill_value_mismatch", + "fill could not verify the expected value; observe the page before deciding whether to retry", + ); } return attachDialogs(deps.cdp, target.tabId, dialogCursor, { tab_id: target.tabId, used_ref: node.usedRef, used_selector: node.usedSelector, - value_length: preparation.valueLength, + value_length: verification.valueLength, }); } catch (err) { - return { - code: "cdp_failed", - message: err instanceof Error ? err.message : String(err), - }; + return fillError("fill_failed", err instanceof Error ? err.message : String(err)); } finally { await nodeCdp.send(target.tabId, "Runtime.releaseObject", { objectId }).catch(() => undefined); } diff --git a/apps/extension/src/transport/types.ts b/apps/extension/src/transport/types.ts index fc9968a..f604993 100644 --- a/apps/extension/src/transport/types.ts +++ b/apps/extension/src/transport/types.ts @@ -26,6 +26,11 @@ export type RpcErrorReason = | "ref_not_found" | "selector_not_found" | "target_not_fillable" + | "fill_value_invalid" + | "fill_target_changed" + | "fill_focus_lost" + | "fill_value_mismatch" + | "fill_failed" | "target_not_select" | "option_not_found" | "single_select_value_count" diff --git a/crates/bsk-cli/skill/SKILL.md b/crates/bsk-cli/skill/SKILL.md index 357625c..f27ab2f 100644 --- a/crates/bsk-cli/skill/SKILL.md +++ b/crates/bsk-cli/skill/SKILL.md @@ -172,6 +172,10 @@ intended. Read `bsk upload --help` and `bsk download --help` for all flags and e - Stale ref: observe again and retry the intended action once. - Unknown tab or session: list current tabs/sessions; never guess identifiers. - Timeout: inspect current page state before deciding whether one longer purposeful wait is useful. +- Fill result unconfirmed (`fill_value_mismatch`): observe the field first; the page may have + formatted the value. Continue if the visible result satisfies the user's intent. Otherwise correct + the remaining difference; do not blindly repeat fill or immediately request human help. For other + fill errors, follow the returned hint and inspect current state before retrying. - Unsupported command: continue with available capabilities; suggest updating only when the missing command is necessary. - Unrecoverable failure: report the blocker and stop the session in a finally-style path. diff --git a/crates/bsk-cli/src/cli/render_error.rs b/crates/bsk-cli/src/cli/render_error.rs index 01f80dd..ef9312f 100644 --- a/crates/bsk-cli/src/cli/render_error.rs +++ b/crates/bsk-cli/src/cli/render_error.rs @@ -43,6 +43,11 @@ pub mod reason { pub const REF_NOT_FOUND: &str = "ref_not_found"; pub const SELECTOR_NOT_FOUND: &str = "selector_not_found"; pub const TARGET_NOT_FILLABLE: &str = "target_not_fillable"; + pub const FILL_VALUE_INVALID: &str = "fill_value_invalid"; + pub const FILL_TARGET_CHANGED: &str = "fill_target_changed"; + pub const FILL_FOCUS_LOST: &str = "fill_focus_lost"; + pub const FILL_VALUE_MISMATCH: &str = "fill_value_mismatch"; + pub const FILL_FAILED: &str = "fill_failed"; pub const TARGET_NOT_SELECT: &str = "target_not_select"; pub const OPTION_NOT_FOUND: &str = "option_not_found"; pub const SINGLE_SELECT_VALUE_COUNT: &str = "single_select_value_count"; @@ -247,7 +252,42 @@ pub fn info_for_error(code: ErrorCode, data: Option<&serde_json::Value>) -> Rend (ErrorCode::InvalidParams, reason::TARGET_NOT_FILLABLE) => RenderInfo { summary: "target element is not fillable", hint: Some( - "choose an input, textarea, or contenteditable element from the latest snapshot", + "observe the page and choose an enabled, editable text input, textarea, or contenteditable; use the appropriate interaction for other control types", + ), + exit_code: base.exit_code, + }, + (ErrorCode::InvalidParams, reason::FILL_VALUE_INVALID) => RenderInfo { + summary: "the requested text does not fit the field constraints", + hint: Some( + "check the field's input type and maxlength; use a compatible value without silently truncating or changing the user's intended data", + ), + exit_code: base.exit_code, + }, + (ErrorCode::CdpFailed, reason::FILL_TARGET_CHANGED) => RenderInfo { + summary: "the fill target changed during the action", + hint: Some( + "observe the current page and field value; if the intended result is missing, use a fresh editable target before retrying", + ), + exit_code: base.exit_code, + }, + (ErrorCode::CdpFailed, reason::FILL_FOCUS_LOST) => RenderInfo { + summary: "the page moved focus away from the fill target", + hint: Some( + "observe the page for a dialog or rerender and resolve it before retrying; fill manages field focus and does not require bringing the browser to the foreground", + ), + exit_code: base.exit_code, + }, + (ErrorCode::CdpFailed, reason::FILL_VALUE_MISMATCH) => RenderInfo { + summary: "the fill result could not be confirmed", + hint: Some( + "observe the field before retrying; the page may have formatted the value. Continue if the visible result satisfies the user's intent; otherwise correct the remaining difference. Do not blindly repeat fill or immediately request human help", + ), + exit_code: base.exit_code, + }, + (ErrorCode::CdpFailed, reason::FILL_FAILED) => RenderInfo { + summary: "fill encountered a browser or page-script error", + hint: Some( + "observe the page before retrying because the field may already have changed; retry only if the intended result is missing and the target is still available", ), exit_code: base.exit_code, }, @@ -442,6 +482,56 @@ mod tests { ErrorCode::NoBrowserConnected, ]; + #[test] + fn fill_reasons_provide_recovery_without_changing_exit_codes() { + for (code, reason, hint_fragment) in [ + ( + ErrorCode::InvalidParams, + reason::TARGET_NOT_FILLABLE, + "enabled, editable", + ), + ( + ErrorCode::InvalidParams, + reason::FILL_VALUE_INVALID, + "maxlength", + ), + ( + ErrorCode::CdpFailed, + reason::FILL_TARGET_CHANGED, + "fresh editable target", + ), + ( + ErrorCode::CdpFailed, + reason::FILL_FOCUS_LOST, + "does not require bringing the browser", + ), + ( + ErrorCode::CdpFailed, + reason::FILL_VALUE_MISMATCH, + "Continue if the visible result satisfies", + ), + ( + ErrorCode::CdpFailed, + reason::FILL_FAILED, + "may already have changed", + ), + ] { + let data = serde_json::json!({ "reason": reason }); + let info = info_for_error(code, Some(&data)); + assert!(info.hint.unwrap().contains(hint_fragment), "{reason}"); + assert_eq!(info.exit_code, info_for(code).exit_code); + } + let mismatch = serde_json::json!({ "reason": reason::FILL_VALUE_MISMATCH }); + let info = info_for_error(ErrorCode::CdpFailed, Some(&mismatch)); + assert_eq!(info.summary, "the fill result could not be confirmed"); + assert!(info.hint.unwrap().contains("Do not blindly repeat fill")); + let unknown = serde_json::json!({ "reason": "future_fill_reason" }); + assert_eq!( + info_for_error(ErrorCode::CdpFailed, Some(&unknown)), + info_for(ErrorCode::CdpFailed) + ); + } + #[test] fn element_not_visible_overrides_permission_denied_copy() { let data = serde_json::json!({ "reason": reason::ELEMENT_NOT_VISIBLE }); diff --git a/packages/dsh-plugin-browserskill/skill/SKILL.md b/packages/dsh-plugin-browserskill/skill/SKILL.md index a2d9c4e..464a24e 100644 --- a/packages/dsh-plugin-browserskill/skill/SKILL.md +++ b/packages/dsh-plugin-browserskill/skill/SKILL.md @@ -97,6 +97,10 @@ Continue from returned sequence cursors instead of rereading the same buffer. - Unknown tab: list tabs instead of guessing. - Unknown session: list owned sessions or start one; never try foreign ids. - Timeout: inspect current state before deciding whether one longer purposeful wait is useful. +- Fill result unconfirmed: observe the field first; the page may have formatted the value. Continue + if the visible result satisfies the user's intent. Otherwise correct the remaining difference; + do not blindly repeat fill or immediately request human help. For other fill errors, follow the + returned hint and inspect current state before retrying. - Unrecoverable failure: report the blocker and stop the owned session. Arbitrary page-script evaluation and interaction recording are intentionally unsupported. Do not diff --git a/packages/dsh-plugin-browserskill/tests/runner.test.ts b/packages/dsh-plugin-browserskill/tests/runner.test.ts index 37b2de1..ad6d81a 100644 --- a/packages/dsh-plugin-browserskill/tests/runner.test.ts +++ b/packages/dsh-plugin-browserskill/tests/runner.test.ts @@ -124,6 +124,28 @@ describe("parseBskJson", () => { expect(() => parseBskJson({ ...base, code: 1, stderr: "boom" }, "x")).toThrow(/boom/); }); + it("surfaces fill recovery guidance to the model without retrying", async () => { + const hint = + "observe the field before retrying; the page may have formatted the value. Continue if the visible result satisfies the user's intent"; + const reply = { + ...base, + code: 3, + stdout: JSON.stringify({ + code: "cdp_failed", + message: "fill could not verify the expected value", + data: { reason: "fill_value_mismatch" }, + hint, + }), + }; + let calls = 0; + const result = await runWithSessionBusyRetry(async () => { + calls++; + return reply; + }); + expect(calls).toBe(1); + expect(() => parseBskJson(result, "fill")).toThrow(hint); + }); + it("reports killed-by-interrupt children (null exit code) as interrupted", () => { expect(() => parseBskJson({ ...base, code: null }, "navigate")).toThrow(/interrupted/); }); diff --git a/skill/SKILL.md b/skill/SKILL.md index 357625c..f27ab2f 100644 --- a/skill/SKILL.md +++ b/skill/SKILL.md @@ -172,6 +172,10 @@ intended. Read `bsk upload --help` and `bsk download --help` for all flags and e - Stale ref: observe again and retry the intended action once. - Unknown tab or session: list current tabs/sessions; never guess identifiers. - Timeout: inspect current page state before deciding whether one longer purposeful wait is useful. +- Fill result unconfirmed (`fill_value_mismatch`): observe the field first; the page may have + formatted the value. Continue if the visible result satisfies the user's intent. Otherwise correct + the remaining difference; do not blindly repeat fill or immediately request human help. For other + fill errors, follow the returned hint and inspect current state before retrying. - Unsupported command: continue with available capabilities; suggest updating only when the missing command is necessary. - Unrecoverable failure: report the blocker and stop the session in a finally-style path.