mirror of
https://github.com/thedotmack/claude-mem
synced 2026-04-25 17:15:04 +02:00
fix: address PR review — shebang, double-escaping, data loss, uninstall scope
- Add shebang banner to NPX CLI esbuild config so npx claude-mem works - Remove manual backslash pre-escaping in WindsurfHooksInstaller (JSON.stringify handles it) - Scope cache deletion to claude-mem only, not entire vendor namespace - Use getWorkerPort() in OpenCodeInstaller instead of hard-coded 37777 - Throw on corrupt JSON in readJsonSafe/readGeminiSettings/Windsurf to prevent data loss - Fix Cursor install stub to warn instead of silently succeeding - Fix Gemini uninstall to remove individual hooks within groups, not whole groups - Update tests for new corrupt-file-throws behavior Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -200,6 +200,7 @@ async function buildHooks() {
|
||||
target: 'node18',
|
||||
format: 'esm',
|
||||
outfile: `${npxCliOutDir}/index.js`,
|
||||
banner: { js: '#!/usr/bin/env node' },
|
||||
minify: true,
|
||||
logLevel: 'error',
|
||||
external: [
|
||||
|
||||
@@ -123,8 +123,7 @@ async function setupIDEs(selectedIDEs: string[]): Promise<void> {
|
||||
break;
|
||||
|
||||
case 'cursor':
|
||||
log.info('Cursor: hook configuration available after first launch.');
|
||||
log.info(` Run: npx claude-mem cursor-setup (coming soon)`);
|
||||
log.warn('Cursor: integration not yet implemented. Skipping.');
|
||||
break;
|
||||
|
||||
case 'gemini-cli': {
|
||||
|
||||
@@ -36,9 +36,9 @@ function removeMarketplaceDirectory(): boolean {
|
||||
}
|
||||
|
||||
function removeCacheDirectory(): boolean {
|
||||
const cacheBaseDirectory = join(pluginsDirectory(), 'cache', 'thedotmack');
|
||||
if (existsSync(cacheBaseDirectory)) {
|
||||
rmSync(cacheBaseDirectory, { recursive: true, force: true });
|
||||
const cacheDirectory = join(pluginsDirectory(), 'cache', 'thedotmack', 'claude-mem');
|
||||
if (existsSync(cacheDirectory)) {
|
||||
rmSync(cacheDirectory, { recursive: true, force: true });
|
||||
return true;
|
||||
}
|
||||
return false;
|
||||
|
||||
@@ -147,19 +147,19 @@ function createHookGroup(hookCommand: string): GeminiHookGroup {
|
||||
// ============================================================================
|
||||
|
||||
/**
|
||||
* Read ~/.gemini/settings.json, returning empty object if missing/corrupt.
|
||||
* Read ~/.gemini/settings.json, returning empty object if missing.
|
||||
* Throws on corrupt JSON to prevent silent data loss.
|
||||
*/
|
||||
function readGeminiSettings(): GeminiSettingsJson {
|
||||
if (!existsSync(GEMINI_SETTINGS_PATH)) {
|
||||
return {};
|
||||
}
|
||||
|
||||
const content = readFileSync(GEMINI_SETTINGS_PATH, 'utf-8');
|
||||
try {
|
||||
const content = readFileSync(GEMINI_SETTINGS_PATH, 'utf-8');
|
||||
return JSON.parse(content) as GeminiSettingsJson;
|
||||
} catch (error) {
|
||||
logger.warn('GEMINI', `Failed to parse ${GEMINI_SETTINGS_PATH}, treating as empty`, {});
|
||||
return {};
|
||||
throw new Error(`Corrupt JSON in ${GEMINI_SETTINGS_PATH}, refusing to overwrite user settings`);
|
||||
}
|
||||
}
|
||||
|
||||
@@ -358,15 +358,15 @@ export function uninstallGeminiCliHooks(): number {
|
||||
|
||||
let removedCount = 0;
|
||||
|
||||
// Remove claude-mem hooks from each event
|
||||
// Remove claude-mem hooks from within each group, preserving other hooks
|
||||
for (const [eventName, groups] of Object.entries(settings.hooks)) {
|
||||
const filteredGroups = groups.filter(group =>
|
||||
!group.hooks.some(hook => hook.name === HOOK_NAME)
|
||||
);
|
||||
|
||||
if (filteredGroups.length < groups.length) {
|
||||
removedCount += groups.length - filteredGroups.length;
|
||||
}
|
||||
const filteredGroups = groups
|
||||
.map(group => {
|
||||
const remainingHooks = group.hooks.filter(hook => hook.name !== HOOK_NAME);
|
||||
removedCount += group.hooks.length - remainingHooks.length;
|
||||
return { ...group, hooks: remainingHooks };
|
||||
})
|
||||
.filter(group => group.hooks.length > 0);
|
||||
|
||||
if (filteredGroups.length > 0) {
|
||||
settings.hooks[eventName] = filteredGroups;
|
||||
@@ -381,7 +381,7 @@ export function uninstallGeminiCliHooks(): number {
|
||||
}
|
||||
|
||||
writeGeminiSettings(settings);
|
||||
console.log(` Removed ${removedCount} claude-mem hook group(s) from ${GEMINI_SETTINGS_PATH}`);
|
||||
console.log(` Removed ${removedCount} claude-mem hook(s) from ${GEMINI_SETTINGS_PATH}`);
|
||||
|
||||
// Remove claude-mem context section from GEMINI.md
|
||||
if (existsSync(GEMINI_MD_PATH)) {
|
||||
|
||||
@@ -19,6 +19,7 @@ import { homedir } from 'os';
|
||||
import { existsSync, readFileSync, writeFileSync, mkdirSync, copyFileSync, unlinkSync } from 'fs';
|
||||
import { logger } from '../../utils/logger.js';
|
||||
import { CONTEXT_TAG_OPEN, CONTEXT_TAG_CLOSE, injectContextIntoMarkdownFile } from '../../utils/context-injection.js';
|
||||
import { getWorkerPort } from '../../shared/worker-utils.js';
|
||||
|
||||
// ============================================================================
|
||||
// Path Resolution
|
||||
@@ -301,10 +302,11 @@ Use claude-mem search tools for manual memory queries.`;
|
||||
|
||||
// Try to fetch real context from worker first
|
||||
try {
|
||||
const healthResponse = await fetch('http://127.0.0.1:37777/api/readiness');
|
||||
const workerPort = getWorkerPort();
|
||||
const healthResponse = await fetch(`http://127.0.0.1:${workerPort}/api/readiness`);
|
||||
if (healthResponse.ok) {
|
||||
const contextResponse = await fetch(
|
||||
`http://127.0.0.1:37777/api/context/inject?project=opencode`,
|
||||
`http://127.0.0.1:${workerPort}/api/context/inject?project=opencode`,
|
||||
);
|
||||
if (contextResponse.ok) {
|
||||
const realContext = await contextResponse.text();
|
||||
|
||||
@@ -213,11 +213,7 @@ function buildHookCommand(bunPath: string, workerServicePath: string, eventName:
|
||||
|
||||
const hookCommand = eventToCommand[eventName] ?? 'observation';
|
||||
|
||||
// Escape backslashes for JSON on Windows
|
||||
const escapedBunPath = bunPath.replace(/\\/g, '\\\\');
|
||||
const escapedWorkerPath = workerServicePath.replace(/\\/g, '\\\\');
|
||||
|
||||
return `"${escapedBunPath}" "${escapedWorkerPath}" hook windsurf ${hookCommand}`;
|
||||
return `"${bunPath}" "${workerServicePath}" hook windsurf ${hookCommand}`;
|
||||
}
|
||||
|
||||
/**
|
||||
@@ -240,10 +236,7 @@ function mergeAndWriteHooksJson(
|
||||
existingConfig.hooks = {};
|
||||
}
|
||||
} catch (error) {
|
||||
logger.error('WINDSURF', 'Corrupt hooks.json, starting fresh', {
|
||||
path: WINDSURF_HOOKS_JSON_PATH,
|
||||
}, error as Error);
|
||||
existingConfig = { hooks: {} };
|
||||
throw new Error(`Corrupt hooks.json at ${WINDSURF_HOOKS_JSON_PATH}, refusing to overwrite`);
|
||||
}
|
||||
}
|
||||
|
||||
@@ -410,9 +403,7 @@ export function uninstallWindsurfHooks(): number {
|
||||
console.log(` Removed claude-mem entries from hooks.json (other hooks preserved)`);
|
||||
}
|
||||
} catch (error) {
|
||||
// Corrupt file — just remove it
|
||||
unlinkSync(WINDSURF_HOOKS_JSON_PATH);
|
||||
console.log(` Removed corrupt hooks.json`);
|
||||
console.log(` Warning: could not parse hooks.json — leaving file intact to preserve other hooks`);
|
||||
}
|
||||
} else {
|
||||
console.log(` No hooks.json found`);
|
||||
|
||||
@@ -9,18 +9,19 @@ import { logger } from './logger.js';
|
||||
|
||||
/**
|
||||
* Read a JSON file safely, returning a default value if the file
|
||||
* does not exist or contains corrupt JSON.
|
||||
* does not exist. Throws on corrupt JSON to prevent silent data loss
|
||||
* when callers merge and write back.
|
||||
*
|
||||
* @param filePath - Absolute path to the JSON file.
|
||||
* @param defaultValue - Value returned when the file is missing or unreadable.
|
||||
* @returns The parsed JSON content, or `defaultValue` on failure.
|
||||
* @param defaultValue - Value returned when the file is missing.
|
||||
* @returns The parsed JSON content, or `defaultValue` when file is missing.
|
||||
* @throws {Error} When the file exists but contains invalid JSON.
|
||||
*/
|
||||
export function readJsonSafe<T>(filePath: string, defaultValue: T): T {
|
||||
if (!existsSync(filePath)) return defaultValue;
|
||||
try {
|
||||
return JSON.parse(readFileSync(filePath, 'utf-8'));
|
||||
} catch (error) {
|
||||
logger.error('JSON', `Corrupt JSON file, using default`, { path: filePath }, error as Error);
|
||||
return defaultValue;
|
||||
throw new Error(`Corrupt JSON file, refusing to overwrite: ${filePath}`);
|
||||
}
|
||||
}
|
||||
|
||||
@@ -46,23 +46,22 @@ describe('JSON Utils', () => {
|
||||
expect(result).toEqual(data);
|
||||
});
|
||||
|
||||
it('returns default value for corrupt JSON file', () => {
|
||||
it('throws on corrupt JSON file to prevent data loss', () => {
|
||||
const filePath = join(tempDir, 'corrupt.json');
|
||||
writeFileSync(filePath, 'this is not valid json {{{');
|
||||
|
||||
const defaultValue = { recovered: true };
|
||||
const result = readJsonSafe(filePath, defaultValue);
|
||||
|
||||
expect(result).toEqual(defaultValue);
|
||||
expect(() => readJsonSafe(filePath, { recovered: true })).toThrow(
|
||||
/Corrupt JSON file, refusing to overwrite/
|
||||
);
|
||||
});
|
||||
|
||||
it('returns default value for empty file', () => {
|
||||
it('throws on empty file to prevent data loss', () => {
|
||||
const filePath = join(tempDir, 'empty.json');
|
||||
writeFileSync(filePath, '');
|
||||
|
||||
const result = readJsonSafe(filePath, []);
|
||||
|
||||
expect(result).toEqual([]);
|
||||
expect(() => readJsonSafe(filePath, [])).toThrow(
|
||||
/Corrupt JSON file, refusing to overwrite/
|
||||
);
|
||||
});
|
||||
|
||||
it('works with array default values', () => {
|
||||
|
||||
@@ -192,35 +192,34 @@ describe('MCP Integrations', () => {
|
||||
});
|
||||
|
||||
describe('corrupt file recovery', () => {
|
||||
it('replaces corrupt JSON with fresh config', () => {
|
||||
it('throws on corrupt JSON to prevent data loss', () => {
|
||||
const configPath = join(tempDir, 'mcp.json');
|
||||
writeFileSync(configPath, 'not valid json {{{{');
|
||||
|
||||
// readJsonSafe returns default {} for corrupt file
|
||||
writeMcpJsonConfig(configPath, '/path/to/mcp.cjs');
|
||||
expect(() => writeMcpJsonConfig(configPath, '/path/to/mcp.cjs')).toThrow(
|
||||
/Corrupt JSON file, refusing to overwrite/
|
||||
);
|
||||
|
||||
const config = JSON.parse(readFileSync(configPath, 'utf-8'));
|
||||
expect(config.mcpServers['claude-mem']).toBeDefined();
|
||||
// Original file should be untouched
|
||||
expect(readFileSync(configPath, 'utf-8')).toBe('not valid json {{{{');
|
||||
});
|
||||
|
||||
it('handles empty file gracefully', () => {
|
||||
it('throws on empty file to prevent data loss', () => {
|
||||
const configPath = join(tempDir, 'mcp.json');
|
||||
writeFileSync(configPath, '');
|
||||
|
||||
writeMcpJsonConfig(configPath, '/path/to/mcp.cjs');
|
||||
|
||||
const config = JSON.parse(readFileSync(configPath, 'utf-8'));
|
||||
expect(config.mcpServers['claude-mem']).toBeDefined();
|
||||
expect(() => writeMcpJsonConfig(configPath, '/path/to/mcp.cjs')).toThrow(
|
||||
/Corrupt JSON file, refusing to overwrite/
|
||||
);
|
||||
});
|
||||
|
||||
it('handles file with only whitespace', () => {
|
||||
it('throws on file with only whitespace', () => {
|
||||
const configPath = join(tempDir, 'mcp.json');
|
||||
writeFileSync(configPath, ' \n\n ');
|
||||
|
||||
writeMcpJsonConfig(configPath, '/path/to/mcp.cjs');
|
||||
|
||||
const config = JSON.parse(readFileSync(configPath, 'utf-8'));
|
||||
expect(config.mcpServers['claude-mem']).toBeDefined();
|
||||
expect(() => writeMcpJsonConfig(configPath, '/path/to/mcp.cjs')).toThrow(
|
||||
/Corrupt JSON file, refusing to overwrite/
|
||||
);
|
||||
});
|
||||
});
|
||||
|
||||
|
||||
Reference in New Issue
Block a user