fix(core): resolve MCP tool naming, IPC security, and migration tests
Pre-PR validation fixes for the .auto-claude → .aperant rename: - Align MCP tool name constants (mcp__aperant__*) across registry and tool definitions to prevent silent agent tool call failures - Use IPC_CHANNELS constants and projectId-based lookup instead of raw strings and paths in migration handlers (security) - Add 9 unit tests for needsMigration() and migrateProject() - Replace console.error with debugLog in migration code - Add ensureGitignoreEntries fallback in migrateProject() - Clear migrateError state on dialog close
This commit is contained in:
@@ -0,0 +1,171 @@
|
||||
/**
|
||||
* Tests for needsMigration() and migrateProject() in project-initializer.ts
|
||||
*/
|
||||
|
||||
import { describe, test, expect, vi, beforeEach } from 'vitest';
|
||||
|
||||
// ---- fs mock ----
|
||||
const mockExistingPaths = new Set<string>();
|
||||
const mockFiles = new Map<string, string>();
|
||||
|
||||
vi.mock('fs', () => {
|
||||
const existsSync = vi.fn((p: string) => mockExistingPaths.has(p));
|
||||
|
||||
const renameSync = vi.fn((oldPath: string, newPath: string) => {
|
||||
// Simulate rename: remove old, add new
|
||||
mockExistingPaths.delete(oldPath);
|
||||
mockExistingPaths.add(newPath);
|
||||
});
|
||||
|
||||
const readFileSync = vi.fn((filePath: string, _encoding?: string): string => {
|
||||
const content = mockFiles.get(filePath);
|
||||
if (content === undefined) {
|
||||
const err = new Error(`ENOENT: no such file or directory, open '${filePath}'`) as NodeJS.ErrnoException;
|
||||
err.code = 'ENOENT';
|
||||
throw err;
|
||||
}
|
||||
return content;
|
||||
});
|
||||
|
||||
const writeFileSync = vi.fn((filePath: string, content: string) => {
|
||||
mockFiles.set(filePath, content);
|
||||
});
|
||||
|
||||
const appendFileSync = vi.fn((filePath: string, content: string) => {
|
||||
const existing = mockFiles.get(filePath) ?? '';
|
||||
mockFiles.set(filePath, existing + content);
|
||||
});
|
||||
|
||||
const mkdirSync = vi.fn();
|
||||
|
||||
return {
|
||||
default: { existsSync, renameSync, readFileSync, writeFileSync, appendFileSync, mkdirSync },
|
||||
existsSync,
|
||||
renameSync,
|
||||
readFileSync,
|
||||
writeFileSync,
|
||||
appendFileSync,
|
||||
mkdirSync,
|
||||
};
|
||||
});
|
||||
|
||||
// ---- stub heavy transitive deps ----
|
||||
vi.mock('child_process', () => ({
|
||||
execFileSync: vi.fn(() => ''),
|
||||
}));
|
||||
|
||||
vi.mock('../cli-tool-manager', () => ({
|
||||
getToolPath: vi.fn(() => 'git'),
|
||||
}));
|
||||
|
||||
// ---- import after mocks ----
|
||||
import { needsMigration, migrateProject } from '../project-initializer';
|
||||
import * as fs from 'fs';
|
||||
|
||||
const PROJECT = '/test/project';
|
||||
const OLD_PATH = `${PROJECT}/.auto-claude`;
|
||||
const NEW_PATH = `${PROJECT}/.aperant`;
|
||||
const GITIGNORE = `${PROJECT}/.gitignore`;
|
||||
|
||||
beforeEach(() => {
|
||||
vi.clearAllMocks();
|
||||
mockExistingPaths.clear();
|
||||
mockFiles.clear();
|
||||
});
|
||||
|
||||
// ────────────────────────────────────────────────────────────
|
||||
// needsMigration()
|
||||
// ────────────────────────────────────────────────────────────
|
||||
|
||||
describe('needsMigration', () => {
|
||||
test('returns true when .auto-claude exists and .aperant does not', () => {
|
||||
mockExistingPaths.add(OLD_PATH);
|
||||
expect(needsMigration(PROJECT)).toBe(true);
|
||||
});
|
||||
|
||||
test('returns false when .aperant already exists', () => {
|
||||
mockExistingPaths.add(OLD_PATH);
|
||||
mockExistingPaths.add(NEW_PATH);
|
||||
expect(needsMigration(PROJECT)).toBe(false);
|
||||
});
|
||||
|
||||
test('returns false when neither exists', () => {
|
||||
expect(needsMigration(PROJECT)).toBe(false);
|
||||
});
|
||||
|
||||
test('returns false when both exist', () => {
|
||||
mockExistingPaths.add(OLD_PATH);
|
||||
mockExistingPaths.add(NEW_PATH);
|
||||
expect(needsMigration(PROJECT)).toBe(false);
|
||||
});
|
||||
});
|
||||
|
||||
// ────────────────────────────────────────────────────────────
|
||||
// migrateProject()
|
||||
// ────────────────────────────────────────────────────────────
|
||||
|
||||
describe('migrateProject', () => {
|
||||
test('successfully renames .auto-claude to .aperant', () => {
|
||||
mockExistingPaths.add(OLD_PATH);
|
||||
|
||||
const result = migrateProject(PROJECT);
|
||||
|
||||
expect(result.success).toBe(true);
|
||||
expect(fs.renameSync).toHaveBeenCalledWith(OLD_PATH, NEW_PATH);
|
||||
});
|
||||
|
||||
test('returns error when .auto-claude does not exist', () => {
|
||||
// OLD_PATH not in mockExistingPaths
|
||||
|
||||
const result = migrateProject(PROJECT);
|
||||
|
||||
expect(result.success).toBe(false);
|
||||
expect(result.error).toMatch(/No \.auto-claude directory/i);
|
||||
expect(fs.renameSync).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
test('returns error when .aperant already exists', () => {
|
||||
mockExistingPaths.add(OLD_PATH);
|
||||
mockExistingPaths.add(NEW_PATH);
|
||||
|
||||
const result = migrateProject(PROJECT);
|
||||
|
||||
expect(result.success).toBe(false);
|
||||
expect(result.error).toMatch(/\.aperant directory already exists/i);
|
||||
expect(fs.renameSync).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
test('updates .gitignore entries during migration', () => {
|
||||
mockExistingPaths.add(OLD_PATH);
|
||||
mockFiles.set(GITIGNORE, '.auto-claude/\n.auto-claude-security.json\n.auto-claude-status\n');
|
||||
|
||||
const result = migrateProject(PROJECT);
|
||||
|
||||
expect(result.success).toBe(true);
|
||||
expect(fs.writeFileSync).toHaveBeenCalled();
|
||||
// Find the call that wrote to the gitignore
|
||||
const gitignoreWrite = (fs.writeFileSync as ReturnType<typeof vi.fn>).mock.calls.find(
|
||||
(call: unknown[]) => call[0] === GITIGNORE
|
||||
);
|
||||
expect(gitignoreWrite).toBeDefined();
|
||||
const writtenContent = gitignoreWrite![1] as string;
|
||||
expect(writtenContent).toContain('.aperant/');
|
||||
expect(writtenContent).not.toContain('.auto-claude/');
|
||||
});
|
||||
|
||||
test('handles .gitignore not existing gracefully', () => {
|
||||
mockExistingPaths.add(OLD_PATH);
|
||||
// mockFiles has no GITIGNORE entry → readFileSync throws ENOENT
|
||||
|
||||
// Should not throw; the catch block swallows the .gitignore read error,
|
||||
// then ensureGitignoreEntries creates the file via writeFileSync
|
||||
const result = migrateProject(PROJECT);
|
||||
expect(result.success).toBe(true);
|
||||
// ensureGitignoreEntries creates a new .gitignore with .aperant/
|
||||
const written = (fs.writeFileSync as ReturnType<typeof vi.fn>).mock.calls.find(
|
||||
(call: unknown[]) => call[0] === GITIGNORE
|
||||
);
|
||||
expect(written).toBeDefined();
|
||||
expect(written![1] as string).toContain('.aperant/');
|
||||
});
|
||||
});
|
||||
@@ -5,7 +5,7 @@
|
||||
* Reports current build progress from implementation_plan.json.
|
||||
* See apps/desktop/src/main/ai/tools/auto-claude/get-build-progress.ts for the TypeScript implementation.
|
||||
*
|
||||
* Tool name: mcp__auto-claude__get_build_progress
|
||||
* Tool name: mcp__aperant__get_build_progress
|
||||
*/
|
||||
|
||||
import * as fs from 'node:fs';
|
||||
@@ -50,7 +50,7 @@ interface ImplementationPlan {
|
||||
|
||||
export const getBuildProgressTool = Tool.define({
|
||||
metadata: {
|
||||
name: 'mcp__auto-claude__get_build_progress',
|
||||
name: 'mcp__aperant__get_build_progress',
|
||||
description:
|
||||
'Get the current build progress including completed subtasks, pending subtasks, and next subtask to work on.',
|
||||
permission: ToolPermission.ReadOnly,
|
||||
|
||||
@@ -9,7 +9,7 @@
|
||||
*
|
||||
* See apps/desktop/src/main/ai/tools/auto-claude/get-session-context.ts for the TypeScript implementation.
|
||||
*
|
||||
* Tool name: mcp__auto-claude__get_session_context
|
||||
* Tool name: mcp__aperant__get_session_context
|
||||
*/
|
||||
|
||||
import * as fs from 'node:fs';
|
||||
@@ -40,7 +40,7 @@ interface CodebaseMap {
|
||||
|
||||
export const getSessionContextTool = Tool.define({
|
||||
metadata: {
|
||||
name: 'mcp__auto-claude__get_session_context',
|
||||
name: 'mcp__aperant__get_session_context',
|
||||
description:
|
||||
'Get context from previous sessions including codebase discoveries, gotchas, and patterns. Call this at the start of a session to pick up where the last session left off.',
|
||||
permission: ToolPermission.ReadOnly,
|
||||
|
||||
@@ -5,7 +5,7 @@
|
||||
* Barrel export for all auto-claude builtin tools.
|
||||
* These replace the Python tools_pkg/tools/* implementations.
|
||||
*
|
||||
* Tool names follow the mcp__auto-claude__* convention to match the
|
||||
* Tool names follow the mcp__aperant__* convention to match the
|
||||
* TOOL_* constants in registry.ts and AGENT_CONFIGS autoClaudeTools arrays.
|
||||
*/
|
||||
|
||||
|
||||
@@ -5,7 +5,7 @@
|
||||
* Records a codebase discovery to session memory (codebase_map.json).
|
||||
* See apps/desktop/src/main/ai/tools/auto-claude/record-discovery.ts for the TypeScript implementation.
|
||||
*
|
||||
* Tool name: mcp__auto-claude__record_discovery
|
||||
* Tool name: mcp__aperant__record_discovery
|
||||
*/
|
||||
|
||||
import * as fs from 'node:fs';
|
||||
@@ -44,7 +44,7 @@ interface CodebaseMap {
|
||||
|
||||
export const recordDiscoveryTool = Tool.define({
|
||||
metadata: {
|
||||
name: 'mcp__auto-claude__record_discovery',
|
||||
name: 'mcp__aperant__record_discovery',
|
||||
description:
|
||||
'Record a codebase discovery to session memory. Use this when you learn something important about the codebase structure or behavior.',
|
||||
permission: ToolPermission.Auto,
|
||||
|
||||
@@ -5,7 +5,7 @@
|
||||
* Records a gotcha or pitfall to specDir/memory/gotchas.md.
|
||||
* See apps/desktop/src/main/ai/tools/auto-claude/record-gotcha.ts for the TypeScript implementation.
|
||||
*
|
||||
* Tool name: mcp__auto-claude__record_gotcha
|
||||
* Tool name: mcp__aperant__record_gotcha
|
||||
*/
|
||||
|
||||
import * as fs from 'node:fs';
|
||||
@@ -33,7 +33,7 @@ const inputSchema = z.object({
|
||||
|
||||
export const recordGotchaTool = Tool.define({
|
||||
metadata: {
|
||||
name: 'mcp__auto-claude__record_gotcha',
|
||||
name: 'mcp__aperant__record_gotcha',
|
||||
description:
|
||||
'Record a gotcha or pitfall to avoid. Use this when you encounter something that future sessions should know about to avoid repeating mistakes.',
|
||||
permission: ToolPermission.Auto,
|
||||
|
||||
@@ -5,7 +5,7 @@
|
||||
* Updates the QA sign-off status in implementation_plan.json.
|
||||
* See apps/desktop/src/main/ai/tools/auto-claude/update-qa-status.ts for the TypeScript implementation.
|
||||
*
|
||||
* Tool name: mcp__auto-claude__update_qa_status
|
||||
* Tool name: mcp__aperant__update_qa_status
|
||||
*
|
||||
* IMPORTANT: Do NOT write plan["status"] or plan["planStatus"] here.
|
||||
* The frontend XState task state machine owns status transitions.
|
||||
@@ -68,7 +68,7 @@ interface ImplementationPlan {
|
||||
|
||||
export const updateQaStatusTool = Tool.define({
|
||||
metadata: {
|
||||
name: 'mcp__auto-claude__update_qa_status',
|
||||
name: 'mcp__aperant__update_qa_status',
|
||||
description:
|
||||
'Update the QA sign-off status in implementation_plan.json. Use this after completing a QA review to record the outcome.',
|
||||
permission: ToolPermission.Auto,
|
||||
|
||||
@@ -5,7 +5,7 @@
|
||||
* Updates the status of a subtask in implementation_plan.json.
|
||||
* See apps/desktop/src/main/ai/tools/auto-claude/update-subtask-status.ts for the TypeScript implementation.
|
||||
*
|
||||
* Tool name: mcp__auto-claude__update_subtask_status
|
||||
* Tool name: mcp__aperant__update_subtask_status
|
||||
*/
|
||||
|
||||
import * as fs from 'node:fs';
|
||||
@@ -82,7 +82,7 @@ function updateSubtaskInPlan(
|
||||
|
||||
export const updateSubtaskStatusTool = Tool.define({
|
||||
metadata: {
|
||||
name: 'mcp__auto-claude__update_subtask_status',
|
||||
name: 'mcp__aperant__update_subtask_status',
|
||||
description:
|
||||
'Update the status of a subtask in implementation_plan.json. Use this when completing or starting a subtask.',
|
||||
permission: ToolPermission.Auto,
|
||||
|
||||
@@ -45,12 +45,12 @@ export {
|
||||
export const BASE_READ_TOOLS = ['Read', 'Glob', 'Grep'] as const;
|
||||
export const BASE_WRITE_TOOLS = ['Write', 'Edit', 'Bash'] as const;
|
||||
export const WEB_TOOLS = ['WebFetch', 'WebSearch'] as const;
|
||||
export const TOOL_UPDATE_SUBTASK_STATUS = 'mcp__auto-claude__update_subtask_status';
|
||||
export const TOOL_GET_BUILD_PROGRESS = 'mcp__auto-claude__get_build_progress';
|
||||
export const TOOL_RECORD_DISCOVERY = 'mcp__auto-claude__record_discovery';
|
||||
export const TOOL_RECORD_GOTCHA = 'mcp__auto-claude__record_gotcha';
|
||||
export const TOOL_GET_SESSION_CONTEXT = 'mcp__auto-claude__get_session_context';
|
||||
export const TOOL_UPDATE_QA_STATUS = 'mcp__auto-claude__update_qa_status';
|
||||
export const TOOL_UPDATE_SUBTASK_STATUS = 'mcp__aperant__update_subtask_status';
|
||||
export const TOOL_GET_BUILD_PROGRESS = 'mcp__aperant__get_build_progress';
|
||||
export const TOOL_RECORD_DISCOVERY = 'mcp__aperant__record_discovery';
|
||||
export const TOOL_RECORD_GOTCHA = 'mcp__aperant__record_gotcha';
|
||||
export const TOOL_GET_SESSION_CONTEXT = 'mcp__aperant__get_session_context';
|
||||
export const TOOL_UPDATE_QA_STATUS = 'mcp__aperant__update_qa_status';
|
||||
|
||||
// =============================================================================
|
||||
// MCP Config for dynamic server resolution
|
||||
|
||||
@@ -448,13 +448,17 @@ export function registerProjectHandlers(
|
||||
// ============================================
|
||||
|
||||
// Check if project needs migration from .auto-claude to .aperant
|
||||
ipcMain.handle('project:needs-migration', async (_event, projectPath: string) => {
|
||||
return needsMigration(projectPath);
|
||||
ipcMain.handle(IPC_CHANNELS.PROJECT_NEEDS_MIGRATION, async (_, projectId: string) => {
|
||||
const project = projectStore.getProject(projectId);
|
||||
if (!project) return false;
|
||||
return needsMigration(project.path);
|
||||
});
|
||||
|
||||
// Migrate project from .auto-claude to .aperant
|
||||
ipcMain.handle('project:migrate', async (_event, projectPath: string) => {
|
||||
return migrateProject(projectPath);
|
||||
ipcMain.handle(IPC_CHANNELS.PROJECT_MIGRATE, async (_, projectId: string) => {
|
||||
const project = projectStore.getProject(projectId);
|
||||
if (!project) return { success: false, error: 'Project not found' };
|
||||
return migrateProject(project.path);
|
||||
});
|
||||
|
||||
// ============================================
|
||||
|
||||
@@ -431,6 +431,8 @@ export function migrateProject(projectPath: string): InitializationResult {
|
||||
} catch {
|
||||
// .gitignore update is non-critical
|
||||
}
|
||||
// Ensure .aperant/ is ignored even if .auto-claude/ was never in .gitignore
|
||||
ensureGitignoreEntries(projectPath, ['.aperant/']);
|
||||
|
||||
debug('Migration complete: .auto-claude → .aperant');
|
||||
return { success: true };
|
||||
|
||||
@@ -30,8 +30,8 @@ export interface ProjectAPI {
|
||||
) => Promise<IPCResult>;
|
||||
initializeProject: (projectId: string) => Promise<IPCResult<InitializationResult>>;
|
||||
checkProjectVersion: (projectId: string) => Promise<IPCResult<AutoBuildVersionInfo>>;
|
||||
needsMigration: (projectPath: string) => Promise<boolean>;
|
||||
migrateProject: (projectPath: string) => Promise<{ success: boolean; error?: string }>;
|
||||
needsMigration: (projectId: string) => Promise<boolean>;
|
||||
migrateProject: (projectId: string) => Promise<{ success: boolean; error?: string }>;
|
||||
|
||||
// Tab State (persisted in main process for reliability)
|
||||
getTabState: () => Promise<IPCResult<TabState>>;
|
||||
@@ -160,11 +160,11 @@ export const createProjectAPI = (): ProjectAPI => ({
|
||||
checkProjectVersion: (projectId: string): Promise<IPCResult<AutoBuildVersionInfo>> =>
|
||||
ipcRenderer.invoke(IPC_CHANNELS.PROJECT_CHECK_VERSION, projectId),
|
||||
|
||||
needsMigration: (projectPath: string): Promise<boolean> =>
|
||||
ipcRenderer.invoke('project:needs-migration', projectPath),
|
||||
needsMigration: (projectId: string): Promise<boolean> =>
|
||||
ipcRenderer.invoke(IPC_CHANNELS.PROJECT_NEEDS_MIGRATION, projectId),
|
||||
|
||||
migrateProject: (projectPath: string): Promise<{ success: boolean; error?: string }> =>
|
||||
ipcRenderer.invoke('project:migrate', projectPath),
|
||||
migrateProject: (projectId: string): Promise<{ success: boolean; error?: string }> =>
|
||||
ipcRenderer.invoke(IPC_CHANNELS.PROJECT_MIGRATE, projectId),
|
||||
|
||||
// Tab State (persisted in main process for reliability)
|
||||
getTabState: (): Promise<IPCResult<TabState>> =>
|
||||
|
||||
@@ -419,7 +419,7 @@ export function App() {
|
||||
openProjectTab(project.id);
|
||||
// Check migration before init
|
||||
try {
|
||||
const requiresMigration = await window.electronAPI.needsMigration(project.path);
|
||||
const requiresMigration = await window.electronAPI.needsMigration(project.id);
|
||||
if (requiresMigration) {
|
||||
setMigrationProject(project);
|
||||
setMigrateError(null);
|
||||
@@ -662,7 +662,7 @@ export function App() {
|
||||
|
||||
// Check for migration before showing init dialog
|
||||
try {
|
||||
const requiresMigration = await window.electronAPI.needsMigration(project.path);
|
||||
const requiresMigration = await window.electronAPI.needsMigration(project.id);
|
||||
if (requiresMigration) {
|
||||
setMigrationProject(project);
|
||||
setMigrateError(null);
|
||||
@@ -670,7 +670,7 @@ export function App() {
|
||||
return;
|
||||
}
|
||||
} catch (error) {
|
||||
console.error('[App] Failed to check migration status:', error);
|
||||
debugLog('[App] Failed to check migration status:', error);
|
||||
}
|
||||
|
||||
if (needsInit) {
|
||||
@@ -751,8 +751,9 @@ export function App() {
|
||||
setIsMigrating(true);
|
||||
setMigrateError(null);
|
||||
try {
|
||||
const result = await window.electronAPI.migrateProject(migrationProject.path);
|
||||
const result = await window.electronAPI.migrateProject(migrationProject.id);
|
||||
if (result.success) {
|
||||
setMigrateError(null);
|
||||
setShowMigrateDialog(false);
|
||||
// Refresh projects so the updated autoBuildPath is reflected
|
||||
await loadProjects();
|
||||
|
||||
@@ -11,6 +11,8 @@ export const IPC_CHANNELS = {
|
||||
PROJECT_UPDATE_SETTINGS: 'project:updateSettings',
|
||||
PROJECT_INITIALIZE: 'project:initialize',
|
||||
PROJECT_CHECK_VERSION: 'project:checkVersion',
|
||||
PROJECT_NEEDS_MIGRATION: 'project:needs-migration',
|
||||
PROJECT_MIGRATE: 'project:migrate',
|
||||
|
||||
// Tab state operations (persisted in main process)
|
||||
TAB_STATE_GET: 'tabState:get',
|
||||
|
||||
@@ -185,8 +185,8 @@ export interface ElectronAPI {
|
||||
updateProjectSettings: (projectId: string, settings: Partial<ProjectSettings>) => Promise<IPCResult>;
|
||||
initializeProject: (projectId: string) => Promise<IPCResult<InitializationResult>>;
|
||||
checkProjectVersion: (projectId: string) => Promise<IPCResult<AutoBuildVersionInfo>>;
|
||||
needsMigration: (projectPath: string) => Promise<boolean>;
|
||||
migrateProject: (projectPath: string) => Promise<{ success: boolean; error?: string }>;
|
||||
needsMigration: (projectId: string) => Promise<boolean>;
|
||||
migrateProject: (projectId: string) => Promise<{ success: boolean; error?: string }>;
|
||||
|
||||
// Tab State (persisted in main process for reliability)
|
||||
getTabState: () => Promise<IPCResult<TabState>>;
|
||||
|
||||
Reference in New Issue
Block a user