fix(mcp): stop the default timeout silently shortening a wait pause

[Avinash Gola (avinashgola)]

The bug

wait with for="time" capped the requested pause at timeout:

let timeout = clamp_timeout(args.timeout, DEFAULT_WAIT_TIMEOUT_MS, MAX_WAIT_TIMEOUT_MS);
let wait_ms = parse_wait_ms(value.as_deref(), DEFAULT_PAUSE_MS).min(timeout);

DEFAULT_WAIT_TIMEOUT_MS and DEFAULT_PAUSE_MS are both 2_000. So the cap never bites on a default call — it only bites when the caller explicitly asks for something longer, which is exactly when they meant it. And it bites silently:

wait { page, for: "time", value: 10000 }  ->  "waited 2000ms",  matched: true

A fifth of the requested pause, reported as a success. Nothing in the description mentions the cap — it says for="time" (default) pauses value ms (default 2000). 10s is well inside the tool's own 30s MAX_WAIT_TIMEOUT_MS; only the default of an unrelated argument blocked it.

The existing contract case is already named "wait: for=time elapses at least the requested duration" but only exercised value: 800, under the default — so the invariant it names was never actually tested at a value that could break it.

The fix

The requested pause governs. A caller-supplied timeout still caps it, because that is their own explicit ceiling, and MAX_WAIT_TIMEOUT_MS stays the hard limit either way.

call	before	after
value: 10000	2000	10000
value: 10000, timeout: 5000	5000	5000
value: 800, timeout: 5000	800	800
value: 120000	2000	30000 (tool max)
neither	2000	2000

The timeout doc comment now says what it actually does, rather than leaving the interaction undocumented.

for="text" and for="selector" are untouched — timeout is their deadline and always was.

Tests

wait_for_time_honours_a_pause_longer_than_the_default_timeout drives the tool through execute_tool under #[tokio::test(start_paused = true)], so it asserts real handler behaviour and still runs instantly. On the old logic:

assertion `left == right` failed: an explicit pause should not be cut down to the default timeout: waited 2000ms
  left: Some(Number(2000))
 right: Some(Number(10000))

Plus table-driven unit coverage of pause_duration_ms for every row above, and the contract case extended to a 3s pause — the case that was broken.

cargo test -p browseros-mcp                              79 passed, 0 failed
cargo clippy --workspace --all-targets -- -D warnings    clean
cargo fmt --all --check                                  clean
bunx tsc --noEmit                                        clean
biome check <contract file>                              clean

Not run: the gated BROWSEROS_BINARY contract suite — no local BrowserOS build — so the extended contract case is unverified against a real browser. The Rust-side behaviour it mirrors is covered by the tests above.

🤖 Generated with Claude Code

[greptile-apps]

PR author is not in the allowed authors list.