Skip to content

[Chore] Unskip VS Code e2e replay for use_mcp_tool#93

Merged
edelauna merged 4 commits into
mainfrom
chore-unskip-e2e-use-mcp-tool
May 19, 2026
Merged

[Chore] Unskip VS Code e2e replay for use_mcp_tool#93
edelauna merged 4 commits into
mainfrom
chore-unskip-e2e-use-mcp-tool

Conversation

@roomote
Copy link
Copy Markdown
Contributor

@roomote roomote Bot commented May 13, 2026

Opened on behalf of Elliott de Launay. View the task or mention @roomote for follow-up asks.

What problem this solves

Fixes an issue where the use_mcp_tool VS Code e2e suite still stayed skipped, which left MCP filesystem delegation unverified in the deterministic replay flow.

Why this change was made

This slice rewrites the skipped MCP suite around stable fixture tags embedded in real prompts, dynamic fixture registration, and a real filesystem MCP server wired into the temporary test workspace. It also pins the filesystem MCP server package and keeps the error-path assertions focused on MCP behavior instead of brittle assistant phrasing.

User impact

No direct product behavior changes. The MCP e2e coverage becomes replayable and helps catch regressions in the MCP delegation path earlier.

Related PRs

@roomote roomote Bot added the roomote:auto-resolve-conflicts Allow Roomote to auto-resolve merge conflicts for this PR label May 13, 2026
@roomote
Copy link
Copy Markdown
Contributor Author

roomote Bot commented May 13, 2026

No actionable issues found. See task

  • use-mcp-tool.test.ts no longer hard-codes requested file in the read_file completion assertion, so the suite no longer depends on one assistant phrase after the MCP read succeeds.
  • The unknown-server case no longer assumes partial MCP approval messages keep serverName and toolName empty before validation fails.
  • The suite pins @modelcontextprotocol/server-filesystem, so the deterministic replay path no longer floats with npm publishes.
  • The prior pending Windows state is superseded by a fresh CI run on the latest head.
  • use-mcp-tool.test.ts now keeps the stable USE_MCP_TOOL_*_SMOKE tags inside real natural-language prompts, so test:ci and test:record still exercise MCP workflows outside replay mode.
  • The new GET_FILE_INFO_PROMPT now accepts either the relative fixture path or the absolute workspace path, so live-model runs no longer fail when get_file_info chooses the prompted relative path.
  • runMcpTask() no longer keeps only the first use_mcp_server ask, so live MCP runs can wait for the later complete request instead of capturing partial payloads with blank toolName or incomplete arguments.

@codecov
Copy link
Copy Markdown

codecov Bot commented May 13, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

Comment thread apps/vscode-e2e/src/suite/tools/use-mcp-tool.test.ts Outdated
Comment thread apps/vscode-e2e/src/suite/tools/use-mcp-tool.test.ts Outdated
Comment thread apps/vscode-e2e/src/suite/tools/use-mcp-tool.test.ts Outdated
@edelauna edelauna force-pushed the chore-unskip-e2e-mutating-tools branch from f491acd to 9fa41b7 Compare May 17, 2026 02:53
@roomote
Copy link
Copy Markdown
Contributor Author

roomote Bot commented May 17, 2026

I see some merge conflicts here. Working on them now...

Base automatically changed from chore-unskip-e2e-mutating-tools to main May 17, 2026 12:57
@roomote
Copy link
Copy Markdown
Contributor Author

roomote Bot commented May 17, 2026

I see some merge conflicts here. Working on them now...

@roomote
Copy link
Copy Markdown
Contributor Author

roomote Bot commented May 17, 2026

Resolved merge conflicts in:

  • apps/vscode-e2e/src/fixtures/apply-diff.ts
  • apps/vscode-e2e/src/fixtures/execute-command.ts
  • apps/vscode-e2e/src/fixtures/write-to-file.ts
  • apps/vscode-e2e/src/runTest.ts

Warnings:

  • A direct pnpm --filter @roo-code/vscode-e2e test:run attempt still stops in unrelated existing e2e suite compilation errors around @roo-code/types module resolution before the touched mock suites execute.
  • The resolved branch was still validated with git diff origin/main --check, pnpm --filter @roo-code/vscode-e2e check-types, the repo lint hook during git merge --continue, and the repo check-types hook during git push.

@roomote roomote Bot force-pushed the chore-unskip-e2e-use-mcp-tool branch from 9f0ddd1 to c31a59a Compare May 17, 2026 15:11
@roomote
Copy link
Copy Markdown
Contributor Author

roomote Bot commented May 17, 2026

Addressed the remaining PR feedback on 77052fc22.

  • Replaced the bare USE_MCP_TOOL_*_SMOKE task text with real MCP instructions while keeping the stable fixture tags embedded for replay-mode matching.
  • Relaxed the read_file completion assertion so it no longer depends on the assistant literally saying requested file.
  • The earlier fixes on this PR remain in place: the filesystem MCP server package is pinned, the unknown-server case no longer depends on empty partial fields, the get_file_info completion check is wording-agnostic, and the stray fixture-only churn is gone.

Validation:

  • The push hook pnpm check-types passed.
  • TEST_FILE=use-mcp-tool.test pnpm --dir apps/vscode-e2e test:ci:mock passed locally with all 6 MCP tests green.
  • pnpm --dir apps/web-roo-code test passed.
  • curl -I http://127.0.0.1:3000 returned 200 OK.

Browser proof is not applicable here because this is test-only e2e fixture work.

Comment thread apps/vscode-e2e/src/suite/tools/use-mcp-tool.test.ts Outdated
Comment thread apps/vscode-e2e/src/suite/tools/use-mcp-tool.test.ts Outdated
@edelauna edelauna force-pushed the chore-unskip-e2e-use-mcp-tool branch from 879e0e8 to 2a54c07 Compare May 17, 2026 23:02
Comment thread apps/vscode-e2e/src/suite/tools/use-mcp-tool.test.ts
@edelauna edelauna force-pushed the chore-unskip-e2e-use-mcp-tool branch from 77052fc to fa293b2 Compare May 19, 2026 02:19
@edelauna edelauna marked this pull request as ready for review May 19, 2026 02:28
Comment thread apps/vscode-e2e/src/suite/tools/use-mcp-tool.test.ts Outdated
@edelauna edelauna force-pushed the chore-unskip-e2e-use-mcp-tool branch from fa293b2 to c946c38 Compare May 19, 2026 02:36
Copy link
Copy Markdown
Contributor

@edelauna edelauna left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unskipping existing e2e tests to increase coverage.

@edelauna edelauna merged commit 7de61e6 into main May 19, 2026
9 checks passed
@edelauna edelauna deleted the chore-unskip-e2e-use-mcp-tool branch May 19, 2026 12:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

roomote:auto-resolve-conflicts Allow Roomote to auto-resolve merge conflicts for this PR

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants