From 39550b2420f01419b8dc23eacc0a9cb431547e43 Mon Sep 17 00:00:00 2001 From: ryoppippi <1560508+ryoppippi@users.noreply.github.com> Date: Tue, 9 Dec 2025 12:49:04 +0000 Subject: [PATCH 1/3] fix(test): remove obsolete stackOneClient from MCP fetch tests Remove references to the non-existent stackOneClient property that was causing type errors. The property was part of an old API that no longer exists after the RPC client refactor (commit 53bce870). Changes: - Remove @stackone/stackone-client-ts import (package no longer used) - Remove vitest import (globals are enabled, no import needed) - Remove stackOneClient mock objects from all test configurations - Remove RPC execution assertions that relied on injected client The tests remain skipped (describe.skip) as they require Bun runtime. --- src/tests/stackone.mcp-fetch.spec.ts | 94 +--------------------------- 1 file changed, 2 insertions(+), 92 deletions(-) diff --git a/src/tests/stackone.mcp-fetch.spec.ts b/src/tests/stackone.mcp-fetch.spec.ts index 6556e5fa..034acc08 100644 --- a/src/tests/stackone.mcp-fetch.spec.ts +++ b/src/tests/stackone.mcp-fetch.spec.ts @@ -4,9 +4,7 @@ import { StreamableHTTPTransport } from '@hono/mcp'; import { McpServer } from '@modelcontextprotocol/sdk/server/mcp.js'; -import type { StackOne } from '@stackone/stackone-client-ts'; import { Hono } from 'hono'; -import { assert, vi } from 'vitest'; import { z } from 'zod'; import { server as mswServer } from '../../mocks/node'; import { ToolSet } from '../toolsets/base'; @@ -109,18 +107,11 @@ describe.skip('ToolSet.fetchTools (MCP + RPC integration)', () => { }); it('creates tools from MCP catalog and wires RPC execution', async () => { - const stackOneClient = { - actions: { - rpcAction: vi.fn(async () => ({ actionsRpcResponse: { data: null } })), - }, - } as unknown as StackOne; - class TestToolSet extends ToolSet {} const toolset = new TestToolSet({ baseUrl: origin, headers: { 'x-account-id': 'test-account' }, - stackOneClient, }); const tools = await toolset.fetchTools(); @@ -140,19 +131,8 @@ describe.skip('ToolSet.fetchTools (MCP + RPC integration)', () => { const executableTool = (await tool.toAISDK()).dummy_action; assert(executableTool.execute, 'execute should be defined'); - const result = await executableTool.execute( - { foo: 'bar' }, - { toolCallId: 'test-id', messages: [] } - ); - - expect(stackOneClient.actions.rpcAction).toHaveBeenCalledWith({ - action: 'dummy_action', - body: { foo: 'bar' }, - headers: { 'x-account-id': 'test-account' }, - path: undefined, - query: undefined, - }); - expect(result).toEqual({ data: null }); + // TODO: Re-enable execution test when RPC mocking is properly set up + // The execute function now uses internal RpcClient, not an injected stackOneClient }); }); @@ -228,16 +208,9 @@ describe.skip('StackOneToolSet account filtering', () => { }); it('supports setAccounts() for chaining', async () => { - const stackOneClient = { - actions: { - rpcAction: vi.fn(async () => ({ actionsRpcResponse: { data: null } })), - }, - } as unknown as StackOne; - const toolset = new StackOneToolSet({ baseUrl: origin, apiKey: 'test-key', - stackOneClient, }); // Test chaining @@ -246,16 +219,9 @@ describe.skip('StackOneToolSet account filtering', () => { }); it('fetches tools without account filtering when no accountIds provided', async () => { - const stackOneClient = { - actions: { - rpcAction: vi.fn(async () => ({ actionsRpcResponse: { data: null } })), - }, - } as unknown as StackOne; - const toolset = new StackOneToolSet({ baseUrl: origin, apiKey: 'test-key', - stackOneClient, }); const tools = await toolset.fetchTools(); @@ -268,16 +234,9 @@ describe.skip('StackOneToolSet account filtering', () => { }); it('uses x-account-id header when fetching tools with accountIds', async () => { - const stackOneClient = { - actions: { - rpcAction: vi.fn(async () => ({ actionsRpcResponse: { data: null } })), - }, - } as unknown as StackOne; - const toolset = new StackOneToolSet({ baseUrl: origin, apiKey: 'test-key', - stackOneClient, }); // Fetch tools for acc1 @@ -291,16 +250,9 @@ describe.skip('StackOneToolSet account filtering', () => { }); it('uses setAccounts when no accountIds provided in fetchTools', async () => { - const stackOneClient = { - actions: { - rpcAction: vi.fn(async () => ({ actionsRpcResponse: { data: null } })), - }, - } as unknown as StackOne; - const toolset = new StackOneToolSet({ baseUrl: origin, apiKey: 'test-key', - stackOneClient, }); // Set accounts using setAccounts @@ -321,16 +273,9 @@ describe.skip('StackOneToolSet account filtering', () => { }); it('overrides setAccounts when accountIds provided in fetchTools', async () => { - const stackOneClient = { - actions: { - rpcAction: vi.fn(async () => ({ actionsRpcResponse: { data: null } })), - }, - } as unknown as StackOne; - const toolset = new StackOneToolSet({ baseUrl: origin, apiKey: 'test-key', - stackOneClient, }); // Set accounts using setAccounts @@ -397,16 +342,9 @@ describe.skip('StackOneToolSet provider and action filtering', () => { }); it('filters tools by providers', async () => { - const stackOneClient = { - actions: { - rpcAction: vi.fn(async () => ({ actionsRpcResponse: { data: null } })), - }, - } as unknown as StackOne; - const toolset = new StackOneToolSet({ baseUrl: origin, apiKey: 'test-key', - stackOneClient, }); // Filter by providers @@ -424,16 +362,9 @@ describe.skip('StackOneToolSet provider and action filtering', () => { }); it('filters tools by actions with exact match', async () => { - const stackOneClient = { - actions: { - rpcAction: vi.fn(async () => ({ actionsRpcResponse: { data: null } })), - }, - } as unknown as StackOne; - const toolset = new StackOneToolSet({ baseUrl: origin, apiKey: 'test-key', - stackOneClient, }); // Filter by exact action names @@ -450,16 +381,9 @@ describe.skip('StackOneToolSet provider and action filtering', () => { }); it('filters tools by actions with glob pattern', async () => { - const stackOneClient = { - actions: { - rpcAction: vi.fn(async () => ({ actionsRpcResponse: { data: null } })), - }, - } as unknown as StackOne; - const toolset = new StackOneToolSet({ baseUrl: origin, apiKey: 'test-key', - stackOneClient, }); // Filter by glob pattern @@ -508,16 +432,9 @@ describe.skip('StackOneToolSet provider and action filtering', () => { acc2: acc2Tools, }); - const stackOneClient = { - actions: { - rpcAction: vi.fn(async () => ({ actionsRpcResponse: { data: null } })), - }, - } as unknown as StackOne; - const toolset = new StackOneToolSet({ baseUrl: server.origin, apiKey: 'test-key', - stackOneClient, }); // Combine account and action filters @@ -561,16 +478,9 @@ describe.skip('StackOneToolSet provider and action filtering', () => { acc1: acc1Tools, }); - const stackOneClient = { - actions: { - rpcAction: vi.fn(async () => ({ actionsRpcResponse: { data: null } })), - }, - } as unknown as StackOne; - const toolset = new StackOneToolSet({ baseUrl: server.origin, apiKey: 'test-key', - stackOneClient, }); // Combine all filters From b4583b95c019a04d5fdbc6909fc12bd3a2135184 Mon Sep 17 00:00:00 2001 From: ryoppippi <1560508+ryoppippi@users.noreply.github.com> Date: Tue, 9 Dec 2025 12:49:15 +0000 Subject: [PATCH 2/3] fix(ci): remove --no-bail to fail on script errors Remove --no-bail flag from test and typecheck scripts. According to pnpm documentation, --no-bail causes the command to exit with code 0 even if scripts fail, which was allowing CI to pass despite type errors. This change ensures CI properly fails when: - Type checking encounters errors - Tests fail The --aggregate-output flag is retained to show all output together. --- package.json | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/package.json b/package.json index c3f2c27f..c50a60e5 100644 --- a/package.json +++ b/package.json @@ -23,11 +23,11 @@ "lint": "biome check .", "preinstall": "npx only-allow pnpm", "prepack": "npm pkg delete scripts.preinstall && pnpm run build", - "test": "pnpm --no-bail --aggregate-output run '/^test:/'", - "test:submodules": "pnpm --parallel -r --no-bail --aggregate-output test", + "test": "pnpm --aggregate-output run '/^test:/'", + "test:submodules": "pnpm --parallel -r --aggregate-output test", "test:root": "vitest", - "typecheck": "pnpm --no-bail --aggregate-output run '/^typecheck:/'", - "typecheck:submodules": "pnpm --parallel -r --no-bail --aggregate-output typecheck", + "typecheck": "pnpm --aggregate-output run '/^typecheck:/'", + "typecheck:submodules": "pnpm --parallel -r --aggregate-output typecheck", "typecheck:root": "tsgo --noEmit" }, "dependencies": { From d526610ffbba24c42e6c8286775f2e6cb4f1150d Mon Sep 17 00:00:00 2001 From: ryoppippi <1560508+ryoppippi@users.noreply.github.com> Date: Tue, 9 Dec 2025 12:49:25 +0000 Subject: [PATCH 3/3] docs(test): document vitest globals usage Add documentation about vitest globals being enabled in the project. Developers should NOT import from 'vitest' as describe, it, expect, vi, assert, and other test utilities are available globally. This prevents unnecessary imports and aligns with project configuration (globals: true in vitest.config.ts). --- .claude/skills/typescript-testing/SKILL.md | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/.claude/skills/typescript-testing/SKILL.md b/.claude/skills/typescript-testing/SKILL.md index d2cec3d3..6ffb5ab1 100644 --- a/.claude/skills/typescript-testing/SKILL.md +++ b/.claude/skills/typescript-testing/SKILL.md @@ -15,6 +15,24 @@ The project uses **Vitest** as the test runner. Run tests with: - `pnpm vitest src/path/to/file.spec.ts` - Run a specific test file - `pnpm vitest -t "test name"` - Run tests matching a pattern +### Vitest Globals + +**Vitest globals are enabled** (`globals: true` in `vitest.config.ts`). Do NOT import test utilities from `'vitest'` - they are available globally: + +```typescript +// ❌ WRONG - Do NOT import from vitest +import { describe, it, expect, vi } from 'vitest'; + +// ✅ CORRECT - Use globals directly (no import needed) +describe('MyTest', () => { + it('should work', () => { + expect(true).toBe(true); + }); +}); +``` + +Available globals: `describe`, `it`, `test`, `expect`, `vi`, `beforeAll`, `afterAll`, `beforeEach`, `afterEach`, `assert`, etc. + ## MSW (Mock Service Worker) **MSW is the preferred HTTP mocking solution.** MSW is configured globally in `vitest.setup.ts`, so no per-file setup is required.