Skip to content

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

Merged
steipete merged 14 commits into
openclaw:mainfrom
ZengWen-DT:fix/96176-browser-act-target-id-alias
Jul 5, 2026
Merged

fix(browser): resolve act targetId aliases before mismatch check#96178
steipete merged 14 commits into
openclaw:mainfrom
ZengWen-DT:fix/96176-browser-act-target-id-alias

Conversation

@ZengWen-DT

@ZengWen-DT ZengWen-DT commented Jun 23, 2026

Copy link
Copy Markdown
Contributor

What Problem This Solves

Fixes #96176. After interacting with a tab, every browser act operation
(evaluate, click, wait, select, fill, drag, ...) could fail with:

403 action targetId must match request targetId (ACT_TARGET_ID_MISMATCH)

while snapshot, open, close, and tabs kept working — exactly the
symptom matrix in the report.

Why This Change Was Made

The /act route resolves the request tab from the caller-supplied targetId
via ensureTabAvailable, which accepts any documented alias form — tabId,
label, suggestedTargetId, the raw CDP id, or a unique id prefix
(browser-tool.schema.ts literally tells the model to prefer
suggestedTargetId/tabId/label). Resolution returns the canonical
tab.targetId.

Both targetId guards then re-checked the request with raw string equality:

  • top-level: action.targetId !== tab.targetId (agent.act.ts)
  • batched sub-actions: validateBatchTargetIds(actions, tab.targetId)

Whenever the caller used an alias (the recommended path), the raw alias never
equals the resolved canonical id, so the guard returned 403 even though the
request named the correct tab. snapshot/open/close/tabs have no such
guard, so they were unaffected — matching the reporter's table.

The fix resolves the action's targetId through the same tab alias resolution
the route already used, and rejects only ids that resolve to a different
tab. Malformed ids (e.g. a non-string that route selection drops but action
normalization stringifies) still correctly fail, since they resolve to no tab.

User Impact

browser act works again when callers reference a tab by suggestedTargetId,
tabId, label, or a unique id prefix — including the form the tool schema
recommends. No config or API surface changes; genuine cross-tab drift in
batched actions is still rejected.

Evidence

Two new contract tests in server.agent-contract-core.test.ts exercise the
real route + server:

  • top-level: act click with a unique prefix (abcd → canonical abcd1234)
    now returns 200 and dispatches to the canonical target.
  • batch: a sub-action referencing the request tab by prefix alias is accepted.

Before/after on the unmodified production code (prod fix stashed):

# BEFORE (prod fix reverted): new tests fail = bug reproduced
Tests  2 failed | 19 passed (21)
  FAIL > accepts a unique targetId prefix that resolves to the request tab
  FAIL > accepts a batched sub-action targetId alias for the request tab

# AFTER (prod fix applied): full file green, existing mismatch tests preserved
Tests  21 passed (21)

Run via node scripts/run-vitest.mjs extensions/browser/src/browser/server.agent-contract-core.test.ts.
The existing ACT_TARGET_ID_MISMATCH tests (non-string targetId, cross-tab
batch override) still pass unchanged.

Real browser proof (Chrome 148, real CDP)

Addressing the prior review's [P1]: the canonicalization landed in 49e8cfc,
and the production dispatch path is now verified against a real headless
Chrome over CDP
(two real tabs), driving the exact production functions the
review cited — getPageForTargetId (pw-session.ts, exact-id page lookup) and
executeActViaPlaywright / executeSingleAction (pw-tools-core.interactions.ts,
effectiveTargetId = action.targetId ?? targetId):

──────── REAL-CHROME PROOF ────────
chrome           : Chrome/148.0.7778.96
tabs open        : 2 (canonical TabA = 7E1BE2E0…33A2)
request alias    : "7e1be2e0" (unique 8-char prefix of TabA targetId)
alias resolves   : resolveTargetIdFromTabs -> 7E1BE2E0…33A2
BEFORE fix (raw alias dispatched): BrowserTabNotFoundError: tab not found
AFTER fix  (canonicalized targetId=7E1BE2E0…33A2):
   executeActViaPlaywright evaluate -> document.title = "PROOF_TAB_A" (TabA, exact)
───────────────────────────────────
  • BEFORE — an un-canonicalized prefix alias reaching the executor fails at
    the real Playwright exact-id lookup (BrowserTabNotFoundError, i.e. the
    reported runtime symptom). With ≥2 tabs the single-page best-effort fallback
    cannot mask the miss.
  • AFTERcanonicalizeActTargetIds rewrites the alias to the canonical id,
    so executeActViaPlaywright evaluates in the exact intended tab and reads
    back its title (PROOF_TAB_A, not the other tab).

This was run with the real production code via a throwaway live harness (not
committed); the alias forms tabId/label/suggestedTargetId are covered by
the canonicalizeActTargetIds unit tests.


AI-assisted.

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.
@clawsweeper

clawsweeper Bot commented Jun 24, 2026

Copy link
Copy Markdown
Contributor

Codex review: needs maintainer review before merge. Reviewed July 5, 2026, 4:06 PM ET / 20:06 UTC.

Summary
The PR updates the browser /act route to canonicalize targetId aliases for top-level and nested batch actions before Playwright dispatch, with route and unit regression tests.

PR surface: Source +7, Tests +101. Total +108 across 4 files.

Reproducibility: yes. Source inspection shows current main and v2026.6.11 resolve the route tab through alias-aware selection, then raw-compare action.targetId against the canonical tab id; I did not run tests in this read-only review.

Review metrics: none identified.

Root-cause cluster
Relationship: fixed_by_candidate
Canonical: #96176
Summary: This PR is the focused candidate fix for the linked browser /act target-id mismatch bug; one parallel PR is already closed as superseded and one older issue partially overlaps.

Members:

Proposal only: this assessment does not dispatch repair, suppress jobs, mutate sibling items, close, or merge anything.

Merge readiness
Overall: 🦞 diamond lobster
Proof: 🦞 diamond lobster
Patch quality: 🦞 diamond lobster
Result: ready for maintainer review.

Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch.

Next step before merge

  • No ClawSweeper repair is needed; the remaining action is maintainer merge handling after required exact-head checks complete.

Security
Cleared: No concrete security or supply-chain concern found; the diff changes browser route target-id normalization and tests only.

Review details

Best possible solution:

Land this focused browser /act alias canonicalization once required exact-head checks are green, then let the linked bug close through the merge.

Do we have a high-confidence way to reproduce the issue?

Yes. Source inspection shows current main and v2026.6.11 resolve the route tab through alias-aware selection, then raw-compare action.targetId against the canonical tab id; I did not run tests in this read-only review.

Is this the best way to solve the issue?

Yes. Canonicalizing aliases at the /act route before the Playwright exact-target handoff is narrower than teaching the executor every alias form and safer than weakening the cross-tab mismatch guard.

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning high; reviewed against a04b6ced4f96.

Label changes

Label justifications:

  • P1: The PR fixes a high-priority browser act workflow regression where documented stable tab aliases can make real browser actions fail.
  • rating: 🦞 diamond lobster: Overall readiness is 🦞 diamond lobster; proof is 🦞 diamond lobster and patch quality is 🦞 diamond lobster.
  • status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Sufficient (live_output): The PR body and follow-up discussion include after-fix real Chrome/CDP output and maintainer live Gateway proof showing aliases canonicalized to the intended tab and acted on successfully.
  • proof: sufficient: Contributor real behavior proof is sufficient. The PR body and follow-up discussion include after-fix real Chrome/CDP output and maintainer live Gateway proof showing aliases canonicalized to the intended tab and acted on successfully.
Evidence reviewed

PR surface:

Source +7, Tests +101. Total +108 across 4 files.

View PR surface stats
Area Files Added Removed Net
Source 2 32 25 +7
Tests 2 102 1 +101
Docs 0 0 0 0
Config 0 0 0 0
Generated 0 0 0 0
Other 0 0 0 0
Total 4 134 26 +108

What I checked:

Likely related people:

  • steipete: Assigned to the PR, provided maintainer verification/refactor notes, and authored stable tab-handle and recent browser route work relevant to this alias contract. (role: recent reviewer and browser area contributor; confidence: high; commits: 45e2a15e29ca, 82020bd78726, 41bc332b3833; files: extensions/browser/src/browser/target-id.ts, extensions/browser/src/browser/routes/agent.act.ts, extensions/browser/src/browser/routes/agent.act.normalize.ts)
  • joshavant: Authored the merged browser /act route consolidation PR that introduced the target-id mismatch validation surface now being repaired. (role: introduced behavior; confidence: high; commits: f096fc440686; files: extensions/browser/src/browser/routes/agent.act.ts, extensions/browser/src/browser/routes/agent.act.normalize.ts)
  • FMLS: Documented stable browser tab references, which are part of the alias contract this PR makes usable for /act. (role: contract/documentation contributor; confidence: medium; commits: 2e1b0becead1; files: extensions/browser/src/browser/target-id.test.ts, docs/cli/browser.md, docs/tools/browser-control.md)
What the crustacean ranks mean
  • 🦀 challenger crab: rare, exceptional readiness with strong proof, clean implementation, and convincing validation.
  • 🦞 diamond lobster: very strong readiness with only minor maintainer review expected.
  • 🐚 platinum hermit: good normal PR, likely mergeable with ordinary maintainer review.
  • 🦐 gold shrimp: useful signal, but proof or patch confidence is still limited.
  • 🦪 silver shellfish: thin signal; proof, validation, or implementation needs work.
  • 🧂 unranked krab: not merge-ready because proof is missing/unusable or there are serious correctness or safety concerns.
  • 🌊 off-meta tidepool: rating does not apply to this item.

Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics.

How this review workflow works
  • ClawSweeper keeps one durable marker-backed review comment per issue or PR.
  • Re-runs edit this comment so the latest verdict, findings, and automation markers stay together instead of adding duplicate bot comments.
  • A fresh review can be triggered by eligible @clawsweeper re-review comments, exact-item GitHub events, scheduled/background review runs, or manual workflow dispatch.
  • PR/issue authors and users with repository write access can comment @clawsweeper re-review or @clawsweeper re-run on an open PR or issue to request a fresh review only.
  • Maintainers can also comment @clawsweeper review to request a fresh review only.
  • Fresh-review commands do not start repair, autofix, rebase, CI repair, or automerge.
  • Maintainer-only repair and merge flows require explicit commands such as @clawsweeper autofix, @clawsweeper automerge, @clawsweeper fix ci, or @clawsweeper address review.
  • Maintainers can comment @clawsweeper explain to ask for more context, or @clawsweeper stop to stop active automation.
Review history (5 earlier review cycles)
  • reviewed 2026-07-01T06:10:15.453Z sha 53b0bd6 :: needs maintainer review before merge. :: none
  • reviewed 2026-07-05T18:33:49.422Z sha d865f61 :: needs changes before merge. :: [P2] Widen the absent-target test action before reading targetId
  • reviewed 2026-07-05T18:52:25.652Z sha b2bfc97 :: needs maintainer review before merge. :: none
  • reviewed 2026-07-05T19:23:38.029Z sha ed49e4d :: needs maintainer review before merge. :: none
  • reviewed 2026-07-05T19:52:31.550Z sha ca60ef6 :: needs maintainer review before merge. :: none

@clawsweeper clawsweeper Bot added rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. P1 High-priority user-facing bug, regression, or broken workflow. labels Jun 24, 2026
…patch

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.
@ZengWen-DT

Copy link
Copy Markdown
Contributor Author

The prior review (🧂 / "patch is incorrect") was run against the first commit bfaea104, before the canonicalization fix. The [P1] it raised — "the raw alias still reaches executeActViaPlaywright; executeSingleAction prefers action.targetId ?? targetId, so the real Playwright lookup uses the alias" — is now addressed in 49e8cfc:

  • canonicalizeActTargetIds rewrites every accepted alias (top-level and recursive batch sub-actions) to the resolved tab.targetId, and the route dispatches the canonicalized action to the Playwright executor.
  • The two new contract tests now assert the action handed to the executor carries the canonical id (action.targetId === "abcd1234"), exercising the exact action.targetId ?? targetId handoff the review flagged.

Real browser proof (the requested behavior proof)

Verified against a real headless Chrome (148) over CDP with two real tabs, driving the production functions the review cited — getPageForTargetId (exact-id page lookup) and executeActViaPlaywright/executeSingleAction:

──────── REAL-CHROME PROOF ────────
chrome           : Chrome/148.0.7778.96
tabs open        : 2 (canonical TabA = 7E1BE2E0…33A2)
request alias    : "7e1be2e0" (unique 8-char prefix of TabA targetId)
alias resolves   : resolveTargetIdFromTabs -> 7E1BE2E0…33A2
BEFORE fix (raw alias dispatched): BrowserTabNotFoundError: tab not found
AFTER fix  (canonicalized targetId=7E1BE2E0…33A2):
   executeActViaPlaywright evaluate -> document.title = "PROOF_TAB_A" (TabA, exact)
───────────────────────────────────

BEFORE: an un-canonicalized prefix alias fails the real Playwright exact-id lookup (BrowserTabNotFoundError, the reported runtime symptom; ≥2 tabs so the single-page fallback can't mask it). AFTER: canonicalization makes the evaluate land on the exact intended tab and read back its title. Full proof block is in the PR body's Evidence section.

@clawsweeper re-review

@clawsweeper

clawsweeper Bot commented Jun 24, 2026

Copy link
Copy Markdown
Contributor

🦞🧹
ClawSweeper re-review requested.

I asked ClawSweeper to review this item again.
Action: item re-review queued (workflow sweep.yml, event repository_dispatch).
Result: the existing ClawSweeper review comment will be edited in place when the review finishes.

@clawsweeper clawsweeper Bot added proof: sufficient ClawSweeper judged the real behavior proof convincing. rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. and removed rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. labels Jun 24, 2026
@clawsweeper clawsweeper Bot added rating: 🦞 diamond lobster Very strong PR readiness with only minor maintainer review expected. rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. and removed rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. rating: 🦞 diamond lobster Very strong PR readiness with only minor maintainer review expected. labels Jun 24, 2026
@clawsweeper clawsweeper Bot added rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. and removed rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. labels Jul 1, 2026
@steipete steipete self-assigned this Jul 5, 2026
@openclaw-barnacle openclaw-barnacle Bot added app: web-ui App: web-ui gateway Gateway runtime plugin: logbook Logbook automatic work journal plugin size: XL and removed size: S labels Jul 5, 2026
@github-actions github-actions Bot added the dependencies-changed PR changes dependency-related files label Jul 5, 2026
@github-actions

github-actions Bot commented Jul 5, 2026

Copy link
Copy Markdown
Contributor

Dependency graph guard cleared

This PR no longer has blocked dependency graph changes. A future dependency graph change requires a fresh /allow-dependencies-change comment after the guard blocks that new head SHA.

  • Current SHA: d1889c589b463547c20f8b922dfe21219e429e38

@openclaw-barnacle openclaw-barnacle Bot added size: S and removed docs Improvements or additions to documentation app: macos App: macos app: web-ui App: web-ui gateway Gateway runtime plugin: logbook Logbook automatic work journal plugin size: XL labels Jul 5, 2026
@github-actions github-actions Bot removed the dependencies-changed PR changes dependency-related files label Jul 5, 2026
@steipete

steipete commented Jul 5, 2026

Copy link
Copy Markdown
Contributor

Maintainer verification complete on ed49e4d3e3af502a2f25290459f514e4f086a1c2.

I refactored the original callback/result/copy implementation into route-local canonicalization, reduced the production change from +55 to +7 net lines, and added a regression for ambiguous nested target prefixes. The final route resolves documented tab aliases against the selected managed-browser tab, rejects aliases that are not globally unique, and leaves the Playwright executor's exact-target invariant intact.

Proof:

No known proof gaps remain.

@clawsweeper clawsweeper Bot added proof: sufficient ClawSweeper judged the real behavior proof convincing. rating: 🦞 diamond lobster Very strong PR readiness with only minor maintainer review expected. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. and removed proof: sufficient ClawSweeper judged the real behavior proof convincing. rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. labels Jul 5, 2026
@steipete
steipete merged commit d405cda into openclaw:main Jul 5, 2026
88 checks passed
@steipete

steipete commented Jul 5, 2026

Copy link
Copy Markdown
Contributor

Merged via squash.

github-actions Bot pushed a commit to Desicool/openclaw that referenced this pull request Jul 6, 2026
…nclaw#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 <[email protected]>
Co-authored-by: Peter Steinberger <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

P1 High-priority user-facing bug, regression, or broken workflow. proof: sufficient ClawSweeper judged the real behavior proof convincing. rating: 🦞 diamond lobster Very strong PR readiness with only minor maintainer review expected. size: S status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Browser act: "action targetId must match request targetId" after gateway restart (2026.6.9)

2 participants