fix(browser): resolve act targetId aliases before mismatch check (#96178)

* fix(browser): resolve act targetId aliases before mismatch check

The /act top-level and batch targetId guards compared the caller-supplied
targetId against the resolved canonical tab.targetId with raw string
equality. Any supported alias form (tabId, label, suggestedTargetId, or a
unique id prefix) resolves to a different canonical id, so act requests that
followed the documented 'prefer suggestedTargetId/tabId/label' guidance were
rejected with 403 ACT_TARGET_ID_MISMATCH even though they named the correct
tab. snapshot/open/close/tabs lack this guard and kept working, matching the
reported symptom matrix.

Resolve the action targetId through the same tab alias resolution the route
used and reject only ids that resolve to a different tab.

* fix(browser): canonicalize act targetId aliases before Playwright dispatch

The /act gate accepted tabId/label/suggested/prefix aliases of the request
tab but left them on action.targetId. The managed executor reads
action.targetId ?? targetId for an exact page lookup (executeSingleAction ->
getPageForTargetId), so an alias missed the lookup and broke the action at
runtime whenever more than one page was open (single-page masked it via the
pages.length===1 fallback). Canonicalize the action targetId (top-level and
nested batch sub-actions) to the resolved tab id before dispatch; reject ids
that resolve to a different tab. Replace the mock-masked contract assertions
with executor-action assertions plus direct canonicalizer unit tests.

* test(browser): brace act-targetId guard clauses for curly lint

* docs(changelog): note browser target alias fix

* docs(changelog): note browser target alias fix

* fix(browser): reject ambiguous batch target aliases

* test(browser): type targetless act fixture

* docs(changelog): move browser alias fix to unreleased

* chore: drop nonessential browser changelog entry

---------

Co-authored-by: Peter Steinberger <peter@steipete.me>
Co-authored-by: Peter Steinberger <steipete@gmail.com>

extensions/browser/src/browser/routes/agent.act.normalize.test.ts

@@ -1,7 +1,67 @@
 // Browser tests cover agent.act.normalize plugin behavior.
 import { describe, expect, it } from "vitest";
 import { MAX_SAFE_TIMEOUT_DELAY_MS } from "../timer-delay.js";
-import { normalizeActRequest } from "./agent.act.normalize.js";
+import { canonicalizeActTargetIds, normalizeActRequest } from "./agent.act.normalize.js";
+
+describe("canonicalizeActTargetIds", () => {
+  const canonical = "abcd1234";
+  const tab = { targetId: canonical, suggestedTargetId: "sg-1", tabId: "tab-7", label: "Inbox" };
+
+  it("rewrites every same-tab alias to the canonical targetId before dispatch", () => {
+    for (const alias of ["abcd", "tab-7", "Inbox", "sg-1", canonical]) {
+      const action = { kind: "click", ref: "1", targetId: alias } as const;
+      expect(canonicalizeActTargetIds(action, tab)).toBeNull();
+      expect(action.targetId).toBe(canonical);
+    }
+  });
+
+  it("canonicalizes batch sub-action aliases recursively", () => {
+    const action = {
+      kind: "batch",
+      targetId: "abcd",
+      actions: [
+        { kind: "click", ref: "1", targetId: "tab-7" },
+        { kind: "batch", actions: [{ kind: "resize", width: 2, height: 2, targetId: "Inbox" }] },
+      ],
+    } satisfies Parameters<typeof canonicalizeActTargetIds>[0];
+    expect(canonicalizeActTargetIds(action, tab, [tab])).toBeNull();
+    expect(action.targetId).toBe(canonical);
+    const [first, nested] = action.actions;
+    expect(first?.targetId).toBe(canonical);
+    if (nested?.kind !== "batch") {
+      throw new Error("expected nested batch");
+    }
+    expect(nested.actions[0]?.targetId).toBe(canonical);
+  });
+
+  it("leaves an absent targetId unset so dispatch falls back to the request tab", () => {
+    const action: Parameters<typeof canonicalizeActTargetIds>[0] = { kind: "click", ref: "1" };
+    expect(canonicalizeActTargetIds(action, tab)).toBeNull();
+    expect(action.targetId).toBeUndefined();
+  });
+
+  it("rejects ids that resolve to a different tab", () => {
+    expect(canonicalizeActTargetIds({ kind: "click", ref: "1", targetId: "zzzz9999" }, tab)).toBe(
+      "action targetId must match request targetId",
+    );
+    expect(
+      canonicalizeActTargetIds(
+        { kind: "batch", actions: [{ kind: "click", ref: "1", targetId: "zzzz9999" }] },
+        tab,
+      ),
+    ).toBe("batched action targetId must match request targetId");
+  });
+
+  it("rejects a batched targetId prefix that is ambiguous across tabs", () => {
+    expect(
+      canonicalizeActTargetIds(
+        { kind: "batch", actions: [{ kind: "click", ref: "1", targetId: "abcd" }] },
+        tab,
+        [tab, { targetId: "abcd9999" }],
+      ),
+    ).toBe("batched action targetId must match request targetId");
+  });
+});
 
 describe("normalizeActRequest numeric fields", () => {
   it("keeps structured numeric action options", () => {

extensions/browser/src/browser/routes/agent.act.normalize.ts

@@ -13,6 +13,7 @@ import {
 } from "../act-policy.js";
 import type { BrowserActRequest, BrowserFormField } from "../client-actions.types.js";
 import { normalizeBrowserFormField } from "../form-fields.js";
+import { resolveTargetIdFromTabs } from "../target-id.js";
 import {
   type ActKind,
   isActKind,
@@ -45,19 +46,28 @@ function countBatchActions(actions: BrowserActRequest[]): number {
   return count;
 }
 
-/** Validate that nested batch actions cannot drift to a different target tab. */
-export function validateBatchTargetIds(
-  actions: BrowserActRequest[],
-  targetId: string,
+/** Keep nested action overrides inside the route-selected tab. */
+export function canonicalizeActTargetIds(
+  action: BrowserActRequest,
+  tab: { targetId: string; suggestedTargetId?: string; tabId?: string; label?: string },
+  tabs = [tab],
+  batched = false,
 ): string | null {
-  for (const action of actions) {
-    if (action.targetId && action.targetId !== targetId) {
-      return "batched action targetId must match request targetId";
+  if (action.targetId) {
+    const resolved = resolveTargetIdFromTabs(action.targetId, batched ? tabs : [tab]);
+    if (!resolved.ok || resolved.targetId !== tab.targetId) {
+      return batched
+        ? "batched action targetId must match request targetId"
+        : "action targetId must match request targetId";
     }
-    if (action.kind === "batch") {
-      const nestedError = validateBatchTargetIds(action.actions, targetId);
-      if (nestedError) {
-        return nestedError;
+    // The Playwright executor treats action.targetId as an exact override.
+    action.targetId = tab.targetId;
+  }
+  if (action.kind === "batch") {
+    for (const subAction of action.actions) {
+      const error = canonicalizeActTargetIds(subAction, tab, tabs, true);
+      if (error) {
+        return error;
       }
     }
   }

extensions/browser/src/browser/routes/agent.act.ts

@@ -36,7 +36,7 @@ import {
   jsonActError,
 } from "./agent.act.errors.js";
 import { registerBrowserAgentActHookRoutes } from "./agent.act.hooks.js";
-import { normalizeActRequest, validateBatchTargetIds } from "./agent.act.normalize.js";
+import { canonicalizeActTargetIds, normalizeActRequest } from "./agent.act.normalize.js";
 import { type ActKind, isActKind } from "./agent.act.shared.js";
 import {
   readBody,
@@ -436,13 +436,16 @@ export function registerBrowserAgentActRoutes(
               ...extra,
             });
           };
-          if (action.targetId && action.targetId !== tab.targetId) {
-            return jsonActError(
-              res,
-              403,
-              ACT_ERROR_CODES.targetIdMismatch,
-              "action targetId must match request targetId",
-            );
+          // Nested batch aliases can differ from the request alias, so prefixes
+          // must stay unique across the full tab set before canonicalization.
+          const actionTabs =
+            action.kind === "batch" && !isExistingSession ? await profileCtx.listTabs() : [tab];
+          if (!actionTabs.some((candidate) => candidate.targetId === tab.targetId)) {
+            actionTabs.unshift(tab);
+          }
+          const targetIdError = canonicalizeActTargetIds(action, tab, actionTabs);
+          if (targetIdError) {
+            return jsonActError(res, 403, ACT_ERROR_CODES.targetIdMismatch, targetIdError);
           }
           const profileName = profileCtx.profile.name;
           if (isExistingSession) {
@@ -656,12 +659,6 @@ export function registerBrowserAgentActRoutes(
           if (!pw) {
             return;
           }
-          if (action.kind === "batch") {
-            const targetIdError = validateBatchTargetIds(action.actions, tab.targetId);
-            if (targetIdError) {
-              return jsonActError(res, 403, ACT_ERROR_CODES.targetIdMismatch, targetIdError);
-            }
-          }
           const result = await pw.executeActViaPlaywright({
             cdpUrl,
             action,

extensions/browser/src/browser/server.agent-contract-core.test.ts

@@ -196,6 +196,47 @@ describe("browser control server", () => {
     slowTimeoutMs,
   );
 
+  it(
+    "canonicalizes a unique targetId prefix to the request tab before dispatch",
+    async () => {
+      const base = await startServerAndBase();
+      // "abcd" is a unique prefix of the canonical targetId "abcd1234". The route
+      // must rewrite the action's targetId to the canonical id, because the
+      // Playwright executor reads `action.targetId ?? targetId` for an exact page
+      // lookup; a surviving alias would miss the lookup and break at runtime.
+      const response = await postJson<{ ok: boolean }>(`${base}/act`, {
+        kind: "click",
+        ref: "1",
+        targetId: "abcd",
+      });
+
+      expect(response.ok).toBe(true);
+      const execArgs = mockFirstArg(pwMocks.executeActViaPlaywright, 0, "executeAct");
+      const action = execArgs.action as { targetId?: string };
+      expect(action.targetId).toBe("abcd1234");
+    },
+    slowTimeoutMs,
+  );
+
+  it(
+    "canonicalizes a batched sub-action targetId alias before dispatch",
+    async () => {
+      const base = await startServerAndBase();
+      const response = await postJson<{ ok: boolean }>(`${base}/act`, {
+        kind: "batch",
+        targetId: "abcd1234",
+        // Sub-action references the same tab via a unique prefix alias.
+        actions: [{ kind: "click", ref: "1", targetId: "abcd" }],
+      });
+
+      expect(response.ok).toBe(true);
+      const execArgs = mockFirstArg(pwMocks.executeActViaPlaywright, 0, "executeAct");
+      const action = execArgs.action as { actions?: Array<{ targetId?: string }> };
+      expect(action.actions?.[0]?.targetId).toBe("abcd1234");
+    },
+    slowTimeoutMs,
+  );
+
   it(
     "returns the replacement targetId after an action-triggered target swap",
     async () => {