refactor: introduce @perstack/skill-manager package#539
Merged
Conversation
48c1c2c to
dcc3d84
Compare
- Create packages/skill-manager with 3-layer architecture: - SkillAdapter (abstract): unified interface for MCP/delegate/interactive skills - SkillManager: 1-Expert-1-SkillManager lifecycle manager with dynamic add/remove - Utilities: command-args, ip-validator, mcp-converters, base-skill-helpers - Migrate runtime state machine from Record<string, BaseSkillManager> to SkillManager - Update CoordinatorExecutor to use SkillManager.fromExpert/fromLockfile - Refactor tool-execution layer (executor-factory, mcp-executor, tool-classifier) - Add comprehensive tests for all adapters and SkillManager Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
The old packages/runtime/src/skill-manager/ is fully replaced by @perstack/skill-manager. Remove @modelcontextprotocol/sdk and @perstack/base from runtime deps, and @perstack/runtime from installer deps, since they are no longer needed. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
The old skill-manager code emitted skillConnected events from each manager's init method. Add spawnDurationMs/connectDurationMs tracking to SkillAdapter and emit skillConnected events from coordinator-executor after SkillManager creation. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Skills whose required environment variables are not available are now filtered out during SkillManager initialization. This replaces the old lazyInit behavior with a cleaner approach: availability is determined by env vars, not a config flag. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
d54f96c to
a89b2ea
Compare
Bridge SkillManager's addSkill/removeSkill/addDelegate/removeDelegate methods to the LLM via MCP tools. Uses a deferred binding pattern where InMemoryBaseSkillAdapter registers tools with placeholder callbacks, then SkillManager.fromExpert() binds real implementations after construction. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
a89b2ea to
ff591a7
Compare
Merged
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
packages/skill-manageras a standalone package with 3-layer architecture:fromExpert()/fromLockfile()static factories and dynamicaddSkill()/removeSkill()/addDelegate()/removeDelegate()methodsRecord<string, BaseSkillManager>toSkillManageracross:skillManagers→skillManager)CoordinatorExecutorusingSkillManager.fromExpert/fromLockfileMotivation
Decouples skill lifecycle management from the runtime to enable:
lazyInitremoval in favor of lockfile-based initialization strategy (future)Test plan
pnpm typecheck— 23/23 packages passpnpm test— 102 files, 1196 tests passpnpm build— 23/23 packages buildpnpm format-and-lint— clean (1 pre-existing issue in filesystem/event.ts)🤖 Generated with Claude Code