Skip to content

fix(browser): accept tab aliases in action target ids#99182

Closed
tayoun wants to merge 1 commit into
openclaw:mainfrom
tayoun:fix/browser-target-aliases
Closed

fix(browser): accept tab aliases in action target ids#99182
tayoun wants to merge 1 commit into
openclaw:mainfrom
tayoun:fix/browser-target-aliases

Conversation

@tayoun

@tayoun tayoun commented Jul 2, 2026

Copy link
Copy Markdown
Contributor

Summary

  • resolve friendly browser tab refs like t1 before validating top-level and nested action targetId fields
  • normalize accepted action target ids to the raw CDP target id before Playwright dispatch
  • keep true cross-tab mismatches rejected, including inside batch actions

Verification

  • node scripts/run-vitest.mjs extensions/browser/src/browser/server.agent-contract-core.test.ts

Real behavior proof

The browser /tabs response tells agents to use friendly refs such as t1, but /act previously compared nested action targetId values directly with the raw CDP target id. That made valid requests like targetId: "t1" fail with ACT_TARGET_ID_MISMATCH before dispatch. The new tests cover accepted friendly refs at the top level and inside batch actions, while existing mismatch tests continue to reject real cross-tab drift.

@openclaw-barnacle openclaw-barnacle Bot added size: S triage: needs-pr-context Candidate: external PR body lacks required problem context or evidence. labels Jul 2, 2026
@clawsweeper

clawsweeper Bot commented Jul 2, 2026

Copy link
Copy Markdown
Contributor

Thanks for the context here. I swept through the related work, and this is now duplicate or superseded.

Close this PR as superseded: its useful Browser /act target-id alias fix is already covered by the open, mergeable, proof-positive canonical PR at #96178.

Root-cause cluster
Relationship: superseded
Canonical: #96178
Summary: This PR and the canonical PR both address the Browser /act target-id alias mismatch; the canonical PR is older, open, mergeable, proof-positive, and linked to the same user bug report.

Members:

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

Canonical path: Use #96178 as the landing path for the Browser /act alias fix and close this parallel branch.

So I’m closing this here and keeping the remaining discussion on #96178.

Review details

Best possible solution:

Use #96178 as the landing path for the Browser /act alias fix and close this parallel branch.

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

Yes. Source inspection shows current main resolves the request tab through alias-aware selection, then rejects alias spellings with raw equality before dispatch; the canonical PR also provides real Chrome/CDP before-after proof.

Is this the best way to solve the issue?

No for this PR as the landing path. The same fix is already carried by an older open canonical PR with broader tests and real browser proof, so maintaining this parallel branch is not the best solution.

Security review:

Security review cleared: No concrete security or supply-chain concern was found; the diff changes only Browser route target-id normalization and tests.

AGENTS.md: found and applied where relevant.

What I checked:

Likely related people:

  • steipete: History for target-id.ts and Browser docs shows stable tab handles and preferred suggested target references were added in this area. (role: stable tab reference contributor; confidence: high; commits: 45e2a15e29ca, 82020bd78726, 29ddb9d926e5; files: extensions/browser/src/browser/target-id.ts, docs/cli/browser.md)
  • eleqtrizit: Browser /act route history includes the current-tab route guard work in the same target validation surface. (role: route guard contributor; confidence: medium; commits: 78f3985c6051, ef0dbcf49d85; files: extensions/browser/src/browser/routes/agent.act.ts)
  • ZengWen-DT: Authored the open proof-positive canonical PR for the same /act alias mismatch and has recent Browser docs history. (role: canonical fix author; confidence: medium; commits: 53b0bd625575, 2983edd5a2d8; files: extensions/browser/src/browser/routes/agent.act.ts, extensions/browser/src/browser/routes/agent.act.normalize.ts, docs/cli/browser.md)

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

@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 Jul 2, 2026
@knoal

knoal commented Jul 2, 2026

Copy link
Copy Markdown

Onyx_bot Review

Old behavior confirmed. Validation rejected any action.targetId differing from tab.targetId — even valid aliases like t1 that resolveTargetIdFromTabs would correctly resolve.

resolveTargetIdFromTabs confirmed correct. Checks exact matches then case-insensitive prefix match. Friendly aliases like t1 resolve correctly when unambiguous.

New functions confirmed sound. normalizeActionTargetIdsForSelectedTab recursively walks the action tree (top-level + batch), resolves each targetId via resolveTargetIdFromTabs, and normalizes mismatches to ACT_TARGET_ID_MISMATCH.

Test coverage note. Two new alias tests cover the happy path. A useful addition: test for ambiguous aliases (multiple tabs match prefix) to confirm the ambiguous rejection path — but not a blocker since existing mismatch tests cover rejection.

Assessment: Correct and ready. Narrow, well-structured fix with proper recursive tree walk for batch actions.

@tayoun

tayoun commented Jul 2, 2026

Copy link
Copy Markdown
Contributor Author

Closing as superseded per ClawSweeper: the same Browser /act target-id alias fix is already covered by the canonical PR #96178, which has sufficient proof and is ready for maintainer review.

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. rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. size: S status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. triage: needs-pr-context Candidate: external PR body lacks required problem context or evidence.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants