feat: make pageId routing always-on#1777
feat: make pageId routing always-on#1777bcfmtolgahan wants to merge 4 commits intoChromeDevTools:mainfrom
Conversation
Remove the --experimental-page-id-routing gate so pageId is always exposed on page-scoped tools. Update evaluate_script which had its own duplicate gate. Fix docs generator to include pageId for page-scoped tools. Regenerate cliDefinitions.ts and tool-reference.md. - Remove experimentalPageIdRouting conditions from schema injection and page resolution in index.ts - Mark --experimental-page-id-routing CLI flag as deprecated - Improve pageIdSchema description to be more helpful - Remove evaluate_script's own experimentalPageIdRouting gate; also fixes a latent bug where getPageById(undefined) was called when the flag was on but pageId was absent - Fix pageId type in script.test.ts (string -> number) - Fix generate-docs.ts to inject pageIdSchema for page-scoped tools so tool-reference.md accurately reflects available parameters
bcfmtolgahan
force-pushed
the
feat/pageid-routing-always-on
branch
from
cceb6af to
394fbd2
Compare
| describe: | ||
| 'Whether to expose pageId on page-scoped tools and route requests by page ID.', | ||
| '(Deprecated, now always enabled) pageId is always exposed on page-scoped tools.', | ||
| hidden: true, |
There was a problem hiding this comment.
so we have options:
- remove the flag altogether
- remove hidden: true an experimental prefix.
If we keep the flag, I think we can decide to keep it off by default or keep an option to turn off pageId routing to save tokens.
cc @natorion wdyt?
There was a problem hiding this comment.
The overhead is only the resident token cost of having pageId in help, right? The parameter per-se is optional.
In CLI scenarios I expect such a resident token cost to be miniscule compared to whatever is returned via take_snapshot.
There was a problem hiding this comment.
It's optional but it is included on each llm iteration even if no heavy output like take_snapshot is used.
There was a problem hiding this comment.
PR #1780 addresses the wait helpers, waitForEventsAfterAction is now on McpPage and each tool uses the correct page's throttling settings.
On the flag: happy to go either way. If keeping it as an opt-out for token savings makes sense, I can rewire index.ts to respect it as a disable toggle and rename it (e.g. --noPageIdRouting). If the consensus is that the overhead is acceptable, I'll remove it entirely. What's your preference?
There was a problem hiding this comment.
We need some time to evaluate if the overhead is acceptable. For now, let's keep the flag in case some users are not happy with the overhead or in case the pageId routing does not work as expected. There is no need to rename to --noPageIdRouting as yargs automatically handles this for boolean args. Right now you can already use --noExperimentalPageRouting, so I would say let's keep the flag, drop the experimental prefix, remove hidden: true and set default to true. If we find the token overhead to be too much, we can update the arg to default to false. Or users can use --no-page-id-routing or --page-id-routing=false.
There was a problem hiding this comment.
Done. Dropped the experimental prefix, removed hidden: true, set default: true. Users can opt out with --no-page-id-routing.
|
TODO: this cannot land before the fixes to the wait helpers are made to correctly operate on the specified page. |
|
TODO: evaluate that the model uses pageId correctly via eval scenarios in https://github.com/ChromeDevTools/chrome-devtools-mcp/tree/main/scripts/eval_scenarios |
Per reviewer feedback: drop the experimental prefix, remove hidden: true, set default to true so pageId routing is on by default. Users can opt out with --no-page-id-routing. Flag is kept as an opt-out toggle in case token overhead is unacceptable or routing does not work as expected.
When pageIdRouting is true but no pageId is provided, getPageById(undefined) was called which always throws. Mirror the same guard used in index.ts: only route by pageId when both the flag is on AND a pageId is present.
Splits out from #1244 per review feedback. 'waitForEventsAfterAction' previously lived in 'McpContext' and always used the selected page's CPU/network throttling settings. With pageId routing, a tool can target a different page than the selected one, meaning wrong throttling multipliers were applied. Moving the method to 'McpPage' fixes this: each tool now calls 'page.waitForEventsAfterAction(...)' and gets the correct page's emulation settings. 'getNetworkMultiplierFromString' is extracted to 'WaitForHelper.ts' to avoid a circular import (McpContext → McpPage already exists). Unblocks #1777.
Splits out from #1244 per review feedback. 'waitForEventsAfterAction' previously lived in 'McpContext' and always used the selected page's CPU/network throttling settings. With pageId routing, a tool can target a different page than the selected one, meaning wrong throttling multipliers were applied. Moving the method to 'McpPage' fixes this: each tool now calls 'page.waitForEventsAfterAction(...)' and gets the correct page's emulation settings. 'getNetworkMultiplierFromString' is extracted to 'WaitForHelper.ts' to avoid a circular import (McpContext → McpPage already exists). Unblocks #1777.
3 participants
Splits out from #1244 per review feedback.
Removes the
experimentalPageIdRoutingflag —pageIdis now alwaysexposed on page-scoped tools. The flag is kept as a no-op for backwards
compatibility.