refactor: split skill templates into workflow modules#698
refactor: split skill templates into workflow modules#698
Conversation
|
Task completed. PR Review: refactor: split skill templates into workflow modules (#698)Overall AssessmentThis is a clean decomposition of a ~3,500 line monolithic file into 11 focused workflow modules. The approach is sound — keeping Issue: Missing exports in
|
| Category | Finding |
|---|---|
| Bug | index.ts missing 3 exports + type re-exports vs the facade |
| Suggestion | Replace generic boilerplate comments with file-specific ones |
| Suggestion | Consider workflows/index.ts barrel file |
| Suggestion | Add hash regeneration instructions to parity test |
| Good | onboard.ts shared helper avoids duplication |
| Good | Parity test with stable hashing is solid |
| Good | Facade pattern preserves backward compatibility |
Powered by 1Code
📝 WalkthroughWalkthroughThis PR adds core template types and splits a monolithic templates file into multiple workflow-specific modules, providing exported factory functions for SkillTemplate and CommandTemplate objects for twelve workflows, updates the templates re-export surface, and adds parity tests to validate template payloads and generated content. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
No actionable comments were generated in the recent review. 🎉 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 OverviewGreptile SummaryThis PR splits the monolithic Key changes
Issues found
Benefits
Confidence Score: 4/5
Important Files Changed
Sequence DiagramsequenceDiagram
participant Consumer
participant index.ts
participant skill-templates.ts
participant workflows/
Note over Consumer,workflows/: Before: Monolithic File
Consumer->>skill-templates.ts: import { getExploreSkillTemplate }
skill-templates.ts-->>Consumer: return template (3500+ lines)
Note over Consumer,workflows/: After: Modular Architecture
Consumer->>skill-templates.ts: import { getExploreSkillTemplate }
skill-templates.ts->>workflows/: re-export from workflows/explore.js
workflows/-->>skill-templates.ts: template function
skill-templates.ts-->>Consumer: return template
Note over Consumer,index.ts: Alternative Import Path
Consumer->>index.ts: import { getExploreSkillTemplate }
index.ts->>skill-templates.ts: re-export
skill-templates.ts->>workflows/: re-export
workflows/-->>Consumer: template function
|
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Additional Comments (1)
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In `@src/core/templates/workflows/feedback.ts`:
- Around line 9-111: The getFeedbackSkillTemplate SkillTemplate factory is
missing the standard fields present in other templates (license, compatibility,
metadata); update the object returned by getFeedbackSkillTemplate to include
license: 'MIT', compatibility: 'Requires openspec CLI.', and metadata: { author:
'openspec', version: '1.0' } so it matches
getOnboardSkillTemplate/getArchiveChangeSkillTemplate/getFfChangeSkillTemplate
conventions and remember this will change the
EXPECTED_FUNCTION_HASHES.getFeedbackSkillTemplate parity test.
In `@src/core/templates/workflows/ff-change.ts`:
- Around line 189-201: The command template's "Artifact Creation Guidelines" in
ff-change.ts is missing the guardrail that `context` and `rules` are constraints
and must not be copied into artifact files; add the same sentence used in the
skill template ("`context` and `rules` are constraints for YOU, not content for
the file — Do NOT copy `<context>`, `<rules>`, `<project_context>` blocks into
the artifact.") into the Artifact Creation Guidelines section of the command
template so it matches the guardrail present in continue-change.ts and the skill
template.
In `@test/core/templates/skill-templates-parity.test.ts`:
- Around line 120-142: Add a brief inline comment above the skillFactories array
explaining that getFeedbackSkillTemplate is intentionally excluded from this
generated-content parity test because feedback is not deployed as a generated
skill file (it is covered in the function-payload parity test); mention the
symbols involved (getFeedbackSkillTemplate, skillFactories,
generateSkillContent) so future readers understand the deliberate omission.
🧹 Nitpick comments (3)
src/core/templates/workflows/explore.ts (1)
298-472: Substantial content duplication betweengetExploreSkillTemplateandgetOpsxExploreCommandTemplate.The
contentfield in the command template is ~90% identical to theinstructionsfield in the skill template. Theonboard.tsmodule demonstrates a cleaner pattern using a sharedgetOnboardInstructions()helper to avoid this. Consider extracting shared prose into a helper and interpolating the small differences (e.g., the additional "Input" section in the command template).This isn't blocking since the parity test locks the output, but it increases maintenance burden for future content changes across all workflow modules that follow this pattern.
src/core/templates/workflows/continue-change.ts (2)
1-6: Nit: Header comment is misleading for ongoing maintenance.The comment says "This file is generated by splitting the legacy monolithic templates file" — this reads as if it's auto-generated. If these modules will be hand-maintained going forward, consider rewording to avoid confusion (e.g., "Extracted from the legacy monolithic templates file"). This same comment appears in all workflow files.
9-239: Significant content duplication betweenSkillTemplate.instructionsandCommandTemplate.content.The
instructionsfield (Lines 13–119) and thecontentfield (Lines 132–238) are ~95% identical — roughly 100+ lines of duplicated markdown with only minor differences (input format mentioning/opsx:continue, output prompt wording, and the "all complete" suggestion). This pattern repeats across every workflow file in this PR.Consider extracting the shared instruction body into a helper and composing the small per-variant deltas. For example:
♻️ Sketch of a shared-content approach
// In a shared helper, e.g., src/core/templates/workflows/_continue-change-content.ts function baseContinueInstructions(opts: { inputLine: string; allCompleteMsg: string; outputPrompt: string; }): string { return `Continue working on a change by creating the next artifact. ${opts.inputLine} **Steps** ...shared steps... **If all artifacts are complete (\`isComplete: true\`)**: ... - Suggest: "${opts.allCompleteMsg}" ... **Output** ... - Prompt: "${opts.outputPrompt}" ...`; }This would make future instruction edits less error-prone and reduce the risk of the two variants drifting apart. The same pattern applies to every other workflow module in this PR.
| export function getFeedbackSkillTemplate(): SkillTemplate { | ||
| return { | ||
| name: 'feedback', | ||
| description: 'Collect and submit user feedback about OpenSpec with context enrichment and anonymization.', | ||
| instructions: `Help the user submit feedback about OpenSpec. | ||
|
|
||
| **Goal**: Guide the user through collecting, enriching, and submitting feedback while ensuring privacy through anonymization. | ||
|
|
||
| **Process** | ||
|
|
||
| 1. **Gather context from the conversation** | ||
| - Review recent conversation history for context | ||
| - Identify what task was being performed | ||
| - Note what worked well or poorly | ||
| - Capture specific friction points or praise | ||
|
|
||
| 2. **Draft enriched feedback** | ||
| - Create a clear, descriptive title (single sentence, no "Feedback:" prefix needed) | ||
| - Write a body that includes: | ||
| - What the user was trying to do | ||
| - What happened (good or bad) | ||
| - Relevant context from the conversation | ||
| - Any specific suggestions or requests | ||
|
|
||
| 3. **Anonymize sensitive information** | ||
| - Replace file paths with \`<path>\` or generic descriptions | ||
| - Replace API keys, tokens, secrets with \`<redacted>\` | ||
| - Replace company/organization names with \`<company>\` | ||
| - Replace personal names with \`<user>\` | ||
| - Replace specific URLs with \`<url>\` unless public/relevant | ||
| - Keep technical details that help understand the issue | ||
|
|
||
| 4. **Present draft for approval** | ||
| - Show the complete draft to the user | ||
| - Display both title and body clearly | ||
| - Ask for explicit approval before submitting | ||
| - Allow the user to request modifications | ||
|
|
||
| 5. **Submit on confirmation** | ||
| - Use the \`openspec feedback\` command to submit | ||
| - Format: \`openspec feedback "title" --body "body content"\` | ||
| - The command will automatically add metadata (version, platform, timestamp) | ||
|
|
||
| **Example Draft** | ||
|
|
||
| \`\`\` | ||
| Title: Error handling in artifact workflow needs improvement | ||
|
|
||
| Body: | ||
| I was working on creating a new change and encountered an issue with | ||
| the artifact workflow. When I tried to continue after creating the | ||
| proposal, the system didn't clearly indicate that I needed to complete | ||
| the specs first. | ||
|
|
||
| Suggestion: Add clearer error messages that explain dependency chains | ||
| in the artifact workflow. Something like "Cannot create design.md | ||
| because specs are not complete (0/2 done)." | ||
|
|
||
| Context: Using the spec-driven schema with <path>/my-project | ||
| \`\`\` | ||
|
|
||
| **Anonymization Examples** | ||
|
|
||
| Before: | ||
| \`\`\` | ||
| Working on /Users/john/mycompany/auth-service/src/oauth.ts | ||
| Failed with API key: sk_live_abc123xyz | ||
| Working at Acme Corp | ||
| \`\`\` | ||
|
|
||
| After: | ||
| \`\`\` | ||
| Working on <path>/oauth.ts | ||
| Failed with API key: <redacted> | ||
| Working at <company> | ||
| \`\`\` | ||
|
|
||
| **Guardrails** | ||
|
|
||
| - MUST show complete draft before submitting | ||
| - MUST ask for explicit approval | ||
| - MUST anonymize sensitive information | ||
| - ALLOW user to modify draft before submitting | ||
| - DO NOT submit without user confirmation | ||
| - DO include relevant technical context | ||
| - DO keep conversation-specific insights | ||
|
|
||
| **User Confirmation Required** | ||
|
|
||
| Always ask: | ||
| \`\`\` | ||
| Here's the feedback I've drafted: | ||
|
|
||
| Title: [title] | ||
|
|
||
| Body: | ||
| [body] | ||
|
|
||
| Does this look good? I can modify it if you'd like, or submit it as-is. | ||
| \`\`\` | ||
|
|
||
| Only proceed with submission after user confirms.` | ||
| }; |
There was a problem hiding this comment.
Inconsistent with other templates: missing license, compatibility, and metadata fields.
Every other SkillTemplate factory (e.g., getOnboardSkillTemplate, getArchiveChangeSkillTemplate, getFfChangeSkillTemplate) includes license: 'MIT', compatibility: 'Requires openspec CLI.', and metadata: { author: 'openspec', version: '1.0' }. This template omits all three. While the fields are optional, the inconsistency looks unintentional.
Proposed fix
Only proceed with submission after user confirms.`
+ license: 'MIT',
+ compatibility: 'Requires openspec CLI.',
+ metadata: { author: 'openspec', version: '1.0' },
};
}Note: This will change the hash in the parity test (EXPECTED_FUNCTION_HASHES.getFeedbackSkillTemplate), which will need updating.
📝 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.
| export function getFeedbackSkillTemplate(): SkillTemplate { | |
| return { | |
| name: 'feedback', | |
| description: 'Collect and submit user feedback about OpenSpec with context enrichment and anonymization.', | |
| instructions: `Help the user submit feedback about OpenSpec. | |
| **Goal**: Guide the user through collecting, enriching, and submitting feedback while ensuring privacy through anonymization. | |
| **Process** | |
| 1. **Gather context from the conversation** | |
| - Review recent conversation history for context | |
| - Identify what task was being performed | |
| - Note what worked well or poorly | |
| - Capture specific friction points or praise | |
| 2. **Draft enriched feedback** | |
| - Create a clear, descriptive title (single sentence, no "Feedback:" prefix needed) | |
| - Write a body that includes: | |
| - What the user was trying to do | |
| - What happened (good or bad) | |
| - Relevant context from the conversation | |
| - Any specific suggestions or requests | |
| 3. **Anonymize sensitive information** | |
| - Replace file paths with \`<path>\` or generic descriptions | |
| - Replace API keys, tokens, secrets with \`<redacted>\` | |
| - Replace company/organization names with \`<company>\` | |
| - Replace personal names with \`<user>\` | |
| - Replace specific URLs with \`<url>\` unless public/relevant | |
| - Keep technical details that help understand the issue | |
| 4. **Present draft for approval** | |
| - Show the complete draft to the user | |
| - Display both title and body clearly | |
| - Ask for explicit approval before submitting | |
| - Allow the user to request modifications | |
| 5. **Submit on confirmation** | |
| - Use the \`openspec feedback\` command to submit | |
| - Format: \`openspec feedback "title" --body "body content"\` | |
| - The command will automatically add metadata (version, platform, timestamp) | |
| **Example Draft** | |
| \`\`\` | |
| Title: Error handling in artifact workflow needs improvement | |
| Body: | |
| I was working on creating a new change and encountered an issue with | |
| the artifact workflow. When I tried to continue after creating the | |
| proposal, the system didn't clearly indicate that I needed to complete | |
| the specs first. | |
| Suggestion: Add clearer error messages that explain dependency chains | |
| in the artifact workflow. Something like "Cannot create design.md | |
| because specs are not complete (0/2 done)." | |
| Context: Using the spec-driven schema with <path>/my-project | |
| \`\`\` | |
| **Anonymization Examples** | |
| Before: | |
| \`\`\` | |
| Working on /Users/john/mycompany/auth-service/src/oauth.ts | |
| Failed with API key: sk_live_abc123xyz | |
| Working at Acme Corp | |
| \`\`\` | |
| After: | |
| \`\`\` | |
| Working on <path>/oauth.ts | |
| Failed with API key: <redacted> | |
| Working at <company> | |
| \`\`\` | |
| **Guardrails** | |
| - MUST show complete draft before submitting | |
| - MUST ask for explicit approval | |
| - MUST anonymize sensitive information | |
| - ALLOW user to modify draft before submitting | |
| - DO NOT submit without user confirmation | |
| - DO include relevant technical context | |
| - DO keep conversation-specific insights | |
| **User Confirmation Required** | |
| Always ask: | |
| \`\`\` | |
| Here's the feedback I've drafted: | |
| Title: [title] | |
| Body: | |
| [body] | |
| Does this look good? I can modify it if you'd like, or submit it as-is. | |
| \`\`\` | |
| Only proceed with submission after user confirms.` | |
| }; | |
| export function getFeedbackSkillTemplate(): SkillTemplate { | |
| return { | |
| name: 'feedback', | |
| description: 'Collect and submit user feedback about OpenSpec with context enrichment and anonymization.', | |
| instructions: `Help the user submit feedback about OpenSpec. | |
| **Goal**: Guide the user through collecting, enriching, and submitting feedback while ensuring privacy through anonymization. | |
| **Process** | |
| 1. **Gather context from the conversation** | |
| - Review recent conversation history for context | |
| - Identify what task was being performed | |
| - Note what worked well or poorly | |
| - Capture specific friction points or praise | |
| 2. **Draft enriched feedback** | |
| - Create a clear, descriptive title (single sentence, no "Feedback:" prefix needed) | |
| - Write a body that includes: | |
| - What the user was trying to do | |
| - What happened (good or bad) | |
| - Relevant context from the conversation | |
| - Any specific suggestions or requests | |
| 3. **Anonymize sensitive information** | |
| - Replace file paths with \`<path>\` or generic descriptions | |
| - Replace API keys, tokens, secrets with \`<redacted>\` | |
| - Replace company/organization names with \`<company>\` | |
| - Replace personal names with \`<user>\` | |
| - Replace specific URLs with \`<url>\` unless public/relevant | |
| - Keep technical details that help understand the issue | |
| 4. **Present draft for approval** | |
| - Show the complete draft to the user | |
| - Display both title and body clearly | |
| - Ask for explicit approval before submitting | |
| - Allow the user to request modifications | |
| 5. **Submit on confirmation** | |
| - Use the \`openspec feedback\` command to submit | |
| - Format: \`openspec feedback "title" --body "body content"\` | |
| - The command will automatically add metadata (version, platform, timestamp) | |
| **Example Draft** | |
| \`\`\` | |
| Title: Error handling in artifact workflow needs improvement | |
| Body: | |
| I was working on creating a new change and encountered an issue with | |
| the artifact workflow. When I tried to continue after creating the | |
| proposal, the system didn't clearly indicate that I needed to complete | |
| the specs first. | |
| Suggestion: Add clearer error messages that explain dependency chains | |
| in the artifact workflow. Something like "Cannot create design.md | |
| because specs are not complete (0/2 done)." | |
| Context: Using the spec-driven schema with <path>/my-project | |
| \`\`\` | |
| **Anonymization Examples** | |
| Before: | |
| \`\`\` | |
| Working on /Users/john/mycompany/auth-service/src/oauth.ts | |
| Failed with API key: sk_live_abc123xyz | |
| Working at Acme Corp | |
| \`\`\` | |
| After: | |
| \`\`\` | |
| Working on <path>/oauth.ts | |
| Failed with API key: <redacted> | |
| Working at <company> | |
| \`\`\` | |
| **Guardrails** | |
| - MUST show complete draft before submitting | |
| - MUST ask for explicit approval | |
| - MUST anonymize sensitive information | |
| - ALLOW user to modify draft before submitting | |
| - DO NOT submit without user confirmation | |
| - DO include relevant technical context | |
| - DO keep conversation-specific insights | |
| **User Confirmation Required** | |
| Always ask: | |
| \`\`\` | |
| Here's the feedback I've drafted: | |
| Title: [title] | |
| Body: | |
| [body] | |
| Does this look good? I can modify it if you'd like, or submit it as-is. | |
| \`\`\` | |
| Only proceed with submission after user confirms.`, | |
| license: 'MIT', | |
| compatibility: 'Requires openspec CLI.', | |
| metadata: { author: 'openspec', version: '1.0' }, | |
| }; | |
| } |
🤖 Prompt for AI Agents
In `@src/core/templates/workflows/feedback.ts` around lines 9 - 111, The
getFeedbackSkillTemplate SkillTemplate factory is missing the standard fields
present in other templates (license, compatibility, metadata); update the object
returned by getFeedbackSkillTemplate to include license: 'MIT', compatibility:
'Requires openspec CLI.', and metadata: { author: 'openspec', version: '1.0' }
so it matches
getOnboardSkillTemplate/getArchiveChangeSkillTemplate/getFfChangeSkillTemplate
conventions and remember this will change the
EXPECTED_FUNCTION_HASHES.getFeedbackSkillTemplate parity test.
| **Artifact Creation Guidelines** | ||
|
|
||
| - Follow the \`instruction\` field from \`openspec instructions\` for each artifact type | ||
| - The schema defines what each artifact should contain - follow it | ||
| - Read dependency artifacts for context before creating new ones | ||
| - Use the \`template\` as a starting point, filling in based on context | ||
|
|
||
| **Guardrails** | ||
| - Create ALL artifacts needed for implementation (as defined by schema's \`apply.requires\`) | ||
| - Always read dependency artifacts before creating a new one | ||
| - If context is critically unclear, ask the user - but prefer making reasonable decisions to keep momentum | ||
| - If a change with that name already exists, ask if user wants to continue it or create a new one | ||
| - Verify each artifact file exists after writing before proceeding to next` |
There was a problem hiding this comment.
Missing guardrail about context/rules in the command template's Artifact Creation Guidelines.
The skill template (Lines 93–95) includes a critical guardrail:
contextandrulesare constraints for YOU, not content for the file — Do NOT copy<context>,<rules>,<project_context>blocks into the artifact.
The command template omits this entirely. Other workflow command templates (e.g., continue-change.ts Lines 236–238) consistently include this guardrail. This looks like an accidental content drift — exactly the risk that arises from maintaining near-duplicate text blocks.
Proposed fix
**Artifact Creation Guidelines**
- Follow the `instruction` field from `openspec instructions` for each artifact type
- The schema defines what each artifact should contain - follow it
- Read dependency artifacts for context before creating new ones
-- Use the \`template\` as a starting point, filling in based on context
+- Use \`template\` as the structure for your output file - fill in its sections
+- **IMPORTANT**: \`context\` and \`rules\` are constraints for YOU, not content for the file
+ - Do NOT copy \`<context>\`, \`<rules>\`, \`<project_context>\` blocks into the artifact
+ - These guide what you write, but should never appear in the output
**Guardrails**📝 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.
| **Artifact Creation Guidelines** | |
| - Follow the \`instruction\` field from \`openspec instructions\` for each artifact type | |
| - The schema defines what each artifact should contain - follow it | |
| - Read dependency artifacts for context before creating new ones | |
| - Use the \`template\` as a starting point, filling in based on context | |
| **Guardrails** | |
| - Create ALL artifacts needed for implementation (as defined by schema's \`apply.requires\`) | |
| - Always read dependency artifacts before creating a new one | |
| - If context is critically unclear, ask the user - but prefer making reasonable decisions to keep momentum | |
| - If a change with that name already exists, ask if user wants to continue it or create a new one | |
| - Verify each artifact file exists after writing before proceeding to next` | |
| **Artifact Creation Guidelines** | |
| - Follow the \`instruction\` field from \`openspec instructions\` for each artifact type | |
| - The schema defines what each artifact should contain - follow it | |
| - Read dependency artifacts for context before creating new ones | |
| - Use \`template\` as the structure for your output file - fill in its sections | |
| - **IMPORTANT**: \`context\` and \`rules\` are constraints for YOU, not content for the file | |
| - Do NOT copy \`<context>\`, \`<rules>\`, \`<project_context>\` blocks into the artifact | |
| - These guide what you write, but should never appear in the output | |
| **Guardrails** | |
| - Create ALL artifacts needed for implementation (as defined by schema's \`apply.requires\`) | |
| - Always read dependency artifacts before creating a new one | |
| - If context is critically unclear, ask the user - but prefer making reasonable decisions to keep momentum | |
| - If a change with that name already exists, ask if user wants to continue it or create a new one | |
| - Verify each artifact file exists after writing before proceeding to next |
🤖 Prompt for AI Agents
In `@src/core/templates/workflows/ff-change.ts` around lines 189 - 201, The
command template's "Artifact Creation Guidelines" in ff-change.ts is missing the
guardrail that `context` and `rules` are constraints and must not be copied into
artifact files; add the same sentence used in the skill template ("`context` and
`rules` are constraints for YOU, not content for the file — Do NOT copy
`<context>`, `<rules>`, `<project_context>` blocks into the artifact.") into the
Artifact Creation Guidelines section of the command template so it matches the
guardrail present in continue-change.ts and the skill template.
|
Hey @TabishB, great refactoring work here. I wanted to raise something that connects this PR with the discussion happening on #701 (lifecycle hooks). Right now, all 10 skill templates generate identical instructions regardless of the target agent. This works today, but it breaks down when you consider that each agent defines its own way of handling hooks, tools, and workflows. Claude Code has PreToolUse/PostToolUse/Stop, Cursor has its own system, Windsurf another, and many agents have no hook system at all. There's no shared capability model — each agent is its own world. Proposal: Agent Skill Profiles Would you consider designing these workflow modules with an agent-based profile system in mind? The idea: The current set of workflow modules becomes the default profile — the universal, agent-agnostic baseline that works for any agent. Specific agents can define their own agent profile that overrides or extends individual workflow modules to leverage agent-specific features (native hooks, custom tool integrations, etc.). Resolution is simple: agent-specific module if it exists, otherwise fall back to default Concretely: Each agent 's config would map to a profile ID (or 'default'), and the skill generation in update.ts would resolve templates through the profile chain before generating content. This doesn't need to be implemented now — I'm just suggesting that the module boundaries you're creating here are the perfect foundation for this pattern. The factory functions per workflow module are exactly the right seam. If designed with this extensibility in mind (e.g., factory functions that accept an optional agent context), it would make the evolution much smoother. What do you think? |
2 participants
Summary
Validation
Summary by CodeRabbit
New Features
Tests