-
Notifications
You must be signed in to change notification settings - Fork 17
perf: exclude browser tools and reduce turns in smoke-copilot #1625
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -244,6 +244,37 @@ for (const workflowPath of workflowPaths) { | |
| console.log(` Replaced ${imageTagMatches.length} --image-tag/--skip-pull with --build-local`); | ||
| } | ||
|
|
||
| // Exclude unused Playwright/browser tools from Copilot CLI for smoke-copilot. | ||
| // The Copilot CLI includes 21 built-in browser_* tools when --allow-all-tools is set. | ||
| // These tools are never used in smoke-copilot but add ~10,500 tokens/turn of dead weight. | ||
| // We inject --excluded-tools after --allow-all-tools to suppress them. | ||
| const isCopilotSmoke = workflowPath.includes('smoke-copilot.lock.yml'); | ||
| if (isCopilotSmoke) { | ||
| const excludedToolsFlag = | ||
| '--excluded-tools=browser_close,browser_resize,browser_console_messages,' + | ||
| 'browser_handle_dialog,browser_evaluate,browser_file_upload,browser_fill_form,' + | ||
| 'browser_press_key,browser_type,browser_navigate,browser_navigate_back,' + | ||
| 'browser_network_requests,browser_run_code,browser_take_screenshot,' + | ||
| 'browser_snapshot,browser_click,browser_drag,browser_hover,' + | ||
| 'browser_select_option,browser_tabs,browser_wait_for'; | ||
| // First, strip any existing --excluded-tools flag to make this idempotent | ||
| const existingExcludedRegex = / --excluded-tools=[^\s'"]*/g; | ||
| const existingMatches = content.match(existingExcludedRegex); | ||
| if (existingMatches) { | ||
| content = content.replace(existingExcludedRegex, ''); | ||
| console.log(` Removed ${existingMatches.length} existing --excluded-tools flag(s)`); | ||
| } | ||
| const allowAllToolsCount = (content.match(/--allow-all-tools/g) || []).length; | ||
| if (allowAllToolsCount > 0) { | ||
| content = content.replace( | ||
| /--allow-all-tools/g, | ||
| `--allow-all-tools ${excludedToolsFlag}` | ||
| ); | ||
|
Comment on lines
+251
to
+272
|
||
| modified = true; | ||
| console.log(` Injected --excluded-tools (21 browser tools) in ${allowAllToolsCount} location(s)`); | ||
| } | ||
| } | ||
|
|
||
| // Remove unused "Setup Scripts" step from update_cache_memory jobs. | ||
| // The step downloads a private action but is never used in these jobs, | ||
| // causing 401 Unauthorized failures when permissions: {} is set. | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The prompt claims “MCP connectivity confirmed” based on the pre-step, but the pre-step fetches PRs via
gh pr list(GitHub API), not via the GitHub MCP server/tools. This change removes the only explicit GitHub MCP tool invocation, so the workflow may no longer detect GitHub MCP breakage. Either reintroduce a minimal GitHub MCP call for this section, or rename/reword the section to reflect that only GitHub API connectivity was pre-validated.See below for a potential fix: