From 7f65621367c40a8aa5d12028430d484cabe7958c Mon Sep 17 00:00:00 2001 From: Sam F <43347795+monadoid@users.noreply.github.com> Date: Sat, 25 Jul 2026 15:53:10 +0200 Subject: [PATCH 1/5] Restore Action inputs to act --- packages/protocol/pending-schemas.ts | 22 ------ packages/protocol/schemas.ts | 2 +- packages/protocol/stagehand.v4.json | 71 ++++++++++--------- packages/protocol/types.ts | 4 -- .../src/stagehand/_generated/models.py | 6 +- .../sdk-python/src/stagehand/stagehand.py | 4 +- packages/sdk-python/tests/test_stagehand.py | 8 ++- packages/sdk-ts/src/index.ts | 2 + packages/sdk-ts/src/stagehand.ts | 4 +- packages/sdk-ts/tests/object-wrapper.test.ts | 44 ++++++++++++ packages/server/services/actService.ts | 25 +++++-- packages/server/tests/act.test.ts | 61 ++++++++++++++++ 12 files changed, 184 insertions(+), 69 deletions(-) diff --git a/packages/protocol/pending-schemas.ts b/packages/protocol/pending-schemas.ts index c509c1b40..ddbd8d54b 100644 --- a/packages/protocol/pending-schemas.ts +++ b/packages/protocol/pending-schemas.ts @@ -1,8 +1,5 @@ import { z } from "zod/v4"; import { - ActOptionsSchema, - ActResultSchema, - ActionSchema, AzureEntraIdAuthSchema, AzureModelProviderOptionsSchema, BrowserbaseSessionCreateParamsSchema, @@ -430,25 +427,6 @@ export const SessionEndResponseSchema = z .strict() .meta({ id: "SessionEndResponse" }); -export const ActRequestSchema = z - .object({ - input: z.string().or(ActionSchema).meta({ - description: "Natural language instruction or Action object", - example: "Click the login button", - }), - options: ActOptionsSchema, - frameId: z.string().nullish().meta({ - description: "Target frame ID for the action", - }), - streamResponse: z.boolean().optional().meta({ - description: "Whether to stream the response via SSE", - example: true, - }), - }) - .meta({ id: "ActRequest" }); - -export const ActResponseSchema = wrapResponse(ActResultSchema, "ActResponse"); - export const ExtractRequestSchema = z .object({ instruction: z.string().optional().meta({ diff --git a/packages/protocol/schemas.ts b/packages/protocol/schemas.ts index d553b8aac..41c73bbec 100644 --- a/packages/protocol/schemas.ts +++ b/packages/protocol/schemas.ts @@ -1343,7 +1343,7 @@ export const RuntimeConfigureParamsSchema = z export const StagehandActParamsSchema = z .object({ pageId: z.string().min(1), - input: z.string().min(1), + input: z.union([z.string().min(1), ActionSchema]), options: ActOptionsSchema, }) .strict() diff --git a/packages/protocol/stagehand.v4.json b/packages/protocol/stagehand.v4.json index 32294180d..d8bd146c7 100644 --- a/packages/protocol/stagehand.v4.json +++ b/packages/protocol/stagehand.v4.json @@ -1615,8 +1615,15 @@ "minLength": 1 }, "input": { - "type": "string", - "minLength": 1 + "anyOf": [ + { + "type": "string", + "minLength": 1 + }, + { + "$ref": "#/$defs/Action" + } + ] }, "options": { "$ref": "#/$defs/ActOptions" @@ -1625,6 +1632,36 @@ "required": ["page_id", "input"], "additionalProperties": false }, + "Action": { + "type": "object", + "properties": { + "selector": { + "type": "string", + "description": "CSS selector or XPath for the element", + "example": "[data-testid='submit-button']" + }, + "description": { + "type": "string", + "description": "Human-readable description of the action", + "example": "Click the submit button" + }, + "method": { + "description": "The method to execute (click, fill, etc.)", + "example": "click", + "type": "string" + }, + "arguments": { + "description": "Arguments to pass to the method", + "example": ["Hello World"], + "type": "array", + "items": { + "type": "string" + } + } + }, + "required": ["selector", "description"], + "description": "Action object returned by observe and used by act" + }, "ActOptions": { "type": "object", "properties": { @@ -1764,36 +1801,6 @@ }, "required": ["success", "message", "action_description", "actions"] }, - "Action": { - "type": "object", - "properties": { - "selector": { - "type": "string", - "description": "CSS selector or XPath for the element", - "example": "[data-testid='submit-button']" - }, - "description": { - "type": "string", - "description": "Human-readable description of the action", - "example": "Click the submit button" - }, - "method": { - "description": "The method to execute (click, fill, etc.)", - "example": "click", - "type": "string" - }, - "arguments": { - "description": "Arguments to pass to the method", - "example": ["Hello World"], - "type": "array", - "items": { - "type": "string" - } - } - }, - "required": ["selector", "description"], - "description": "Action object returned by observe and used by act" - }, "CacheStatus": { "type": "string", "enum": ["HIT", "MISS"] diff --git a/packages/protocol/types.ts b/packages/protocol/types.ts index 5f22bd7d1..1b8a4fcc9 100644 --- a/packages/protocol/types.ts +++ b/packages/protocol/types.ts @@ -189,8 +189,6 @@ import type { VertexProviderOptionsSchema, } from "./schemas.js"; import type { - ActRequestSchema, - ActResponseSchema, AISDKApiKeyProviderSchema, AnthropicClientOptionsSchema, ApiKeyAuthSchema, @@ -460,8 +458,6 @@ export type VertexResolvedProviderClientOptions = z.infer< typeof VertexResolvedProviderClientOptionsSchema >; -export type ActRequest = z.infer; -export type ActResponse = z.infer; export type BrowserConfig = z.infer; export type BrowserbaseBrowserSettings = z.infer; export type BrowserbaseContext = z.infer; diff --git a/packages/sdk-python/src/stagehand/_generated/models.py b/packages/sdk-python/src/stagehand/_generated/models.py index 14ff92a52..06f6b6b8f 100644 --- a/packages/sdk-python/src/stagehand/_generated/models.py +++ b/packages/sdk-python/src/stagehand/_generated/models.py @@ -701,6 +701,10 @@ class ImplementationInfo(WireModel): version: Annotated[StrictStr, Field(min_length=1)] +class Input(RootModel[StrictStr]): + root: Annotated[StrictStr, Field(min_length=1)] + + class JSONRPCErrorObject(WireModel): model_config = ConfigDict( extra="forbid", @@ -1881,7 +1885,7 @@ class StagehandActParams(WireModel): validate_by_name=True, ) page_id: Annotated[StrictStr, Field(min_length=1)] - input: Annotated[StrictStr, Field(min_length=1)] + input: Union[Input, Action] options: Optional[ActOptions] = None diff --git a/packages/sdk-python/src/stagehand/stagehand.py b/packages/sdk-python/src/stagehand/stagehand.py index 80a23615b..7784870e8 100644 --- a/packages/sdk-python/src/stagehand/stagehand.py +++ b/packages/sdk-python/src/stagehand/stagehand.py @@ -464,7 +464,7 @@ async def generate(params: LLMGenerateParams) -> LLMGenerateResult: async def act( self, - input: str, + input: str | Action, *, page: Page | None = None, model: ModelConfig | None = None, @@ -487,7 +487,7 @@ async def act( target_page = page or await self.context.active_page() if target_page is None: raise RuntimeError("Stagehand has no active page") - params = StagehandActParams(page_id=target_page.page_id, input=input) + params = StagehandActParams.model_validate({"page_id": target_page.page_id, "input": input}) if options.model_fields_set: params.options = options result = await self._connected_rpc_client.send("stagehand.act", params, ActResult) diff --git a/packages/sdk-python/tests/test_stagehand.py b/packages/sdk-python/tests/test_stagehand.py index be9649b59..f210c96b2 100644 --- a/packages/sdk-python/tests/test_stagehand.py +++ b/packages/sdk-python/tests/test_stagehand.py @@ -281,6 +281,7 @@ async def connect(**_: object) -> RPCClient: cache=CacheOptions(threshold=1), ) actions = await stagehand.observe(instruction="Find the link", model=model, locator=locator) + replay_result = await stagehand.act(actions[0], page=page) page_info = await stagehand.extract( instruction="Extract the heading", schema=PageInfo, @@ -291,12 +292,14 @@ async def connect(**_: object) -> RPCClient: assert action_result == act_result assert actions == [action] + assert replay_result == act_result assert page_info == PageInfo(heading="Example Domain") assert [call[0] for call in recording.calls] == [ "stagehand.init", "stagehand.act", "context.active_page", "stagehand.observe", + "stagehand.act", "stagehand.extract", ] act_params = recording.calls[1][1] @@ -315,7 +318,10 @@ async def connect(**_: object) -> RPCClient: assert observe_params.options is not None assert observe_params.options.model == model assert observe_params.options.locator == locator - extract_params = recording.calls[4][1] + replay_params = recording.calls[4][1] + assert isinstance(replay_params, StagehandActParams) + assert replay_params.model_dump(by_alias=True)["input"] == action.model_dump(by_alias=True) + extract_params = recording.calls[5][1] assert isinstance(extract_params, StagehandExtractParams) assert extract_params.page_id == "explicit-page" assert extract_params.options is not None diff --git a/packages/sdk-ts/src/index.ts b/packages/sdk-ts/src/index.ts index e80fcd7a9..e3bcb5c3e 100644 --- a/packages/sdk-ts/src/index.ts +++ b/packages/sdk-ts/src/index.ts @@ -15,6 +15,8 @@ export { Page, type ScreenshotOptions } from "./page.js"; export type { InitScriptSource } from "./pageScripts.js"; export { Stagehand } from "./stagehand.js"; export type { + Action, + ActResultData, BrowserGetVersionResult, RuntimeLoopbackStatusResult, StagehandMetrics, diff --git a/packages/sdk-ts/src/stagehand.ts b/packages/sdk-ts/src/stagehand.ts index 2fa6cbb65..29864d496 100644 --- a/packages/sdk-ts/src/stagehand.ts +++ b/packages/sdk-ts/src/stagehand.ts @@ -140,7 +140,9 @@ export class Stagehand { this.closePromise = undefined; } - async act(input: string, options?: StagehandClientActOptions): Promise { + async act(instruction: string, options?: StagehandClientActOptions): Promise; + async act(action: Action, options?: StagehandClientActOptions): Promise; + async act(input: string | Action, options?: StagehandClientActOptions): Promise { const { page, ...protocolOptions } = StagehandClientActOptionsSchema.parse(options ?? {}); const targetPage = page ?? (await this.context.activePage()); if (!targetPage) throw new Error("Stagehand has no active page."); diff --git a/packages/sdk-ts/tests/object-wrapper.test.ts b/packages/sdk-ts/tests/object-wrapper.test.ts index 912c0cc50..3925dc675 100644 --- a/packages/sdk-ts/tests/object-wrapper.test.ts +++ b/packages/sdk-ts/tests/object-wrapper.test.ts @@ -750,6 +750,50 @@ describe("Stagehand TS object wrapper", () => { ]); }); + it("passes an Action returned by observe back to act unchanged", async () => { + const client = new FakeProtocolClient(); + const observedAction = { + selector: "xpath=/html/body/button", + description: "Submit button", + method: "click", + arguments: [], + }; + client.queueResponse(StagehandMethods.stagehandObserve, { + result: [observedAction], + }); + client.queueResponse(StagehandMethods.stagehandAct, { + result: { + success: true, + message: "Clicked the submit button", + actionDescription: "Submit button", + actions: [observedAction], + }, + }); + const stagehand = createStagehandWithClientForTest(client); + await stagehand.init(); + const page = new Page(client, { pageId: "page-1" }); + + const actions = await stagehand.observe("Find the submit button", { page }); + await expect(stagehand.act(actions[0]!, { page })).resolves.toMatchObject({ + success: true, + actions: [observedAction], + }); + + expect(client.calls).toStrictEqual([ + stagehandInitCall, + requestCall(StagehandMethods.stagehandObserve, { + pageId: "page-1", + instruction: "Find the submit button", + options: {}, + }), + requestCall(StagehandMethods.stagehandAct, { + pageId: "page-1", + input: observedAction, + options: {}, + }), + ]); + }); + it("routes stagehand.observe with an explicit page and options", async () => { const client = new FakeProtocolClient(); client.queueResponse(StagehandMethods.stagehandObserve, { diff --git a/packages/server/services/actService.ts b/packages/server/services/actService.ts index 9f806a68d..a01371922 100644 --- a/packages/server/services/actService.ts +++ b/packages/server/services/actService.ts @@ -77,6 +77,17 @@ export async function act({ }; ensureTimeRemaining(); + if (typeof input !== "string") { + return actResult( + await takeDeterministicAction({ + action: input, + variables, + context, + }), + ); + } + + const instruction = input; await waitForDomNetworkQuiet(page.mainFrame(), logger, domSettleTimeoutMs); ensureTimeRemaining(); @@ -87,7 +98,7 @@ export async function act({ caching: options?.cache, context: cache, logger, - onHit: (value) => replayCachedActions(value, input, variables, context), + onHit: (value) => replayCachedActions(value, instruction, variables, context), execute: async () => { const result = await runActPipeline(); return { @@ -103,11 +114,15 @@ export async function act({ async function runActPipeline(): Promise { const { combinedTree, combinedXpathMap } = await page.captureSnapshot({}); - const instruction = buildActPrompt(input, Object.values(SupportedUnderstudyAction), variables); + const actPrompt = buildActPrompt( + instruction, + Object.values(SupportedUnderstudyAction), + variables, + ); ensureTimeRemaining(); const firstInference = await getActionFromLLM({ - instruction, + instruction: actPrompt, domElements: combinedTree, xpathMap: combinedXpathMap, context, @@ -120,7 +135,7 @@ export async function act({ return actResult({ success: false, message: "Failed to perform act: No action found", - actionDescription: input, + actionDescription: instruction, actions: [], }); } @@ -142,7 +157,7 @@ export async function act({ ); const changedTree = diffCombinedTrees(combinedTree, nextTree); const secondInstruction = buildStepTwoPrompt( - input, + instruction, describeAction(firstInference.action), Object.values(SupportedUnderstudyAction).filter( ( diff --git a/packages/server/tests/act.test.ts b/packages/server/tests/act.test.ts index 677f31724..bb22902d2 100644 --- a/packages/server/tests/act.test.ts +++ b/packages/server/tests/act.test.ts @@ -157,6 +157,67 @@ describe("act service", () => { }); }); + it("self-heals a supplied Action after deterministic replay fails", async () => { + const frame = {}; + const captureSnapshot = vi.fn(async () => snapshot("0-20", "/html/body/button[2]")); + const page = actPage(frame, captureSnapshot); + const clientLLMGenerate = vi.fn( + async (): Promise => + actGeneration({ + elementId: "0-20", + description: "Submit button", + method: "click", + arguments: [], + }), + ); + performAction.mockRejectedValueOnce(new Error("Element detached")).mockResolvedValueOnce(); + + const result = await actService.act({ + params: { + pageId: "page-1", + input: { + selector: "xpath=/html/body/button[1]", + description: "Submit button", + method: "click", + arguments: [], + }, + }, + page, + model: { source: "client" }, + clientLLMGenerate, + logger: testLogger(), + selfHeal: true, + }); + + expect(waitForQuiet).not.toHaveBeenCalled(); + expect(captureSnapshot).toHaveBeenCalledOnce(); + expect(clientLLMGenerate).toHaveBeenCalledOnce(); + expect(performAction).toHaveBeenNthCalledWith( + 1, + page, + frame, + "click", + "xpath=/html/body/button[1]", + [], + expect.any(StagehandLogger), + undefined, + ); + expect(performAction).toHaveBeenNthCalledWith( + 2, + page, + frame, + "click", + "xpath=/html/body/button[2]", + [], + expect.any(StagehandLogger), + undefined, + ); + expect(result.result).toMatchObject({ + success: true, + actions: [{ selector: "xpath=/html/body/button[2]" }], + }); + }); + it("preserves two-step action behavior", async () => { const frame = {}; const captureSnapshot = vi From 23929cd8dd03202b7da1908f18b7616ff31b71f4 Mon Sep 17 00:00:00 2001 From: Vishal Anton Date: Mon, 27 Jul 2026 01:16:06 +0530 Subject: [PATCH 2/5] Add ActionTarget schema and validation for observed actions --- packages/protocol/schemas.ts | 23 +++ packages/protocol/stagehand.v4.json | 35 ++++ .../protocol/object-model-protocol.test.ts | 30 ++++ packages/protocol/types.ts | 2 + packages/sdk-python/src/stagehand/__init__.py | 2 + .../src/stagehand/_generated/models.py | 20 +++ packages/sdk-python/tests/test_stagehand.py | 16 +- packages/sdk-ts/src/index.ts | 1 + packages/sdk-ts/tests/object-wrapper.test.ts | 2 + packages/server/actionTarget.ts | 47 +++++ .../handlers/handlerUtils/actHandlerUtils.ts | 22 ++- packages/server/services/actService.ts | 56 ++++-- packages/server/services/cacheService.ts | 25 +++ packages/server/services/observeService.ts | 61 ++++++- packages/server/tests/act.test.ts | 72 +++++++- .../tests/action-target-validation.test.ts | 168 ++++++++++++++++++ packages/server/tests/cache-actions.test.ts | 50 ++++++ .../server/tests/locator-target-guard.test.ts | 59 ++++++ packages/server/tests/observe.test.ts | 66 +++++++ packages/server/understudy/locator.ts | 46 ++++- 20 files changed, 771 insertions(+), 32 deletions(-) create mode 100644 packages/server/actionTarget.ts create mode 100644 packages/server/tests/action-target-validation.test.ts create mode 100644 packages/server/tests/cache-actions.test.ts create mode 100644 packages/server/tests/locator-target-guard.test.ts diff --git a/packages/protocol/schemas.ts b/packages/protocol/schemas.ts index 41c73bbec..e1ae8e334 100644 --- a/packages/protocol/schemas.ts +++ b/packages/protocol/schemas.ts @@ -978,6 +978,21 @@ export const BrowserbaseBrowserSourceSchema = BrowserbaseSessionCreateParamsSche .meta({ id: "BrowserbaseBrowserSource" }); /** Action object returned by observe and used by act */ +export const ActionTargetSchema = z + .object({ + frameOrdinal: z.number().int().nonnegative().meta({ + description: "Stable frame ordinal assigned for the lifetime of the page", + }), + backendNodeId: z.number().int().positive().meta({ + description: "CDP backend node ID captured when the action was observed", + }), + }) + .strict() + .meta({ + id: "ActionTarget", + description: "Short-lived identity of the DOM node selected by an observed action", + }); + export const ActionSchema = z .object({ selector: z.string().meta({ @@ -999,6 +1014,14 @@ export const ActionSchema = z description: "Arguments to pass to the method", example: ["Hello World"], }), + target: ActionTargetSchema.optional().meta({ + description: + "Optional observed target identity. When present, act validates it before mutating the page.", + }), + argumentTargets: z.record(z.string().regex(/^\d+$/), ActionTargetSchema).optional().meta({ + description: + "Optional observed identities for selector arguments, keyed by their argument index.", + }), }) .meta({ id: "Action", diff --git a/packages/protocol/stagehand.v4.json b/packages/protocol/stagehand.v4.json index d8bd146c7..1967e1553 100644 --- a/packages/protocol/stagehand.v4.json +++ b/packages/protocol/stagehand.v4.json @@ -1657,11 +1657,46 @@ "items": { "type": "string" } + }, + "target": { + "description": "Optional observed target identity. When present, act validates it before mutating the page.", + "$ref": "#/$defs/ActionTarget" + }, + "argument_targets": { + "description": "Optional observed identities for selector arguments, keyed by their argument index.", + "type": "object", + "propertyNames": { + "type": "string", + "pattern": "^\\d+$" + }, + "additionalProperties": { + "$ref": "#/$defs/ActionTarget" + } } }, "required": ["selector", "description"], "description": "Action object returned by observe and used by act" }, + "ActionTarget": { + "type": "object", + "properties": { + "frame_ordinal": { + "type": "integer", + "minimum": 0, + "maximum": 9007199254740991, + "description": "Stable frame ordinal assigned for the lifetime of the page" + }, + "backend_node_id": { + "type": "integer", + "exclusiveMinimum": 0, + "maximum": 9007199254740991, + "description": "CDP backend node ID captured when the action was observed" + } + }, + "required": ["frame_ordinal", "backend_node_id"], + "additionalProperties": false, + "description": "Short-lived identity of the DOM node selected by an observed action" + }, "ActOptions": { "type": "object", "properties": { diff --git a/packages/protocol/tests/protocol/object-model-protocol.test.ts b/packages/protocol/tests/protocol/object-model-protocol.test.ts index 52e44be8e..a8ad6bc9d 100644 --- a/packages/protocol/tests/protocol/object-model-protocol.test.ts +++ b/packages/protocol/tests/protocol/object-model-protocol.test.ts @@ -134,6 +134,36 @@ describe("Stagehand object-model protocol", () => { }); }); + it("accepts observed Action targets and rejects invalid identities", () => { + const action = { + selector: "xpath=/html/body/button", + description: "Submit button", + method: "click", + arguments: [], + target: { frameOrdinal: 0, backendNodeId: 12 }, + argumentTargets: { "0": { frameOrdinal: 0, backendNodeId: 20 } }, + }; + + expect( + StagehandMethods.stagehandAct.params.parse({ pageId: "target-1", input: action }), + ).toStrictEqual({ pageId: "target-1", input: action }); + expect(() => + StagehandMethods.stagehandAct.params.parse({ + pageId: "target-1", + input: { ...action, target: { frameOrdinal: -1, backendNodeId: 0 } }, + }), + ).toThrow(); + expect(() => + StagehandMethods.stagehandAct.params.parse({ + pageId: "target-1", + input: { + ...action, + argumentTargets: { destination: { frameOrdinal: 0, backendNodeId: 20 } }, + }, + }), + ).toThrow(); + }); + it("defines extraction with a page, instruction, JSON Schema, and optional call settings", () => { const params = StagehandMethods.stagehandExtract.params.parse({ pageId: "target-1", diff --git a/packages/protocol/types.ts b/packages/protocol/types.ts index 1b8a4fcc9..cdaad5585 100644 --- a/packages/protocol/types.ts +++ b/packages/protocol/types.ts @@ -7,6 +7,7 @@ import type { } from "./schema-registry.js"; import type { ActionSchema, + ActionTargetSchema, ActOptionsSchema, ActResultDataSchema, ActResultSchema, @@ -299,6 +300,7 @@ export type LLMGenerateParams = z.infer; export type LLMGenerateResult = z.infer; export type ClientModelReference = z.infer; export type Action = z.infer; +export type ActionTarget = z.infer; export type ActOptions = z.infer; export type ActResultData = z.infer; export type ActResult = z.infer; diff --git a/packages/sdk-python/src/stagehand/__init__.py b/packages/sdk-python/src/stagehand/__init__.py index 8a33047e8..fc2382c59 100644 --- a/packages/sdk-python/src/stagehand/__init__.py +++ b/packages/sdk-python/src/stagehand/__init__.py @@ -2,6 +2,7 @@ from ._generated.models import ( Action, + ActionTarget, ActResultData, Animations, BrowserbaseBrowserSettings, @@ -55,6 +56,7 @@ __all__ = [ "ActResultData", "Action", + "ActionTarget", "Animations", "BrowserGetVersionResult", "BrowserClipboard", diff --git a/packages/sdk-python/src/stagehand/_generated/models.py b/packages/sdk-python/src/stagehand/_generated/models.py index 06f6b6b8f..293fdf9a5 100644 --- a/packages/sdk-python/src/stagehand/_generated/models.py +++ b/packages/sdk-python/src/stagehand/_generated/models.py @@ -14,6 +14,7 @@ StrictFloat, StrictInt, StrictStr, + constr, ) from stagehand._validation import ( PageScreenshotOptionsValidation, @@ -125,6 +126,25 @@ class Action(WireModel): Example: ['Hello World'] """ + target: Optional[ActionTarget] = None + """Optional observed target identity. When present, act validates it before mutating the page.""" + argument_targets: Optional[dict[constr(pattern=r"^\d+$", strict=True), ActionTarget]] = ( + None + ) + """Optional observed identities for selector arguments, keyed by their argument index.""" + + +class ActionTarget(WireModel): + """Short-lived identity of the DOM node selected by an observed action""" + + model_config = ConfigDict( + extra="forbid", + validate_by_name=True, + ) + frame_ordinal: Annotated[StrictInt, Field(ge=0, le=9007199254740991)] + """Stable frame ordinal assigned for the lifetime of the page""" + backend_node_id: Annotated[StrictInt, Field(gt=0, le=9007199254740991)] + """CDP backend node ID captured when the action was observed""" class Animations(StrEnum): diff --git a/packages/sdk-python/tests/test_stagehand.py b/packages/sdk-python/tests/test_stagehand.py index f210c96b2..1477c38d0 100644 --- a/packages/sdk-python/tests/test_stagehand.py +++ b/packages/sdk-python/tests/test_stagehand.py @@ -8,7 +8,14 @@ import pytest from pydantic import BaseModel -from stagehand import LLMGenerateInput, LLMGenerateOutput, Page, ProtocolLocator, Stagehand +from stagehand import ( + ActionTarget, + LLMGenerateInput, + LLMGenerateOutput, + Page, + ProtocolLocator, + Stagehand, +) from stagehand._generated.models import ( Action, ActResult, @@ -243,7 +250,12 @@ async def connect(**_: object) -> RPCClient: async def test_stagehand_ai_methods_resolve_pages_and_validate_results( monkeypatch: pytest.MonkeyPatch, ) -> None: - action = Action(selector="a", description="More information") + action = Action( + selector="a", + description="More information", + target=ActionTarget(frame_ordinal=0, backend_node_id=12), + argument_targets={"0": ActionTarget(frame_ordinal=0, backend_node_id=20)}, + ) act_result = ActResultData( success=True, message="Clicked the link", diff --git a/packages/sdk-ts/src/index.ts b/packages/sdk-ts/src/index.ts index e3bcb5c3e..0aa6c7323 100644 --- a/packages/sdk-ts/src/index.ts +++ b/packages/sdk-ts/src/index.ts @@ -16,6 +16,7 @@ export type { InitScriptSource } from "./pageScripts.js"; export { Stagehand } from "./stagehand.js"; export type { Action, + ActionTarget, ActResultData, BrowserGetVersionResult, RuntimeLoopbackStatusResult, diff --git a/packages/sdk-ts/tests/object-wrapper.test.ts b/packages/sdk-ts/tests/object-wrapper.test.ts index 3925dc675..5ac371eed 100644 --- a/packages/sdk-ts/tests/object-wrapper.test.ts +++ b/packages/sdk-ts/tests/object-wrapper.test.ts @@ -757,6 +757,8 @@ describe("Stagehand TS object wrapper", () => { description: "Submit button", method: "click", arguments: [], + target: { frameOrdinal: 0, backendNodeId: 12 }, + argumentTargets: { "0": { frameOrdinal: 0, backendNodeId: 20 } }, }; client.queueResponse(StagehandMethods.stagehandObserve, { result: [observedAction], diff --git a/packages/server/actionTarget.ts b/packages/server/actionTarget.ts new file mode 100644 index 000000000..2b6cf2342 --- /dev/null +++ b/packages/server/actionTarget.ts @@ -0,0 +1,47 @@ +import type { ActionTarget } from "../protocol/types.js"; +import type { EncodedId } from "./types/private/internal.js"; +import { trimTrailingTextNode } from "./utils.js"; + +export class ActionTargetMismatchError extends Error { + constructor( + readonly expected: ActionTarget, + readonly actual: ActionTarget, + ) { + super( + `Observed action target changed: expected frame ${expected.frameOrdinal} node ${expected.backendNodeId}, ` + + `resolved frame ${actual.frameOrdinal} node ${actual.backendNodeId}`, + ); + this.name = "ActionTargetMismatchError"; + } +} + +export function actionTargetFromEncodedId(elementId: EncodedId): ActionTarget { + const [frameOrdinal, backendNodeId] = elementId.split("-").map(Number); + return { frameOrdinal: frameOrdinal!, backendNodeId: backendNodeId! }; +} + +/** + * Return the identity of the DOM element addressed by an emitted XPath. + * Snapshot inference may select a text node whose XPath is normalized to its + * parent element, so the selected encoded ID is not always the actionable ID. + */ +export function actionTargetForSnapshotSelector( + selector: string, + xpathMap: Record, +): ActionTarget | undefined { + const xpath = selector.replace(/^xpath=/iu, ""); + const match = Object.entries(xpathMap).find(([, candidateXpath]) => candidateXpath === xpath); + return match ? actionTargetFromEncodedId(match[0] as EncodedId) : undefined; +} + +export function selectorAndTargetForSnapshotElement( + elementId: EncodedId, + xpathMap: Record, +): { selector: string; target?: ActionTarget } | undefined { + const xpath = trimTrailingTextNode(xpathMap[elementId]); + if (!xpath) return undefined; + + const selector = `xpath=${xpath}`; + const target = actionTargetForSnapshotSelector(selector, xpathMap); + return { selector, ...(target ? { target } : {}) }; +} diff --git a/packages/server/handlers/handlerUtils/actHandlerUtils.ts b/packages/server/handlers/handlerUtils/actHandlerUtils.ts index 1c8da899c..d98ecbc23 100644 --- a/packages/server/handlers/handlerUtils/actHandlerUtils.ts +++ b/packages/server/handlers/handlerUtils/actHandlerUtils.ts @@ -2,11 +2,14 @@ import { Protocol } from "devtools-protocol"; import { Frame } from "../../understudy/frame.js"; import { Locator } from "../../understudy/locator.js"; -import type { MouseButton } from "../../../protocol/types.js"; +import type { Action, ActionTarget, MouseButton } from "../../../protocol/types.js"; import { resolveLocatorWithHops } from "../../understudy/deepLocator.js"; import type { Page } from "../../understudy/page.js"; import type { StagehandLogger } from "../../logger.js"; import { toTitleCase } from "../../utils.js"; +export { ActionTargetMismatchError } from "../../actionTarget.js"; + +export type ActionTargetConstraints = Pick; export interface UnderstudyMethodHandlerContext { method: string; @@ -18,6 +21,7 @@ export interface UnderstudyMethodHandlerContext { initialUrl: string; logger: StagehandLogger; domSettleTimeoutMs?: number; + argumentTargets?: Record; } // Normalize cases where the XPath is the root "/" to point to the HTML element. @@ -50,6 +54,7 @@ export async function performUnderstudyMethod( args: ReadonlyArray, logger: StagehandLogger, domSettleTimeoutMs?: number, + targetConstraints?: ActionTargetConstraints, ): Promise { const selectorRaw = normalizeRootXPath(rawXPath); @@ -59,7 +64,8 @@ export async function performUnderstudyMethod( { target: selectorRaw }, async (spanLogger) => { // Unified resolver: supports '>>' hops and XPath across iframes. - const locator: Locator = await resolveLocatorWithHops(page, frame, selectorRaw); + const resolvedLocator: Locator = await resolveLocatorWithHops(page, frame, selectorRaw); + const locator = bindTargetGuard(page, resolvedLocator, targetConstraints?.target); const initialUrl = await getFrameUrl(frame); spanLogger.debug("Performing understudy method", { @@ -79,6 +85,7 @@ export async function performUnderstudyMethod( initialUrl, logger: spanLogger, domSettleTimeoutMs, + argumentTargets: targetConstraints?.argumentTargets, }; const handler = METHOD_HANDLER_MAP[method] ?? null; @@ -109,6 +116,12 @@ export async function performUnderstudyMethod( } } +function bindTargetGuard(page: Page, locator: Locator, target?: ActionTarget): Locator { + return target + ? locator.withTargetGuard(target, page.getOrdinal(locator.getFrame().frameId)) + : locator; +} + /* ===================== Handlers & Map ===================== */ const METHOD_HANDLER_MAP: Record Promise> = { @@ -282,11 +295,12 @@ async function doubleClick(ctx: UnderstudyMethodHandlerContext): Promise { } async function dragAndDrop(ctx: UnderstudyMethodHandlerContext): Promise { - const { page, frame, locator, args, xpath, logger } = ctx; + const { page, frame, locator, args, xpath, logger, argumentTargets } = ctx; const toXPath = String(args[0] ?? "").trim(); if (!toXPath) throw new Error("dragAndDrop requires a target XPath arg"); - const targetLocator = await resolveLocatorWithHops(page, frame, toXPath); + const resolvedTargetLocator = await resolveLocatorWithHops(page, frame, toXPath); + const targetLocator = bindTargetGuard(page, resolvedTargetLocator, argumentTargets?.["0"]); try { // 1) Centers in local (owning-frame) viewport diff --git a/packages/server/services/actService.ts b/packages/server/services/actService.ts index a01371922..2d69a7c8d 100644 --- a/packages/server/services/actService.ts +++ b/packages/server/services/actService.ts @@ -7,9 +7,11 @@ import type { StagehandActParams, Variables, } from "../../protocol/types.js"; +import { selectorAndTargetForSnapshotElement } from "../actionTarget.js"; import { TimeoutError } from "../errors.js"; import { performUnderstudyMethod, + type ActionTargetConstraints, waitForDomNetworkQuiet, } from "../handlers/handlerUtils/actHandlerUtils.js"; import { createTimeoutGuard } from "../handlers/handlerUtils/timeoutGuard.js"; @@ -22,7 +24,6 @@ import type { EncodedId } from "../types/private/internal.js"; import { SupportedUnderstudyAction } from "../types/private/handlers.js"; import { diffCombinedTrees } from "../understudy/a11y/snapshot/index.js"; import type { Page } from "../understudy/page.js"; -import { trimTrailingTextNode } from "../utils.js"; import * as cacheService from "./cacheService.js"; import * as llmService from "./llmService.js"; @@ -297,7 +298,7 @@ async function takeDeterministicAction({ try { context.ensureTimeRemaining(); - await performUnderstudyMethod( + const actionArgs = [ context.page, context.page.mainFrame(), method, @@ -305,7 +306,13 @@ async function takeDeterministicAction({ resolvedArgs, context.logger, context.domSettleTimeoutMs, - ); + ] as const; + const targetConstraints = targetConstraintsForAction(action); + if (targetConstraints) { + await performUnderstudyMethod(...actionArgs, targetConstraints); + } else { + await performUnderstudyMethod(...actionArgs); + } return successfulActionResult(action, method, action.selector, placeholderArgs); } catch (error) { if (error instanceof TimeoutError) throw error; @@ -375,7 +382,7 @@ async function selfHealAction({ const selector = inferenceResult.action?.selector ?? action.selector; context.ensureTimeRemaining(); - await performUnderstudyMethod( + const actionArgs = [ context.page, context.page.mainFrame(), method, @@ -383,8 +390,21 @@ async function selfHealAction({ resolvedArgs, context.logger, context.domSettleTimeoutMs, + ] as const; + const targetConstraints = inferenceResult.action + ? targetConstraintsForAction(inferenceResult.action) + : undefined; + if (targetConstraints) { + await performUnderstudyMethod(...actionArgs, targetConstraints); + } else { + await performUnderstudyMethod(...actionArgs); + } + return successfulActionResult( + inferenceResult.action ?? action, + method, + selector, + placeholderArgs, ); - return successfulActionResult(action, method, selector, placeholderArgs); } catch (error) { if (error instanceof TimeoutError) throw error; const message = error instanceof Error ? error.message : String(error); @@ -402,10 +422,11 @@ function normalizeActInferenceElement( xpathMap: Record, logger: StagehandLogger, ): Action | undefined { - const xpath = trimTrailingTextNode(xpathMap[element.elementId as EncodedId]); - if (!xpath) return undefined; + const source = selectorAndTargetForSnapshotElement(element.elementId as EncodedId, xpathMap); + if (!source) return undefined; let args = element.arguments; + let argumentTargets: Action["argumentTargets"]; if (element.method === SupportedUnderstudyAction.DRAG_AND_DROP && args.length > 0) { const targetElementId = args[0]; if (!targetElementId || !/^\d+-\d+$/.test(targetElementId)) { @@ -417,8 +438,8 @@ function normalizeActInferenceElement( return undefined; } - const targetXpath = trimTrailingTextNode(xpathMap[targetElementId as EncodedId]); - if (!targetXpath) { + const destination = selectorAndTargetForSnapshotElement(targetElementId as EncodedId, xpathMap); + if (!destination) { logger.debug("Drag-and-drop target element lookup failed", { category: "action", targetElementId, @@ -426,14 +447,17 @@ function normalizeActInferenceElement( }); return undefined; } - args = [`xpath=${targetXpath}`, ...args.slice(1)]; + args = [destination.selector, ...args.slice(1)]; + if (destination.target) argumentTargets = { "0": destination.target }; } return { - selector: `xpath=${xpath}`, + selector: source.selector, description: element.description, method: element.method, arguments: args, + ...(source.target ? { target: source.target } : {}), + ...(argumentTargets ? { argumentTargets } : {}), }; } @@ -468,6 +492,8 @@ function successfulActionResult( description: action.description || `action (${method})`, method, arguments: arguments_, + ...(action.target ? { target: action.target } : {}), + ...(action.argumentTargets ? { argumentTargets: action.argumentTargets } : {}), }, ], }; @@ -477,6 +503,14 @@ function actResult(result: ActResultData): ActResult { return { result }; } +function targetConstraintsForAction(action: Action): ActionTargetConstraints | undefined { + if (!action.target && !action.argumentTargets) return undefined; + return { + ...(action.target ? { target: action.target } : {}), + ...(action.argumentTargets ? { argumentTargets: action.argumentTargets } : {}), + }; +} + function describeAction(action: Action): string { return `method: ${action.method}, description: ${action.description}, arguments: ${action.arguments?.join(", ") ?? ""}`; } diff --git a/packages/server/services/cacheService.ts b/packages/server/services/cacheService.ts index 20a33d6b7..f79cbcf41 100644 --- a/packages/server/services/cacheService.ts +++ b/packages/server/services/cacheService.ts @@ -1,6 +1,8 @@ import type { Protocol } from "devtools-protocol"; +import { ActionTargetSchema } from "../../protocol/schemas.js"; import type { Action, + ActionTarget, Caching, StagehandActParams, StagehandExtractParams, @@ -127,6 +129,8 @@ export function normalizeCachedActions(value: unknown): Action[] { if (entry === null || typeof entry !== "object") continue; const candidate = entry as Record; if (typeof candidate.selector !== "string" || candidate.selector.length === 0) continue; + const target = normalizeCachedActionTarget(candidate.target); + const argumentTargets = normalizeCachedArgumentTargets(candidate.argumentTargets); actions.push({ selector: candidate.selector, description: typeof candidate.description === "string" ? candidate.description : "", @@ -134,11 +138,32 @@ export function normalizeCachedActions(value: unknown): Action[] { arguments: Array.isArray(candidate.arguments) ? candidate.arguments.filter((arg): arg is string => typeof arg === "string") : [], + ...(target ? { target } : {}), + ...(argumentTargets ? { argumentTargets } : {}), }); } return actions; } +function normalizeCachedActionTarget(value: unknown): ActionTarget | undefined { + const parsed = ActionTargetSchema.safeParse(value); + return parsed.success ? parsed.data : undefined; +} + +function normalizeCachedArgumentTargets( + value: unknown, +): Action["argumentTargets"] | undefined { + if (value === null || typeof value !== "object" || Array.isArray(value)) return undefined; + + const argumentTargets: NonNullable = {}; + for (const [key, entry] of Object.entries(value)) { + if (!/^\d+$/u.test(key)) continue; + const target = normalizeCachedActionTarget(entry); + if (target) argumentTargets[key] = target; + } + return Object.keys(argumentTargets).length > 0 ? argumentTargets : undefined; +} + export interface CacheExecuteOutcome { result: Result; /** What to persist on a miss; undefined/null skips the cache write. */ diff --git a/packages/server/services/observeService.ts b/packages/server/services/observeService.ts index 13359118d..474291bdf 100644 --- a/packages/server/services/observeService.ts +++ b/packages/server/services/observeService.ts @@ -5,6 +5,10 @@ import type { ObserveResult, StagehandObserveParams, } from "../../protocol/types.js"; +import { + actionTargetForSnapshotSelector, + selectorAndTargetForSnapshotElement, +} from "../actionTarget.js"; import { TimeoutError } from "../errors.js"; import { createTimeoutGuard } from "../handlers/handlerUtils/timeoutGuard.js"; import * as inference from "../inference.js"; @@ -13,7 +17,6 @@ import type { StagehandLogger } from "../logger.js"; import type { Page } from "../understudy/page.js"; import { SupportedUnderstudyAction } from "../types/private/handlers.js"; import type { EncodedId } from "../types/private/internal.js"; -import { trimTrailingTextNode } from "../utils.js"; import * as cacheService from "./cacheService.js"; import * as llmService from "./llmService.js"; @@ -58,12 +61,13 @@ export async function observe({ caching: options?.cache, context: cache, logger, - onHit: (value) => { + onHit: async (value) => { const actions = cacheService.normalizeCachedActions(value); if (actions.length === 0) { throw new Error("Cached observe value contained no usable actions"); } - return { result: actions }; + const reboundActions = await bindActionsToCurrentSnapshot(actions); + return { result: reboundActions }; }, execute: () => runObservation(), }); @@ -94,8 +98,8 @@ export async function observe({ const actions: Action[] = []; for (const element of observation.elements) { - const sourceXpath = trimTrailingTextNode(xpathMap[element.elementId as EncodedId]); - if (!sourceXpath) { + const source = selectorAndTargetForSnapshotElement(element.elementId as EncodedId, xpathMap); + if (!source) { logger.warn("Observed element could not be resolved to an XPath", { category: "observation", elementId: element.elementId, @@ -104,6 +108,7 @@ export async function observe({ } let resolvedArguments = element.arguments; + let argumentTargets: Action["argumentTargets"]; if (element.method === SupportedUnderstudyAction.DRAG_AND_DROP) { const targetElementId = element.arguments[0]; if (!targetElementId || !/^\d+-\d+$/.test(targetElementId)) { @@ -115,8 +120,11 @@ export async function observe({ continue; } - const targetXpath = trimTrailingTextNode(xpathMap[targetElementId as EncodedId]); - if (!targetXpath) { + const destination = selectorAndTargetForSnapshotElement( + targetElementId as EncodedId, + xpathMap, + ); + if (!destination) { logger.warn("Drag-and-drop target could not be resolved to an XPath", { category: "observation", sourceElementId: element.elementId, @@ -124,14 +132,17 @@ export async function observe({ }); continue; } - resolvedArguments = [`xpath=${targetXpath}`, ...element.arguments.slice(1)]; + resolvedArguments = [destination.selector, ...element.arguments.slice(1)]; + if (destination.target) argumentTargets = { "0": destination.target }; } actions.push({ - selector: `xpath=${sourceXpath}`, + selector: source.selector, description: element.description, method: element.method, arguments: resolvedArguments, + ...(source.target ? { target: source.target } : {}), + ...(argumentTargets ? { argumentTargets } : {}), }); } @@ -156,4 +167,36 @@ export async function observe({ }, }; } + + async function bindActionsToCurrentSnapshot(actions: Action[]): Promise { + const { combinedXpathMap } = await page.captureSnapshot({ + focusSelector: focusSelector || undefined, + ignoreSelectors: options?.ignoreSelectors, + }); + return actions.map((action) => { + const target = actionTargetForSnapshotSelector(action.selector, combinedXpathMap); + if (!target) { + throw new Error(`Cached observe action no longer resolves: ${action.selector}`); + } + + const destinationSelector = + action.method === SupportedUnderstudyAction.DRAG_AND_DROP + ? action.arguments?.[0] + : undefined; + const destinationTarget = destinationSelector + ? actionTargetForSnapshotSelector(destinationSelector, combinedXpathMap) + : undefined; + if (destinationSelector && !destinationTarget) { + throw new Error( + `Cached observe action argument no longer resolves: ${destinationSelector}`, + ); + } + + return { + ...action, + target, + ...(destinationTarget ? { argumentTargets: { "0": destinationTarget } } : {}), + }; + }); + } } diff --git a/packages/server/tests/act.test.ts b/packages/server/tests/act.test.ts index bb22902d2..cc4960459 100644 --- a/packages/server/tests/act.test.ts +++ b/packages/server/tests/act.test.ts @@ -139,6 +139,7 @@ describe("act service", () => { ["user@example.com"], logger, 2_000, + { target: { frameOrdinal: 0, backendNodeId: 13 } }, ); expect(result).toStrictEqual({ result: { @@ -151,6 +152,7 @@ describe("act service", () => { description: "Email field", method: "fill", arguments: ["%accountEmail%"], + target: { frameOrdinal: 0, backendNodeId: 13 }, }, ], }, @@ -170,7 +172,13 @@ describe("act service", () => { arguments: [], }), ); - performAction.mockRejectedValueOnce(new Error("Element detached")).mockResolvedValueOnce(); + performAction + .mockRejectedValueOnce( + new Error( + "Observed action target changed: expected frame 0 node 12, resolved frame 0 node 99", + ), + ) + .mockResolvedValueOnce(); const result = await actService.act({ params: { @@ -180,6 +188,7 @@ describe("act service", () => { description: "Submit button", method: "click", arguments: [], + target: { frameOrdinal: 0, backendNodeId: 12 }, }, }, page, @@ -201,6 +210,7 @@ describe("act service", () => { [], expect.any(StagehandLogger), undefined, + { target: { frameOrdinal: 0, backendNodeId: 12 } }, ); expect(performAction).toHaveBeenNthCalledWith( 2, @@ -211,10 +221,52 @@ describe("act service", () => { [], expect.any(StagehandLogger), undefined, + { target: { frameOrdinal: 0, backendNodeId: 20 } }, ); expect(result.result).toMatchObject({ success: true, - actions: [{ selector: "xpath=/html/body/button[2]" }], + actions: [ + { + selector: "xpath=/html/body/button[2]", + target: { frameOrdinal: 0, backendNodeId: 20 }, + }, + ], + }); + }); + + it("fails safely when an observed Action resolves to a different target", async () => { + const page = actPage({}, vi.fn()); + const clientLLMGenerate = vi.fn(); + performAction.mockRejectedValueOnce( + new Error( + "Observed action target changed: expected frame 0 node 12, resolved frame 0 node 99", + ), + ); + + const result = await actService.act({ + params: { + pageId: "page-1", + input: { + selector: "xpath=/html/body/button[1]", + description: "Delete Alice", + method: "click", + arguments: [], + target: { frameOrdinal: 0, backendNodeId: 12 }, + }, + }, + page, + model: { source: "client" }, + clientLLMGenerate, + logger: testLogger(), + selfHeal: false, + }); + + expect(performAction).toHaveBeenCalledOnce(); + expect(clientLLMGenerate).not.toHaveBeenCalled(); + expect(result.result).toMatchObject({ + success: false, + message: expect.stringContaining("Observed action target changed"), + actions: [], }); }); @@ -306,10 +358,16 @@ describe("act service", () => { [], expect.any(StagehandLogger), undefined, + { target: { frameOrdinal: 0, backendNodeId: 20 } }, ); expect(result.result).toMatchObject({ success: true, - actions: [{ selector: "xpath=/html/body/button[2]" }], + actions: [ + { + selector: "xpath=/html/body/button[2]", + target: { frameOrdinal: 0, backendNodeId: 20 }, + }, + ], }); }); @@ -393,9 +451,15 @@ function actGeneration( } function snapshot(elementId: string, xpath: string) { + const combinedXpathMap: Record = { [elementId]: xpath }; + const normalizedXpath = xpath.replace(/\/text\(\)(\[\d+\])?$/u, ""); + if (normalizedXpath !== xpath) { + const [frameOrdinal, backendNodeId] = elementId.split("-").map(Number); + combinedXpathMap[`${frameOrdinal}-${backendNodeId! + 1}`] = normalizedXpath; + } return { combinedTree: `[${elementId}] button: Target`, - combinedXpathMap: { [elementId]: xpath }, + combinedXpathMap, combinedUrlMap: {}, }; } diff --git a/packages/server/tests/action-target-validation.test.ts b/packages/server/tests/action-target-validation.test.ts new file mode 100644 index 000000000..3f0cd6bca --- /dev/null +++ b/packages/server/tests/action-target-validation.test.ts @@ -0,0 +1,168 @@ +import { trace } from "@opentelemetry/api"; +import { beforeEach, describe, expect, it, vi } from "vitest"; +import { resolveLocatorWithHops } from "../understudy/deepLocator.js"; +import { + ActionTargetMismatchError, + performUnderstudyMethod, +} from "../handlers/handlerUtils/actHandlerUtils.js"; +import { StagehandLogger } from "../logger.js"; +import type { Frame } from "../understudy/frame.js"; +import type { Locator } from "../understudy/locator.js"; +import type { Page } from "../understudy/page.js"; + +vi.mock("../understudy/deepLocator.js", () => ({ + resolveLocatorWithHops: vi.fn(), +})); + +const resolveLocator = vi.mocked(resolveLocatorWithHops); + +describe("action target validation", () => { + const logger = new StagehandLogger( + { tracer: trace.getTracer("action-target-validation-test") }, + () => {}, + ); + + beforeEach(() => { + resolveLocator.mockReset(); + }); + + it("blocks the action handler when the selector resolves to a different node", async () => { + const click = vi.fn(); + resolveLocator.mockResolvedValue(locatorForNode(0, 20, click)); + + await expect( + performUnderstudyMethod( + pageWithOrdinals(), + rootFrame(), + "click", + "xpath=/html/body/button[1]", + [], + logger, + undefined, + { target: { frameOrdinal: 0, backendNodeId: 12 } }, + ), + ).rejects.toBeInstanceOf(ActionTargetMismatchError); + + expect(click).not.toHaveBeenCalled(); + }); + + it("executes the action handler when the observed target still matches", async () => { + const click = vi.fn().mockResolvedValue(undefined); + resolveLocator.mockResolvedValue(locatorForNode(0, 12, click)); + + await performUnderstudyMethod( + pageWithOrdinals(), + rootFrame(), + "click", + "xpath=/html/body/button[1]", + [], + logger, + undefined, + { target: { frameOrdinal: 0, backendNodeId: 12 } }, + ); + + expect(click).toHaveBeenCalledOnce(); + }); + + it("keeps legacy actions without target metadata working", async () => { + const click = vi.fn().mockResolvedValue(undefined); + const readBackendNodeId = vi.fn(async () => 20); + const locator = locatorForNode(0, 20, click, readBackendNodeId); + resolveLocator.mockResolvedValue(locator); + + await performUnderstudyMethod( + pageWithOrdinals(), + rootFrame(), + "click", + "xpath=/html/body/button[1]", + [], + logger, + ); + + expect(readBackendNodeId).not.toHaveBeenCalled(); + expect(click).toHaveBeenCalledOnce(); + }); + + it("blocks drag-and-drop when the destination resolves to a different node", async () => { + const dragAndDrop = vi.fn(); + const page = pageWithOrdinals(dragAndDrop); + resolveLocator + .mockResolvedValueOnce(locatorForNode(0, 12, vi.fn())) + .mockResolvedValueOnce(locatorForNode(0, 99, vi.fn())); + + await expect( + performUnderstudyMethod( + page, + rootFrame(), + "dragAndDrop", + "xpath=/html/body/source", + ["xpath=/html/body/destination"], + logger, + undefined, + { + target: { frameOrdinal: 0, backendNodeId: 12 }, + argumentTargets: { "0": { frameOrdinal: 0, backendNodeId: 20 } }, + }, + ), + ).rejects.toBeInstanceOf(ActionTargetMismatchError); + + expect(dragAndDrop).not.toHaveBeenCalled(); + }); +}); + +function rootFrame(): Frame { + return { + frameId: "root-frame", + evaluate: vi.fn(async () => "https://example.test"), + } as unknown as Frame; +} + +function pageWithOrdinals(dragAndDrop: ReturnType = vi.fn()): Page { + return { + getOrdinal: (frameId: string) => (frameId === "target-frame" ? 0 : 1), + dragAndDrop, + } as unknown as Page; +} + +function locatorForNode( + frameOrdinal: number, + backendNodeId: number, + click: () => unknown, + readBackendNodeId: () => Promise = vi.fn(async () => backendNodeId), +): Locator { + const frame = { + frameId: frameOrdinal === 0 ? "target-frame" : `target-frame-${frameOrdinal}`, + evaluate: vi.fn(async (_fn: unknown, value: unknown) => value), + } as unknown as Frame; + const centroid = vi.fn(async () => ({ x: 10, y: 20 })); + const validate = async (expected: { frameOrdinal: number; backendNodeId: number }) => { + const actual = { + frameOrdinal, + backendNodeId: await readBackendNodeId(), + }; + if ( + actual.frameOrdinal !== expected.frameOrdinal || + actual.backendNodeId !== expected.backendNodeId + ) { + throw new ActionTargetMismatchError(expected, actual); + } + }; + const locator = { + getFrame: () => frame, + backendNodeId: readBackendNodeId, + click, + centroid, + withTargetGuard: (expected: { frameOrdinal: number; backendNodeId: number }) => ({ + ...locator, + click: async () => { + await validate(expected); + await click(); + }, + centroid: async () => { + await validate(expected); + return await centroid(); + }, + }), + }; + return locator as unknown as Locator; +} diff --git a/packages/server/tests/cache-actions.test.ts b/packages/server/tests/cache-actions.test.ts new file mode 100644 index 000000000..87433bf31 --- /dev/null +++ b/packages/server/tests/cache-actions.test.ts @@ -0,0 +1,50 @@ +import { describe, expect, it } from "vitest"; +import { normalizeCachedActions } from "../services/cacheService.js"; + +describe("normalizeCachedActions", () => { + it("preserves observed target identities when present", () => { + expect( + normalizeCachedActions([ + { + selector: "xpath=/html/body/button", + description: "Submit", + method: "click", + arguments: [], + target: { frameOrdinal: 0, backendNodeId: 12 }, + argumentTargets: { "0": { frameOrdinal: 0, backendNodeId: 20 } }, + }, + ]), + ).toStrictEqual([ + { + selector: "xpath=/html/body/button", + description: "Submit", + method: "click", + arguments: [], + target: { frameOrdinal: 0, backendNodeId: 12 }, + argumentTargets: { "0": { frameOrdinal: 0, backendNodeId: 20 } }, + }, + ]); + }); + + it("drops malformed target metadata without rejecting the action", () => { + expect( + normalizeCachedActions([ + { + selector: "xpath=/html/body/button", + description: "Submit", + method: "click", + arguments: [], + target: { frameOrdinal: -1, backendNodeId: 0 }, + argumentTargets: { destination: { frameOrdinal: 0, backendNodeId: 20 } }, + }, + ]), + ).toStrictEqual([ + { + selector: "xpath=/html/body/button", + description: "Submit", + method: "click", + arguments: [], + }, + ]); + }); +}); diff --git a/packages/server/tests/locator-target-guard.test.ts b/packages/server/tests/locator-target-guard.test.ts new file mode 100644 index 000000000..6a2735fea --- /dev/null +++ b/packages/server/tests/locator-target-guard.test.ts @@ -0,0 +1,59 @@ +import { describe, expect, it, vi } from "vitest"; +import { ActionTargetMismatchError } from "../actionTarget.js"; +import { Locator } from "../understudy/locator.js"; +import type { Frame } from "../understudy/frame.js"; + +const { resolveAtIndex } = vi.hoisted(() => ({ resolveAtIndex: vi.fn() })); + +vi.mock("../understudy/selectorResolver.js", () => ({ + FrameSelectorResolver: class { + static parseSelector(selector: string) { + return selector; + } + + resolveAtIndex = resolveAtIndex; + }, +})); + +describe("locator target guard", () => { + it("validates and mutates the same resolved object", async () => { + resolveAtIndex.mockResolvedValueOnce({ nodeId: null, objectId: "object-12" }); + const send = sessionSendForNode(12); + const locator = new Locator(frameWithSession(send), "xpath=/html/body/button").withTargetGuard( + { frameOrdinal: 0, backendNodeId: 12 }, + 0, + ); + + await locator.click(); + + expect(send).toHaveBeenCalledWith("DOM.describeNode", { objectId: "object-12" }); + expect(send).toHaveBeenCalledWith("DOM.getBoxModel", { objectId: "object-12" }); + }); + + it("rejects a replacement before dispatching input", async () => { + resolveAtIndex.mockResolvedValueOnce({ nodeId: null, objectId: "object-99" }); + const send = sessionSendForNode(99); + const locator = new Locator(frameWithSession(send), "xpath=/html/body/button").withTargetGuard( + { frameOrdinal: 0, backendNodeId: 12 }, + 0, + ); + + await expect(locator.click()).rejects.toBeInstanceOf(ActionTargetMismatchError); + + expect(send).not.toHaveBeenCalledWith("Input.dispatchMouseEvent", expect.anything()); + }); +}); + +function frameWithSession(send: ReturnType): Frame { + return { session: { send } } as unknown as Frame; +} + +function sessionSendForNode(backendNodeId: number) { + return vi.fn(async (method: string) => { + if (method === "DOM.describeNode") return { node: { backendNodeId } }; + if (method === "DOM.getBoxModel") { + return { model: { content: [0, 0, 20, 0, 20, 20, 0, 20] } }; + } + return {}; + }); +} diff --git a/packages/server/tests/observe.test.ts b/packages/server/tests/observe.test.ts index 64bee10ff..78c7c01fb 100644 --- a/packages/server/tests/observe.test.ts +++ b/packages/server/tests/observe.test.ts @@ -3,6 +3,7 @@ import { describe, expect, it, vi } from "vitest"; import type { LLMGenerateParams, LLMGenerateResult } from "../../protocol/types.js"; import * as inference from "../inference.js"; import { StagehandLogger } from "../logger.js"; +import * as cacheService from "../services/cacheService.js"; import * as observeService from "../services/observeService.js"; describe("observe inference", () => { @@ -89,8 +90,10 @@ describe("observe service", () => { combinedTree: "[0-12] textbox: Email\n[0-20] listitem: Source\n[0-21] listitem: Target", combinedXpathMap: { "0-12": "/html/body/main/input/text()", + "0-13": "/html/body/main/input", "0-20": "/html/body/main/ul/li[1]", "0-21": "/html/body/main/ul/li[2]/text()", + "0-22": "/html/body/main/ul/li[2]", }, combinedUrlMap: {}, })); @@ -158,12 +161,15 @@ describe("observe service", () => { description: "Email field", method: "fill", arguments: ["%accountEmail%"], + target: { frameOrdinal: 0, backendNodeId: 13 }, }, { selector: "xpath=/html/body/main/ul/li[1]", description: "Draggable source", method: "dragAndDrop", arguments: ["xpath=/html/body/main/ul/li[2]"], + target: { frameOrdinal: 0, backendNodeId: 20 }, + argumentTargets: { "0": { frameOrdinal: 0, backendNodeId: 22 } }, }, ], }); @@ -210,6 +216,66 @@ describe("observe service", () => { now.mockRestore(); } }); + + it("rebinds cached actions to target identities from the current snapshot", async () => { + const captureSnapshot = vi.fn(async () => ({ + combinedTree: "[0-99] button: Submit", + combinedXpathMap: { "0-99": "/html/body/button" }, + combinedUrlMap: {}, + })); + const frame = { + frameId: "root-frame", + getAccessibilityTree: vi.fn(async () => []), + }; + const page = { + captureSnapshot, + url: () => "https://example.test", + frames: () => [frame], + mainFrame: () => frame, + }; + const get = vi.fn(async () => ({ + hit: true, + value: [ + { + selector: "xpath=/html/body/button", + description: "Submit button", + method: "click", + arguments: [], + target: { frameOrdinal: 0, backendNodeId: 12 }, + }, + ], + })); + + const result = await observeService.observe({ + params: { pageId: "page-1" }, + page, + model: { source: "client" }, + clientLLMGenerate: vi.fn(), + logger: new StagehandLogger( + { tracer: trace.getTracer("observe-cache-target-test") }, + () => {}, + ), + cache: { + sessionId: "session-1", + defaultCaching: true, + client: { get } as unknown as cacheService.CacheContext["client"], + }, + }); + + expect(result).toStrictEqual({ + result: [ + { + selector: "xpath=/html/body/button", + description: "Submit button", + method: "click", + arguments: [], + target: { frameOrdinal: 0, backendNodeId: 99 }, + }, + ], + cacheStatus: "HIT", + }); + expect(captureSnapshot).toHaveBeenCalledOnce(); + }); }); function observationResult( diff --git a/packages/server/understudy/locator.ts b/packages/server/understudy/locator.ts index 13b84d43e..53d167d08 100644 --- a/packages/server/understudy/locator.ts +++ b/packages/server/understudy/locator.ts @@ -19,12 +19,18 @@ import { import type { Frame } from "./frame.js"; import { FrameSelectorResolver, type SelectorQuery } from "./selectorResolver.js"; import { bytesToBase64, normalizeInputFiles } from "./fileUploadUtils.js"; -import type { MouseButton } from "../../protocol/types.js"; +import type { ActionTarget, MouseButton } from "../../protocol/types.js"; +import { ActionTargetMismatchError } from "../actionTarget.js"; import type { SetInputFilesArgument } from "../types/private/fileUpload.js"; import type { NormalizedFilePayload } from "../types/private/locator.js"; const MAX_REMOTE_UPLOAD_BYTES = 50 * 1024 * 1024; // 50MB guard copied from Playwright +type LocatorTargetGuard = { + expected: ActionTarget; + frameOrdinal: number; +}; + /** * Locator * @@ -57,6 +63,7 @@ export class Locator { readonly selector: string, readonly options?: { deep?: boolean; depth?: number }, nthIndex: number = -1, + readonly targetGuard?: LocatorTargetGuard, ) { this.selectorResolver = new FrameSelectorResolver(this.frame); this.selectorQuery = FrameSelectorResolver.parseSelector(selector); @@ -69,6 +76,14 @@ export class Locator { return this.frame; } + /** Bind an observed identity to every subsequent resolution by this locator. */ + public withTargetGuard(expected: ActionTarget, frameOrdinal: number): Locator { + return new Locator(this.frame, this.selector, this.options, this.nthIndex, { + expected, + frameOrdinal, + }); + } + /** * Set files on an element. * @@ -752,7 +767,7 @@ export class Locator { return this; } - return new Locator(this.frame, this.selector, this.options, nextIndex); + return new Locator(this.frame, this.selector, this.options, nextIndex, this.targetGuard); } // ---------- helpers ---------- @@ -776,9 +791,36 @@ export class Locator { throw new Error(`Could not find an element for the given xPath(s): ${this.selector}`); } + await this.validateTarget(resolved.objectId); + return resolved; } + private async validateTarget(objectId: Protocol.Runtime.RemoteObjectId): Promise { + if (!this.targetGuard) return; + + try { + const { node } = await this.frame.session.send<{ node: Protocol.DOM.Node }>( + "DOM.describeNode", + { objectId }, + ); + const actual: ActionTarget = { + frameOrdinal: this.targetGuard.frameOrdinal, + backendNodeId: node.backendNodeId, + }; + const { expected } = this.targetGuard; + if ( + actual.frameOrdinal !== expected.frameOrdinal || + actual.backendNodeId !== expected.backendNodeId + ) { + throw new ActionTargetMismatchError(expected, actual); + } + } catch (error) { + await this.frame.session.send("Runtime.releaseObject", { objectId }).catch(() => {}); + throw error; + } + } + /** * Resolve all matching nodes for this locator. * If the locator is narrowed via nth(), only that index is returned. From 6658d8e34cd2c05e997f43d29c26acb466934902 Mon Sep 17 00:00:00 2001 From: Vishal Anton Date: Mon, 27 Jul 2026 01:42:14 +0530 Subject: [PATCH 3/5] Enhance action target handling and validation --- packages/server/actionTarget.ts | 50 +++++++++-- .../handlers/handlerUtils/actHandlerUtils.ts | 35 ++++++-- packages/server/services/actService.ts | 39 ++++++--- packages/server/services/observeService.ts | 30 +------ packages/server/tests/act.test.ts | 74 ++++++++++++++++ .../tests/action-target-validation.test.ts | 86 +++++++++++++++++++ packages/server/tests/action-target.test.ts | 49 +++++++++++ .../server/tests/locator-target-guard.test.ts | 62 ++++++++++--- packages/server/understudy/locator.ts | 74 ++++++++++++++++ 9 files changed, 439 insertions(+), 60 deletions(-) create mode 100644 packages/server/tests/action-target.test.ts diff --git a/packages/server/actionTarget.ts b/packages/server/actionTarget.ts index 2b6cf2342..5bf2f9bbe 100644 --- a/packages/server/actionTarget.ts +++ b/packages/server/actionTarget.ts @@ -1,4 +1,4 @@ -import type { ActionTarget } from "../protocol/types.js"; +import type { Action, ActionTarget } from "../protocol/types.js"; import type { EncodedId } from "./types/private/internal.js"; import { trimTrailingTextNode } from "./utils.js"; @@ -20,6 +20,11 @@ export function actionTargetFromEncodedId(elementId: EncodedId): ActionTarget { return { frameOrdinal: frameOrdinal!, backendNodeId: backendNodeId! }; } +export function normalizeSnapshotRootXPath(xpath: string): string { + const trimmed = xpath.trim(); + return trimmed === "/" ? "/html[1]" : trimmed; +} + /** * Return the identity of the DOM element addressed by an emitted XPath. * Snapshot inference may select a text node whose XPath is normalized to its @@ -29,8 +34,12 @@ export function actionTargetForSnapshotSelector( selector: string, xpathMap: Record, ): ActionTarget | undefined { - const xpath = selector.replace(/^xpath=/iu, ""); - const match = Object.entries(xpathMap).find(([, candidateXpath]) => candidateXpath === xpath); + const xpath = normalizeSnapshotRootXPath(selector.replace(/^xpath=/iu, "")); + const match = + Object.entries(xpathMap).find(([, candidateXpath]) => candidateXpath === xpath) ?? + (xpath === "/html[1]" + ? Object.entries(xpathMap).find(([, candidateXpath]) => candidateXpath === "/html") + : undefined); return match ? actionTargetFromEncodedId(match[0] as EncodedId) : undefined; } @@ -38,10 +47,41 @@ export function selectorAndTargetForSnapshotElement( elementId: EncodedId, xpathMap: Record, ): { selector: string; target?: ActionTarget } | undefined { - const xpath = trimTrailingTextNode(xpathMap[elementId]); - if (!xpath) return undefined; + const rawXpath = trimTrailingTextNode(xpathMap[elementId]); + if (!rawXpath) return undefined; + const xpath = normalizeSnapshotRootXPath(rawXpath); const selector = `xpath=${xpath}`; const target = actionTargetForSnapshotSelector(selector, xpathMap); return { selector, ...(target ? { target } : {}) }; } + +/** + * Refresh short-lived target identities from the current snapshot while keeping + * cached selectors. Throws when a selector no longer resolves. + */ +export function rebindActionTargetsToSnapshot( + actions: Action[], + xpathMap: Record, +): Action[] { + return actions.map((action) => { + const target = actionTargetForSnapshotSelector(action.selector, xpathMap); + if (!target) { + throw new Error(`Cached action no longer resolves: ${action.selector}`); + } + + const destinationSelector = action.method === "dragAndDrop" ? action.arguments?.[0] : undefined; + const destinationTarget = destinationSelector + ? actionTargetForSnapshotSelector(destinationSelector, xpathMap) + : undefined; + if (destinationSelector && !destinationTarget) { + throw new Error(`Cached action argument no longer resolves: ${destinationSelector}`); + } + + return { + ...action, + target, + ...(destinationTarget ? { argumentTargets: { "0": destinationTarget } } : {}), + }; + }); +} diff --git a/packages/server/handlers/handlerUtils/actHandlerUtils.ts b/packages/server/handlers/handlerUtils/actHandlerUtils.ts index d98ecbc23..325ae8711 100644 --- a/packages/server/handlers/handlerUtils/actHandlerUtils.ts +++ b/packages/server/handlers/handlerUtils/actHandlerUtils.ts @@ -25,10 +25,11 @@ export interface UnderstudyMethodHandlerContext { } // Normalize cases where the XPath is the root "/" to point to the HTML element. +// Prefer `/html[1]` to match snapshot xpath maps built by buildChildXPathSegments. function normalizeRootXPath(input: string): string { const s = String(input ?? "").trim(); - if (s === "/") return "/html"; - if (/^xpath=\/$/i.test(s)) return "xpath=/html"; + if (s === "/") return "/html[1]"; + if (/^xpath=\/$/i.test(s)) return "xpath=/html[1]"; return s; } @@ -196,7 +197,13 @@ async function scrollByPixelOffset(ctx: UnderstudyMethodHandlerContext): Promise } async function wheelScroll(ctx: UnderstudyMethodHandlerContext): Promise { - const { frame, args, logger } = ctx; + const { locator, frame, args, logger } = ctx; + const { objectId } = await locator.resolveNode(); + await locator + .getFrame() + .session.send("Runtime.releaseObject", { objectId }) + .catch(() => {}); + const deltaY = Number(args[0] ?? 200); logger.debug("Dispatching mouse wheel", { category: "action", @@ -243,9 +250,22 @@ async function typeText(ctx: UnderstudyMethodHandlerContext): Promise { } async function pressKey(ctx: UnderstudyMethodHandlerContext): Promise { - const { args, xpath, page, logger } = ctx; + const { locator, args, xpath, page, logger } = ctx; const key = args[0] ?? ""; try { + // Resolve (and focus) so observed target guards run before key input. + const session = locator.getFrame().session; + const { objectId } = await locator.resolveNode(); + try { + await session.send("Runtime.callFunctionOn", { + objectId, + functionDeclaration: "function() { this.focus?.(); }", + returnByValue: true, + }); + } finally { + await session.send("Runtime.releaseObject", { objectId }).catch(() => {}); + } + logger.debug("Pressing key", { category: "action", key, @@ -303,7 +323,7 @@ async function dragAndDrop(ctx: UnderstudyMethodHandlerContext): Promise { const targetLocator = bindTargetGuard(page, resolvedTargetLocator, argumentTargets?.["0"]); try { - // 1) Centers in local (owning-frame) viewport + // 1) Centers in local (owning-frame) viewport (hit-checks when guarded) const { x: fromLocalX, y: fromLocalY } = await locator.centroid(); const { x: toLocalX, y: toLocalY } = await targetLocator.centroid(); @@ -348,6 +368,11 @@ async function dragAndDrop(ctx: UnderstudyMethodHandlerContext): Promise { { x: toLocalX, y: toLocalY }, ); + // Re-check immediately before coordinate drag — centroid alone leaves a + // window during abs conversion where a rerender can steal the drop. + await locator.assertPointerTargetAt(fromLocalX, fromLocalY); + await targetLocator.assertPointerTargetAt(toLocalX, toLocalY); + // 3) Perform drag in main session await page.dragAndDrop(fromAbs.x, fromAbs.y, toAbs.x, toAbs.y, { steps: 10, diff --git a/packages/server/services/actService.ts b/packages/server/services/actService.ts index 2d69a7c8d..4eebf6125 100644 --- a/packages/server/services/actService.ts +++ b/packages/server/services/actService.ts @@ -7,7 +7,10 @@ import type { StagehandActParams, Variables, } from "../../protocol/types.js"; -import { selectorAndTargetForSnapshotElement } from "../actionTarget.js"; +import { + rebindActionTargetsToSnapshot, + selectorAndTargetForSnapshotElement, +} from "../actionTarget.js"; import { TimeoutError } from "../errors.js"; import { performUnderstudyMethod, @@ -215,8 +218,13 @@ async function replayCachedActions( throw new Error("Cached act value contained no usable actions"); } + context.ensureTimeRemaining(); + const { combinedXpathMap } = await context.page.captureSnapshot({}); + context.ensureTimeRemaining(); + const reboundActions = rebindActionTargetsToSnapshot(actions, combinedXpathMap); + const results: ActResultData[] = []; - for (const action of actions) { + for (const action of reboundActions) { const result = await takeDeterministicAction({ action, variables, @@ -337,6 +345,7 @@ async function takeDeterministicAction({ method, resolvedArgs, placeholderArgs, + variables, context, }); } @@ -347,12 +356,14 @@ async function selfHealAction({ method, resolvedArgs, placeholderArgs, + variables, context, }: { action: Action; method: string; resolvedArgs: string[]; placeholderArgs: string[]; + variables?: Variables; context: ActContext; }): Promise { const actionInstruction = action.description @@ -380,14 +391,27 @@ async function selfHealAction({ }; } - const selector = inferenceResult.action?.selector ?? action.selector; + const healed = inferenceResult.action ?? action; + const selector = healed.selector; + // Drag-and-drop destinations are re-inferred as XPaths paired with + // argumentTargets. Other methods (e.g. fill) must keep the caller's + // already-substituted argument values. + const useHealedArgs = + method === SupportedUnderstudyAction.DRAG_AND_DROP && + Array.isArray(inferenceResult.action?.arguments); + const healedPlaceholders = useHealedArgs + ? [...inferenceResult.action!.arguments!] + : placeholderArgs; + const healedResolvedArgs = useHealedArgs + ? (substituteVariablesInArguments(healedPlaceholders, variables) ?? healedPlaceholders) + : resolvedArgs; context.ensureTimeRemaining(); const actionArgs = [ context.page, context.page.mainFrame(), method, selector, - resolvedArgs, + healedResolvedArgs, context.logger, context.domSettleTimeoutMs, ] as const; @@ -399,12 +423,7 @@ async function selfHealAction({ } else { await performUnderstudyMethod(...actionArgs); } - return successfulActionResult( - inferenceResult.action ?? action, - method, - selector, - placeholderArgs, - ); + return successfulActionResult(healed, method, selector, healedPlaceholders); } catch (error) { if (error instanceof TimeoutError) throw error; const message = error instanceof Error ? error.message : String(error); diff --git a/packages/server/services/observeService.ts b/packages/server/services/observeService.ts index 474291bdf..3092f3ea3 100644 --- a/packages/server/services/observeService.ts +++ b/packages/server/services/observeService.ts @@ -6,7 +6,7 @@ import type { StagehandObserveParams, } from "../../protocol/types.js"; import { - actionTargetForSnapshotSelector, + rebindActionTargetsToSnapshot, selectorAndTargetForSnapshotElement, } from "../actionTarget.js"; import { TimeoutError } from "../errors.js"; @@ -169,34 +169,12 @@ export async function observe({ } async function bindActionsToCurrentSnapshot(actions: Action[]): Promise { + ensureTimeRemaining(); const { combinedXpathMap } = await page.captureSnapshot({ focusSelector: focusSelector || undefined, ignoreSelectors: options?.ignoreSelectors, }); - return actions.map((action) => { - const target = actionTargetForSnapshotSelector(action.selector, combinedXpathMap); - if (!target) { - throw new Error(`Cached observe action no longer resolves: ${action.selector}`); - } - - const destinationSelector = - action.method === SupportedUnderstudyAction.DRAG_AND_DROP - ? action.arguments?.[0] - : undefined; - const destinationTarget = destinationSelector - ? actionTargetForSnapshotSelector(destinationSelector, combinedXpathMap) - : undefined; - if (destinationSelector && !destinationTarget) { - throw new Error( - `Cached observe action argument no longer resolves: ${destinationSelector}`, - ); - } - - return { - ...action, - target, - ...(destinationTarget ? { argumentTargets: { "0": destinationTarget } } : {}), - }; - }); + ensureTimeRemaining(); + return rebindActionTargetsToSnapshot(actions, combinedXpathMap); } } diff --git a/packages/server/tests/act.test.ts b/packages/server/tests/act.test.ts index cc4960459..4f761d557 100644 --- a/packages/server/tests/act.test.ts +++ b/packages/server/tests/act.test.ts @@ -234,6 +234,80 @@ describe("act service", () => { }); }); + it("self-heals drag-and-drop with a fresh destination selector and target", async () => { + const frame = {}; + const captureSnapshot = vi.fn(async () => ({ + combinedTree: "[0-20] listitem: Source\n[0-21] listitem: Target", + combinedXpathMap: { + "0-20": "/html/body/ul/li[1]", + "0-21": "/html/body/ul/li[2]", + }, + combinedUrlMap: {}, + })); + const page = actPage(frame, captureSnapshot); + const clientLLMGenerate = vi.fn( + async (): Promise => + actGeneration({ + elementId: "0-20", + description: "Drag source onto target", + method: "dragAndDrop", + arguments: ["0-21"], + }), + ); + performAction + .mockRejectedValueOnce( + new Error( + "Observed action target changed: expected frame 0 node 12, resolved frame 0 node 99", + ), + ) + .mockResolvedValueOnce(); + + const result = await actService.act({ + params: { + pageId: "page-1", + input: { + selector: "xpath=/html/body/ul/li[1]", + description: "Drag source onto target", + method: "dragAndDrop", + arguments: ["xpath=/html/body/ul/li[3]"], + target: { frameOrdinal: 0, backendNodeId: 12 }, + argumentTargets: { "0": { frameOrdinal: 0, backendNodeId: 30 } }, + }, + }, + page, + model: { source: "client" }, + clientLLMGenerate, + logger: testLogger(), + selfHeal: true, + }); + + expect(performAction).toHaveBeenNthCalledWith( + 2, + page, + frame, + "dragAndDrop", + "xpath=/html/body/ul/li[1]", + ["xpath=/html/body/ul/li[2]"], + expect.any(StagehandLogger), + undefined, + { + target: { frameOrdinal: 0, backendNodeId: 20 }, + argumentTargets: { "0": { frameOrdinal: 0, backendNodeId: 21 } }, + }, + ); + expect(result.result).toMatchObject({ + success: true, + actions: [ + { + selector: "xpath=/html/body/ul/li[1]", + arguments: ["xpath=/html/body/ul/li[2]"], + target: { frameOrdinal: 0, backendNodeId: 20 }, + argumentTargets: { "0": { frameOrdinal: 0, backendNodeId: 21 } }, + }, + ], + }); + }); + it("fails safely when an observed Action resolves to a different target", async () => { const page = actPage({}, vi.fn()); const clientLLMGenerate = vi.fn(); diff --git a/packages/server/tests/action-target-validation.test.ts b/packages/server/tests/action-target-validation.test.ts index 3f0cd6bca..50d021a28 100644 --- a/packages/server/tests/action-target-validation.test.ts +++ b/packages/server/tests/action-target-validation.test.ts @@ -108,6 +108,77 @@ describe("action target validation", () => { expect(dragAndDrop).not.toHaveBeenCalled(); }); + + it("blocks drag-and-drop when a replacement appears under the drop point before dispatch", async () => { + const dragAndDrop = vi.fn(); + const page = pageWithOrdinals(dragAndDrop); + const source = locatorForNode(0, 12, vi.fn()); + const destination = locatorForNode(0, 20, vi.fn()); + const guardedDestination = destination.withTargetGuard( + { frameOrdinal: 0, backendNodeId: 20 }, + 0, + ); + guardedDestination.assertPointerTargetAt = async () => { + throw new ActionTargetMismatchError( + { frameOrdinal: 0, backendNodeId: 20 }, + { frameOrdinal: 0, backendNodeId: 99 }, + ); + }; + destination.withTargetGuard = () => guardedDestination; + resolveLocator.mockResolvedValueOnce(source).mockResolvedValueOnce(destination); + + await expect( + performUnderstudyMethod( + page, + rootFrame(), + "dragAndDrop", + "xpath=/html/body/source", + ["xpath=/html/body/destination"], + logger, + undefined, + { + target: { frameOrdinal: 0, backendNodeId: 12 }, + argumentTargets: { "0": { frameOrdinal: 0, backendNodeId: 20 } }, + }, + ), + ).rejects.toBeInstanceOf(ActionTargetMismatchError); + + expect(dragAndDrop).not.toHaveBeenCalled(); + }); + + it("resolves the guarded locator before press so stale targets fail closed", async () => { + const resolveNode = vi.fn(async () => { + throw new ActionTargetMismatchError( + { frameOrdinal: 0, backendNodeId: 12 }, + { frameOrdinal: 0, backendNodeId: 99 }, + ); + }); + const keyPress = vi.fn(); + resolveLocator.mockResolvedValue( + locatorForNode( + 0, + 99, + vi.fn(), + vi.fn(async () => 99), + resolveNode, + ), + ); + + await expect( + performUnderstudyMethod( + Object.assign(pageWithOrdinals(), { keyPress }) as unknown as Page, + rootFrame(), + "press", + "xpath=/html/body/input", + ["Enter"], + logger, + undefined, + { target: { frameOrdinal: 0, backendNodeId: 12 } }, + ), + ).rejects.toBeInstanceOf(ActionTargetMismatchError); + + expect(keyPress).not.toHaveBeenCalled(); + }); }); function rootFrame(): Frame { @@ -129,10 +200,16 @@ function locatorForNode( backendNodeId: number, click: () => unknown, readBackendNodeId: () => Promise = vi.fn(async () => backendNodeId), + resolveNode: () => Promise<{ objectId: string }> = vi.fn(async () => ({ + objectId: `object-${backendNodeId}`, + })), ): Locator { const frame = { frameId: frameOrdinal === 0 ? "target-frame" : `target-frame-${frameOrdinal}`, evaluate: vi.fn(async (_fn: unknown, value: unknown) => value), + session: { + send: vi.fn(async () => ({})), + }, } as unknown as Frame; const centroid = vi.fn(async () => ({ x: 10, y: 20 })); const validate = async (expected: { frameOrdinal: number; backendNodeId: number }) => { @@ -150,10 +227,16 @@ function locatorForNode( const locator = { getFrame: () => frame, backendNodeId: readBackendNodeId, + resolveNode, click, centroid, + assertPointerTargetAt: vi.fn(async () => undefined), withTargetGuard: (expected: { frameOrdinal: number; backendNodeId: number }) => ({ ...locator, + resolveNode: async () => { + await validate(expected); + return await resolveNode(); + }, click: async () => { await validate(expected); await click(); @@ -162,6 +245,9 @@ function locatorForNode( await validate(expected); return await centroid(); }, + assertPointerTargetAt: async () => { + await validate(expected); + }, }), }; return locator as unknown as Locator; diff --git a/packages/server/tests/action-target.test.ts b/packages/server/tests/action-target.test.ts new file mode 100644 index 000000000..38335c08b --- /dev/null +++ b/packages/server/tests/action-target.test.ts @@ -0,0 +1,49 @@ +import { describe, expect, it } from "vitest"; +import { + actionTargetForSnapshotSelector, + rebindActionTargetsToSnapshot, + selectorAndTargetForSnapshotElement, +} from "../actionTarget.js"; + +describe("actionTarget helpers", () => { + it("maps document-root selectors to the HTML element identity", () => { + const xpathMap = { + "0-1": "/", + "0-2": "/html[1]", + }; + + expect(selectorAndTargetForSnapshotElement("0-1", xpathMap)).toStrictEqual({ + selector: "xpath=/html[1]", + target: { frameOrdinal: 0, backendNodeId: 2 }, + }); + expect(actionTargetForSnapshotSelector("xpath=/", xpathMap)).toStrictEqual({ + frameOrdinal: 0, + backendNodeId: 2, + }); + }); + + it("rebinds cached action targets from the current snapshot", () => { + expect( + rebindActionTargetsToSnapshot( + [ + { + selector: "xpath=/html/body/button", + description: "Submit", + method: "click", + arguments: [], + target: { frameOrdinal: 0, backendNodeId: 12 }, + }, + ], + { "0-99": "/html/body/button" }, + ), + ).toStrictEqual([ + { + selector: "xpath=/html/body/button", + description: "Submit", + method: "click", + arguments: [], + target: { frameOrdinal: 0, backendNodeId: 99 }, + }, + ]); + }); +}); diff --git a/packages/server/tests/locator-target-guard.test.ts b/packages/server/tests/locator-target-guard.test.ts index 6a2735fea..3e69a913d 100644 --- a/packages/server/tests/locator-target-guard.test.ts +++ b/packages/server/tests/locator-target-guard.test.ts @@ -18,42 +18,76 @@ vi.mock("../understudy/selectorResolver.js", () => ({ describe("locator target guard", () => { it("validates and mutates the same resolved object", async () => { resolveAtIndex.mockResolvedValueOnce({ nodeId: null, objectId: "object-12" }); - const send = sessionSendForNode(12); - const locator = new Locator(frameWithSession(send), "xpath=/html/body/button").withTargetGuard( - { frameOrdinal: 0, backendNodeId: 12 }, - 0, - ); + const send = sessionSendForNode(12, 12); + const locator = new Locator( + frameWithSession(send, 12), + "xpath=/html/body/button", + ).withTargetGuard({ frameOrdinal: 0, backendNodeId: 12 }, 0); await locator.click(); expect(send).toHaveBeenCalledWith("DOM.describeNode", { objectId: "object-12" }); expect(send).toHaveBeenCalledWith("DOM.getBoxModel", { objectId: "object-12" }); + expect(send).toHaveBeenCalledWith("Input.dispatchMouseEvent", expect.anything()); }); it("rejects a replacement before dispatching input", async () => { resolveAtIndex.mockResolvedValueOnce({ nodeId: null, objectId: "object-99" }); - const send = sessionSendForNode(99); - const locator = new Locator(frameWithSession(send), "xpath=/html/body/button").withTargetGuard( - { frameOrdinal: 0, backendNodeId: 12 }, - 0, - ); + const send = sessionSendForNode(99, 99); + const locator = new Locator( + frameWithSession(send, 99), + "xpath=/html/body/button", + ).withTargetGuard({ frameOrdinal: 0, backendNodeId: 12 }, 0); await expect(locator.click()).rejects.toBeInstanceOf(ActionTargetMismatchError); expect(send).not.toHaveBeenCalledWith("Input.dispatchMouseEvent", expect.anything()); }); + + it("rejects when the hit-tested node under the cursor differs after geometry", async () => { + resolveAtIndex.mockResolvedValueOnce({ nodeId: null, objectId: "object-12" }); + const send = sessionSendForNode(12, 99); + const locator = new Locator( + frameWithSession(send, 99), + "xpath=/html/body/button", + ).withTargetGuard({ frameOrdinal: 0, backendNodeId: 12 }, 0); + + await expect(locator.click()).rejects.toBeInstanceOf(ActionTargetMismatchError); + + expect(send).not.toHaveBeenCalledWith("Input.dispatchMouseEvent", expect.anything()); + }); + + it("rejects guarded centroid when the hit-tested node differs", async () => { + resolveAtIndex.mockResolvedValueOnce({ nodeId: null, objectId: "object-12" }); + const send = sessionSendForNode(12, 99); + const locator = new Locator( + frameWithSession(send, 99), + "xpath=/html/body/button", + ).withTargetGuard({ frameOrdinal: 0, backendNodeId: 12 }, 0); + + await expect(locator.centroid()).rejects.toBeInstanceOf(ActionTargetMismatchError); + }); }); -function frameWithSession(send: ReturnType): Frame { - return { session: { send } } as unknown as Frame; +function frameWithSession(send: ReturnType, hitBackendNodeId: number): Frame { + return { + session: { send }, + getNodeAtLocation: vi.fn(async () => ({ backendNodeId: hitBackendNodeId })), + } as unknown as Frame; } -function sessionSendForNode(backendNodeId: number) { +function sessionSendForNode(resolvedBackendNodeId: number, hitBackendNodeId: number) { return vi.fn(async (method: string) => { - if (method === "DOM.describeNode") return { node: { backendNodeId } }; + if (method === "DOM.describeNode") return { node: { backendNodeId: resolvedBackendNodeId } }; if (method === "DOM.getBoxModel") { return { model: { content: [0, 0, 20, 0, 20, 20, 0, 20] } }; } + if (method === "DOM.resolveNode") { + return { object: { objectId: `hit-${hitBackendNodeId}` } }; + } + if (method === "Runtime.callFunctionOn") { + return { result: { value: hitBackendNodeId === resolvedBackendNodeId } }; + } return {}; }); } diff --git a/packages/server/understudy/locator.ts b/packages/server/understudy/locator.ts index 53d167d08..b99335103 100644 --- a/packages/server/understudy/locator.ts +++ b/packages/server/understudy/locator.ts @@ -195,6 +195,8 @@ export class Locator { /** * Return the center of the element's bounding box in the owning frame's viewport * (CSS pixels), rounded to integers. Scrolls into view best-effort. + * When a target guard is bound, also verifies the centroid still hit-tests + * this element before returning coordinates for pointer input. */ public async centroid(): Promise<{ x: number; y: number }> { const session = this.frame.session; @@ -206,12 +208,28 @@ export class Locator { }); if (!box.model) throw new Error(`Element not visible (no box model): ${this.selector}`); const { cx, cy } = this.centerFromBoxContent(box.model.content); + await this.assertPointerTargetMatchesGuard(cx, cy, objectId); return { x: Math.round(cx), y: Math.round(cy) }; } finally { await session.send("Runtime.releaseObject", { objectId }).catch(() => {}); } } + /** + * When a target guard is bound, re-resolve and ensure (x, y) still hit-tests + * this element. No-op without a guard. Used immediately before coordinate + * pointer sequences (e.g. drag-and-drop) after geometry was computed earlier. + */ + public async assertPointerTargetAt(x: number, y: number): Promise { + if (!this.targetGuard) return; + const { objectId } = await this.resolveNode(); + try { + await this.assertPointerTargetMatchesGuard(x, y, objectId); + } finally { + await this.frame.session.send("Runtime.releaseObject", { objectId }).catch(() => {}); + } + } + /** * Highlight the element's bounding box using the CDP Overlay domain. * - Scrolls element into view best-effort. @@ -300,6 +318,8 @@ export class Locator { if (!box.model) throw new Error(`Element not visible (no box model): ${this.selector}`); const { cx, cy } = this.centerFromBoxContent(box.model.content); + await this.assertPointerTargetMatchesGuard(cx, cy, objectId); + await session.send("Input.dispatchMouseEvent", { type: "mouseMoved", x: cx, @@ -337,6 +357,10 @@ export class Locator { if (!box.model) throw new Error(`Element not visible (no box model): ${this.selector}`); const { cx, cy } = this.centerFromBoxContent(box.model.content); + // Coordinate clicks hit-test live pixels; re-check the guard against the + // node currently under the cursor before dispatching input. + await this.assertPointerTargetMatchesGuard(cx, cy, objectId); + // Dispatch click events in a pipelined burst to reduce inter-click delay // from network/CPU jitter between round trips. const dispatches: Array> = []; @@ -821,6 +845,56 @@ export class Locator { } } + /** + * Coordinate-based input hit-tests live pixels. After geometry is computed, + * ensure the node under the cursor is still the guarded element (or a + * descendant), otherwise a rerender/overlay can steal the click. + */ + private async assertPointerTargetMatchesGuard( + x: number, + y: number, + expectedObjectId: Protocol.Runtime.RemoteObjectId, + ): Promise { + if (!this.targetGuard) return; + + await this.validateTarget(expectedObjectId); + + const hit = await this.frame.getNodeAtLocation(Math.round(x), Math.round(y)); + const { object } = await this.frame.session.send<{ + object: { objectId?: Protocol.Runtime.RemoteObjectId }; + }>("DOM.resolveNode", { backendNodeId: hit.backendNodeId }); + const hitObjectId = object.objectId; + if (!hitObjectId) { + throw new ActionTargetMismatchError(this.targetGuard.expected, { + frameOrdinal: this.targetGuard.frameOrdinal, + backendNodeId: hit.backendNodeId, + }); + } + + try { + const res = await this.frame.session.send( + "Runtime.callFunctionOn", + { + objectId: expectedObjectId, + functionDeclaration: + "function(hit) { return this === hit || (typeof this.contains === 'function' && this.contains(hit)); }", + arguments: [{ objectId: hitObjectId }], + returnByValue: true, + }, + ); + if (!res.result?.value) { + throw new ActionTargetMismatchError(this.targetGuard.expected, { + frameOrdinal: this.targetGuard.frameOrdinal, + backendNodeId: hit.backendNodeId, + }); + } + } finally { + await this.frame.session + .send("Runtime.releaseObject", { objectId: hitObjectId }) + .catch(() => {}); + } + } + /** * Resolve all matching nodes for this locator. * If the locator is narrowed via nth(), only that index is returned. From 0d6e05009d99f705b3491a898f6adb2e73ab49ee Mon Sep 17 00:00:00 2001 From: Vishal Anton Date: Mon, 27 Jul 2026 02:08:40 +0530 Subject: [PATCH 4/5] Refactor action target error handling and enhance validation --- packages/server/actionTarget.ts | 5 +- packages/server/errors.ts | 11 ++ .../handlers/handlerUtils/actHandlerUtils.ts | 39 ++++--- packages/server/services/cacheService.ts | 4 +- .../tests/action-target-validation.test.ts | 34 +++++- packages/server/tests/action-target.test.ts | 102 ++++++++++++++++++ .../server/tests/locator-target-guard.test.ts | 11 ++ packages/server/understudy/locator.ts | 11 +- 8 files changed, 194 insertions(+), 23 deletions(-) diff --git a/packages/server/actionTarget.ts b/packages/server/actionTarget.ts index 5bf2f9bbe..7e1fe37db 100644 --- a/packages/server/actionTarget.ts +++ b/packages/server/actionTarget.ts @@ -1,4 +1,5 @@ import type { Action, ActionTarget } from "../protocol/types.js"; +import { CachedActionRebindError } from "./errors.js"; import type { EncodedId } from "./types/private/internal.js"; import { trimTrailingTextNode } from "./utils.js"; @@ -67,7 +68,7 @@ export function rebindActionTargetsToSnapshot( return actions.map((action) => { const target = actionTargetForSnapshotSelector(action.selector, xpathMap); if (!target) { - throw new Error(`Cached action no longer resolves: ${action.selector}`); + throw new CachedActionRebindError("selector"); } const destinationSelector = action.method === "dragAndDrop" ? action.arguments?.[0] : undefined; @@ -75,7 +76,7 @@ export function rebindActionTargetsToSnapshot( ? actionTargetForSnapshotSelector(destinationSelector, xpathMap) : undefined; if (destinationSelector && !destinationTarget) { - throw new Error(`Cached action argument no longer resolves: ${destinationSelector}`); + throw new CachedActionRebindError("argument"); } return { diff --git a/packages/server/errors.ts b/packages/server/errors.ts index d64dd03fc..1d335f581 100644 --- a/packages/server/errors.ts +++ b/packages/server/errors.ts @@ -4,3 +4,14 @@ export class TimeoutError extends Error { this.name = "TimeoutError"; } } + +export class CachedActionRebindError extends Error { + constructor(readonly kind: "selector" | "argument" = "selector") { + super( + kind === "argument" + ? "Cached action argument no longer resolves in the current page snapshot" + : "Cached action no longer resolves in the current page snapshot", + ); + this.name = "CachedActionRebindError"; + } +} diff --git a/packages/server/handlers/handlerUtils/actHandlerUtils.ts b/packages/server/handlers/handlerUtils/actHandlerUtils.ts index 325ae8711..f0469792a 100644 --- a/packages/server/handlers/handlerUtils/actHandlerUtils.ts +++ b/packages/server/handlers/handlerUtils/actHandlerUtils.ts @@ -198,11 +198,15 @@ async function scrollByPixelOffset(ctx: UnderstudyMethodHandlerContext): Promise async function wheelScroll(ctx: UnderstudyMethodHandlerContext): Promise { const { locator, frame, args, logger } = ctx; - const { objectId } = await locator.resolveNode(); - await locator - .getFrame() - .session.send("Runtime.releaseObject", { objectId }) - .catch(() => {}); + // Only resolve when an observed target guard is present; legacy wheel keeps + // the page-level (0,0) path without requiring the selector to resolve. + if (locator.targetGuard) { + const { objectId } = await locator.resolveNode(); + await locator + .getFrame() + .session.send("Runtime.releaseObject", { objectId }) + .catch(() => {}); + } const deltaY = Number(args[0] ?? 200); logger.debug("Dispatching mouse wheel", { @@ -253,17 +257,20 @@ async function pressKey(ctx: UnderstudyMethodHandlerContext): Promise { const { locator, args, xpath, page, logger } = ctx; const key = args[0] ?? ""; try { - // Resolve (and focus) so observed target guards run before key input. - const session = locator.getFrame().session; - const { objectId } = await locator.resolveNode(); - try { - await session.send("Runtime.callFunctionOn", { - objectId, - functionDeclaration: "function() { this.focus?.(); }", - returnByValue: true, - }); - } finally { - await session.send("Runtime.releaseObject", { objectId }).catch(() => {}); + // Only resolve/focus when an observed target guard is present. Legacy + // targetless press must keep sending keys to the currently focused element. + if (locator.targetGuard) { + const session = locator.getFrame().session; + const { objectId } = await locator.resolveNode(); + try { + await session.send("Runtime.callFunctionOn", { + objectId, + functionDeclaration: "function() { this.focus?.(); }", + returnByValue: true, + }); + } finally { + await session.send("Runtime.releaseObject", { objectId }).catch(() => {}); + } } logger.debug("Pressing key", { diff --git a/packages/server/services/cacheService.ts b/packages/server/services/cacheService.ts index f79cbcf41..bc8114eb4 100644 --- a/packages/server/services/cacheService.ts +++ b/packages/server/services/cacheService.ts @@ -150,9 +150,7 @@ function normalizeCachedActionTarget(value: unknown): ActionTarget | undefined { return parsed.success ? parsed.data : undefined; } -function normalizeCachedArgumentTargets( - value: unknown, -): Action["argumentTargets"] | undefined { +function normalizeCachedArgumentTargets(value: unknown): Action["argumentTargets"] | undefined { if (value === null || typeof value !== "object" || Array.isArray(value)) return undefined; const argumentTargets: NonNullable = {}; diff --git a/packages/server/tests/action-target-validation.test.ts b/packages/server/tests/action-target-validation.test.ts index 50d021a28..ed01a67e9 100644 --- a/packages/server/tests/action-target-validation.test.ts +++ b/packages/server/tests/action-target-validation.test.ts @@ -179,6 +179,34 @@ describe("action target validation", () => { expect(keyPress).not.toHaveBeenCalled(); }); + + it("keeps targetless press on the focused element without resolving the locator", async () => { + const resolveNode = vi.fn(async () => { + throw new Error("should not resolve for targetless press"); + }); + const keyPress = vi.fn(); + resolveLocator.mockResolvedValue( + locatorForNode( + 0, + 99, + vi.fn(), + vi.fn(async () => 99), + resolveNode, + ), + ); + + await performUnderstudyMethod( + Object.assign(pageWithOrdinals(), { keyPress }) as unknown as Page, + rootFrame(), + "press", + "xpath=/html/body/stale", + ["Enter"], + logger, + ); + + expect(resolveNode).not.toHaveBeenCalled(); + expect(keyPress).toHaveBeenCalledWith("Enter"); + }); }); function rootFrame(): Frame { @@ -231,8 +259,12 @@ function locatorForNode( click, centroid, assertPointerTargetAt: vi.fn(async () => undefined), - withTargetGuard: (expected: { frameOrdinal: number; backendNodeId: number }) => ({ + withTargetGuard: ( + expected: { frameOrdinal: number; backendNodeId: number }, + frameOrdinal: number = expected.frameOrdinal, + ) => ({ ...locator, + targetGuard: { expected, frameOrdinal }, resolveNode: async () => { await validate(expected); return await resolveNode(); diff --git a/packages/server/tests/action-target.test.ts b/packages/server/tests/action-target.test.ts index 38335c08b..531c9c5da 100644 --- a/packages/server/tests/action-target.test.ts +++ b/packages/server/tests/action-target.test.ts @@ -4,6 +4,7 @@ import { rebindActionTargetsToSnapshot, selectorAndTargetForSnapshotElement, } from "../actionTarget.js"; +import { CachedActionRebindError } from "../errors.js"; describe("actionTarget helpers", () => { it("maps document-root selectors to the HTML element identity", () => { @@ -46,4 +47,105 @@ describe("actionTarget helpers", () => { }, ]); }); + + it("rebinds drag-and-drop destination argumentTargets from the current snapshot", () => { + expect( + rebindActionTargetsToSnapshot( + [ + { + selector: "xpath=/html/body/ul/li[1]", + description: "Drag source onto target", + method: "dragAndDrop", + arguments: ["xpath=/html/body/ul/li[2]"], + target: { frameOrdinal: 0, backendNodeId: 12 }, + argumentTargets: { "0": { frameOrdinal: 0, backendNodeId: 20 } }, + }, + ], + { + "0-30": "/html/body/ul/li[1]", + "0-31": "/html/body/ul/li[2]", + }, + ), + ).toStrictEqual([ + { + selector: "xpath=/html/body/ul/li[1]", + description: "Drag source onto target", + method: "dragAndDrop", + arguments: ["xpath=/html/body/ul/li[2]"], + target: { frameOrdinal: 0, backendNodeId: 30 }, + argumentTargets: { "0": { frameOrdinal: 0, backendNodeId: 31 } }, + }, + ]); + }); + + it("throws a sanitized error when a drag destination no longer resolves", () => { + expect(() => + rebindActionTargetsToSnapshot( + [ + { + selector: "xpath=/html/body/ul/li[1]", + description: "Drag source onto target", + method: "dragAndDrop", + arguments: ["xpath=/html/body/ul/li[2]"], + target: { frameOrdinal: 0, backendNodeId: 12 }, + argumentTargets: { "0": { frameOrdinal: 0, backendNodeId: 20 } }, + }, + ], + { "0-30": "/html/body/ul/li[1]" }, + ), + ).toThrow(CachedActionRebindError); + + try { + rebindActionTargetsToSnapshot( + [ + { + selector: "xpath=/html/body/ul/li[1]", + description: "Drag source onto target", + method: "dragAndDrop", + arguments: ["xpath=/html/body/missing"], + target: { frameOrdinal: 0, backendNodeId: 12 }, + }, + ], + { "0-30": "/html/body/ul/li[1]" }, + ); + } catch (error) { + expect(error).toBeInstanceOf(CachedActionRebindError); + expect((error as Error).message).not.toContain("xpath="); + expect((error as CachedActionRebindError).kind).toBe("argument"); + } + }); + + it("throws a sanitized error when the primary selector no longer resolves", () => { + expect(() => + rebindActionTargetsToSnapshot( + [ + { + selector: "xpath=/html/body/gone", + description: "Missing", + method: "click", + arguments: [], + }, + ], + { "0-30": "/html/body/ul/li[1]" }, + ), + ).toThrow(CachedActionRebindError); + + try { + rebindActionTargetsToSnapshot( + [ + { + selector: "xpath=/html/body/gone", + description: "Missing", + method: "click", + arguments: [], + }, + ], + { "0-30": "/html/body/ul/li[1]" }, + ); + } catch (error) { + expect(error).toBeInstanceOf(CachedActionRebindError); + expect((error as Error).message).not.toContain("xpath="); + expect((error as CachedActionRebindError).kind).toBe("selector"); + } + }); }); diff --git a/packages/server/tests/locator-target-guard.test.ts b/packages/server/tests/locator-target-guard.test.ts index 3e69a913d..61276d028 100644 --- a/packages/server/tests/locator-target-guard.test.ts +++ b/packages/server/tests/locator-target-guard.test.ts @@ -15,6 +15,12 @@ vi.mock("../understudy/selectorResolver.js", () => ({ }, })); +vi.mock("../understudy/executionContextRegistry.js", () => ({ + executionContexts: { + waitForLocatorWorld: vi.fn(async () => ({ contextId: 7 })), + }, +})); + describe("locator target guard", () => { it("validates and mutates the same resolved object", async () => { resolveAtIndex.mockResolvedValueOnce({ nodeId: null, objectId: "object-12" }); @@ -28,6 +34,10 @@ describe("locator target guard", () => { expect(send).toHaveBeenCalledWith("DOM.describeNode", { objectId: "object-12" }); expect(send).toHaveBeenCalledWith("DOM.getBoxModel", { objectId: "object-12" }); + expect(send).toHaveBeenCalledWith("DOM.resolveNode", { + backendNodeId: 12, + executionContextId: 7, + }); expect(send).toHaveBeenCalledWith("Input.dispatchMouseEvent", expect.anything()); }); @@ -71,6 +81,7 @@ describe("locator target guard", () => { function frameWithSession(send: ReturnType, hitBackendNodeId: number): Frame { return { + frameId: "frame-1", session: { send }, getNodeAtLocation: vi.fn(async () => ({ backendNodeId: hitBackendNodeId })), } as unknown as Frame; diff --git a/packages/server/understudy/locator.ts b/packages/server/understudy/locator.ts index b99335103..5eba4bb2d 100644 --- a/packages/server/understudy/locator.ts +++ b/packages/server/understudy/locator.ts @@ -23,6 +23,7 @@ import type { ActionTarget, MouseButton } from "../../protocol/types.js"; import { ActionTargetMismatchError } from "../actionTarget.js"; import type { SetInputFilesArgument } from "../types/private/fileUpload.js"; import type { NormalizedFilePayload } from "../types/private/locator.js"; +import { executionContexts } from "./executionContextRegistry.js"; const MAX_REMOTE_UPLOAD_BYTES = 50 * 1024 * 1024; // 50MB guard copied from Playwright @@ -860,9 +861,17 @@ export class Locator { await this.validateTarget(expectedObjectId); const hit = await this.frame.getNodeAtLocation(Math.round(x), Math.round(y)); + const { contextId } = await executionContexts.waitForLocatorWorld( + this.frame.session, + this.frame.frameId, + 1000, + ); const { object } = await this.frame.session.send<{ object: { objectId?: Protocol.Runtime.RemoteObjectId }; - }>("DOM.resolveNode", { backendNodeId: hit.backendNodeId }); + }>("DOM.resolveNode", { + backendNodeId: hit.backendNodeId, + executionContextId: contextId, + }); const hitObjectId = object.objectId; if (!hitObjectId) { throw new ActionTargetMismatchError(this.targetGuard.expected, { From 892a139b08e9e4bea0a0fc7202f3a5cccc1f3001 Mon Sep 17 00:00:00 2001 From: Vishal Anton Date: Mon, 27 Jul 2026 09:08:28 +0530 Subject: [PATCH 5/5] Enhance action target validation tests to handle stale targets and improve error handling for mouse wheel events --- .../tests/action-target-validation.test.ts | 71 +++++++++++++++++++ packages/server/tests/action-target.test.ts | 53 ++++---------- 2 files changed, 86 insertions(+), 38 deletions(-) diff --git a/packages/server/tests/action-target-validation.test.ts b/packages/server/tests/action-target-validation.test.ts index ed01a67e9..65dba08cd 100644 --- a/packages/server/tests/action-target-validation.test.ts +++ b/packages/server/tests/action-target-validation.test.ts @@ -207,6 +207,77 @@ describe("action target validation", () => { expect(resolveNode).not.toHaveBeenCalled(); expect(keyPress).toHaveBeenCalledWith("Enter"); }); + + it("resolves the guarded locator before mouse.wheel so stale targets fail closed", async () => { + const resolveNode = vi.fn(async () => { + throw new ActionTargetMismatchError( + { frameOrdinal: 0, backendNodeId: 12 }, + { frameOrdinal: 0, backendNodeId: 99 }, + ); + }); + const send = vi.fn(async () => ({})); + resolveLocator.mockResolvedValue( + locatorForNode( + 0, + 99, + vi.fn(), + vi.fn(async () => 99), + resolveNode, + ), + ); + + await expect( + performUnderstudyMethod( + pageWithOrdinals(), + { ...rootFrame(), session: { send } } as unknown as Frame, + "mouse.wheel", + "xpath=/html/body/scrollable", + ["200"], + logger, + undefined, + { target: { frameOrdinal: 0, backendNodeId: 12 } }, + ), + ).rejects.toBeInstanceOf(ActionTargetMismatchError); + + expect(send).not.toHaveBeenCalledWith("Input.dispatchMouseEvent", expect.anything()); + }); + + it("keeps targetless mouse.wheel on the page without resolving the locator", async () => { + const resolveNode = vi.fn(async () => { + throw new Error("should not resolve for targetless mouse.wheel"); + }); + const send = vi.fn(async () => ({})); + resolveLocator.mockResolvedValue( + locatorForNode( + 0, + 99, + vi.fn(), + vi.fn(async () => 99), + resolveNode, + ), + ); + + await performUnderstudyMethod( + pageWithOrdinals(), + { ...rootFrame(), session: { send } } as unknown as Frame, + "mouse.wheel", + "xpath=/html/body/stale", + ["200"], + logger, + ); + + expect(resolveNode).not.toHaveBeenCalled(); + expect(send).toHaveBeenCalledWith( + "Input.dispatchMouseEvent", + expect.objectContaining({ + type: "mouseWheel", + x: 0, + y: 0, + deltaY: 200, + deltaX: 0, + }), + ); + }); }); function rootFrame(): Frame { diff --git a/packages/server/tests/action-target.test.ts b/packages/server/tests/action-target.test.ts index 531c9c5da..20faea570 100644 --- a/packages/server/tests/action-target.test.ts +++ b/packages/server/tests/action-target.test.ts @@ -79,7 +79,8 @@ describe("actionTarget helpers", () => { }); it("throws a sanitized error when a drag destination no longer resolves", () => { - expect(() => + let error: unknown; + try { rebindActionTargetsToSnapshot( [ { @@ -92,44 +93,18 @@ describe("actionTarget helpers", () => { }, ], { "0-30": "/html/body/ul/li[1]" }, - ), - ).toThrow(CachedActionRebindError); - - try { - rebindActionTargetsToSnapshot( - [ - { - selector: "xpath=/html/body/ul/li[1]", - description: "Drag source onto target", - method: "dragAndDrop", - arguments: ["xpath=/html/body/missing"], - target: { frameOrdinal: 0, backendNodeId: 12 }, - }, - ], - { "0-30": "/html/body/ul/li[1]" }, ); - } catch (error) { - expect(error).toBeInstanceOf(CachedActionRebindError); - expect((error as Error).message).not.toContain("xpath="); - expect((error as CachedActionRebindError).kind).toBe("argument"); + } catch (e) { + error = e; } + + expect(error).toBeInstanceOf(CachedActionRebindError); + expect((error as Error).message).not.toContain("xpath="); + expect((error as CachedActionRebindError).kind).toBe("argument"); }); it("throws a sanitized error when the primary selector no longer resolves", () => { - expect(() => - rebindActionTargetsToSnapshot( - [ - { - selector: "xpath=/html/body/gone", - description: "Missing", - method: "click", - arguments: [], - }, - ], - { "0-30": "/html/body/ul/li[1]" }, - ), - ).toThrow(CachedActionRebindError); - + let error: unknown; try { rebindActionTargetsToSnapshot( [ @@ -142,10 +117,12 @@ describe("actionTarget helpers", () => { ], { "0-30": "/html/body/ul/li[1]" }, ); - } catch (error) { - expect(error).toBeInstanceOf(CachedActionRebindError); - expect((error as Error).message).not.toContain("xpath="); - expect((error as CachedActionRebindError).kind).toBe("selector"); + } catch (e) { + error = e; } + + expect(error).toBeInstanceOf(CachedActionRebindError); + expect((error as Error).message).not.toContain("xpath="); + expect((error as CachedActionRebindError).kind).toBe("selector"); }); });