fix: recreate missing openspec template files in extend mode #238
Conversation
The init command now checks for and recreates missing template files (like openspec/AGENTS.md and openspec/project.md) when running in extend mode, instead of skipping file generation entirely. Previously, if a user deleted openspec/AGENTS.md and ran init again, the file would not be recreated because the openspec/ directory existed, triggering extend mode which skipped all file generation. Changes: - Added ensureTemplateFiles() method to check and recreate missing files - Modified extend mode to call ensureTemplateFiles() instead of skipping - Updated message to "Checking for missing files..." for clarity - Added tests for recreating deleted openspec/AGENTS.md and project.md
WalkthroughIn extend mode, InitCommand now ensures the OpenSpec directory structure exists and recreates any missing template files instead of skipping scaffolding. New private methods generateFiles, ensureTemplateFiles, and writeTemplateFiles were added; writeTemplateFiles accepts a skipExisting flag to preserve existing files. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant InitCommand
participant TemplateLoader
participant FileSystem
User->>InitCommand: execute(extendMode=true)
InitCommand->>FileSystem: ensureDirectoryStructure(openspec/)
InitCommand->>InitCommand: log("Checking for missing files...")
InitCommand->>TemplateLoader: listTemplates(config)
loop for each template
InitCommand->>FileSystem: exists?(targetPath)
alt not exists
InitCommand->>TemplateLoader: loadTemplate(templateId)
InitCommand->>FileSystem: writeFile(targetPath, content)
Note right of FileSystem: Missing template created
else exists
Note right of FileSystem: Existing file preserved
end
end
InitCommand-->>User: Init complete
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/core/init.ts (1)
698-721: Consider consolidating duplicate logic withgenerateFiles.The new
ensureTemplateFilesmethod correctly implements conditional file creation, but it duplicates most of the logic fromgenerateFiles(lines 677-696). Both methods create the same context, retrieve templates, handle function/string content, and write files—the only difference is the existence check.Consider extracting shared logic to reduce duplication:
+ private async writeTemplateFiles( + openspecPath: string, + config: OpenSpecConfig, + onlyIfMissing: boolean = false + ): Promise<void> { + const context: ProjectContext = {}; + const templates = TemplateManager.getTemplates(context); + + for (const template of templates) { + const filePath = path.join(openspecPath, template.path); + + if (onlyIfMissing && (await FileSystemUtils.fileExists(filePath))) { + continue; + } + + const content = + typeof template.content === 'function' + ? template.content(context) + : template.content; + + await FileSystemUtils.writeFile(filePath, content); + } + } private async generateFiles( openspecPath: string, config: OpenSpecConfig ): Promise<void> { - const context: ProjectContext = { - // Could be enhanced with prompts for project details - }; - - const templates = TemplateManager.getTemplates(context); - - for (const template of templates) { - const filePath = path.join(openspecPath, template.path); - const content = - typeof template.content === 'function' - ? template.content(context) - : template.content; - - await FileSystemUtils.writeFile(filePath, content); - } + await this.writeTemplateFiles(openspecPath, config, false); } private async ensureTemplateFiles( openspecPath: string, config: OpenSpecConfig ): Promise<void> { - const context: ProjectContext = { - // Could be enhanced with prompts for project details - }; - - const templates = TemplateManager.getTemplates(context); - - for (const template of templates) { - const filePath = path.join(openspecPath, template.path); - - // Only write if file doesn't exist - if (!(await FileSystemUtils.fileExists(filePath))) { - const content = - typeof template.content === 'function' - ? template.content(context) - : template.content; - - await FileSystemUtils.writeFile(filePath, content); - } - } + await this.writeTemplateFiles(openspecPath, config, true); }
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/core/init.ts(2 hunks)test/core/init.test.ts(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
src/core/init.ts (4)
src/core/config.ts (1)
OpenSpecConfig(8-10)src/core/templates/index.ts (2)
ProjectContext(44-44)TemplateManager(13-42)src/core/templates/project-template.ts (1)
ProjectContext(1-6)src/utils/file-system.ts (1)
FileSystemUtils(44-187)
test/core/init.test.ts (2)
src/utils/file-system.ts (1)
fileExists(76-86)test/core/update.test.ts (2)
updateCommand(971-983)rootAgentsPath(985-1008)
🔇 Additional comments (2)
test/core/init.test.ts (1)
582-622: LGTM! Well-structured tests for extend mode file recovery.The two new test cases properly validate that deleted template files are recreated when
openspec initruns in extend mode. The tests follow the existing patterns, use appropriate assertions, and verify both file existence and content correctness.src/core/init.ts (1)
430-434: LGTM! Extend mode now properly ensures file structure.The updated logic correctly addresses the issue by calling
createDirectoryStructureandensureTemplateFilesin extend mode instead of skipping scaffolding entirely. The log message change accurately reflects the new behavior.
Extracted shared logic from generateFiles() and ensureTemplateFiles() into a new writeTemplateFiles() method with a skipExisting parameter. This eliminates code duplication while maintaining the same behavior.
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/core/init.ts (1)
691-717: LGTM! Solid implementation of conditional template writing.The
writeTemplateFilesmethod correctly implements the core logic for template file generation with proper async handling and clear conditional logic for skipping existing files.Optional refinement: Consider replacing the
skipExistingboolean parameter with an enum for better readability at call sites:enum WriteMode { Overwrite, SkipExisting } private async writeTemplateFiles( openspecPath: string, config: OpenSpecConfig, mode: WriteMode ): Promise<void> { // ... if (mode === WriteMode.SkipExisting && (await FileSystemUtils.fileExists(filePath))) { continue; } // ... }However, the current boolean implementation is perfectly acceptable and clear.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/core/init.ts(3 hunks)
🔇 Additional comments (3)
src/core/init.ts (3)
428-435: LGTM! Extend mode now correctly regenerates missing files.The updated logic properly addresses the issue where extend mode was skipping file generation entirely. By calling
createDirectoryStructureandensureTemplateFiles, deleted template files will now be recreated while preserving existing ones.
677-682: LGTM! Clean delegation for fresh initialization.The
generateFilesmethod correctly delegates towriteTemplateFileswithskipExisting: false, which is appropriate for non-extend mode where all template files should be written.
684-689: LGTM! Proper delegation for extend mode.The
ensureTemplateFilesmethod correctly delegates towriteTemplateFileswithskipExisting: true, ensuring that existing files are preserved while missing ones are recreated.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/core/init.ts (1)
631-641: Good refactor: parallel execution improves performance.Using
Promise.allwithObject.fromEntriesis more idiomatic and runs all tool state checks concurrently rather than sequentially. Theas constassertion ensures proper tuple typing.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/core/init.ts(4 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
src/core/init.ts (2)
src/core/config.ts (2)
AI_TOOLS(19-34)OpenSpecConfig(8-10)src/utils/file-system.ts (1)
FileSystemUtils(44-187)
🔇 Additional comments (2)
src/core/init.ts (2)
428-435: LGTM! Core fix correctly addresses the issue.The extend mode now properly recreates missing template files instead of skipping scaffolding entirely. The updated message accurately reflects the new behavior, and calling
createDirectoryStructureensures consistency even if the directory structure is incomplete.
679-719: Well-structured solution with correct skipExisting logic.The three-method design provides clear separation:
generateFiles: Initial mode creates all filesensureTemplateFiles: Extend mode recreates only missing fileswriteTemplateFiles: Shared implementation with flexible behaviorThe conditional logic at lines 708-710 correctly preserves existing files in extend mode while recreating deleted ones.
TabishB
force-pushed
the
TabishB/fix-init-recreate-agents
branch
from
d454ccf to
499241b
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
test/core/init.test.ts (2)
582-622: Consider extracting a helper function to reduce code duplication.The two tests share nearly identical structure (approximately 90% code overlap). Consider extracting a reusable helper to improve maintainability:
async function testFileRecreationInExtendMode( testDir: string, initCommand: InitCommand, relativePath: string, expectedContent: string ) { queueSelections('claude', DONE, DONE); // First init await initCommand.execute(testDir); const filePath = path.join(testDir, relativePath); expect(await fileExists(filePath)).toBe(true); // Delete the file await fs.unlink(filePath); expect(await fileExists(filePath)).toBe(false); // Run init again - should recreate the file await initCommand.execute(testDir); expect(await fileExists(filePath)).toBe(true); const content = await fs.readFile(filePath, 'utf-8'); expect(content).toContain(expectedContent); }Then simplify the tests:
it('should recreate deleted openspec/AGENTS.md in extend mode', async () => { await testFileRecreationInExtendMode( testDir, initCommand, 'openspec/AGENTS.md', 'OpenSpec Instructions' ); }); it('should recreate deleted openspec/project.md in extend mode', async () => { await testFileRecreationInExtendMode( testDir, initCommand, 'openspec/project.md', 'Project Context' ); });
582-622: Consider adding a test to verify existing files are preserved in extend mode.The current tests validate that missing files are recreated, but there's no test confirming that existing files are NOT overwritten when
skipExistingis true. This would provide stronger coverage of the extend mode behavior.it('should preserve existing template files in extend mode', async () => { queueSelections('claude', DONE, DONE); // First init await initCommand.execute(testDir); const agentsPath = path.join(testDir, 'openspec', 'AGENTS.md'); const customContent = '# My Custom AGENTS Content\nDo not overwrite this!'; // Modify the file with custom content await fs.writeFile(agentsPath, customContent); // Run init again - should NOT overwrite await initCommand.execute(testDir); const content = await fs.readFile(agentsPath, 'utf-8'); expect(content).toBe(customContent); expect(content).not.toContain('OpenSpec Instructions'); });
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/core/init.ts(3 hunks)test/core/init.test.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/core/init.ts
🧰 Additional context used
🧬 Code graph analysis (1)
test/core/init.test.ts (1)
src/utils/file-system.ts (1)
fileExists(76-86)
🔇 Additional comments (1)
test/core/init.test.ts (1)
582-622: Well-structured tests that correctly validate the extend mode file recreation feature.The tests properly verify that deleted template files are recreated during extend mode, following the workflow: initial init → file deletion → re-init → file recreation with correct content.
- Extracted testFileRecreationInExtendMode helper to reduce code duplication - Added test to verify existing files are preserved in extend mode - Ensures skipExisting behavior is properly tested
TabishB
force-pushed
the
TabishB/fix-init-recreate-agents
branch
from
e886801 to
f036fbe
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
test/core/init.test.ts (1)
1201-1225: LGTM! Well-structured helper function.The helper properly tests the file recreation workflow with clear step-by-step assertions. The logic correctly simulates extend mode by queueing an empty selection (
DONEonly) for the second init run.Minor suggestion: If additional tests need similar file recreation logic, consider extracting this to a shared test utilities module. For now, the current placement is fine.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/core/init.ts(3 hunks)test/core/init.test.ts(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/core/init.ts
🧰 Additional context used
🧬 Code graph analysis (1)
test/core/init.test.ts (1)
src/utils/file-system.ts (1)
fileExists(76-86)
🔇 Additional comments (2)
test/core/init.test.ts (2)
582-598: LGTM! Tests properly verify template file recreation in extend mode.Both tests correctly validate that deleted template files (
openspec/AGENTS.mdandopenspec/project.md) are recreated when runningopenspec initagain in extend mode, with appropriate content assertions.
600-618: LGTM! Excellent coverage of the preservation behavior.The test properly verifies that user-modified template files are not overwritten during subsequent init runs in extend mode. The dual assertion (line 616: exact match, line 617: no template content) provides strong validation.
Summary
Fixes an issue where
openspec initwould not recreate missing template files (likeopenspec/AGENTS.md) when running in extend mode.Problem
Previously, if a user:
openspec init(createsopenspec/directory and all files)openspec/AGENTS.mdopenspec initagainThe command would detect the existing
openspec/directory, enter extend mode, and skip all file generation - thus not recreating the deleted file.Solution
ensureTemplateFiles()method that checks if each template file exists before writingensureTemplateFiles()instead of completely skipping file generationChanges
src/core/init.ts: AddedensureTemplateFiles()method and updated extend mode logictest/core/init.test.ts: Added tests for recreating deletedopenspec/AGENTS.mdandopenspec/project.mdTesting
🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Tests