diff --git a/control-plane/internal/skillkit/skillkit_edge_test.go b/control-plane/internal/skillkit/skillkit_edge_test.go index 0557eb198..72c16f1ef 100644 --- a/control-plane/internal/skillkit/skillkit_edge_test.go +++ b/control-plane/internal/skillkit/skillkit_edge_test.go @@ -788,7 +788,7 @@ func TestTargetSpecificEdgeCases(t *testing.T) { // holding nothing but the user's own text. {name: "codex", target: codexTarget{}, dir: filepath.Join(home, ".codex"), path: filepath.Join(home, ".codex", "AGENTS.override.md")}, {name: "gemini", target: geminiTarget{}, dir: filepath.Join(home, ".gemini"), path: filepath.Join(home, ".gemini", "GEMINI.md")}, - {name: "opencode", target: opencodeTarget{}, dir: filepath.Join(home, ".config", "opencode"), path: filepath.Join(home, ".config", "opencode", "AGENTS.md")}, + {name: "opencode", target: opencodeTarget{}, dir: filepath.Join(home, ".config", "opencode", "skills"), path: filepath.Join(home, ".config", "opencode", "skills", "agentfield")}, {name: "windsurf", target: windsurfTarget{}, dir: filepath.Join(home, ".codeium", "windsurf", "memories"), path: filepath.Join(home, ".codeium", "windsurf", "memories", "global_rules.md")}, } diff --git a/control-plane/internal/skillkit/skillkit_test.go b/control-plane/internal/skillkit/skillkit_test.go index c105b0aa3..51862776e 100644 --- a/control-plane/internal/skillkit/skillkit_test.go +++ b/control-plane/internal/skillkit/skillkit_test.go @@ -463,13 +463,13 @@ func TestHelpersAndTargets(t *testing.T) { t.Fatalf("mkdir opencode dir: %v", err) } opencode := opencodeTarget{} - if opencode.DisplayName() != "OpenCode" || opencode.Method() != "marker-block" { + if opencode.DisplayName() != "OpenCode" || opencode.Method() != "symlink" { t.Fatalf("unexpected opencode metadata: %q %q", opencode.DisplayName(), opencode.Method()) } if !opencode.Detected() { t.Fatal("opencode target should be detected") } - if _, err := opencode.Install(skill, filepath.Join(home, "canonical", "current")); err != nil { + if _, err := opencode.Install(skill, filepath.Join(home, "canonical", skill.Version)); err != nil { t.Fatalf("opencode install: %v", err) } if installed, version, err := opencode.Status(); err != nil || !installed || version != skill.Version { diff --git a/control-plane/internal/skillkit/target_opencode.go b/control-plane/internal/skillkit/target_opencode.go index cd3bd11e3..5e4fac198 100644 --- a/control-plane/internal/skillkit/target_opencode.go +++ b/control-plane/internal/skillkit/target_opencode.go @@ -2,18 +2,29 @@ package skillkit import ( "errors" + "fmt" + "os" "path/filepath" + "strings" + "time" ) -// opencodeTarget installs into OpenCode by appending a marker block to -// ~/.config/opencode/AGENTS.md. +// opencodeTarget installs skills where OpenCode discovers them natively: a +// directory at ~/.config/opencode/skills//, symlinked at the canonical +// versioned store so updates flow through without rewriting anything OpenCode +// owns. +// +// Older af binaries instead appended a marker block to +// ~/.config/opencode/AGENTS.md. Every install/uninstall now strips that block +// so upgrading users are left with the native skill instead of the native +// skill plus stale instructions. type opencodeTarget struct{} func init() { RegisterTarget(opencodeTarget{}) } func (opencodeTarget) Name() string { return "opencode" } func (opencodeTarget) DisplayName() string { return "OpenCode" } -func (opencodeTarget) Method() string { return "marker-block" } +func (opencodeTarget) Method() string { return "symlink" } func (opencodeTarget) Detected() bool { return commandAvailable("opencode") || dirExists(filepath.Join(homeDir(), ".config", "opencode")) @@ -24,29 +35,88 @@ func (opencodeTarget) TargetPath() (string, error) { if h == "" { return "", errors.New("could not resolve home directory") } - return filepath.Join(h, ".config", "opencode", "AGENTS.md"), nil + return filepath.Join(h, ".config", "opencode", "skills"), nil +} + +// legacyRulesPath is the file older af binaries appended marker blocks to. It +// is derived from an already-resolved skills root, so unlike Codex's variant +// it cannot fail: every caller has proven TargetPath() succeeds before it gets +// here. +// +// Unlike Codex's AGENTS.override.md — a file af created for itself — this one +// is authored by the user and read by OpenCode, so it is only ever read, and +// only rewritten when it still holds a block of ours. +func (opencodeTarget) legacyRulesPath(root string) string { + return filepath.Join(filepath.Dir(root), "AGENTS.md") +} + +func (t opencodeTarget) skillLink(skill Skill) (string, error) { + root, err := t.TargetPath() + if err != nil { + return "", err + } + return filepath.Join(root, skill.Name), nil } func (t opencodeTarget) Install(skill Skill, canonicalCurrentDir string) (InstalledTarget, error) { - path, err := t.TargetPath() + root, err := t.TargetPath() if err != nil { return InstalledTarget{}, err } - inst, err := installMarkerBlock(skill, canonicalCurrentDir, path) + if err := os.MkdirAll(root, 0o755); err != nil { + return InstalledTarget{}, fmt.Errorf("create %s: %w", root, err) + } + link, err := t.skillLink(skill) if err != nil { return InstalledTarget{}, err } - inst.TargetName = t.Name() - return inst, nil + if info, err := os.Lstat(link); err == nil { + if info.Mode()&os.ModeSymlink != 0 || info.IsDir() || info.Mode().IsRegular() { + if err := os.RemoveAll(link); err != nil { + return InstalledTarget{}, fmt.Errorf("remove existing %s: %w", link, err) + } + } + } else if !os.IsNotExist(err) { + return InstalledTarget{}, fmt.Errorf("inspect %s: %w", link, err) + } + if err := os.Symlink(canonicalCurrentDir, link); err != nil { + return InstalledTarget{}, fmt.Errorf("symlink %s -> %s: %w", link, canonicalCurrentDir, err) + } + // The native skill is in place; finish the migration off the old + // AGENTS.md block so the user is not left carrying both. + // + // AGENTS.md belongs to the user, and by this point the integration is + // already live on disk. Failing the install over a file the skill does + // not need would push the caller down its failure path, which records + // nothing in state — leaving `af skill list` reporting OpenCode as not + // installed and every later install exiting non-zero, over a stale block + // that has nothing to do with whether OpenCode can load the skill. So the + // cleanup is advisory here and only Uninstall, where the block is the + // whole point of the call, treats it as fatal. + if err := t.removeLegacyMarkerBlock(skill, root); err != nil { + fmt.Fprintf(os.Stderr, "warning: could not clean the legacy OpenCode rules block: %v\n", err) + } + return InstalledTarget{TargetName: t.Name(), Method: t.Method(), Path: link, Version: skill.Version, InstalledAt: time.Now().UTC()}, nil } func (t opencodeTarget) Uninstall() error { - path, err := t.TargetPath() + // Resolve the target root up front so failures (for example, an + // unavailable home directory) are reported to the caller instead of + // being silently ignored while iterating over the catalog. + root, err := t.TargetPath() if err != nil { return err } for _, s := range Catalog { - if err := uninstallMarkerBlock(s, path); err != nil { + link := filepath.Join(root, s.Name) + if info, err := os.Lstat(link); err == nil && (info.Mode()&os.ModeSymlink != 0 || info.IsDir() || info.Mode().IsRegular()) { + if err := os.RemoveAll(link); err != nil { + return fmt.Errorf("remove %s: %w", link, err) + } + } + // Machines that never ran an install in between still carry the + // legacy block; uninstall has to clear it too. + if err := t.removeLegacyMarkerBlock(s, root); err != nil { return err } } @@ -54,13 +124,86 @@ func (t opencodeTarget) Uninstall() error { } func (t opencodeTarget) Status() (bool, string, error) { - path, err := t.TargetPath() + link, err := t.skillLink(Catalog[0]) if err != nil { return false, "", err } - v := readMarkerVersion(Catalog[0], path) - if v == "" { + info, err := os.Lstat(link) + if os.IsNotExist(err) { return false, "", nil } - return true, v, nil + if err != nil { + return false, "", err + } + if info.Mode()&os.ModeSymlink == 0 { + return true, "manual", nil + } + dest, err := os.Readlink(link) + if err != nil { + return false, "", err + } + if !filepath.IsAbs(dest) { + dest = filepath.Join(filepath.Dir(link), dest) + } + base := filepath.Base(dest) + // Older installations link directly to a version directory, which may + // have been removed temporarily. Preserve that version from the link name. + if base != "current" && strings.Count(base, ".") >= 2 && len(base) > 0 && base[0] >= '0' && base[0] <= '9' { + return true, base, nil + } + resolved, err := filepath.EvalSymlinks(dest) + if err != nil { + return false, "", err + } + return true, filepath.Base(resolved), nil +} + +// removeLegacyMarkerBlock strips this skill's marker block from +// ~/.config/opencode/AGENTS.md, the rules file older af binaries wrote into. +// +// That file belongs to the user — OpenCode reads it, and af never created it +// on its own — so the rules are deliberately stricter than the Codex +// equivalent: a file holding no block of ours is not opened for writing at +// all (its bytes and mtime stay exactly as the user left them), and the file +// is deleted only when removing our block is what emptied it. Other tools' +// blocks and any user prose are preserved. A missing file is a no-op; every +// other failure is returned, and the two callers weigh it differently — +// Uninstall propagates it, Install warns (see there). +// +// Every filesystem call goes through the package's reconcile* seams so each +// failure branch below is reachable from a test. +func (t opencodeTarget) removeLegacyMarkerBlock(skill Skill, root string) error { + path := t.legacyRulesPath(root) + data, err := reconcileReadFile(path) + if os.IsNotExist(err) { + return nil + } + if err != nil { + return fmt.Errorf("read legacy OpenCode rules file %s: %w", path, err) + } + if _, ours := findMarkerBlock(string(data), skill); !ours { + return nil // nothing of ours in there; leave the user's file alone + } + + cleaned := strings.TrimRight(stripMarkerBlock(string(data), skill), "\n") + if strings.TrimSpace(cleaned) == "" { + // Our block was the only thing in it, so the file was ours alone. + if err := reconcileRemove(path); err != nil && !os.IsNotExist(err) { + return fmt.Errorf("remove %s: %w", path, err) + } + return nil + } + + perm := os.FileMode(0o644) + if info, err := os.Stat(path); err == nil { + perm = info.Mode().Perm() + } + tmp := path + ".af-tmp" + if err := reconcileWriteFile(tmp, []byte(cleaned+"\n"), perm); err != nil { + return fmt.Errorf("write %s: %w", tmp, err) + } + if err := reconcileRename(tmp, path); err != nil { + return fmt.Errorf("rename into %s: %w", path, err) + } + return nil } diff --git a/control-plane/internal/skillkit/target_opencode_cleanup_test.go b/control-plane/internal/skillkit/target_opencode_cleanup_test.go new file mode 100644 index 000000000..f367ad281 --- /dev/null +++ b/control-plane/internal/skillkit/target_opencode_cleanup_test.go @@ -0,0 +1,346 @@ +package skillkit + +import ( + "errors" + "os" + "path/filepath" + "strings" + "testing" + "time" +) + +// opencodeLegacyHome prepares an isolated home with ~/.config/opencode present +// and returns the home plus the legacy rules file path inside it. +func opencodeLegacyHome(t *testing.T) (string, string) { + t.Helper() + home := withTempHome(t) + dir := filepath.Join(home, ".config", "opencode") + if err := os.MkdirAll(dir, 0o755); err != nil { + t.Fatalf("mkdir .config/opencode: %v", err) + } + return home, filepath.Join(dir, "AGENTS.md") +} + +// seedCurrentDir returns a canonical current/ directory an install can link at. +func seedCurrentDir(t *testing.T) string { + t.Helper() + current := filepath.Join(t.TempDir(), "current") + if err := os.MkdirAll(current, 0o755); err != nil { + t.Fatalf("mkdir current: %v", err) + } + return current +} + +// foreignBlock is another tool's marker block: same file, different owner. +const foreignBlock = "\nplandb rules\n" + +// Contract (a) + (e): installing the native skill strips this skill's legacy +// marker block from ~/.config/opencode/AGENTS.md while leaving the user's prose +// on both sides of it — and another tool's block — untouched. +func TestOpenCodeInstallStripsLegacyMarkerBlockAndKeepsForeignContent(t *testing.T) { + _, legacy := opencodeLegacyHome(t) + content := "# my own opencode notes\n\n" + + renderPointerBlock(Catalog[0], "/gone/canonical/current") + "\n\n" + + foreignBlock + "\n\nnotes that come after the block\n" + if err := os.WriteFile(legacy, []byte(content), 0o644); err != nil { + t.Fatalf("seed legacy rules file: %v", err) + } + + if _, err := (opencodeTarget{}).Install(Catalog[0], seedCurrentDir(t)); err != nil { + t.Fatalf("Install: %v", err) + } + + data, err := os.ReadFile(legacy) + if err != nil { + t.Fatalf("read legacy rules file: %v", err) + } + got := string(data) + if strings.Contains(got, markerStartPattern(Catalog[0])) { + t.Fatalf("legacy marker block survived the install:\n%s", got) + } + for _, keep := range []string{"# my own opencode notes", foreignBlock, "notes that come after the block"} { + if !strings.Contains(got, keep) { + t.Fatalf("migration destroyed content it does not own (%q missing):\n%s", keep, got) + } + } +} + +// Contract (c): a rules file that held nothing but our block was ours alone, so +// it is deleted rather than left behind as an empty file OpenCode keeps reading. +func TestOpenCodeInstallDeletesLegacyRulesFileItOwnedAlone(t *testing.T) { + for _, tc := range []struct { + name string + content string + }{ + {name: "block only", content: renderPointerBlock(Catalog[0], "/gone/current") + "\n"}, + {name: "block and whitespace", content: "\n \n" + renderPointerBlock(Catalog[0], "/gone/current") + "\n \n\t\n"}, + } { + t.Run(tc.name, func(t *testing.T) { + _, legacy := opencodeLegacyHome(t) + if err := os.WriteFile(legacy, []byte(tc.content), 0o644); err != nil { + t.Fatalf("seed legacy rules file: %v", err) + } + if _, err := (opencodeTarget{}).Install(Catalog[0], seedCurrentDir(t)); err != nil { + t.Fatalf("Install: %v", err) + } + if _, err := os.Lstat(legacy); !os.IsNotExist(err) { + data, _ := os.ReadFile(legacy) + t.Fatalf("legacy rules file should be removed, lstat err=%v content=%q", err, data) + } + }) + } +} + +// Contract (d) + (e): ~/.config/opencode/AGENTS.md is the user's own file. When +// it carries no block of ours, neither install nor uninstall may touch it — +// same bytes, same modification time, whitespace-only content included. +func TestOpenCodeLeavesARulesFileWithoutOurBlockUntouched(t *testing.T) { + for _, tc := range []struct { + name string + content string + }{ + {name: "user prose", content: "# my rules\n\nbe concise\n"}, + {name: "foreign block only", content: foreignBlock + "\n"}, + {name: "whitespace only", content: "\n \n\t\n"}, + {name: "empty", content: ""}, + } { + t.Run(tc.name, func(t *testing.T) { + _, legacy := opencodeLegacyHome(t) + if err := os.WriteFile(legacy, []byte(tc.content), 0o644); err != nil { + t.Fatalf("seed legacy rules file: %v", err) + } + // Backdate so a rewrite is visible even at coarse mtime resolution. + stamp := time.Date(2020, time.March, 4, 5, 6, 7, 0, time.UTC) + if err := os.Chtimes(legacy, stamp, stamp); err != nil { + t.Fatalf("chtimes: %v", err) + } + + target := opencodeTarget{} + if _, err := target.Install(Catalog[0], seedCurrentDir(t)); err != nil { + t.Fatalf("Install: %v", err) + } + assertRulesFileUnchanged(t, legacy, tc.content, stamp, "install") + + if err := target.Uninstall(); err != nil { + t.Fatalf("Uninstall: %v", err) + } + assertRulesFileUnchanged(t, legacy, tc.content, stamp, "uninstall") + }) + } +} + +func assertRulesFileUnchanged(t *testing.T, path, want string, stamp time.Time, stage string) { + t.Helper() + data, err := os.ReadFile(path) + if err != nil { + t.Fatalf("%s removed the user's rules file: %v", stage, err) + } + if string(data) != want { + t.Fatalf("%s rewrote the user's rules file:\ngot: %q\nwant: %q", stage, data, want) + } + info, err := os.Stat(path) + if err != nil { + t.Fatalf("stat after %s: %v", stage, err) + } + if !info.ModTime().Equal(stamp) { + t.Fatalf("%s opened the user's rules file for writing: mtime %s, want %s", + stage, info.ModTime().UTC(), stamp) + } +} + +// Contract (b) + (e): uninstall removes every catalog skill's link and finishes +// the migration for machines that never ran an install in between, keeping the +// user's prose and other tools' blocks. +func TestOpenCodeUninstallRemovesLinksAndLegacyBlocks(t *testing.T) { + home, legacy := opencodeLegacyHome(t) + var content strings.Builder + content.WriteString("user prose\n\n") + for _, s := range Catalog { + content.WriteString(renderPointerBlock(s, "/gone/current")) + content.WriteString("\n\n") + } + content.WriteString(foreignBlock + "\n") + if err := os.WriteFile(legacy, []byte(content.String()), 0o644); err != nil { + t.Fatalf("seed legacy rules file: %v", err) + } + + target := opencodeTarget{} + root, err := target.TargetPath() + if err != nil { + t.Fatalf("TargetPath: %v", err) + } + if err := os.MkdirAll(root, 0o755); err != nil { + t.Fatalf("mkdir skills root: %v", err) + } + for _, s := range Catalog { + if err := os.Symlink(filepath.Join(home, "gone"), filepath.Join(root, s.Name)); err != nil { + t.Fatalf("seed link for %s: %v", s.Name, err) + } + } + + if err := target.Uninstall(); err != nil { + t.Fatalf("Uninstall: %v", err) + } + for _, s := range Catalog { + if _, err := os.Lstat(filepath.Join(root, s.Name)); !os.IsNotExist(err) { + t.Fatalf("link for %s remains: %v", s.Name, err) + } + } + data, err := os.ReadFile(legacy) + if err != nil { + t.Fatalf("read legacy rules file: %v", err) + } + if strings.Contains(string(data), "agentfield-skill:") { + t.Fatalf("legacy blocks remain after uninstall:\n%s", data) + } + if !strings.Contains(string(data), "user prose") || !strings.Contains(string(data), foreignBlock) { + t.Fatalf("uninstall destroyed content it does not own:\n%s", data) + } + // Uninstalling twice is a no-op, not an error. + if err := target.Uninstall(); err != nil { + t.Fatalf("second Uninstall: %v", err) + } +} + +// Contract (f): with no legacy rules file on disk, install and uninstall both +// succeed and neither conjures the file into existence. +func TestOpenCodeCleanupIsANoOpWithoutALegacyRulesFile(t *testing.T) { + _, legacy := opencodeLegacyHome(t) + target := opencodeTarget{} + + if _, err := target.Install(Catalog[0], seedCurrentDir(t)); err != nil { + t.Fatalf("Install: %v", err) + } + if _, err := os.Lstat(legacy); !os.IsNotExist(err) { + t.Fatalf("install created a legacy rules file: %v", err) + } + if err := target.Uninstall(); err != nil { + t.Fatalf("Uninstall: %v", err) + } + if _, err := os.Lstat(legacy); !os.IsNotExist(err) { + t.Fatalf("uninstall created a legacy rules file: %v", err) + } +} + +// Contract (g): uninstall reports a legacy rules file it cannot read, rather +// than silently leaving the block behind — there, stripping the block is the +// entire point of the call. +func TestOpenCodeUninstallReportsAnUnreadableLegacyRulesFile(t *testing.T) { + _, legacy := opencodeLegacyHome(t) + // A directory where the rules file belongs: readable path, unreadable file. + if err := os.MkdirAll(legacy, 0o755); err != nil { + t.Fatalf("mkdir over legacy rules path: %v", err) + } + + err := (opencodeTarget{}).Uninstall() + if err == nil { + t.Fatal("Uninstall should report a legacy rules file it cannot read") + } + if !strings.Contains(err.Error(), "AGENTS.md") { + t.Fatalf("error should name the file it could not read: %v", err) + } +} + +// Contract (h): the cleanup is a migration courtesy, not part of making the +// skill work. A legacy rules file af cannot clean up must not fail an install +// whose symlink is already on disk: the caller records nothing for a failed +// target, so failing here would report OpenCode as not installed while it is +// live, and every later `af skill install` would exit non-zero over a stale +// block OpenCode never reads. +func TestOpenCodeInstallSurvivesALegacyRulesFileItCannotClean(t *testing.T) { + home := withTempHome(t) + legacy := filepath.Join(home, ".config", "opencode", "AGENTS.md") + // A directory where the rules file belongs: readable path, unreadable file. + if err := os.MkdirAll(legacy, 0o755); err != nil { + t.Fatalf("mkdir over legacy rules path: %v", err) + } + + report, err := Install(InstallOptions{SkillName: Catalog[0].Name, Targets: []string{"opencode"}}) + if err != nil { + t.Fatalf("Install: %v", err) + } + if len(report.TargetsFailed) != 0 { + t.Fatalf("an uncleanable legacy rules file failed the install: %+v", report.TargetsFailed) + } + if len(report.TargetsInstalled) != 1 || report.TargetsInstalled[0].TargetName != "opencode" { + t.Fatalf("opencode was not reported as installed: %+v", report) + } + + link := filepath.Join(home, ".config", "opencode", "skills", Catalog[0].Name) + if _, err := os.Lstat(link); err != nil { + t.Fatalf("native skill link missing: %v", err) + } + // The link is on disk, so state has to agree — otherwise `af skill list` + // and the next install both disagree with reality. + state, err := LoadState() + if err != nil { + t.Fatalf("LoadState: %v", err) + } + recorded, ok := state.Skills[Catalog[0].Name].Targets["opencode"] + if !ok { + t.Fatal("a live OpenCode install was not recorded in state") + } + if recorded.Path != link { + t.Fatalf("recorded path = %q, want the link on disk %q", recorded.Path, link) + } +} + +// Contract (i): every write the cleanup performs is reported when it fails. +// These branches are unreachable through real filesystem permissions on some +// platforms, so they are driven through the package's reconcile* seams — the +// same way the reconciler's own rewrite failures are covered. +func TestOpenCodeUninstallReportsLegacyRewriteFailures(t *testing.T) { + ourBlock := renderPointerBlock(Catalog[0], "/gone/current") + for _, tc := range []struct { + name string + content string + inject func(t *testing.T) + }{ + { + name: "write", + content: "user prose\n\n" + ourBlock + "\n", + inject: func(t *testing.T) { + old := reconcileWriteFile + reconcileWriteFile = func(string, []byte, os.FileMode) error { + return errors.New("forced write failure") + } + t.Cleanup(func() { reconcileWriteFile = old }) + }, + }, + { + name: "rename", + content: "user prose\n\n" + ourBlock + "\n", + inject: func(t *testing.T) { + old := reconcileRename + reconcileRename = func(string, string) error { return errors.New("forced rename failure") } + t.Cleanup(func() { reconcileRename = old }) + }, + }, + { + name: "remove", + content: ourBlock + "\n", + inject: func(t *testing.T) { + old := reconcileRemove + reconcileRemove = func(string) error { return errors.New("forced remove failure") } + t.Cleanup(func() { reconcileRemove = old }) + }, + }, + } { + t.Run(tc.name, func(t *testing.T) { + _, legacy := opencodeLegacyHome(t) + if err := os.WriteFile(legacy, []byte(tc.content), 0o644); err != nil { + t.Fatalf("seed legacy rules file: %v", err) + } + tc.inject(t) + + err := (opencodeTarget{}).Uninstall() + if err == nil { + t.Fatalf("Uninstall should report a failed %s of the legacy rules file", tc.name) + } + if !strings.Contains(err.Error(), "AGENTS.md") || + !strings.Contains(err.Error(), "forced "+tc.name+" failure") { + t.Fatalf("error should name the file and the cause: %v", err) + } + }) + } +} diff --git a/control-plane/internal/skillkit/target_opencode_test.go b/control-plane/internal/skillkit/target_opencode_test.go new file mode 100644 index 000000000..2cc7e0a44 --- /dev/null +++ b/control-plane/internal/skillkit/target_opencode_test.go @@ -0,0 +1,189 @@ +package skillkit + +import ( + "os" + "path/filepath" + "testing" +) + +func TestOpenCodeTargetInstallsSkillSymlink(t *testing.T) { + home := withTempHome(t) + t.Setenv("USERPROFILE", home) + canonical := filepath.Join(home, ".agentfield", "skills", "agentfield", "1.2.3") + if err := os.MkdirAll(canonical, 0o755); err != nil { + t.Fatal(err) + } + target := opencodeTarget{} + installed, err := target.Install(Skill{Name: "agentfield", Version: "1.2.3"}, canonical) + if err != nil { + t.Fatal(err) + } + want := filepath.Join(home, ".config", "opencode", "skills", "agentfield") + if installed.Method != "symlink" || installed.Path != want { + t.Fatalf("installed target = %#v", installed) + } + got, err := os.Readlink(want) + if err != nil || got != canonical { + t.Fatalf("OpenCode link = %q, %v; want %q", got, err, canonical) + } + if installed, version, err := target.Status(); err != nil || !installed || version != "1.2.3" { + t.Fatalf("OpenCode status = %v %q %v", installed, version, err) + } + if err := target.Uninstall(); err != nil { + t.Fatal(err) + } + if _, err := os.Lstat(want); !os.IsNotExist(err) { + t.Fatalf("skill link still exists after uninstall: %v", err) + } +} + +func TestOpenCodeTargetReplacesExistingEntryAndReportsManualEntry(t *testing.T) { + home := withTempHome(t) + target := opencodeTarget{} + root, err := target.TargetPath() + if err != nil { + t.Fatal(err) + } + if err := os.MkdirAll(root, 0o755); err != nil { + t.Fatal(err) + } + link := filepath.Join(root, Catalog[0].Name) + if err := os.WriteFile(link, []byte("manual"), 0o644); err != nil { + t.Fatal(err) + } + if installed, version, err := target.Status(); err != nil || !installed || version != "manual" { + t.Fatalf("manual Status = %v %q %v", installed, version, err) + } + canonical := filepath.Join(home, ".agentfield", "skills", Catalog[0].Name, Catalog[0].Version) + if err := os.MkdirAll(canonical, 0o755); err != nil { + t.Fatal(err) + } + if _, err := target.Install(Catalog[0], canonical); err != nil { + t.Fatal(err) + } + if _, err := os.Readlink(link); err != nil { + t.Fatalf("replacement is not a symlink: %v", err) + } +} + +func TestOpenCodeTargetInstallReportsRootCreationFailure(t *testing.T) { + home := withTempHome(t) + if err := os.WriteFile(filepath.Join(home, ".config"), []byte("not a directory"), 0o644); err != nil { + t.Fatal(err) + } + + _, err := (opencodeTarget{}).Install(Catalog[0], filepath.Join(home, "canonical")) + if err == nil { + t.Fatal("Install should report a failure creating the OpenCode skills directory") + } +} + +func TestOpenCodeTargetUninstallReportsMissingHome(t *testing.T) { + t.Setenv("HOME", "") + t.Setenv("USERPROFILE", "") + t.Setenv("AGENTFIELD_HOME", "") + if err := (opencodeTarget{}).Uninstall(); err == nil { + t.Fatal("Uninstall should report an unavailable home directory") + } +} + +func TestOpenCodeTargetStatusHandlesMissingAndBrokenLinks(t *testing.T) { + home := withTempHome(t) + target := opencodeTarget{} + + installed, version, err := target.Status() + if err != nil || installed || version != "" { + t.Fatalf("missing Status = %v %q %v", installed, version, err) + } + + link, err := target.skillLink(Catalog[0]) + if err != nil { + t.Fatal(err) + } + if err := os.MkdirAll(filepath.Dir(link), 0o755); err != nil { + t.Fatal(err) + } + if err := os.Symlink(filepath.Join(home, "missing"), link); err != nil { + t.Fatal(err) + } + if installed, version, err := target.Status(); err == nil || installed || version != "" { + t.Fatalf("broken-link Status = %v %q %v", installed, version, err) + } +} + +func TestOpenCodeTargetStatusResolvesCurrentLink(t *testing.T) { + home := withTempHome(t) + target := opencodeTarget{} + link, err := target.skillLink(Catalog[0]) + if err != nil { + t.Fatal(err) + } + if err := os.MkdirAll(filepath.Dir(link), 0o755); err != nil { + t.Fatal(err) + } + versionDir := filepath.Join(home, "canonical", "1.2.3") + if err := os.MkdirAll(versionDir, 0o755); err != nil { + t.Fatal(err) + } + if err := os.Symlink(versionDir, filepath.Join(filepath.Dir(link), "current")); err != nil { + t.Fatal(err) + } + if err := os.Symlink("current", link); err != nil { + t.Fatal(err) + } + if installed, version, err := target.Status(); err != nil || !installed || version != "1.2.3" { + t.Fatalf("current-link Status = %v %q %v", installed, version, err) + } +} + +func TestOpenCodeTargetStatusPreservesVersionFromRemovedDirectLink(t *testing.T) { + home := withTempHome(t) + target := opencodeTarget{} + link, err := target.skillLink(Catalog[0]) + if err != nil { + t.Fatal(err) + } + if err := os.MkdirAll(filepath.Dir(link), 0o755); err != nil { + t.Fatal(err) + } + if err := os.Symlink(filepath.Join(home, "canonical", "1.2.3"), link); err != nil { + t.Fatal(err) + } + + if installed, version, err := target.Status(); err != nil || !installed || version != "1.2.3" { + t.Fatalf("removed-direct-link Status = %v %q %v", installed, version, err) + } +} + +func TestOpenCodeTargetUninstallRemovesCatalogEntries(t *testing.T) { + withTempHome(t) + target := opencodeTarget{} + root, err := target.TargetPath() + if err != nil { + t.Fatal(err) + } + if err := os.MkdirAll(root, 0o755); err != nil { + t.Fatal(err) + } + for _, skill := range Catalog { + path, err := target.skillLink(skill) + if err != nil { + t.Fatal(err) + } + if err := os.MkdirAll(path, 0o755); err != nil { + t.Fatal(err) + } + } + if err := target.Uninstall(); err != nil { + t.Fatal(err) + } + for _, skill := range Catalog { + path, err := target.skillLink(skill) + if err != nil { + t.Fatal(err) + } + if _, err := os.Lstat(path); !os.IsNotExist(err) { + t.Fatalf("catalog entry %q remains after uninstall: %v", skill.Name, err) + } + } +} diff --git a/control-plane/internal/skillkit/testmain_test.go b/control-plane/internal/skillkit/testmain_test.go index 0fce5f08a..5244a1aca 100644 --- a/control-plane/internal/skillkit/testmain_test.go +++ b/control-plane/internal/skillkit/testmain_test.go @@ -147,6 +147,7 @@ func realHomeSnapshot(t *testing.T) string { filepath.Join(realHomeBeforeIsolation, ".codex", "skills"), filepath.Join(realHomeBeforeIsolation, ".codex", "AGENTS.override.md"), filepath.Join(realHomeBeforeIsolation, ".gemini", "GEMINI.md"), + filepath.Join(realHomeBeforeIsolation, ".config", "opencode", "skills"), filepath.Join(realHomeBeforeIsolation, ".config", "opencode", "AGENTS.md"), filepath.Join(realHomeBeforeIsolation, ".aider.conventions.md"), filepath.Join(realHomeBeforeIsolation, ".aider.conf.yml"),