Skip to content

Conversation

@ryoppippi
Copy link
Member

@ryoppippi ryoppippi commented Dec 9, 2025

Summary

Refactors MCP integration tests to use MSW (Mock Service Worker) with Hono instead of Bun.serve(), eliminating the Bun runtime dependency and enabling tests to run in any environment.

What Changed

  • Added mocks/mcp-server.ts: Created createMcpApp() function that generates a Hono app implementing the MCP protocol using @hono/mcp
  • Updated mocks/handlers.ts: Integrated MCP protocol handlers that delegate to the Hono app via app.fetch()
  • Refactored src/tests/stackone.mcp-fetch.spec.ts:
    • Removed Bun.serve() and createMockMcpServer() helper
    • Removed all describe.skip() - tests now run with standard MSW setup
    • Eliminated vi.fn() mocks and type assertions (as unknown as RpcClient)
    • Simplified test setup by using shared MSW handlers
  • Updated vitest.config.ts: Removed test file from exclude array
  • Removed @hono/node-server: Cleaned up unused dependency

Why

The previous implementation used Bun.serve() which:

  1. Required Bun runtime to be available
  2. Was excluded from the test suite
  3. Used vi.fn() mocks that bypassed actual MCP protocol code paths

The new approach:

  1. Works in any JavaScript runtime (Node.js, Bun, etc.)
  2. Uses real MCP protocol implementation via @hono/mcp
  3. Integrates seamlessly with existing MSW setup
  4. Provides better test coverage of actual code paths

Testing

  • ✅ All 284 tests pass
  • ✅ MCP fetch tests (11 tests) now run successfully
  • ✅ Account filtering, provider filtering, and action filtering verified
  • ✅ Authentication via Hono's basicAuth middleware tested

Closes #170
closes #166


Summary by cubic

Refactors MCP integration tests to use MSW with a Hono-based mock MCP server, removing the Bun runtime and enabling tests to run anywhere. Satisfies #170 by exercising real MCP protocol via @hono/mcp and re-enabling previously skipped suites.

  • Refactors
    • Added mocks/mcp-server.ts with createMcpApp() using Hono + @hono/mcp and basicAuth.
    • Delegated https://api.stackone.com/mcp and https://api.stackone-dev.com/mcp in MSW handlers to the Hono app.
    • Rewrote stackone.mcp-fetch.spec.ts to use shared MSW handlers; removed Bun.serve(), helpers, skips, and manual mocks.
    • Updated vitest.config.ts to include MCP tests; removed exclude entry.
    • Removed unused @hono/node-server.

Written for commit efb9e1d. Summary will update automatically on new commits.

Create createMcpApp() function that generates a Hono app implementing
the MCP protocol using @hono/mcp's StreamableHTTPTransport. This
replaces the Bun.serve() approach with a runtime-agnostic solution
that works with MSW via app.fetch().

The mock server supports:
- Account-based tool filtering via x-account-id header
- Basic authentication using Hono's basicAuth middleware
- Pre-defined tool sets for common test scenarios

This implementation allows MCP tests to run without starting a real
HTTP server, using Hono's request/response testing pattern instead.

Related to #170
Add MCP protocol endpoints to handlers.ts that delegate to the Hono
MCP app. This enables MCP tests to use MSW's standard setup without
requiring manual server.use() calls in each test.

The handlers support both production (api.stackone.com) and test
(api.stackone-dev.com) endpoints, with full account filtering and
authentication via the Hono app.

Related to #170
Refactor MCP integration tests to use MSW with Hono app.fetch()
instead of Bun.serve(). This change:

- Removes Bun runtime dependency and createMockMcpServer() helper
- Removes all describe.skip() - tests now run with standard MSW setup
- Eliminates vi.fn() mocks and type assertions
- Uses real MCP protocol implementation via @hono/mcp
- Simplifies test setup by using shared MSW handlers

Tests now verify account filtering, provider filtering, and action
filtering using the actual MCP client code paths.

Closes #170
Remove src/toolsets/tests/stackone.mcp-fetch.spec.ts from the
exclude array now that tests use MSW instead of Bun.serve().

Related to #170
Remove @hono/node-server dev dependency as it's no longer needed.
The MCP mock server uses Hono's app.fetch() directly without
requiring a Node.js HTTP server adapter.

Related to #170
Copilot AI review requested due to automatic review settings December 9, 2025 16:31
@ryoppippi ryoppippi requested a review from a team as a code owner December 9, 2025 16:31
@pkg-pr-new
Copy link

pkg-pr-new bot commented Dec 9, 2025

Open in StackBlitz

npm i https://pkg.pr.new/StackOneHQ/stackone-ai-node/@stackone/ai@189

commit: efb9e1d

Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR successfully refactors MCP integration tests from using Bun-specific Bun.serve() to a runtime-agnostic approach using MSW (Mock Service Worker) with Hono, enabling the tests to run in any JavaScript environment. The refactoring eliminates the need for a real HTTP server while maintaining full MCP protocol implementation.

Key Changes:

  • Created a reusable createMcpApp() function in mocks/mcp-server.ts that generates Hono apps implementing the MCP protocol
  • Integrated MCP protocol handlers into the existing MSW mock setup via mocks/handlers.ts
  • Removed describe.skip() calls and Bun-specific code from test file, enabling 11 previously skipped tests to run

Reviewed changes

Copilot reviewed 5 out of 6 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
mocks/mcp-server.ts New file implementing MCP protocol mock server using Hono, with pre-defined tool sets for various test scenarios and basic authentication support
mocks/handlers.ts Added MCP protocol endpoints that delegate to the Hono app via app.fetch()
src/tests/stackone.mcp-fetch.spec.ts Removed Bun.serve() implementation and skipped tests; refactored to use MSW handlers with proper MCP protocol testing
vitest.config.ts Removed test file from exclude array, enabling it to run in the test suite
pnpm-workspace.yaml Added empty catalog section (appears unintentional)
pnpm-lock.yaml Removed unused @ai-sdk/openai catalog entry
Files not reviewed (1)
  • pnpm-lock.yaml: Language not supported
Comments suppressed due to low confidence (1)

mocks/mcp-server.ts:7

  • Unused imports HttpResponse, http.
import { http, HttpResponse } from 'msw';

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

* without starting a real HTTP server.
*/
import type { Hono as HonoApp } from 'hono';
import { http, HttpResponse } from 'msw';
Copy link

Copilot AI Dec 9, 2025

Choose a reason for hiding this comment

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

Unused imports detected. The http and HttpResponse imports from 'msw' are not used anywhere in this file. They should be removed.

Suggested change
import { http, HttpResponse } from 'msw';

Copilot uses AI. Check for mistakes.
Comment on lines +30 to +46
* Creates an MSW handler for mocking MCP protocol requests.
* Uses Hono's app.request() to handle requests without starting a server.
*
* @example
* ```ts
* import { server } from './mocks/node';
* import { createMcpHandler, defaultMcpTools, accountMcpTools } from './mocks/mcp-server';
*
* // In your test setup
* server.use(
* createMcpHandler({
* accountTools: {
* default: defaultMcpTools,
* 'account-1': accountMcpTools.acc1,
* },
* })
* );
Copy link

Copilot AI Dec 9, 2025

Choose a reason for hiding this comment

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

The documentation comment has multiple inaccuracies:

  1. Line 30 says "Creates an MSW handler" but the function returns a Hono app, not an MSW handler
  2. Line 31 references app.request() but the actual method used is app.fetch() (as seen in line 242, 304, etc.)
  3. Line 36 references createMcpHandler but the function is named createMcpApp

The documentation should be updated to accurately reflect what the function does and its correct name.

Suggested change
* Creates an MSW handler for mocking MCP protocol requests.
* Uses Hono's app.request() to handle requests without starting a server.
*
* @example
* ```ts
* import { server } from './mocks/node';
* import { createMcpHandler, defaultMcpTools, accountMcpTools } from './mocks/mcp-server';
*
* // In your test setup
* server.use(
* createMcpHandler({
* accountTools: {
* default: defaultMcpTools,
* 'account-1': accountMcpTools.acc1,
* },
* })
* );
* Creates a Hono app for mocking MCP protocol requests.
* This can be used in tests to handle MCP requests without starting a real HTTP server.
*
* @example
* ```ts
* import { server } from './mocks/node';
* import { createMcpApp, defaultMcpTools, accountMcpTools } from './mocks/mcp-server';
*
* // In your test setup
* const mcpApp = createMcpApp({
* accountTools: {
* default: defaultMcpTools,
* 'account-1': accountMcpTools.acc1,
* },
* });
* // Use mcpApp.fetch(...) in your test or MSW handler

Copilot uses AI. Check for mistakes.
- examples

catalogMode: strict

Copy link

Copilot AI Dec 9, 2025

Choose a reason for hiding this comment

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

An empty catalog: section has been added. This appears to be unintentional as there's no content under it, and the existing catalogs: section (line 9) already defines the catalog structure. This empty section should be removed.

Suggested change

Copilot uses AI. Check for mistakes.
Copy link
Contributor

@cubic-dev-ai cubic-dev-ai bot left a comment

Choose a reason for hiding this comment

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

1 issue found across 6 files

Prompt for AI agents (all 1 issues)

Check if these issues are valid — if so, understand the root cause of each and fix them.


<file name="mocks/mcp-server.ts">

<violation number="1" location="mocks/mcp-server.ts:36">
P2: JSDoc example references `createMcpHandler` but the actual exported function is named `createMcpApp`. Update the documentation to use the correct function name.</violation>
</file>

Reply to cubic to teach it or ask questions. Re-run a review with @cubic-dev-ai review this PR

* @example
* ```ts
* import { server } from './mocks/node';
* import { createMcpHandler, defaultMcpTools, accountMcpTools } from './mocks/mcp-server';
Copy link
Contributor

@cubic-dev-ai cubic-dev-ai bot Dec 9, 2025

Choose a reason for hiding this comment

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

P2: JSDoc example references createMcpHandler but the actual exported function is named createMcpApp. Update the documentation to use the correct function name.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At mocks/mcp-server.ts, line 36:

<comment>JSDoc example references `createMcpHandler` but the actual exported function is named `createMcpApp`. Update the documentation to use the correct function name.</comment>

<file context>
@@ -0,0 +1,230 @@
+ * @example
+ * ```ts
+ * import { server } from &#39;./mocks/node&#39;;
+ * import { createMcpHandler, defaultMcpTools, accountMcpTools } from &#39;./mocks/mcp-server&#39;;
+ *
+ * // In your test setup
</file context>
Fix with Cubic

Applied 'as const satisfies McpToolDefinition[]' pattern consistently
across all mock tool definitions (defaultMcpTools, accountMcpTools,
mixedProviderTools) for improved type safety and immutability.

This ensures TypeScript can infer the most specific literal types
whilst maintaining type compatibility, following the project's
typescript-patterns skill guidelines.
@ryoppippi ryoppippi merged commit 46f31f3 into main Dec 9, 2025
11 checks passed
@ryoppippi ryoppippi deleted the refactor/mcp-tests-msw-hono branch December 9, 2025 17:13
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.

Refactor MCP tests to use MSW instead of vi.fn() mocks Add unit tests for mcp-client.ts

3 participants