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
2 changes: 2 additions & 0 deletions .please/docs/knowledge/product.md
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,8 @@ One command (`ask install`) generates `AGENTS.md` with lazy documentation refere

The lazy commands `ask src <spec>` and `ask docs <spec>` give coding agents on-demand documentation access: they print absolute paths to a cached source tree (and any documentation directories), fetching on cache miss. Both commands work via shell substitution, e.g. `rg "pattern" $(ask src react)`.

`ask skills <spec>` is a sibling namespace that surfaces **producer-side skills** shipped by libraries (a `skills/` directory convention). `ask skills install` vendors those skills into `.ask/skills/` and symlinks them into every detected coding agent directory (Claude Code, Cursor, OpenCode, Codex) so the agent can consume library-authored instruction bundles directly.

## Product Components

| Component | Purpose |
Expand Down
1 change: 1 addition & 0 deletions .please/docs/tracks.jsonl
Original file line number Diff line number Diff line change
Expand Up @@ -22,3 +22,4 @@
{"id":"interactive-add-20260412","type":"feature","status":"in_progress","phase":"implement","issue":"#71","created":"2026-04-12","section":"active"}
{"id":"monorepo-tag-pattern-20260413","type":"feature","status":"review","phase":"finalize","issue":"#77","pr":"#79","created":"2026-04-13","section":"completed"}
{"id":"telemetry-driven-registry-20260413","type":"feature","status":"in_progress","phase":"implement","issue":"#78","created":"2026-04-13","section":"active"}
{"id":"ask-skills-command-20260414","type":"feature","status":"review","phase":"finalize","issue":"#86","pr":"#87","created":"2026-04-14","section":"completed"}
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
{
"track_id": "ask-skills-command-20260414",
"type": "feature",
"status": "review",
"created_at": "2026-04-14T00:00:00Z",
"updated_at": "2026-04-15T17:30:00Z",
"issue": "#86",
"pr": "#87",
"project": "",
"project_item_id": ""
}
163 changes: 163 additions & 0 deletions .please/docs/tracks/completed/ask-skills-command-20260414/plan.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,163 @@
# Plan: `ask skills` Command

> Track: ask-skills-command-20260414
> Spec: [spec.md](./spec.md)

## Overview

- **Source**: /please:plan
- **Track**: ask-skills-command-20260414
- **Issue**: (assigned in metadata after Issue creation)
- **Created**: 2026-04-14
- **Approach**: New `skills` citty namespace with `list|install|remove` subcommands, backed by a `.ask/skills/` vendored store, `.ask/skills-lock.json` lock file, and relative POSIX symlinks into each selected agent's `skills/` directory. Reuses `ensureCheckout` and the existing `findDocLikePaths` walker pattern.

## Purpose

Give users a single command family to surface and consume library-provided (producer-side) skill directories, mirroring `ask docs` for documentation. `install` makes skills actually usable by any supported coding agent (Claude Code, Cursor, OpenCode, Codex) without duplicating bytes across agent dirs.

## Context

- `ask docs` and `ask src` already use a shared `ensureCheckout` helper (`packages/cli/src/commands/ensure-checkout.ts`) and a generic walker (`packages/cli/src/commands/find-doc-paths.ts`). `ask skills` reuses the first verbatim and parallels the second with a `/skill/i` regex.
- Vendoring under `.ask/` is an established pattern: `.ask/docs/` + `.ask/resolved.json` are already managed by `packages/cli/src/ignore-files.ts` via a `# ask:start ... # ask:end` marker block. The same mechanism extends to `.ask/skills/` + `.ask/skills-lock.json` with a single patch payload edit.
- The existing `generateSkill` in `packages/cli/src/skill.ts` emits consumer-side `.claude/skills/<name>-docs/SKILL.md` files (docs pointers). Producer-side skills are orthogonal and must never collide with that path — we install under `<agent>/skills/<skill-name>/` (no `-docs` suffix).

## Architecture Decision

**Why a namespace command instead of flags on `ask install`**: `list` and `install` have incompatible semantics (read-only vs mutating) and `ask install` is already overloaded. A dedicated `skills` namespace keeps the mental model clean and keeps future subcommands (e.g. `ask skills update`) discoverable.

**Why a lock file instead of scanning**: `remove` must undo exactly what `install` did. Scanning `<agent>/skills/` at remove time would either over-delete (user-authored skills) or under-delete (agents removed from the detection list). The lock is the only safe source of truth.

**Why symlinks from agent dirs to `.ask/skills/` instead of copies**: copies triple/quadruple bytes across agent dirs and drift if the vendored copy is refreshed. A relative symlink from `.claude/skills/foo` → `../../.ask/skills/<key>/foo` stays in sync automatically and is safe to commit-ignore at the agent level (via the vendored-skills marker block).

**Why `.ask/skills/<spec-key>/<skill-name>/` not `.ask/skills/<skill-name>/`**: multiple libraries can ship skills with the same name. Namespacing by spec avoids collisions and makes `remove` a single `rm -rf` of the spec subdir.

**`<spec-key>` encoding**: replace `/`, `@`, `:` with `__` — reversible, grep-friendly, filesystem-safe on every platform. Examples: `npm__next__14.2.3`, `github__vercel__ai__v5.0.0`.

## Architecture Diagram

```
ensureCheckout (existing)
|
v
+----------------------------------+
| ~/.ask/github/.../<ref>/ |
+----------------------------------+
|
findSkillLikePaths (new)
|
+---list---+ +----install----+
| stdout | | |
+----------+ v v
vendor(copy) agent-detect + select
| |
v v
.ask/skills/<key>/ symlinks in .claude/skills/ etc
\ /
\ /
v v
.ask/skills-lock.json
(updated atomically)
```

## Tasks

- [x] T001 [P] Add `findSkillLikePaths` walker in `packages/cli/src/commands/find-skill-paths.ts` plus tests in `packages/cli/test/commands/find-skill-paths.test.ts` (file: packages/cli/src/commands/find-skill-paths.ts). Mirrors `findDocLikePaths` with regex `/skill/i`; same MAX_DEPTH=4 and SKIP_DIRS. Acceptance: returns root + all `/skill/i` subdirs; empty array on missing root.
- [x] T002 [P] Add spec-key encoder `encodeSpecKey` in `packages/cli/src/skills/spec-key.ts` + tests (file: packages/cli/src/skills/spec-key.ts). Converts resolved `(ecosystem, name, version)` to `{ecosystem}__{name}__{version}` (with `/` → `__`). Acceptance: round-trips via `decodeSpecKey` for at least npm, github, scoped-npm, monorepo-tag cases.
- [x] T003 [P] Add `.ask/skills-lock.json` schema + IO helpers in `packages/cli/src/skills/lock.ts` + tests (file: packages/cli/src/skills/lock.ts). Functions: `readLock`, `upsertEntry`, `removeEntry`, `writeLockAtomic` (tmp-file + rename). Lock shape: `{ version: 1, entries: { [specKey]: { spec, specKey, skills: [{ name, agents: ["claude", ...] }], installedAt } } }`.
- [x] T004 [P] Add agent detector in `packages/cli/src/skills/agent-detect.ts` + tests (file: packages/cli/src/skills/agent-detect.ts). Function `detectAgents(projectDir)` returns `{ name, label, skillsDir }[]` for every agent marker present: `.claude/` → `.claude/skills`, `.cursor/` → `.cursor/skills`, `.opencode/` → `.opencode/skills`, `.codex/` → `.codex/skills`. `AGENTS.md` alone does not enable any target.
- [x] T005 [P] Implement `ask skills list` in `packages/cli/src/commands/skills/list.ts` (file: packages/cli/src/commands/skills/list.ts) (depends on T001). Mirrors `runDocs`: calls `ensureCheckout`, walks `node_modules/<npmPackageName>/` if set, walks the checkout, prints paths via `log()`. Honors `--no-fetch`. Includes unit tests with mocked `ensureCheckout`.
- [x] T006 Implement vendor step `vendorSkills(projectDir, specKey, sourcePaths)` in `packages/cli/src/skills/vendor.ts` + tests (file: packages/cli/src/skills/vendor.ts) (depends on T002). Atomic: copy to `.ask/skills/<specKey>/<basename(sourcePath)>/` via staging dir + rename on success. Refresh-safe: replaces any prior version of the same vendored key.
- [x] T007 Implement symlink utilities `linkSkill`, `verifyLink`, `unlinkIfOwned` in `packages/cli/src/skills/symlinks.ts` + tests (file: packages/cli/src/skills/symlinks.ts). Uses relative POSIX symlinks. `unlinkIfOwned` only unlinks when `fs.readlinkSync` resolves to the expected vendored target; never deletes real directories.
- [x] T008 Extend `ignore-files.ts` to also vendor `.ask/skills/` and `.ask/skills-lock.json` (file: packages/cli/src/ignore-files.ts). Update `ROOT_PATCHES` payloads for `.gitignore`, `.prettierignore`, `sonar-project.properties`, `.markdownlintignore`. Add a test that `manageIgnoreFiles('install')` produces the new marker block content.
- [x] T009 Implement `ask skills install` in `packages/cli/src/commands/skills/install.ts` (file: packages/cli/src/commands/skills/install.ts) (depends on T002, T003, T004, T005, T006, T007, T008). Orchestrates: resolve via `ensureCheckout` → walk → vendor → detect → multiselect (if >1) via `consola.prompt` → symlink → update lock → `manageIgnoreFiles('install')`. Flags: `--force`, `--no-fetch`, `--agent <name>[,<name>...]` (opt-in explicit override of detection). Emits `consola.info` summary of what was installed.
- [x] T010 Implement `ask skills remove` in `packages/cli/src/commands/skills/remove.ts` (file: packages/cli/src/commands/skills/remove.ts) (depends on T003, T007). Reads lock, iterates recorded symlinks, `unlinkIfOwned` each, deletes `.ask/skills/<specKey>/`, purges lock entry. Errors if lock entry missing (unless `--ignore-missing`).
- [x] T011 Wire `skillsCmd` into `packages/cli/src/index.ts` (file: packages/cli/src/index.ts) (depends on T005, T009, T010). Default run (no subcommand) dispatches to `list`. Update `cli/commands.test.ts` / `src-docs-registration.test.ts` style test to assert `skills`, `skills list`, `skills install`, `skills remove` are all registered.
- [x] T012 End-to-end integration test in `packages/cli/test/commands/skills.integration.test.ts` (file: packages/cli/test/commands/skills.integration.test.ts) (depends on T009, T010, T011). Happy path: install into a fixture with `.claude/` + `.cursor/`, verify vendored files, verify symlinks, verify lock; re-install is no-op; remove deletes everything.
- [x] T013 Documentation update in root `README.md` and `packages/cli/README.md` (file: README.md) (depends on T011). Add `ask skills` section under CLI usage; mention v1 platform limits (Linux/macOS only).

## Dependencies

```
T001 ---\
T002 ---+--> T005 ---\
T003 ---| +--> T009 --> T011 --> T012 --> T013
T004 ---| | ^
T006 ---| | |
T007 ---+------------+ |
T008 ---+-------------------------+
T010 ---(depends on T003, T007)---+
```

T001–T004 and T008 are [P] (parallel). T005 depends only on T001. T006–T007 are leaf utilities runnable after T002/T003 land. T009 is the integration point; it blocks T010–T013.

## Key Files

- **Reuse**: `packages/cli/src/commands/ensure-checkout.ts`, `packages/cli/src/commands/find-doc-paths.ts` (pattern reference)
- **Reuse**: `packages/cli/src/ignore-files.ts` (extend `ROOT_PATCHES` + `NESTED_CONFIGS`)
- **Reuse**: `packages/cli/src/markers.ts` (wrap/inject/remove)
- **Reference (unchanged)**: `packages/cli/src/skill.ts` (consumer-side `*-docs` SKILL.md — must NOT be touched)
- **New**: `packages/cli/src/skills/` (vendor, symlinks, lock, agent-detect, spec-key)
- **New**: `packages/cli/src/commands/skills/` (list, install, remove) + `packages/cli/src/commands/find-skill-paths.ts`
- **Modified**: `packages/cli/src/index.ts` (register `skillsCmd`)
- **Modified**: `README.md`, `packages/cli/README.md`

## Verification

- `bun run --cwd packages/cli lint` — zero errors.
- `bun run --cwd packages/cli test` — all new unit + integration tests pass.
- Manual smoke:
1. `cd /tmp/ask-smoke && mkdir -p .claude .cursor && npm init -y && node <ask-cli>/dist/index.js skills install github:pleaseai/some-repo@v1`
2. Confirm `.ask/skills/github__pleaseai__some-repo__v1/` populated, symlinks present in `.claude/skills/` and `.cursor/skills/`, `.gitignore` patched.
3. Repeat same command — no error, no duplication.
4. `skills remove ...` — all symlinks and vendored dir gone; lock entry purged.
- AC-1 through AC-7 from spec each covered by at least one automated test.

## Progress

- [x] Phase 1 (T001–T004, T008): foundation utilities
- [x] Phase 2 (T005–T007): list command + vendor/symlink primitives
- [x] Phase 3 (T009–T011): install/remove orchestration + wiring
- [x] Phase 4 (T012–T013): integration test + docs

## Decision Log

- 2026-04-14: chose symlink-based model over copy-per-agent after user guidance (중복 설치 방지).
- 2026-04-14: `<spec-key>` encoding uses `__` separator for grep-friendliness; alternative (URL encoding) rejected as harder to eyeball.
- 2026-04-14: Windows junction fallback deferred to a follow-up track; v1 assumes POSIX symlink support.

## Surprises & Discoveries

- `findSkillLikePaths` needed a tight basename match (`/^skills?$/i`) when used as the "skills parent" selector in install. A loose substring match was vendoring the checkout root whenever its tempdir name happened to contain "skill" (e.g. `ask-skills-e2e-ck-XXXX`) — harmless in production, but it pulled `skills/` into itself and left the lock entry with `name: "skills"` instead of `name: "alpha"`. The walker still uses loose `/skill/i` for `list` output; the stricter filter lives in `collectSkillDirs`.
- `manageIgnoreFiles` validates `ask.json` against the real `AskJsonSchema` (`{ libraries: [] }`), not a fabricated `{ entries: [] }` shape. Test fixtures had to match — a reminder that the schema package is authoritative and cannot be faked.
- The integration test needed schema package built first (`bun run --cwd packages/schema build`) because `packages/cli/src/schemas.ts` imports `@pleaseai/ask-schema` from the workspace, and an unbuilt workspace dep surfaces as a runtime "Cannot find module" error rather than a TS build error.

- [x] (2026-04-15 17:30 KST) Review fixes applied (SHA: `44de2d6`)

## Outcomes & Retrospective

### What Was Shipped

- `ask skills` CLI namespace with `list`, `install`, `remove` subcommands (+ `ask skills <spec>` shorthand for `list`).
- `.ask/skills/<spec-key>/<skill-name>/` vendored layout backed by `.ask/skills-lock.json` for deterministic reverse.
- Relative POSIX symlinks into auto-detected agent dirs (`.claude/`, `.cursor/`, `.opencode/`, `.codex/`) with multiselect prompt when >1 agent is present and `--agent` CSV override.
- 5 new foundation modules under `packages/cli/src/skills/` (spec-key, lock, agent-detect, vendor, symlinks).
- Ignore-file management extended to mark skills paths vendored, now gated by a multi-signal opt-in check so skills-only users are covered.

### What Went Well

- The existing `ensureCheckout` + `findDocLikePaths` + `ignore-files.ts` patterns dropped in almost verbatim. Minimal new primitives.
- Symlink + lock architecture means `remove` can never touch user-authored content — `unlinkIfOwned` compares the symlink target against the vendored path first.
- TDD cycle (67 unit + integration tests) caught two design bugs before merge: the walker's substring-match surfacing the tempdir root, and `ask.json`-only opt-in gate missing skills-only users.
- Review-fix loop surfaced the FR-11/AC-6 gap that the original integration tests masked by always seeding `ask.json`.

### What Could Improve

- Initial spec-key encode/decode design encoded `@` which broke scoped-npm round-trip. Should have exercised round-trip tests against all four representative specs before landing the encoder. Now fixed (only `/`/`:` encoded, `@` preserved).
- `collectSkillDirs` needed a tighter `^skills?$` regex separate from the walker's looser `/skill/i` — worth documenting the dual regex convention in future producer-side skill features.
- `manageIgnoreFiles` implicit opt-in (ask.json-only) was easy to overlook. The multi-signal `hasAskOptIn` helper should probably become the canonical predicate anywhere new ASK entry points appear.

### Tech Debt Created

- Windows native junction fallback is deferred (noted in spec Out of Scope + README). Any Windows user will hit a symlink permission issue today — follow-up track required.
- Convention-based auto-discovery of skill packages (via `package.json` keywords / intent adapter) deferred.
- No declarative `ask.json` entry shape for skills yet — users must invoke `ask skills install` imperatively per library.
Loading