refactor(setup): simplify config creation and fix test hanging #537
+96
−275
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
- Replace interactive config prompts with automatic config creation using default schema. The generated config includes helpful comments explaining context and rules options. - Remove unused promptForConfig, promptForArtifactRules, and isExitPromptError functions from config-prompts.ts - Add forceExit: true to vitest config to prevent worker processes from hanging after tests complete
Contributor
📝 WalkthroughWalkthroughRemoves interactive config creation prompts from the artifact workflow and replaces them with automatic default configuration generation. Simplifies serializeConfig to build YAML strings directly rather than serializing objects. Introduces schema aliasing support documentation and adds process cleanup to test teardown. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Greptile SummarySimplified config creation by removing interactive prompts and automatically generating config files with helpful comments. Also fixed test hanging issue by adding Key Changes:
Benefits:
Confidence Score: 5/5
Important Files Changed
Sequence DiagramsequenceDiagram
participant User
participant CLI as openspec setup
participant FileSystem
participant ConfigPrompts as config-prompts.ts
User->>CLI: Run `openspec setup`
CLI->>CLI: Check if config exists
alt Config already exists
CLI->>User: Skip creation (already exists)
else Non-interactive mode
CLI->>User: Skip creation (non-interactive)
else Interactive mode (new behavior)
CLI->>ConfigPrompts: serializeConfig({schema: DEFAULT_SCHEMA})
ConfigPrompts->>ConfigPrompts: Build YAML with comments
ConfigPrompts-->>CLI: Return YAML string
CLI->>FileSystem: Write config.yaml
FileSystem-->>CLI: Success/Error
CLI->>User: Display success message
end
|
Review CompleteYour review story is ready! Comment !reviewfast on this PR to re-generate the story. |
Proposal to add schema alias support so `openspec-default` and `spec-driven` can be used interchangeably, enabling a rename without breaking existing configs.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@src/core/config-prompts.ts`:
- Around line 9-38: serializeConfig currently prints only comments and blindly
emits "schema: undefined" when config.schema is missing and drops
config.context/config.rules; update serializeConfig to validate that
config.schema is present (throw or return an error) and when config.context or
config.rules exist, render them into the output instead of discarding them:
render context as a YAML block scalar (e.g., "context: |" followed by each line
of config.context indented) and render rules as a proper YAML mapping/sequence
(preserve nested artifact keys and arrays) while keeping the comment sections
when those fields are absent; ensure the final output still ends with a newline.
In `@vitest.config.ts`:
- Around line 45-47: Remove the invalid Vitest config property forceExit from
the vitest.config.ts export (it’s a Jest CLI flag, not a Vitest option) and keep
teardownTimeout as-is; to handle hanging workers instead, add or wire up a
Vitest global teardown/globalSetup hook (e.g., implement a globalTeardown
function referenced by the config) or call project.exit(force) programmatically
in your teardown code so worker processes are explicitly terminated and
resources are cleaned up.
Comment on lines
9
to
+38
| export function serializeConfig(config: Partial<ProjectConfig>): string { | ||
| // Build clean config object (only include defined fields) | ||
| const cleanConfig: Record<string, unknown> = { | ||
| schema: config.schema, | ||
| }; | ||
|
|
||
| if (config.context) { | ||
| cleanConfig.context = config.context; | ||
| } | ||
|
|
||
| if (config.rules && Object.keys(config.rules).length > 0) { | ||
| cleanConfig.rules = config.rules; | ||
| } | ||
|
|
||
| // Serialize to YAML with proper formatting | ||
| return stringifyYaml(cleanConfig, { | ||
| indent: 2, | ||
| lineWidth: 0, // Don't wrap long lines | ||
| defaultStringType: 'PLAIN', | ||
| defaultKeyType: 'PLAIN', | ||
| }); | ||
| const lines: string[] = []; | ||
|
|
||
| // Schema (required) | ||
| lines.push(`schema: ${config.schema}`); | ||
| lines.push(''); | ||
|
|
||
| // Context section with comments | ||
| lines.push('# Project context (optional)'); | ||
| lines.push('# This is shown to AI when creating artifacts.'); | ||
| lines.push('# Add your tech stack, conventions, style guides, domain knowledge, etc.'); | ||
| lines.push('# Example:'); | ||
| lines.push('# context: |'); | ||
| lines.push('# Tech stack: TypeScript, React, Node.js'); | ||
| lines.push('# We use conventional commits'); | ||
| lines.push('# Domain: e-commerce platform'); | ||
| lines.push(''); | ||
|
|
||
| // Rules section with comments | ||
| lines.push('# Per-artifact rules (optional)'); | ||
| lines.push('# Add custom rules for specific artifacts.'); | ||
| lines.push('# Example:'); | ||
| lines.push('# rules:'); | ||
| lines.push('# proposal:'); | ||
| lines.push('# - Keep proposals under 500 words'); | ||
| lines.push('# - Always include a "Non-goals" section'); | ||
| lines.push('# tasks:'); | ||
| lines.push('# - Break tasks into chunks of max 2 hours'); | ||
|
|
||
| return lines.join('\n') + '\n'; |
Contributor
There was a problem hiding this comment.
serializeConfig drops provided context/rules and can emit invalid schema.
This function accepts Partial<ProjectConfig> but only prints comments plus schema. If schema is missing, it writes schema: undefined, and any supplied context/rules are silently discarded. Either render actual values when present or narrow the API to a schema-only template and update the docs.
🛠️ One possible fix (validate schema, render context/rules when provided)
export function serializeConfig(config: Partial<ProjectConfig>): string {
+ if (!config.schema) {
+ throw new Error('serializeConfig requires config.schema');
+ }
const lines: string[] = [];
// Schema (required)
lines.push(`schema: ${config.schema}`);
lines.push('');
- // Context section with comments
- lines.push('# Project context (optional)');
- lines.push('# This is shown to AI when creating artifacts.');
- lines.push('# Add your tech stack, conventions, style guides, domain knowledge, etc.');
- lines.push('# Example:');
- lines.push('# context: |');
- lines.push('# Tech stack: TypeScript, React, Node.js');
- lines.push('# We use conventional commits');
- lines.push('# Domain: e-commerce platform');
- lines.push('');
+ if (config.context) {
+ lines.push('context: |');
+ for (const line of config.context.split('\n')) {
+ lines.push(` ${line}`);
+ }
+ lines.push('');
+ } else {
+ // Context section with comments
+ lines.push('# Project context (optional)');
+ lines.push('# This is shown to AI when creating artifacts.');
+ lines.push('# Add your tech stack, conventions, style guides, domain knowledge, etc.');
+ lines.push('# Example:');
+ lines.push('# context: |');
+ lines.push('# Tech stack: TypeScript, React, Node.js');
+ lines.push('# We use conventional commits');
+ lines.push('# Domain: e-commerce platform');
+ lines.push('');
+ }
- // Rules section with comments
- lines.push('# Per-artifact rules (optional)');
- lines.push('# Add custom rules for specific artifacts.');
- lines.push('# Example:');
- lines.push('# rules:');
- lines.push('# proposal:');
- lines.push('# - Keep proposals under 500 words');
- lines.push('# - Always include a "Non-goals" section');
- lines.push('# tasks:');
- lines.push('# - Break tasks into chunks of max 2 hours');
+ if (config.rules && Object.keys(config.rules).length > 0) {
+ lines.push('rules:');
+ for (const [artifactId, rules] of Object.entries(config.rules)) {
+ lines.push(` ${artifactId}:`);
+ for (const rule of rules) {
+ lines.push(` - ${rule}`);
+ }
+ }
+ } else {
+ // Rules section with comments
+ lines.push('# Per-artifact rules (optional)');
+ lines.push('# Add custom rules for specific artifacts.');
+ lines.push('# Example:');
+ lines.push('# rules:');
+ lines.push('# proposal:');
+ lines.push('# - Keep proposals under 500 words');
+ lines.push('# - Always include a "Non-goals" section');
+ lines.push('# tasks:');
+ lines.push('# - Break tasks into chunks of max 2 hours');
+ }
return lines.join('\n') + '\n';
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| export function serializeConfig(config: Partial<ProjectConfig>): string { | |
| // Build clean config object (only include defined fields) | |
| const cleanConfig: Record<string, unknown> = { | |
| schema: config.schema, | |
| }; | |
| if (config.context) { | |
| cleanConfig.context = config.context; | |
| } | |
| if (config.rules && Object.keys(config.rules).length > 0) { | |
| cleanConfig.rules = config.rules; | |
| } | |
| // Serialize to YAML with proper formatting | |
| return stringifyYaml(cleanConfig, { | |
| indent: 2, | |
| lineWidth: 0, // Don't wrap long lines | |
| defaultStringType: 'PLAIN', | |
| defaultKeyType: 'PLAIN', | |
| }); | |
| const lines: string[] = []; | |
| // Schema (required) | |
| lines.push(`schema: ${config.schema}`); | |
| lines.push(''); | |
| // Context section with comments | |
| lines.push('# Project context (optional)'); | |
| lines.push('# This is shown to AI when creating artifacts.'); | |
| lines.push('# Add your tech stack, conventions, style guides, domain knowledge, etc.'); | |
| lines.push('# Example:'); | |
| lines.push('# context: |'); | |
| lines.push('# Tech stack: TypeScript, React, Node.js'); | |
| lines.push('# We use conventional commits'); | |
| lines.push('# Domain: e-commerce platform'); | |
| lines.push(''); | |
| // Rules section with comments | |
| lines.push('# Per-artifact rules (optional)'); | |
| lines.push('# Add custom rules for specific artifacts.'); | |
| lines.push('# Example:'); | |
| lines.push('# rules:'); | |
| lines.push('# proposal:'); | |
| lines.push('# - Keep proposals under 500 words'); | |
| lines.push('# - Always include a "Non-goals" section'); | |
| lines.push('# tasks:'); | |
| lines.push('# - Break tasks into chunks of max 2 hours'); | |
| return lines.join('\n') + '\n'; | |
| export function serializeConfig(config: Partial<ProjectConfig>): string { | |
| if (!config.schema) { | |
| throw new Error('serializeConfig requires config.schema'); | |
| } | |
| const lines: string[] = []; | |
| // Schema (required) | |
| lines.push(`schema: ${config.schema}`); | |
| lines.push(''); | |
| if (config.context) { | |
| lines.push('context: |'); | |
| for (const line of config.context.split('\n')) { | |
| lines.push(` ${line}`); | |
| } | |
| lines.push(''); | |
| } else { | |
| // Context section with comments | |
| lines.push('# Project context (optional)'); | |
| lines.push('# This is shown to AI when creating artifacts.'); | |
| lines.push('# Add your tech stack, conventions, style guides, domain knowledge, etc.'); | |
| lines.push('# Example:'); | |
| lines.push('# context: |'); | |
| lines.push('# Tech stack: TypeScript, React, Node.js'); | |
| lines.push('# We use conventional commits'); | |
| lines.push('# Domain: e-commerce platform'); | |
| lines.push(''); | |
| } | |
| if (config.rules && Object.keys(config.rules).length > 0) { | |
| lines.push('rules:'); | |
| for (const [artifactId, rules] of Object.entries(config.rules)) { | |
| lines.push(` ${artifactId}:`); | |
| for (const rule of rules) { | |
| lines.push(` - ${rule}`); | |
| } | |
| } | |
| } else { | |
| // Rules section with comments | |
| lines.push('# Per-artifact rules (optional)'); | |
| lines.push('# Add custom rules for specific artifacts.'); | |
| lines.push('# Example:'); | |
| lines.push('# rules:'); | |
| lines.push('# proposal:'); | |
| lines.push('# - Keep proposals under 500 words'); | |
| lines.push('# - Always include a "Non-goals" section'); | |
| lines.push('# tasks:'); | |
| lines.push('# - Break tasks into chunks of max 2 hours'); | |
| } | |
| return lines.join('\n') + '\n'; | |
| } |
🤖 Prompt for AI Agents
In `@src/core/config-prompts.ts` around lines 9 - 38, serializeConfig currently
prints only comments and blindly emits "schema: undefined" when config.schema is
missing and drops config.context/config.rules; update serializeConfig to
validate that config.schema is present (throw or return an error) and when
config.context or config.rules exist, render them into the output instead of
discarding them: render context as a YAML block scalar (e.g., "context: |"
followed by each line of config.context indented) and render rules as a proper
YAML mapping/sequence (preserve nested artifact keys and arrays) while keeping
the comment sections when those fields are absent; ensure the final output still
ends with a newline.
- Remove `forceExit: true` from vitest.config.ts (Jest option, not Vitest) - Add actual teardown logic in vitest.setup.ts that forces exit after 1s grace period if processes are still hanging
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
forceExit: trueto vitest config to prevent worker processes from hangingTest plan
openspec setupand verify config is created with default schema and commentspnpm testand verify tests don't hang after completion🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Refactor
Bug Fixes
✏️ Tip: You can customize this high-level summary in your review settings.