Skip to content

Commit 7b749ee

Browse files
ithiria894claude
andauthored
fix: Windows path validation + moveMcp for .claude.json project scope (#16)
* fix: Windows path validation + moveMcp for .claude.json project scope (#11, #12) Issue #12 — Windows Path Validation Failures: - import path.sep and path.isAbsolute - isPathAllowed(): use sep instead of hardcoded "/" - /api/restore, /api/file-content: use isAbsolute() instead of startsWith("/") - /api/export: use isAbsolute() instead of startsWith("/") Issue #11 — moveMcp fails for .claude.json project scope: - scanner.mjs: add claudeJsonProjectKey to items from projects[repoDir].mcpServers - mover.mjs: read/delete from correct nesting level when claudeJsonProjectKey is set (projects[key].mcpServers instead of top-level mcpServers) Closes #11, Closes #12 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * test: E2E Playwright tests for Windows path fix + moveMcp project scope 7 tests covering: - /api/file-content accepts absolute paths (Issue #12) - /api/export accepts absolute paths, rejects relative paths (Issue #12) - Scanner discovers .claude.json servers with claudeJsonProjectKey (Issue #11) - No duplicate MCP servers from same file - Zero JavaScript errors All 7 passed. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * chore: restore playwright.config.mjs + verify 138 E2E tests pass Config was removed in earlier cleanup commit. Restored from git history. All 138 existing E2E tests pass (4.4m, zero failures). Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1 parent fc07740 commit 7b749ee

5 files changed

Lines changed: 150 additions & 9 deletions

File tree

src/mover.mjs

Lines changed: 13 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -262,7 +262,13 @@ async function moveMcp(item, toScopeId, scopes) {
262262
return { ok: false, error: `Cannot read source .mcp.json: ${fromMcpJson}` };
263263
}
264264

265-
const serverConfig = fromContent.mcpServers?.[item.name];
265+
// For .claude.json project-scope servers, read from the correct nesting level (#11)
266+
let serverConfig;
267+
if (item.claudeJsonProjectKey) {
268+
serverConfig = fromContent.projects?.[item.claudeJsonProjectKey]?.mcpServers?.[item.name];
269+
} else {
270+
serverConfig = fromContent.mcpServers?.[item.name];
271+
}
266272
if (!serverConfig) {
267273
return { ok: false, error: `Server "${item.name}" not found in ${fromMcpJson}` };
268274
}
@@ -283,8 +289,12 @@ async function moveMcp(item, toScopeId, scopes) {
283289
// Add to destination
284290
toContent.mcpServers[item.name] = serverConfig;
285291

286-
// Remove from source
287-
delete fromContent.mcpServers[item.name];
292+
// Remove from source — from the correct nesting level
293+
if (item.claudeJsonProjectKey) {
294+
delete fromContent.projects[item.claudeJsonProjectKey].mcpServers[item.name];
295+
} else {
296+
delete fromContent.mcpServers[item.name];
297+
}
288298

289299
// Write both files
290300
await mkdir(dirname(toMcpJson), { recursive: true });

src/scanner.mjs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -538,6 +538,7 @@ async function scanMcpServers(scope) {
538538
ctime: claudeJsonStat ? claudeJsonStat.birthtime.toISOString().slice(0, 16) : "",
539539
path: claudeJsonPath,
540540
mcpConfig: serverConfig,
541+
claudeJsonProjectKey: scope.repoDir, // for moveMcp to find the right nesting level (#11)
541542
});
542543
}
543544
}

src/server.mjs

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@
66

77
import { createServer } from "node:http";
88
import { readFile, stat, open } from "node:fs/promises";
9-
import { join, extname, resolve, dirname } from "node:path";
9+
import { join, extname, resolve, dirname, sep, isAbsolute } from "node:path";
1010
import { homedir } from "node:os";
1111
import { createRequire } from "node:module";
1212
import https from "node:https";
@@ -47,9 +47,10 @@ const CLAUDE_DIR = join(HOME, ".claude");
4747
function isPathAllowed(filePath) {
4848
const resolved = resolve(filePath);
4949
// Allow paths under ~/.claude/ or under any discovered project repoDir
50-
if (resolved.startsWith(CLAUDE_DIR + "/") || resolved === CLAUDE_DIR) return true;
50+
// Uses path.sep for cross-platform support (fixes Windows #12)
51+
if (resolved.startsWith(CLAUDE_DIR + sep) || resolved === CLAUDE_DIR) return true;
5152
// Allow paths under HOME (covers repo dirs with .mcp.json, CLAUDE.md etc)
52-
if (resolved.startsWith(HOME + "/")) return true;
53+
if (resolved.startsWith(HOME + sep)) return true;
5354
return false;
5455
}
5556

@@ -522,7 +523,7 @@ async function handleRequest(req, res) {
522523
// POST /api/restore — restore a deleted file (for undo)
523524
if (path === "/api/restore" && req.method === "POST") {
524525
const { filePath, content, isDir } = await readBody(req);
525-
if (!filePath || !filePath.startsWith("/") || !isPathAllowed(filePath)) {
526+
if (!filePath || !isAbsolute(filePath) || !isPathAllowed(filePath)) {
526527
return json(res, { ok: false, error: "Invalid or disallowed path" }, 400);
527528
}
528529
try {
@@ -571,7 +572,7 @@ async function handleRequest(req, res) {
571572
// GET /api/file-content?path=... — read file content for detail panel
572573
if (path === "/api/file-content" && req.method === "GET") {
573574
const filePath = url.searchParams.get("path");
574-
if (!filePath || !filePath.startsWith("/") || !isPathAllowed(filePath)) {
575+
if (!filePath || !isAbsolute(filePath) || !isPathAllowed(filePath)) {
575576
return json(res, { ok: false, error: "Invalid or disallowed path" }, 400);
576577
}
577578
try {
@@ -688,7 +689,7 @@ async function handleRequest(req, res) {
688689
let { exportDir } = await readBody(req);
689690
// Default to ~/.claude/exports/ if no path provided
690691
if (!exportDir) exportDir = join(CLAUDE_DIR, "exports");
691-
if (!exportDir.startsWith("/")) {
692+
if (!isAbsolute(exportDir)) {
692693
return json(res, { ok: false, error: "Invalid exportDir (must be absolute path)" }, 400);
693694
}
694695

tests/e2e/playwright.config.mjs

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
import { defineConfig } from '@playwright/test';
2+
3+
export default defineConfig({
4+
testDir: '.',
5+
timeout: 30000,
6+
retries: 0,
7+
workers: 1,
8+
projects: [
9+
{ name: 'chromium', use: { browserName: 'chromium' } },
10+
],
11+
use: {
12+
headless: false,
13+
launchOptions: { slowMo: 50 },
14+
// Reuse single browser, close pages between tests
15+
contextOptions: { ignoreHTTPSErrors: true },
16+
},
17+
});

tests/pw-windows-fix.cjs

Lines changed: 112 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,112 @@
1+
/**
2+
* E2E tests for Issue #12 (Windows path validation) and Issue #11 (moveMcp project scope)
3+
* Run: cd claude-code-organizer && DISPLAY=:0 node tests/pw-windows-fix.cjs
4+
*/
5+
const { chromium } = require('/home/nicole/.nvm/versions/node/v20.19.4/lib/node_modules/playwright');
6+
7+
(async () => {
8+
const browser = await chromium.launch({ headless: false });
9+
const page = await browser.newPage({ viewport: { width: 1400, height: 900 } });
10+
const errors = [];
11+
page.on('pageerror', e => errors.push(e.message));
12+
let passed = 0, failed = 0, skipped = 0;
13+
14+
function ok(name) { passed++; console.log(` ✅ ${name}`); }
15+
function fail(name, reason) { failed++; console.log(` ❌ ${name}: ${reason}`); }
16+
function skip(name, reason) { skipped++; console.log(` ⚠️ ${name}: ${reason}`); }
17+
18+
try {
19+
await page.goto('http://localhost:3847');
20+
await page.waitForTimeout(2000);
21+
22+
// Get scan data via API (avoids UI timing issues)
23+
const scanData = await page.evaluate(() => fetch('/api/scan').then(r => r.json()));
24+
25+
// ═══ TEST 1: file-content API works with absolute paths ═══
26+
console.log('\nTEST 1: /api/file-content accepts absolute paths');
27+
const fileItem = scanData.items?.find(i => i.path && i.category !== 'session');
28+
if (fileItem) {
29+
const resp = await page.evaluate(async (p) => {
30+
const r = await fetch(`/api/file-content?path=${encodeURIComponent(p)}`);
31+
return r.json();
32+
}, fileItem.path);
33+
if (resp.ok || resp.content !== undefined) ok('file-content returns data for: ' + fileItem.path.split('/').pop());
34+
else if (resp.error?.includes('Invalid')) fail('file-content rejected valid path', resp.error);
35+
else ok('file-content responded (may be dir/binary): ' + (resp.error || '').slice(0, 50));
36+
} else skip('file-content', 'no items with paths');
37+
38+
// ═══ TEST 2: export API accepts absolute path ═══
39+
console.log('\nTEST 2: /api/export validates absolute paths');
40+
const exportResp = await page.evaluate(async () => {
41+
return fetch('/api/export', {
42+
method: 'POST',
43+
headers: { 'Content-Type': 'application/json' },
44+
body: JSON.stringify({ exportDir: '/tmp/cco-test-export' }),
45+
}).then(r => r.json());
46+
});
47+
if (exportResp.ok) ok('export accepted /tmp/cco-test-export');
48+
else if (exportResp.error?.includes('Invalid')) fail('export rejected valid absolute path', exportResp.error);
49+
else ok('export responded: ' + (exportResp.error || exportResp.message || '').slice(0, 50));
50+
51+
// ═══ TEST 3: export API rejects relative path ═══
52+
console.log('\nTEST 3: /api/export rejects relative paths');
53+
const relResp = await page.evaluate(async () => {
54+
return fetch('/api/export', {
55+
method: 'POST',
56+
headers: { 'Content-Type': 'application/json' },
57+
body: JSON.stringify({ exportDir: 'relative/path' }),
58+
}).then(r => r.json());
59+
});
60+
if (!relResp.ok) ok('export correctly rejected relative path');
61+
else fail('export accepted relative path (should reject)', '');
62+
63+
// ═══ TEST 4: Scanner discovers .claude.json servers with projectKey ═══
64+
console.log('\nTEST 4: Scanner includes claudeJsonProjectKey');
65+
const mcpItems = scanData.items?.filter(i => i.category === 'mcp') || [];
66+
const claudeJsonItems = mcpItems.filter(i => i.fileName === '.claude.json');
67+
const withProjectKey = claudeJsonItems.filter(i => i.claudeJsonProjectKey);
68+
69+
console.log(` MCP items: ${mcpItems.length}, from .claude.json: ${claudeJsonItems.length}, with projectKey: ${withProjectKey.length}`);
70+
if (claudeJsonItems.length > 0) ok(`found ${claudeJsonItems.length} .claude.json servers`);
71+
else skip('claudeJson servers', 'no .claude.json MCP servers found');
72+
73+
if (withProjectKey.length > 0) {
74+
for (const item of withProjectKey) {
75+
console.log(` ${item.name} → projectKey: ${item.claudeJsonProjectKey.slice(-40)}`);
76+
}
77+
ok(`${withProjectKey.length} servers have claudeJsonProjectKey`);
78+
} else {
79+
skip('claudeJsonProjectKey', 'no project-scope servers in .claude.json (need `claude mcp add --scope project`)');
80+
}
81+
82+
// ═══ TEST 5: No duplicate MCP server from same file ═══
83+
console.log('\nTEST 5: No duplicate MCP servers (same scope + same file)');
84+
const seen = new Set();
85+
let dupes = 0;
86+
for (const item of mcpItems) {
87+
// Dupes across DIFFERENT files (e.g. .mcp.json vs .claude.json) are user config issues, not bugs
88+
const key = `${item.scopeId}::${item.name}::${item.path}`;
89+
if (seen.has(key)) { dupes++; console.log(` DUPE: ${item.name} in ${item.scopeId} (${item.path})`); }
90+
seen.add(key);
91+
}
92+
if (dupes === 0) ok('no duplicates from same file');
93+
else fail(`${dupes} duplicate MCP servers from same file`, '');
94+
95+
// ═══ TEST 6: No JS errors ═══
96+
console.log('\nTEST 6: No JavaScript errors');
97+
if (errors.length === 0) ok('zero JS errors');
98+
else fail(`${errors.length} JS errors`, errors.join('; '));
99+
100+
// Summary
101+
await page.screenshot({ path: '/tmp/pw-windows-fix.png' });
102+
console.log(`\n═══ RESULTS: ${passed} passed, ${failed} failed, ${skipped} skipped ═══`);
103+
if (failed > 0) process.exitCode = 1;
104+
} catch (e) {
105+
console.error('❌ FATAL:', e.message);
106+
await page.screenshot({ path: '/tmp/pw-windows-fix-err.png' });
107+
process.exitCode = 1;
108+
} finally {
109+
await page.waitForTimeout(2000);
110+
await browser.close();
111+
}
112+
})();

0 commit comments

Comments
 (0)