-
Notifications
You must be signed in to change notification settings - Fork 1.5k
fix: use Skill tool for sync invocation in archive templates #632
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
fix: use Skill tool for sync invocation in archive templates #632
Conversation
Update archive skill templates to properly instruct the AI to use the Skill tool to invoke sync commands instead of executing command logic directly. Co-Authored-By: Claude Opus 4.5 <[email protected]>
📝 WalkthroughWalkthroughReplaced two direct Changes
Sequence Diagram(s)sequenceDiagram
participant User as User
participant Agent as Agent (archival skill)
participant TaskTool as Task Tool (subagent)
participant Skill as Skill Tool (openspec-sync-specs)
participant Archive as Archive Process
User->>Agent: Request archive / choose to sync
Agent->>TaskTool: Launch Task (subagent_type: "general-purpose") with prompt to invoke Skill and analyze delta specs (include delta summary)
TaskTool->>Skill: Invoke openspec-sync-specs (via Skill tool)
Skill-->>TaskTool: Delta-spec analysis & sync actions result
TaskTool-->>Agent: Return analysis summary / sync decision outcome
Agent->>User: Show delta-spec analysis summary and ask final confirmation
Agent->>Archive: Proceed with archival (regardless of sync choice)
Archive-->>User: Archive result
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
🧪 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 |
Review CompleteYour review story is ready! Comment !reviewfast on this PR to re-generate the story. |
Greptile OverviewGreptile SummaryThis PR updates archive templates to use the Skill tool for programmatic sync invocation instead of referencing slash command logic. This prevents agents from improvising incorrect sync behavior that could result in malformed spec files. Key changes:
Issues found:
Confidence Score: 3/5
Important Files Changed
Sequence DiagramsequenceDiagram
participant User
participant Agent
participant SkillTool as Skill Tool
participant Archive as Archive Template
participant Sync as openspec-sync-specs
User->>Agent: /opsx:archive <change-name>
Agent->>Archive: Execute archive logic
Archive->>Archive: Check for delta specs
alt Delta specs exist and changes needed
Archive->>User: Prompt: "Sync now (recommended)" or "Archive without syncing"
User->>Archive: Choose "Sync now"
Archive->>Agent: Invoke sync via Skill tool
Agent->>SkillTool: Skill("openspec-sync-specs", change-name)
SkillTool->>Sync: Execute sync skill
Sync->>Sync: Read delta specs
Sync->>Sync: Apply changes to main specs
Sync-->>Agent: Sync complete
else No changes or user skips
Archive->>Archive: Continue without sync
end
Archive->>Archive: Move change to archive/YYYY-MM-DD-<name>
Archive->>User: Display summary
|
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.
1 file reviewed, 2 comments
| - If already synced: "Archive now", "Sync anyway", "Cancel" | ||
|
|
||
| If user chooses sync, execute \`/opsx:sync\` logic. Proceed to archive regardless of choice. | ||
| If user chooses sync, use the Skill tool to invoke \`opsx:sync\` with the change name. Proceed to archive regardless of choice. |
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.
Inconsistent skill name - should be openspec-sync-specs not opsx:sync
Line 2158 correctly uses openspec-sync-specs, but this line and line 2929 use opsx:sync (the slash command ID). The Skill tool requires the actual skill name openspec-sync-specs as registered in skill-generation.ts:58.
| If user chooses sync, use the Skill tool to invoke \`opsx:sync\` with the change name. Proceed to archive regardless of choice. | |
| If user chooses sync, use the Skill tool to invoke `openspec-sync-specs` with the change name. Proceed to archive regardless of choice. |
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/core/templates/skill-templates.ts
Line: 2835:2835
Comment:
Inconsistent skill name - should be `openspec-sync-specs` not `opsx:sync`
Line 2158 correctly uses `openspec-sync-specs`, but this line and line 2929 use `opsx:sync` (the slash command ID). The Skill tool requires the actual skill name `openspec-sync-specs` as registered in `skill-generation.ts:58`.
```suggestion
If user chooses sync, use the Skill tool to invoke `openspec-sync-specs` with the change name. Proceed to archive regardless of choice.
```
How can I resolve this? If you propose a fix, please make it concise.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.
This is also right ^
| - Preserve .openspec.yaml when moving to archive (it moves with the directory) | ||
| - Show clear summary of what happened | ||
| - If sync is requested, use /opsx:sync approach (agent-driven) | ||
| - If sync is requested, use the Skill tool to invoke \`opsx:sync\` (agent-driven) |
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.
Inconsistent skill name - should be openspec-sync-specs not opsx:sync
Same issue as line 2835. The skill name is openspec-sync-specs, not the command ID opsx:sync.
| - If sync is requested, use the Skill tool to invoke \`opsx:sync\` (agent-driven) | |
| - If sync is requested, use the Skill tool to invoke `openspec-sync-specs` (agent-driven) |
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/core/templates/skill-templates.ts
Line: 2929:2929
Comment:
Inconsistent skill name - should be `openspec-sync-specs` not `opsx:sync`
Same issue as line 2835. The skill name is `openspec-sync-specs`, not the command ID `opsx:sync`.
```suggestion
- If sync is requested, use the Skill tool to invoke `openspec-sync-specs` (agent-driven)
```
How can I resolve this? If you propose a fix, please make it concise.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: 1
🤖 Fix all issues with AI agents
In `@src/core/templates/skill-templates.ts`:
- Line 2835: The Skill tool invocations currently call the slash command string
"opsx:sync" which fails in Skill-only contexts; update all Skill tool
invocations in this template to use the skill identifier "openspec-sync-specs"
instead of "opsx:sync" (search for occurrences of the literal "opsx:sync" in
skill-templates.ts and replace them with "openspec-sync-specs" where the Skill
tool is invoked so calls to the Skill tool use the correct skill ID).
|
I've verified it is working with '/opsx:sync'. Claude code treats slash command same as a skill. |
|
@cheolwanpark Yes i get that claude code treats slash commands the same as skill but I think semantically it needs to match if we mention a skill tool we should mention a skill instead of a slash command. |
Skill tool terminates after completion and doesn't return control to the caller, causing archive to not continue after sync. Changed to spawn a subagent via Task tool which properly returns control after completion. Also updated opsx:sync references to openspec-sync-specs for semantic coherence across templates. Co-Authored-By: Claude Opus 4.5 <[email protected]>
|
Fixed the semantic mismatch. Now skill tool references use openspec-sync-specs instead of opsx:sync. Also fixed another issue. The skill tool terminates after completion and doesn't return control to the caller, causing the archive step to not continue after sync. Changed to spawn a subagent via Task tool which properly returns control, allowing the archive flow to continue. @TabishB would appreciate a review when you get a chance! |
Summary
Fix
/opsx:archivetemplate to properly invoke sync via Skill tool instead of referencing slash command logicProblem
The archive template told the agent to "execute
/opsx:synclogic" - but agents can't directly execute slash commands. They must use the Skill tool to invoke skills programmatically.This caused the agent to improvise sync behavior incorrectly, resulting in malformed spec files without
## Purposesection.openspec list --specswould show 0 requirements even when specs existed.Fix
Replace command references with explicit Skill tool invocations:
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.