refactor: merge slashcommand behavior into skill tool to reduce prompt size#1698
Conversation
There was a problem hiding this comment.
No issues found across 7 files
Confidence score: 5/5
- Automated review surfaced no issues in the provided summaries.
- No files require special attention.
Requires human review: Large refactor merging skill tool into slashcommand tool (118 lines added). Changes MCP integration, tool routing, and core agent prompts. Cannot be 100% sure of no regressions without testing skill加载
a59e18e to
41b454c
Compare
|
@code-yeongyu Hi! should this be merged? I think it’s a valid PR to trim token usage. |
|
I think merging slashcommand tool to command tool will be valid |
|
And along with some prompt changes will be required |
code-yeongyu
left a comment
There was a problem hiding this comment.
Can you merge slashcommand tool instead?
|
@code-yeongyu Correction to my previous note (that note was outdated). This PR now follows your requested direction: merge Current shape in this branch:
Could you please re-review with this direction in mind? |
Per reviewer feedback (code-yeongyu), keep the 'skill' tool as the main tool and merge slashcommand functionality INTO it, rather than the reverse. Changes: - skill/tools.ts: Add command discovery (discoverCommandsSync) support; handle both SKILL.md skills and .omo/commands/ slash commands in a single tool; show combined listing in tool description - skill/types.ts: Add 'commands' option to SkillLoadOptions - skill/constants.ts: Update description to mention both skills and commands - plugin/tool-registry.ts: Replace createSlashcommandTool with createSkillTool; register tool as 'skill' instead of 'slashcommand' - tools/index.ts: Export createSkillTool instead of createSlashcommandTool - plugin/tool-execute-before.ts: Update tool name checks from 'slashcommand' to 'skill'; update arg name from 'command' to 'name' - agents/dynamic-agent-prompt-builder.ts: Categorize 'skill' tool as 'command' - tools/skill-mcp/tools.ts: Update hint message to reference 'skill' tool - hooks/auto-slash-command/executor.ts: Update error message The slashcommand/ module files are kept (they provide shared utilities used by the skill tool), but the slashcommand tool itself is no longer registered.
41b454c to
462bf7b
Compare
|
I noticed the current head branch name ( The code itself is already minimal and valid, so if you’re okay with the branch name as-is, I’m happy to proceed with merging this PR directly. If you’d prefer clearer naming, I can open a new PR from |
|
Token impact measurement for this PR is complete. This is based on real prompt generation with my current setup.
Measured savings
Total across these 3 prompts: 4,898 tokens saved. Why Atlas has more redundancy (detailed root cause)Atlas had structural double-rendering of skill descriptions in a single prompt build.
So Atlas effectively had this duplication:
What this PR changes:
This is why Atlas saves much more than Sisyphus/Hephaestus: Atlas removed one whole duplicated description block, while the others mostly benefit from the long-name-echo reduction. Scale note:
|
|
[sisyphus-bot] Thank you for this refactoring PR. The token savings analysis (4,898 tokens, 35% reduction for Atlas) is excellent work. The direction to consolidate slashcommand into the skill tool aligns with our architecture goals. Awaiting maintainer review. |
Summary
This PR merges
slashcommandbehavior into theskilltool so we keep a single command-style entry point while reducing duplicated prompt/description surface.Fixes #1666
Why
skillandslashcommandhad overlapping responsibilities for command/skill discovery and execution.What Changed
src/plugin/tool-registry.tsskillas the primary tool entry.slashcommandtool registration.createSkillTool(...).src/tools/skill/tools.tsskilltool to handle both skill loading and slash-command execution.src/tools/skill/types.ts,src/tools/skill/constants.tssrc/tools/index.tscreateSkillToolfor primary routing.src/plugin/tool-execute-before.tssrc/hooks/auto-slash-command/executor.tssrc/agents/dynamic-agent-prompt-builder.tsskillentry point.src/tools/slashcommand/slashcommand-tool.tssrc/tools/slashcommand/types.tssrc/tools/slashcommand/skill-formatter.tsResult
skill) for both skills and slash-style commands.