Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

## [Unreleased]

- Added `skill remove-registry` with project-scoped config removal and global removal that also deletes the registry's cached repository.
- Bias the brainstorm skill toward minimal solutions: baseline-first divergence, speculative-generality and verified-claim checks, and deletion cost in comparisons.

## [0.54.0] - 2026-08-21
Expand Down
67 changes: 67 additions & 0 deletions docs/ai/design/2026-08-22-feature-skill-remove-registry.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,67 @@
---
phase: design
title: Skill Registry Removal Design
description: Scoped config removal with guarded global cache deletion
---

# Skill Registry Removal Design

## Architecture Overview

```mermaid
flowchart LR
CLI[remove-registry command] --> Validate[validateRegistryId]
Validate --> Read[read selected config map]
Read --> Guard[own-property guard]
Guard --> Config[selected ConfigManager]
Config -->|global only| Contain[resolve and contain cache path]
Contain --> Cache[remove registry cache directory]
Config -. unchanged .-> Index[(Seed-backed discovery index)]
```

The command validates and guards the selected scope, while config managers persist the pure planner result. Project removal stops after config mutation. Global removal additionally deletes the contained registry cache path. The discovery index is intentionally unchanged because its seed contains unconfigured registries by design.

## Data Models

```ts
type SkillRegistryRemoveStatus = 'removed' | 'not-registered';
interface SkillRegistryRemoveMutation {
registries: Record<string, string>;
status: SkillRegistryRemoveStatus;
}
```

The planner tests own-property presence, copies the input, and omits only the selected ID.

## API Design

- `skill remove-registry <id> [-g|--global]`
- `planSkillRegistryRemove(registries, id)` is pure.
- Project/global config managers expose `removeSkillRegistry(id)`.
- `SkillManager.removeRegistryCache(id)` resolves `~/.ai-devkit/skills/<id>`, verifies containment under the cache root, and recursively removes it; the command only delegates to it.
- Missing selected-scope entries return the concise `try --global` error.

## Component Breakdown

| Component | Change |
|---|---|
| `util/skill-registry.ts` | Pure removal planner and types |
| `Config.ts`, `GlobalConfig.ts` | Scoped removal writers |
| `commands/skill.ts` | Scope guard and contained global cache deletion |
| Tests/docs | Behavior, exact copy, and follow-ups |

## Design Decisions

- Mutate only the selected scope.
- Do not clean the index: seed catalog entries are valid even without local registry configuration.
- Never use the network during removal.
- Protect the built-in ID explicitly; default sources are structurally protected by absence from user config maps.
- Preserve cache for project removal; delete the selected cached repository for global removal.
- Keep `remove-registry` paired with `add-registry`; defer registry-group migration.

## Non-Functional Requirements

- Work is constant-time apart from recursive global cache deletion.
- Writes retain existing config safety conventions.
- Invalid IDs cannot influence filesystem paths.
- Resolved cache targets must be strict descendants of `SKILL_CACHE_DIR`.
69 changes: 69 additions & 0 deletions docs/ai/implementation/2026-08-22-feature-skill-remove-registry.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,69 @@
---
phase: implementation
title: Skill Registry Removal Implementation
description: Implementation record for skill remove-registry
---

# Skill Registry Removal Implementation

## Development Setup

Use repository Node/npm. Run `npm ci` and `npm run build` before full gates.

## Code Structure

Changes are confined to CLI registry utilities, config managers, command registration, their tests, and user/lifecycle documentation. The follow-up removes the earlier index/manager additions.

## Implementation Notes

- Follow red-green-refactor for planner, config, index, and command slices.
- Reuse validation and add-registry scope conventions.
- Keep the planner copy-on-write and I/O-free.
- Remove only the selected map entry and leave the seed-backed discovery index unchanged.
- Never call registry fetch/update during removal.
- Preserve cache for project removal; delete the contained registry cache path for global removal.
- Implemented the pure own-property removal planner and project/global persistence methods. Targeted planner/config suites pass (71 tests).
- Removed focused index filtering because `SEED_INDEX_URL` intentionally catalogs unconfigured registries, making removed-registry entries equivalent to normal seed entries.
- Removed the frozen default-registry ID snapshot. Defaults are structurally protected because the planner and command only act on own properties in the selected user config map.
- Removed redundant command-level planner execution; config managers remain the single persistence planning layer.

## Integration Points

The command reads only the selected config map and delegates removal to its manager. For global removal, it additionally calls `SkillManager.removeRegistryCache(id)`, which resolves the cache root and target, verifies that the target is a strict descendant, and recursively removes that cache directory. The command layer contains no filesystem logic.

## Error Handling

Reject invalid IDs and the built-in registry before writes. A missing own property returns `Registry <id> is not registered (try --global).`. Unsafe resolved cache paths are rejected before config mutation or recursive deletion.

## Performance Considerations

Config maps are small. Project removal is constant-time apart from config I/O; global cache deletion is proportional to the cached repository size.

## Security Notes

Validate IDs before path construction. Resolve both cache root and target, require a strict contained target, and only then permit recursive removal. Never traverse installed-skill paths.

## Validation Evidence

Fresh validation on 2026-08-22 completed after resuming the interrupted session:

- `npm run build`: exit 0; Nx built all 6 projects.
- `npm test`: exit 0; all 6 projects passed (1,962 tests across 140 files).
- `npm run lint`: exit 0; all 6 projects passed with 4 existing unused-catch warnings and no errors.
- `npx ai-devkit@latest lint`: exit 0.
- `npx ai-devkit@latest lint --feature skill-remove-registry`: exit 0.
- Targeted Vitest command for planner, config, index/manager, and command suites: exit 0; 184 tests across 5 files.
- Planner-module coverage: 100% statements, branches, functions, and lines (12/12 statements, 11/11 branches, 2/2 functions, 12/12 lines).
- `node dist/cli.js skill remove-registry --help`: exit 0; exposes `-g, --global` and standard help only.
- `git diff --check`: exit 0.

Optional task tracing was unavailable: `npx ai-devkit@latest task list --name skill-remove-registry --json` returned `error: unknown command 'task'`.

Review follow-up validation on 2026-08-22:

- `npm run build`: exit 0; Nx built all 6 projects.
- `npm test`: exit 0; all 6 projects passed (1,954 tests across 140 files).
- `npm run lint`: exit 0; all 6 projects passed with 4 existing unused-catch warnings and no errors.
- Targeted Vitest command for command, planner, config, and manager suites: exit 0; 176 tests across 5 files.
- Planner-module coverage: 100% statements, branches, functions, and lines (11/11 statements, 11/11 branches, 2/2 functions, 11/11 lines).
- Feature lifecycle lint and built `remove-registry --help`: exit 0.
55 changes: 55 additions & 0 deletions docs/ai/planning/2026-08-22-feature-skill-remove-registry.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,55 @@
---
phase: planning
title: Skill Registry Removal Plan
description: TDD implementation and validation tasks
---

# Skill Registry Removal Plan

## Milestones

- [x] Planner and persistence behavior implemented with tests.
- [x] Simplified scoped command and guarded global cache deletion implemented with tests.
- [x] Documentation and full local validation gates completed.
- [x] Initial feature commit published in PR #196; review simplification prepared for the same PR.

## Task Breakdown

### Phase 1: Pure behavior and persistence

- [x] Add failing planner tests covering every branch and immutability.
- [x] Implement `planSkillRegistryRemove`; final coverage gate remains.
- [x] Add failing project/global config preservation tests, then removal methods.

### Phase 2: Command and cache behavior

- [x] Add command tests for validation, selected-scope guards, project cache preservation, and global cache deletion.
- [x] Implement `remove-registry` beside `add-registry` with resolved-path containment.
- [x] Remove focused index cleanup because seed catalog entries are valid without local configuration.
- [x] Remove the frozen default-registry ID snapshot and rely on config-map structure.

### Phase 3: Integration and polish

- [x] Update user docs, changelog, implementation, and testing records.
- [x] Run targeted tests/coverage, build, full workspace tests, lint, and lifecycle lint.
- [x] Create a conventional commit and open PR #196.
- [x] Validate and prepare the reviewed simplification follow-up for commit and push.

## Dependencies

Planner precedes persistence. The command uses existing config managers and `SKILL_CACHE_DIR`; no index API or external service is required.

## Timeline & Estimates

Single feature iteration: implementation and targeted tests, documentation, full gates, review/publish.

## Risks & Mitigation

- Unsafe recursive deletion: validate the registry ID and require the resolved target to remain inside the cache root.
- Wrong-scope deletion: read and mutate only the selected config map.
- Seed catalog inconsistency: leave the discovery index unchanged, matching its unconfigured-registry semantics.
- Default deletion: defaults are absent from user config maps and fail the own-property guard.

## Resources Needed

Existing CLI/config/cache modules, Vitest suites, lifecycle skills, npm workspace tooling, and GitHub CLI.
49 changes: 49 additions & 0 deletions docs/ai/requirements/2026-08-22-feature-skill-remove-registry.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
---
phase: requirements
title: Skill Registry Removal Requirements
description: Add the safe inverse of skill add-registry
---

# Skill Registry Removal Requirements

## Problem Statement

Users can register project or global skill registries with `skill add-registry`, but cannot unregister them without editing configuration manually.

## Goals & Objectives

- Add `skill remove-registry <id>` beside `add-registry`.
- Default to project-only removal and preserve its cached repository.
- Use `-g`/`--global` to remove the global entry and recursively delete that registry's cache directory.
- Leave the discovery index unchanged because seed entries do not imply local registration.
- Protect the built-in registry explicitly; default registries remain protected structurally because they are absent from user config maps.

Non-goals: discovery-index cleanup, installed-skill traversal/removal, registry-group command migration, or changes to unrelated update behavior.

## User Stories & Use Cases

- Remove a project registry without affecting a same-ID global registration.
- Remove a global registration and its cache with `--global`.
- Give automation deterministic output without prompts or network traffic.
- Reject missing registrations with a concise `try --global` hint.

## Success Criteria

- Reuse `validateRegistryId` before config work or path deletion.
- A pure copy-on-write planner returns `removed` or `not-registered` with the next registry map.
- Config writers preserve unrelated keys and registry entries.
- Removal leaves the seed-backed discovery index unchanged.
- Global cache deletion resolves the target and proves it is contained inside `SKILL_CACHE_DIR` before recursive removal.
- The built-in source cannot be removed; defaults absent from the selected config map fail the own-property guard.
- Tests cover planner behavior, both scopes, cache deletion, validation order, and exact messages.
- User docs and changelog describe the command and safe boundary.

## Constraints & Assumptions

- Removal never fetches registry or index data.
- Project removal never deletes cache data.
- Global removal deletes `~/.ai-devkit/skills/<id>` after containment validation; it does not traverse installed-skill locations.

## Questions & Open Items

All material questions are resolved by the approved simplification. A coordinated registry-group alias/migration remains a possible follow-up.
72 changes: 72 additions & 0 deletions docs/ai/testing/2026-08-22-feature-skill-remove-registry.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,72 @@
---
phase: testing
title: Skill Registry Removal Testing
description: Coverage and validation strategy for skill remove-registry
---

# Skill Registry Removal Testing

## Test Coverage Goals

- 100% statements, branches, functions, and lines for `planSkillRegistryRemove`.
- Critical command, persistence, cache containment, and failure paths covered.
- Full workspace test and lint gates remain green.

## Unit Tests

### Pure planner

- [x] Present/absent IDs return correct status and registry maps.
- [x] Input is immutable; siblings, empty maps, and inherited properties behave correctly.
- [x] Coverage demonstrates 100% statements, branches, functions, and lines for the planner module.

### Config and cache behavior

- [x] Project/global removers preserve unrelated registries and config keys.
- [x] Existing missing/malformed-config guarantees remain intact in the full config suites.
- [x] Project removal preserves cache; global removal deletes only the resolved registry cache path.

## Integration Tests

- [x] Default removes only project; `-g`/`--global` remove only global.
- [x] Built-in removal is rejected before config mutation.
- [x] Missing selected-scope registrations use the concise `try --global` error.
- [x] Invalid IDs cause no config reads or cache deletion.
- [x] Global cache deletion targets a strict descendant of the cache root (unit-tested in `SkillManager.removeRegistryCache`).
- [x] The discovery index is not touched during removal.

## End-to-End Tests

- [x] CLI help exposes only the scope option beside `add-registry`.
- [x] Full workspace build/tests/lint and lifecycle lint pass.
- [x] Adjacent add-registry behavior remains green.

## Test Data

Use command mocks and temporary filesystem fixtures. Seed mixed registry maps to prove sibling preservation and assert the global cache target path.

## Test Reporting & Coverage

Run targeted Vitest suites and planner coverage, then repository-native full test/lint gates. Record fresh results during completion.

Fresh simplification results from 2026-08-22:

- Targeted suites: 5 files passed, 176 tests passed.
- Planner module: 100% statements (11/11), branches (11/11), functions (2/2), and lines (11/11).
- Workspace build: 6 projects passed.
- Workspace tests: 6 projects passed; 140 files and 1,954 tests passed.
- Workspace lint: 6 projects passed with no errors; 4 unrelated existing warnings were reported.
- Base and feature lifecycle lint: passed.
- Built CLI help: exit 0 and lists `-g, --global` plus standard `--help`.

## Manual Testing

Inspect CLI help and exact output assertions; no browser/device checks apply.

## Performance Testing

No load test is required; the only size-dependent operation is recursive global cache deletion.

## Bug Tracking

Fix regressions before review and document intentional follow-ups.
Loading
Loading