|
| 1 | +--- |
| 2 | +phase: design |
| 3 | +title: System Design & Architecture |
| 4 | +description: Define the technical architecture, components, and data models |
| 5 | +--- |
| 6 | + |
| 7 | +# System Design & Architecture: Skill Update |
| 8 | + |
| 9 | +## Architecture Overview |
| 10 | +**What is the high-level system structure?** |
| 11 | + |
| 12 | +```mermaid |
| 13 | +graph TD |
| 14 | + CLI[skill.ts Command] -->|calls| SM[SkillManager] |
| 15 | + SM -->|uses| GU[Git Utilities] |
| 16 | + SM -->|reads| Cache[~/.ai-devkit/skills/] |
| 17 | + SM -->|validates| GC[GlobalConfigManager] |
| 18 | + |
| 19 | + GU -->|executes| Git[git pull] |
| 20 | + Cache -->|contains| R1[Registry 1/.git] |
| 21 | + Cache -->|contains| R2[Registry 2/.git] |
| 22 | + Cache -->|contains| R3[Registry 3/no-git] |
| 23 | + |
| 24 | + SM -->|collects| Results[Update Results] |
| 25 | + Results -->|displays| Summary[Success/Skip/Error Summary] |
| 26 | +``` |
| 27 | + |
| 28 | +### Key Components |
| 29 | +1. **CLI Command Handler** (`skill.ts`): Parses user input and delegates to SkillManager |
| 30 | +2. **SkillManager** (`SkillManager.ts`): Orchestrates update logic, error handling, and reporting |
| 31 | +3. **Git Utilities** (`util/git.ts`): Provides git operations (new `pullRepository` function) |
| 32 | +4. **Skill Cache** (`~/.ai-devkit/skills/`): File system storage for cloned registries |
| 33 | + |
| 34 | +### Technology Stack |
| 35 | +- **Language**: TypeScript |
| 36 | +- **CLI Framework**: Commander.js (existing) |
| 37 | +- **File System**: fs-extra (existing) |
| 38 | +- **Process Execution**: child_process.exec (existing in git utils) |
| 39 | +- **Styling**: chalk (existing) |
| 40 | + |
| 41 | +## Data Models |
| 42 | +**What data do we need to manage?** |
| 43 | + |
| 44 | +### UpdateResult Interface |
| 45 | +```typescript |
| 46 | +interface UpdateResult { |
| 47 | + registryId: string; // e.g., "anthropic/skills" |
| 48 | + status: 'success' | 'skipped' | 'error'; |
| 49 | + message: string; // Human-readable status message |
| 50 | + error?: Error; // Error object if status is 'error' |
| 51 | +} |
| 52 | +``` |
| 53 | + |
| 54 | +### UpdateSummary Interface |
| 55 | +```typescript |
| 56 | +interface UpdateSummary { |
| 57 | + total: number; // Total registries processed |
| 58 | + successful: number; // Successfully updated |
| 59 | + skipped: number; // Skipped (non-git) |
| 60 | + failed: number; // Failed with errors |
| 61 | + results: UpdateResult[]; // Detailed results |
| 62 | +} |
| 63 | +``` |
| 64 | + |
| 65 | +### Data Flow |
| 66 | +1. User runs command → CLI parses arguments |
| 67 | +2. SkillManager scans cache directory → identifies registries |
| 68 | +3. For each registry → check if git repo → attempt pull |
| 69 | +4. Collect results → aggregate summary → display to user |
| 70 | + |
| 71 | +## API Design |
| 72 | +**How do components communicate?** |
| 73 | + |
| 74 | +### New SkillManager Methods |
| 75 | + |
| 76 | +#### `updateSkills(registryId?: string): Promise<UpdateSummary>` |
| 77 | +Updates all skills or a specific registry. |
| 78 | + |
| 79 | +**Parameters**: |
| 80 | +- `registryId` (optional): Specific registry to update (e.g., "anthropic/skills") |
| 81 | + |
| 82 | +**Returns**: `UpdateSummary` with detailed results |
| 83 | + |
| 84 | +**Behavior**: |
| 85 | +- If `registryId` provided: Update only that registry |
| 86 | +- If `registryId` omitted: Update all registries in cache |
| 87 | +- Validates git installation before proceeding |
| 88 | +- Continues on errors, collecting all results |
| 89 | + |
| 90 | +#### `private updateRegistry(registryPath: string, registryId: string): Promise<UpdateResult>` |
| 91 | +Updates a single registry. |
| 92 | + |
| 93 | +**Parameters**: |
| 94 | +- `registryPath`: Absolute path to registry directory |
| 95 | +- `registryId`: Registry identifier (e.g., "anthropic/skills") |
| 96 | + |
| 97 | +**Returns**: `UpdateResult` for this registry |
| 98 | + |
| 99 | +**Behavior**: |
| 100 | +1. Check if `.git` directory exists |
| 101 | +2. If not git repo: return 'skipped' status |
| 102 | +3. If git repo: call `pullRepository` |
| 103 | +4. If pull succeeds: return 'success' status |
| 104 | +5. If pull fails: return 'error' status with error details |
| 105 | + |
| 106 | +### New Git Utility Functions |
| 107 | + |
| 108 | +#### `pullRepository(repoPath: string): Promise<void>` |
| 109 | +Pulls latest changes for a git repository. |
| 110 | + |
| 111 | +**Parameters**: |
| 112 | +- `repoPath`: Absolute path to git repository |
| 113 | + |
| 114 | +**Throws**: Error if git pull fails |
| 115 | + |
| 116 | +**Implementation**: |
| 117 | +```typescript |
| 118 | +export async function pullRepository(repoPath: string): Promise<void> { |
| 119 | + try { |
| 120 | + await execAsync('git pull', { |
| 121 | + cwd: repoPath, |
| 122 | + timeout: 30000, |
| 123 | + }); |
| 124 | + } catch (error: any) { |
| 125 | + throw new Error(`Git pull failed: ${error.message}`); |
| 126 | + } |
| 127 | +} |
| 128 | +``` |
| 129 | + |
| 130 | +#### `isGitRepository(dirPath: string): Promise<boolean>` |
| 131 | +Checks if a directory is a git repository. |
| 132 | + |
| 133 | +**Parameters**: |
| 134 | +- `dirPath`: Absolute path to directory |
| 135 | + |
| 136 | +**Returns**: `true` if `.git` exists, `false` otherwise |
| 137 | + |
| 138 | +**Implementation**: |
| 139 | +```typescript |
| 140 | +export async function isGitRepository(dirPath: string): Promise<boolean> { |
| 141 | + const gitDir = path.join(dirPath, '.git'); |
| 142 | + return await fs.pathExists(gitDir); |
| 143 | +} |
| 144 | +``` |
| 145 | + |
| 146 | +### CLI Command Interface |
| 147 | + |
| 148 | +```typescript |
| 149 | +skillCommand |
| 150 | + .command('update [registry-id]') |
| 151 | + .description('Update skills from registries (e.g., ai-devkit skill update or ai-devkit skill update anthropic/skills)') |
| 152 | + .action(async (registryId?: string) => { |
| 153 | + // Implementation |
| 154 | + }); |
| 155 | +``` |
| 156 | + |
| 157 | +## Component Breakdown |
| 158 | +**What are the major building blocks?** |
| 159 | + |
| 160 | +### 1. Command Handler (`skill.ts`) |
| 161 | +**Responsibilities**: |
| 162 | +- Parse command arguments |
| 163 | +- Create SkillManager instance |
| 164 | +- Call `updateSkills()` method |
| 165 | +- Handle top-level errors |
| 166 | +- Exit with appropriate code |
| 167 | + |
| 168 | +**Changes Required**: |
| 169 | +- Add new `update [registry-id]` command |
| 170 | +- Wire up to SkillManager.updateSkills() |
| 171 | + |
| 172 | +### 2. SkillManager (`SkillManager.ts`) |
| 173 | +**Responsibilities**: |
| 174 | +- Scan skill cache directory |
| 175 | +- Filter registries (all or specific) |
| 176 | +- Orchestrate updates with progress feedback |
| 177 | +- Collect and aggregate results |
| 178 | +- Format and display summary |
| 179 | + |
| 180 | +**New Methods**: |
| 181 | +- `updateSkills(registryId?: string): Promise<UpdateSummary>` |
| 182 | +- `private updateRegistry(registryPath: string, registryId: string): Promise<UpdateResult>` |
| 183 | +- `private displayUpdateSummary(summary: UpdateSummary): void` |
| 184 | + |
| 185 | +### 3. Git Utilities (`util/git.ts`) |
| 186 | +**Responsibilities**: |
| 187 | +- Execute git pull command |
| 188 | +- Validate git repository status |
| 189 | +- Handle git errors |
| 190 | + |
| 191 | +**New Functions**: |
| 192 | +- `pullRepository(repoPath: string): Promise<void>` |
| 193 | +- `isGitRepository(dirPath: string): Promise<boolean>` |
| 194 | + |
| 195 | +### 4. Progress Display |
| 196 | +**Responsibilities**: |
| 197 | +- Show real-time progress for each registry |
| 198 | +- Use chalk for colored output |
| 199 | +- Provide clear status indicators |
| 200 | + |
| 201 | +**Output Format**: |
| 202 | +``` |
| 203 | +Updating skills... |
| 204 | +
|
| 205 | + → anthropic/skills... ✓ Updated |
| 206 | + → openai/skills... ⊘ Skipped (not a git repository) |
| 207 | + → custom/tools... ✗ Failed (uncommitted changes) |
| 208 | +
|
| 209 | +Summary: |
| 210 | + ✓ 1 updated |
| 211 | + ⊘ 1 skipped |
| 212 | + ✗ 1 failed |
| 213 | +
|
| 214 | +Errors: |
| 215 | + • custom/tools: Git pull failed: You have unstaged changes. |
| 216 | + Tip: Run 'git status' in ~/.ai-devkit/skills/custom/tools to see details. |
| 217 | +``` |
| 218 | + |
| 219 | +## Design Decisions |
| 220 | +**Why did we choose this approach?** |
| 221 | + |
| 222 | +### Decision 1: Update Registries, Not Individual Skills |
| 223 | +**Rationale**: |
| 224 | +- Skills are stored within registry repositories |
| 225 | +- Registries are the git units (have `.git` directories) |
| 226 | +- Simpler mental model: update the source, not individual items |
| 227 | +- Consistent with how skills are installed (from registries) |
| 228 | + |
| 229 | +**Alternatives Considered**: |
| 230 | +- Update individual skills: Would require tracking which registry each skill came from |
| 231 | +- Update all at once: Chosen approach (with optional registry filter) |
| 232 | + |
| 233 | +### Decision 2: Continue on Errors |
| 234 | +**Rationale**: |
| 235 | +- Users may have multiple registries; one failure shouldn't block others |
| 236 | +- Collect all errors and report at end for better UX |
| 237 | +- Allows users to see full picture before taking action |
| 238 | + |
| 239 | +**Alternatives Considered**: |
| 240 | +- Stop on first error: Too disruptive, poor UX |
| 241 | +- Ignore errors silently: Dangerous, users wouldn't know about issues |
| 242 | + |
| 243 | +### Decision 3: Skip Non-Git Directories |
| 244 | +**Rationale**: |
| 245 | +- Users might manually copy skills or have other files in cache |
| 246 | +- Attempting git operations on non-git directories would fail |
| 247 | +- Better to detect and skip gracefully |
| 248 | + |
| 249 | +**Alternatives Considered**: |
| 250 | +- Error on non-git directories: Too strict, would break existing workflows |
| 251 | +- Try to update anyway: Would cause confusing errors |
| 252 | + |
| 253 | +### Decision 4: No Automatic Conflict Resolution |
| 254 | +**Rationale**: |
| 255 | +- Git conflicts require human judgment |
| 256 | +- Automatic resolution could lose user changes |
| 257 | +- Better to fail with helpful message |
| 258 | + |
| 259 | +**Alternatives Considered**: |
| 260 | +- Auto-stash changes: Could hide important modifications |
| 261 | +- Force pull: Would lose local changes |
| 262 | + |
| 263 | +## Non-Functional Requirements |
| 264 | +**How should the system perform?** |
| 265 | + |
| 266 | +### Performance Targets |
| 267 | +- **Update Speed**: < 5 seconds per registry (network dependent) |
| 268 | +- **Startup Time**: < 500ms to begin first update |
| 269 | +- **Memory Usage**: Minimal (streaming git output, not buffering) |
| 270 | + |
| 271 | +### Scalability Considerations |
| 272 | +- Handle 10+ registries efficiently |
| 273 | +- Parallel updates not required (sequential is fine for v1) |
| 274 | +- Future: Consider parallel updates with concurrency limit |
| 275 | + |
| 276 | +### Security Requirements |
| 277 | +- No credential storage (rely on system git credentials) |
| 278 | +- No arbitrary command execution (use parameterized git commands) |
| 279 | +- Validate registry paths to prevent directory traversal |
| 280 | + |
| 281 | +### Reliability/Availability |
| 282 | +- Graceful degradation on network failures |
| 283 | +- Clear error messages for common issues |
| 284 | +- No data loss (read-only operations on cache) |
| 285 | +- Timeout protection (30s per git pull) |
| 286 | + |
| 287 | +### Error Handling Strategy |
| 288 | +1. **Git Not Installed**: Fail fast with installation instructions |
| 289 | +2. **Network Errors**: Catch, log, continue to next registry |
| 290 | +3. **Merge Conflicts**: Catch, provide helpful message, continue |
| 291 | +4. **Permission Errors**: Catch, explain issue, continue |
| 292 | +5. **Timeout**: Catch, suggest manual intervention, continue |
| 293 | + |
| 294 | +### User Experience |
| 295 | +- **Progress Feedback**: Show which registry is being updated |
| 296 | +- **Color Coding**: Green for success, yellow for skip, red for error |
| 297 | +- **Helpful Messages**: Suggest remediation for common errors |
| 298 | +- **Summary**: Clear overview of what happened |
| 299 | + |
0 commit comments