Skip to content

fix(tools): pass time parameter to page.evaluate in waitForTimeout#41037

Merged
pavelfeldman merged 1 commit into
microsoft:mainfrom
SebTardif:fix/wait-for-timeout-ignores-time
May 29, 2026
Merged

fix(tools): pass time parameter to page.evaluate in waitForTimeout#41037
pavelfeldman merged 1 commit into
microsoft:mainfrom
SebTardif:fix/wait-for-timeout-ignores-time

Conversation

@SebTardif
Copy link
Copy Markdown
Contributor

waitForTimeout(time) has two branches:

  • Blocked path (dialog active): correctly uses the time parameter
  • Normal path: hardcodes 1000ms, ignoring time

Both callers in utils.ts pass 500, so every waitForCompletion cycle (click, fill, navigate, and other interactive tools) waits 1000ms instead of the intended 500ms. This adds an unnecessary 500ms to each tool call.

The fix passes the time parameter through to page.evaluate, making the normal path consistent with the blocked path.

Before:

await this.page.evaluate(() => new Promise(f => setTimeout(f, 1000))).catch(() => {});

After:

await this.page.evaluate(ms => new Promise(f => setTimeout(f, ms)), time).catch(() => {});

This was introduced in #37286 when MCP tools were moved into the monorepo.

waitForTimeout(time) had two branches: the blocked-path branch
correctly used the time parameter, but the normal path hardcoded
1000ms. All callers pass 500, so every waitForCompletion cycle
waited 2x longer than intended.

This was introduced in microsoft#37286 when MCP tools were moved into the
monorepo.

Signed-off-by: Sebastien Tardif <sebtardif@ncf.ca>
@github-actions
Copy link
Copy Markdown
Contributor

Test results for "MCP"

1 failed
❌ [chrome] › mcp/cli-killall.spec.ts:42 › kill-all kills filtered dashboard pid @mcp-windows-latest-chrome

7206 passed, 1113 skipped


Merge workflow run.

@pavelfeldman pavelfeldman merged commit 37a5aa6 into microsoft:main May 29, 2026
16 of 18 checks passed
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.

2 participants