aboutsummaryrefslogtreecommitdiff
path: root/src
diff options
context:
space:
mode:
authoramankumarsingh77 <amankumarsingh7702@gmail.com>2026-07-28 19:55:53 +0530
committerAman Kumar <145339995+amankumarsingh77@users.noreply.github.com>2026-08-02 19:12:46 +0530
commitcfa1892e17e3696da004c71a4942d5b12186b12c (patch)
tree228b15fc7ada4193bdcabeaf913421a2958215ce /src
parent254de089ee468795bac175d99e09ddaec4ac1469 (diff)
fix(domains): one fill engine, so browser_fill stops lying about checkboxes
browser_fill and browser_fill_form carried separate in-page implementations that disagreed on the same element. The worst case was silent: browser_fill on a checkbox fired the HTMLInputElement value setter, which writes the submit value and leaves `checked` alone, then reported status ok — a no-op the agent had no reason to doubt. Both tools now share one engine, with browser_fill's documented <select> rejection kept as an explicit pre-check and per-tool focus behaviour kept as a parameter. Deliberate, user-approved behaviour changes: - browser_fill on a DISABLED input now returns an error instead of success. The old success was the bug; browser_fill_form already refused it. - browser_fill on a checkbox/radio now toggles `checked` instead of the value. - browser_fill_form on a contenteditable now uses execCommand("insertText"), so it drives ProseMirror/Slack-style editors as browser_fill already did. browser_fill keeps appending interactiveDiff and republishing refs; fill_form still does not. Report: docs/superpowers/reports/2026-07-26-bug-audit.md#f5 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Diffstat (limited to 'src')
-rw-r--r--src/domains/fill-engine.ts95
-rw-r--r--src/domains/form-batch.ts55
-rw-r--r--src/domains/form.ts115
3 files changed, 143 insertions, 122 deletions
diff --git a/src/domains/fill-engine.ts b/src/domains/fill-engine.ts
new file mode 100644
index 0000000..32102fe
--- /dev/null
+++ b/src/domains/fill-engine.ts
@@ -0,0 +1,95 @@
+/**
+ * The one in-page fill implementation, shared by browser_fill and
+ * browser_fill_form so the two can no longer disagree about the same element.
+ *
+ * `this` is the target element and `value` is the requested value. Two call-site
+ * differences are parameters rather than forks:
+ * rejectSelect — browser_fill refuses a <select> and points at
+ * browser_select_option; browser_fill_form fills it.
+ * focusFirst — browser_fill_form focuses the field first; browser_fill
+ * documents that it does not move focus.
+ *
+ * Returns a PageResult (see form.ts): { ok, reason?, kind?, value?, text?,
+ * checked?, changed?, options?, tag? }.
+ */
+
+export type FillOptions = {
+ readonly rejectSelect: boolean;
+ readonly focusFirst: boolean;
+};
+
+const ENGINE = `
+ const el = this;
+ if (!el || el.nodeType !== 1) return { ok: false, reason: "ref does not point to an element" };
+ const tagName = el.tagName || "";
+ const tag = tagName.toLowerCase();
+ const type = (el.type || "").toLowerCase();
+
+ if (__rejectSelect && tag === "select") {
+ return { ok: false, reason: "element is a <select>", kind: "select", tag: tagName };
+ }
+ if (el.disabled) return { ok: false, reason: "element is disabled", tag: tagName };
+
+ const fire = function (t) { el.dispatchEvent(new Event(t, { bubbles: true })); };
+ if (__focusFirst) { try { el.focus(); } catch (e) {} }
+
+ if (tag === "select") {
+ const want = String(value);
+ let matched = null;
+ for (let i = 0; i < el.options.length; i++) {
+ const o = el.options[i];
+ if (o.value === want || o.label === want || o.text === want) { matched = o; break; }
+ }
+ if (!matched) {
+ const opts = [];
+ for (let i = 0; i < el.options.length; i++) opts.push({ value: el.options[i].value, text: el.options[i].text });
+ return { ok: false, reason: "no matching option", kind: "select", options: opts, tag: tagName };
+ }
+ el.value = matched.value;
+ fire("input"); fire("change");
+ return { ok: true, kind: "select", value: el.value, text: matched.text, tag: tagName };
+ }
+
+ if (tag === "input" && (type === "checkbox" || type === "radio")) {
+ const want = value === true || value === "true" || value === "on" || value === 1;
+ let changed = false;
+ if (el.checked !== want) {
+ try { el.click(); } catch (e) {}
+ if (el.checked !== want) { el.checked = want; fire("input"); fire("change"); }
+ changed = true;
+ }
+ return { ok: true, kind: type, checked: el.checked, changed: changed, tag: tagName };
+ }
+
+ if (el.isContentEditable) {
+ try { el.focus(); } catch (e) {}
+ let done = false;
+ try {
+ const sel = window.getSelection();
+ if (sel) { sel.removeAllRanges(); const rng = document.createRange(); rng.selectNodeContents(el); sel.addRange(rng); }
+ done = document.execCommand("insertText", false, String(value));
+ } catch (e) { done = false; }
+ if (!done) {
+ el.textContent = String(value);
+ el.dispatchEvent(new InputEvent("input", { bubbles: true }));
+ }
+ return { ok: true, kind: "contenteditable", value: el.textContent || "", tag: tagName };
+ }
+
+ if (tag === "input" || tag === "textarea") {
+ const proto = tag === "textarea" ? window.HTMLTextAreaElement.prototype : window.HTMLInputElement.prototype;
+ const desc = Object.getOwnPropertyDescriptor(proto, "value");
+ if (desc && desc.set) desc.set.call(el, String(value));
+ else el.value = String(value);
+ fire("input"); fire("change");
+ return { ok: true, kind: el.type || tag, value: el.value, tag: tagName };
+ }
+
+ return { ok: false, reason: "element is not a fillable field (tag=" + tag + ")", tag: tagName };
+`;
+
+export const fillBody = (opts: FillOptions): string =>
+ `const __rejectSelect = ${opts.rejectSelect}; const __focusFirst = ${opts.focusFirst};\n${ENGINE}`;
+
+export const fillDeclaration = (opts: FillOptions): string =>
+ `function (value) { ${fillBody(opts)} }`;
diff --git a/src/domains/form-batch.ts b/src/domains/form-batch.ts
index c4b4df6..1d49cf6 100644
--- a/src/domains/form-batch.ts
+++ b/src/domains/form-batch.ts
@@ -1,61 +1,10 @@
import { Type } from "typebox";
import { type Result, err, ok } from "../util/result";
import { defineBrowserTool, type ToolErr, type ToolOk } from "../util/tool";
+import { fillDeclaration } from "./fill-engine";
import { detailsOf, resolveAndCall } from "./form";
-const FILL_FN = `
-function(value) {
- const el = this;
- if (!el || el.nodeType !== 1) return { ok: false, reason: "ref does not point to an element" };
- if (el.disabled) return { ok: false, reason: "element is disabled" };
- const tag = (el.tagName || "").toLowerCase();
- const type = (el.type || "").toLowerCase();
- try { el.focus(); } catch (e) {}
- const fire = function(t) { el.dispatchEvent(new Event(t, { bubbles: true })); };
-
- if (tag === "select") {
- const want = String(value);
- let matched = null;
- for (let i = 0; i < el.options.length; i++) {
- const o = el.options[i];
- if (o.value === want || o.label === want || o.text === want) { matched = o; break; }
- }
- if (!matched) {
- const opts = [];
- for (let i = 0; i < el.options.length; i++) opts.push({ value: el.options[i].value, text: el.options[i].text });
- return { ok: false, reason: "no matching option", kind: "select", options: opts };
- }
- el.value = matched.value;
- fire("input"); fire("change");
- return { ok: true, kind: "select", value: el.value, text: matched.text };
- }
-
- if (tag === "input" && (type === "checkbox" || type === "radio")) {
- const want = value === true || value === "true" || value === "on" || value === 1;
- if (el.checked !== want) {
- try { el.click(); } catch (e) {}
- if (el.checked !== want) { el.checked = want; fire("input"); fire("change"); }
- }
- return { ok: true, kind: type, checked: el.checked };
- }
-
- if (el.isContentEditable) {
- el.textContent = String(value);
- fire("input"); fire("change");
- return { ok: true, kind: "contenteditable", value: el.textContent };
- }
-
- if (tag === "input" || tag === "textarea") {
- const proto = tag === "textarea" ? window.HTMLTextAreaElement.prototype : window.HTMLInputElement.prototype;
- const desc = Object.getOwnPropertyDescriptor(proto, "value");
- if (desc && desc.set) desc.set.call(el, String(value));
- else el.value = String(value);
- fire("input"); fire("change");
- return { ok: true, kind: tag, value: el.value };
- }
-
- return { ok: false, reason: "element is not a fillable field (tag=" + tag + ")" };
-}`;
+const FILL_FN = fillDeclaration({ rejectSelect: false, focusFirst: true });
const FieldValue = Type.Union([Type.String(), Type.Boolean()], {
description: "Value to set. String for text inputs / textareas / selects / contenteditable; boolean for checkboxes & radios.",
diff --git a/src/domains/form.ts b/src/domains/form.ts
index 26268d9..d8cbf87 100644
--- a/src/domains/form.ts
+++ b/src/domains/form.ts
@@ -4,6 +4,7 @@ import { safeJs } from "../util/js-template";
import { type Result, err, ok } from "../util/result";
import { defineBrowserTool, type ToolErr, type ToolOk } from "../util/tool";
import { cdpCall } from "./cdp-call";
+import { fillBody } from "./fill-engine";
import { interactiveDiff, resolveRefToObjectId } from "./ref-resolve";
const FillArgs = Type.Object({
@@ -29,7 +30,13 @@ const FillArgs = Type.Object({
*/
const runOnElement = async (
client: BrowserClient,
- opts: { ref?: string | undefined; selector?: string | undefined; fnBody: string; arg: unknown },
+ opts: {
+ ref?: string | undefined;
+ selector?: string | undefined;
+ fnBody: string;
+ arg: unknown;
+ notFound?: unknown;
+ },
): Promise<Result<unknown, ToolErr>> => {
if (opts.ref !== undefined) {
const objectId = await resolveRefToObjectId(client, opts.ref);
@@ -52,7 +59,7 @@ const runOnElement = async (
const prelude = safeJs`
(() => {
const el = document.querySelector(${opts.selector});
- if (!el) return { status: "not_found" };
+ if (!el) return ${opts.notFound ?? { status: "not_found" }};
const __arg = ${opts.arg};
return (function (arg) {`;
const expr = `${prelude} ${opts.fnBody} }).call(el, __arg); })()`;
@@ -61,75 +68,21 @@ const runOnElement = async (
return ok(r.data);
};
-/**
- * Result shape returned by the in-page fill script. Discriminated by `status`
- * so the handler can map page-level outcomes to typed ToolErr kinds.
- */
-type FillResult =
- | { status: "ok"; tag: string; kind: string; value: string }
- | { status: "not_found" }
- | { status: "not_fillable"; tag: string };
+const FILL_BODY = `const value = arg;\n${fillBody({ rejectSelect: true, focusFirst: false })}`;
-const isFillResult = (v: unknown): v is FillResult => {
- if (typeof v !== "object" || v === null || !("status" in v)) return false;
- if (v.status === "not_found") return true;
- if (v.status === "not_fillable") return "tag" in v && typeof v.tag === "string";
- if (v.status === "ok") {
- return "tag" in v && typeof v.tag === "string"
- && "kind" in v && typeof v.kind === "string"
- && "value" in v && typeof v.value === "string";
- }
- return false;
-};
-
-// Element-scoped fill logic. `this` is the target element, `arg` is the value.
-// Write via the native prototype value setter so React's _valueTracker
-// registers the change — assigning el.value directly does NOT, which is why
-// controlled inputs revert on re-render. Shared verbatim by the ref path
-// (callFunctionOn) and the selector path (querySelector + .call).
-const FILL_FN = `
- const el = this;
- const tag = el.tagName;
- const proto =
- el instanceof HTMLTextAreaElement ? HTMLTextAreaElement.prototype
- : el instanceof HTMLInputElement ? HTMLInputElement.prototype
- : null;
- if (proto) {
- const desc = Object.getOwnPropertyDescriptor(proto, "value");
- const setter = desc && desc.set;
- if (setter) setter.call(el, arg);
- else el.value = arg;
- el.dispatchEvent(new Event("input", { bubbles: true }));
- el.dispatchEvent(new Event("change", { bubbles: true }));
- return { status: "ok", tag, kind: el.type || tag.toLowerCase(), value: el.value };
- }
- if (el.isContentEditable) {
- // Rich-text editors (Slack/Notion/ProseMirror) listen for beforeinput/input.
- el.focus();
- let done = false;
- try {
- const sel = window.getSelection();
- if (sel) { sel.removeAllRanges(); const rng = document.createRange(); rng.selectNodeContents(el); sel.addRange(rng); }
- done = document.execCommand("insertText", false, arg);
- } catch (e) { done = false; }
- if (!done) {
- el.textContent = arg;
- el.dispatchEvent(new InputEvent("input", { bubbles: true }));
- }
- return { status: "ok", tag, kind: "contenteditable", value: el.textContent || "" };
- }
- return { status: "not_fillable", tag };
-`;
+const NOT_FOUND: PageResult = { ok: false, reason: "not_found" };
export const fillTool = defineBrowserTool({
name: "browser_fill",
label: "Browser Fill",
description:
- "Fill a form field (input/textarea/contenteditable). PREFERRED: pass `ref` (e.g. 'e7') from browser_snapshot — survives re-renders. Fallback: a CSS `selector`. Writes through the native value setter and fires bubbling 'input'/'change' events, so React/Vue/Angular controlled components and rich-text editors update correctly — unlike browser_type. Returns the field's value after writing, plus a compact diff of page changes.",
+ "Fill a form field (input/textarea/contenteditable), or tick a checkbox/radio by passing 'true'/'false'. PREFERRED: pass `ref` (e.g. 'e7') from browser_snapshot — survives re-renders. Fallback: a CSS `selector`. Writes through the native value setter and fires bubbling 'input'/'change' events, so React/Vue/Angular controlled components and rich-text editors update correctly — unlike browser_type. Refuses a disabled field rather than writing a value the page will ignore. Returns the field's value after writing, plus a compact diff of page changes.",
promptSnippet: "Fill a form field by ref (preferred) or selector (works with React/Vue controlled inputs)",
promptGuidelines: [
"PREFER `ref` from browser_snapshot (the '[eN]' handle) over a guessed CSS selector — refs survive re-renders and don't require you to invent a selector.",
"Pass the desired value; no browser_click is needed first.",
+ "For a checkbox or radio pass 'true' or 'false' — it toggles `checked`, not the submit value. browser_set_checked does the same thing with a boolean.",
+ "A disabled field is refused: the error says so, rather than reporting a write the page never accepted.",
"A 'ref is stale' error means the page changed — re-run browser_snapshot to get fresh refs.",
"Use browser_type instead only for keystroke-sensitive widgets (autocomplete, masked/segmented inputs).",
"Fill sets the value but does not focus or submit. To submit a tag/autocomplete input, follow with browser_dispatch_key({ ref, key: 'Enter' }) — not browser_press_key, which targets the focused element and may miss this field.",
@@ -139,28 +92,50 @@ export const fillTool = defineBrowserTool({
parameters: FillArgs,
concurrency: "serialized",
async handler(args, { client }): Promise<Result<ToolOk, ToolErr>> {
- const r = await runOnElement(client, { ref: args.ref, selector: args.selector, fnBody: FILL_FN, arg: args.value });
+ const r = await runOnElement(client, {
+ ref: args.ref,
+ selector: args.selector,
+ fnBody: FILL_BODY,
+ arg: args.value,
+ notFound: NOT_FOUND,
+ });
if (!r.success) return r;
- const res = isFillResult(r.data) ? r.data : undefined;
+ const res = isPageResult(r.data) ? r.data : undefined;
const target = args.ref ?? args.selector ?? "";
- if (res === undefined || res.status === "not_found") {
+ if (res === undefined || res.reason === "not_found") {
return err({
kind: "invalid_state",
message: `No element matched: ${target}`,
details: { ref: args.ref, selector: args.selector },
});
}
- if (res.status === "not_fillable") {
+ if (!res.ok) {
+ const details = { ref: args.ref, selector: args.selector, ...(res.tag !== undefined ? { tag: res.tag } : {}) };
+ if (res.kind === "select") {
+ return err({
+ kind: "invalid_state",
+ message: `Element <select> is not fillable (not an input, textarea, or contenteditable): ${target}. For <select> use browser_select_option.`,
+ details,
+ });
+ }
return err({
kind: "invalid_state",
- message: `Element <${res.tag.toLowerCase()}> is not fillable (not an input, textarea, or contenteditable): ${target}. For <select> use browser_select_option.`,
- details: { ref: args.ref, selector: args.selector, tag: res.tag },
+ message: `Could not fill ${target}: ${res.reason ?? "unknown reason"}`,
+ details,
});
}
+ const shown = res.value !== undefined ? JSON.stringify(res.value) : `checked=${res.checked === true}`;
const diff = await interactiveDiff(client);
return ok({
- text: `Filled ${target} = ${JSON.stringify(res.value)}${diff}`,
- details: { ref: args.ref, selector: args.selector, value: args.value, verified: res.value, tag: res.tag, kind: res.kind },
+ text: `Filled ${target} = ${shown}${diff}`,
+ details: {
+ ref: args.ref,
+ selector: args.selector,
+ value: args.value,
+ verified: res.value ?? res.checked,
+ ...(res.tag !== undefined ? { tag: res.tag } : {}),
+ ...(res.kind !== undefined ? { kind: res.kind } : {}),
+ },
});
},
});
@@ -320,6 +295,7 @@ export type PageResult = {
readonly text?: string;
readonly checked?: boolean;
readonly changed?: boolean;
+ readonly tag?: string;
readonly options?: ReadonlyArray<{ value: string; text: string }>;
};
@@ -336,6 +312,7 @@ const isPageResult = (v: unknown): v is PageResult => {
if ("text" in v && typeof v.text !== "string") return false;
if ("checked" in v && typeof v.checked !== "boolean") return false;
if ("changed" in v && typeof v.changed !== "boolean") return false;
+ if ("tag" in v && typeof v.tag !== "string") return false;
if ("options" in v && !(Array.isArray(v.options) && v.options.every(isOption))) return false;
return true;
};