feat(profile): add 'all' profile preset for installing all workflows#1207
feat(profile): add 'all' profile preset for installing all workflows#1207bgc2020 wants to merge 1 commit into
Conversation
Add a third profile option 'all' alongside 'core' and 'custom': - openspec init --profile all installs all 11 workflows - openspec config profile all preset shortcut - deriveProfileFromWorkflowSelection returns 'all' when all workflows selected - Migration sets 'all' profile when all workflows are already installed - Interactive picker auto-derives 'all' when all 11 workflows are checked Source changes: Profile type, Zod schemas, getProfileWorkflows, resolveProfileOverride, migration logic, config command presets, workspace skill state schemas, CLI descriptions, completions. Test additions: profile resolution, init --profile all, config schema validation, deriveProfileFromWorkflowSelection, config profile preset, migration to 'all', global-config round-trip. Docs updates: cli.md, commands.md, getting-started.md, workflows.md, migration-guide.md, supported-tools.md, CHANGELOG.md, spec.md. 🤖 AI[100%] 👌 AI Adopted[100%] 🧑 Human[0%] Co-authored-by: opencode (glm-5.1) <ai@local>
📝 WalkthroughWalkthroughThis PR extends OpenSpec with two complementary features: an ChangesProfile System Enhancement and Language Support
Sequence Diagram(s)sequenceDiagram
participant User
participant InitCmd as InitCommand
participant ProjConfig as ProjectConfig
participant Instr as InstructionLoader
participant Prompts as Artifact Prompts
User->>InitCmd: openspec init --profile all --language ja
InitCmd->>InitCmd: validate profile='all'
InitCmd->>InitCmd: validate language='ja'
InitCmd->>ProjConfig: serializeConfig({language: 'ja'})
ProjConfig->>Prompts: language: ja
Instr->>ProjConfig: readProjectConfig()
ProjConfig-->>Instr: {language: 'ja'}
Instr->>Instr: prependLanguageInstruction('ja', context)
Instr->>Prompts: Japanese instructions + original context
Prompts->>User: localized artifact prompts
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~30 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
test/core/init.test.ts (1)
539-554: 💤 Low valueTest title suggests more comprehensive verification than implemented.
The test is titled "should use --profile all to install all workflows" but only verifies 2 of the 11 workflows are created (propose and onboard). While this is acceptable as a smoke test verifying both core and expanded workflows are included, the title could be more accurate.
Consider either:
- Verifying all 11 workflows are created for comprehensive coverage
- Renaming to "should use --profile all to install core and expanded workflows"
The test logic itself is correct and follows established patterns.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@test/core/init.test.ts` around lines 539 - 554, The test title is misleading because it asserts only two workflows; update the test to either assert all 11 workflows exist or change the test title to reflect it checks core + expanded workflows. Locate the InitCommand usage in the test (new InitCommand({ tools: 'claude', force: true, profile: 'all' })) and either add expectations for the remaining workflow SKILL.md files (matching the other nine workflow identifiers) or rename the it(...) description from "should use --profile all to install all workflows" to "should use --profile all to install core and expanded workflows" so the name matches the assertions.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@test/core/init.test.ts`:
- Around line 539-554: The test title is misleading because it asserts only two
workflows; update the test to either assert all 11 workflows exist or change the
test title to reflect it checks core + expanded workflows. Locate the
InitCommand usage in the test (new InitCommand({ tools: 'claude', force: true,
profile: 'all' })) and either add expectations for the remaining workflow
SKILL.md files (matching the other nine workflow identifiers) or rename the
it(...) description from "should use --profile all to install all workflows" to
"should use --profile all to install core and expanded workflows" so the name
matches the assertions.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: f5900077-cbec-42b8-a56c-d79dd26fed83
📒 Files selected for processing (25)
AGENTS.mdCHANGELOG.mddocs/cli.mddocs/commands.mddocs/getting-started.mddocs/migration-guide.mddocs/supported-tools.mddocs/workflows.mdopenspec/specs/cli-config/spec.mdsrc/cli/index.tssrc/commands/config.tssrc/core/completions/command-registry.tssrc/core/config-schema.tssrc/core/global-config.tssrc/core/init.tssrc/core/migration.tssrc/core/profiles.tssrc/core/workspace/foundation.tssrc/core/workspace/legacy-state.tstest/commands/config-profile.test.tstest/commands/config.test.tstest/core/global-config.test.tstest/core/init.test.tstest/core/migration.test.tstest/core/profiles.test.ts
alfred-openspec
left a comment
There was a problem hiding this comment.
Reviewed the profile/config/migration surface and this looks good to merge. The all preset is wired through schema/types, init override validation, config profile preset, workflow resolution, workspace state parsing, migration inference, docs, and completions.\n\nVerified locally: focused config/profile/init/migration/profile tests pass, and pnpm exec tsc --noEmit passes. Small note: the branch deletes an empty root AGENTS.md, which is harmless but also unrelated if you want to keep the diff tidier.
There was a problem hiding this comment.
🧹 Nitpick comments (1)
test/core/artifact-graph/instruction-loader.test.ts (1)
469-486: 💤 Low valueConsider moving temp directory cleanup outside the loop (affects multiple tests).
Both sites use a loop-based pattern that creates and cleans up a temp directory per iteration. If an assertion fails mid-loop, subsequent iterations won't run and their temp directories won't be cleaned up.
test/core/artifact-graph/instruction-loader.test.ts#L469-L486: The loop overnonEnLangscreates/cleans temp dirs inside the loop body, making cleanup fragile when assertions fail.test/core/project-config.test.ts#L95-L105: The loop over supported languages creates/cleans temp dirs inside the loop body with the same fragility.Consider refactoring to use a single temp directory for all iterations, or extracting to a parameterized test pattern (e.g.,
it.each([...])) for more robust cleanup handled by the test framework.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@test/core/artifact-graph/instruction-loader.test.ts` around lines 469 - 486, The test at test/core/artifact-graph/instruction-loader.test.ts#L469-L486 uses a loop-based pattern where temp directories are created and cleaned up inside the loop body, making cleanup fragile if assertions fail mid-loop. Refactor this test to use parameterized testing with it.each() passing the nonEnLangs array as test parameters, moving the temp directory creation and cleanup outside the loop body so the test framework manages cleanup properly. Apply the same refactoring pattern to the sibling test at test/core/project-config.test.ts#L95-L105 which has the same loop-based cleanup fragility issue with its supported languages iteration.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@test/core/artifact-graph/instruction-loader.test.ts`:
- Around line 469-486: The test at
test/core/artifact-graph/instruction-loader.test.ts#L469-L486 uses a loop-based
pattern where temp directories are created and cleaned up inside the loop body,
making cleanup fragile if assertions fail mid-loop. Refactor this test to use
parameterized testing with it.each() passing the nonEnLangs array as test
parameters, moving the temp directory creation and cleanup outside the loop body
so the test framework manages cleanup properly. Apply the same refactoring
pattern to the sibling test at test/core/project-config.test.ts#L95-L105 which
has the same loop-based cleanup fragility issue with its supported languages
iteration.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 60ddda00-958c-4fa8-ad1c-15ffe49afc73
📒 Files selected for processing (14)
docs/cli.mddocs/multi-language.mdsrc/cli/index.tssrc/commands/config.tssrc/core/artifact-graph/instruction-loader.tssrc/core/completions/command-registry.tssrc/core/config-prompts.tssrc/core/init.tssrc/core/language.tssrc/core/project-config.tstest/core/artifact-graph/instruction-loader.test.tstest/core/init.test.tstest/core/language.test.tstest/core/project-config.test.ts
✅ Files skipped from review due to trivial changes (1)
- docs/multi-language.md
🚧 Files skipped from review as they are similar to previous changes (2)
- docs/cli.md
- src/core/completions/command-registry.ts
alfred-openspec
left a comment
There was a problem hiding this comment.
Rechecked after the previous approval was dismissed. The profile schema/types, config preset, init override, completions, migration inference, workspace state parsing, docs, and focused tests still look coherent. The deleted root AGENTS.md is an empty placeholder in the diff, and the remaining CodeRabbit point is a non-blocking test-title nit. Still looks good to merge.
Add a third profile option 'all' alongside 'core' and 'custom':
Source changes: Profile type, Zod schemas, getProfileWorkflows, resolveProfileOverride, migration logic, config command presets, workspace skill state schemas, CLI descriptions, completions.
Test additions: profile resolution, init --profile all, config schema validation, deriveProfileFromWorkflowSelection, config profile preset, migration to 'all', global-config round-trip.
Docs updates: cli.md, commands.md, getting-started.md, workflows.md, migration-guide.md, supported-tools.md, CHANGELOG.md, spec.md.
🤖 AI[100%] 👌 AI Adopted[100%] 🧑 Human[0%]
Summary by CodeRabbit
allprofile option foropenspec init --profileandopenspec config profileto install every available workflow.openspec init --language <code>andlanguagesupport inopenspec/config.yamlto drive language-aware prompt context.allwhen all workflows are present.allprofile behavior and the new--languageworkflow.