From 93326be3e9c3721cdb90d4478b732a621afbd38e Mon Sep 17 00:00:00 2001 From: Tabish Bidiwale Date: Sat, 25 Oct 2025 19:37:48 +1100 Subject: [PATCH 1/2] fix: prevent false "already configured" detection for tools MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fixes #195 ## Problem Users with existing tool config files (like CLAUDE.md) would see those tools marked as "already configured" even when running `openspec init` for the first time. This caused confusion as users thought OpenSpec was already set up when it wasn't. Root causes: 1. Tool detection checked only for file existence, not OpenSpec ownership 2. Detection ran even in fresh projects without openspec/ folder ## Solution Two-part fix: 1. **Conditional detection**: Only check tool configuration when in extend mode (when openspec/ directory already exists). Fresh initializations skip the check entirely, treating all tools as unconfigured. 2. **Marker-based validation**: For tools to be considered "configured by OpenSpec", their files must contain OpenSpec markers ( and ). For tools with both config files and slash commands (like Claude Code), BOTH must have markers. ## Changes - Modified getExistingToolStates() to accept extendMode parameter - Rewrote isToolConfigured() to verify OpenSpec markers in files - Added OPENSPEC_MARKERS to imports - Added 4 comprehensive test cases covering the new behavior ## Test Coverage - Fresh init with existing CLAUDE.md → NOT shown as configured ✅ - Fresh init with existing slash commands → NOT shown as configured ✅ - Extend mode with OpenSpec files → shown as configured ✅ - Fresh init with global Codex prompts → NOT shown as configured ✅ All 240 tests pass. --- src/core/init.ts | 92 ++++++++++++++++++++++++++++++++++-------- test/core/init.test.ts | 81 +++++++++++++++++++++++++++++++++++++ 2 files changed, 157 insertions(+), 16 deletions(-) diff --git a/src/core/init.ts b/src/core/init.ts index 1c4f14fb..e702a89f 100644 --- a/src/core/init.ts +++ b/src/core/init.ts @@ -21,6 +21,7 @@ import { AI_TOOLS, OPENSPEC_DIR_NAME, AIToolOption, + OPENSPEC_MARKERS, } from './config.js'; import { PALETTE } from './styles/palette.js'; @@ -388,7 +389,7 @@ export class InitCommand { // Validation happens silently in the background const extendMode = await this.validate(projectPath, openspecPath); - const existingToolStates = await this.getExistingToolStates(projectPath); + const existingToolStates = await this.getExistingToolStates(projectPath, extendMode); this.renderBanner(extendMode); @@ -627,12 +628,25 @@ export class InitCommand { } private async getExistingToolStates( - projectPath: string + projectPath: string, + extendMode: boolean ): Promise> { const states: Record = {}; - for (const tool of AI_TOOLS) { - states[tool.value] = await this.isToolConfigured(projectPath, tool.value); + + // Only check tool configuration if OpenSpec is already initialized (extend mode) + // This prevents false "already configured" messages when users have existing + // tool config files (like CLAUDE.md) that weren't created by OpenSpec + if (extendMode) { + for (const tool of AI_TOOLS) { + states[tool.value] = await this.isToolConfigured(projectPath, tool.value); + } + } else { + // Fresh initialization - no tools configured yet + for (const tool of AI_TOOLS) { + states[tool.value] = false; + } } + return states; } @@ -640,22 +654,68 @@ export class InitCommand { projectPath: string, toolId: string ): Promise { + // A tool is only considered "configured by OpenSpec" if BOTH conditions are met: + // 1. Config file (e.g., CLAUDE.md) exists with OpenSpec markers + // 2. At least one slash command file exists with OpenSpec markers + + let hasConfigFile = false; + let hasSlashCommands = false; + + // Check if the tool has a config file with OpenSpec markers const configFile = ToolRegistry.get(toolId)?.configFileName; - if ( - configFile && - (await FileSystemUtils.fileExists(path.join(projectPath, configFile))) - ) - return true; + if (configFile) { + const configPath = path.join(projectPath, configFile); + if (await FileSystemUtils.fileExists(configPath)) { + try { + const content = await FileSystemUtils.readFile(configPath); + hasConfigFile = content.includes(OPENSPEC_MARKERS.start) && content.includes(OPENSPEC_MARKERS.end); + } catch { + // If we can't read the file, treat it as not configured + hasConfigFile = false; + } + } + } + // Check if slash command files exist with OpenSpec markers const slashConfigurator = SlashCommandRegistry.get(toolId); - if (!slashConfigurator) return false; - for (const target of slashConfigurator.getTargets()) { - const absolute = slashConfigurator.resolveAbsolutePath( - projectPath, - target.id - ); - if (await FileSystemUtils.fileExists(absolute)) return true; + if (slashConfigurator) { + for (const target of slashConfigurator.getTargets()) { + const absolute = slashConfigurator.resolveAbsolutePath( + projectPath, + target.id + ); + if (await FileSystemUtils.fileExists(absolute)) { + try { + const content = await FileSystemUtils.readFile(absolute); + if (content.includes(OPENSPEC_MARKERS.start) && content.includes(OPENSPEC_MARKERS.end)) { + hasSlashCommands = true; + break; // Found at least one valid slash command + } + } catch { + // If we can't read the file, continue checking others + continue; + } + } + } } + + // Tool is only configured if BOTH exist with markers + // OR if the tool has no config file requirement (slash commands only) + // OR if the tool has no slash commands requirement (config file only) + const hasConfigFileRequirement = configFile !== undefined; + const hasSlashCommandRequirement = slashConfigurator !== undefined; + + if (hasConfigFileRequirement && hasSlashCommandRequirement) { + // Both are required - both must be present with markers + return hasConfigFile && hasSlashCommands; + } else if (hasConfigFileRequirement) { + // Only config file required + return hasConfigFile; + } else if (hasSlashCommandRequirement) { + // Only slash commands required + return hasSlashCommands; + } + return false; } diff --git a/test/core/init.test.ts b/test/core/init.test.ts index 99be9740..4c7a91d4 100644 --- a/test/core/init.test.ts +++ b/test/core/init.test.ts @@ -1051,6 +1051,87 @@ describe('InitCommand', () => { }); }); + describe('already configured detection', () => { + it('should NOT show tools as already configured in fresh project with existing CLAUDE.md', async () => { + // Simulate user having their own CLAUDE.md before running openspec init + const claudePath = path.join(testDir, 'CLAUDE.md'); + await fs.writeFile(claudePath, '# My Custom Claude Instructions\n'); + + queueSelections('claude', DONE); + + await initCommand.execute(testDir); + + // In the first run (non-interactive mode via queueSelections), + // the prompt is called with configured: false for claude + const firstCallArgs = mockPrompt.mock.calls[0][0]; + const claudeChoice = firstCallArgs.choices.find( + (choice: any) => choice.value === 'claude' + ); + + expect(claudeChoice.configured).toBe(false); + }); + + it('should NOT show tools as already configured in fresh project with existing slash commands', async () => { + // Simulate user having their own custom slash commands + const customCommandDir = path.join(testDir, '.claude/commands/custom'); + await fs.mkdir(customCommandDir, { recursive: true }); + await fs.writeFile( + path.join(customCommandDir, 'mycommand.md'), + '# My Custom Command\n' + ); + + queueSelections('claude', DONE); + + await initCommand.execute(testDir); + + const firstCallArgs = mockPrompt.mock.calls[0][0]; + const claudeChoice = firstCallArgs.choices.find( + (choice: any) => choice.value === 'claude' + ); + + expect(claudeChoice.configured).toBe(false); + }); + + it('should show tools as already configured in extend mode', async () => { + // First initialization + queueSelections('claude', DONE); + await initCommand.execute(testDir); + + // Second initialization (extend mode) + queueSelections('cursor', DONE); + await initCommand.execute(testDir); + + const secondCallArgs = mockPrompt.mock.calls[1][0]; + const claudeChoice = secondCallArgs.choices.find( + (choice: any) => choice.value === 'claude' + ); + + expect(claudeChoice.configured).toBe(true); + }); + + it('should NOT show already configured for Codex in fresh init even with global prompts', async () => { + // Create global Codex prompts (simulating previous installation) + const codexPromptsDir = path.join(testDir, '.codex/prompts'); + await fs.mkdir(codexPromptsDir, { recursive: true }); + await fs.writeFile( + path.join(codexPromptsDir, 'openspec-proposal.md'), + '# Existing prompt\n' + ); + + queueSelections('claude', DONE); + + await initCommand.execute(testDir); + + const firstCallArgs = mockPrompt.mock.calls[0][0]; + const codexChoice = firstCallArgs.choices.find( + (choice: any) => choice.value === 'codex' + ); + + // In fresh init, even global tools should not show as configured + expect(codexChoice.configured).toBe(false); + }); + }); + describe('error handling', () => { it('should provide helpful error for insufficient permissions', async () => { // This is tricky to test cross-platform, but we can test the error message From 8382ee8660a777b1ad87abe821c3e264431d313f Mon Sep 17 00:00:00 2001 From: Tabish Bidiwale Date: Sat, 25 Oct 2025 20:27:28 +1100 Subject: [PATCH 2/2] refactor: optimize tool state detection and improve code clarity Address code review feedback: 1. **Parallelize tool state checks**: Changed from sequential `for` loop to `Promise.all()` for checking multiple tools simultaneously. This reduces I/O latency during extend mode initialization. 2. **Extract marker validation helper**: Created `fileHasMarkers()` helper function to eliminate code duplication between config file and slash command checks. Makes the logic clearer and more maintainable. 3. **Clarify slash command policy**: Added explicit comment that "at least one file with markers is sufficient" (not all required) for slash commands. This is correct because OpenSpec creates all files together - if any exists with markers, the tool was configured by OpenSpec. 4. **Simplify fresh init path**: Use `Object.fromEntries()` for cleaner initialization of all-false states. Performance improvement: Extend mode now checks tools in parallel instead of serially, reducing init time especially for projects with many tools. --- src/core/init.ts | 70 +++++++++++++++++++----------------------------- 1 file changed, 27 insertions(+), 43 deletions(-) diff --git a/src/core/init.ts b/src/core/init.ts index e702a89f..ab3dc55b 100644 --- a/src/core/init.ts +++ b/src/core/init.ts @@ -631,32 +631,35 @@ export class InitCommand { projectPath: string, extendMode: boolean ): Promise> { - const states: Record = {}; - - // Only check tool configuration if OpenSpec is already initialized (extend mode) - // This prevents false "already configured" messages when users have existing - // tool config files (like CLAUDE.md) that weren't created by OpenSpec - if (extendMode) { - for (const tool of AI_TOOLS) { - states[tool.value] = await this.isToolConfigured(projectPath, tool.value); - } - } else { - // Fresh initialization - no tools configured yet - for (const tool of AI_TOOLS) { - states[tool.value] = false; - } + // Fresh initialization - no tools configured yet + if (!extendMode) { + return Object.fromEntries(AI_TOOLS.map(t => [t.value, false])); } - return states; + // Extend mode - check all tools in parallel for better performance + const entries = await Promise.all( + AI_TOOLS.map(async (t) => [t.value, await this.isToolConfigured(projectPath, t.value)] as const) + ); + return Object.fromEntries(entries); } private async isToolConfigured( projectPath: string, toolId: string ): Promise { - // A tool is only considered "configured by OpenSpec" if BOTH conditions are met: - // 1. Config file (e.g., CLAUDE.md) exists with OpenSpec markers - // 2. At least one slash command file exists with OpenSpec markers + // A tool is only considered "configured by OpenSpec" if its files contain OpenSpec markers. + // For tools with both config files and slash commands, BOTH must have markers. + // For slash commands, at least one file with markers is sufficient (not all required). + + // Helper to check if a file exists and contains OpenSpec markers + const fileHasMarkers = async (absolutePath: string): Promise => { + try { + const content = await FileSystemUtils.readFile(absolutePath); + return content.includes(OPENSPEC_MARKERS.start) && content.includes(OPENSPEC_MARKERS.end); + } catch { + return false; + } + }; let hasConfigFile = false; let hasSlashCommands = false; @@ -665,36 +668,17 @@ export class InitCommand { const configFile = ToolRegistry.get(toolId)?.configFileName; if (configFile) { const configPath = path.join(projectPath, configFile); - if (await FileSystemUtils.fileExists(configPath)) { - try { - const content = await FileSystemUtils.readFile(configPath); - hasConfigFile = content.includes(OPENSPEC_MARKERS.start) && content.includes(OPENSPEC_MARKERS.end); - } catch { - // If we can't read the file, treat it as not configured - hasConfigFile = false; - } - } + hasConfigFile = (await FileSystemUtils.fileExists(configPath)) && (await fileHasMarkers(configPath)); } - // Check if slash command files exist with OpenSpec markers + // Check if any slash command file exists with OpenSpec markers const slashConfigurator = SlashCommandRegistry.get(toolId); if (slashConfigurator) { for (const target of slashConfigurator.getTargets()) { - const absolute = slashConfigurator.resolveAbsolutePath( - projectPath, - target.id - ); - if (await FileSystemUtils.fileExists(absolute)) { - try { - const content = await FileSystemUtils.readFile(absolute); - if (content.includes(OPENSPEC_MARKERS.start) && content.includes(OPENSPEC_MARKERS.end)) { - hasSlashCommands = true; - break; // Found at least one valid slash command - } - } catch { - // If we can't read the file, continue checking others - continue; - } + const absolute = slashConfigurator.resolveAbsolutePath(projectPath, target.id); + if ((await FileSystemUtils.fileExists(absolute)) && (await fileHasMarkers(absolute))) { + hasSlashCommands = true; + break; // At least one file with markers is sufficient } } }