From 01424e7b0c573dfb7a3fa0c36f16a432cb669e5c Mon Sep 17 00:00:00 2001 From: mojoho81 Date: Mon, 6 Jul 2026 20:30:40 +0000 Subject: [PATCH] test(mcp): fix store tests writing fixtures outside getStateDir; sync hub-search specs with pagination API Root cause A (23 tests): mcp-presets-store, mcp-hub-sources-store, mcp-tools-cache and the /api/mcp/presets route tests wrote fixture files at $HERMES_HOME/, but the stores resolve paths via getStateDir(), which appends a workspace/ segment ($HERMES_HOME/workspace/). The stores never saw the fixtures and bootstrapped from seed instead, so every user-file/invalid-path assertion failed on any machine. Fix: tests now derive paths from the modules' own exported helpers (presetsFilePath(), hubSourcesFilePath(), getStateDir()) instead of hand-building them. Also un-vacuouses the two mcp-tools-cache corrupt- file tests, which previously wrote the corrupt file to an unread path. Root cause B (4 tests): -hub-search.test.ts still expected the old 3-arg unifiedSearch signature and a limit clamp of 100; the route now clamps to 500 and forwards an offset param. Updated expectations, added offset coverage, and corrected the stale limit/offset docblock in hub-search.ts (comment-only product change). Before: 27 failed across these 5 files. After: 70/70 pass; full vitest run leaves only the known out-of-scope failures (chat components, i18n, e2e playwright). --- src/routes/api/mcp/-hub-search.test.ts | 20 ++++++++++------ src/routes/api/mcp/-presets.test.ts | 13 ++++++---- src/routes/api/mcp/hub-search.ts | 3 ++- src/server/mcp-hub-sources-store.test.ts | 10 ++++---- src/server/mcp-presets-store.test.ts | 30 ++++++++++++++---------- src/server/mcp-tools-cache.test.ts | 10 +++++--- 6 files changed, 55 insertions(+), 31 deletions(-) diff --git a/src/routes/api/mcp/-hub-search.test.ts b/src/routes/api/mcp/-hub-search.test.ts index 09d4c6076e..851a109dcf 100644 --- a/src/routes/api/mcp/-hub-search.test.ts +++ b/src/routes/api/mcp/-hub-search.test.ts @@ -65,28 +65,34 @@ describe('GET /api/mcp/hub-search — auth', () => { }) describe('GET /api/mcp/hub-search — query parsing', () => { - it('passes q, source, limit to unifiedSearch', async () => { + it('passes q, source, limit, offset to unifiedSearch', async () => { mockUnifiedSearch.mockResolvedValue({ results: [], source: 'mcp-get', total: 0 }) - await callGet('http://localhost/api/mcp/hub-search?q=github&source=mcp-get&limit=5') - expect(mockUnifiedSearch).toHaveBeenCalledWith('github', 'mcp-get', 5) + await callGet('http://localhost/api/mcp/hub-search?q=github&source=mcp-get&limit=5&offset=10') + expect(mockUnifiedSearch).toHaveBeenCalledWith('github', 'mcp-get', 5, 10) }) it('uses defaults when params absent', async () => { mockUnifiedSearch.mockResolvedValue({ results: [], source: 'all', total: 0 }) await callGet('http://localhost/api/mcp/hub-search') - expect(mockUnifiedSearch).toHaveBeenCalledWith('', 'all', 20) + expect(mockUnifiedSearch).toHaveBeenCalledWith('', 'all', 20, 0) }) - it('clamps limit to 100', async () => { + it('clamps limit to 500', async () => { mockUnifiedSearch.mockResolvedValue({ results: [], source: 'all', total: 0 }) await callGet('http://localhost/api/mcp/hub-search?limit=9999') - expect(mockUnifiedSearch).toHaveBeenCalledWith('', 'all', 100) + expect(mockUnifiedSearch).toHaveBeenCalledWith('', 'all', 500, 0) }) it('defaults invalid source to all', async () => { mockUnifiedSearch.mockResolvedValue({ results: [], source: 'all', total: 0 }) await callGet('http://localhost/api/mcp/hub-search?source=invalid') - expect(mockUnifiedSearch).toHaveBeenCalledWith('', 'all', 20) + expect(mockUnifiedSearch).toHaveBeenCalledWith('', 'all', 20, 0) + }) + + it('clamps negative offset to 0', async () => { + mockUnifiedSearch.mockResolvedValue({ results: [], source: 'all', total: 0 }) + await callGet('http://localhost/api/mcp/hub-search?offset=-5') + expect(mockUnifiedSearch).toHaveBeenCalledWith('', 'all', 20, 0) }) }) diff --git a/src/routes/api/mcp/-presets.test.ts b/src/routes/api/mcp/-presets.test.ts index 7b45c075a8..42bd064f8b 100644 --- a/src/routes/api/mcp/-presets.test.ts +++ b/src/routes/api/mcp/-presets.test.ts @@ -1,7 +1,9 @@ import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' -import { mkdtempSync, rmSync, writeFileSync } from 'node:fs' +import { mkdirSync, mkdtempSync, rmSync, writeFileSync } from 'node:fs' import { tmpdir } from 'node:os' -import { join } from 'node:path' +import { dirname, join } from 'node:path' + +import { presetsFilePath } from '../../../server/mcp-presets-store' const VALID_SEED = { version: 1, @@ -95,7 +97,10 @@ describe('GET /api/mcp/presets', () => { it('returns 200 with source=invalid + error fields when user file is malformed', async () => { delete process.env.CLAUDE_PASSWORD - writeFileSync(join(homeDir, 'mcp-presets.json'), '{not valid json') + // The store resolves the user file via getStateDir() (HERMES_HOME/workspace) + const userFile = presetsFilePath() + mkdirSync(dirname(userFile), { recursive: true }) + writeFileSync(userFile, '{not valid json') const mod = await loadRoute() const res = await mod.Route.server.handlers.GET({ request: new Request('http://localhost/api/mcp/presets'), @@ -111,7 +116,7 @@ describe('GET /api/mcp/presets', () => { expect(body.ok).toBe(false) expect(body.source).toBe('invalid') expect(body.error).toBeTruthy() - expect(body.errorPath).toBe(join(homeDir, 'mcp-presets.json')) + expect(body.errorPath).toBe(userFile) expect((body.validationErrors ?? []).length).toBeGreaterThan(0) }) }) diff --git a/src/routes/api/mcp/hub-search.ts b/src/routes/api/mcp/hub-search.ts index 7ba263a5ee..5a7c144e07 100644 --- a/src/routes/api/mcp/hub-search.ts +++ b/src/routes/api/mcp/hub-search.ts @@ -6,7 +6,8 @@ * Query params: * q Free-text search query (default '') * source 'all' | 'mcp-get' | 'local' (default 'all') - * limit Max results 1..100 (default 20) + * limit Max results 1..500 (default 20) + * offset Pagination offset >= 0 (default 0) * * Auth-gated via isAuthenticated. * Rate-limited: 60 req/min per IP. diff --git a/src/server/mcp-hub-sources-store.test.ts b/src/server/mcp-hub-sources-store.test.ts index 40c0e450aa..195c37881b 100644 --- a/src/server/mcp-hub-sources-store.test.ts +++ b/src/server/mcp-hub-sources-store.test.ts @@ -3,13 +3,14 @@ */ import { afterEach, beforeEach, describe, expect, it } from 'vitest' import { + mkdirSync, mkdtempSync, readFileSync, rmSync, writeFileSync, } from 'node:fs' import { tmpdir } from 'node:os' -import { join } from 'node:path' +import { dirname, join } from 'node:path' import { __resetHubSourcesCacheForTests, @@ -27,7 +28,8 @@ let homeDir: string let originalHermesHome: string | undefined function writeSourcesFile(payload: unknown): void { - const path = join(homeDir, 'mcp-hub-sources.json') + const path = hubSourcesFilePath() + mkdirSync(dirname(path), { recursive: true }) writeFileSync(path, typeof payload === 'string' ? payload : JSON.stringify(payload, null, 2)) } @@ -86,12 +88,12 @@ describe('readHubSources', () => { it('returns source=invalid for malformed JSON, preserves file', async () => { writeSourcesFile('not-json{{{{') - const before = readFileSync(join(homeDir, 'mcp-hub-sources.json'), 'utf8') + const before = readFileSync(hubSourcesFilePath(), 'utf8') const result = await readHubSources() expect(result.source).toBe('invalid') expect(result.error).toBeTruthy() // File is preserved - const after = readFileSync(join(homeDir, 'mcp-hub-sources.json'), 'utf8') + const after = readFileSync(hubSourcesFilePath(), 'utf8') expect(after).toBe(before) }) diff --git a/src/server/mcp-presets-store.test.ts b/src/server/mcp-presets-store.test.ts index 90746a2b90..0861a85ae4 100644 --- a/src/server/mcp-presets-store.test.ts +++ b/src/server/mcp-presets-store.test.ts @@ -11,7 +11,7 @@ import { writeFileSync, } from 'node:fs' import { tmpdir } from 'node:os' -import { join } from 'node:path' +import { dirname, join } from 'node:path' import { __resetPresetsCacheForTests, @@ -52,7 +52,8 @@ function writeSeed(payload: unknown): void { } function writeUserFile(payload: unknown): void { - const path = join(homeDir, 'mcp-presets.json') + const path = presetsFilePath() + mkdirSync(dirname(path), { recursive: true }) writeFileSync(path, typeof payload === 'string' ? payload : JSON.stringify(payload)) } @@ -125,7 +126,7 @@ describe('readPresets', () => { }) it('returns source=invalid for malformed user JSON and preserves the file unchanged', async () => { - const path = join(homeDir, 'mcp-presets.json') + const path = presetsFilePath() const corrupt = '{this is not valid json' writeUserFile(corrupt) const result = await readPresets() @@ -264,7 +265,10 @@ describe('readPresets', () => { try { const result = await readPresets() expect(result.source).toBe('seed') - expect(existsSync(join(altHome, 'mcp-presets.json'))).toBe(true) + // presetsFilePath() resolves through getStateDir(), which honors + // HERMES_HOME at call time — the bootstrapped file must live there. + expect(presetsFilePath().startsWith(altHome)).toBe(true) + expect(existsSync(presetsFilePath())).toBe(true) } finally { rmSync(altHome, { recursive: true, force: true }) process.env.HERMES_HOME = homeDir @@ -286,7 +290,8 @@ describe('readPresets', () => { it('returns source=invalid when user file exists but is permission-denied (EACCES)', async () => { // Skip on platforms where chmod doesn't restrict root if (process.getuid?.() === 0) return - const path = join(homeDir, 'mcp-presets.json') + const path = presetsFilePath() + mkdirSync(dirname(path), { recursive: true }) writeFileSync(path, JSON.stringify(VALID_SEED)) chmodSync(path, 0o000) __resetPresetsCacheForTests() @@ -301,7 +306,8 @@ describe('readPresets', () => { }) it('returns source=invalid when user file path is a dangling symlink', async () => { - const path = join(homeDir, 'mcp-presets.json') + const path = presetsFilePath() + mkdirSync(dirname(path), { recursive: true }) const nonexistent = join(homeDir, 'does-not-exist.json') symlinkSync(nonexistent, path) __resetPresetsCacheForTests() @@ -313,9 +319,9 @@ describe('readPresets', () => { // MED-5: cache detects same-size same-mtime edits via inode/ctime it('cache invalidates on same-size same-mtime edit (detects via ctime/inode)', async () => { - const path = join(homeDir, 'mcp-presets.json') + const path = presetsFilePath() const base = JSON.stringify(VALID_SEED) - writeFileSync(path, base) + writeUserFile(base) const r1 = await readPresets() expect(r1.presets.length).toBe(1) @@ -347,10 +353,10 @@ describe('readPresets', () => { // MED-6: category allowlist it('rejects preset with unknown category', async () => { - writeFileSync(join(homeDir, 'mcp-presets.json'), JSON.stringify({ + writeUserFile({ version: 1, presets: [{ ...VALID_SEED.presets[0], category: 'RandomCategory' }], - })) + }) __resetPresetsCacheForTests() const result = await readPresets() expect(result.source).toBe('invalid') @@ -362,10 +368,10 @@ describe('readPresets', () => { it('defaults category to Custom when missing', async () => { const presetWithoutCategory = { ...VALID_SEED.presets[0] } as Record delete presetWithoutCategory.category - writeFileSync(join(homeDir, 'mcp-presets.json'), JSON.stringify({ + writeUserFile({ version: 1, presets: [presetWithoutCategory], - })) + }) __resetPresetsCacheForTests() const result = await readPresets() expect(result.source).toBe('user-file') diff --git a/src/server/mcp-tools-cache.test.ts b/src/server/mcp-tools-cache.test.ts index 2502fe0b1d..c35de1bdc4 100644 --- a/src/server/mcp-tools-cache.test.ts +++ b/src/server/mcp-tools-cache.test.ts @@ -12,6 +12,8 @@ import { mkdirSync, mkdtempSync, readFileSync, rmSync, writeFileSync } from 'nod import { tmpdir } from 'node:os' import { join } from 'node:path' +import { getStateDir } from './workspace-state-dir' + // We re-import the module fresh for each test using dynamic import + cache busting, // but since Vitest caches modules, we use the exported reset helper instead. @@ -76,7 +78,7 @@ describe('write → read roundtrip', () => { describe('corrupt file → empty cache', () => { it('ignores corrupt JSON and starts with empty cache', async () => { // Write corrupt file before module load - const cacheDir = join(tmpDir, 'cache') + const cacheDir = join(getStateDir(), 'cache') mkdirSync(cacheDir, { recursive: true }) writeFileSync(join(cacheDir, 'mcp-tools.json'), '{ not valid json !!!', 'utf8') @@ -94,7 +96,7 @@ describe('corrupt file → empty cache', () => { }) it('ignores wrong schema (version != 1)', async () => { - const cacheDir = join(tmpDir, 'cache') + const cacheDir = join(getStateDir(), 'cache') mkdirSync(cacheDir, { recursive: true }) writeFileSync( join(cacheDir, 'mcp-tools.json'), @@ -164,7 +166,9 @@ describe('HERMES_HOME override for path resolution', () => { vi.resetModules() const mod = await loadCache() - expect(mod.cacheFilePath()).toBe(join(customHome, 'cache', 'mcp-tools.json')) + // cacheFilePath() resolves via getStateDir() → $HERMES_HOME/workspace/cache + expect(mod.cacheFilePath()).toBe(join(getStateDir(), 'cache', 'mcp-tools.json')) + expect(mod.cacheFilePath().startsWith(customHome)).toBe(true) mod.setProbe('server-x', { status: 'failed',