-
Notifications
You must be signed in to change notification settings - Fork 956
feat: add CoStrict AI assistant support #240
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Add support for Costrict AI tool with slash commands and configuration template. Includes configurator, registry entries, templates, and comprehensive test coverage.
WalkthroughAdds first-class support for the CoStrict AI tool: README entry, AI_TOOLS config entry, CoStrictConfigurator, CoStrict slash-command configurator, template export and TemplateManager accessor, registry registrations, and comprehensive init/update tests exercising marker-bounded file updates and error cases. Changes
Sequence Diagram(s)sequenceDiagram
participant CLI as CLI (init/update)
participant Registry as Configurators Registry
participant Template as TemplateManager
participant FS as FileSystemUtils
Note right of CLI: Init/Update invoked with CoStrict selected
CLI->>Registry: lookup configurator 'costrict'
Registry->>CLI: return CoStrictConfigurator / CostrictSlashConfigurator
CLI->>Template: getCostrictTemplate()
Template-->>CLI: costrictTemplate
CLI->>FS: updateFileWithMarkers(path: COSTRICT.md, markers, template)
alt file writable
FS-->>CLI: write success
CLI->>CLI: mark tool configured
else write/read error
FS-->>CLI: error logged (no throw)
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~30 minutes
Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (4)
src/core/configurators/slash/costrict.ts (2)
4-8: Lock in key coverage with satisfies/as constUse TS
satisfies+as constto guarantee compile-time exhaustiveness and avoid accidental mutation.Apply:
-const FILE_PATHS: Record<SlashCommandId, string> = { - proposal: '.cospec/openspec/commands/openspec-proposal.md', - apply: '.cospec/openspec/commands/openspec-apply.md', - archive: '.cospec/openspec/commands/openspec-archive.md' -}; +const FILE_PATHS = { + proposal: '.cospec/openspec/commands/openspec-proposal.md', + apply: '.cospec/openspec/commands/openspec-apply.md', + archive: '.cospec/openspec/commands/openspec-archive.md', +} as const satisfies Record<SlashCommandId, string>;If your TS version < 4.9, drop
satisfiesand keepas const, or keep the original typing.
10-23: Same for FRONTMATTER mapHarden typing/immutability for the frontmatter map.
-const FRONTMATTER: Record<SlashCommandId, string> = { +const FRONTMATTER = { proposal: `--- description: "Scaffold a new OpenSpec change and validate strictly." argument-hint: feature description or request ---`, apply: `--- description: "Implement an approved OpenSpec change and keep tasks in sync." argument-hint: change-id ---`, archive: `--- description: "Archive a deployed OpenSpec change and update specs." argument-hint: change-id ---` -}; +} as const satisfies Record<SlashCommandId, string>;test/core/update.test.ts (2)
822-861: Good parity test for Costrict slash refreshCovers marker-only updates and expected log lines. Matches path and guardrail content.
Consider extracting a shared helper (e.g., assertSlashUpdateBetweenMarkers(tool, file)) to reduce duplication across tools.
966-1005: Make error-path test OS-agnosticchmod(0o444) can be flaky on Windows. Rely on the writeFile mock alone to simulate EACCES and keep the test portable.
- await fs.chmod(costrictPath, 0o444); // Read-only @@ - // Restore permissions for cleanup - await fs.chmod(costrictPath, 0o644);The mock already throws for COSTRICT.md, so behavior is preserved without filesystem permission fiddling.
If you want to keep chmod, guard it:
if (os.platform() !== 'win32') { await fs.chmod(costrictPath, 0o444); }
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
README.md(1 hunks)src/core/config.ts(1 hunks)src/core/configurators/costrict.ts(1 hunks)src/core/configurators/registry.ts(2 hunks)src/core/configurators/slash/costrict.ts(1 hunks)src/core/configurators/slash/registry.ts(3 hunks)src/core/templates/costrict-template.ts(1 hunks)src/core/templates/index.ts(2 hunks)test/core/init.test.ts(1 hunks)test/core/update.test.ts(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (8)
src/core/configurators/slash/registry.ts (1)
src/core/configurators/slash/costrict.ts (1)
CostrictSlashCommandConfigurator(25-36)
src/core/templates/costrict-template.ts (2)
src/core/init.ts (1)
configureRootAgentsStub(788-803)src/core/configurators/agents.ts (1)
AgentsStandardConfigurator(7-23)
src/core/config.ts (1)
src/core/init.ts (3)
formatToolNames(919-934)promptForAITools(559-630)displaySuccessMessage(805-917)
src/core/configurators/costrict.ts (4)
src/core/configurators/base.ts (1)
ToolConfigurator(1-6)src/core/templates/index.ts (1)
TemplateManager(14-47)src/utils/file-system.ts (1)
FileSystemUtils(44-187)src/core/config.ts (1)
OPENSPEC_MARKERS(3-6)
test/core/init.test.ts (1)
src/utils/file-system.ts (1)
fileExists(76-86)
src/core/configurators/registry.ts (2)
src/core/configurators/costrict.ts (1)
CostrictConfigurator(7-23)src/core/configurators/agents.ts (1)
AgentsStandardConfigurator(7-23)
test/core/update.test.ts (1)
src/utils/file-system.ts (2)
FileSystemUtils(44-187)fileExists(76-86)
src/core/configurators/slash/costrict.ts (1)
src/core/templates/index.ts (1)
SlashCommandId(50-50)
🔇 Additional comments (15)
src/core/configurators/slash/registry.ts (1)
15-15: LGTM! Clean integration following established patterns.The Costrict slash command configurator is properly imported, instantiated, and registered following the same pattern as existing tools.
Also applies to: 34-34, 49-49
src/core/config.ts (1)
24-24: LGTM! Proper tool configuration.The Costrict entry is correctly positioned alphabetically and includes all required fields with consistent naming conventions.
README.md (1)
95-95: LGTM! Clear and consistent documentation.The Costrict entry follows the established documentation pattern with correct command names, directory path, and reference link.
src/core/configurators/registry.ts (1)
5-5: LGTM! Proper registry integration.The CostrictConfigurator is correctly imported, instantiated, and registered with the appropriate key that matches the tool identifier in the configuration.
Also applies to: 15-15, 21-21
src/core/templates/index.ts (1)
5-5: LGTM! Consistent template management.The Costrict template is properly imported and exposed through a static method following the established pattern for other tool templates.
Also applies to: 36-38
src/core/templates/costrict-template.ts (1)
1-1: LGTM! Appropriate template reuse.The re-export creates a named alias for the agents root stub template, which is an efficient approach when tools share the same template content.
test/core/init.test.ts (3)
999-1039: LGTM! Comprehensive slash command file tests.The test thoroughly validates the creation of Costrict slash command files with proper paths (
.cospec/openspec/commands/), frontmatter, markers, and content. Follows established testing patterns for other tools.
1041-1051: LGTM! Proper extend mode testing.The test correctly verifies that Costrict is marked as already configured during extend mode, following the same pattern as other tools.
1053-1084: LGTM! Complete COSTRICT.md file handling tests.Both tests properly verify COSTRICT.md creation with markers and updating existing files while preserving custom content. Follows established patterns for tool-specific configuration files.
src/core/configurators/costrict.ts (1)
1-23: LGTM! Well-structured configurator implementation.The CostrictConfigurator properly implements the ToolConfigurator interface and follows the exact pattern established by other tool configurators. The implementation correctly:
- Uses marker-based file updates to preserve custom content
- Places COSTRICT.md in the project root
- Retrieves the appropriate template via TemplateManager
test/core/update.test.ts (4)
863-896: Skip-create behavior validatedEnsures proposal/archive aren’t created when only apply exists. Solid.
898-934: COSTRICT.md update behavior validatedAsserts managed block update and logging. Looks correct.
936-946: No creation on absenceConfirms COSTRICT.md isn’t created on update. Good negative coverage.
948-964: Preserves content outside markersVerifies only managed region changes; mirrors Windsurf test. Good.
src/core/configurators/slash/costrict.ts (1)
25-36: Registry wiring verified and correct
CostrictSlashCommandConfiguratoris properly imported, instantiated, and registered insrc/core/configurators/slash/registry.ts(lines 15, 34, 49). Top-level registry also correctly wiresCostrictConfigurator. No issues found.
feat(config): update branding from 'Costrict' to 'CoStrict'
TabishB
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for this @mini2s Good to merge 🚀
Looking forward to the new version of Openspec adding CoStrict support 🤝 |
Add support for Costrict AI tool with slash commands and configuration template. Includes configurator, registry entries, templates, and comprehensive test coverage.
Openspec and CoStrict work together like magic. 👍👍👍 @TabishB
Summary by CodeRabbit