Skip to content

Commit 18421c0

Browse files
authored
Merge pull request #832 from aRustyDev/pr/3-core-sdk-extraction
refactor: extract @agents/core + @agents/sdk packages
2 parents 0c1fc82 + 7e39184 commit 18421c0

220 files changed

Lines changed: 9271 additions & 1589 deletions

File tree

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

.claude/plans/agents-cli/provider-migration.md

Lines changed: 444 additions & 0 deletions
Large diffs are not rendered by default.
Lines changed: 138 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,138 @@
1+
# CLI-to-SDK Provider Migration — Plan Index
2+
3+
> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan phase-by-phase.
4+
5+
## Overview
6+
7+
**Goal:** Replace CLI's 11 provider files + factory with SDK providers, so all future consumers share one provider implementation.
8+
9+
**Architecture:** CLI's `createComponentManager()` switches from importing local provider classes to calling SDK's `createDefaultProviders()` with dependency injection. CLI provides a `SkillOperations` adapter that wraps existing `skill-*.ts` modules. Provider tests migrate to SDK.
10+
11+
**Tech Stack:** Bun workspaces, TypeScript, `@agents/sdk`
12+
13+
## Current State
14+
15+
```text
16+
CLI (packages/cli/src/lib/component/) SDK (packages/sdk/src/providers/)
17+
+-- factory.ts (40 lines) +-- factory.ts (46 lines)
18+
+-- provider-local.ts (216 lines) +-- local/index.ts (216 lines) <-- DI
19+
+-- provider-agent.ts (243 lines) +-- local/agent.ts (254 lines)
20+
+-- provider-command.ts (173 lines) +-- local/command.ts (177 lines)
21+
+-- provider-output-style.ts (208 lines) +-- local/output-style.ts (212 lines)
22+
+-- provider-plugin.ts (190 lines) +-- local/plugin.ts (208 lines)
23+
+-- provider-rule.ts (168 lines) +-- local/rule.ts (172 lines)
24+
+-- provider-smithery.ts (188 lines) +-- smithery/index.ts (193 lines)
25+
+-- smithery-auth.ts (91 lines) +-- smithery/auth.ts (97 lines)
26+
+-- smithery-publish.ts (359 lines) +-- smithery/publish.ts (376 lines)
27+
+-- index.ts (5 lines) +-- github/index.ts (NEW)
28+
(11 CLI files total)
29+
```
30+
31+
**Key difference:** SDK's `LocalSkillProvider` uses a `SkillOperations` DI interface instead of dynamic imports to `skill-list.ts`, `skill-add.ts`, etc.
32+
33+
## Target State
34+
35+
```text
36+
CLI (packages/cli/src/lib/component/) SDK (unchanged)
37+
+-- skill-ops-impl.ts (NEW)
38+
+-- factory.ts (REWRITTEN)
39+
+-- index.ts (updated barrel)
40+
```
41+
42+
All `provider-*.ts` and `smithery-*.ts` files deleted from CLI. Tests migrated to SDK.
43+
44+
## Phase Summary
45+
46+
| Phase | Title | Depends On | Files Changed | Key Risk |
47+
|-------|-------|-----------|---------------|----------|
48+
| [1](phase/1-skill-ops-adapter.md) | Wire SkillOperations adapter | -- | +2 create | Adapter signature mismatch |
49+
| [2](phase/2-factory-rewrite.md) | Rewrite CLI factory | Phase 1 | ~2 modify | smitheryBaseUrl forwarding |
50+
| [3](phase/3-test-migration.md) | Migrate provider tests to SDK | Phase 2 | ~10 move, ~2 modify | Error code translations |
51+
| [4](phase/4-delete-cli-providers.md) | Delete CLI provider files | Phase 3 | -9 delete, ~1 modify | Stale imports break build |
52+
| [5](phase/5-final-verification.md) | Final verification | Phase 4 | 0 (audit only) | Missed regression |
53+
54+
## Test Baselines
55+
56+
| Package | Pass | Fail |
57+
|---------|------|------|
58+
| CLI | 1929 | 11 |
59+
| SDK | 132 | 0 |
60+
| Core | 10 | 0 |
61+
62+
## Provider Count Change
63+
64+
CLI currently registers **7** providers. SDK factory creates **8** (adds `GitHubProvider`). Tests that assert provider count must update `7 -> 8`.
65+
66+
## Error Code Translation (CLI -> SDK)
67+
68+
Used in Phases 3 and 4 when updating test assertions:
69+
70+
| CLI Code (`CliError`) | SDK Code (`SdkError`) |
71+
|---|---|
72+
| `E_UNSUPPORTED` | `E_PROVIDER_UNAVAILABLE` |
73+
| `E_UNSUPPORTED_OP` | `E_PROVIDER_UNAVAILABLE` |
74+
| `E_UNSUPPORTED_TYPE` | `E_PROVIDER_UNAVAILABLE` |
75+
| `E_UNSUPPORTED_OPERATION` | `E_PROVIDER_UNAVAILABLE` |
76+
| `E_NO_PROVIDER` | `E_PROVIDER_UNAVAILABLE` |
77+
| `E_NOT_FOUND` | `E_COMPONENT_NOT_FOUND` |
78+
| `E_COMMAND_NOT_FOUND` | `E_COMPONENT_NOT_FOUND` |
79+
| `E_SKILL_NOT_FOUND` | `E_COMPONENT_NOT_FOUND` |
80+
| `E_RULE_NOT_FOUND` | `E_COMPONENT_NOT_FOUND` |
81+
| `E_AGENT_NOT_FOUND` | `E_COMPONENT_NOT_FOUND` |
82+
| `E_OUTPUT_STYLE_NOT_FOUND` | `E_COMPONENT_NOT_FOUND` |
83+
| `E_PLUGIN_NOT_FOUND` | `E_COMPONENT_NOT_FOUND` |
84+
| `E_SERVER_NOT_FOUND` | `E_COMPONENT_NOT_FOUND` |
85+
| `E_ADD_FAILED` | `E_PROVIDER_UNAVAILABLE` |
86+
| `E_REMOVE_FAILED` | `E_COMPONENT_NOT_FOUND` |
87+
| `E_AUTH_FAILED` | `E_PROVIDER_UNAVAILABLE` |
88+
| `E_AUTH_REQUIRED` | `E_PROVIDER_UNAVAILABLE` |
89+
| `E_RATE_LIMITED` | `E_PROVIDER_UNAVAILABLE` |
90+
| `E_TIMEOUT` | `E_PROVIDER_TIMEOUT` |
91+
| `E_MISSING_SOURCE` | `E_VALIDATION_FAILED` |
92+
| `E_MISSING_NAME` | `E_VALIDATION_FAILED` |
93+
| `E_MISSING_MANIFEST` | `E_VALIDATION_FAILED` |
94+
| `E_INVALID_NAME` | `E_VALIDATION_FAILED` |
95+
| `E_INVALID_MANIFEST` | `E_VALIDATION_FAILED` |
96+
| `E_POLL_FAILED` | `E_PROVIDER_UNAVAILABLE` |
97+
| `E_DEPLOY_TIMEOUT` | `E_PROVIDER_TIMEOUT` |
98+
| `E_API_ERROR` | `E_PROVIDER_UNAVAILABLE` |
99+
| `E_NETWORK` | `E_PROVIDER_UNAVAILABLE` |
100+
101+
## Global Acceptance Criteria
102+
103+
1. `createComponentManager()` in CLI returns a `ProviderManager` backed by SDK providers
104+
2. All 7 CLI commands that call `createComponentManager()` work unchanged
105+
3. All provider tests pass in their new locations (SDK test directory)
106+
4. `bun test --cwd packages/cli` -- zero regressions from baseline (1929 pass / 11 fail)
107+
5. `bun test --cwd packages/sdk` -- migrated tests pass (132+ pass / 0 fail)
108+
6. No CLI file imports from `lib/component/provider-*` or `lib/component/smithery-*`
109+
7. CLI `component/` directory contains exactly 3 files: `factory.ts`, `index.ts`, `skill-ops-impl.ts`
110+
111+
## Cross-Phase Consistency Notes
112+
113+
- **Import paths:** All moved tests must use `@agents/sdk/...` package imports, never relative `../../` paths back into CLI source.
114+
- **Error types:** Moved tests must import `SdkError` from `@agents/sdk/util/errors`, not `CliError`.
115+
- **Provider IDs:** SDK uses the same provider IDs as CLI (`local`, `local-agent`, `smithery`, etc.) plus `github`. No ID mapping needed.
116+
- **Return types:** `ProviderManager` (SDK) is the same class CLI already re-exports as `ComponentManager`. The alias is preserved in `index.ts`.
117+
- **SkillOperations stays lazy:** The adapter uses dynamic `import()` so skill modules are only loaded when skill operations are actually called, preserving CLI startup performance.
118+
119+
## File Inventory
120+
121+
| Phase | Created | Moved | Deleted | Modified |
122+
|-------|---------|-------|---------|----------|
123+
| 1 | 2 | 0 | 0 | 0 |
124+
| 2 | 0 | 0 | 0 | 2 |
125+
| 3 | 0 | 10 | 0 | 2 |
126+
| 4 | 0 | 0 | 9 | 1 |
127+
| 5 | 0 | 0 | 0 | 0 |
128+
| **Total** | **2** | **10** | **9** | **5** |
129+
130+
## Risks
131+
132+
| Risk | Likelihood | Impact | Mitigation |
133+
|------|-----------|--------|-----------|
134+
| Mock SkillOperations doesn't match real behavior | Medium | Medium | Test factory integration in CLI, not just unit tests |
135+
| Provider test imports break after move | Medium | Low | grep before move, fix in same commit |
136+
| `createComponentManager()` callers break | Low | High | Return type is same ProviderManager, method signatures unchanged |
137+
| CLI barrel re-exports miss something | Medium | Medium | grep for all `from.*lib/component` imports after cleanup |
138+
| Error code mapping incomplete | Medium | Medium | Full mapping table above; grep for all `error.code` assertions |
Lines changed: 124 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,124 @@
1+
# Phase 1: Wire SkillOperations Adapter
2+
3+
## Goal
4+
5+
Create a CLI adapter that implements the SDK's `SkillOperations` interface by delegating to the existing CLI skill modules (`skill-list.ts`, `skill-add.ts`, `skill-remove.ts`, `skill-info.ts`).
6+
7+
## Non-goals
8+
9+
- Do NOT modify the CLI factory yet (that is Phase 2)
10+
- Do NOT move or delete any existing provider files
11+
- Do NOT change any existing test files
12+
- Do NOT modify the SDK's `SkillOperations` interface
13+
14+
## Prerequisites
15+
16+
- SDK's `SkillOperations` interface exists at `packages/sdk/src/providers/local/skill-ops.ts`
17+
- CLI skill modules exist and export their functions: `listSkills`, `addSkill`, `removeSkills`, `skillInfo`
18+
- All tests pass at baseline: CLI 1929/11, SDK 132/0
19+
20+
## Files
21+
22+
| Action | Path |
23+
|--------|------|
24+
| Create | `packages/cli/src/lib/component/skill-ops-impl.ts` |
25+
| Create | `packages/cli/test/component/skill-ops-impl.test.ts` |
26+
27+
## Steps
28+
29+
- [ ] **1.1** Create `packages/cli/src/lib/component/skill-ops-impl.ts` with the `createSkillOps()` factory function
30+
- [ ] **1.2** Create `packages/cli/test/component/skill-ops-impl.test.ts` with structural assertions
31+
- [ ] **1.3** Run `bun test --cwd packages/cli` -- verify zero regressions (1929 pass / 11 fail)
32+
- [ ] **1.4** Commit: `feat(cli): add SkillOperations adapter for SDK provider DI`
33+
34+
## Acceptance Criteria
35+
36+
1. `createSkillOps()` returns an object satisfying `SkillOperations` (has `list`, `add`, `remove`, `info` methods)
37+
2. Each method is a function (typeof check)
38+
3. Dynamic imports use correct relative paths to CLI skill modules
39+
4. New test passes: `bun test packages/cli/test/component/skill-ops-impl.test.ts`
40+
5. Existing tests unaffected -- CLI baseline unchanged
41+
42+
## Failure Criteria
43+
44+
- If any existing CLI test that previously passed now fails, STOP -- the adapter file is somehow affecting module resolution
45+
- If `bun test` cannot find the skill modules at the dynamic import paths, the relative paths are wrong
46+
47+
## Fallback Logic
48+
49+
- If dynamic `import()` paths are wrong, check the actual locations of `skill-list.ts` etc. with `find packages/cli/src -name 'skill-*.ts'` and adjust paths
50+
- If TypeScript complains about the `SkillOperations` type import, verify the SDK package exports it: check `packages/sdk/package.json` exports map
51+
52+
## Examples
53+
54+
### Before (no adapter exists)
55+
56+
CLI factory directly imports provider classes:
57+
58+
```typescript
59+
// packages/cli/src/lib/component/factory.ts (CURRENT)
60+
import { LocalProvider } from './provider-local'
61+
// LocalProvider internally does: await import('../skill-list')
62+
```
63+
64+
### After (adapter created)
65+
66+
```typescript
67+
// packages/cli/src/lib/component/skill-ops-impl.ts (NEW)
68+
import type { SkillOperations } from '@agents/sdk/providers/local/skill-ops'
69+
70+
/**
71+
* CLI implementation of SkillOperations.
72+
* Bridges SDK's DI interface to CLI's skill-*.ts modules via dynamic imports.
73+
*/
74+
export function createSkillOps(): SkillOperations {
75+
return {
76+
async list(opts) {
77+
const { listSkills } = await import('../skill-list')
78+
return listSkills(opts)
79+
},
80+
async add(source, opts) {
81+
const { addSkill } = await import('../skill-add')
82+
return addSkill(source, opts)
83+
},
84+
async remove(names, opts) {
85+
const { removeSkills } = await import('../skill-remove')
86+
return removeSkills(names, opts)
87+
},
88+
async info(name, opts) {
89+
const { skillInfo } = await import('../skill-info')
90+
return skillInfo(name, opts)
91+
},
92+
}
93+
}
94+
```
95+
96+
### Test file
97+
98+
```typescript
99+
// packages/cli/test/component/skill-ops-impl.test.ts (NEW)
100+
import { describe, expect, test } from 'bun:test'
101+
import { createSkillOps } from '../../src/lib/component/skill-ops-impl'
102+
103+
describe('createSkillOps', () => {
104+
test('returns object with list, add, remove, info methods', () => {
105+
const ops = createSkillOps()
106+
expect(typeof ops.list).toBe('function')
107+
expect(typeof ops.add).toBe('function')
108+
expect(typeof ops.remove).toBe('function')
109+
expect(typeof ops.info).toBe('function')
110+
})
111+
112+
test('returns exactly 4 methods', () => {
113+
const ops = createSkillOps()
114+
const keys = Object.keys(ops).sort()
115+
expect(keys).toEqual(['add', 'info', 'list', 'remove'])
116+
})
117+
})
118+
```
119+
120+
## Notes
121+
122+
- The adapter uses dynamic `import()` (not static `import`) so that skill modules are only loaded when actually invoked. This preserves CLI startup performance -- commands that do not touch skills never load the skill modules.
123+
- This file stays in CLI permanently. It is CLI-specific glue code, not portable SDK logic.
124+
- The test file `skill-ops-impl.test.ts` also stays in CLI permanently (listed in the "do NOT move" set).

0 commit comments

Comments
 (0)