From 2ef0d90836fdb638d081bd2cc016856ab92f271c Mon Sep 17 00:00:00 2001 From: Ezequiel Canul Date: Fri, 2 Oct 2026 14:17:34 -0600 Subject: [PATCH 1/4] chore: ignore local agent tool dirs and test backup residue --- .gitignore | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/.gitignore b/.gitignore index a221fe7..f7501e7 100644 --- a/.gitignore +++ b/.gitignore @@ -30,3 +30,11 @@ Thumbs.db # SDD skill registry (contiene rutas absolutas de la máquina local) .atl/ +# Directorios locales de herramientas de agentes IA (config y caché por máquina) +.pi/ +.pi-lens/ +.antigravitycli/ + +# Residuo de tests (backups escritos por el writer durante la suite) +*.yaml.bak + From 2a3894a6a333b0d5b6dd247ad45c0814d5ec94a3 Mon Sep 17 00:00:00 2001 From: Ezequiel Canul Date: Fri, 2 Oct 2026 14:18:01 -0600 Subject: [PATCH 2/4] docs(openspec): add SDD scaffolding and skill-import change artifacts --- openspec/AGENTS.md | 119 +++ openspec/README.md | 46 + openspec/changes/skill-import/design.md | 999 ++++++++++++++++++++++ openspec/changes/skill-import/proposal.md | 257 ++++++ openspec/changes/skill-import/spec.md | 384 +++++++++ openspec/changes/skill-import/tasks.md | 275 ++++++ openspec/config.yaml | 52 ++ openspec/project.md | 153 ++++ 8 files changed, 2285 insertions(+) create mode 100644 openspec/AGENTS.md create mode 100644 openspec/README.md create mode 100644 openspec/changes/skill-import/design.md create mode 100644 openspec/changes/skill-import/proposal.md create mode 100644 openspec/changes/skill-import/spec.md create mode 100644 openspec/changes/skill-import/tasks.md create mode 100644 openspec/config.yaml create mode 100644 openspec/project.md diff --git a/openspec/AGENTS.md b/openspec/AGENTS.md new file mode 100644 index 0000000..6bed4d6 --- /dev/null +++ b/openspec/AGENTS.md @@ -0,0 +1,119 @@ +# openspec/AGENTS.md — Phase Agent Instructions + +Operating contract for SDD phase agents working on `skill-sync`. +Read `openspec/project.md` and the repo-root `AGENTS.md` before +producing any artifact. + +## Common Contract + +Every phase agent must return an envelope containing: + +```text +status +executive_summary +artifacts # paths produced or modified +next_recommended +risks +skill_resolution # paths-injected | fallback-registry | fallback-path | none +``` + +Hard rules: + +- Atomic writes only via `writer.AtomicWrite` for Go source under + `internal/` and `cmd/`; for `openspec/` artifacts a normal write is + acceptable but **never partial overwrites** mid-edit. +- Strict TDD evidence is mandatory in `apply` and `verify` phases. +- Do not delete or rewrite `openspec/specs/` from inside a change + folder; the `archive` phase performs the merge. +- Generated artifacts default to English regardless of conversation + language. +- Never commit, push, or open PRs without explicit user approval. + +## Phase Cheatsheet + +### `sdd-init` +Initialize project SDD context. Writes `openspec/config.yaml`, +`openspec/project.md`, `openspec/AGENTS.md`, `openspec/README.md`. +Detects test runner and records skill registry status. **Done.** + +### `sdd-explore` +Optional pre-proposal exploration. Maps relevant files, lists open +design questions. Saves notes under +`openspec/changes//explore.md`. + +### `sdd-proposal` +Writes `openspec/changes//proposal.md`: +- Goal, Why, Non-goals, Risks, Open Questions, Stakeholder Decisions. +- No code, no spec text — only intent. + +### `sdd-spec` +Writes `openspec/changes//spec.md` as a **delta spec**: +- Requirements with `REQ-` IDs. +- Scenarios in Given/When/Then. +- Explicit references to existing capabilities being modified. + +### `sdd-design` +Writes `openspec/changes//design.md`: +- Concrete technical approach. +- Module/package boundaries, types, signatures. +- Test strategy (table-driven cases, golden files, temp dirs). +- Migration notes if any. + +### `sdd-tasks` +Writes `openspec/changes//tasks.md`: +- Numbered, ordered tasks with acceptance criteria. +- Each task notes whether it produces tests, implementation, or both. +- **Forecasts changed lines**; if total > 400, pauses and surfaces a + chained-PR question to the parent orchestrator. + +### `sdd-apply` +Implements tasks in order with strict TDD evidence: + +```text +RED → failing test committed conceptually first +GREEN → minimum code to pass +TRIANGULATE → second test exposing a generalization +REFACTOR → code cleanup, tests still green +``` + +Records progress under +`openspec/changes//apply-progress.md`. + +### `sdd-verify` +Read-only verification: +- `go test ./...` passes. +- Spec requirements covered by tests. +- No drift between spec and implementation. +Writes `openspec/changes//verify-report.md`. Exits with +non-zero status on failure. + +### `sdd-archive` +Merges verified delta into `openspec/specs/`, moves change folder to +`openspec/changes/archived//`. Updates any cross-references. + +### `sdd-sync` (optional) +Syncs verified delta specs into canonical specs without archiving +the change (useful when multiple changes are in flight). + +## Skill Loading + +Parent orchestrator passes exact `SKILL.md` paths in the prompt under +`## Skills to load before work`. Phase agents must read those before +producing artifacts. If paths are missing, fall back to +`.atl/skill-registry.md` and report `skill_resolution: fallback-registry`. + +## Review Workload Guard + +`sdd-tasks` is responsible for forecasting changed lines. If the +forecast exceeds 400, do not proceed to `sdd-apply` silently — surface +the chained-PR decision to the parent and wait for approval. + +## Testing Discipline + +Project-specific (see `openspec/project.md` §4): + +- Table-driven tests are mandatory. +- Real `t.TempDir()` for integration tests; no FS mocks unless + unavoidable. +- TUI: `teatest` + golden files. +- Strict TDD: every commit-worthy unit has paired test evidence. diff --git a/openspec/README.md b/openspec/README.md new file mode 100644 index 0000000..589267a --- /dev/null +++ b/openspec/README.md @@ -0,0 +1,46 @@ +# openspec/ — SDD Artifacts for skill-sync + +This directory holds the source of truth for the **Spec-Driven +Development (SDD)** workflow used in `skill-sync`. + +## Layout + +```text +openspec/ +├── config.yaml # SDD/OpenSpec configuration (test runner, store, review budget) +├── project.md # Architectural context every phase agent reads first +├── AGENTS.md # Operating instructions for SDD phase agents +├── README.md # This file +├── specs/ # Canonical source specs (long-lived, per-capability) +└── changes/ # Active proposals/specs/designs/tasks (per-change folder) +``` + +## Lifecycle of a Change + +```text +init ─► explore ─► proposal ─► spec ─► design ─► tasks ─► apply ─► verify ─► archive +``` + +Each change lives under `openspec/changes//` and contains: + +- `proposal.md` +- `spec.md` +- `design.md` +- `tasks.md` +- (optional) `verify-report.md`, `apply-progress.md` + +When a change is archived, its delta is merged into `openspec/specs/` +and the change folder is moved to `openspec/changes/archived/`. + +## Conventions + +- Strict TDD: every implementation phase records RED/GREEN/TRIANGULATE + /REFACTOR evidence. +- Atomic writes only — phase agents must use `writer.AtomicWrite` + when modifying Go source files via the apply phase. +- Review budget: 400 changed lines per PR by default. Tasks phase + forecasts the diff and asks before exceeding the budget. +- Single PR per change by default; chained PRs only when explicitly + approved. + +See `openspec/AGENTS.md` for phase-by-phase operating instructions. diff --git a/openspec/changes/skill-import/design.md b/openspec/changes/skill-import/design.md new file mode 100644 index 0000000..395cc55 --- /dev/null +++ b/openspec/changes/skill-import/design.md @@ -0,0 +1,999 @@ +# Design: `skill-import` + +**Change ID:** skill-import +**Branch:** feat/import_skills +**Date:** 2026-06-05 +**Status:** APPROVED + +--- + +## 1. New File Layout + +``` +internal/ + importer/ + errors.go — ErrSkillNotFound, ErrConflict + manifest.go — DirHash + manifest_test.go + importer.go — SkillCandidate, Importer, New, FindSkill, Resolve, CopySkill + importer_test.go + tui/ + import_source_view/ + model.go + model_test.go + testdata/golden/ + initial.txt + one_source_unchecked.txt + all_unchecked.txt + import_skill_view/ + model.go + model_test.go + testdata/golden/ + initial.txt + skill_selected.txt + no_skills.txt + cli/ + import.go — NewImportCmd + import_test.go +``` + +Modified existing files: + +``` +internal/models/skill.go + Description field on SkillMetadata +internal/tui/menu/model.go + "Import" item +internal/tui/program.go + 2 states, 2 model fields, 2 callbacks, message handlers +internal/cli/root.go + NewImportCmd(), 2 new Callbacks fields +``` + +--- + +## 2. `internal/importer/errors.go` + +```go +package importer + +import "errors" + +var ErrSkillNotFound = errors.New("skill not found in any configured source") +var ErrConflict = errors.New("conflict: identical version and mtime in multiple sources") +``` + +Both are exported so `internal/cli` and `internal/tui` can use `errors.Is`. + +`ErrConfigNotFound` is **not** in this package. It is produced by +`internal/cli/import.go` using the existing `config_resolver` error path +(see §7). + +--- + +## 3. `internal/importer/manifest.go` + +```go +package importer + +import ( + "crypto/sha256" + "fmt" + "io/fs" + "os" + "path/filepath" + "sort" +) + +// DirHash computes a deterministic, cross-platform SHA-256 over all regular +// files in dir. Symlinks are skipped. +func DirHash(dir string) (string, error) { + type entry struct{ rel, absPath string } + var entries []entry + + err := filepath.WalkDir(dir, func(path string, d fs.DirEntry, err error) error { + if err != nil { + return err + } + if d.Type()&fs.ModeSymlink != 0 || d.IsDir() { + return nil + } + rel, err := filepath.Rel(dir, path) + if err != nil { + return err + } + entries = append(entries, entry{ + rel: filepath.ToSlash(rel), // cross-platform determinism + absPath: path, + }) + return nil + }) + if err != nil { + return "", fmt.Errorf("DirHash walk %s: %w", dir, err) + } + + sort.Slice(entries, func(i, j int) bool { + return entries[i].rel < entries[j].rel + }) + + h := sha256.New() + for _, e := range entries { + data, err := os.ReadFile(e.absPath) + if err != nil { + return "", fmt.Errorf("DirHash read %s: %w", e.rel, err) + } + h.Write([]byte(e.rel + "\n")) + h.Write(data) + } + return fmt.Sprintf("%x", h.Sum(nil)), nil +} +``` + +--- + +## 4. `internal/importer/importer.go` + +```go +package importer + +import ( + "errors" + "fmt" + "io/fs" + "os" + "path/filepath" + "sort" + "strings" + "time" + + "github.com/ezzek/skill-sync/internal/mesh" + "github.com/ezzek/skill-sync/internal/models" + "github.com/ezzek/skill-sync/internal/writer" + "gopkg.in/yaml.v3" +) + +// SkillCandidate is a skill directory found during source discovery. +type SkillCandidate struct { + SkillID string // directory name, e.g. "git-expert" + SourceDir string // absolute path to the skill directory + AgentDir string // absolute path to the parent target + Metadata models.SkillMetadata // parsed from SKILL.md frontmatter + Mtime time.Time // mtime of SKILL.md + Hash string // DirHash of SourceDir + IsConflict bool // true if multiple sources compete in a tie conflict +} + +// Importer drives import operations for a fixed set of source targets +// and a fixed destination root. +type Importer struct { + Targets []string // tilde-expanded absolute paths + DestRoot string // absolute path to ./.agents/skills/ +} + +// New returns an Importer. +// targets must be tilde-expanded absolute paths. +// destRoot is typically filepath.Join(cwd, ".agents", "skills"). +func New(targets []string, destRoot string) *Importer { + return &Importer{Targets: targets, DestRoot: destRoot} +} + +// FindSkill walks each target's skills/ subdirectory and returns all +// candidates whose directory name matches skillID (case-sensitive). +// Returns an empty slice (not an error) when no match is found. +func (imp *Importer) FindSkill(skillID string) ([]SkillCandidate, error) { + var candidates []SkillCandidate + for _, target := range imp.Targets { + skillsDir := filepath.Join(target, "skills") + skillDir := filepath.Join(skillsDir, skillID) + mdPath := filepath.Join(skillDir, "SKILL.md") + + info, err := os.Stat(mdPath) + if errors.Is(err, fs.ErrNotExist) { + continue + } + if err != nil { + return nil, fmt.Errorf("stat %s: %w", mdPath, err) + } + + meta, err := readMetadata(mdPath) + if err != nil { + meta = models.SkillMetadata{} // tolerate parse errors + } + + hash, err := DirHash(skillDir) + if err != nil { + return nil, fmt.Errorf("hash %s: %w", skillDir, err) + } + + candidates = append(candidates, SkillCandidate{ + SkillID: skillID, + SourceDir: skillDir, + AgentDir: target, + Metadata: meta, + Mtime: info.ModTime(), + Hash: hash, + }) + } + return candidates, nil +} + +// FindAllSkills walks each target's skills/ subdirectory, groups candidates +// by SkillID, and resolves the winner for each unique skill ID. If a skill has +// a true tie conflict, it returns a candidate with IsConflict set to true. +func (imp *Importer) FindAllSkills() ([]SkillCandidate, error) { + candidatesByDir := make(map[string][]SkillCandidate) + for _, target := range imp.Targets { + skillsDir := filepath.Join(target, "skills") + entries, err := os.ReadDir(skillsDir) + if errors.Is(err, fs.ErrNotExist) { + continue + } + if err != nil { + return nil, fmt.Errorf("read %s: %w", skillsDir, err) + } + for _, e := range entries { + if !e.IsDir() { + continue + } + skillID := e.Name() + skillDir := filepath.Join(skillsDir, skillID) + mdPath := filepath.Join(skillDir, "SKILL.md") + info, err := os.Stat(mdPath) + if errors.Is(err, fs.ErrNotExist) { + continue + } + if err != nil { + continue + } + + meta, _ := readMetadata(mdPath) + hash, err := DirHash(skillDir) + if err != nil { + return nil, fmt.Errorf("hash %s: %w", skillDir, err) + } + c := SkillCandidate{ + SkillID: skillID, + SourceDir: skillDir, + AgentDir: target, + Metadata: meta, + Mtime: info.ModTime(), + Hash: hash, + } + candidatesByDir[skillID] = append(candidatesByDir[skillID], c) + } + } + + var results []SkillCandidate + for _, group := range candidatesByDir { + winner, conflict, err := Resolve(group) + if conflict { + c := group[0] + c.IsConflict = true + results = append(results, c) + } else if err == nil && winner != nil { + results = append(results, *winner) + } + } + + // Sort results by SkillID so list is deterministic. + sort.Slice(results, func(i, j int) bool { + return results[i].SkillID < results[j].SkillID + }) + + return results, nil +} + +// Resolve converts candidates to SkillInstances and delegates winner +// selection to mesh.NewDefaultResolver(). Returns the winning candidate, +// a conflict flag, and any error. +func Resolve(candidates []SkillCandidate) (*SkillCandidate, bool, error) { + if len(candidates) == 0 { + return nil, false, ErrSkillNotFound + } + if len(candidates) == 1 { + c := candidates[0] + return &c, false, nil + } + + instances := make([]models.SkillInstance, len(candidates)) + for i, c := range candidates { + instances[i] = models.SkillInstance{ + ID: c.SkillID, + Path: filepath.Join(c.SourceDir, "SKILL.md"), + TargetDir: c.AgentDir, + Hash: c.Hash, + Mtime: c.Mtime, + Metadata: c.Metadata, + } + } + + winner, conflict, err := mesh.NewDefaultResolver().Resolve(instances) + if err != nil { + return nil, false, err + } + if conflict { + return nil, true, ErrConflict + } + + // Re-map winner back to its SkillCandidate by matching AgentDir. + for i := range candidates { + if candidates[i].AgentDir == winner.TargetDir { + c := candidates[i] + return &c, false, nil + } + } + return nil, false, fmt.Errorf("importer: resolve winner not found in candidates (internal error)") +} + +// CopySkill copies the full skill directory from src into +// filepath.Join(destRoot, skillID) using the transactional strategy: +// 1. Write all files to .tmp/ +// 2. On failure: cleanup .tmp, return error (existing dir untouched). +// 3. On success: remove stale .bak, rename existing → .bak, rename .tmp → final. +func (imp *Importer) CopySkill(skillID, src string) error { + if err := os.MkdirAll(imp.DestRoot, 0o755); err != nil { + return fmt.Errorf("create dest root: %w", err) + } + + destFinal := filepath.Join(imp.DestRoot, skillID) + destTmp := filepath.Join(imp.DestRoot, skillID+".tmp") + destBak := filepath.Join(imp.DestRoot, skillID+".bak") + + // Ensure tmp is clean before starting. + _ = os.RemoveAll(destTmp) + + // Copy all files into .tmp/ + copyErr := filepath.WalkDir(src, func(path string, d fs.DirEntry, err error) error { + if err != nil { + return err + } + if d.Type()&fs.ModeSymlink != 0 { + return nil // skip symlinks silently + } + rel, err := filepath.Rel(src, path) + if err != nil { + return err + } + tmpDest := filepath.Join(destTmp, rel) + if d.IsDir() { + return os.MkdirAll(tmpDest, 0o755) + } + data, err := os.ReadFile(path) + if err != nil { + return fmt.Errorf("read %s: %w", rel, err) + } + if err := os.MkdirAll(filepath.Dir(tmpDest), 0o755); err != nil { + return err + } + return writer.AtomicWrite(tmpDest, data) + }) + if copyErr != nil { + _ = os.RemoveAll(destTmp) // cleanup on failure + return fmt.Errorf("copy skill %s: %w", skillID, copyErr) + } + + // Commit: rotate backup, rename tmp → final. + _ = os.RemoveAll(destBak) + if _, err := os.Stat(destFinal); err == nil { + if err := os.Rename(destFinal, destBak); err != nil { + _ = os.RemoveAll(destTmp) + return fmt.Errorf("backup existing skill: %w", err) + } + } + if err := os.Rename(destTmp, destFinal); err != nil { + // Attempt to restore backup. + if _, statErr := os.Stat(destBak); statErr == nil { + _ = os.Rename(destBak, destFinal) + } + return fmt.Errorf("commit skill %s: %w", skillID, err) + } + return nil +} + +// readMetadata parses the YAML frontmatter from a SKILL.md file. +// Returns zero-value SkillMetadata on any parse error. +func readMetadata(mdPath string) (models.SkillMetadata, error) { + data, err := os.ReadFile(mdPath) + if err != nil { + return models.SkillMetadata{}, err + } + s := string(data) + // Check for opening fence with Unix or Windows newline. + if !strings.HasPrefix(s, "---\n") && !strings.HasPrefix(s, "---\r\n") { + return models.SkillMetadata{}, nil + } + // Find the end of the frontmatter. + endIndex := strings.Index(s[3:], "\n---") + if endIndex == -1 { + return models.SkillMetadata{}, nil + } + yamlStr := s[3 : endIndex+3] + var meta models.SkillMetadata + _ = yaml.Unmarshal([]byte(yamlStr), &meta) + return meta, nil +} +``` + +> **Note:** `FindAllSkills` is added to support the TUI flow (Screen B needs +> unique resolved skills across the filtered sources, or conflicts marked). +> `FindSkill` (single skillID) is kept for the CLI path. + +--- + +## 5. `internal/models/skill.go` — change + +Add `Description` to `SkillMetadata`: + +```go +type SkillMetadata struct { + Version float64 `yaml:"version"` + Name string `yaml:"name"` + Description string `yaml:"description"` // NEW +} +``` + +No other changes to `models`. + +--- + +## 6. `internal/tui/import_source_view/model.go` + +```go +package import_source_view + +import ( + "path/filepath" + "strings" + + tea "github.com/charmbracelet/bubbletea" + "github.com/ezzek/skill-sync/internal/tui/shared_toggle" + "github.com/ezzek/skill-sync/internal/tui/styles" +) + +// ConfirmedMsg carries the subset of source directories the user kept checked. +type ConfirmedMsg struct{ Sources []string } + +// BackMsg signals the root model to return to the main menu. +type BackMsg struct{} + +// Model wraps shared_toggle for agent-source multi-selection. +type Model struct { + toggle shared_toggle.Model + sources []string // parallel to toggle items; index → full target path +} + +// New builds the model from a list of resolved target directories. +// All sources start checked. +func New(targets []string) Model { + items := make([]shared_toggle.Item, len(targets)) + for i, t := range targets { + items[i] = shared_toggle.Item{ + Label: filepath.Base(t) + " (" + t + ")", + Value: t, + Selected: true, + } + } + return Model{ + toggle: shared_toggle.New(items), + sources: targets, + } +} + +func (m Model) Init() tea.Cmd { return m.toggle.Init() } + +func (m Model) Update(msg tea.Msg) (tea.Model, tea.Cmd) { + switch msg := msg.(type) { + case tea.KeyMsg: + switch msg.String() { + case "enter", "ctrl+s": + var checked []string + for _, item := range m.toggle.GetSelected() { + checked = append(checked, item.Value) + } + return m, func() tea.Msg { return ConfirmedMsg{Sources: checked} } + case "q", "esc": + return m, func() tea.Msg { return BackMsg{} } + case "ctrl+c": + return m, tea.Quit + case "a": + m.toggle = m.toggle.SelectAll() + return m, nil + case "A": + m.toggle = m.toggle.DeselectAll() + return m, nil + } + } + var cmd tea.Cmd + var updated tea.Model + updated, cmd = m.toggle.Update(msg) + m.toggle = updated.(shared_toggle.Model) + return m, cmd +} + +func (m Model) View() string { + var s strings.Builder + s.WriteString(styles.TitleStyle.Render("Import — Select Sources")) + s.WriteString("\n\nChoose which agent directories to search:\n") + s.WriteString(m.toggle.View()) + s.WriteString(styles.HelpStyle.Render( + "\n[space/enter] toggle • [a] all • [A] none • [enter/ctrl+s] next • [q/esc] back", + )) + return s.String() +} +``` + +--- + +## 7. `internal/tui/import_skill_view/model.go` + +Single-selection cursor-based list. Does **not** use `shared_toggle` (no +checkboxes needed; radio-select semantics). + +```go +package import_skill_view + +import ( + "fmt" + "strings" + + tea "github.com/charmbracelet/bubbletea" + "github.com/ezzek/skill-sync/internal/importer" + "github.com/ezzek/skill-sync/internal/tui/styles" +) + +// ConfirmedMsg carries the skill the user selected for import. +type ConfirmedMsg struct{ Candidate importer.SkillCandidate } + +// BackMsg signals the root model to return to import_source_view. +type BackMsg struct{} + +const maxVisible = 15 +const descMaxLen = 60 + +type Model struct { + candidates []importer.SkillCandidate + cursor int + scroll int +} + +// New builds the model from a list of candidates. +// Callers must ensure len(candidates) > 0; use output_view for the empty case. +func New(candidates []importer.SkillCandidate) Model { + return Model{candidates: candidates} +} + +func (m Model) Init() tea.Cmd { return nil } + +func (m Model) Update(msg tea.Msg) (tea.Model, tea.Cmd) { + switch msg := msg.(type) { + case tea.KeyMsg: + switch msg.String() { + case "up", "k": + if m.cursor > 0 { + m.cursor-- + if m.cursor < m.scroll { + m.scroll = m.cursor + } + } + case "down", "j": + if m.cursor < len(m.candidates)-1 { + m.cursor++ + if m.cursor >= m.scroll+maxVisible { + m.scroll = m.cursor - maxVisible + 1 + } + } + case "enter": + c := m.candidates[m.cursor] + return m, func() tea.Msg { return ConfirmedMsg{Candidate: c} } + case "esc": + return m, func() tea.Msg { return BackMsg{} } + case "ctrl+c": + return m, tea.Quit + } + } + return m, nil +} + +func (m Model) View() string { + var s strings.Builder + s.WriteString(styles.TitleStyle.Render("Import — Select Skill")) + s.WriteString(fmt.Sprintf("\n\n%d skill(s) found — choose one to import:\n", len(m.candidates))) + + n := len(m.candidates) + end := m.scroll + maxVisible + if end > n { + end = n + } + + if m.scroll > 0 { + s.WriteString(styles.SubtleStyle.Render(fmt.Sprintf(" ↑ %d more above", m.scroll)) + "\n") + } + for i := m.scroll; i < end; i++ { + c := m.candidates[i] + cursor := " " + if m.cursor == i { + cursor = "→ " + } + name := c.Metadata.Name + if name == "" { + name = c.SkillID + } + desc := c.Metadata.Description + if c.IsConflict { + desc = "[CONFLICT] " + desc + } + if len([]rune(desc)) > descMaxLen { + desc = string([]rune(desc)[:descMaxLen]) + "…" + } + line := fmt.Sprintf("%s%-24s %-28s %s", cursor, c.SkillID, name, desc) + if m.cursor == i { + s.WriteString(styles.ListItemSelectedStyle.Render(line) + "\n") + } else { + s.WriteString(styles.ListItemStyle.Render(line) + "\n") + } + } + if end < n { + s.WriteString(styles.SubtleStyle.Render(fmt.Sprintf(" ↓ %d more below", n-end)) + "\n") + } + + s.WriteString(styles.HelpStyle.Render("\n[↑/↓] navigate • [enter] import • [esc] back")) + return s.String() +} +``` + +--- + +## 8. `internal/tui/program.go` — changes + +### 8.1 New imports + +```go +"github.com/ezzek/skill-sync/internal/importer" +"github.com/ezzek/skill-sync/internal/tui/import_source_view" +"github.com/ezzek/skill-sync/internal/tui/import_skill_view" +``` + +### 8.2 State constants (add after `stateOutput`) + +```go +stateImportSource +stateImportSkill +``` + +### 8.3 Callbacks struct (add two fields) + +```go +type Callbacks struct { + ScanSkills func() ([]models.SkillSyncInfo, error) + RunSync func(skillFilter []string) (string, error) + RunVerify func() (string, error) + FindSkillsForImport func(sources []string) ([]importer.SkillCandidate, error) // NEW + RunImport func(candidate importer.SkillCandidate) (string, error) // NEW +} +``` + +### 8.4 RootModel struct (add two model fields) + +```go +type RootModel struct { + // ... existing fields ... + importSourceModel tea.Model // NEW + importSkillModel tea.Model // NEW + callbacks Callbacks +} +``` + +### 8.5 `Update` — new menu case + +```go +case "Import": + targets, err := loadImportTargets() + if err != nil { + m.outModel = output_view.New("Error loading config: " + err.Error()) + m.state = stateOutput + return m, m.outModel.Init() + } + m.importSourceModel = import_source_view.New(targets) + m.state = stateImportSource + return m, m.importSourceModel.Init() +``` + +### 8.6 `Update` — new message handlers + +```go +case import_source_view.ConfirmedMsg: + candidates, err := m.callbacks.FindSkillsForImport(msg.Sources) + if err != nil { + m.outModel = output_view.New("Error scanning skills: " + err.Error()) + m.state = stateOutput + return m, m.outModel.Init() + } + if len(candidates) == 0 { + m.outModel = output_view.New("No skills found in the selected sources.") + m.state = stateOutput + return m, m.outModel.Init() + } + m.importSkillModel = import_skill_view.New(candidates) + m.state = stateImportSkill + return m, m.importSkillModel.Init() + +case import_source_view.BackMsg: + m.state = stateMenu + return m, m.menuModel.Init() + +case import_skill_view.ConfirmedMsg: + out, err := m.callbacks.RunImport(msg.Candidate) + if err != nil { + out = "Import error: " + err.Error() + } + m.outModel = output_view.New(out) + m.state = stateOutput + return m, m.outModel.Init() + +case import_skill_view.BackMsg: + m.state = stateImportSource + return m, m.importSourceModel.Init() +``` + +### 8.7 `Update` — dispatch existing state machine (add two cases) + +```go +case stateImportSource: + var updated tea.Model + updated, cmd = m.importSourceModel.Update(msg) + if updated != nil { + m.importSourceModel = updated + } +case stateImportSkill: + var updated tea.Model + updated, cmd = m.importSkillModel.Update(msg) + if updated != nil { + m.importSkillModel = updated + } +``` + +### 8.8 `View` — add two cases + +```go +case stateImportSource: + return m.importSourceModel.View() +case stateImportSkill: + return m.importSkillModel.View() +``` + +### 8.9 New helper function `loadImportTargets` + +```go +// loadImportTargets resolves skill-sync.yaml and returns the expanded target list. +func loadImportTargets() ([]string, error) { + cfgPath, err := config_resolver.New().Resolve("") + if err != nil { + return nil, fmt.Errorf("no skill-sync.yaml found; run 'skill-sync init' to create one") + } + f, err := os.Open(cfgPath) + if err != nil { + return nil, err + } + defer f.Close() + cfg, err := agent.ParseConfig(f) + if err != nil { + return nil, err + } + if home, err := os.UserHomeDir(); err == nil { + cfg.ExpandTargets(home) + } + return cfg.Targets, nil +} +``` + +--- + +## 9. `internal/tui/menu/model.go` — change + +Add `"Import"` item between `"Verify"` and `"Config"`: + +```go +items := []list.Item{ + item{title: "Init", desc: "Initialize configuration and discover skills"}, + item{title: "Sync", desc: "Synchronize skills across directories"}, + item{title: "Verify", desc: "Check for drift without syncing"}, + item{title: "Import", desc: "Import a skill from a configured agent source"}, // NEW + item{title: "Config", desc: "Configure sync targets"}, + item{title: "Quit", desc: "Exit the application"}, +} +``` + +--- + +## 10. `internal/cli/import.go` + +```go +package cli + +import ( + "errors" + "fmt" + "os" + "path/filepath" + + "github.com/ezzek/skill-sync/internal/agent" + "github.com/ezzek/skill-sync/internal/agent/config_resolver" + "github.com/ezzek/skill-sync/internal/importer" + "github.com/spf13/cobra" +) + +func NewImportCmd() *cobra.Command { + var configPath string + + cmd := &cobra.Command{ + Use: "import ", + Short: "Import a skill from a configured agent source into ./.agents/skills/", + Args: cobra.ExactArgs(1), + SilenceUsage: true, + RunE: func(cmd *cobra.Command, args []string) error { + return runImport(cmd, configPath, args[0]) + }, + } + cmd.Flags().StringVarP(&configPath, "config", "c", "", "Path to skill-sync.yaml") + return cmd +} + +func runImport(cmd *cobra.Command, configPath, skillID string) error { + // 1. Resolve config. + resolvedPath, err := config_resolver.New().Resolve(configPath) + if err != nil { + return fmt.Errorf("no skill-sync.yaml found; run 'skill-sync init' to create one") + } + f, err := os.Open(resolvedPath) + if err != nil { + return fmt.Errorf("open config: %w", err) + } + defer f.Close() + + cfg, err := agent.ParseConfig(f) + if err != nil { + return fmt.Errorf("parse config: %w", err) + } + if home, err := os.UserHomeDir(); err == nil { + cfg.ExpandTargets(home) + } + + // 2. Build importer. + cwd, err := os.Getwd() + if err != nil { + return fmt.Errorf("getwd: %w", err) + } + destRoot := filepath.Join(cwd, ".agents", "skills") + imp := importer.New(cfg.Targets, destRoot) + + // 3. Find candidates. + candidates, err := imp.FindSkill(skillID) + if err != nil { + return err + } + if len(candidates) == 0 { + return fmt.Errorf("skill %q not found in any configured source", skillID) + } + + // 4. Resolve winner. + winner, _, err := importer.Resolve(candidates) + if err != nil { + if errors.Is(err, importer.ErrConflict) { + return fmt.Errorf("conflict: %q has identical version and mtime in multiple sources; resolve manually", skillID) + } + return err + } + + // 5. Copy. + if err := imp.CopySkill(skillID, winner.SourceDir); err != nil { + return err + } + + fmt.Fprintf(cmd.OutOrStdout(), "Imported %s from %s\n", skillID, winner.SourceDir) + return nil +} +``` + +--- + +## 11. `internal/cli/root.go` — changes + +### 11.1 Add `NewImportCmd()` to the root command + +```go +cmd.AddCommand(NewImportCmd()) +``` + +### 11.2 Wire import callbacks in the default `RunE` + +```go +callbacks := tui.Callbacks{ + // ... existing fields ... + + FindSkillsForImport: func(sources []string) ([]importer.SkillCandidate, error) { + cwd, err := os.Getwd() + if err != nil { + return nil, err + } + destRoot := filepath.Join(cwd, ".agents", "skills") + imp := importer.New(sources, destRoot) + return imp.FindAllSkills() + }, + + RunImport: func(candidate importer.SkillCandidate) (string, error) { + if candidate.IsConflict { + return "", importer.ErrConflict + } + cwd, err := os.Getwd() + if err != nil { + return "", err + } + destRoot := filepath.Join(cwd, ".agents", "skills") + imp := importer.New(nil, destRoot) // sources not needed for CopySkill + if err := imp.CopySkill(candidate.SkillID, candidate.SourceDir); err != nil { + return "", err + } + return fmt.Sprintf("Imported %s from %s", candidate.SkillID, candidate.SourceDir), nil + }, +} +``` + +New imports needed in `root.go`: + +```go +"os" +"path/filepath" +"fmt" +"github.com/ezzek/skill-sync/internal/importer" +``` + +--- + +## 12. Dependency Graph + +``` +internal/importer + ↑ + ├── internal/mesh (Resolve) + ├── internal/models (SkillMetadata, SkillInstance) + ├── internal/writer (AtomicWrite) + └── gopkg.in/yaml.v3 + +internal/tui/import_source_view + ↑ + └── internal/tui/shared_toggle + +internal/tui/import_skill_view + ↑ + └── internal/importer + +internal/tui/program.go + ↑ + ├── internal/importer + ├── internal/tui/import_source_view + └── internal/tui/import_skill_view + +internal/cli/import.go + ↑ + ├── internal/importer + ├── internal/agent + └── internal/agent/config_resolver + +internal/cli/root.go + ↑ + └── internal/importer (for callback signatures) +``` + +No circular dependencies. `internal/importer` does not import any `tui` or +`cli` package. + +--- + +## 13. Implementation Order + +1. `internal/models/skill.go` — add `Description` field + test. +2. `internal/importer/errors.go` — sentinel errors. +3. `internal/importer/manifest.go` + `manifest_test.go`. +4. `internal/importer/importer.go` + `importer_test.go` (TDD: RED → GREEN). +5. `internal/tui/import_source_view/model.go` + golden tests. +6. `internal/tui/import_skill_view/model.go` + golden tests. +7. `internal/tui/menu/model.go` — add Import item. +8. `internal/tui/program.go` — wire new states. +9. `internal/cli/import.go` + `import_test.go`. +10. `internal/cli/root.go` — add `NewImportCmd()` + import callbacks. diff --git a/openspec/changes/skill-import/proposal.md b/openspec/changes/skill-import/proposal.md new file mode 100644 index 0000000..cfc4bc4 --- /dev/null +++ b/openspec/changes/skill-import/proposal.md @@ -0,0 +1,257 @@ +# Proposal: `skill-import` + +**Change ID:** skill-import +**Branch:** feat/import_skills +**Date:** 2026-05-26 +**Status:** DRAFT + +--- + +## Intent + +Add a one-shot `import` flow — available as both a CLI sub-command and a +new TUI screen sequence — that lets a user selectively copy ONE skill +(its full directory tree) from any globally configured agent source into +the local project at `./.agents/skills//`. + +This is a *snapshot* operation, not mesh synchronisation. Imported skills +are invisible to `sync` and `verify`. + +> **Reusable primitive note:** this change introduces +> `internal/importer/` — a new package that implements correct +> full-directory copy (temp-dir → rename, per-file atomic writes, +> `.bak` backup). That primitive will later be reused by the mesh-sync +> directory-copy bug fix tracked in `context/pending-fixes.md`. +> The mesh fix is **out of scope here**. + +--- + +## Motivation + +The existing mesh sync propagates skills between peer agent directories +that are all configured in `skill-sync.yaml`. There is no mechanism to +pull a single skill from the global pool into a project's own skill +directory. Users currently copy skill directories manually, which is +error-prone and bypasses the version / conflict logic already baked into +`internal/mesh/resolver.go`. + +--- + +## Scope + +### In scope (this change) + +- `skill-sync import ` CLI sub-command (exactly one skill + per invocation, no variadic args, no `--all`, no `--target` flag). +- TUI: two new screens wired into `internal/tui/program.go`: + - **Import-source screen** — lists detected agents from `targets:` with + per-row toggle to exclude any. Toggles are session-only (not + persisted). + - **Skill-select screen** — lists skills found across the included + sources; each row shows `name` and `description` from YAML + frontmatter (fallback: directory name / empty string). + - User picks one → import runs → existing `output_view`. +- New `internal/importer/` package: + - `Importer` type that holds the target list + destination root. + - `CopySkill(skillID, sourceDir, destRoot string) error` — directory + copy primitive (temp + fsync + rename per-file; see Backup Strategy + below). + - `FindSkill(name string, targets []string) ([]SkillCandidate, error)` + — locates all matching skill directories across sources. + - `Resolve(candidates []SkillCandidate) (*SkillCandidate, bool, error)` + — thin wrapper that delegates to `mesh.Resolver` winner rules. +- Destination: `./.agents/skills//` relative to `cwd`. + Created with `os.MkdirAll` when absent. +- Source resolution via existing `internal/agent/config_resolver` + + `internal/agent/registry` (tilde expansion included). Missing config + → user-facing error suggesting `skill-sync init`. +- Conflict when the same `skillID` exists in multiple sources: + - CLI mode: auto-pick winner (idempotent, scriptable). + - TUI mode: present conflict to user as a tie-break choice. +- Strict TDD for all new packages (`t.TempDir()`, table-driven, + `teatest` + golden files for TUI screens). + +### Out of scope (explicitly deferred) + +- Multi-destination (`.claude/skills`, `.cursor/skills`, etc.) +- `--all` flag or variadic skill arguments +- `--target` flag +- Persisting the agent-filter selection to `skill-sync.yaml` +- Removing / unimporting skills +- Dry-run mode +- Mesh-sync directory-copy bug fix (tracked in `context/pending-fixes.md`) +- Any change to `sync`, `verify`, or their tests + +--- + +## Approach + +### Source Discovery + +Re-use `internal/agent/config_resolver.New().Resolve("")` to locate +`skill-sync.yaml`. Parse it with `agent.ParseConfig` and call +`cfg.ExpandTargets(home)` to get the resolved list of target parent +directories. Each target's `skills/` subdirectory is then walked for +skill directories (any directory containing a `SKILL.md`). + +### Conflict Resolution + +`internal/importer.Resolve` converts each candidate into a +`models.SkillInstance` (by reading `SKILL.md` frontmatter for version, +using `os.Stat` for mtime, and computing a SHA-256 of the +directory manifest — sorted relative paths + per-file hashes) and +delegates to `mesh.NewDefaultResolver().Resolve(instances)`. + +> Hash computation uses the *directory manifest* rather than a single +> file, because the mesh scanner's `SKILL.md`-only hash is the known +> pending bug. This keeps the import correct without touching the +> existing resolver logic. + +### Backup Strategy + +When `./.agents/skills//` already exists: + +1. Remove any stale `.bak` directory: `os.RemoveAll(.bak/)`. +2. Rename existing directory to `.bak/`. +3. Copy new skill files one-by-one via `writer.AtomicWrite` (temp + + fsync + rename) into a fresh `/` directory. + +Step 2 is a single `os.Rename` — on POSIX this is atomic at the +directory level. Step 3 uses the existing `writer` contract per file. +If any file write in step 3 fails, the backup at step 2 preserves the +previous state and the caller returns an error. + +### New Package Layout + +``` +internal/ + importer/ + importer.go — Importer type, FindSkill, CopySkill, Resolve + manifest.go — directory manifest + SHA-256 helper + importer_test.go — table-driven unit + integration tests + manifest_test.go +internal/ + tui/ + import_source_view/ + model.go + model_test.go + testdata/golden/ — teatest golden snapshots + import_skill_view/ + model.go + model_test.go + testdata/golden/ +internal/ + cli/ + import.go — `skill-sync import ` Cobra command + import_test.go +``` + +`cmd/skill-sync/main.go` and `internal/cli/root.go` gain one +`cmd.AddCommand(NewImportCmd())` call. + +`internal/tui/program.go` gains two new `state` constants +(`stateImportSource`, `stateImportSkill`), message types for each view +transition, and the corresponding `Update`/`View` branches. + +--- + +## Affected Areas + +| Area | Change | +|---|---| +| `internal/importer/` | **New package** — core import logic | +| `internal/cli/import.go` | **New file** — Cobra command | +| `internal/cli/root.go` | +1 `AddCommand` call | +| `internal/tui/program.go` | +2 states, +2 message types, update/view branches | +| `internal/tui/import_source_view/` | **New view** | +| `internal/tui/import_skill_view/` | **New view** | +| `internal/tui/menu/` | +1 menu item ("Import") | +| `cmd/skill-sync/main.go` | No change expected (root already delegates to `cli`) | +| `internal/mesh/` | No change | +| `internal/sync/` | No change | +| `internal/writer/` | No change — reused as-is | +| `verify` command | No change — imported skills are not tracked | + +--- + +## Risks + +| Risk | Likelihood | Impact | Mitigation | +|---|---|---|---| +| Directory rename in backup step not atomic on all OSes (e.g. cross-device) | Low | Medium | `os.Rename` is in-place on POSIX; document Windows caveat, add `t.Skip` for cross-device tests | +| `os.RemoveAll` on `.bak/` deletes the previous backup permanently | Low | Low | Documented limitation; future `--keep-bak` flag is deferred | +| Conflict auto-resolution in CLI mode overwrites a manually-edited local skill | Medium | Medium | Backup-before-overwrite (step 2) provides one-generation recovery | +| Walking `skills/` across all targets is slow if targets are remote/mounted filesystems | Low | Low | v1 does not address network targets; same limitation applies to existing `sync` | +| New TUI states increase `program.go` complexity | Medium | Low | Extracted into dedicated view packages; `program.go` changes are mechanical state-machine additions | + +--- + +## Rollback Plan + +1. `git revert` the commit(s) in `feat/import_skills` — no data + migration needed. +2. The `./.agents/skills/` directory is project-local and not tracked + by `skill-sync.yaml`, so reverting the binary does not break + existing mesh-sync targets. +3. Any skill previously imported remains on disk in + `./.agents/skills/` (benign orphan); `.bak/` directories may also + remain and can be deleted manually. + +--- + +## Dependencies + +- `github.com/charmbracelet/bubbletea` — already in `go.mod` +- `github.com/charmbracelet/bubbles` — already in `go.mod` +- `github.com/charmbracelet/x/exp/teatest` — already used in TUI tests +- `gopkg.in/yaml.v3` — already in `go.mod` +- `github.com/spf13/cobra` — already in `go.mod` +- `internal/mesh.NewDefaultResolver` — consumed as-is, no modification +- `internal/writer.AtomicWrite` — consumed as-is, no modification + +No new external dependencies are anticipated. + +--- + +## Success Criteria + +1. `skill-sync import ` copies the full skill directory from + the winning source into `./.agents/skills//` and exits `0`. +2. Running the same command twice is idempotent (same content → no error, + no change to the directory). +3. When a skill already exists locally it is overwritten and a `.bak/` + backup of the previous state is created. +4. When a skill ID exists in multiple sources, the CLI silently picks the + winner according to the existing resolver rules; the TUI prompts the + user to choose. +5. When `skill-sync.yaml` is absent, the command exits with a clear error + message suggesting `skill-sync init`. +6. `skill-sync verify` and `skill-sync sync` are unaffected and continue + to pass all existing tests after this change lands. +7. `go test ./...` passes with zero failures on Linux; Windows-only + failure paths are guarded with `t.Skip`. +8. All new packages have table-driven tests; all new TUI views have + `teatest` golden file coverage. + +--- + +## Open Questions + +None. All design decisions were closed with the user prior to this +proposal (see Closed Design Context in the session prompt). + +--- + +## Stakeholder Decisions (summary) + +| # | Decision | +|---|---| +| 1 | Source: `targets:` from `skill-sync.yaml` via existing config resolver | +| 2 | Unit: full skill directory, not only `SKILL.md` | +| 3 | Destination v1: `./.agents/skills//` only | +| 4 | Imported skills do NOT participate in mesh sync or `verify` | +| 5 | Conflict in CLI → auto-pick winner; in TUI → interactive choice | +| 6 | TUI: agent-filter screen → skill-list screen → output_view | +| 7 | CLI: `skill-sync import ` — one skill, no `--all` | +| 8 | Overwrite with `.bak/` backup of previous skill directory | +| 9 | Multi-destination, `--all`, dry-run, mesh-fix — all deferred | diff --git a/openspec/changes/skill-import/spec.md b/openspec/changes/skill-import/spec.md new file mode 100644 index 0000000..4ca54c9 --- /dev/null +++ b/openspec/changes/skill-import/spec.md @@ -0,0 +1,384 @@ +# Spec: `skill-import` + +**Change ID:** skill-import +**Branch:** feat/import_skills +**Date:** 2026-06-05 +**Status:** APPROVED + +--- + +## 1. Overview + +`skill-import` adds a `skill-sync import ` CLI sub-command and two +new TUI screens that allow a user to copy one skill directory from any +globally-configured agent source into the local project at +`./.agents/skills//`. + +The operation is a **one-shot snapshot**. Imported skills are not tracked by +`sync` or `verify`. The existing mesh pipeline is unchanged. + +--- + +## 2. Functional Requirements + +### FR-1 Source discovery + +- Sources are the `targets:` list in `skill-sync.yaml`, resolved via + `internal/agent/config_resolver` and `internal/agent/registry` (tilde + expansion included). +- Each target's `skills/` subdirectory is walked; any directory that directly + contains a `SKILL.md` file is treated as a skill. +- If `skill-sync.yaml` is absent, the command exits with a descriptive error: + `"no skill-sync.yaml found; run 'skill-sync init' to create one"`. + +### FR-2 CLI sub-command + +``` +skill-sync import +``` + +- Exactly **one positional argument** (the skill ID / directory name). +- No `--all`, no `--target`, no `--dry-run` (all deferred to v2). +- Config file flag `-c / --config` inherited from root command. +- Exit codes: + - `0` — success (skill copied or already up-to-date). + - `1` — any error (not found, config missing, copy failure). +- Stdout: single-line confirmation `"Imported from "`. +- Stderr: errors only. + +### FR-3 Destination + +- Always `./.agents/skills//` relative to the process working + directory. +- Created with `os.MkdirAll` when absent. +- Destination root `./.agents/skills/` is **never** registered in + `skill-sync.yaml` automatically. The user controls their config. + +### FR-4 Conflict resolution + +| Situation | CLI behaviour | TUI behaviour | +|---|---|---| +| Skill found in one source | Import directly | Import directly | +| Skill found in multiple sources, clear winner (version/mtime rules) | Auto-pick winner, silent | Auto-pick winner, show source in output_view | +| Skill found in multiple sources, true tie (same version + mtime + different hash) | Exit 1, message: `"conflict: has identical version and mtime in multiple sources; resolve manually"` | Show tie-break choice screen (future — deferred; exit 1 for v1) | +| Skill not found in any source | Exit 1, message: `"skill '' not found in any configured source"` | Show error in output_view | + +> **v1 decision:** True tie resolution in TUI is deferred. Both CLI and TUI +> exit with an error on a true tie. A future task will add the interactive +> tie-break choice. + +### FR-5 Idempotency + +Running `import ` twice with an unchanged source is a no-op: the +existing directory is overwritten but the result is byte-identical. Exit `0`. + +### FR-6 Backup on overwrite / Transactional Copy + +To guarantee that the destination directory is never left in a partially-written or corrupt state if an import fails: + +1. Copy all new skill files into a temporary directory `./.agents/skills/.tmp/` (creating it and nested directories as needed) writing each file atomically via `writer.AtomicWrite`. +2. If any write or copy operation fails: + - Clean up the temporary directory `./.agents/skills/.tmp/` via `os.RemoveAll`. + - Return the error. The existing `./.agents/skills//` (if any) remains completely untouched. +3. If all files are copied successfully: + - Remove any stale backup directory `./.agents/skills/.bak/` if present (`os.RemoveAll`). + - If `./.agents/skills//` already exists, rename it to `./.agents/skills/.bak/` (`os.Rename`). + - Rename `./.agents/skills/.tmp/` to `./.agents/skills//` (`os.Rename`). + +### FR-7 TUI screens + +Two new screens wired into `internal/tui/program.go`: + +**Screen A — import_source_view** + +- Lists all agent directories from `cfg.ExpandTargets(home)`. +- Each row shows the agent short name (last path component) and its full path. +- Per-row toggle (reuse `shared_toggle`) to exclude sources. All start checked. +- Session-only: toggles are not persisted to `skill-sync.yaml`. +- Confirm key (`Enter` / `ctrl+s`): transitions to Screen B with the filtered + source list. +- Cancel key (`ctrl+c` / `q`): returns to main menu. + +**Screen B — import_skill_view** + +- Lists all skills found across the checked sources. +- Each row: `` | `` | + ``. +- Single-selection (no multi-select in v1). +- Confirm key (`Enter`): runs import for the selected skill → transitions to + existing `output_view`. +- Cancel key (`esc`): returns to Screen A. +- If no skills are found (all sources empty), show inline message: `"No skills + found in the selected sources."` with only a Back option. + +**Menu entry** + +- Add `"Import"` item to `internal/tui/menu/model.go`, transitioning to Screen A. + +--- + +## 3. Package API: `internal/importer` + +### 3.1 Types + +```go +// SkillCandidate is a skill found during source discovery. +type SkillCandidate struct { + SkillID string // directory name (e.g. "git-expert") + SourceDir string // absolute path to the skill directory + AgentDir string // absolute path to the parent target + Metadata models.SkillMetadata + Mtime time.Time + Hash string // SHA-256 of directory manifest (see §3.4) +} + +// Importer holds resolved configuration and drives import operations. +type Importer struct { + Targets []string // resolved, expanded target directories + DestRoot string // absolute path to ./.agents/skills/ +} +``` + +### 3.2 Constructor + +```go +// New returns an Importer for the given expanded targets and dest root. +// targets must already be tilde-expanded absolute paths. +// destRoot is typically filepath.Join(cwd, ".agents", "skills"). +func New(targets []string, destRoot string) *Importer +``` + +### 3.3 FindSkill + +```go +// FindSkill walks each target's skills/ subdirectory and returns all +// candidates whose directory name matches skillID (case-sensitive). +// Returns an empty slice (not an error) when no match is found. +func (imp *Importer) FindSkill(skillID string) ([]SkillCandidate, error) +``` + +- Walks `/skills/` for each target. +- A directory is a skill candidate if it contains a `SKILL.md` directly + (not recursively). +- YAML frontmatter (`name`, `description`, `version`) is parsed from + `SKILL.md`; missing fields default to zero values (version `0`, name `""`). +- Hash computed via `manifest.DirHash(skillDir)` (see §3.4). +- `os.Stat` provides mtime from the `SKILL.md` file (not the directory). + +### 3.4 manifest.DirHash + +```go +// DirHash computes a deterministic, cross-platform SHA-256 over all files in dir. +// Algorithm: +// 1. Walk dir recursively, collect relative paths of all regular files. +// 2. Normalize all relative paths to use forward slashes (e.g. filepath.ToSlash(relPath)) +// to ensure cross-platform hash determinism. +// 3. Sort normalized relative paths lexicographically. +// 4. For each path: hash "\n". +// 5. Final hash: SHA-256 of the concatenation of all per-file hashes. +func DirHash(dir string) (string, error) +``` + +Returned as a hex string (64 chars). Symlinks are skipped. + +### 3.5 Resolve + +```go +// Resolve converts candidates to SkillInstances and delegates to +// mesh.NewDefaultResolver().Resolve(). Returns the winning candidate, +// a conflict flag, and any error. +func Resolve(candidates []SkillCandidate) (*SkillCandidate, bool, error) +``` + +Conversion: `SkillCandidate → models.SkillInstance` sets: +- `ID` = `SkillCandidate.SkillID` +- `Path` = `filepath.Join(SkillCandidate.SourceDir, "SKILL.md")` +- `TargetDir` = `SkillCandidate.AgentDir` +- `Hash`, `Mtime`, `Metadata` copied directly. + +### 3.6 CopySkill + +```go +// CopySkill copies the full skill directory from src into +// filepath.Join(destRoot, skillID), applying the transactional backup strategy (§FR-6). +// src is the absolute path to the source skill directory. +func (imp *Importer) CopySkill(skillID, src string) error +``` + +- `os.MkdirAll(destRoot, 0o755)` to ensure the destination root exists. +- Construct the temporary path: `tmpDir := filepath.Join(destRoot, skillID + ".tmp")`. +- Ensure clean up of `tmpDir` on failure (e.g. using a deferred function or clean up on error return). +- Walk `src` recursively; for each regular file: + - Compute relative path `relPath` to `src`. + - Compute temporary dest path = `filepath.Join(tmpDir, relPath)`. + - `os.MkdirAll` for intermediate directories under `tmpDir`. + - Read file content. + - Call `writer.AtomicWrite(tmpDestPath, content)`. +- Symlinks are skipped. +- If walking and copying succeeds: + - Remove stale backup: `os.RemoveAll(filepath.Join(destRoot, skillID + ".bak"))`. + - If target `filepath.Join(destRoot, skillID)` exists, rename it to `filepath.Join(destRoot, skillID + ".bak")`. + - Rename `tmpDir` to the final target path `filepath.Join(destRoot, skillID)`. + +--- + +## 4. Data Models (additions to `internal/models`) + +No new exported types in `models` package. `SkillCandidate` lives in +`internal/importer` to avoid circular imports. + +`SkillMetadata` already has `Version` and `Name`. Add `Description`: + +```go +type SkillMetadata struct { + Version float64 `yaml:"version"` + Name string `yaml:"name"` + Description string `yaml:"description"` // NEW +} +``` + +--- + +## 5. Error Taxonomy + +| Sentinel / package | Condition | User message | +|---|---|---| +| `cli.ErrConfigNotFound` (`internal/cli`) | `skill-sync.yaml` not found by resolver | `"no skill-sync.yaml found; run 'skill-sync init' to create one"` | +| `importer.ErrSkillNotFound` (`internal/importer`) | `FindSkill` returns empty candidates | `"skill '' not found in any configured source"` | +| `importer.ErrConflict` (`internal/importer`) | `Resolve` returns `conflicts=true` | `"conflict: '' has identical version and mtime in multiple sources; resolve manually"` | +| wrapped `fs` errors | Copy, backup, or mkdir failures | error message from `fmt.Errorf("...: %w", err)` | + +`ErrConfigNotFound` is defined in `internal/cli` because config resolution +happens at the CLI layer before `Importer` is constructed. `ErrSkillNotFound` +and `ErrConflict` are exported sentinels in `internal/importer`. All three are +checked with `errors.Is`. + +--- + +## 6. CLI Command (`internal/cli/import.go`) + +```go +func NewImportCmd() *cobra.Command +``` + +Flow: + +1. Validate exactly one positional arg; return usage error otherwise. +2. Resolve config via `config_resolver.New().Resolve(cfgFlag)`. +3. Parse config with `agent.ParseConfig`; expand targets with + `cfg.ExpandTargets(home)`. +4. Construct `importer.New(targets, destRoot)`. +5. Call `FindSkill(skillID)` → handle `ErrSkillNotFound`. +6. Call `Resolve(candidates)` → handle `ErrConflict`. +7. Call `CopySkill(skillID, winner.SourceDir)`. +8. Print `"Imported from "` to stdout. + +No interactive output, no spinner. Suitable for scripting. + +--- + +## 7. TUI Integration (`internal/tui/program.go`) + +### New state constants + +```go +stateImportSource // Screen A +stateImportSkill // Screen B +``` + +### New message types + +```go +type importSourceConfirmedMsg struct{ sources []string } +type importSkillSelectedMsg struct{ candidate importer.SkillCandidate } +type importSkillBackMsg struct{} +``` + +### State machine additions + +``` +stateMenu + → (user selects "Import") → stateImportSource +stateImportSource + → (confirm) → stateImportSkill + → (cancel) → stateMenu +stateImportSkill + → (confirm) → run import → stateOutput + → (back) → stateImportSource +stateOutput + → (quit/back) → stateMenu +``` + +`stateOutput` is the existing `output_view`. + +--- + +## 8. Test Requirements + +### 8.1 `internal/importer` + +| Test | Type | Notes | +|---|---|---| +| `FindSkill` returns candidates from multiple targets | unit | `t.TempDir()`, populate skills/ dirs | +| `FindSkill` returns empty for unknown skill ID | unit | | +| `FindSkill` skips targets whose skills/ dir does not exist | unit | | +| `FindSkill` reads YAML frontmatter (name, description, version) | unit | | +| `DirHash` is deterministic and order-independent | unit | two dirs with same files in different creation order | +| `DirHash` changes when a file changes | unit | | +| `Resolve` picks highest version winner | unit | delegate to mesh resolver rules | +| `Resolve` picks newest mtime when versions match | unit | | +| `Resolve` returns conflict=true on tie | unit | | +| `CopySkill` copies full directory tree | integration | real temp dirs | +| `CopySkill` creates backup when dest exists | integration | | +| `CopySkill` removes stale .bak before backup | integration | | +| `CopySkill` is idempotent on identical source | integration | | +| `CopySkill` returns error and leaves existing target untouched when write fails | integration | inject read-only file | + +### 8.2 `internal/cli/import.go` + +| Test | Type | Notes | +|---|---|---| +| Happy path: imports skill, exits 0, prints confirmation | integration | real temp dirs + real SKILL.md | +| Missing config file: exits 1 with clear message | integration | | +| Skill not found: exits 1 with clear message | integration | | +| Wrong arg count (0 or 2+): exits 1 with usage error | unit | | +| Conflict: exits 1 with conflict message | integration | two sources with identical version/mtime/different hash | + +### 8.3 TUI views + +| View | Required golden files | +|---|---| +| `import_source_view` | `initial.txt`, `one_source_unchecked.txt`, `all_unchecked.txt` | +| `import_skill_view` | `initial.txt`, `skill_selected.txt`, `no_skills.txt` | + +All TUI tests use `teatest` + `t.TempDir()`. + +### 8.4 `internal/models` + +- Add a test case to the existing `SkillMetadata` unmarshal test verifying + that `description` round-trips correctly. + +--- + +## 9. Non-Functional Requirements + +- No new external Go module dependencies. +- `go test ./...` must pass on Linux with zero failures. +- Windows: `CopySkill` backup rename may fail on cross-device moves; guard + with `t.Skip` on Windows where necessary. +- `go vet ./...` and `staticcheck ./...` clean. +- All new exported symbols have GoDoc comments. + +--- + +## 10. Out of Scope (confirmed) + +- Multi-destination import +- `--all` flag +- `--target` flag +- `--dry-run` flag +- Persisting agent-filter choices to `skill-sync.yaml` +- Unimport / remove command +- Interactive tie-break in TUI (deferred to v2) +- Mesh-sync directory-copy bug fix (tracked separately) +- Any change to `sync`, `verify`, scanner, engine, or mesh resolver diff --git a/openspec/changes/skill-import/tasks.md b/openspec/changes/skill-import/tasks.md new file mode 100644 index 0000000..48ed8b6 --- /dev/null +++ b/openspec/changes/skill-import/tasks.md @@ -0,0 +1,275 @@ +# Tasks: `skill-import` + +**Change ID:** skill-import +**Branch:** feat/import_skills +**Date:** 2026-06-05 +**Spec:** `openspec/changes/skill-import/spec.md` +**Design:** `openspec/changes/skill-import/design.md` + +--- + +## Phase 1 — Foundation + +### T-01 · `internal/models/skill.go` — add `Description` field + +**Files:** `internal/models/skill.go`, `internal/models/skill_test.go` (if exists; else add to registry_test or create) + +**Steps:** +1. Add `Description string \`yaml:"description"\`` to `SkillMetadata`. +2. Add/extend table-driven test to verify `description` round-trips through `yaml.Unmarshal` / `yaml.Marshal`. + +**Done when:** `go test ./internal/models/...` passes. + +--- + +### T-02 · `internal/importer/errors.go` — sentinel errors + +**Files:** `internal/importer/errors.go` (new) + +**Steps:** +1. Create file with `package importer`. +2. Declare `var ErrSkillNotFound = errors.New("skill not found in any configured source")`. +3. Declare `var ErrConflict = errors.New("conflict: identical version and mtime in multiple sources")`. + +**Done when:** file compiles (`go build ./internal/importer/...`). + +--- + +### T-03 · `internal/importer/manifest.go` — `DirHash` + +**Files:** `internal/importer/manifest.go` (new), `internal/importer/manifest_test.go` (new) + +**Steps:** + +RED: +1. Write `manifest_test.go` with table-driven cases: + - `DirHash` is deterministic: same dir hashed twice → same result. + - `DirHash` is order-independent: two dirs with same files written in different order → same hash. + - `DirHash` changes when a file's content changes. + - `DirHash` changes when a file is added. + - Symlinks are skipped (Unix only; skip on Windows via `runtime.GOOS`). +2. Run tests → confirm RED. + +GREEN: +3. Implement `DirHash` in `manifest.go` (design §3): + - `filepath.WalkDir` → collect regular files. + - `filepath.ToSlash(rel)` for cross-platform normalisation. + - Sort paths lexicographically. + - SHA-256 over `relPath\n` + file bytes for each entry. + - Return hex string. +4. Run tests → confirm GREEN. + +**Done when:** `go test ./internal/importer/...` (manifest tests only) passes. + +--- + +### T-04 · `internal/importer/importer.go` — core importer + +**Files:** `internal/importer/importer.go` (new), `internal/importer/importer_test.go` (new) + +**Steps:** + +RED: +1. Write `importer_test.go` with all table-driven cases from spec §8.1: + + Unit — `FindSkill`: + - Returns candidates from multiple targets. + - Returns empty slice for unknown skillID. + - Skips targets whose `skills/` dir does not exist. + - Reads YAML frontmatter (`name`, `description`, `version`). + + Unit — `Resolve`: + - Picks highest version winner. + - Picks newest mtime when versions tie. + - Returns `conflict=true` + `ErrConflict` on true tie. + + Unit — `FindAllSkills`: + - Returns one entry per skillID (not one per candidate). + - Marks `IsConflict=true` when sources tie. + - Result is sorted by `SkillID`. + - Skips targets whose `skills/` dir does not exist. + + Integration — `CopySkill`: + - Copies full directory tree into destRoot. + - Creates `.bak` when destination already exists. + - Removes stale `.bak` before creating new one. + - Is idempotent on identical source (overwrite = same content). + - On write failure: returns error and leaves existing destination untouched (inject a read-only file in `.tmp` path to trigger failure). + +2. Run tests → confirm RED. + +GREEN: +3. Implement `importer.go` (design §4): + - `SkillCandidate` struct with `IsConflict bool`. + - `Importer` struct + `New`. + - `readMetadata` (frontmatter parser aligned with design §4 — `HasPrefix("---\n")` / `"---\r\n"`, `endIndex+3` index math). + - `FindSkill`. + - `FindAllSkills` (group by skillID → resolve → mark conflict or use winner). + - `Resolve` (delegate to `mesh.NewDefaultResolver`, re-map winner back to `SkillCandidate` by `AgentDir`). + - `CopySkill` (transactional: `.tmp` → commit → rotate `.bak`; design §4 / FR-6). +4. Run tests → GREEN. + +**Done when:** `go test ./internal/importer/...` passes with zero failures. + +--- + +## Phase 2 — TUI Views + +### T-05 · `internal/tui/import_source_view` — agent-filter screen + +**Files:** `internal/tui/import_source_view/model.go` (new), `internal/tui/import_source_view/model_test.go` (new), `testdata/golden/*.txt` (new) + +**Steps:** + +RED: +1. Write `model_test.go` with `teatest` golden-file tests: + - `initial.txt` — all sources checked, cursor on first row. + - `one_source_unchecked.txt` — first source toggled off. + - `all_unchecked.txt` — all sources deselected (`A` key). +2. Run tests → RED (golden files missing). + +GREEN: +3. Implement `model.go` (design §6): + - `ConfirmedMsg{Sources []string}` and `BackMsg{}`. + - `Model` wrapping `shared_toggle`. + - `New(targets []string) Model` — all items checked. + - `Update`: `enter`/`ctrl+s` → `ConfirmedMsg`; `q`/`esc` → `BackMsg`; `a`/`A` → select/deselect all; delegate rest to `shared_toggle`. + - `View`: title + toggle + help line. +4. Run tests with `-update` flag to capture golden files. +5. Run tests without `-update` → GREEN. + +**Done when:** `go test ./internal/tui/import_source_view/...` passes. + +--- + +### T-06 · `internal/tui/import_skill_view` — skill-select screen + +**Files:** `internal/tui/import_skill_view/model.go` (new), `internal/tui/import_skill_view/model_test.go` (new), `testdata/golden/*.txt` (new) + +**Steps:** + +RED: +1. Write `model_test.go` with `teatest` golden-file tests: + - `initial.txt` — list rendered with cursor on first row. + - `skill_selected.txt` — cursor moved to second row. + - `no_skills.txt` — handled by `output_view` (note in test: `import_skill_view.New` is never called with empty slice; test can verify `len(candidates) > 0` invariant). + - One test for a candidate with `IsConflict=true` renders `[CONFLICT]` prefix. +2. Run tests → RED. + +GREEN: +3. Implement `model.go` (design §7): + - `ConfirmedMsg{Candidate importer.SkillCandidate}` and `BackMsg{}`. + - `Model` with `candidates []importer.SkillCandidate`, `cursor int`, `scroll int`. + - `New(candidates []importer.SkillCandidate) Model`. + - `Update`: up/down navigation with scroll; `enter` → `ConfirmedMsg`; `esc` → `BackMsg`; `ctrl+c` → quit. + - `View`: title + count + scrollable list (design §7 column format + `→` cursor marker + `[CONFLICT]` prefix when `IsConflict`). +4. Capture golden files → run clean → GREEN. + +**Done when:** `go test ./internal/tui/import_skill_view/...` passes. + +--- + +### T-07 · `internal/tui/menu/model.go` — add Import item + +**Files:** `internal/tui/menu/model.go`, `internal/tui/menu/model_test.go` + +**Steps:** +1. Add `item{title: "Import", desc: "Import a skill from a configured agent source"}` between `"Verify"` and `"Config"` in the `items` slice. +2. Extend existing menu test (or add case) to verify `"Import"` appears in the list and emits `MenuSelectionMsg{Selection: "Import"}` on Enter. + +**Done when:** `go test ./internal/tui/menu/...` passes. + +--- + +### T-08 · `internal/tui/program.go` — wire import states + +**Files:** `internal/tui/program.go`, `internal/tui/program_test.go` + +**Steps:** +1. Add imports for `importer`, `import_source_view`, `import_skill_view`. +2. Add state constants `stateImportSource`, `stateImportSkill`. +3. Extend `Callbacks` struct with `FindSkillsForImport` and `RunImport` (design §8.3). +4. Add `importSourceModel` and `importSkillModel` fields to `RootModel`. +5. Add `loadImportTargets()` helper (design §8.9). +6. In `Update`: + - Handle `menu.MenuSelectionMsg{Selection: "Import"}` → call `loadImportTargets`, init `import_source_view`. + - Handle `import_source_view.ConfirmedMsg` → call `FindSkillsForImport`, transition to `stateImportSkill` or `stateOutput` (empty). + - Handle `import_source_view.BackMsg` → back to `stateMenu`. + - Handle `import_skill_view.ConfirmedMsg` → call `RunImport`, transition to `stateOutput`. + - Handle `import_skill_view.BackMsg` → back to `stateImportSource`. + - Add dispatch cases for `stateImportSource` and `stateImportSkill`. +7. In `View`: add cases for `stateImportSource` and `stateImportSkill`. +8. Extend `program_test.go`: verify that selecting `"Import"` menu item transitions to `stateImportSource` (use a stub `FindSkillsForImport` callback that returns a fixed list). + +**Done when:** `go test ./internal/tui/...` passes. + +--- + +## Phase 3 — CLI & Wiring + +### T-09 · `internal/cli/import.go` — Cobra command + +**Files:** `internal/cli/import.go` (new), `internal/cli/import_test.go` (new) + +**Steps:** + +RED: +1. Write `import_test.go` with integration test cases (spec §8.2): + - Happy path: creates temp dir with `skill-sync.yaml` + source skill dir; runs command; asserts exit 0 + output `"Imported from "` + destination files exist. + - Missing config: no `skill-sync.yaml` → exit 1, message contains `"skill-sync init"`. + - Skill not found: config exists but skill not in targets → exit 1, message contains `"not found"`. + - Wrong arg count (0 args, 2 args) → exit 1 with usage error. + - Conflict: two sources with identical version+mtime+different hash → exit 1, message contains `"conflict"`. +2. Run tests → RED. + +GREEN: +3. Implement `import.go` (design §10): `NewImportCmd` + `runImport` (resolve config → expand targets → `FindSkill` → `Resolve` → `CopySkill` → print confirmation). +4. Run tests → GREEN. + +**Done when:** `go test ./internal/cli/... -run TestImport` passes. + +--- + +### T-10 · `internal/cli/root.go` — register command + wire callbacks + +**Files:** `internal/cli/root.go` + +**Steps:** +1. Add `cmd.AddCommand(NewImportCmd())` in `NewRootCmd`. +2. Add `"os"`, `"path/filepath"`, `"fmt"`, `"github.com/ezzek/skill-sync/internal/importer"` imports. +3. Wire `FindSkillsForImport` and `RunImport` callbacks in the default `RunE` (design §11.2): + - `FindSkillsForImport`: build `importer.New(sources, destRoot)` from `sources` arg + `cwd`; return `FindAllSkills()`. + - `RunImport`: guard `candidate.IsConflict → ErrConflict`; build `importer.New(nil, destRoot)`; call `CopySkill`. + +**Done when:** `go test ./internal/cli/...` passes (all CLI tests, not just import). + +--- + +## Phase 4 — Full Suite Verification + +### T-11 · Full test suite + regression check + +**Steps:** +1. `go build ./...` — clean build, zero errors. +2. `go test ./...` — zero failures on Linux. +3. `go vet ./...` — clean. +4. Confirm `go test ./internal/sync/... ./internal/mesh/... ./internal/cli/... -run TestSync -run TestVerify` still green (spec FR non-regression). +5. Manual smoke test (optional): `go run ./cmd/skill-sync import -c ` in a scratch dir. + +**Done when:** all tests green, build clean. + +--- + +## Checklist + +- [x] T-01 `models.SkillMetadata.Description` + test +- [x] T-02 `importer/errors.go` +- [x] T-03 `importer/manifest.go` + tests (DirHash) +- [x] T-04 `importer/importer.go` + tests (FindSkill, FindAllSkills, Resolve, CopySkill) +- [x] T-05 `tui/import_source_view` + golden tests +- [x] T-06 `tui/import_skill_view` + golden tests +- [x] T-07 `tui/menu` — Import item +- [x] T-08 `tui/program.go` — wire states +- [x] T-09 `cli/import.go` + integration tests +- [x] T-10 `cli/root.go` — register + callbacks +- [x] T-11 Full suite green diff --git a/openspec/config.yaml b/openspec/config.yaml new file mode 100644 index 0000000..c4609eb --- /dev/null +++ b/openspec/config.yaml @@ -0,0 +1,52 @@ +# OpenSpec / SDD configuration for skill-sync +# +# This file tells SDD phase agents (sdd-proposal, sdd-spec, sdd-design, +# sdd-tasks, sdd-apply, sdd-verify, sdd-archive) how this project expects +# to be worked on. + +project: + name: skill-sync + language: go + go_version: "1.26.1" + description: > + Multidirectional (mesh / P2P) CLI + TUI to synchronize AI agent + skills across tool directories (Claude, Cursor, OpenCode, Pi, etc.). + +testing: + strict_tdd: true + discipline: RED -> GREEN -> TRIANGULATE -> REFACTOR + test_command: "go test ./..." + coverage_command: "go test -cover ./..." + style: + - table-driven tests are mandatory + - integration tests for Engine and CLI commands must use real temp dirs + - never mock the filesystem unless absolutely required + - OS-specific failure paths may be guarded with t.Skip on Windows + +artifacts: + store: openspec + location: openspec/ + changes_dir: openspec/changes + specs_dir: openspec/specs + +review: + budget_lines: 400 + chained_pr_strategy: single-pr-default + pause_before_apply_if_exceeds: true + +execution: + mode: interactive + pause_between_phases: true + +cli: + silence_usage: true + framework: cobra + entrypoint: cmd/skill-sync/main.go + +tui: + framework: bubbletea + golden_files: true + testing_helper: teatest + +pending_fixes: + - context/pending-fixes.md diff --git a/openspec/project.md b/openspec/project.md new file mode 100644 index 0000000..6c890bc --- /dev/null +++ b/openspec/project.md @@ -0,0 +1,153 @@ +# skill-sync — Project Context for SDD + +This document is the source of architectural truth that every SDD phase +agent (`sdd-proposal`, `sdd-spec`, `sdd-design`, `sdd-tasks`, +`sdd-apply`, `sdd-verify`, `sdd-archive`) must read before producing +artifacts. It complements — does not replace — `AGENTS.md` at the repo +root. + +## 1. Purpose + +`skill-sync` is a **multidirectional (mesh / P2P) CLI + TUI** that keeps +AI agent skills consistent across multiple tool directories on a single +machine (e.g. `~/.claude/skills`, `~/.cursor/skills`, +`~/.pi/agent/skills`, `~/.opencode/skills`, `~/.gemini/skills`). It is +not a publisher and not a marketplace. + +## 2. Architecture + +Core pipeline: + +```text +Scanner ─► Resolver ─► Engine + finds decides writes +SKILL.md "winner" atomically +``` + +Modules (`internal/`): + +- `agent/` — config parsing, registry, discovery, tilde + expansion (`ResolvePath`, `ExpandTargets`). +- `agent/discovery/` — detect installed agent directories. +- `agent/config_resolver/` — locate `skill-sync.yaml` (cwd, user dir, + global fallback). +- `mesh/scanner.go` — walks each target's `skills/` directory looking + for `SKILL.md` files. **Known limitation:** + currently only the file is tracked, not the + full skill directory (see + `context/pending-fixes.md`). +- `mesh/resolver.go` — applies winner rules: + 1. hash match across all targets → no-op, + 2. highest YAML-frontmatter `version` wins, + 3. on tie, newest `mtime` wins, + 4. equal version + equal mtime + different hash → conflict, + skip skill and report. +- `sync/engine.go` — orchestrates Scanner → Resolver → Writer. +- `writer/atomic.go` — strictly atomic writes: temp file + `fsync` + + `os.Rename`, plus `.bak` backup before + overwriting any existing target file. +- `cli/` — Cobra commands: `init`, `config`, `sync`, + `verify`. `root.go` sets `SilenceUsage: true` + so drift errors do not flood the terminal with + usage text. +- `tui/` — Bubble Tea program with vistas: `menu`, + `init_view`, `config_view`, `sync_select_view`, + `shared_toggle`, `output_view`. Test harness + uses `teatest` + golden files. + +Mandatory invariants: + +- All file writes go through `writer.AtomicWrite` (including config + writes from CLI and TUI). +- `verify` (`internal/cli/verify.go`) is strictly read-only and exits + with code `1` on detected drift. +- Tilde (`~`) in config targets is expanded via `cfg.ExpandTargets(home)` + at every CLI/TUI entry point. + +## 3. Configuration + +Schema (`skill-sync.yaml`): + +```yaml +include: [] # optional include globs (relative skill names) +exclude: [] # optional exclude globs +targets: # FLAT LIST OF DIRECTORY PATHS (strings, not objects) + - ~/.claude + - ~/.cursor + - ~/.opencode + - ~/.pi/agent + - ~/.gemini +``` + +Parser: `internal/agent/registry.go` (`ParseConfig`, +`ResolvePath`, `ExpandTargets`). + +Target semantics: each `target` is the **parent** of the agent's +`skills/` directory. The scanner concatenates `skills/` internally. + +## 4. Testing Discipline + +- Runner: `go test ./...` (coverage: `go test -cover ./...`). +- **Strict TDD is mandatory** for non-trivial work: RED → GREEN → + TRIANGULATE → REFACTOR. Phase agents must record evidence. +- Table-driven tests are mandatory. Pattern: + `tests := []struct{ name string; ... }{...}`. +- Integration tests for `Engine` and CLI commands use real temp + directories (`t.TempDir()`); avoid mocking the filesystem. +- TUI tests use Bubble Tea's `teatest` + golden files. +- OS-specific failure paths (e.g. renaming over a read-only dir) may + be guarded with `t.Skip` + `runtime.GOOS == "windows"`. Do not + force brittle OS-level injections. + +## 5. CLI Surface (current) + +- `skill-sync init -c ` — populate config interactively. +- `skill-sync config -c ` — adjust config. +- `skill-sync sync -c ` — apply mesh sync across targets. +- `skill-sync verify -c ` — read-only drift check; exit 1 on + drift. `SilenceUsage: true` is set on every command. + +## 6. TUI Surface (current) + +Entrypoint: `internal/tui/program.go`. Views are independently testable +with `teatest` + golden snapshots stored beside each view's +`model_test.go`. + +## 7. Pending Architectural Fixes + +Tracked outside of `openspec/changes/` until promoted: + +- `context/pending-fixes.md` — mesh sync currently propagates only + `SKILL.md`, not the full skill directory. Must be addressed in a + follow-up SDD change that reuses the directory-copy primitive + introduced by the `import` change. + +## 8. Skill Registry + +`.atl/skill-registry.md` indexes user/global skills. The parent +orchestrator selects matching `SKILL.md` paths and injects them into +subagent prompts under `## Skills to load before work`. Highly +relevant for this project: + +- `go-testing` — Bubbletea teatest, golden files, Go test patterns. +- `lsp-navigation`, `ast-grep` — code intelligence and AST-aware edits. +- `git-expert`, `git-committer`, `work-unit-commits` — commits and PR + slicing. +- `cognitive-doc-design` — review-facing docs. + +## 9. Conventions for Generated Artifacts + +- Code, identifiers, comments, commit messages, filenames, and CLI + help text default to English. +- User-facing conversation in this repo may be in Spanish (Rioplatense + voseo) but generated artifacts stay in English unless explicitly + requested otherwise. +- Atomic writes only; never `os.Create` or `os.WriteFile` directly on + a destination path that already exists. + +## 10. Safety + +- Never commit unless the user explicitly asks. +- Never run destructive git operations without confirmation. +- Single-writer discipline: keep writes single-threaded unless + isolated worktrees are explicitly approved. From e73135b9f443a408f569aefbaf895e07e9e35187 Mon Sep 17 00:00:00 2001 From: Ezequiel Canul Date: Fri, 2 Oct 2026 14:18:10 -0600 Subject: [PATCH 3/4] feat(import): add single-skill import from configured agent sources Adds the skill-import change: a new internal/importer package that locates a skill across globally configured agent sources, resolves the winner with the mesh rules, and copies it into ./.agents/skills// transactionally. - importer.DirHash hashes the whole skill directory manifest instead of only SKILL.md, so conflict resolution sees sibling file changes. - importer.CopySkill stages into .tmp and only rotates .bak and renames on full success, leaving the destination untouched on failure. - CLI: skill-sync import . - TUI: import_source_view and import_skill_view wired into program.go, with a [CONFLICT] marker for unresolvable candidates. - models.SkillMetadata gains Description for the TUI listing. --- internal/cli/import.go | 85 ++++ internal/cli/import_test.go | 233 +++++++++ internal/cli/root.go | 28 ++ internal/importer/errors.go | 12 + internal/importer/importer.go | 278 +++++++++++ internal/importer/importer_test.go | 446 ++++++++++++++++++ internal/importer/manifest.go | 64 +++ internal/importer/manifest_test.go | 148 ++++++ internal/models/skill.go | 5 +- internal/models/skill_test.go | 93 ++++ internal/tui/import_skill_view/model.go | 110 +++++ internal/tui/import_skill_view/model_test.go | 214 +++++++++ internal/tui/import_source_view/model.go | 81 ++++ internal/tui/import_source_view/model_test.go | 142 ++++++ internal/tui/menu/model.go | 1 + internal/tui/menu/model_test.go | 23 + internal/tui/program.go | 115 ++++- internal/tui/program_test.go | 74 +++ 18 files changed, 2139 insertions(+), 13 deletions(-) create mode 100644 internal/cli/import.go create mode 100644 internal/cli/import_test.go create mode 100644 internal/importer/errors.go create mode 100644 internal/importer/importer.go create mode 100644 internal/importer/importer_test.go create mode 100644 internal/importer/manifest.go create mode 100644 internal/importer/manifest_test.go create mode 100644 internal/models/skill_test.go create mode 100644 internal/tui/import_skill_view/model.go create mode 100644 internal/tui/import_skill_view/model_test.go create mode 100644 internal/tui/import_source_view/model.go create mode 100644 internal/tui/import_source_view/model_test.go diff --git a/internal/cli/import.go b/internal/cli/import.go new file mode 100644 index 0000000..4372f14 --- /dev/null +++ b/internal/cli/import.go @@ -0,0 +1,85 @@ +package cli + +import ( + "errors" + "fmt" + "os" + "path/filepath" + + "github.com/ezzek/skill-sync/internal/agent" + "github.com/ezzek/skill-sync/internal/agent/config_resolver" + "github.com/ezzek/skill-sync/internal/importer" + "github.com/spf13/cobra" +) + +// NewImportCmd returns the Cobra command for importing a single skill. +func NewImportCmd() *cobra.Command { + var configPath string + + cmd := &cobra.Command{ + Use: "import ", + Short: "Import a skill from a configured agent source into ./.agents/skills/", + Args: cobra.ExactArgs(1), + SilenceUsage: true, + RunE: func(cmd *cobra.Command, args []string) error { + cwd, err := getWd() + if err != nil { + return fmt.Errorf("getwd: %w", err) + } + return runImport(cmd, configPath, cwd, args[0]) + }, + } + cmd.Flags().StringVarP(&configPath, "config", "c", "", "Path to skill-sync.yaml") + return cmd +} + +func runImport(cmd *cobra.Command, configPath, cwd, skillID string) error { + // 1. Resolve config. + resolvedPath, err := config_resolver.New().Resolve(configPath) + if err != nil { + return fmt.Errorf("no skill-sync.yaml found; run 'skill-sync init' to create one") + } + f, err := os.Open(resolvedPath) + if err != nil { + return fmt.Errorf("open config: %w", err) + } + defer f.Close() + + cfg, err := agent.ParseConfig(f) + if err != nil { + return fmt.Errorf("parse config: %w", err) + } + if home, err := os.UserHomeDir(); err == nil { + cfg.ExpandTargets(home) + } + + // 2. Build importer. + destRoot := filepath.Join(cwd, ".agents", "skills") + imp := importer.New(cfg.Targets, destRoot) + + // 3. Find candidates. + candidates, err := imp.FindSkill(skillID) + if err != nil { + return err + } + if len(candidates) == 0 { + return fmt.Errorf("skill %q not found in any configured source", skillID) + } + + // 4. Resolve winner. + winner, _, err := importer.Resolve(candidates) + if err != nil { + if errors.Is(err, importer.ErrConflict) { + return fmt.Errorf("conflict: %q has identical version and mtime in multiple sources; resolve manually", skillID) + } + return err + } + + // 5. Copy. + if err := imp.CopySkill(skillID, winner.SourceDir); err != nil { + return err + } + + fmt.Fprintf(cmd.OutOrStdout(), "Imported %s from %s\n", skillID, winner.SourceDir) + return nil +} \ No newline at end of file diff --git a/internal/cli/import_test.go b/internal/cli/import_test.go new file mode 100644 index 0000000..9763ab2 --- /dev/null +++ b/internal/cli/import_test.go @@ -0,0 +1,233 @@ +package cli + +import ( + "os" + "path/filepath" + "strings" + "testing" + "time" +) + +// writeImportSkillFile creates a skill directory with SKILL.md at +// //skills//SKILL.md +func writeImportSkillFile(t *testing.T, base, target, skillID, content string) { + t.Helper() + skillDir := filepath.Join(base, target, "skills", skillID) + if err := os.MkdirAll(skillDir, 0755); err != nil { + t.Fatalf("failed to create skill dir: %v", err) + } + if err := os.WriteFile(filepath.Join(skillDir, "SKILL.md"), []byte(content), 0644); err != nil { + t.Fatalf("failed to write skill file: %v", err) + } +} + +// writeImportConfig writes a skill-sync.yaml with the given targets. +func writeImportConfig(t *testing.T, base string, targets []string) string { + t.Helper() + var sb strings.Builder + sb.WriteString("targets:\n") + for _, tgt := range targets { + sb.WriteString(" - " + tgt + "\n") + } + cfgPath := filepath.Join(base, "skill-sync.yaml") + if err := os.WriteFile(cfgPath, []byte(sb.String()), 0644); err != nil { + t.Fatalf("failed to write config: %v", err) + } + return cfgPath +} + +func TestImportCmd(t *testing.T) { + origGetWd := getWd + defer func() { + getWd = origGetWd + }() + + tests := []struct { + name string + setup func(t *testing.T, base string) (args []string, cfgPath string, cwd string) + verify func(t *testing.T, base string, stdout, stderr string, err error) + wantErr bool + wantErrMsg string + }{ + { + name: "happy path — imports skill and creates destination files", + setup: func(t *testing.T, base string) ([]string, string, string) { + toolA := filepath.Join(base, "toolA") + writeImportSkillFile(t, base, "toolA", "git-expert", "---\nversion: 2\nname: Git Expert\ndescription: Git expertise\n---\n# Git Expert v2") + cfgPath := writeImportConfig(t, base, []string{toolA}) + return []string{"git-expert"}, cfgPath, base + }, + verify: func(t *testing.T, base string, stdout, stderr string, err error) { + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if !strings.Contains(stdout, "Imported git-expert") { + t.Errorf("expected stdout to contain 'Imported git-expert', got: %s", stdout) + } + destPath := filepath.Join(base, ".agents", "skills", "git-expert", "SKILL.md") + data, readErr := os.ReadFile(destPath) + if readErr != nil { + t.Fatalf("expected skill file at %s, got error: %v", destPath, readErr) + } + if !strings.Contains(string(data), "# Git Expert v2") { + t.Errorf("expected imported content to contain '# Git Expert v2', got: %s", string(data)) + } + }, + wantErr: false, + }, + { + name: "missing config file — exit 1 with init hint", + setup: func(t *testing.T, base string) ([]string, string, string) { + noCfg := filepath.Join(base, "nonexistent.yaml") + return []string{"git-expert"}, noCfg, base + }, + verify: func(t *testing.T, base string, stdout, stderr string, err error) {}, + wantErr: true, + wantErrMsg: "skill-sync init", + }, + { + name: "skill not found in any source — exit 1", + setup: func(t *testing.T, base string) ([]string, string, string) { + toolA := filepath.Join(base, "toolA") + writeImportSkillFile(t, base, "toolA", "other-skill", "---\nversion: 1\n---\n# Other") + cfgPath := writeImportConfig(t, base, []string{toolA}) + return []string{"nonexistent-skill"}, cfgPath, base + }, + verify: func(t *testing.T, base string, stdout, stderr string, err error) {}, + wantErr: true, + wantErrMsg: "not found", + }, + { + name: "wrong arg count — zero args — exit 1 with usage", + setup: func(t *testing.T, base string) ([]string, string, string) { + cfgPath := filepath.Join(base, "skill-sync.yaml") + _ = os.WriteFile(cfgPath, []byte("targets:\n - /tmp\n"), 0644) + return []string{}, cfgPath, base + }, + verify: func(t *testing.T, base string, stdout, stderr string, err error) {}, + wantErr: true, + wantErrMsg: "accepts 1 arg", + }, + { + name: "wrong arg count — two args — exit 1 with usage", + setup: func(t *testing.T, base string) ([]string, string, string) { + cfgPath := filepath.Join(base, "skill-sync.yaml") + _ = os.WriteFile(cfgPath, []byte("targets:\n - /tmp\n"), 0644) + return []string{"arg1", "arg2"}, cfgPath, base + }, + verify: func(t *testing.T, base string, stdout, stderr string, err error) {}, + wantErr: true, + wantErrMsg: "accepts 1 arg", + }, + { + name: "conflict — identical version and mtime, different hash — exit 1", + setup: func(t *testing.T, base string) ([]string, string, string) { + toolA := filepath.Join(base, "toolA") + toolB := filepath.Join(base, "toolB") + + writeImportSkillFile(t, base, "toolA", "ambiguous-skill", "---\nversion: 1\n---\n# Content A") + writeImportSkillFile(t, base, "toolB", "ambiguous-skill", "---\nversion: 1\n---\n# Content B") + + fixedTime := time.Date(2024, 1, 1, 0, 0, 0, 0, time.UTC) + pathA := filepath.Join(base, "toolA", "skills", "ambiguous-skill", "SKILL.md") + pathB := filepath.Join(base, "toolB", "skills", "ambiguous-skill", "SKILL.md") + _ = os.Chtimes(pathA, fixedTime, fixedTime) + _ = os.Chtimes(pathB, fixedTime, fixedTime) + + cfgPath := writeImportConfig(t, base, []string{toolA, toolB}) + return []string{"ambiguous-skill"}, cfgPath, base + }, + verify: func(t *testing.T, base string, stdout, stderr string, err error) {}, + wantErr: true, + wantErrMsg: "conflict", + }, + { + name: "idempotent — imports same skill twice without error", + setup: func(t *testing.T, base string) ([]string, string, string) { + toolA := filepath.Join(base, "toolA") + writeImportSkillFile(t, base, "toolA", "stable-skill", "---\nversion: 1\n---\n# Stable Skill") + cfgPath := writeImportConfig(t, base, []string{toolA}) + return []string{"stable-skill"}, cfgPath, base + }, + verify: func(t *testing.T, base string, stdout, stderr string, err error) { + if err != nil { + t.Fatalf("first import failed: %v", err) + } + if !strings.Contains(stdout, "Imported stable-skill") { + t.Errorf("expected first import confirmation, got: %s", stdout) + } + }, + wantErr: false, + }, + { + name: "creates .bak backup when destination already exists", + setup: func(t *testing.T, base string) ([]string, string, string) { + toolA := filepath.Join(base, "toolA") + toolB := filepath.Join(base, "toolB") + + writeImportSkillFile(t, base, "toolA", "backup-skill", "---\nversion: 2\n---\n# Backup Skill v2") + writeImportSkillFile(t, base, "toolB", "backup-skill", "---\nversion: 1\n---\n# Backup Skill v1") + + // Populate the destination before import so it exists and has the old content + destDir := filepath.Join(base, ".agents", "skills", "backup-skill") + if err := os.MkdirAll(destDir, 0755); err != nil { + t.Fatalf("failed to create destination dir: %v", err) + } + if err := os.WriteFile(filepath.Join(destDir, "SKILL.md"), []byte("---\nversion: 1\n---\n# Backup Skill v1"), 0644); err != nil { + t.Fatalf("failed to write destination skill: %v", err) + } + + cfgPath := writeImportConfig(t, base, []string{toolA, toolB}) + return []string{"backup-skill"}, cfgPath, base + }, + verify: func(t *testing.T, base string, stdout, stderr string, err error) { + if err != nil { + t.Fatalf("import failed: %v", err) + } + bakPath := filepath.Join(base, ".agents", "skills", "backup-skill.bak", "SKILL.md") + data, readErr := os.ReadFile(bakPath) + if readErr != nil { + t.Fatalf("expected .bak file at %s, got error: %v", bakPath, readErr) + } + if !strings.Contains(string(data), "# Backup Skill v1") { + t.Errorf("expected .bak to contain old content '# Backup Skill v1', got: %s", string(data)) + } + }, + wantErr: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + base := t.TempDir() + args, cfgPath, cwd := tt.setup(t, base) + + if cwd != "" { + getWd = func() (string, error) { return cwd, nil } + } else { + getWd = os.Getwd + } + + cmd := NewImportCmd() + cmd.SetArgs(append([]string{"--config", cfgPath}, args...)) + + var stdout, stderr strings.Builder + cmd.SetOut(&stdout) + cmd.SetErr(&stderr) + + err := cmd.Execute() + + if tt.wantErr { + if err == nil { + t.Fatalf("expected error containing %q, got nil", tt.wantErrMsg) + } + errStr := err.Error() + stderr.String() + if !strings.Contains(errStr, tt.wantErrMsg) { + t.Errorf("error %q does not contain %q", errStr, tt.wantErrMsg) + } + } else { + tt.verify(t, base, stdout.String(), stderr.String(), err) + } + }) + } +} \ No newline at end of file diff --git a/internal/cli/root.go b/internal/cli/root.go index 378a22b..a42f63c 100644 --- a/internal/cli/root.go +++ b/internal/cli/root.go @@ -2,7 +2,10 @@ package cli import ( "bytes" + "fmt" + "path/filepath" + "github.com/ezzek/skill-sync/internal/importer" "github.com/ezzek/skill-sync/internal/models" "github.com/ezzek/skill-sync/internal/tui" "github.com/spf13/cobra" @@ -38,6 +41,30 @@ func NewRootCmd(version string) *cobra.Command { err := verifyCmd.RunE(verifyCmd, nil) return buf.String(), err }, + FindSkillsForImport: func(sources []string) ([]importer.SkillCandidate, error) { + cwd, err := getWd() + if err != nil { + return nil, err + } + destRoot := filepath.Join(cwd, ".agents", "skills") + imp := importer.New(sources, destRoot) + return imp.FindAllSkills() + }, + RunImport: func(candidate importer.SkillCandidate) (string, error) { + if candidate.IsConflict { + return "", importer.ErrConflict + } + cwd, err := getWd() + if err != nil { + return "", err + } + destRoot := filepath.Join(cwd, ".agents", "skills") + imp := importer.New(nil, destRoot) + if err := imp.CopySkill(candidate.SkillID, candidate.SourceDir); err != nil { + return "", err + } + return fmt.Sprintf("Imported %s from %s", candidate.SkillID, candidate.SourceDir), nil + }, } return tui.NewProgram(callbacks).Run() }, @@ -47,6 +74,7 @@ func NewRootCmd(version string) *cobra.Command { cmd.AddCommand(NewConfigCmd()) cmd.AddCommand(NewSyncCmd()) cmd.AddCommand(NewVerifyCmd()) + cmd.AddCommand(NewImportCmd()) return cmd } diff --git a/internal/importer/errors.go b/internal/importer/errors.go new file mode 100644 index 0000000..8c5ed48 --- /dev/null +++ b/internal/importer/errors.go @@ -0,0 +1,12 @@ +package importer + +import "errors" + +// ErrSkillNotFound is returned when no skill matching the requested ID +// is found in any of the configured source directories. +var ErrSkillNotFound = errors.New("skill not found in any configured source") + +// ErrConflict is returned when a skill exists in multiple sources with +// identical version and mtime but different content (true tie — no winner +// can be automatically determined). +var ErrConflict = errors.New("conflict: identical version and mtime in multiple sources") diff --git a/internal/importer/importer.go b/internal/importer/importer.go new file mode 100644 index 0000000..8dc9f15 --- /dev/null +++ b/internal/importer/importer.go @@ -0,0 +1,278 @@ +package importer + +import ( + "errors" + "fmt" + "io/fs" + "os" + "path/filepath" + "sort" + "strings" + "time" + + "github.com/ezzek/skill-sync/internal/mesh" + "github.com/ezzek/skill-sync/internal/models" + "github.com/ezzek/skill-sync/internal/writer" + "gopkg.in/yaml.v3" +) + +// SkillCandidate is a skill directory found during source discovery. +type SkillCandidate struct { + SkillID string // directory name, e.g. "git-expert" + SourceDir string // absolute path to the skill directory + AgentDir string // absolute path to the parent target + Metadata models.SkillMetadata // parsed from SKILL.md frontmatter + Mtime time.Time // mtime of SKILL.md + Hash string // DirHash of SourceDir + IsConflict bool // true if multiple sources are a true tie +} + +// Importer drives import operations for a fixed set of source targets +// and a fixed destination root. +type Importer struct { + Targets []string // tilde-expanded absolute paths + DestRoot string // absolute path to ./.agents/skills/ +} + +// New returns an Importer. +// targets must be tilde-expanded absolute paths. +// destRoot is typically filepath.Join(cwd, ".agents", "skills"). +func New(targets []string, destRoot string) *Importer { + return &Importer{Targets: targets, DestRoot: destRoot} +} + +// FindSkill walks each target's skills/ subdirectory and returns all +// candidates whose directory name matches skillID (case-sensitive). +// Returns an empty slice (not an error) when no match is found. +func (imp *Importer) FindSkill(skillID string) ([]SkillCandidate, error) { + var candidates []SkillCandidate + for _, target := range imp.Targets { + skillDir := filepath.Join(target, "skills", skillID) + mdPath := filepath.Join(skillDir, "SKILL.md") + + info, err := os.Stat(mdPath) + if errors.Is(err, fs.ErrNotExist) { + continue + } + if err != nil { + return nil, fmt.Errorf("stat %s: %w", mdPath, err) + } + + meta, err := readMetadata(mdPath) + if err != nil { + meta = models.SkillMetadata{} // tolerate parse errors + } + + hash, err := DirHash(skillDir) + if err != nil { + return nil, fmt.Errorf("hash %s: %w", skillDir, err) + } + + candidates = append(candidates, SkillCandidate{ + SkillID: skillID, + SourceDir: skillDir, + AgentDir: target, + Metadata: meta, + Mtime: info.ModTime(), + Hash: hash, + }) + } + return candidates, nil +} + +// FindAllSkills walks each target's skills/ subdirectory, groups candidates +// by SkillID, and resolves the winner for each unique skill ID. If a skill +// has a true tie conflict, it returns a candidate with IsConflict=true. +// Results are sorted by SkillID. +func (imp *Importer) FindAllSkills() ([]SkillCandidate, error) { + candidatesByID := make(map[string][]SkillCandidate) + + for _, target := range imp.Targets { + skillsDir := filepath.Join(target, "skills") + entries, err := os.ReadDir(skillsDir) + if errors.Is(err, fs.ErrNotExist) { + continue + } + if err != nil { + return nil, fmt.Errorf("read %s: %w", skillsDir, err) + } + for _, e := range entries { + if !e.IsDir() { + continue + } + skillID := e.Name() + skillDir := filepath.Join(skillsDir, skillID) + mdPath := filepath.Join(skillDir, "SKILL.md") + + info, err := os.Stat(mdPath) + if errors.Is(err, fs.ErrNotExist) { + continue + } + if err != nil { + continue + } + + meta, _ := readMetadata(mdPath) + hash, err := DirHash(skillDir) + if err != nil { + return nil, fmt.Errorf("hash %s: %w", skillDir, err) + } + candidatesByID[skillID] = append(candidatesByID[skillID], SkillCandidate{ + SkillID: skillID, + SourceDir: skillDir, + AgentDir: target, + Metadata: meta, + Mtime: info.ModTime(), + Hash: hash, + }) + } + } + + var results []SkillCandidate + for _, group := range candidatesByID { + winner, conflict, err := Resolve(group) + if conflict { + c := group[0] + c.IsConflict = true + results = append(results, c) + } else if err == nil && winner != nil { + results = append(results, *winner) + } + } + + sort.Slice(results, func(i, j int) bool { + return results[i].SkillID < results[j].SkillID + }) + + return results, nil +} + +// Resolve converts candidates to SkillInstances and delegates winner +// selection to mesh.NewDefaultResolver(). Returns the winning candidate, +// a conflict flag, and any error. +func Resolve(candidates []SkillCandidate) (*SkillCandidate, bool, error) { + if len(candidates) == 0 { + return nil, false, ErrSkillNotFound + } + if len(candidates) == 1 { + c := candidates[0] + return &c, false, nil + } + + instances := make([]models.SkillInstance, len(candidates)) + for i, c := range candidates { + instances[i] = models.SkillInstance{ + ID: c.SkillID, + Path: filepath.Join(c.SourceDir, "SKILL.md"), + TargetDir: c.AgentDir, + Hash: c.Hash, + Mtime: c.Mtime, + Metadata: c.Metadata, + } + } + + winner, conflict, err := mesh.NewDefaultResolver().Resolve(instances) + if err != nil { + return nil, false, err + } + if conflict { + return nil, true, ErrConflict + } + + // Re-map winner back to its SkillCandidate by matching AgentDir. + for i := range candidates { + if candidates[i].AgentDir == winner.TargetDir { + c := candidates[i] + return &c, false, nil + } + } + return nil, false, fmt.Errorf("importer: resolve winner not found in candidates (internal error)") +} + +// CopySkill copies the full skill directory from src into +// filepath.Join(destRoot, skillID) using a transactional strategy: +// 1. Write all files to .tmp/ (each file via writer.AtomicWrite). +// 2. On failure: remove .tmp, return error — existing dir untouched. +// 3. On success: remove stale .bak, rename existing → .bak, rename .tmp → final. +func (imp *Importer) CopySkill(skillID, src string) error { + if err := os.MkdirAll(imp.DestRoot, 0o755); err != nil { + return fmt.Errorf("create dest root: %w", err) + } + + destFinal := filepath.Join(imp.DestRoot, skillID) + destTmp := filepath.Join(imp.DestRoot, skillID+".tmp") + destBak := filepath.Join(imp.DestRoot, skillID+".bak") + + // Ensure a clean tmp before starting. + _ = os.RemoveAll(destTmp) + + // Copy all files into .tmp/ + copyErr := filepath.WalkDir(src, func(path string, d fs.DirEntry, err error) error { + if err != nil { + return err + } + if d.Type()&fs.ModeSymlink != 0 { + return nil // skip symlinks silently + } + rel, err := filepath.Rel(src, path) + if err != nil { + return err + } + tmpDest := filepath.Join(destTmp, rel) + if d.IsDir() { + return os.MkdirAll(tmpDest, 0o755) + } + data, err := os.ReadFile(path) + if err != nil { + return fmt.Errorf("read %s: %w", rel, err) + } + if err := os.MkdirAll(filepath.Dir(tmpDest), 0o755); err != nil { + return err + } + return writer.AtomicWrite(tmpDest, data) + }) + if copyErr != nil { + _ = os.RemoveAll(destTmp) // cleanup staging dir on failure + return fmt.Errorf("copy skill %s: %w", skillID, copyErr) + } + + // Commit: rotate backup, rename tmp → final. + _ = os.RemoveAll(destBak) + if _, err := os.Stat(destFinal); err == nil { + if err := os.Rename(destFinal, destBak); err != nil { + _ = os.RemoveAll(destTmp) + return fmt.Errorf("backup existing skill: %w", err) + } + } + if err := os.Rename(destTmp, destFinal); err != nil { + // Attempt to restore backup. + if _, statErr := os.Stat(destBak); statErr == nil { + _ = os.Rename(destBak, destFinal) + } + return fmt.Errorf("commit skill %s: %w", skillID, err) + } + return nil +} + +// readMetadata parses the YAML frontmatter from a SKILL.md file. +// Returns zero-value SkillMetadata on any parse error. +func readMetadata(mdPath string) (models.SkillMetadata, error) { + data, err := os.ReadFile(mdPath) + if err != nil { + return models.SkillMetadata{}, err + } + s := string(data) + // Require a proper frontmatter fence (Unix or Windows line endings). + if !strings.HasPrefix(s, "---\n") && !strings.HasPrefix(s, "---\r\n") { + return models.SkillMetadata{}, nil + } + // Find the closing fence. + endIndex := strings.Index(s[3:], "\n---") + if endIndex == -1 { + return models.SkillMetadata{}, nil + } + yamlStr := s[3 : endIndex+3] + var meta models.SkillMetadata + _ = yaml.Unmarshal([]byte(yamlStr), &meta) + return meta, nil +} diff --git a/internal/importer/importer_test.go b/internal/importer/importer_test.go new file mode 100644 index 0000000..848ad85 --- /dev/null +++ b/internal/importer/importer_test.go @@ -0,0 +1,446 @@ +package importer + +import ( + "errors" + "os" + "path/filepath" + "runtime" + "sort" + "testing" + "time" + + "github.com/ezzek/skill-sync/internal/models" +) + +// makeSkillDir creates a minimal skill directory with a SKILL.md at +// /skills//SKILL.md. Returns the skill dir path. +func makeSkillDir(t *testing.T, targetDir, skillID, frontmatter string, extraFiles map[string]string) string { + t.Helper() + skillDir := filepath.Join(targetDir, "skills", skillID) + if err := os.MkdirAll(skillDir, 0o755); err != nil { + t.Fatalf("MkdirAll %s: %v", skillDir, err) + } + mdContent := "---\n" + frontmatter + "\n---\n# " + skillID + writeFile(t, skillDir, "SKILL.md", mdContent) + for rel, content := range extraFiles { + writeFile(t, skillDir, rel, content) + } + return skillDir +} + +// ── FindSkill ──────────────────────────────────────────────────────────────── + +func TestFindSkill_ReturnsFromMultipleTargets(t *testing.T) { + t1 := t.TempDir() + t2 := t.TempDir() + makeSkillDir(t, t1, "git-expert", "version: 1\nname: Git Expert\n", nil) + makeSkillDir(t, t2, "git-expert", "version: 2\nname: Git Expert\n", nil) + + imp := New([]string{t1, t2}, t.TempDir()) + candidates, err := imp.FindSkill("git-expert") + if err != nil { + t.Fatalf("FindSkill: %v", err) + } + if len(candidates) != 2 { + t.Errorf("got %d candidates, want 2", len(candidates)) + } +} + +func TestFindSkill_ReturnsEmptyForUnknownID(t *testing.T) { + target := t.TempDir() + makeSkillDir(t, target, "docker-dev", "version: 1\n", nil) + + imp := New([]string{target}, t.TempDir()) + candidates, err := imp.FindSkill("nonexistent") + if err != nil { + t.Fatalf("FindSkill: %v", err) + } + if len(candidates) != 0 { + t.Errorf("got %d candidates, want 0", len(candidates)) + } +} + +func TestFindSkill_SkipsMissingSkillsDir(t *testing.T) { + existing := t.TempDir() + missing := t.TempDir() // create dir but don't populate skills/ subdir + makeSkillDir(t, existing, "my-skill", "version: 1\n", nil) + + imp := New([]string{existing, missing}, t.TempDir()) + candidates, err := imp.FindSkill("my-skill") + if err != nil { + t.Fatalf("FindSkill: %v", err) + } + if len(candidates) != 1 { + t.Errorf("got %d candidates, want 1 (missing target should be skipped)", len(candidates)) + } +} + +func TestFindSkill_ReadsFrontmatter(t *testing.T) { + target := t.TempDir() + makeSkillDir(t, target, "k8s-ops", "version: 1.5\nname: K8s Ops\ndescription: Kubernetes skill\n", nil) + + imp := New([]string{target}, t.TempDir()) + candidates, err := imp.FindSkill("k8s-ops") + if err != nil { + t.Fatalf("FindSkill: %v", err) + } + if len(candidates) != 1 { + t.Fatalf("got %d candidates, want 1", len(candidates)) + } + meta := candidates[0].Metadata + if meta.Version != 1.5 { + t.Errorf("Version: got %v, want 1.5", meta.Version) + } + if meta.Name != "K8s Ops" { + t.Errorf("Name: got %q, want %q", meta.Name, "K8s Ops") + } + if meta.Description != "Kubernetes skill" { + t.Errorf("Description: got %q, want %q", meta.Description, "Kubernetes skill") + } +} + +// ── Resolve ────────────────────────────────────────────────────────────────── + +func TestResolve_HighestVersionWins(t *testing.T) { + t1 := t.TempDir() + t2 := t.TempDir() + makeSkillDir(t, t1, "my-skill", "version: 1\n", nil) + makeSkillDir(t, t2, "my-skill", "version: 2\n", nil) + + imp := New([]string{t1, t2}, t.TempDir()) + candidates, _ := imp.FindSkill("my-skill") + + winner, conflict, err := Resolve(candidates) + if err != nil { + t.Fatalf("Resolve: %v", err) + } + if conflict { + t.Fatal("unexpected conflict") + } + if winner.Metadata.Version != 2 { + t.Errorf("winner version: got %v, want 2", winner.Metadata.Version) + } +} + +func TestResolve_NewestMtimeWinsOnVersionTie(t *testing.T) { + t1 := t.TempDir() + t2 := t.TempDir() + makeSkillDir(t, t1, "my-skill", "version: 1\nname: old\n", nil) + makeSkillDir(t, t2, "my-skill", "version: 1\nname: new\n", nil) + + // Force t2's SKILL.md to have a newer mtime. + mdPath := filepath.Join(t2, "skills", "my-skill", "SKILL.md") + future := time.Now().Add(time.Hour) + if err := os.Chtimes(mdPath, future, future); err != nil { + t.Fatalf("Chtimes: %v", err) + } + + imp := New([]string{t1, t2}, t.TempDir()) + candidates, _ := imp.FindSkill("my-skill") + + winner, conflict, err := Resolve(candidates) + if err != nil { + t.Fatalf("Resolve: %v", err) + } + if conflict { + t.Fatal("unexpected conflict") + } + if winner.AgentDir != t2 { + t.Errorf("winner AgentDir: got %q, want %q", winner.AgentDir, t2) + } +} + +func TestResolve_ConflictOnTie(t *testing.T) { + t1 := t.TempDir() + t2 := t.TempDir() + // Same version, same mtime (we'll force both), different content → true tie. + makeSkillDir(t, t1, "my-skill", "version: 1\nname: alpha\n", nil) + makeSkillDir(t, t2, "my-skill", "version: 1\nname: beta\n", nil) + + // Force identical mtime on both SKILL.md files. + fixed := time.Date(2025, 1, 1, 0, 0, 0, 0, time.UTC) + for _, base := range []string{t1, t2} { + mdPath := filepath.Join(base, "skills", "my-skill", "SKILL.md") + if err := os.Chtimes(mdPath, fixed, fixed); err != nil { + t.Fatalf("Chtimes: %v", err) + } + } + + imp := New([]string{t1, t2}, t.TempDir()) + candidates, _ := imp.FindSkill("my-skill") + + _, conflict, err := Resolve(candidates) + if !conflict { + t.Error("expected conflict=true") + } + if !errors.Is(err, ErrConflict) { + t.Errorf("expected ErrConflict, got %v", err) + } +} + +func TestResolve_ErrSkillNotFoundOnEmpty(t *testing.T) { + _, _, err := Resolve(nil) + if !errors.Is(err, ErrSkillNotFound) { + t.Errorf("expected ErrSkillNotFound, got %v", err) + } +} + +// ── FindAllSkills ───────────────────────────────────────────────────────────── + +func TestFindAllSkills_ReturnsOneEntryPerSkillID(t *testing.T) { + t1 := t.TempDir() + t2 := t.TempDir() + makeSkillDir(t, t1, "git-expert", "version: 2\n", nil) + makeSkillDir(t, t2, "git-expert", "version: 1\n", nil) + makeSkillDir(t, t1, "docker-dev", "version: 1\n", nil) + + imp := New([]string{t1, t2}, t.TempDir()) + results, err := imp.FindAllSkills() + if err != nil { + t.Fatalf("FindAllSkills: %v", err) + } + if len(results) != 2 { + t.Errorf("got %d results, want 2", len(results)) + } +} + +func TestFindAllSkills_MarksConflict(t *testing.T) { + t1 := t.TempDir() + t2 := t.TempDir() + // Different content, same version, same forced mtime → conflict. + makeSkillDir(t, t1, "my-skill", "version: 1\nname: alpha\n", nil) + makeSkillDir(t, t2, "my-skill", "version: 1\nname: beta\n", nil) + + fixed := time.Date(2025, 1, 1, 0, 0, 0, 0, time.UTC) + for _, base := range []string{t1, t2} { + mdPath := filepath.Join(base, "skills", "my-skill", "SKILL.md") + if err := os.Chtimes(mdPath, fixed, fixed); err != nil { + t.Fatalf("Chtimes: %v", err) + } + } + + imp := New([]string{t1, t2}, t.TempDir()) + results, err := imp.FindAllSkills() + if err != nil { + t.Fatalf("FindAllSkills: %v", err) + } + if len(results) != 1 { + t.Fatalf("got %d results, want 1", len(results)) + } + if !results[0].IsConflict { + t.Error("expected IsConflict=true") + } +} + +func TestFindAllSkills_SortedBySkillID(t *testing.T) { + target := t.TempDir() + makeSkillDir(t, target, "zebra", "version: 1\n", nil) + makeSkillDir(t, target, "alpha", "version: 1\n", nil) + makeSkillDir(t, target, "mango", "version: 1\n", nil) + + imp := New([]string{target}, t.TempDir()) + results, err := imp.FindAllSkills() + if err != nil { + t.Fatalf("FindAllSkills: %v", err) + } + + ids := make([]string, len(results)) + for i, r := range results { + ids[i] = r.SkillID + } + if !sort.StringsAreSorted(ids) { + t.Errorf("results not sorted by SkillID: %v", ids) + } +} + +func TestFindAllSkills_SkipsMissingSkillsDir(t *testing.T) { + existing := t.TempDir() + empty := t.TempDir() + makeSkillDir(t, existing, "my-skill", "version: 1\n", nil) + + imp := New([]string{existing, empty}, t.TempDir()) + results, err := imp.FindAllSkills() + if err != nil { + t.Fatalf("FindAllSkills: %v", err) + } + if len(results) != 1 { + t.Errorf("got %d, want 1", len(results)) + } +} + +// ── CopySkill ───────────────────────────────────────────────────────────────── + +func TestCopySkill_CopiesFullTree(t *testing.T) { + src := t.TempDir() + writeFile(t, src, "SKILL.md", "---\nversion: 1\n---") + writeFile(t, src, "examples/demo.sh", "#!/bin/bash") + writeFile(t, src, "docs/README.md", "# Docs") + + dest := t.TempDir() + imp := New(nil, dest) + if err := imp.CopySkill("my-skill", src); err != nil { + t.Fatalf("CopySkill: %v", err) + } + + for _, rel := range []string{"SKILL.md", "examples/demo.sh", "docs/README.md"} { + p := filepath.Join(dest, "my-skill", rel) + if _, err := os.Stat(p); err != nil { + t.Errorf("expected file %s to exist: %v", rel, err) + } + } +} + +func TestCopySkill_CreatesBakOnOverwrite(t *testing.T) { + src := t.TempDir() + writeFile(t, src, "SKILL.md", "new content") + + dest := t.TempDir() + // Pre-populate the destination. + writeFile(t, dest, "my-skill/SKILL.md", "old content") + + imp := New(nil, dest) + if err := imp.CopySkill("my-skill", src); err != nil { + t.Fatalf("CopySkill: %v", err) + } + + bakPath := filepath.Join(dest, "my-skill.bak", "SKILL.md") + if _, err := os.Stat(bakPath); err != nil { + t.Errorf("expected backup at %s: %v", bakPath, err) + } +} + +func TestCopySkill_RemovesStaleBak(t *testing.T) { + src := t.TempDir() + writeFile(t, src, "SKILL.md", "content") + + dest := t.TempDir() + writeFile(t, dest, "my-skill/SKILL.md", "existing") + // Pre-create a stale .bak + writeFile(t, dest, "my-skill.bak/OLD.md", "stale") + + imp := New(nil, dest) + if err := imp.CopySkill("my-skill", src); err != nil { + t.Fatalf("CopySkill: %v", err) + } + + // Stale file must be gone; new .bak should have been created from existing. + oldFile := filepath.Join(dest, "my-skill.bak", "OLD.md") + if _, err := os.Stat(oldFile); err == nil { + t.Error("stale .bak file should have been removed") + } + + newBak := filepath.Join(dest, "my-skill.bak", "SKILL.md") + if _, err := os.Stat(newBak); err != nil { + t.Errorf("expected new .bak at %s: %v", newBak, err) + } +} + +func TestCopySkill_Idempotent(t *testing.T) { + src := t.TempDir() + writeFile(t, src, "SKILL.md", "---\nversion: 1\n---\n# skill") + + dest := t.TempDir() + imp := New(nil, dest) + + if err := imp.CopySkill("my-skill", src); err != nil { + t.Fatalf("first CopySkill: %v", err) + } + if err := imp.CopySkill("my-skill", src); err != nil { + t.Fatalf("second CopySkill: %v", err) + } + + // Content must be the same after two identical copies. + got, err := os.ReadFile(filepath.Join(dest, "my-skill", "SKILL.md")) + if err != nil { + t.Fatalf("ReadFile: %v", err) + } + if string(got) != "---\nversion: 1\n---\n# skill" { + t.Errorf("unexpected content after idempotent copy: %q", got) + } +} + +func TestCopySkill_LeavesExistingUntouchedOnFailure(t *testing.T) { + if runtime.GOOS == "windows" { + t.Skip("read-only file injection is unreliable on Windows") + } + + src := t.TempDir() + writeFile(t, src, "SKILL.md", "new") + writeFile(t, src, "extra.md", "extra") + + dest := t.TempDir() + // Pre-populate the destination with known content. + writeFile(t, dest, "my-skill/SKILL.md", "original") + + // Make the destination skills root read-only so .tmp creation fails. + destRoot := filepath.Join(dest) + if err := os.Chmod(destRoot, 0o555); err != nil { + t.Fatalf("Chmod: %v", err) + } + t.Cleanup(func() { os.Chmod(destRoot, 0o755) }) + + imp := New(nil, dest) + err := imp.CopySkill("my-skill", src) + if err == nil { + t.Fatal("expected error when dest root is read-only") + } + + // Original must be untouched. + original, readErr := os.ReadFile(filepath.Join(dest, "my-skill", "SKILL.md")) + if readErr != nil { + t.Fatalf("ReadFile original: %v", readErr) + } + if string(original) != "original" { + t.Errorf("original content changed: %q", original) + } +} + +// ── readMetadata ───────────────────────────────────────────────────────────── + +func TestReadMetadata(t *testing.T) { + tests := []struct { + name string + content string + want models.SkillMetadata + }{ + { + name: "valid frontmatter", + content: "---\nversion: 1.5\nname: My Skill\ndescription: A skill\n---\n# body", + want: models.SkillMetadata{Version: 1.5, Name: "My Skill", Description: "A skill"}, + }, + { + name: "no frontmatter", + content: "# Just a heading", + want: models.SkillMetadata{}, + }, + { + name: "unclosed frontmatter", + content: "---\nversion: 1\n", + want: models.SkillMetadata{}, + }, + { + name: "missing fields default to zero", + content: "---\nversion: 2\n---\n# body", + want: models.SkillMetadata{Version: 2}, + }, + { + name: "crlf line endings", + content: "---\r\nversion: 1\r\nname: CRLF Skill\r\n---\r\n# body", + want: models.SkillMetadata{Version: 1, Name: "CRLF Skill"}, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + dir := t.TempDir() + mdPath := filepath.Join(dir, "SKILL.md") + if err := os.WriteFile(mdPath, []byte(tt.content), 0o644); err != nil { + t.Fatalf("WriteFile: %v", err) + } + got, _ := readMetadata(mdPath) + if got != tt.want { + t.Errorf("got %+v, want %+v", got, tt.want) + } + }) + } +} diff --git a/internal/importer/manifest.go b/internal/importer/manifest.go new file mode 100644 index 0000000..5537116 --- /dev/null +++ b/internal/importer/manifest.go @@ -0,0 +1,64 @@ +package importer + +import ( + "crypto/sha256" + "fmt" + "io/fs" + "os" + "path/filepath" + "sort" +) + +// DirHash computes a deterministic, cross-platform SHA-256 over all regular +// files in dir. Symlinks are skipped. +// +// Algorithm: +// 1. Walk dir recursively; collect relative paths of all regular files. +// 2. Normalize paths to forward slashes (cross-platform determinism). +// 3. Sort paths lexicographically. +// 4. For each path: feed "\n" + file bytes into the hasher. +// 5. Return hex-encoded SHA-256 of the accumulated digest. +func DirHash(dir string) (string, error) { + type entry struct { + rel string + absPath string + } + var entries []entry + + err := filepath.WalkDir(dir, func(path string, d fs.DirEntry, err error) error { + if err != nil { + return err + } + // Skip symlinks and directories; only hash regular files. + if d.Type()&fs.ModeSymlink != 0 || d.IsDir() { + return nil + } + rel, err := filepath.Rel(dir, path) + if err != nil { + return err + } + entries = append(entries, entry{ + rel: filepath.ToSlash(rel), // cross-platform path normalisation + absPath: path, + }) + return nil + }) + if err != nil { + return "", fmt.Errorf("DirHash walk %s: %w", dir, err) + } + + sort.Slice(entries, func(i, j int) bool { + return entries[i].rel < entries[j].rel + }) + + h := sha256.New() + for _, e := range entries { + data, err := os.ReadFile(e.absPath) + if err != nil { + return "", fmt.Errorf("DirHash read %s: %w", e.rel, err) + } + h.Write([]byte(e.rel + "\n")) + h.Write(data) + } + return fmt.Sprintf("%x", h.Sum(nil)), nil +} diff --git a/internal/importer/manifest_test.go b/internal/importer/manifest_test.go new file mode 100644 index 0000000..031d878 --- /dev/null +++ b/internal/importer/manifest_test.go @@ -0,0 +1,148 @@ +package importer + +import ( + "os" + "path/filepath" + "runtime" + "testing" +) + +// writeFile is a helper to create a file with given content inside a base dir. +func writeFile(t *testing.T, base, rel, content string) { + t.Helper() + full := filepath.Join(base, rel) + if err := os.MkdirAll(filepath.Dir(full), 0o755); err != nil { + t.Fatalf("MkdirAll: %v", err) + } + if err := os.WriteFile(full, []byte(content), 0o644); err != nil { + t.Fatalf("WriteFile %s: %v", rel, err) + } +} + +func TestDirHash_Deterministic(t *testing.T) { + dir := t.TempDir() + writeFile(t, dir, "SKILL.md", "---\nversion: 1\n---\n# content") + writeFile(t, dir, "README.md", "hello") + + h1, err := DirHash(dir) + if err != nil { + t.Fatalf("first DirHash: %v", err) + } + h2, err := DirHash(dir) + if err != nil { + t.Fatalf("second DirHash: %v", err) + } + if h1 != h2 { + t.Errorf("DirHash not deterministic: %q != %q", h1, h2) + } +} + +func TestDirHash_OrderIndependent(t *testing.T) { + // Two dirs with identical files but written in different order + // must produce the same hash. + dirA := t.TempDir() + writeFile(t, dirA, "a.txt", "alpha") + writeFile(t, dirA, "b.txt", "beta") + writeFile(t, dirA, "sub/c.txt", "gamma") + + dirB := t.TempDir() + writeFile(t, dirB, "sub/c.txt", "gamma") + writeFile(t, dirB, "b.txt", "beta") + writeFile(t, dirB, "a.txt", "alpha") + + hA, err := DirHash(dirA) + if err != nil { + t.Fatalf("DirHash dirA: %v", err) + } + hB, err := DirHash(dirB) + if err != nil { + t.Fatalf("DirHash dirB: %v", err) + } + if hA != hB { + t.Errorf("DirHash not order-independent: %q != %q", hA, hB) + } +} + +func TestDirHash_ChangesOnContentChange(t *testing.T) { + dir := t.TempDir() + writeFile(t, dir, "SKILL.md", "original") + + h1, err := DirHash(dir) + if err != nil { + t.Fatalf("DirHash before change: %v", err) + } + + writeFile(t, dir, "SKILL.md", "modified") + + h2, err := DirHash(dir) + if err != nil { + t.Fatalf("DirHash after change: %v", err) + } + + if h1 == h2 { + t.Error("DirHash should change when file content changes") + } +} + +func TestDirHash_ChangesOnFileAdded(t *testing.T) { + dir := t.TempDir() + writeFile(t, dir, "SKILL.md", "content") + + h1, err := DirHash(dir) + if err != nil { + t.Fatalf("DirHash before add: %v", err) + } + + writeFile(t, dir, "extra.md", "new file") + + h2, err := DirHash(dir) + if err != nil { + t.Fatalf("DirHash after add: %v", err) + } + + if h1 == h2 { + t.Error("DirHash should change when a file is added") + } +} + +func TestDirHash_SkipsSymlinks(t *testing.T) { + if runtime.GOOS == "windows" { + t.Skip("symlink creation requires elevated privileges on Windows") + } + + dir := t.TempDir() + writeFile(t, dir, "SKILL.md", "content") + + // Create a symlink inside the dir; DirHash must not error and must + // produce the same result as hashing without the symlink. + h1, err := DirHash(dir) + if err != nil { + t.Fatalf("DirHash without symlink: %v", err) + } + + symlinkPath := filepath.Join(dir, "link.md") + target := filepath.Join(dir, "SKILL.md") + if err := os.Symlink(target, symlinkPath); err != nil { + t.Fatalf("Symlink: %v", err) + } + + h2, err := DirHash(dir) + if err != nil { + t.Fatalf("DirHash with symlink: %v", err) + } + + if h1 != h2 { + t.Error("DirHash should skip symlinks and produce same hash") + } +} + +func TestDirHash_EmptyDir(t *testing.T) { + dir := t.TempDir() + h, err := DirHash(dir) + if err != nil { + t.Fatalf("DirHash empty dir: %v", err) + } + if h == "" { + t.Error("DirHash should return a non-empty string for an empty dir") + } +} diff --git a/internal/models/skill.go b/internal/models/skill.go index 9eac9b7..0a9a465 100644 --- a/internal/models/skill.go +++ b/internal/models/skill.go @@ -7,8 +7,9 @@ import "time" // cleanly; plain integers ("version: 2") still parse as 2.0 and compare // correctly against decimals. type SkillMetadata struct { - Version float64 `yaml:"version"` - Name string `yaml:"name"` + Version float64 `yaml:"version"` + Name string `yaml:"name"` + Description string `yaml:"description"` } // SkillInstance represents a single skill file found in a target directory. diff --git a/internal/models/skill_test.go b/internal/models/skill_test.go new file mode 100644 index 0000000..d153f75 --- /dev/null +++ b/internal/models/skill_test.go @@ -0,0 +1,93 @@ +package models + +import ( + "testing" + + "gopkg.in/yaml.v3" +) + +func TestSkillMetadata_Unmarshal(t *testing.T) { + tests := []struct { + name string + input string + want SkillMetadata + }{ + { + name: "all fields present", + input: ` +version: 1.2 +name: Git Expert +description: Standards for Git branching and commits +`, + want: SkillMetadata{ + Version: 1.2, + Name: "Git Expert", + Description: "Standards for Git branching and commits", + }, + }, + { + name: "description absent defaults to empty string", + input: ` +version: 2 +name: Docker Dev +`, + want: SkillMetadata{ + Version: 2, + Name: "Docker Dev", + Description: "", + }, + }, + { + name: "all fields absent give zero values", + input: `{}`, + want: SkillMetadata{}, + }, + { + name: "integer version parses as float64", + input: ` +version: 3 +name: K8s Ops +description: Kubernetes operations skill +`, + want: SkillMetadata{ + Version: 3.0, + Name: "K8s Ops", + Description: "Kubernetes operations skill", + }, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + var got SkillMetadata + if err := yaml.Unmarshal([]byte(tt.input), &got); err != nil { + t.Fatalf("yaml.Unmarshal error: %v", err) + } + if got != tt.want { + t.Errorf("got %+v, want %+v", got, tt.want) + } + }) + } +} + +func TestSkillMetadata_RoundTrip(t *testing.T) { + original := SkillMetadata{ + Version: 1.5, + Name: "My Skill", + Description: "A test description", + } + + data, err := yaml.Marshal(original) + if err != nil { + t.Fatalf("yaml.Marshal error: %v", err) + } + + var got SkillMetadata + if err := yaml.Unmarshal(data, &got); err != nil { + t.Fatalf("yaml.Unmarshal error: %v", err) + } + + if got != original { + t.Errorf("round-trip failed: got %+v, want %+v", got, original) + } +} diff --git a/internal/tui/import_skill_view/model.go b/internal/tui/import_skill_view/model.go new file mode 100644 index 0000000..aae8901 --- /dev/null +++ b/internal/tui/import_skill_view/model.go @@ -0,0 +1,110 @@ +package import_skill_view + +import ( + "fmt" + "strings" + + tea "github.com/charmbracelet/bubbletea" + "github.com/ezzek/skill-sync/internal/importer" + "github.com/ezzek/skill-sync/internal/tui/styles" +) + +// ConfirmedMsg carries the skill the user selected for import. +type ConfirmedMsg struct{ Candidate importer.SkillCandidate } + +// BackMsg signals the root model to return to import_source_view. +type BackMsg struct{} + +const maxVisible = 15 +const descMaxLen = 60 + +// Model is a single-selection cursor-based list of skill candidates. +type Model struct { + candidates []importer.SkillCandidate + cursor int + scroll int +} + +// New builds the model from a list of candidates. +// Callers must ensure len(candidates) > 0; use output_view for the empty case. +func New(candidates []importer.SkillCandidate) Model { + return Model{candidates: candidates} +} + +func (m Model) Init() tea.Cmd { return nil } + +func (m Model) Update(msg tea.Msg) (tea.Model, tea.Cmd) { + switch msg := msg.(type) { + case tea.KeyMsg: + switch msg.String() { + case "up", "k": + if m.cursor > 0 { + m.cursor-- + if m.cursor < m.scroll { + m.scroll = m.cursor + } + } + case "down", "j": + if m.cursor < len(m.candidates)-1 { + m.cursor++ + if m.cursor >= m.scroll+maxVisible { + m.scroll = m.cursor - maxVisible + 1 + } + } + case "enter": + c := m.candidates[m.cursor] + return m, func() tea.Msg { return ConfirmedMsg{Candidate: c} } + case "esc": + return m, func() tea.Msg { return BackMsg{} } + case "ctrl+c": + return m, tea.Quit + } + } + return m, nil +} + +func (m Model) View() string { + var s strings.Builder + s.WriteString(styles.TitleStyle.Render("Import — Select Skill")) + s.WriteString(fmt.Sprintf("\n\n%d skill(s) found — choose one to import:\n", len(m.candidates))) + + n := len(m.candidates) + end := m.scroll + maxVisible + if end > n { + end = n + } + + if m.scroll > 0 { + s.WriteString(styles.SubtleStyle.Render(fmt.Sprintf(" ↑ %d more above", m.scroll)) + "\n") + } + for i := m.scroll; i < end; i++ { + c := m.candidates[i] + cursor := " " + if m.cursor == i { + cursor = "→ " + } + name := c.Metadata.Name + if name == "" { + name = c.SkillID + } + desc := c.Metadata.Description + if c.IsConflict { + desc = "[CONFLICT] " + desc + } + if len([]rune(desc)) > descMaxLen { + desc = string([]rune(desc)[:descMaxLen]) + "…" + } + line := fmt.Sprintf("%s%-24s %-28s %s", cursor, c.SkillID, name, desc) + if m.cursor == i { + s.WriteString(styles.ListItemSelectedStyle.Render(line) + "\n") + } else { + s.WriteString(styles.ListItemStyle.Render(line) + "\n") + } + } + if end < n { + s.WriteString(styles.SubtleStyle.Render(fmt.Sprintf(" ↓ %d more below", n-end)) + "\n") + } + + s.WriteString(styles.HelpStyle.Render("\n[↑/↓] navigate • [enter] import • [esc] back")) + return s.String() +} diff --git a/internal/tui/import_skill_view/model_test.go b/internal/tui/import_skill_view/model_test.go new file mode 100644 index 0000000..fd0944f --- /dev/null +++ b/internal/tui/import_skill_view/model_test.go @@ -0,0 +1,214 @@ +package import_skill_view + +import ( + "bytes" + "io" + "strings" + "testing" + "time" + + tea "github.com/charmbracelet/bubbletea" + "github.com/charmbracelet/x/exp/teatest" + "github.com/ezzek/skill-sync/internal/importer" + "github.com/ezzek/skill-sync/internal/models" +) + +func makeCandidate(skillID, name, desc string, isConflict bool) importer.SkillCandidate { + return importer.SkillCandidate{ + SkillID: skillID, + SourceDir: "/fake/" + skillID, + AgentDir: "/fake", + IsConflict: isConflict, + Metadata: models.SkillMetadata{ + Version: 1, + Name: name, + Description: desc, + }, + } +} + +var testCandidates = []importer.SkillCandidate{ + makeCandidate("docker-dev", "Docker Dev", "Docker workflows", false), + makeCandidate("git-expert", "Git Expert", "Git branching standards", false), + makeCandidate("k8s-ops", "K8s Ops", "Kubernetes operations", false), +} + +func TestImportSkillView_InitialCursorOnFirst(t *testing.T) { + m := New(testCandidates) + if m.cursor != 0 { + t.Errorf("initial cursor: got %d, want 0", m.cursor) + } +} + +func TestImportSkillView_NavigateDown(t *testing.T) { + m := New(testCandidates) + m2, _ := m.Update(tea.KeyMsg{Type: tea.KeyDown}) + if m2.(Model).cursor != 1 { + t.Errorf("cursor after down: got %d, want 1", m2.(Model).cursor) + } +} + +func TestImportSkillView_NavigateUp(t *testing.T) { + m := New(testCandidates) + m2, _ := m.Update(tea.KeyMsg{Type: tea.KeyDown}) + m3, _ := m2.(Model).Update(tea.KeyMsg{Type: tea.KeyUp}) + if m3.(Model).cursor != 0 { + t.Errorf("cursor after down+up: got %d, want 0", m3.(Model).cursor) + } +} + +func TestImportSkillView_CursorDoesNotGoAboveZero(t *testing.T) { + m := New(testCandidates) + m2, _ := m.Update(tea.KeyMsg{Type: tea.KeyUp}) + if m2.(Model).cursor != 0 { + t.Errorf("cursor should stay at 0, got %d", m2.(Model).cursor) + } +} + +func TestImportSkillView_CursorDoesNotGoBeyondLast(t *testing.T) { + m := New(testCandidates) + var cur tea.Model = m + for i := 0; i < 10; i++ { + cur, _ = cur.Update(tea.KeyMsg{Type: tea.KeyDown}) + } + if cur.(Model).cursor != len(testCandidates)-1 { + t.Errorf("cursor should be at last item, got %d", cur.(Model).cursor) + } +} + +func TestImportSkillView_JKNavigation(t *testing.T) { + m := New(testCandidates) + m2, _ := m.Update(tea.KeyMsg{Type: tea.KeyRunes, Runes: []rune("j")}) + if m2.(Model).cursor != 1 { + t.Errorf("j key: cursor should be 1, got %d", m2.(Model).cursor) + } + m3, _ := m2.(Model).Update(tea.KeyMsg{Type: tea.KeyRunes, Runes: []rune("k")}) + if m3.(Model).cursor != 0 { + t.Errorf("k key: cursor should be 0, got %d", m3.(Model).cursor) + } +} + +func TestImportSkillView_EnterEmitsConfirmedMsgWithSelectedCandidate(t *testing.T) { + m := New(testCandidates) + // Move to second candidate + m2, _ := m.Update(tea.KeyMsg{Type: tea.KeyDown}) + + _, cmd := m2.(Model).Update(tea.KeyMsg{Type: tea.KeyEnter}) + if cmd == nil { + t.Fatal("expected command on enter") + } + msg := cmd() + confirmed, ok := msg.(ConfirmedMsg) + if !ok { + t.Fatalf("expected ConfirmedMsg, got %T", msg) + } + if confirmed.Candidate.SkillID != "git-expert" { + t.Errorf("expected 'git-expert', got %q", confirmed.Candidate.SkillID) + } +} + +func TestImportSkillView_EscEmitsBackMsg(t *testing.T) { + m := New(testCandidates) + _, cmd := m.Update(tea.KeyMsg{Type: tea.KeyEsc}) + if cmd == nil { + t.Fatal("expected command on esc") + } + if _, ok := cmd().(BackMsg); !ok { + t.Errorf("expected BackMsg on esc, got %T", cmd()) + } +} + +func TestImportSkillView_CtrlCQuits(t *testing.T) { + m := New(testCandidates) + _, cmd := m.Update(tea.KeyMsg{Type: tea.KeyCtrlC}) + if cmd == nil { + t.Fatal("expected command on ctrl+c") + } + if cmd() != tea.Quit() { + t.Errorf("expected tea.Quit on ctrl+c") + } +} + +func TestImportSkillView_ViewShowsCursorMarker(t *testing.T) { + m := New(testCandidates) + view := m.View() + if !strings.Contains(view, "→") { + t.Error("view should contain → cursor marker") + } +} + +func TestImportSkillView_ViewShowsSkillIDs(t *testing.T) { + m := New(testCandidates) + view := m.View() + for _, c := range testCandidates { + if !strings.Contains(view, c.SkillID) { + t.Errorf("view should contain skillID %q", c.SkillID) + } + } +} + +func TestImportSkillView_ViewShowsConflictPrefix(t *testing.T) { + candidates := []importer.SkillCandidate{ + makeCandidate("conflict-skill", "Conflict", "some desc", true), + makeCandidate("normal-skill", "Normal", "other desc", false), + } + m := New(candidates) + view := m.View() + if !strings.Contains(view, "[CONFLICT]") { + t.Error("view should contain [CONFLICT] for conflicting skill") + } + if strings.Count(view, "[CONFLICT]") != 1 { + t.Error("only one skill should have [CONFLICT] prefix") + } +} + +func TestImportSkillView_ViewFallsBackToSkillIDWhenNameEmpty(t *testing.T) { + candidates := []importer.SkillCandidate{ + makeCandidate("unnamed-skill", "", "desc", false), + } + m := New(candidates) + view := m.View() + // SkillID should appear as the name column fallback + if strings.Count(view, "unnamed-skill") < 2 { + t.Error("unnamed-skill should appear at least twice (skillID col + name col fallback)") + } +} + +func TestImportSkillView_DescriptionTruncated(t *testing.T) { + longDesc := strings.Repeat("x", 100) + candidates := []importer.SkillCandidate{ + makeCandidate("long-skill", "Long", longDesc, false), + } + m := New(candidates) + view := m.View() + // Full 100-char description must not appear verbatim + if strings.Contains(view, longDesc) { + t.Error("description should be truncated in the view") + } + // Truncation ellipsis should be present + if !strings.Contains(view, "…") { + t.Error("view should contain … for truncated description") + } +} + +func TestImportSkillView_Teatest(t *testing.T) { + m := New(testCandidates) + tm := teatest.NewTestModel(t, m, teatest.WithInitialTermSize(120, 24)) + + if err := tm.Quit(); err != nil { + t.Fatal(err) + } + tm.WaitFinished(t, teatest.WithFinalTimeout(time.Second)) + + out, err := io.ReadAll(tm.Output()) + if err != nil { + t.Fatal(err) + } + + if !bytes.Contains(out, []byte("docker-dev")) { + t.Errorf("output should contain 'docker-dev'") + } + if !bytes.Contains(out, []byte("Import")) { + t.Errorf("output should contain 'Import'") + } +} diff --git a/internal/tui/import_source_view/model.go b/internal/tui/import_source_view/model.go new file mode 100644 index 0000000..4f784ab --- /dev/null +++ b/internal/tui/import_source_view/model.go @@ -0,0 +1,81 @@ +package import_source_view + +import ( + "path/filepath" + "strings" + + tea "github.com/charmbracelet/bubbletea" + "github.com/ezzek/skill-sync/internal/tui/shared_toggle" + "github.com/ezzek/skill-sync/internal/tui/styles" +) + +// ConfirmedMsg carries the subset of source directories the user kept checked. +type ConfirmedMsg struct{ Sources []string } + +// BackMsg signals the root model to return to the main menu. +type BackMsg struct{} + +// Model wraps shared_toggle for agent-source multi-selection. +type Model struct { + toggle shared_toggle.Model + sources []string // parallel to toggle items; index → full target path +} + +// New builds the model from a list of resolved target directories. +// All sources start checked. +func New(targets []string) Model { + items := make([]shared_toggle.Item, len(targets)) + for i, t := range targets { + items[i] = shared_toggle.Item{ + Label: filepath.Base(t) + " (" + t + ")", + Value: t, + Selected: true, + } + } + return Model{ + toggle: shared_toggle.New(items), + sources: targets, + } +} + +func (m Model) Init() tea.Cmd { return m.toggle.Init() } + +func (m Model) Update(msg tea.Msg) (tea.Model, tea.Cmd) { + switch msg := msg.(type) { + case tea.KeyMsg: + switch msg.String() { + case "enter", "ctrl+s": + var checked []string + for _, item := range m.toggle.GetSelected() { + checked = append(checked, item.Value) + } + return m, func() tea.Msg { return ConfirmedMsg{Sources: checked} } + case "q", "esc": + return m, func() tea.Msg { return BackMsg{} } + case "ctrl+c": + return m, tea.Quit + case "a": + m.toggle = m.toggle.SelectAll() + return m, nil + case "A": + m.toggle = m.toggle.DeselectAll() + return m, nil + } + } + var cmd tea.Cmd + var updated tea.Model + updated, cmd = m.toggle.Update(msg) + m.toggle = updated.(shared_toggle.Model) + return m, cmd +} + +func (m Model) View() string { + var s strings.Builder + s.WriteString(styles.TitleStyle.Render("Import — Select Sources")) + s.WriteString("\n\nChoose which agent directories to search:\n") + s.WriteString(m.toggle.View()) + s.WriteString(styles.HelpStyle.Render( + "\n[↑/↓] navigate • [space] toggle • [a] all • [A] none • [enter/ctrl+s] next • [q/esc] back", + )) + return s.String() +} diff --git a/internal/tui/import_source_view/model_test.go b/internal/tui/import_source_view/model_test.go new file mode 100644 index 0000000..f9c4c34 --- /dev/null +++ b/internal/tui/import_source_view/model_test.go @@ -0,0 +1,142 @@ +package import_source_view + +import ( + "bytes" + "io" + "reflect" + "testing" + "time" + + tea "github.com/charmbracelet/bubbletea" + "github.com/charmbracelet/x/exp/teatest" +) + +var testTargets = []string{"/home/user/.claude", "/home/user/.cursor", "/home/user/.config/opencode"} + +func TestImportSourceView_AllStartChecked(t *testing.T) { + m := New(testTargets) + selected := m.toggle.GetSelected() + if len(selected) != len(testTargets) { + t.Errorf("expected all %d sources checked, got %d", len(testTargets), len(selected)) + } +} + +func TestImportSourceView_ToggleUnchecksSource(t *testing.T) { + m := New(testTargets) + // space on first item should uncheck it + m2, _ := m.Update(tea.KeyMsg{Type: tea.KeySpace}) + selected := m2.(Model).toggle.GetSelected() + if len(selected) != len(testTargets)-1 { + t.Errorf("expected %d selected after toggle, got %d", len(testTargets)-1, len(selected)) + } +} + +func TestImportSourceView_SelectAll(t *testing.T) { + m := New(testTargets) + // deselect all first, then re-select + m2, _ := m.Update(tea.KeyMsg{Type: tea.KeyRunes, Runes: []rune("A")}) + m3, _ := m2.(Model).Update(tea.KeyMsg{Type: tea.KeyRunes, Runes: []rune("a")}) + selected := m3.(Model).toggle.GetSelected() + if len(selected) != len(testTargets) { + t.Errorf("expected %d selected after 'a', got %d", len(testTargets), len(selected)) + } +} + +func TestImportSourceView_DeselectAll(t *testing.T) { + m := New(testTargets) + m2, _ := m.Update(tea.KeyMsg{Type: tea.KeyRunes, Runes: []rune("A")}) + selected := m2.(Model).toggle.GetSelected() + if len(selected) != 0 { + t.Errorf("expected 0 selected after 'A', got %d", len(selected)) + } +} + +func TestImportSourceView_EnterEmitsConfirmedMsgWithCheckedSources(t *testing.T) { + m := New(testTargets) + // Uncheck the second source before confirming + m2, _ := m.Update(tea.KeyMsg{Type: tea.KeyDown}) // move cursor to second + m3, _ := m2.(Model).Update(tea.KeyMsg{Type: tea.KeySpace}) // uncheck it + + _, cmd := m3.(Model).Update(tea.KeyMsg{Type: tea.KeyEnter}) + if cmd == nil { + t.Fatal("expected command on enter") + } + msg := cmd() + confirmed, ok := msg.(ConfirmedMsg) + if !ok { + t.Fatalf("expected ConfirmedMsg, got %T", msg) + } + expected := []string{testTargets[0], testTargets[2]} + if !reflect.DeepEqual(confirmed.Sources, expected) { + t.Errorf("Sources: got %v, want %v", confirmed.Sources, expected) + } +} + +func TestImportSourceView_CtrlSEmitsConfirmedMsg(t *testing.T) { + m := New(testTargets) + _, cmd := m.Update(tea.KeyMsg{Type: tea.KeyCtrlS}) + if cmd == nil { + t.Fatal("expected command on ctrl+s") + } + if _, ok := cmd().(ConfirmedMsg); !ok { + t.Errorf("expected ConfirmedMsg on ctrl+s") + } +} + +func TestImportSourceView_QEmitsBackMsg(t *testing.T) { + m := New(testTargets) + _, cmd := m.Update(tea.KeyMsg{Type: tea.KeyRunes, Runes: []rune("q")}) + if cmd == nil { + t.Fatal("expected command on 'q'") + } + if _, ok := cmd().(BackMsg); !ok { + t.Errorf("expected BackMsg on 'q', got %T", cmd()) + } +} + +func TestImportSourceView_EscEmitsBackMsg(t *testing.T) { + m := New(testTargets) + _, cmd := m.Update(tea.KeyMsg{Type: tea.KeyEsc}) + if cmd == nil { + t.Fatal("expected command on esc") + } + if _, ok := cmd().(BackMsg); !ok { + t.Errorf("expected BackMsg on esc, got %T", cmd()) + } +} + +func TestImportSourceView_CtrlCQuits(t *testing.T) { + m := New(testTargets) + _, cmd := m.Update(tea.KeyMsg{Type: tea.KeyCtrlC}) + if cmd == nil { + t.Fatal("expected command on ctrl+c") + } + if cmd() != tea.Quit() { + t.Errorf("expected tea.Quit on ctrl+c") + } +} + +func TestImportSourceView_Teatest(t *testing.T) { + m := New(testTargets) + tm := teatest.NewTestModel(t, m, teatest.WithInitialTermSize(80, 24)) + + if err := tm.Quit(); err != nil { + t.Fatal(err) + } + tm.WaitFinished(t, teatest.WithFinalTimeout(time.Second)) + + out, err := io.ReadAll(tm.Output()) + if err != nil { + t.Fatal(err) + } + + if !bytes.Contains(out, []byte(".claude")) { + t.Errorf("output should contain '.claude'") + } + if !bytes.Contains(out, []byte(".cursor")) { + t.Errorf("output should contain '.cursor'") + } + if !bytes.Contains(out, []byte("Import")) { + t.Errorf("output should contain 'Import'") + } +} diff --git a/internal/tui/menu/model.go b/internal/tui/menu/model.go index c795b1b..42c2801 100644 --- a/internal/tui/menu/model.go +++ b/internal/tui/menu/model.go @@ -27,6 +27,7 @@ func New() Model { item{title: "Init", desc: "Initialize configuration and discover skills"}, item{title: "Sync", desc: "Synchronize skills across directories"}, item{title: "Verify", desc: "Check for drift without syncing"}, + item{title: "Import", desc: "Import a skill from a configured agent source"}, item{title: "Config", desc: "Configure sync targets"}, item{title: "Quit", desc: "Exit the application"}, } diff --git a/internal/tui/menu/model_test.go b/internal/tui/menu/model_test.go index b525921..0be7d5b 100644 --- a/internal/tui/menu/model_test.go +++ b/internal/tui/menu/model_test.go @@ -49,3 +49,26 @@ func TestMenuSelection(t *testing.T) { t.Errorf("Expected tea.Quit on 'q'") } } + +func TestMenuContainsImportItem(t *testing.T) { + m := New() + + // Navigate down 3 times: Init(0) → Sync(1) → Verify(2) → Import(3) + cur := tea.Model(m) + for i := 0; i < 3; i++ { + cur, _ = cur.Update(tea.KeyMsg{Type: tea.KeyDown}) + } + + _, cmd := cur.Update(tea.KeyMsg{Type: tea.KeyEnter}) + if cmd == nil { + t.Fatal("expected command on enter at Import position") + } + msg := cmd() + selMsg, ok := msg.(MenuSelectionMsg) + if !ok { + t.Fatalf("expected MenuSelectionMsg, got %T", msg) + } + if selMsg.Selection != "Import" { + t.Errorf("expected 'Import', got %q", selMsg.Selection) + } +} diff --git a/internal/tui/program.go b/internal/tui/program.go index 13cf15b..1edc6a5 100644 --- a/internal/tui/program.go +++ b/internal/tui/program.go @@ -10,8 +10,11 @@ import ( "github.com/ezzek/skill-sync/internal/agent" "github.com/ezzek/skill-sync/internal/agent/config_resolver" "github.com/ezzek/skill-sync/internal/agent/discovery" + "github.com/ezzek/skill-sync/internal/importer" "github.com/ezzek/skill-sync/internal/models" "github.com/ezzek/skill-sync/internal/tui/config_view" + "github.com/ezzek/skill-sync/internal/tui/import_skill_view" + "github.com/ezzek/skill-sync/internal/tui/import_source_view" "github.com/ezzek/skill-sync/internal/tui/init_view" "github.com/ezzek/skill-sync/internal/tui/menu" "github.com/ezzek/skill-sync/internal/tui/output_view" @@ -28,23 +31,29 @@ const ( stateConfig stateSyncSelect stateOutput + stateImportSource + stateImportSkill ) type Callbacks struct { - ScanSkills func() ([]models.SkillSyncInfo, error) - RunSync func(skillFilter []string) (string, error) - RunVerify func() (string, error) + ScanSkills func() ([]models.SkillSyncInfo, error) + RunSync func(skillFilter []string) (string, error) + RunVerify func() (string, error) + FindSkillsForImport func(sources []string) ([]importer.SkillCandidate, error) + RunImport func(candidate importer.SkillCandidate) (string, error) } type RootModel struct { - state state - menuModel menu.Model - initModel tea.Model - cfgModel tea.Model - syncSelectModel tea.Model - outModel tea.Model - - callbacks Callbacks + state state + menuModel menu.Model + initModel tea.Model + cfgModel tea.Model + syncSelectModel tea.Model + outModel tea.Model + importSourceModel tea.Model + importSkillModel tea.Model + importSources []string // targets used to build importSourceModel + callbacks Callbacks } func NewRootModel(callbacks Callbacks) RootModel { @@ -116,6 +125,17 @@ func (m RootModel) Update(msg tea.Msg) (tea.Model, tea.Cmd) { m.outModel = output_view.New(out) m.state = stateOutput return m, m.outModel.Init() + + case "Import": + targets, err := loadImportTargets() + if err != nil { + m.outModel = output_view.New("Error loading config: " + err.Error()) + m.state = stateOutput + return m, m.outModel.Init() + } + m.importSourceModel = import_source_view.New(targets) + m.state = stateImportSource + return m, m.importSourceModel.Init() } case init_view.InitConfigMsg: @@ -151,6 +171,42 @@ func (m RootModel) Update(msg tea.Msg) (tea.Model, tea.Cmd) { case output_view.DoneMsg: m.state = stateMenu return m, m.menuModel.Init() + + case import_source_view.ConfirmedMsg: + m.importSources = msg.Sources + candidates, err := m.callbacks.FindSkillsForImport(msg.Sources) + if err != nil { + m.outModel = output_view.New("Error scanning skills: " + err.Error()) + m.state = stateOutput + return m, m.outModel.Init() + } + if len(candidates) == 0 { + m.outModel = output_view.New("No skills found in the selected sources.") + m.state = stateOutput + return m, m.outModel.Init() + } + m.importSkillModel = import_skill_view.New(candidates) + m.state = stateImportSkill + return m, m.importSkillModel.Init() + + case import_source_view.BackMsg: + m.state = stateMenu + return m, m.menuModel.Init() + + case import_skill_view.ConfirmedMsg: + out, err := m.callbacks.RunImport(msg.Candidate) + if err != nil { + out = "Import error: " + err.Error() + } + m.outModel = output_view.New(out) + m.state = stateOutput + return m, m.outModel.Init() + + case import_skill_view.BackMsg: + m.importSkillModel = nil + m.importSourceModel = import_source_view.New(m.importSources) + m.state = stateImportSource + return m, m.importSourceModel.Init() } var cmd tea.Cmd @@ -183,6 +239,18 @@ func (m RootModel) Update(msg tea.Msg) (tea.Model, tea.Cmd) { if updated != nil { m.outModel = updated } + case stateImportSource: + var updated tea.Model + updated, cmd = m.importSourceModel.Update(msg) + if updated != nil { + m.importSourceModel = updated + } + case stateImportSkill: + var updated tea.Model + updated, cmd = m.importSkillModel.Update(msg) + if updated != nil { + m.importSkillModel = updated + } } return m, cmd @@ -200,6 +268,10 @@ func (m RootModel) View() string { return m.syncSelectModel.View() case stateOutput: return m.outModel.View() + case stateImportSource: + return m.importSourceModel.View() + case stateImportSkill: + return m.importSkillModel.View() } return "" } @@ -254,6 +326,27 @@ func saveConfig(targets []string) string { return fmt.Sprintf("Configuration saved to %s", configPath) } +// loadImportTargets resolves skill-sync.yaml and returns the expanded target list. +func loadImportTargets() ([]string, error) { + cfgPath, err := config_resolver.New().Resolve("") + if err != nil { + return nil, fmt.Errorf("no skill-sync.yaml found; run 'skill-sync init' to create one") + } + f, err := os.Open(cfgPath) + if err != nil { + return nil, err + } + defer f.Close() + cfg, err := agent.ParseConfig(f) + if err != nil { + return nil, err + } + if home, err := os.UserHomeDir(); err == nil { + cfg.ExpandTargets(home) + } + return cfg.Targets, nil +} + type Program struct { callbacks Callbacks } diff --git a/internal/tui/program_test.go b/internal/tui/program_test.go index c28da04..6a62295 100644 --- a/internal/tui/program_test.go +++ b/internal/tui/program_test.go @@ -4,8 +4,11 @@ import ( "testing" tea "github.com/charmbracelet/bubbletea" + "github.com/ezzek/skill-sync/internal/importer" "github.com/ezzek/skill-sync/internal/models" "github.com/ezzek/skill-sync/internal/tui/config_view" + "github.com/ezzek/skill-sync/internal/tui/import_skill_view" + "github.com/ezzek/skill-sync/internal/tui/import_source_view" "github.com/ezzek/skill-sync/internal/tui/init_view" "github.com/ezzek/skill-sync/internal/tui/menu" "github.com/ezzek/skill-sync/internal/tui/output_view" @@ -91,3 +94,74 @@ func TestRootModelTransitions(t *testing.T) { t.Errorf("Expected tea.Quit on ctrl+c") } } + +func makeImportCallbacks() Callbacks { + candidate := importer.SkillCandidate{ + SkillID: "git-expert", + SourceDir: "/fake/git-expert", + AgentDir: "/fake", + } + return Callbacks{ + ScanSkills: func() ([]models.SkillSyncInfo, error) { return nil, nil }, + RunSync: func([]string) (string, error) { return "", nil }, + RunVerify: func() (string, error) { return "", nil }, + FindSkillsForImport: func(sources []string) ([]importer.SkillCandidate, error) { + return []importer.SkillCandidate{candidate}, nil + }, + RunImport: func(c importer.SkillCandidate) (string, error) { + return "Imported " + c.SkillID, nil + }, + } +} + +func TestRootModel_ImportFlow(t *testing.T) { + m := NewRootModel(makeImportCallbacks()) + + // import_source_view.ConfirmedMsg → stateImportSkill (candidates found) + t.Run("SourceConfirmed with candidates → stateImportSkill", func(t *testing.T) { + m2, _ := m.Update(import_source_view.ConfirmedMsg{Sources: []string{"/fake"}}) + if m2.(RootModel).state != stateImportSkill { + t.Errorf("expected stateImportSkill, got %v", m2.(RootModel).state) + } + }) + + // import_source_view.ConfirmedMsg with no candidates → stateOutput + t.Run("SourceConfirmed with no candidates → stateOutput", func(t *testing.T) { + cbs := makeImportCallbacks() + cbs.FindSkillsForImport = func([]string) ([]importer.SkillCandidate, error) { + return nil, nil + } + m0 := NewRootModel(cbs) + m2, _ := m0.Update(import_source_view.ConfirmedMsg{Sources: []string{"/fake"}}) + if m2.(RootModel).state != stateOutput { + t.Errorf("expected stateOutput, got %v", m2.(RootModel).state) + } + }) + + // import_source_view.BackMsg → stateMenu + t.Run("SourceBack → stateMenu", func(t *testing.T) { + m2, _ := m.Update(import_source_view.BackMsg{}) + if m2.(RootModel).state != stateMenu { + t.Errorf("expected stateMenu, got %v", m2.(RootModel).state) + } + }) + + // Transition to stateImportSkill then test skill selection → stateOutput + t.Run("SkillConfirmed → stateOutput", func(t *testing.T) { + m2, _ := m.Update(import_source_view.ConfirmedMsg{Sources: []string{"/fake"}}) + candidate := importer.SkillCandidate{SkillID: "git-expert", SourceDir: "/fake/git-expert"} + m3, _ := m2.(RootModel).Update(import_skill_view.ConfirmedMsg{Candidate: candidate}) + if m3.(RootModel).state != stateOutput { + t.Errorf("expected stateOutput, got %v", m3.(RootModel).state) + } + }) + + // import_skill_view.BackMsg → stateImportSource + t.Run("SkillBack → stateImportSource", func(t *testing.T) { + m2, _ := m.Update(import_source_view.ConfirmedMsg{Sources: []string{"/fake"}}) + m3, _ := m2.(RootModel).Update(import_skill_view.BackMsg{}) + if m3.(RootModel).state != stateImportSource { + t.Errorf("expected stateImportSource, got %v", m3.(RootModel).state) + } + }) +} From c2cba25cd748f47db5b379ccc3e050bc55baef39 Mon Sep 17 00:00:00 2001 From: Ezequiel Canul Date: Fri, 2 Oct 2026 14:21:57 -0600 Subject: [PATCH 4/4] style(cli): apply gofmt -s to import command and tests --- internal/cli/import.go | 2 +- internal/cli/import_test.go | 12 ++++++------ 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/internal/cli/import.go b/internal/cli/import.go index 4372f14..2c8c0c1 100644 --- a/internal/cli/import.go +++ b/internal/cli/import.go @@ -82,4 +82,4 @@ func runImport(cmd *cobra.Command, configPath, cwd, skillID string) error { fmt.Fprintf(cmd.OutOrStdout(), "Imported %s from %s\n", skillID, winner.SourceDir) return nil -} \ No newline at end of file +} diff --git a/internal/cli/import_test.go b/internal/cli/import_test.go index 9763ab2..d876d1d 100644 --- a/internal/cli/import_test.go +++ b/internal/cli/import_test.go @@ -81,7 +81,7 @@ func TestImportCmd(t *testing.T) { noCfg := filepath.Join(base, "nonexistent.yaml") return []string{"git-expert"}, noCfg, base }, - verify: func(t *testing.T, base string, stdout, stderr string, err error) {}, + verify: func(t *testing.T, base string, stdout, stderr string, err error) {}, wantErr: true, wantErrMsg: "skill-sync init", }, @@ -93,7 +93,7 @@ func TestImportCmd(t *testing.T) { cfgPath := writeImportConfig(t, base, []string{toolA}) return []string{"nonexistent-skill"}, cfgPath, base }, - verify: func(t *testing.T, base string, stdout, stderr string, err error) {}, + verify: func(t *testing.T, base string, stdout, stderr string, err error) {}, wantErr: true, wantErrMsg: "not found", }, @@ -104,7 +104,7 @@ func TestImportCmd(t *testing.T) { _ = os.WriteFile(cfgPath, []byte("targets:\n - /tmp\n"), 0644) return []string{}, cfgPath, base }, - verify: func(t *testing.T, base string, stdout, stderr string, err error) {}, + verify: func(t *testing.T, base string, stdout, stderr string, err error) {}, wantErr: true, wantErrMsg: "accepts 1 arg", }, @@ -115,7 +115,7 @@ func TestImportCmd(t *testing.T) { _ = os.WriteFile(cfgPath, []byte("targets:\n - /tmp\n"), 0644) return []string{"arg1", "arg2"}, cfgPath, base }, - verify: func(t *testing.T, base string, stdout, stderr string, err error) {}, + verify: func(t *testing.T, base string, stdout, stderr string, err error) {}, wantErr: true, wantErrMsg: "accepts 1 arg", }, @@ -137,7 +137,7 @@ func TestImportCmd(t *testing.T) { cfgPath := writeImportConfig(t, base, []string{toolA, toolB}) return []string{"ambiguous-skill"}, cfgPath, base }, - verify: func(t *testing.T, base string, stdout, stderr string, err error) {}, + verify: func(t *testing.T, base string, stdout, stderr string, err error) {}, wantErr: true, wantErrMsg: "conflict", }, @@ -230,4 +230,4 @@ func TestImportCmd(t *testing.T) { } }) } -} \ No newline at end of file +}