Skip to content

fix: let selector arguments accept an explicit null - #36

Open
vishalhabib99 wants to merge 1 commit into
CursorTouch:masterfrom
vishalhabib99:fix/selector-args-accept-null
Open

vishalhabib99 wants to merge 1 commit into
CursorTouch:masterfrom
vishalhabib99:fix/selector-args-accept-null

Conversation

@vishalhabib99

Copy link
Copy Markdown

ClickBySelector and WaitForElement type their four selector arguments (text, resourceId, className, description) as str with a None default. The generated schema therefore advertises "default": null on a field whose type is "string". A client or agent framework that sends that default back explicitly gets a validation error, and the tool never runs.

Repro (offline, no device needed; FastMCP's in-memory Client on current master, 0640a5d):

async with Client(mcp) as c:
    await c.call_tool("WaitForElement", {"text": "OK", "resourceId": None}, raise_on_error=False)
1 validation error for call[wait_for_element_tool]
resourceId
  Input should be a valid string [type=string_type, input_value=None, input_type=NoneType]

ClickBySelector fails the same way. Both tools already treat a missing selector as "not given" (if text: ...), so null should mean the same thing.

Change: the 8 arguments are now str | None = None. Nothing else changes.

Tests (tests/unit/test_tool_null_args.py):

  • every tool parameter that defaults to None allows null in its schema (enumerates all tools, so a new one can't slip back in)
  • both tools get past argument validation with explicit nulls (the device lookup is stubbed, so no device is needed)

All 3 fail on master and pass with the fix. Full suite: 33 passed. Ruff on src/: 46 → 38 findings (the 8 RUF013 implicit-Optional warnings on these arguments are gone; nothing new).

Same fix as CursorTouch/MacOS-MCP#69, found the same way (mcp-doctor's none_default_type check, then reproduced).

ClickBySelector and WaitForElement typed text, resourceId, className and
description as str with a None default, so the schema advertised
"default": null on a field whose type excludes null. A client that sends
that default back got a validation error before the tool ran.

Type them as str | None and add tests: every parameter defaulting to None
allows null, and both tools get past validation with explicit nulls.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant