Skip to content

fix(skills): narrow npm and npx grants - #42789

Open
Devin Rousso (dcrousso) wants to merge 1 commit into
microsoft:mainfrom
dcrousso:fix-42745
Open

Devin Rousso (dcrousso) wants to merge 1 commit into
microsoft:mainfrom
dcrousso:fix-42745

Conversation

@dcrousso

Copy link
Copy Markdown
Contributor

the CLI and trace skills preapprove unrelated npm and npx commands

limit automatic approval to Playwright commands

fix #42745

the CLI and trace skills preapprove unrelated `npm` and `npx` commands

limit automatic approval to Playwright commands
@github-actions

Copy link
Copy Markdown
Contributor

Test results for "MCP"

2 failed
❌ [chrome] › mcp/cli-webmcp.spec.ts:188 › webmcp-call disambiguates same-name tools in identical same-origin frames @mcp-macos-latest-chrome
❌ [chrome] › mcp/cli-webmcp.spec.ts:162 › webmcp-call disambiguates duplicate tool names by frame @mcp-ubuntu-latest-chrome

8690 passed, 1474 skipped


Merge workflow run.

@github-actions

Copy link
Copy Markdown
Contributor

🟢 Both failures are a pre-existing WebMCP flake — this PR is clear

Hi, I'm the Playwright bot and I took a look at the red CI here.

The two mcp/cli-webmcp.spec.ts failures also fail on plain main and on other PRs, so they're not on you. This PR only edits allowed-tools: frontmatter in two SKILL.md files plus the two tests that assert on it — nothing that the WebMCP frame-registration path touches.

Details

Pre-existing flake / infra

  • [chrome] › mcp/cli-webmcp.spec.ts:188 › webmcp-call disambiguates same-name tools in identical same-origin frames and [chrome] › mcp/cli-webmcp.spec.ts:162 › webmcp-call disambiguates duplicate tool names by frame — same story, one flake. Both flip verdict across runs on SHAs this PR can't be responsible for:

    where sha which test
    push on main 07f1a615 (this PR's base) same-name tools, mcp-ubuntu-latest-chrome
    #42788 1f166c07 same-name tools, mcp-ubuntu-latest-chromium
    #42785 fabe18a1 same-name tools, mcp-macos-latest-chrome

    Run history for the two bots that went red here (oldest first, one square per run):

    same-name tools on mcp-macos-latest-chrome
    🟩🟩🟩🟩🟩🟩🟩🟥🟩🟩🟩🟩🟥

    duplicate tool names on mcp-ubuntu-latest-chrome
    🟩🟩🟩🟩🟩🟩🟩🟩🟩🟩🟩🟩🟩🟥

    The failure signature is the same race every time — the second frame's tool registration hasn't landed when the assertion runs, so the tool list is short:

    Expected substring: "- frame: http://localhost:8907/widget.html (frame 1)"
    Received string:    "### Result
    Found 1 WebMCP tool(s). ..."
    

    and in the duplicate tool names case the call succeeds instead of reporting the ambiguity, because only one frame had registered echo by then. Both are the same underlying timing problem, one bot each.

Why the diff can't reach it

The four changed files are playwright-cli/SKILL.md and playwright-trace/SKILL.md (only the allowed-tools: line and one sentence of prose), plus assertions in tests/mcp/cli-misc.spec.ts and tests/mcp/init-agents.spec.ts. No product code, and nothing shared with the WebMCP tool-registration path under test.

I'm a first pass, so treat this as a strong lean rather than the last word — but the same test going red on the base main SHA is about as clean as this evidence gets.

Triaged by the Playwright bot - agent run

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.

[BUG] bundled skills grant Bash(npx:*) / Bash(npm:*), far wider than anything they invoke

1 participant