docs: refine Copilot generalization design#3047
Conversation
There was a problem hiding this comment.
Code Review
This pull request outlines the technical design for generalizing XBuilder's Copilot, moving from a project-specific implementation to a modular, skill-based architecture. Key changes include decoupling platform-level logic from project-specific context, introducing an in-memory skill registry, and implementing a progressive disclosure model via a new load_skill tool. Review feedback identifies inconsistencies in the proposed TypeScript interfaces and contradictions between the tool implementation snippets and the specified output format.
| name: string | ||
| path: string | ||
| content: string | ||
| resourcePaths: string[] |
There was a problem hiding this comment.
The LoadedSkillDocument type includes resourcePaths: string[]. However, the examples in Section 6 show that only the main skill document lists resources, while individual resource documents do not. To align with this, consider making resourcePaths optional so it can be omitted for resource documents.
| .describe('Optional relative path of a markdown resource within the skill bundle. Omit to load SKILL.md') | ||
| }), | ||
| async implementation({ skillId, resourcePath }) { | ||
| return skillRegistry.load(skillId, resourcePath ?? null) |
There was a problem hiding this comment.
This code snippet shows the tool returning a LoadedSkillDocument object directly. This contradicts Section 6, which specifies that the tool should return a structured XML-like string (e.g., wrapped in <skill_content>). The implementation should be updated to include the formatting step to ensure consistency within the design.
Refs #2808
Summary
docs/develop/generalization/copilot.mdCopilotRoot, the generic Copilot core, project-specific integrations, and the separate skill-support moduleNotes