From d55b132992c004aa611c96c735543d8b85c9f0e3 Mon Sep 17 00:00:00 2001 From: Facundo Farias Date: Mon, 22 Jun 2026 08:15:50 +0200 Subject: [PATCH 1/6] feat(skills): detect installed AI agents and install the DeployHQ skill MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add a Wrangler-style post-login hook and a standalone `dhq skills` command that detect AI coding agents on this machine and install the DeployHQ skill (skills/deployhq/) into each agent's native format. Twelve targets supported: - Aider (user-scope, owned file + wire-up note) - Antigravity (project-scope, repo-root AGENTS.md) - Claude Code (user-scope, ~/.claude/skills/ tree) - Cline (project-scope, .clinerules/deployhq.md) - Codex CLI (user-scope, ~/.codex/AGENTS.md sentinel section) - Continue.dev (user-scope, ~/.continue/rules/deployhq.md) - Cursor (user-scope, ~/.cursor/rules/deployhq.mdc) - Gemini CLI (user-scope, ~/.gemini/GEMINI.md sentinel section) - GitHub Copilot (project-scope, .github/copilot-instructions.md) - Kiro CLI (project-scope, .kiro/steering/deployhq.md) - OpenCode (user-scope, $XDG_CONFIG_HOME/opencode/AGENTS.md) - Windsurf (user-scope, ~/.codeium/windsurf/memories/global_rules.md) Key design choices: - Scope (User vs Project) is first-class on the Target interface; the hello prompt only auto-offers ScopeUser targets so we never mutate a user's repo as a side effect of logging in. - Targets that share an instructions file with the user (Windsurf, Codex, Gemini, OpenCode, Copilot, Antigravity) own only a sentinel-bounded section; user content outside the markers is preserved byte-for-byte across (re)installs. - Targets that own their file entirely (Aider, Cline, Continue) use a top-of-file `` HTML comment for versioning. - All installs are idempotent: install -> re-install produces byte-identical files. Outdated installs upgrade silently. - An optional Noter interface lets Aider surface the one-line `~/.aider.conf.yml` wire-up step we explicitly refuse to automate. The runtime agent (per harness.Detect) is auto-installed during `dhq hello` without prompting — if the user is running dhq from inside Claude Code right now, they want the skill. Other user-scope agents detected on disk are batched into a single Y/n prompt. Co-Authored-By: Claude Opus 4.7 (1M context) --- internal/commands/hello.go | 11 +- internal/commands/hello_skills.go | 98 +++++++++ internal/commands/root.go | 1 + internal/commands/skills.go | 114 +++++++++++ internal/skillinstaller/aider.go | 130 ++++++++++++ internal/skillinstaller/aider_test.go | 191 ++++++++++++++++++ internal/skillinstaller/antigravity.go | 133 ++++++++++++ internal/skillinstaller/antigravity_test.go | 151 ++++++++++++++ internal/skillinstaller/claude.go | 131 ++++++++++++ internal/skillinstaller/cline.go | 136 +++++++++++++ internal/skillinstaller/cline_test.go | 145 ++++++++++++++ internal/skillinstaller/codex.go | 121 +++++++++++ internal/skillinstaller/codex_test.go | 174 ++++++++++++++++ internal/skillinstaller/continue.go | 95 +++++++++ internal/skillinstaller/continue_test.go | 125 ++++++++++++ internal/skillinstaller/copilot.go | 157 +++++++++++++++ internal/skillinstaller/copilot_test.go | 203 +++++++++++++++++++ internal/skillinstaller/cursor.go | 129 ++++++++++++ internal/skillinstaller/cursor_test.go | 165 +++++++++++++++ internal/skillinstaller/flatten.go | 114 +++++++++++ internal/skillinstaller/frontmatter.go | 65 ++++++ internal/skillinstaller/gemini.go | 109 ++++++++++ internal/skillinstaller/gemini_test.go | 158 +++++++++++++++ internal/skillinstaller/installer.go | 178 +++++++++++++++++ internal/skillinstaller/installer_test.go | 171 ++++++++++++++++ internal/skillinstaller/kiro.go | 106 ++++++++++ internal/skillinstaller/kiro_test.go | 142 +++++++++++++ internal/skillinstaller/opencode.go | 119 +++++++++++ internal/skillinstaller/opencode_test.go | 146 ++++++++++++++ internal/skillinstaller/section.go | 58 ++++++ internal/skillinstaller/section_test.go | 65 ++++++ internal/skillinstaller/windsurf.go | 161 +++++++++++++++ internal/skillinstaller/windsurf_test.go | 211 ++++++++++++++++++++ skills/embed.go | 22 ++ 34 files changed, 4232 insertions(+), 3 deletions(-) create mode 100644 internal/commands/hello_skills.go create mode 100644 internal/commands/skills.go create mode 100644 internal/skillinstaller/aider.go create mode 100644 internal/skillinstaller/aider_test.go create mode 100644 internal/skillinstaller/antigravity.go create mode 100644 internal/skillinstaller/antigravity_test.go create mode 100644 internal/skillinstaller/claude.go create mode 100644 internal/skillinstaller/cline.go create mode 100644 internal/skillinstaller/cline_test.go create mode 100644 internal/skillinstaller/codex.go create mode 100644 internal/skillinstaller/codex_test.go create mode 100644 internal/skillinstaller/continue.go create mode 100644 internal/skillinstaller/continue_test.go create mode 100644 internal/skillinstaller/copilot.go create mode 100644 internal/skillinstaller/copilot_test.go create mode 100644 internal/skillinstaller/cursor.go create mode 100644 internal/skillinstaller/cursor_test.go create mode 100644 internal/skillinstaller/flatten.go create mode 100644 internal/skillinstaller/frontmatter.go create mode 100644 internal/skillinstaller/gemini.go create mode 100644 internal/skillinstaller/gemini_test.go create mode 100644 internal/skillinstaller/installer.go create mode 100644 internal/skillinstaller/installer_test.go create mode 100644 internal/skillinstaller/kiro.go create mode 100644 internal/skillinstaller/kiro_test.go create mode 100644 internal/skillinstaller/opencode.go create mode 100644 internal/skillinstaller/opencode_test.go create mode 100644 internal/skillinstaller/section.go create mode 100644 internal/skillinstaller/section_test.go create mode 100644 internal/skillinstaller/windsurf.go create mode 100644 internal/skillinstaller/windsurf_test.go create mode 100644 skills/embed.go diff --git a/internal/commands/hello.go b/internal/commands/hello.go index 5e58e20..ffaeaf7 100644 --- a/internal/commands/hello.go +++ b/internal/commands/hello.go @@ -58,7 +58,12 @@ func newHelloCmd() *cobra.Command { env.Status("Logged in as %s on %s.%s", creds.Email, creds.Account, host) } - // Step 2: Fetch projects + // Step 2: Offer to install the DeployHQ skill for any AI agents + // installed on this machine. Auto-installs for the runtime agent, + // prompts for others. Non-fatal — hello continues on errors. + offerSkillInstall(env) + + // Step 3: Fetch projects var sdkOpts []sdk.Option if baseURL := cliCtx.Config.BaseURL(creds.Account); baseURL != "" { sdkOpts = append(sdkOpts, sdk.WithBaseURL(baseURL)) @@ -101,7 +106,7 @@ func newHelloCmd() *cobra.Command { return initCmd.RunE(initCmd, nil) } - // Step 3: Default project + // Step 4: Default project defaultProject := cliCtx.Config.Project if defaultProject != "" { env.Status("Default project: %s", defaultProject) @@ -134,7 +139,7 @@ func newHelloCmd() *cobra.Command { output.ColorGreen.Fprintf(env.Stderr, "Saved to %s\n", path) //nolint:errcheck } - // Step 4: Orientation + // Step 5: Orientation env.Status("") env.Status("You're all set! Here are some useful commands:") env.Status(" dhq deploy Deploy your project") diff --git a/internal/commands/hello_skills.go b/internal/commands/hello_skills.go new file mode 100644 index 0000000..e586981 --- /dev/null +++ b/internal/commands/hello_skills.go @@ -0,0 +1,98 @@ +package commands + +import ( + "fmt" + "strings" + + "github.com/deployhq/deployhq-cli/internal/harness" + "github.com/deployhq/deployhq-cli/internal/output" + "github.com/deployhq/deployhq-cli/internal/skillinstaller" + "github.com/manifoldco/promptui" +) + +// offerSkillInstall is the Wrangler-style post-login hook that detects locally +// installed AI agents and offers to install the DeployHQ skill for them. +// +// Behaviour: +// - Runtime agent (the one currently running dhq, per harness.Detect) is +// auto-installed without prompting when an install is Needed — if the +// user is using dhq from inside Claude Code right now, they want it. +// - Other agents detected on disk are batched into a single Y/n prompt. +// - Errors are non-fatal: hello succeeds even if installs fail; users can +// re-run `dhq skills install` later. +// +// The function is a no-op when nothing is detected, when nothing needs +// installing, or when env.NonInteractive is set. +func offerSkillInstall(env *output.Envelope) { + detected := skillinstaller.DetectInstalled() + if len(detected) == 0 { + return + } + + runtimeName := harness.Detect().Name + + var runtime *skillinstaller.DetectResult + var others []skillinstaller.DetectResult + for i, d := range detected { + if !skillinstaller.Needed(d.Status) { + continue + } + // Project-scope targets (e.g. Copilot's .github/copilot-instructions.md) + // modify the current repo. Never install those as a side effect of + // 'dhq hello' — they're opt-in via 'dhq skills install --agent '. + if d.Target.Scope() != skillinstaller.ScopeUser { + continue + } + if d.Target.Name() == runtimeName { + runtime = &detected[i] + continue + } + others = append(others, d) + } + + if runtime != nil { + installOne(env, runtime.Target, "Installing DeployHQ skill for %s (you're using it now)…") + } + + if len(others) == 0 || env.NonInteractive { + return + } + + names := make([]string, len(others)) + for i, d := range others { + names[i] = d.Target.DisplayName() + } + label := fmt.Sprintf("Detected AI agents that could use the DeployHQ skill: %s.\n Install for them now?", strings.Join(names, ", ")) + + prompt := promptui.Prompt{ + Label: label, + IsConfirm: true, + Default: "Y", + } + if _, err := prompt.Run(); err != nil { + // User said no, or aborted — fine. + env.Status("Skipping. Run `dhq skills install` later if you change your mind.") + return + } + + for _, d := range others { + installOne(env, d.Target, "Installing DeployHQ skill for %s…") + } +} + +// installOne runs Install on a single target and prints a result line. +// statusFmt receives the DisplayName via %s. +func installOne(env *output.Envelope, t skillinstaller.Target, statusFmt string) { + env.Status(statusFmt, t.DisplayName()) + path, err := t.Install() + if err != nil { + env.Warn("Could not install %s skill: %v", t.DisplayName(), err) + return + } + output.ColorGreen.Fprintf(env.Stderr, " Installed %s skill → %s\n", t.DisplayName(), path) //nolint:errcheck + if n, ok := t.(skillinstaller.Noter); ok { + if note := n.PostInstallNote(); note != "" { + env.Status(" %s", note) + } + } +} diff --git a/internal/commands/root.go b/internal/commands/root.go index 3dac90c..df34926 100644 --- a/internal/commands/root.go +++ b/internal/commands/root.go @@ -230,6 +230,7 @@ Support: support@deployhq.com`, newShowCmd(), newURLCmd(), newSetupCmd(), + newSkillsCmd(), newMCPCmd(), newCompletionCmd(), newTelemetryCmd(), diff --git a/internal/commands/skills.go b/internal/commands/skills.go new file mode 100644 index 0000000..c73d8fc --- /dev/null +++ b/internal/commands/skills.go @@ -0,0 +1,114 @@ +package commands + +import ( + "fmt" + + "github.com/deployhq/deployhq-cli/internal/output" + "github.com/deployhq/deployhq-cli/internal/skillinstaller" + "github.com/spf13/cobra" +) + +func newSkillsCmd() *cobra.Command { + cmd := &cobra.Command{ + Use: "skills", + Short: "Manage DeployHQ skill installs for local AI agents", + Long: `Install the DeployHQ agent skill into AI coding tools on this machine +(Claude Code, Cursor, etc.). Run during 'dhq hello' or standalone. + +Examples: + dhq skills list Show detected agents and skill status + dhq skills install Install for every detected agent + dhq skills install --agent claude-code Install for a single agent`, + } + cmd.AddCommand(newSkillsListCmd()) + cmd.AddCommand(newSkillsInstallCmd()) + return cmd +} + +// skillRow is the rendered row type for `dhq skills list`. +type skillRow struct { + Name string `json:"name"` + DisplayName string `json:"display_name"` + Status string `json:"status"` +} + +func newSkillsListCmd() *cobra.Command { + return &cobra.Command{ + Use: "list", + Short: "List detected AI agents and skill install status", + RunE: func(cmd *cobra.Command, args []string) error { + env := cliCtx.Envelope + + rows := []skillRow{} + for _, t := range skillinstaller.All() { + rows = append(rows, skillRow{ + Name: t.Name(), + DisplayName: t.DisplayName(), + Status: t.Detect().String(), + }) + } + + return env.WriteData(rows, []string{"NAME", "DISPLAY NAME", "STATUS"}, func(v interface{}) []string { + r := v.(skillRow) + return []string{r.Name, r.DisplayName, r.Status} + }) + }, + } +} + +func newSkillsInstallCmd() *cobra.Command { + var agentFlag string + cmd := &cobra.Command{ + Use: "install", + Short: "Install the DeployHQ skill for detected (or named) AI agents", + RunE: func(cmd *cobra.Command, args []string) error { + env := cliCtx.Envelope + + var targets []skillinstaller.Target + if agentFlag != "" { + t := skillinstaller.Find(agentFlag) + if t == nil { + return &output.UserError{ + Message: fmt.Sprintf("Unknown agent: %q", agentFlag), + Hint: "Run 'dhq skills list' to see supported agents.", + } + } + targets = []skillinstaller.Target{t} + } else { + for _, d := range skillinstaller.DetectInstalled() { + targets = append(targets, d.Target) + } + } + + if len(targets) == 0 { + return &output.UserError{ + Message: "No supported AI agents detected on this machine", + Hint: "Install one (e.g. Claude Code) and re-run, or pass --agent .", + } + } + + var failed int + for _, t := range targets { + path, err := t.Install() + if err != nil { + env.Warn("Could not install %s skill: %v", t.DisplayName(), err) + failed++ + continue + } + output.ColorGreen.Fprintf(env.Stderr, "Installed %s skill → %s\n", t.DisplayName(), path) //nolint:errcheck + if n, ok := t.(skillinstaller.Noter); ok { + if note := n.PostInstallNote(); note != "" { + env.Status(" %s", note) + } + } + } + + if failed > 0 { + return &output.InternalError{Message: fmt.Sprintf("%d install(s) failed", failed)} + } + return nil + }, + } + cmd.Flags().StringVar(&agentFlag, "agent", "", "Install for a specific agent only (e.g. claude-code)") + return cmd +} diff --git a/internal/skillinstaller/aider.go b/internal/skillinstaller/aider.go new file mode 100644 index 0000000..e3ff7c4 --- /dev/null +++ b/internal/skillinstaller/aider.go @@ -0,0 +1,130 @@ +package skillinstaller + +import ( + "fmt" + "os" + "os/exec" + "path/filepath" + + "github.com/deployhq/deployhq-cli/skills" +) + +func init() { Register(&aider{}) } + +// aider installs the skill into a file Aider can read on demand. +// +// Aider doesn't auto-discover any file by name — unlike Copilot's +// .github/copilot-instructions.md or Claude's skills/ directory, every +// file Aider reads must be explicitly listed in .aider.conf.yml's `read:` +// directive or passed as `--read FILE` on the command line. +// +// Auto-editing .aider.conf.yml safely is hard (YAML doesn't have nestable +// block comments; the user may already have a `read:` key in a way that +// collides with our markers). So we install the skill file, then surface +// a PostInstallNote with the exact line to add. One manual step, zero +// risk of corrupting the user's config. +// +// Layout: +// - ~/.aider/deployhq-skill.md +// A single self-contained markdown file with a `` +// comment at the top for version tracking. Aider reads this as one +// conventions file; the version comment is invisible to the agent +// but lets Detect() know when to mark an install outdated. +// +// Detection uses `aider` on PATH because ~/.aider/ may not exist before +// any explicit setup, and config files are optional. +type aider struct{} + +func (aider) Name() string { return "aider" } +func (aider) DisplayName() string { return "Aider" } +func (aider) Scope() Scope { return ScopeUser } + +const ( + aiderSkillDir = ".aider" + aiderSkillFile = "deployhq-skill.md" +) + +// findAider is the binary-on-PATH lookup. Overridable in tests so they +// don't depend on whether the dev box actually has aider installed. +var findAider = func() bool { + _, err := exec.LookPath("aider") + return err == nil +} + +func (a aider) skillDir() (string, error) { + home, err := homeDir() + if err != nil { + return "", err + } + return filepath.Join(home, aiderSkillDir), nil +} + +func (a aider) skillPath() (string, error) { + dir, err := a.skillDir() + if err != nil { + return "", err + } + return filepath.Join(dir, aiderSkillFile), nil +} + +func (a aider) Detect() Status { + if !findAider() { + return StatusNotInstalled + } + p, err := a.skillPath() + if err != nil { + return StatusNotInstalled + } + data, err := os.ReadFile(p) + if err != nil { + return StatusAvailable + } + switch parseOwnedFileVersion(string(data)) { + case "": + // File exists but with no version marker — treat as available so + // the user gets a fresh install with proper versioning. We can't + // safely assume an unmarked file is ours. + return StatusAvailable + case skills.Version: + return StatusInstalled + default: + return StatusOutdated + } +} + +func (a aider) Install() (string, error) { + dir, err := a.skillDir() + if err != nil { + return "", err + } + if err := os.MkdirAll(dir, 0o755); err != nil { + return "", err + } + + body, err := buildOwnedSkillFile(skills.FS, "deployhq") + if err != nil { + return "", err + } + + dst := filepath.Join(dir, aiderSkillFile) + if err := os.WriteFile(dst, body, 0o644); err != nil { + return "", err + } + return dst, nil +} + +// PostInstallNote tells the user how to wire the skill into Aider, since +// we can't safely auto-edit .aider.conf.yml. Surfaced by both the hello +// hook and `dhq skills install` via the Noter interface. +func (a aider) PostInstallNote() string { + p, err := a.skillPath() + if err != nil { + return "" + } + return fmt.Sprintf( + "To load on every Aider run: add `read: [%s]` to ~/.aider.conf.yml "+ + "(or pass `--read %s` ad-hoc).", + p, p, + ) +} + diff --git a/internal/skillinstaller/aider_test.go b/internal/skillinstaller/aider_test.go new file mode 100644 index 0000000..9b7fc7c --- /dev/null +++ b/internal/skillinstaller/aider_test.go @@ -0,0 +1,191 @@ +package skillinstaller + +import ( + "os" + "path/filepath" + "strings" + "testing" + + "github.com/deployhq/deployhq-cli/skills" +) + +// withAiderPresent overrides the binary-on-PATH check for the duration of +// the test. We avoid depending on whether the dev machine actually has +// aider installed. +func withAiderPresent(t *testing.T, present bool) { + t.Helper() + orig := findAider + findAider = func() bool { return present } + t.Cleanup(func() { findAider = orig }) +} + +func TestAider_Detect_NoBinary(t *testing.T) { + withHomeDir(t, t.TempDir()) + withAiderPresent(t, false) + if got := (aider{}).Detect(); got != StatusNotInstalled { + t.Fatalf("Detect() without aider on PATH = %v, want StatusNotInstalled", got) + } +} + +func TestAider_Detect_BinaryPresentNoSkill(t *testing.T) { + withHomeDir(t, t.TempDir()) + withAiderPresent(t, true) + if got := (aider{}).Detect(); got != StatusAvailable { + t.Fatalf("Detect() = %v, want StatusAvailable", got) + } +} + +func TestAider_Install_WritesVersionedFile(t *testing.T) { + home := t.TempDir() + withHomeDir(t, home) + withAiderPresent(t, true) + + got, err := (aider{}).Install() + if err != nil { + t.Fatalf("Install() = %v", err) + } + want := filepath.Join(home, aiderSkillDir, aiderSkillFile) + if got != want { + t.Errorf("Install() path = %q, want %q", got, want) + } + + body, err := os.ReadFile(want) + if err != nil { + t.Fatal(err) + } + s := string(body) + if !strings.HasPrefix(s, ownedFileVersionPrefix+skills.Version+ownedFileVersionSuffix+"\n") { + t.Errorf("file must start with version marker; got:\n%.120q", s) + } + // SKILL.md body should be present after the marker. + if !strings.Contains(s, "DeployHQ CLI — Agent Skill Guide") { + t.Error("SKILL.md body missing from output") + } + // References should be inlined. + if !strings.Contains(s, "## reference: references/") { + t.Error("references not concatenated into output") + } +} + +func TestAider_Detect_InstalledThenOutdated(t *testing.T) { + home := t.TempDir() + withHomeDir(t, home) + withAiderPresent(t, true) + + if _, err := (aider{}).Install(); err != nil { + t.Fatal(err) + } + if got := (aider{}).Detect(); got != StatusInstalled { + t.Fatalf("post-install Detect() = %v, want StatusInstalled", got) + } + + // Tweak the marker to simulate an older version. + path := filepath.Join(home, aiderSkillDir, aiderSkillFile) + body, err := os.ReadFile(path) + if err != nil { + t.Fatal(err) + } + stale := strings.Replace(string(body), ownedFileVersionPrefix+skills.Version, ownedFileVersionPrefix+"0", 1) + if err := os.WriteFile(path, []byte(stale), 0o644); err != nil { + t.Fatal(err) + } + if got := (aider{}).Detect(); got != StatusOutdated { + t.Fatalf("Detect() stale marker = %v, want StatusOutdated", got) + } +} + +func TestAider_Detect_FileWithoutMarkerIsAvailable(t *testing.T) { + home := t.TempDir() + withHomeDir(t, home) + withAiderPresent(t, true) + + dir := filepath.Join(home, aiderSkillDir) + if err := os.MkdirAll(dir, 0o755); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(dir, aiderSkillFile), []byte("user wrote this\n"), 0o644); err != nil { + t.Fatal(err) + } + // We can't assume an unmarked file is ours — Detect() returns Available + // so the user can confirm and we'll overwrite with a versioned install. + if got := (aider{}).Detect(); got != StatusAvailable { + t.Fatalf("unmarked file Detect() = %v, want StatusAvailable", got) + } +} + +func TestAider_Install_Idempotent(t *testing.T) { + home := t.TempDir() + withHomeDir(t, home) + withAiderPresent(t, true) + + if _, err := (aider{}).Install(); err != nil { + t.Fatal(err) + } + first, err := os.ReadFile(filepath.Join(home, aiderSkillDir, aiderSkillFile)) + if err != nil { + t.Fatal(err) + } + if _, err := (aider{}).Install(); err != nil { + t.Fatalf("second Install() = %v", err) + } + second, err := os.ReadFile(filepath.Join(home, aiderSkillDir, aiderSkillFile)) + if err != nil { + t.Fatal(err) + } + if string(first) != string(second) { + t.Errorf("idempotence broken\n--- first ---\n%s\n--- second ---\n%s", first, second) + } +} + +func TestAider_PostInstallNote_MentionsExactPath(t *testing.T) { + home := t.TempDir() + withHomeDir(t, home) + + note := (aider{}).PostInstallNote() + if note == "" { + t.Fatal("PostInstallNote() returned empty string") + } + want := filepath.Join(home, aiderSkillDir, aiderSkillFile) + if !strings.Contains(note, want) { + t.Errorf("note doesn't mention exact path %q: %s", want, note) + } + if !strings.Contains(note, ".aider.conf.yml") { + t.Errorf("note should point at .aider.conf.yml: %s", note) + } +} + +func TestAider_ImplementsNoter(t *testing.T) { + var _ Noter = (*aider)(nil) +} + +func TestAider_Scope_IsUser(t *testing.T) { + if got := (aider{}).Scope(); got != ScopeUser { + t.Errorf("Scope() = %v, want ScopeUser", got) + } +} + +func TestRegistry_ContainsAider(t *testing.T) { + if Find("aider") == nil { + t.Fatal("aider target not registered") + } +} + +func TestParseOwnedFileVersion(t *testing.T) { + cases := []struct { + name string + in string + want string + }{ + {"empty", "", ""}, + {"no marker", "# user content\n", ""}, + {"v1", "\nbody\n", "1"}, + {"v42 mid file", "x\n\nbody\n", "42"}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + if got := parseOwnedFileVersion(tc.in); got != tc.want { + t.Errorf("got %q, want %q", got, tc.want) + } + }) + } +} diff --git a/internal/skillinstaller/antigravity.go b/internal/skillinstaller/antigravity.go new file mode 100644 index 0000000..2903571 --- /dev/null +++ b/internal/skillinstaller/antigravity.go @@ -0,0 +1,133 @@ +package skillinstaller + +import ( + "fmt" + "os" + "path/filepath" + "strings" + + "github.com/deployhq/deployhq-cli/skills" +) + +func init() { Register(&antigravity{}) } + +// antigravity installs the skill into the repo-root AGENTS.md file that +// Google's Antigravity IDE reads for project context. +// +// AGENTS.md is a cross-tool convention (Codex CLI inside projects, Amp, +// and a handful of others read it too), so we own only a sentinel-bounded +// section. User content — including instructions for other agents — is +// preserved byte-for-byte. +// +// Layout: +// - /AGENTS.md +// Sentinel-bounded section. +// - /.antigravity/deployhq/SKILL.md + references/*.md +// Full reference tree, namespaced under .antigravity/ to keep the +// repo root tidy. The section in AGENTS.md points at this tree by +// repo-relative path so it stays portable across clones. +// +// ScopeProject — modifies the user's repo, opt-in via: +// +// dhq skills install --agent antigravity +type antigravity struct{} + +func (antigravity) Name() string { return "antigravity" } +func (antigravity) DisplayName() string { return "Antigravity" } +func (antigravity) Scope() Scope { return ScopeProject } + +const ( + antigravityInstructionsFile = "AGENTS.md" + antigravityRefsDir = ".antigravity/deployhq" +) + +func (a antigravity) inRepo() bool { + dir, err := getCwd() + if err != nil { + return false + } + for { + if _, err := os.Stat(filepath.Join(dir, ".git")); err == nil { + return true + } + parent := filepath.Dir(dir) + if parent == dir { + return false + } + dir = parent + } +} + +func (a antigravity) Detect() Status { + if !a.inRepo() { + return StatusNotInstalled + } + cwd, err := getCwd() + if err != nil { + return StatusNotInstalled + } + data, err := os.ReadFile(filepath.Join(cwd, antigravityInstructionsFile)) + if err != nil { + return StatusAvailable + } + switch parseSectionVersion(string(data)) { + case "": + return StatusAvailable + case skills.Version: + return StatusInstalled + default: + return StatusOutdated + } +} + +func (a antigravity) Install() (string, error) { + if !a.inRepo() { + cwd, _ := getCwd() + return "", fmt.Errorf("not a git repository (cwd=%s); run from inside a repo", cwd) + } + cwd, err := getCwd() + if err != nil { + return "", err + } + + // Refresh the reference tree at /.antigravity/deployhq/. + refsRoot := filepath.Join(cwd, antigravityRefsDir) + if err := os.RemoveAll(refsRoot); err != nil { + return "", err + } + if err := writeEmbeddedTree(skills.FS, "deployhq", refsRoot); err != nil { + return "", err + } + + instrPath := filepath.Join(cwd, antigravityInstructionsFile) + existing, err := os.ReadFile(instrPath) + if err != nil && !os.IsNotExist(err) { + return "", err + } + + section := buildAntigravitySection(antigravityRefsDir) + merged := mergeSection(string(existing), section) + if err := os.WriteFile(instrPath, []byte(merged), 0o644); err != nil { + return "", err + } + return instrPath, nil +} + +// buildAntigravitySection produces the sentinel-bounded block in AGENTS.md. +// Repo-relative reference paths keep the rendered file portable across +// clones — agents resolve them from the repo root, not from where dhq +// happened to run. +func buildAntigravitySection(refsRelPath string) string { + var b strings.Builder + fmt.Fprintf(&b, "%s%s%s\n", sectionBeginPrefix, skills.Version, sectionBeginSuffix) + b.WriteString("This project uses DeployHQ. The `dhq` CLI is the canonical way to deploy ") + b.WriteString("code, manage projects, servers, and repos via the DeployHQ platform.\n\n") + b.WriteString("Skill guide and per-domain references (read before suggesting `dhq` commands):\n") + fmt.Fprintf(&b, " - %s/SKILL.md\n", refsRelPath) + fmt.Fprintf(&b, " - %s/references/*.md\n\n", refsRelPath) + b.WriteString("Domains: deployments, projects, servers, repos, configuration, operations, ") + b.WriteString("global-resources, auth-setup. When suggesting deploys, prefer `dhq deploy` and ") + b.WriteString("its flags over raw API calls.\n") + b.WriteString(sectionEndMarker) + return b.String() +} diff --git a/internal/skillinstaller/antigravity_test.go b/internal/skillinstaller/antigravity_test.go new file mode 100644 index 0000000..8f30b77 --- /dev/null +++ b/internal/skillinstaller/antigravity_test.go @@ -0,0 +1,151 @@ +package skillinstaller + +import ( + "os" + "path/filepath" + "strings" + "testing" + + "github.com/deployhq/deployhq-cli/skills" +) + +func TestAntigravity_Detect_NotInRepo(t *testing.T) { + withCwd(t, t.TempDir()) + if got := (antigravity{}).Detect(); got != StatusNotInstalled { + t.Fatalf("Detect() outside repo = %v, want StatusNotInstalled", got) + } +} + +func TestAntigravity_Detect_InRepoNoAgentsFile(t *testing.T) { + withCwd(t, fakeRepo(t)) + if got := (antigravity{}).Detect(); got != StatusAvailable { + t.Fatalf("Detect() = %v, want StatusAvailable", got) + } +} + +func TestAntigravity_Install_OutsideRepo_Errors(t *testing.T) { + withCwd(t, t.TempDir()) + if _, err := (antigravity{}).Install(); err == nil { + t.Fatal("expected error outside repo") + } +} + +func TestAntigravity_Install_WritesSectionAndRefs(t *testing.T) { + dir := fakeRepo(t) + withCwd(t, dir) + + got, err := (antigravity{}).Install() + if err != nil { + t.Fatalf("Install() = %v", err) + } + want := filepath.Join(dir, antigravityInstructionsFile) + if got != want { + t.Errorf("Install() path = %q, want %q", got, want) + } + + body, err := os.ReadFile(want) + if err != nil { + t.Fatal(err) + } + for _, must := range []string{ + sectionBeginPrefix + skills.Version + sectionBeginSuffix, + sectionEndMarker, + antigravityRefsDir + "/SKILL.md", + } { + if !strings.Contains(string(body), must) { + t.Errorf("AGENTS.md missing %q\n%s", must, body) + } + } + + if _, err := os.Stat(filepath.Join(dir, antigravityRefsDir, "SKILL.md")); err != nil { + t.Errorf("expected SKILL.md under refs root: %v", err) + } + refs, err := os.ReadDir(filepath.Join(dir, antigravityRefsDir, "references")) + if err != nil || len(refs) == 0 { + t.Errorf("expected references/*.md: err=%v entries=%d", err, len(refs)) + } +} + +func TestAntigravity_Install_PreservesCrossToolContent(t *testing.T) { + // AGENTS.md is a shared convention — the user may already have + // instructions for other tools in there. Those must survive install. + dir := fakeRepo(t) + withCwd(t, dir) + userContent := "# Instructions for All Agents\n\nAlways write tests.\n" + if err := os.WriteFile(filepath.Join(dir, antigravityInstructionsFile), []byte(userContent), 0o644); err != nil { + t.Fatal(err) + } + + if _, err := (antigravity{}).Install(); err != nil { + t.Fatalf("Install() = %v", err) + } + got, err := os.ReadFile(filepath.Join(dir, antigravityInstructionsFile)) + if err != nil { + t.Fatal(err) + } + if !strings.Contains(string(got), "Always write tests.") { + t.Errorf("cross-tool content lost:\n%s", got) + } + if !strings.Contains(string(got), sectionEndMarker) { + t.Errorf("DeployHQ section not appended:\n%s", got) + } +} + +func TestAntigravity_Install_Idempotent(t *testing.T) { + dir := fakeRepo(t) + withCwd(t, dir) + + if _, err := (antigravity{}).Install(); err != nil { + t.Fatal(err) + } + first, err := os.ReadFile(filepath.Join(dir, antigravityInstructionsFile)) + if err != nil { + t.Fatal(err) + } + if _, err := (antigravity{}).Install(); err != nil { + t.Fatalf("second Install() = %v", err) + } + second, err := os.ReadFile(filepath.Join(dir, antigravityInstructionsFile)) + if err != nil { + t.Fatal(err) + } + if string(first) != string(second) { + t.Errorf("idempotence broken\n--- first ---\n%s\n--- second ---\n%s", first, second) + } +} + +func TestAntigravity_Detect_Outdated(t *testing.T) { + dir := fakeRepo(t) + withCwd(t, dir) + if _, err := (antigravity{}).Install(); err != nil { + t.Fatal(err) + } + if got := (antigravity{}).Detect(); got != StatusInstalled { + t.Fatalf("post-install Detect() = %v, want StatusInstalled", got) + } + + path := filepath.Join(dir, antigravityInstructionsFile) + body, err := os.ReadFile(path) + if err != nil { + t.Fatal(err) + } + stale := strings.Replace(string(body), sectionBeginPrefix+skills.Version, sectionBeginPrefix+"0", 1) + if err := os.WriteFile(path, []byte(stale), 0o644); err != nil { + t.Fatal(err) + } + if got := (antigravity{}).Detect(); got != StatusOutdated { + t.Fatalf("Detect() stale marker = %v, want StatusOutdated", got) + } +} + +func TestAntigravity_Scope_IsProject(t *testing.T) { + if got := (antigravity{}).Scope(); got != ScopeProject { + t.Errorf("Scope() = %v, want ScopeProject", got) + } +} + +func TestRegistry_ContainsAntigravity(t *testing.T) { + if Find("antigravity") == nil { + t.Fatal("antigravity target not registered") + } +} diff --git a/internal/skillinstaller/claude.go b/internal/skillinstaller/claude.go new file mode 100644 index 0000000..efaa5e8 --- /dev/null +++ b/internal/skillinstaller/claude.go @@ -0,0 +1,131 @@ +package skillinstaller + +import ( + "io/fs" + "os" + "path/filepath" + "strings" + + "github.com/deployhq/deployhq-cli/skills" +) + +func init() { Register(&claudeCode{}) } + +// homeDir is the user home-directory lookup. Overridable in tests. +var homeDir = os.UserHomeDir + +// claudeCode installs the skill into Claude Code's user-level skills directory. +// +// Layout: ~/.claude/skills/deployhq/SKILL.md + references/*.md +// Version marker: ~/.claude/skills/deployhq/.deployhq-skill-version (one line, schema version) +// +// We chose user-level (~/.claude/skills/) over project-level (.claude/skills/) +// because dhq hello is typically run once per machine, not once per project. +// Users who want project-scoped skills can copy the directory themselves. +type claudeCode struct{} + +func (claudeCode) Name() string { return "claude-code" } +func (claudeCode) DisplayName() string { return "Claude Code" } +func (claudeCode) Scope() Scope { return ScopeUser } + +// skillDirName is the on-disk directory name under each agent's skills root. +// Matches the `name:` field in skills/deployhq/SKILL.md frontmatter so Claude +// Code finds it via its standard discovery flow. +const skillDirName = "deployhq" + +// versionMarker is the filename of the schema-version sentinel. +const versionMarker = ".deployhq-skill-version" + +func (c claudeCode) configDir() (string, error) { + home, err := homeDir() + if err != nil { + return "", err + } + return filepath.Join(home, ".claude"), nil +} + +func (c claudeCode) skillDir() (string, error) { + cfg, err := c.configDir() + if err != nil { + return "", err + } + return filepath.Join(cfg, "skills", skillDirName), nil +} + +func (c claudeCode) Detect() Status { + cfg, err := c.configDir() + if err != nil { + return StatusNotInstalled + } + if _, err := os.Stat(cfg); err != nil { + // No ~/.claude → Claude Code isn't installed (or has never run). + return StatusNotInstalled + } + + dir, err := c.skillDir() + if err != nil { + return StatusNotInstalled + } + if _, err := os.Stat(filepath.Join(dir, "SKILL.md")); err != nil { + return StatusAvailable + } + + // SKILL.md present — compare version markers. + switch readVersion(filepath.Join(dir, versionMarker)) { + case skills.Version: + return StatusInstalled + default: + return StatusOutdated + } +} + +func (c claudeCode) Install() (string, error) { + dir, err := c.skillDir() + if err != nil { + return "", err + } + if err := writeEmbeddedTree(skills.FS, "deployhq", dir); err != nil { + return "", err + } + if err := os.WriteFile(filepath.Join(dir, versionMarker), []byte(skills.Version+"\n"), 0o644); err != nil { + return "", err + } + return dir, nil +} + +// writeEmbeddedTree copies srcRoot from the embedded FS into dst on disk, +// creating directories as needed. Existing files are overwritten — this is +// the simplest definition of idempotent and matches what users expect from +// "re-install" or "upgrade". +func writeEmbeddedTree(efs fs.FS, srcRoot, dst string) error { + return fs.WalkDir(efs, srcRoot, func(p string, d fs.DirEntry, err error) error { + if err != nil { + return err + } + rel := strings.TrimPrefix(p, srcRoot) + rel = strings.TrimPrefix(rel, "/") + out := filepath.Join(dst, rel) + + if d.IsDir() { + return os.MkdirAll(out, 0o755) + } + data, err := fs.ReadFile(efs, p) + if err != nil { + return err + } + if err := os.MkdirAll(filepath.Dir(out), 0o755); err != nil { + return err + } + return os.WriteFile(out, data, 0o644) + }) +} + +// readVersion returns the trimmed contents of the version marker, or empty +// string if the file is missing or unreadable. +func readVersion(path string) string { + b, err := os.ReadFile(path) + if err != nil { + return "" + } + return strings.TrimSpace(string(b)) +} diff --git a/internal/skillinstaller/cline.go b/internal/skillinstaller/cline.go new file mode 100644 index 0000000..da15142 --- /dev/null +++ b/internal/skillinstaller/cline.go @@ -0,0 +1,136 @@ +package skillinstaller + +import ( + "fmt" + "os" + "path/filepath" + + "github.com/deployhq/deployhq-cli/skills" +) + +func init() { Register(&cline{}) } + +// cline installs the skill as a project-level Cline rule. +// +// Layout: /.clinerules/deployhq.md +// +// Cline's project rules support two shapes: a single-file `.clinerules` +// (legacy) or a `.clinerules/` directory of *.md files (newer, preferred). +// We always write the directory form. If the user has the file form, we +// refuse with a clear migration hint rather than silently destroying it. +// +// Like Copilot, this is ScopeProject — modifying a user's repo as a side +// effect of `dhq hello` would be hostile. Opt in via: +// +// dhq skills install --agent cline +type cline struct{} + +func (cline) Name() string { return "cline" } +func (cline) DisplayName() string { return "Cline" } +func (cline) Scope() Scope { return ScopeProject } + +const ( + clineRulesDir = ".clinerules" + clineSkillFile = "deployhq.md" +) + +// inRepo mirrors copilot's ancestor-walk: any .git/ in the cwd or an +// ancestor means we're inside a project where installing makes sense. +func (c cline) inRepo() bool { + dir, err := getCwd() + if err != nil { + return false + } + for { + if _, err := os.Stat(filepath.Join(dir, ".git")); err == nil { + return true + } + parent := filepath.Dir(dir) + if parent == dir { + return false + } + dir = parent + } +} + +// skillFile returns /.clinerules/deployhq.md. +func (c cline) skillFile() (string, error) { + cwd, err := getCwd() + if err != nil { + return "", err + } + return filepath.Join(cwd, clineRulesDir, clineSkillFile), nil +} + +// rulesPathStat returns info about /.clinerules so callers can tell +// the difference between "absent", "directory" (ours), and "file" (legacy +// single-file form, which would conflict with our directory writes). +func (c cline) rulesPathStat() (os.FileInfo, error) { + cwd, err := getCwd() + if err != nil { + return nil, err + } + return os.Stat(filepath.Join(cwd, clineRulesDir)) +} + +func (c cline) Detect() Status { + if !c.inRepo() { + return StatusNotInstalled + } + + // If .clinerules exists as a file (legacy shape), we can't safely + // proceed. Report Available so it shows up in listings, and let + // Install() fail loudly with an actionable message. + if info, err := c.rulesPathStat(); err == nil && !info.IsDir() { + return StatusAvailable + } + + p, err := c.skillFile() + if err != nil { + return StatusNotInstalled + } + data, err := os.ReadFile(p) + if err != nil { + return StatusAvailable + } + switch parseOwnedFileVersion(string(data)) { + case "": + return StatusAvailable + case skills.Version: + return StatusInstalled + default: + return StatusOutdated + } +} + +func (c cline) Install() (string, error) { + if !c.inRepo() { + cwd, _ := getCwd() + return "", fmt.Errorf("not a git repository (cwd=%s); run from inside a repo", cwd) + } + if info, err := c.rulesPathStat(); err == nil && !info.IsDir() { + return "", fmt.Errorf( + "%s exists as a file (legacy Cline single-rule shape); "+ + "move its contents into %s/main.md (or any *.md name), delete the file, "+ + "then re-run `dhq skills install --agent cline`", + clineRulesDir, clineRulesDir, + ) + } + + p, err := c.skillFile() + if err != nil { + return "", err + } + if err := os.MkdirAll(filepath.Dir(p), 0o755); err != nil { + return "", err + } + + body, err := buildOwnedSkillFile(skills.FS, "deployhq") + if err != nil { + return "", err + } + if err := os.WriteFile(p, body, 0o644); err != nil { + return "", err + } + return p, nil +} diff --git a/internal/skillinstaller/cline_test.go b/internal/skillinstaller/cline_test.go new file mode 100644 index 0000000..18ea36b --- /dev/null +++ b/internal/skillinstaller/cline_test.go @@ -0,0 +1,145 @@ +package skillinstaller + +import ( + "os" + "path/filepath" + "strings" + "testing" + + "github.com/deployhq/deployhq-cli/skills" +) + +func TestCline_Detect_NotInRepo(t *testing.T) { + withCwd(t, t.TempDir()) + if got := (cline{}).Detect(); got != StatusNotInstalled { + t.Fatalf("Detect() outside repo = %v, want StatusNotInstalled", got) + } +} + +func TestCline_Detect_InRepoNoRules(t *testing.T) { + withCwd(t, fakeRepo(t)) + if got := (cline{}).Detect(); got != StatusAvailable { + t.Fatalf("Detect() = %v, want StatusAvailable", got) + } +} + +func TestCline_Detect_LegacyFileShape(t *testing.T) { + dir := fakeRepo(t) + withCwd(t, dir) + // .clinerules exists as a single file (legacy shape). + if err := os.WriteFile(filepath.Join(dir, clineRulesDir), []byte("user rule\n"), 0o644); err != nil { + t.Fatal(err) + } + // Detect still reports Available so it shows up in `dhq skills list`; + // Install will refuse with an actionable message. + if got := (cline{}).Detect(); got != StatusAvailable { + t.Fatalf("Detect() with legacy file = %v, want StatusAvailable", got) + } +} + +func TestCline_Install_OutsideRepo_Errors(t *testing.T) { + withCwd(t, t.TempDir()) + if _, err := (cline{}).Install(); err == nil { + t.Fatal("expected error outside repo") + } +} + +func TestCline_Install_LegacyFileShape_Errors(t *testing.T) { + dir := fakeRepo(t) + withCwd(t, dir) + if err := os.WriteFile(filepath.Join(dir, clineRulesDir), []byte("legacy\n"), 0o644); err != nil { + t.Fatal(err) + } + _, err := (cline{}).Install() + if err == nil { + t.Fatal("expected error when .clinerules is a file") + } + if !strings.Contains(err.Error(), "legacy Cline single-rule shape") { + t.Errorf("expected actionable migration hint, got: %v", err) + } +} + +func TestCline_Install_WritesSkillFile(t *testing.T) { + dir := fakeRepo(t) + withCwd(t, dir) + + got, err := (cline{}).Install() + if err != nil { + t.Fatalf("Install() = %v", err) + } + want := filepath.Join(dir, clineRulesDir, clineSkillFile) + if got != want { + t.Errorf("Install() path = %q, want %q", got, want) + } + + body, err := os.ReadFile(want) + if err != nil { + t.Fatal(err) + } + s := string(body) + if !strings.HasPrefix(s, ownedFileVersionPrefix+skills.Version+ownedFileVersionSuffix+"\n") { + t.Errorf("file must start with version marker; got: %.120q", s) + } + if !strings.Contains(s, "DeployHQ CLI — Agent Skill Guide") { + t.Error("SKILL.md body missing from output") + } +} + +func TestCline_Detect_InstalledThenOutdated(t *testing.T) { + dir := fakeRepo(t) + withCwd(t, dir) + if _, err := (cline{}).Install(); err != nil { + t.Fatal(err) + } + if got := (cline{}).Detect(); got != StatusInstalled { + t.Fatalf("post-install Detect() = %v, want StatusInstalled", got) + } + + path := filepath.Join(dir, clineRulesDir, clineSkillFile) + body, err := os.ReadFile(path) + if err != nil { + t.Fatal(err) + } + stale := strings.Replace(string(body), ownedFileVersionPrefix+skills.Version, ownedFileVersionPrefix+"0", 1) + if err := os.WriteFile(path, []byte(stale), 0o644); err != nil { + t.Fatal(err) + } + if got := (cline{}).Detect(); got != StatusOutdated { + t.Fatalf("Detect() stale marker = %v, want StatusOutdated", got) + } +} + +func TestCline_Install_Idempotent(t *testing.T) { + dir := fakeRepo(t) + withCwd(t, dir) + + if _, err := (cline{}).Install(); err != nil { + t.Fatal(err) + } + first, err := os.ReadFile(filepath.Join(dir, clineRulesDir, clineSkillFile)) + if err != nil { + t.Fatal(err) + } + if _, err := (cline{}).Install(); err != nil { + t.Fatalf("second Install() = %v", err) + } + second, err := os.ReadFile(filepath.Join(dir, clineRulesDir, clineSkillFile)) + if err != nil { + t.Fatal(err) + } + if string(first) != string(second) { + t.Errorf("idempotence broken\n--- first ---\n%s\n--- second ---\n%s", first, second) + } +} + +func TestCline_Scope_IsProject(t *testing.T) { + if got := (cline{}).Scope(); got != ScopeProject { + t.Errorf("Scope() = %v, want ScopeProject", got) + } +} + +func TestRegistry_ContainsCline(t *testing.T) { + if Find("cline") == nil { + t.Fatal("cline target not registered") + } +} diff --git a/internal/skillinstaller/codex.go b/internal/skillinstaller/codex.go new file mode 100644 index 0000000..f1c2dc8 --- /dev/null +++ b/internal/skillinstaller/codex.go @@ -0,0 +1,121 @@ +package skillinstaller + +import ( + "fmt" + "os" + "path/filepath" + "strings" + + "github.com/deployhq/deployhq-cli/skills" +) + +func init() { Register(&codex{}) } + +// codex installs the skill into OpenAI's Codex CLI user-level instructions. +// +// Layout: +// - ~/.codex/AGENTS.md +// A single file Codex CLI reads for global agent instructions. We own +// only a sentinel-bounded section in it; user content is preserved +// byte-for-byte across (re)installs. +// - ~/.codex/deployhq-references/SKILL.md + references/*.md +// The full reference tree, written so the agent can pull in detail on +// demand. The instructions section in AGENTS.md is intentionally short +// and points at this tree by absolute path. +// +// Same shape as the Windsurf target — single shared instructions file with +// section-level coexistence. The only differences are the paths and the +// per-target intro text. +type codex struct{} + +func (codex) Name() string { return "codex" } +func (codex) DisplayName() string { return "Codex CLI" } +func (codex) Scope() Scope { return ScopeUser } + +const ( + codexAgentsFile = "AGENTS.md" + codexRefsDir = "deployhq-references" +) + +func (c codex) configDir() (string, error) { + home, err := homeDir() + if err != nil { + return "", err + } + return filepath.Join(home, ".codex"), nil +} + +func (c codex) Detect() Status { + cfg, err := c.configDir() + if err != nil { + return StatusNotInstalled + } + if _, err := os.Stat(cfg); err != nil { + // No ~/.codex → Codex CLI has never run on this machine. + return StatusNotInstalled + } + + data, err := os.ReadFile(filepath.Join(cfg, codexAgentsFile)) + if err != nil { + // Codex installed but AGENTS.md not written yet — skill available. + return StatusAvailable + } + switch parseSectionVersion(string(data)) { + case "": + return StatusAvailable + case skills.Version: + return StatusInstalled + default: + return StatusOutdated + } +} + +func (c codex) Install() (string, error) { + cfg, err := c.configDir() + if err != nil { + return "", err + } + if err := os.MkdirAll(cfg, 0o755); err != nil { + return "", err + } + + // Refresh the reference tree at ~/.codex/deployhq-references/. + refsRoot := filepath.Join(cfg, codexRefsDir) + if err := os.RemoveAll(refsRoot); err != nil { + return "", err + } + if err := writeEmbeddedTree(skills.FS, "deployhq", refsRoot); err != nil { + return "", err + } + + agentsPath := filepath.Join(cfg, codexAgentsFile) + existing, err := os.ReadFile(agentsPath) + if err != nil && !os.IsNotExist(err) { + return "", err + } + + section := buildCodexSection(refsRoot) + merged := mergeSection(string(existing), section) + if err := os.WriteFile(agentsPath, []byte(merged), 0o644); err != nil { + return "", err + } + return agentsPath, nil +} + +// buildCodexSection produces the sentinel-bounded block we own in +// ~/.codex/AGENTS.md. The reference path is absolute because Codex CLI +// reads AGENTS.md verbatim — agents won't infer the user's home directory. +func buildCodexSection(refsAbsPath string) string { + var b strings.Builder + fmt.Fprintf(&b, "%s%s%s\n", sectionBeginPrefix, skills.Version, sectionBeginSuffix) + b.WriteString("DeployHQ CLI (`dhq`) is available. Use it to deploy code, manage projects, ") + b.WriteString("servers, and repos via the DeployHQ platform.\n\n") + b.WriteString("Skill guide and per-domain references (read before suggesting `dhq` commands):\n") + fmt.Fprintf(&b, " - %s/SKILL.md\n", refsAbsPath) + fmt.Fprintf(&b, " - %s/references/*.md\n\n", refsAbsPath) + b.WriteString("Domains: deployments, projects, servers, repos, configuration, operations, ") + b.WriteString("global-resources, auth-setup. When the user asks to deploy, prefer `dhq deploy` ") + b.WriteString("with its flags over raw API calls.\n") + b.WriteString(sectionEndMarker) + return b.String() +} diff --git a/internal/skillinstaller/codex_test.go b/internal/skillinstaller/codex_test.go new file mode 100644 index 0000000..f9eb2c4 --- /dev/null +++ b/internal/skillinstaller/codex_test.go @@ -0,0 +1,174 @@ +package skillinstaller + +import ( + "os" + "path/filepath" + "strings" + "testing" + + "github.com/deployhq/deployhq-cli/skills" +) + +func TestCodex_Detect_NoConfigDir(t *testing.T) { + withHomeDir(t, t.TempDir()) + if got := (codex{}).Detect(); got != StatusNotInstalled { + t.Fatalf("Detect() = %v, want StatusNotInstalled", got) + } +} + +func TestCodex_Detect_InstalledNoAgentsFile(t *testing.T) { + home := t.TempDir() + withHomeDir(t, home) + if err := os.MkdirAll(filepath.Join(home, ".codex"), 0o755); err != nil { + t.Fatal(err) + } + if got := (codex{}).Detect(); got != StatusAvailable { + t.Fatalf("Detect() = %v, want StatusAvailable", got) + } +} + +func TestCodex_Detect_AgentsFileWithoutSection(t *testing.T) { + home := t.TempDir() + withHomeDir(t, home) + cfg := filepath.Join(home, ".codex") + if err := os.MkdirAll(cfg, 0o755); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(cfg, codexAgentsFile), []byte("# User instructions\nbe concise\n"), 0o644); err != nil { + t.Fatal(err) + } + if got := (codex{}).Detect(); got != StatusAvailable { + t.Fatalf("Detect() with unrelated content = %v, want StatusAvailable", got) + } +} + +func TestCodex_Install_WritesSectionAndRefs(t *testing.T) { + home := t.TempDir() + withHomeDir(t, home) + if err := os.MkdirAll(filepath.Join(home, ".codex"), 0o755); err != nil { + t.Fatal(err) + } + + got, err := (codex{}).Install() + if err != nil { + t.Fatalf("Install() = %v", err) + } + want := filepath.Join(home, ".codex", codexAgentsFile) + if got != want { + t.Errorf("Install() path = %q, want %q", got, want) + } + + body, err := os.ReadFile(want) + if err != nil { + t.Fatal(err) + } + for _, must := range []string{ + sectionBeginPrefix + skills.Version + sectionBeginSuffix, + sectionEndMarker, + filepath.Join(home, ".codex", codexRefsDir, "SKILL.md"), + } { + if !strings.Contains(string(body), must) { + t.Errorf("AGENTS.md missing %q\n%s", must, body) + } + } + + if _, err := os.Stat(filepath.Join(home, ".codex", codexRefsDir, "SKILL.md")); err != nil { + t.Errorf("expected SKILL.md under refs root: %v", err) + } + refs, err := os.ReadDir(filepath.Join(home, ".codex", codexRefsDir, "references")) + if err != nil || len(refs) == 0 { + t.Errorf("expected references/*.md: err=%v entries=%d", err, len(refs)) + } +} + +func TestCodex_Install_PreservesUserContent(t *testing.T) { + home := t.TempDir() + withHomeDir(t, home) + cfg := filepath.Join(home, ".codex") + if err := os.MkdirAll(cfg, 0o755); err != nil { + t.Fatal(err) + } + userContent := "# My agent rules\n\nAlways explain changes.\n" + if err := os.WriteFile(filepath.Join(cfg, codexAgentsFile), []byte(userContent), 0o644); err != nil { + t.Fatal(err) + } + + if _, err := (codex{}).Install(); err != nil { + t.Fatalf("Install() = %v", err) + } + got, err := os.ReadFile(filepath.Join(cfg, codexAgentsFile)) + if err != nil { + t.Fatal(err) + } + if !strings.Contains(string(got), "Always explain changes.") { + t.Errorf("user content lost:\n%s", got) + } + if !strings.Contains(string(got), sectionEndMarker) { + t.Errorf("DeployHQ section not appended:\n%s", got) + } +} + +func TestCodex_Install_Idempotent(t *testing.T) { + home := t.TempDir() + withHomeDir(t, home) + if err := os.MkdirAll(filepath.Join(home, ".codex"), 0o755); err != nil { + t.Fatal(err) + } + + if _, err := (codex{}).Install(); err != nil { + t.Fatal(err) + } + first, err := os.ReadFile(filepath.Join(home, ".codex", codexAgentsFile)) + if err != nil { + t.Fatal(err) + } + if _, err := (codex{}).Install(); err != nil { + t.Fatalf("second Install() = %v", err) + } + second, err := os.ReadFile(filepath.Join(home, ".codex", codexAgentsFile)) + if err != nil { + t.Fatal(err) + } + if string(first) != string(second) { + t.Errorf("idempotence broken\n--- first ---\n%s\n--- second ---\n%s", first, second) + } +} + +func TestCodex_Detect_Outdated(t *testing.T) { + home := t.TempDir() + withHomeDir(t, home) + if err := os.MkdirAll(filepath.Join(home, ".codex"), 0o755); err != nil { + t.Fatal(err) + } + if _, err := (codex{}).Install(); err != nil { + t.Fatal(err) + } + if got := (codex{}).Detect(); got != StatusInstalled { + t.Fatalf("post-install Detect() = %v, want StatusInstalled", got) + } + + path := filepath.Join(home, ".codex", codexAgentsFile) + body, err := os.ReadFile(path) + if err != nil { + t.Fatal(err) + } + stale := strings.Replace(string(body), sectionBeginPrefix+skills.Version, sectionBeginPrefix+"0", 1) + if err := os.WriteFile(path, []byte(stale), 0o644); err != nil { + t.Fatal(err) + } + if got := (codex{}).Detect(); got != StatusOutdated { + t.Fatalf("Detect() stale marker = %v, want StatusOutdated", got) + } +} + +func TestCodex_Scope_IsUser(t *testing.T) { + if got := (codex{}).Scope(); got != ScopeUser { + t.Errorf("Scope() = %v, want ScopeUser", got) + } +} + +func TestRegistry_ContainsCodex(t *testing.T) { + if Find("codex") == nil { + t.Fatal("codex target not registered") + } +} diff --git a/internal/skillinstaller/continue.go b/internal/skillinstaller/continue.go new file mode 100644 index 0000000..19eb67d --- /dev/null +++ b/internal/skillinstaller/continue.go @@ -0,0 +1,95 @@ +package skillinstaller + +import ( + "os" + "path/filepath" + + "github.com/deployhq/deployhq-cli/skills" +) + +func init() { Register(&continueDev{}) } + +// continueDev installs the skill as a user-level Continue.dev rule. +// +// Layout: +// - ~/.continue/rules/deployhq.md +// +// Newer Continue versions auto-load every *.md file under ~/.continue/rules/ +// — no config wiring needed. We own the file entirely, so version tracking +// uses the top-of-file HTML comment marker (same as Aider/Cline). +// +// The type is named continueDev (not continue) because `continue` is a Go +// reserved word. Name() still returns the user-facing "continue" string. +type continueDev struct{} + +func (continueDev) Name() string { return "continue" } +func (continueDev) DisplayName() string { return "Continue.dev" } +func (continueDev) Scope() Scope { return ScopeUser } + +const ( + continueConfigDir = ".continue" + continueRulesDir = "rules" + continueSkillFile = "deployhq.md" +) + +func (c continueDev) configDir() (string, error) { + home, err := homeDir() + if err != nil { + return "", err + } + return filepath.Join(home, continueConfigDir), nil +} + +func (c continueDev) skillFile() (string, error) { + cfg, err := c.configDir() + if err != nil { + return "", err + } + return filepath.Join(cfg, continueRulesDir, continueSkillFile), nil +} + +func (c continueDev) Detect() Status { + cfg, err := c.configDir() + if err != nil { + return StatusNotInstalled + } + if _, err := os.Stat(cfg); err != nil { + return StatusNotInstalled + } + + p, err := c.skillFile() + if err != nil { + return StatusNotInstalled + } + data, err := os.ReadFile(p) + if err != nil { + return StatusAvailable + } + switch parseOwnedFileVersion(string(data)) { + case "": + return StatusAvailable + case skills.Version: + return StatusInstalled + default: + return StatusOutdated + } +} + +func (c continueDev) Install() (string, error) { + p, err := c.skillFile() + if err != nil { + return "", err + } + if err := os.MkdirAll(filepath.Dir(p), 0o755); err != nil { + return "", err + } + + body, err := buildOwnedSkillFile(skills.FS, "deployhq") + if err != nil { + return "", err + } + if err := os.WriteFile(p, body, 0o644); err != nil { + return "", err + } + return p, nil +} diff --git a/internal/skillinstaller/continue_test.go b/internal/skillinstaller/continue_test.go new file mode 100644 index 0000000..b7a2ba5 --- /dev/null +++ b/internal/skillinstaller/continue_test.go @@ -0,0 +1,125 @@ +package skillinstaller + +import ( + "os" + "path/filepath" + "strings" + "testing" + + "github.com/deployhq/deployhq-cli/skills" +) + +func TestContinue_Detect_NoConfigDir(t *testing.T) { + withHomeDir(t, t.TempDir()) + if got := (continueDev{}).Detect(); got != StatusNotInstalled { + t.Fatalf("Detect() = %v, want StatusNotInstalled", got) + } +} + +func TestContinue_Detect_InstalledNoSkillFile(t *testing.T) { + home := t.TempDir() + withHomeDir(t, home) + if err := os.MkdirAll(filepath.Join(home, continueConfigDir), 0o755); err != nil { + t.Fatal(err) + } + if got := (continueDev{}).Detect(); got != StatusAvailable { + t.Fatalf("Detect() = %v, want StatusAvailable", got) + } +} + +func TestContinue_Install_WritesVersionedFile(t *testing.T) { + home := t.TempDir() + withHomeDir(t, home) + if err := os.MkdirAll(filepath.Join(home, continueConfigDir), 0o755); err != nil { + t.Fatal(err) + } + + got, err := (continueDev{}).Install() + if err != nil { + t.Fatalf("Install() = %v", err) + } + want := filepath.Join(home, continueConfigDir, continueRulesDir, continueSkillFile) + if got != want { + t.Errorf("Install() path = %q, want %q", got, want) + } + + body, err := os.ReadFile(want) + if err != nil { + t.Fatal(err) + } + s := string(body) + if !strings.HasPrefix(s, ownedFileVersionPrefix+skills.Version+ownedFileVersionSuffix+"\n") { + t.Errorf("file must start with version marker; got: %.120q", s) + } + if !strings.Contains(s, "DeployHQ CLI — Agent Skill Guide") { + t.Error("SKILL.md body missing from output") + } + if !strings.Contains(s, "## reference: references/") { + t.Error("references not concatenated into output") + } +} + +func TestContinue_Detect_InstalledThenOutdated(t *testing.T) { + home := t.TempDir() + withHomeDir(t, home) + if err := os.MkdirAll(filepath.Join(home, continueConfigDir), 0o755); err != nil { + t.Fatal(err) + } + if _, err := (continueDev{}).Install(); err != nil { + t.Fatal(err) + } + if got := (continueDev{}).Detect(); got != StatusInstalled { + t.Fatalf("post-install Detect() = %v, want StatusInstalled", got) + } + + path := filepath.Join(home, continueConfigDir, continueRulesDir, continueSkillFile) + body, err := os.ReadFile(path) + if err != nil { + t.Fatal(err) + } + stale := strings.Replace(string(body), ownedFileVersionPrefix+skills.Version, ownedFileVersionPrefix+"0", 1) + if err := os.WriteFile(path, []byte(stale), 0o644); err != nil { + t.Fatal(err) + } + if got := (continueDev{}).Detect(); got != StatusOutdated { + t.Fatalf("Detect() stale marker = %v, want StatusOutdated", got) + } +} + +func TestContinue_Install_Idempotent(t *testing.T) { + home := t.TempDir() + withHomeDir(t, home) + if err := os.MkdirAll(filepath.Join(home, continueConfigDir), 0o755); err != nil { + t.Fatal(err) + } + + if _, err := (continueDev{}).Install(); err != nil { + t.Fatal(err) + } + first, err := os.ReadFile(filepath.Join(home, continueConfigDir, continueRulesDir, continueSkillFile)) + if err != nil { + t.Fatal(err) + } + if _, err := (continueDev{}).Install(); err != nil { + t.Fatalf("second Install() = %v", err) + } + second, err := os.ReadFile(filepath.Join(home, continueConfigDir, continueRulesDir, continueSkillFile)) + if err != nil { + t.Fatal(err) + } + if string(first) != string(second) { + t.Errorf("idempotence broken\n--- first ---\n%s\n--- second ---\n%s", first, second) + } +} + +func TestContinue_Scope_IsUser(t *testing.T) { + if got := (continueDev{}).Scope(); got != ScopeUser { + t.Errorf("Scope() = %v, want ScopeUser", got) + } +} + +func TestRegistry_ContainsContinue(t *testing.T) { + if Find("continue") == nil { + t.Fatal("continue target not registered") + } +} diff --git a/internal/skillinstaller/copilot.go b/internal/skillinstaller/copilot.go new file mode 100644 index 0000000..24f8630 --- /dev/null +++ b/internal/skillinstaller/copilot.go @@ -0,0 +1,157 @@ +package skillinstaller + +import ( + "fmt" + "os" + "path/filepath" + "strings" + + "github.com/deployhq/deployhq-cli/skills" +) + +func init() { Register(&copilot{}) } + +// copilot installs the skill into GitHub Copilot's repo-level instructions. +// +// Unlike Claude/Cursor/Windsurf, Copilot has no reliable per-user "is it +// installed" signal — it could be a VS Code extension, a JetBrains plugin, +// `gh copilot`, or none of these. The thing Copilot consistently *does* read +// is `.github/copilot-instructions.md` in the current repo. So this target +// is project-scope: install writes to the cwd, not the user's home. +// +// Layout: +// - /.github/copilot-instructions.md +// Sentinel-bounded section that coexists with the user's own +// instructions. Replaced in place on upgrade; user content outside +// the markers is preserved. +// - /.github/copilot/deployhq/SKILL.md + references/*.md +// Full reference tree the agent can read on demand. Keeping the +// instructions section terse and parking detail in a side directory +// respects Copilot's instruction-length budget. +// +// Because installing modifies the user's repo, this target is excluded +// from the post-login 'dhq hello' prompt (see Scope == ScopeProject). +// Users opt in explicitly with 'dhq skills install --agent copilot'. +type copilot struct{} + +func (copilot) Name() string { return "copilot" } +func (copilot) DisplayName() string { return "GitHub Copilot" } +func (copilot) Scope() Scope { return ScopeProject } + +const ( + copilotInstructionsFile = ".github/copilot-instructions.md" + copilotRefsDir = ".github/copilot/deployhq" +) + +// getCwd is the working-directory lookup. Overridable in tests. +var getCwd = os.Getwd + +func (c copilot) repoRoot() (string, error) { + cwd, err := getCwd() + if err != nil { + return "", err + } + return cwd, nil +} + +// inRepo reports whether the cwd looks like a git repo. We accept any +// ancestor having .git/, which matches how someone running 'dhq' inside a +// subdirectory of their project would expect this to behave. +func (c copilot) inRepo() bool { + dir, err := c.repoRoot() + if err != nil { + return false + } + for { + if _, err := os.Stat(filepath.Join(dir, ".git")); err == nil { + return true + } + parent := filepath.Dir(dir) + if parent == dir { + return false + } + dir = parent + } +} + +func (c copilot) Detect() Status { + if !c.inRepo() { + // Not a git repo — nothing to install into. We treat this as + // "not installed" so 'dhq skills list' stays informative without + // implying we'd write to a random directory. + return StatusNotInstalled + } + cwd, err := c.repoRoot() + if err != nil { + return StatusNotInstalled + } + + data, err := os.ReadFile(filepath.Join(cwd, copilotInstructionsFile)) + if err != nil { + return StatusAvailable + } + switch parseSectionVersion(string(data)) { + case "": + return StatusAvailable + case skills.Version: + return StatusInstalled + default: + return StatusOutdated + } +} + +func (c copilot) Install() (string, error) { + cwd, err := c.repoRoot() + if err != nil { + return "", err + } + if !c.inRepo() { + return "", fmt.Errorf("not a git repository (cwd=%s); run from inside a repo or use a user-scope target", cwd) + } + + // Refresh the reference tree at .github/copilot/deployhq/. + refsRoot := filepath.Join(cwd, copilotRefsDir) + if err := os.RemoveAll(refsRoot); err != nil { + return "", err + } + if err := writeEmbeddedTree(skills.FS, "deployhq", refsRoot); err != nil { + return "", err + } + + instrPath := filepath.Join(cwd, copilotInstructionsFile) + if err := os.MkdirAll(filepath.Dir(instrPath), 0o755); err != nil { + return "", err + } + existing, err := os.ReadFile(instrPath) + if err != nil && !os.IsNotExist(err) { + return "", err + } + + section := buildCopilotSection(copilotRefsDir) + merged := mergeSection(string(existing), section) + if err := os.WriteFile(instrPath, []byte(merged), 0o644); err != nil { + return "", err + } + return instrPath, nil +} + +// buildCopilotSection produces the sentinel-bounded block we own in +// .github/copilot-instructions.md. Kept short because Copilot's instruction +// budget is finite and most projects have other guidance to share. +// +// The reference path is repo-relative so the rendered instructions remain +// portable when the repo is cloned to a different absolute path. +func buildCopilotSection(refsRelPath string) string { + var b strings.Builder + fmt.Fprintf(&b, "%s%s%s\n", sectionBeginPrefix, skills.Version, sectionBeginSuffix) + b.WriteString("This project uses DeployHQ. The `dhq` CLI is the canonical way to deploy ") + b.WriteString("code, manage projects, servers, and repos via the DeployHQ platform.\n\n") + b.WriteString("Skill guide and per-domain references (read these before suggesting `dhq` commands):\n") + fmt.Fprintf(&b, " - %s/SKILL.md\n", refsRelPath) + fmt.Fprintf(&b, " - %s/references/*.md\n\n", refsRelPath) + b.WriteString("Domains: deployments, projects, servers, repos, configuration, operations, ") + b.WriteString("global-resources, auth-setup. When suggesting deploys, prefer `dhq deploy` and ") + b.WriteString("its flags over raw API calls.\n") + b.WriteString(sectionEndMarker) + return b.String() +} diff --git a/internal/skillinstaller/copilot_test.go b/internal/skillinstaller/copilot_test.go new file mode 100644 index 0000000..1c6182d --- /dev/null +++ b/internal/skillinstaller/copilot_test.go @@ -0,0 +1,203 @@ +package skillinstaller + +import ( + "os" + "path/filepath" + "strings" + "testing" + + "github.com/deployhq/deployhq-cli/skills" +) + +// withCwd swaps getCwd for the duration of the test, pointing at a temp dir +// that may or may not be a fake git repo depending on caller intent. +func withCwd(t *testing.T, dir string) { + t.Helper() + orig := getCwd + getCwd = func() (string, error) { return dir, nil } + t.Cleanup(func() { getCwd = orig }) +} + +// fakeRepo returns a temp dir with a .git subdirectory so copilot.inRepo() +// sees it as a real repo. +func fakeRepo(t *testing.T) string { + t.Helper() + dir := t.TempDir() + if err := os.MkdirAll(filepath.Join(dir, ".git"), 0o755); err != nil { + t.Fatal(err) + } + return dir +} + +func TestCopilot_Detect_NotInRepo(t *testing.T) { + withCwd(t, t.TempDir()) + if got := (copilot{}).Detect(); got != StatusNotInstalled { + t.Fatalf("Detect() outside repo = %v, want StatusNotInstalled", got) + } +} + +func TestCopilot_Detect_InRepoNoInstructions(t *testing.T) { + withCwd(t, fakeRepo(t)) + if got := (copilot{}).Detect(); got != StatusAvailable { + t.Fatalf("Detect() in fresh repo = %v, want StatusAvailable", got) + } +} + +func TestCopilot_Detect_InRepoWithUnrelatedInstructions(t *testing.T) { + dir := fakeRepo(t) + withCwd(t, dir) + if err := os.MkdirAll(filepath.Join(dir, ".github"), 0o755); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(dir, copilotInstructionsFile), []byte("# repo guidance\n"), 0o644); err != nil { + t.Fatal(err) + } + if got := (copilot{}).Detect(); got != StatusAvailable { + t.Fatalf("Detect() with unrelated content = %v, want StatusAvailable", got) + } +} + +func TestCopilot_Install_OutsideRepo_Errors(t *testing.T) { + withCwd(t, t.TempDir()) + _, err := (copilot{}).Install() + if err == nil { + t.Fatal("Install() outside repo expected to error") + } + if !strings.Contains(err.Error(), "not a git repository") { + t.Errorf("unexpected error: %v", err) + } +} + +func TestCopilot_Install_WritesSectionAndRefs(t *testing.T) { + dir := fakeRepo(t) + withCwd(t, dir) + + got, err := (copilot{}).Install() + if err != nil { + t.Fatalf("Install() = %v", err) + } + want := filepath.Join(dir, copilotInstructionsFile) + if got != want { + t.Errorf("Install() path = %q, want %q", got, want) + } + + body, err := os.ReadFile(want) + if err != nil { + t.Fatal(err) + } + for _, must := range []string{ + sectionBeginPrefix + skills.Version + sectionBeginSuffix, + sectionEndMarker, + copilotRefsDir + "/SKILL.md", + } { + if !strings.Contains(string(body), must) { + t.Errorf("instructions missing %q\n%s", must, body) + } + } + + if _, err := os.Stat(filepath.Join(dir, copilotRefsDir, "SKILL.md")); err != nil { + t.Errorf("expected SKILL.md under refs root: %v", err) + } + refs, err := os.ReadDir(filepath.Join(dir, copilotRefsDir, "references")) + if err != nil || len(refs) == 0 { + t.Errorf("expected references/*.md: err=%v entries=%d", err, len(refs)) + } +} + +func TestCopilot_Install_PreservesUserContent(t *testing.T) { + dir := fakeRepo(t) + withCwd(t, dir) + if err := os.MkdirAll(filepath.Join(dir, ".github"), 0o755); err != nil { + t.Fatal(err) + } + userRules := "# Team rules\n\nAlways write tests.\n" + if err := os.WriteFile(filepath.Join(dir, copilotInstructionsFile), []byte(userRules), 0o644); err != nil { + t.Fatal(err) + } + + if _, err := (copilot{}).Install(); err != nil { + t.Fatalf("Install() = %v", err) + } + got, err := os.ReadFile(filepath.Join(dir, copilotInstructionsFile)) + if err != nil { + t.Fatal(err) + } + if !strings.Contains(string(got), "Always write tests.") { + t.Errorf("user content lost; body=\n%s", got) + } + if !strings.Contains(string(got), sectionEndMarker) { + t.Errorf("section not appended; body=\n%s", got) + } +} + +func TestCopilot_Install_Idempotent(t *testing.T) { + dir := fakeRepo(t) + withCwd(t, dir) + + if _, err := (copilot{}).Install(); err != nil { + t.Fatal(err) + } + first, err := os.ReadFile(filepath.Join(dir, copilotInstructionsFile)) + if err != nil { + t.Fatal(err) + } + if _, err := (copilot{}).Install(); err != nil { + t.Fatalf("second Install() = %v", err) + } + second, err := os.ReadFile(filepath.Join(dir, copilotInstructionsFile)) + if err != nil { + t.Fatal(err) + } + if string(first) != string(second) { + t.Errorf("idempotence broken\n--- first ---\n%s\n--- second ---\n%s", first, second) + } +} + +func TestCopilot_Detect_Outdated(t *testing.T) { + dir := fakeRepo(t) + withCwd(t, dir) + if _, err := (copilot{}).Install(); err != nil { + t.Fatal(err) + } + if got := (copilot{}).Detect(); got != StatusInstalled { + t.Fatalf("post-install Detect() = %v, want StatusInstalled", got) + } + + path := filepath.Join(dir, copilotInstructionsFile) + body, err := os.ReadFile(path) + if err != nil { + t.Fatal(err) + } + stale := strings.Replace(string(body), sectionBeginPrefix+skills.Version, sectionBeginPrefix+"0", 1) + if err := os.WriteFile(path, []byte(stale), 0o644); err != nil { + t.Fatal(err) + } + if got := (copilot{}).Detect(); got != StatusOutdated { + t.Fatalf("Detect() with stale marker = %v, want StatusOutdated", got) + } +} + +func TestCopilot_InRepo_FindsAncestor(t *testing.T) { + // .git/ lives at root; cwd is a nested subdirectory. + root := fakeRepo(t) + sub := filepath.Join(root, "a", "b", "c") + if err := os.MkdirAll(sub, 0o755); err != nil { + t.Fatal(err) + } + withCwd(t, sub) + if got := (copilot{}).Detect(); got != StatusAvailable { + t.Fatalf("Detect() nested in repo = %v, want StatusAvailable", got) + } +} + +func TestCopilot_Scope_IsProject(t *testing.T) { + if got := (copilot{}).Scope(); got != ScopeProject { + t.Errorf("Scope() = %v, want ScopeProject", got) + } +} + +func TestRegistry_ContainsCopilot(t *testing.T) { + if Find("copilot") == nil { + t.Fatal("copilot target not registered") + } +} diff --git a/internal/skillinstaller/cursor.go b/internal/skillinstaller/cursor.go new file mode 100644 index 0000000..4c2a322 --- /dev/null +++ b/internal/skillinstaller/cursor.go @@ -0,0 +1,129 @@ +package skillinstaller + +import ( + "fmt" + "io/fs" + "os" + "path/filepath" + "strings" + + "github.com/deployhq/deployhq-cli/skills" +) + +func init() { Register(&cursor{}) } + +// cursor installs the skill into Cursor's user-level rules directory. +// +// Layout: ~/.cursor/rules/deployhq.mdc (single file — Cursor only loads +// *.mdc and has no directory tree concept like Claude's skills/) +// Version marker: ~/.cursor/rules/.deployhq-skill-version (hidden, so +// Cursor's *.mdc glob ignores it) +// +// The .mdc has Cursor frontmatter (description, alwaysApply) and the body +// concatenates SKILL.md with every reference file under clear "## reference:" +// headers so the agent can find them in one read. +type cursor struct{} + +func (cursor) Name() string { return "cursor" } +func (cursor) DisplayName() string { return "Cursor" } +func (cursor) Scope() Scope { return ScopeUser } + +const cursorSkillFile = "deployhq.mdc" + +func (c cursor) configDir() (string, error) { + home, err := homeDir() + if err != nil { + return "", err + } + return filepath.Join(home, ".cursor"), nil +} + +func (c cursor) rulesDir() (string, error) { + cfg, err := c.configDir() + if err != nil { + return "", err + } + return filepath.Join(cfg, "rules"), nil +} + +func (c cursor) Detect() Status { + cfg, err := c.configDir() + if err != nil { + return StatusNotInstalled + } + if _, err := os.Stat(cfg); err != nil { + // No ~/.cursor → Cursor has never run on this machine. + return StatusNotInstalled + } + + rules, err := c.rulesDir() + if err != nil { + return StatusNotInstalled + } + if _, err := os.Stat(filepath.Join(rules, cursorSkillFile)); err != nil { + return StatusAvailable + } + + switch readVersion(filepath.Join(rules, versionMarker)) { + case skills.Version: + return StatusInstalled + default: + return StatusOutdated + } +} + +func (c cursor) Install() (string, error) { + rules, err := c.rulesDir() + if err != nil { + return "", err + } + if err := os.MkdirAll(rules, 0o755); err != nil { + return "", err + } + + content, err := buildCursorMDC(skills.FS, "deployhq") + if err != nil { + return "", err + } + + dst := filepath.Join(rules, cursorSkillFile) + if err := os.WriteFile(dst, content, 0o644); err != nil { + return "", err + } + if err := os.WriteFile(filepath.Join(rules, versionMarker), []byte(skills.Version+"\n"), 0o644); err != nil { + return "", err + } + return dst, nil +} + +// buildCursorMDC wraps the flattened skill body in Cursor's .mdc frontmatter. +// +// Output shape: +// +// --- +// description: +// alwaysApply: false +// --- +// +// +// +// alwaysApply: false means Cursor treats this as an agent-requested rule — +// the agent decides when to pull it in based on the description. Same model +// as Claude Code's progressive-disclosure skill discovery. +func buildCursorMDC(efs fs.FS, root string) ([]byte, error) { + description, body, err := flattenSkill(efs, root) + if err != nil { + return nil, err + } + if description == "" { + description = "DeployHQ CLI — deploy code, manage servers, automate infrastructure via the dhq command." + } + + var buf strings.Builder + buf.WriteString("---\n") + fmt.Fprintf(&buf, "description: %s\n", oneLine(description)) + buf.WriteString("alwaysApply: false\n") + buf.WriteString("---\n\n") + buf.Write(body) + return []byte(buf.String()), nil +} diff --git a/internal/skillinstaller/cursor_test.go b/internal/skillinstaller/cursor_test.go new file mode 100644 index 0000000..db55207 --- /dev/null +++ b/internal/skillinstaller/cursor_test.go @@ -0,0 +1,165 @@ +package skillinstaller + +import ( + "os" + "path/filepath" + "strings" + "testing" + + "github.com/deployhq/deployhq-cli/skills" +) + +func TestCursor_Detect_NoConfigDir(t *testing.T) { + withHomeDir(t, t.TempDir()) + if got := (cursor{}).Detect(); got != StatusNotInstalled { + t.Fatalf("Detect() = %v, want StatusNotInstalled", got) + } +} + +func TestCursor_Detect_AgentInstalledSkillMissing(t *testing.T) { + home := t.TempDir() + withHomeDir(t, home) + if err := os.MkdirAll(filepath.Join(home, ".cursor"), 0o755); err != nil { + t.Fatal(err) + } + if got := (cursor{}).Detect(); got != StatusAvailable { + t.Fatalf("Detect() = %v, want StatusAvailable", got) + } +} + +func TestCursor_Install_WritesMDC(t *testing.T) { + home := t.TempDir() + withHomeDir(t, home) + if err := os.MkdirAll(filepath.Join(home, ".cursor"), 0o755); err != nil { + t.Fatal(err) + } + + path, err := (cursor{}).Install() + if err != nil { + t.Fatalf("Install() error = %v", err) + } + + wantPath := filepath.Join(home, ".cursor", "rules", "deployhq.mdc") + if path != wantPath { + t.Errorf("Install() path = %q, want %q", path, wantPath) + } + + content, err := os.ReadFile(wantPath) + if err != nil { + t.Fatalf("read mdc: %v", err) + } + body := string(content) + + if !strings.HasPrefix(body, "---\n") { + t.Error("mdc missing leading frontmatter delimiter") + } + if !strings.Contains(body, "alwaysApply: false") { + t.Error("mdc missing alwaysApply: false") + } + // The description from SKILL.md ("Deploy code…") should make it into + // the Cursor frontmatter, collapsed to one line. + if !strings.Contains(body, "description: Deploy code") { + t.Errorf("mdc missing description extracted from SKILL.md\n--- body ---\n%s", body[:min(400, len(body))]) + } + // SKILL.md body content should appear (post-frontmatter heading). + if !strings.Contains(body, "DeployHQ CLI — Agent Skill Guide") { + t.Error("mdc missing SKILL.md body content") + } + // References must be inlined under a discoverable header. + if !strings.Contains(body, "## reference: references/") { + t.Error("mdc missing reference sections") + } + + v, err := os.ReadFile(filepath.Join(home, ".cursor", "rules", versionMarker)) + if err != nil { + t.Fatalf("read version marker: %v", err) + } + if strings.TrimSpace(string(v)) != skills.Version { + t.Errorf("version marker = %q, want %q", string(v), skills.Version) + } +} + +func TestCursor_Detect_InstalledThenOutdated(t *testing.T) { + home := t.TempDir() + withHomeDir(t, home) + if err := os.MkdirAll(filepath.Join(home, ".cursor"), 0o755); err != nil { + t.Fatal(err) + } + if _, err := (cursor{}).Install(); err != nil { + t.Fatal(err) + } + if got := (cursor{}).Detect(); got != StatusInstalled { + t.Fatalf("after install Detect() = %v, want StatusInstalled", got) + } + // Force-stale the version marker. + if err := os.WriteFile( + filepath.Join(home, ".cursor", "rules", versionMarker), + []byte("0\n"), 0o644, + ); err != nil { + t.Fatal(err) + } + if got := (cursor{}).Detect(); got != StatusOutdated { + t.Fatalf("with stale marker Detect() = %v, want StatusOutdated", got) + } +} + +func TestCursor_Install_Idempotent(t *testing.T) { + home := t.TempDir() + withHomeDir(t, home) + if err := os.MkdirAll(filepath.Join(home, ".cursor"), 0o755); err != nil { + t.Fatal(err) + } + if _, err := (cursor{}).Install(); err != nil { + t.Fatal(err) + } + if _, err := (cursor{}).Install(); err != nil { + t.Fatalf("second Install() = %v", err) + } + if got := (cursor{}).Detect(); got != StatusInstalled { + t.Fatalf("after re-install Detect() = %v, want StatusInstalled", got) + } +} + +func TestExtractDescription(t *testing.T) { + cases := []struct { + name string + fm string + want string + }{ + { + name: "inline", + fm: "description: hello world\nother: x", + want: "hello world", + }, + { + name: "literal block", + fm: "name: foo\ndescription: |\n Line one of the description.\n Line two of the description.\nlicense: MIT", + want: "Line one of the description. Line two of the description.", + }, + { + name: "missing", + fm: "name: foo\nlicense: MIT", + want: "", + }, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + if got := extractDescription(tc.fm); got != tc.want { + t.Errorf("got %q, want %q", got, tc.want) + } + }) + } +} + +func TestRegistry_ContainsCursor(t *testing.T) { + if Find("cursor") == nil { + t.Fatal("cursor target not registered") + } +} + +func min(a, b int) int { + if a < b { + return a + } + return b +} diff --git a/internal/skillinstaller/flatten.go b/internal/skillinstaller/flatten.go new file mode 100644 index 0000000..4156ab8 --- /dev/null +++ b/internal/skillinstaller/flatten.go @@ -0,0 +1,114 @@ +package skillinstaller + +import ( + "fmt" + "io/fs" + "os" + "path" + "sort" + "strings" + + "github.com/deployhq/deployhq-cli/skills" +) + +// Markers for targets that own their skill file entirely — there's no user +// content to coexist with, so we use a single top-of-file HTML comment for +// version tracking rather than the sentinel-section pattern used by +// shared-file targets (Windsurf/Copilot/Codex/Gemini/OpenCode). +// +// Used by Aider, Cline, Continue. +const ( + ownedFileVersionPrefix = "" +) + +// buildOwnedSkillFile flattens the embedded skill tree into a single +// markdown file, prepended with a version-tracking HTML comment. +// +// Output: +// +// +// +// +func buildOwnedSkillFile(efs fs.FS, root string) ([]byte, error) { + _, body, err := flattenSkill(efs, root) + if err != nil { + return nil, err + } + + var buf strings.Builder + fmt.Fprintf(&buf, "%s%s%s\n\n", ownedFileVersionPrefix, skills.Version, ownedFileVersionSuffix) + buf.Write(body) + return []byte(buf.String()), nil +} + +// parseOwnedFileVersion returns the version embedded in our top-of-file +// HTML comment, or "" if no marker is present. +func parseOwnedFileVersion(body string) string { + idx := strings.Index(body, ownedFileVersionPrefix) + if idx < 0 { + return "" + } + rest := body[idx+len(ownedFileVersionPrefix):] + end := strings.Index(rest, ownedFileVersionSuffix) + if end < 0 { + return "" + } + return strings.TrimSpace(rest[:end]) +} + +// flattenSkill reads SKILL.md plus every references/*.md from the embedded +// skill tree and returns: +// +// - description: the value of the YAML `description:` field in SKILL.md's +// frontmatter (handles the literal-block `description: |` form). Empty +// when no description is present. +// - body: SKILL.md's body (frontmatter stripped) followed by each +// reference file under a `## reference: references/` header, +// sorted alphabetically for deterministic output. +// +// Targets that ship the skill as a single document (Cursor's .mdc, +// Aider's read-file) build their output by wrapping this body in their +// own header/frontmatter. Targets that ship the tree as-is (Claude Code, +// Windsurf, Codex, Copilot) bypass this and call writeEmbeddedTree +// directly instead. +func flattenSkill(efs fs.FS, root string) (description string, body []byte, err error) { + skill, err := fs.ReadFile(efs, path.Join(root, "SKILL.md")) + if err != nil { + return "", nil, fmt.Errorf("read SKILL.md: %w", err) + } + description, skillBody := splitSkillFrontmatter(skill) + + var buf strings.Builder + buf.Write(skillBody) + if !strings.HasSuffix(string(skillBody), "\n") { + buf.WriteByte('\n') + } + + refsDir := path.Join(root, "references") + entries, err := fs.ReadDir(efs, refsDir) + if err != nil { + if os.IsNotExist(err) { + return description, []byte(buf.String()), nil + } + return "", nil, fmt.Errorf("read references/: %w", err) + } + sort.Slice(entries, func(i, j int) bool { return entries[i].Name() < entries[j].Name() }) + + for _, e := range entries { + if e.IsDir() { + continue + } + data, err := fs.ReadFile(efs, path.Join(refsDir, e.Name())) + if err != nil { + return "", nil, fmt.Errorf("read %s: %w", e.Name(), err) + } + fmt.Fprintf(&buf, "\n\n## reference: references/%s\n\n", e.Name()) + buf.Write(data) + if !strings.HasSuffix(string(data), "\n") { + buf.WriteByte('\n') + } + } + + return description, []byte(buf.String()), nil +} diff --git a/internal/skillinstaller/frontmatter.go b/internal/skillinstaller/frontmatter.go new file mode 100644 index 0000000..2fdb15b --- /dev/null +++ b/internal/skillinstaller/frontmatter.go @@ -0,0 +1,65 @@ +package skillinstaller + +import "strings" + +// splitSkillFrontmatter strips the leading YAML frontmatter from SKILL.md +// and pulls out the `description:` value. The frontmatter uses the literal +// block style (`description: |`) followed by an indented block, so we +// can't lean on a regex match against the same line. +// +// Returns (description, body). If no frontmatter is found, returns ("", input). +func splitSkillFrontmatter(input []byte) (description string, body []byte) { + s := string(input) + if !strings.HasPrefix(s, "---\n") { + return "", input + } + end := strings.Index(s[4:], "\n---") + if end < 0 { + return "", input + } + frontmatter := s[4 : 4+end] + rest := s[4+end+len("\n---"):] + rest = strings.TrimPrefix(rest, "\n") + + description = extractDescription(frontmatter) + return description, []byte(rest) +} + +// extractDescription reads the YAML `description:` field, handling both the +// inline form (`description: foo`) and the literal block form +// (`description: |` followed by indented lines). Returns an empty string if +// the field is absent or unparseable. +func extractDescription(frontmatter string) string { + lines := strings.Split(frontmatter, "\n") + for i, line := range lines { + trim := strings.TrimRight(line, "\r") + if !strings.HasPrefix(trim, "description:") { + continue + } + rest := strings.TrimSpace(strings.TrimPrefix(trim, "description:")) + if rest != "|" && rest != ">" && rest != "" { + return rest + } + var out []string + for j := i + 1; j < len(lines); j++ { + l := lines[j] + if l == "" { + out = append(out, "") + continue + } + if !strings.HasPrefix(l, " ") && !strings.HasPrefix(l, "\t") { + break + } + out = append(out, strings.TrimSpace(l)) + } + return strings.TrimSpace(strings.Join(out, " ")) + } + return "" +} + +// oneLine collapses internal whitespace into single spaces — useful when +// a multi-line YAML block description has to fit on a single-line field +// (e.g. Cursor's .mdc frontmatter). +func oneLine(s string) string { + return strings.Join(strings.Fields(s), " ") +} diff --git a/internal/skillinstaller/gemini.go b/internal/skillinstaller/gemini.go new file mode 100644 index 0000000..a0b3484 --- /dev/null +++ b/internal/skillinstaller/gemini.go @@ -0,0 +1,109 @@ +package skillinstaller + +import ( + "fmt" + "os" + "path/filepath" + "strings" + + "github.com/deployhq/deployhq-cli/skills" +) + +func init() { Register(&gemini{}) } + +// gemini installs the skill into Google's Gemini CLI user-level +// instructions file. +// +// Layout (same shape as Codex CLI): +// - ~/.gemini/GEMINI.md +// Single shared file; we own a sentinel-bounded section, user +// content outside it is preserved. +// - ~/.gemini/deployhq-references/SKILL.md + references/*.md +// Full reference tree the agent reads on demand. +type gemini struct{} + +func (gemini) Name() string { return "gemini" } +func (gemini) DisplayName() string { return "Gemini CLI" } +func (gemini) Scope() Scope { return ScopeUser } + +const ( + geminiInstructionsFile = "GEMINI.md" + geminiRefsDir = "deployhq-references" +) + +func (g gemini) configDir() (string, error) { + home, err := homeDir() + if err != nil { + return "", err + } + return filepath.Join(home, ".gemini"), nil +} + +func (g gemini) Detect() Status { + cfg, err := g.configDir() + if err != nil { + return StatusNotInstalled + } + if _, err := os.Stat(cfg); err != nil { + return StatusNotInstalled + } + + data, err := os.ReadFile(filepath.Join(cfg, geminiInstructionsFile)) + if err != nil { + return StatusAvailable + } + switch parseSectionVersion(string(data)) { + case "": + return StatusAvailable + case skills.Version: + return StatusInstalled + default: + return StatusOutdated + } +} + +func (g gemini) Install() (string, error) { + cfg, err := g.configDir() + if err != nil { + return "", err + } + if err := os.MkdirAll(cfg, 0o755); err != nil { + return "", err + } + + refsRoot := filepath.Join(cfg, geminiRefsDir) + if err := os.RemoveAll(refsRoot); err != nil { + return "", err + } + if err := writeEmbeddedTree(skills.FS, "deployhq", refsRoot); err != nil { + return "", err + } + + instrPath := filepath.Join(cfg, geminiInstructionsFile) + existing, err := os.ReadFile(instrPath) + if err != nil && !os.IsNotExist(err) { + return "", err + } + + section := buildGeminiSection(refsRoot) + merged := mergeSection(string(existing), section) + if err := os.WriteFile(instrPath, []byte(merged), 0o644); err != nil { + return "", err + } + return instrPath, nil +} + +func buildGeminiSection(refsAbsPath string) string { + var b strings.Builder + fmt.Fprintf(&b, "%s%s%s\n", sectionBeginPrefix, skills.Version, sectionBeginSuffix) + b.WriteString("DeployHQ CLI (`dhq`) is available. Use it to deploy code, manage projects, ") + b.WriteString("servers, and repos via the DeployHQ platform.\n\n") + b.WriteString("Skill guide and per-domain references (read before suggesting `dhq` commands):\n") + fmt.Fprintf(&b, " - %s/SKILL.md\n", refsAbsPath) + fmt.Fprintf(&b, " - %s/references/*.md\n\n", refsAbsPath) + b.WriteString("Domains: deployments, projects, servers, repos, configuration, operations, ") + b.WriteString("global-resources, auth-setup. When the user asks to deploy, prefer `dhq deploy` ") + b.WriteString("with its flags over raw API calls.\n") + b.WriteString(sectionEndMarker) + return b.String() +} diff --git a/internal/skillinstaller/gemini_test.go b/internal/skillinstaller/gemini_test.go new file mode 100644 index 0000000..9564917 --- /dev/null +++ b/internal/skillinstaller/gemini_test.go @@ -0,0 +1,158 @@ +package skillinstaller + +import ( + "os" + "path/filepath" + "strings" + "testing" + + "github.com/deployhq/deployhq-cli/skills" +) + +func TestGemini_Detect_NoConfigDir(t *testing.T) { + withHomeDir(t, t.TempDir()) + if got := (gemini{}).Detect(); got != StatusNotInstalled { + t.Fatalf("Detect() = %v, want StatusNotInstalled", got) + } +} + +func TestGemini_Detect_InstalledNoInstructions(t *testing.T) { + home := t.TempDir() + withHomeDir(t, home) + if err := os.MkdirAll(filepath.Join(home, ".gemini"), 0o755); err != nil { + t.Fatal(err) + } + if got := (gemini{}).Detect(); got != StatusAvailable { + t.Fatalf("Detect() = %v, want StatusAvailable", got) + } +} + +func TestGemini_Install_WritesSectionAndRefs(t *testing.T) { + home := t.TempDir() + withHomeDir(t, home) + if err := os.MkdirAll(filepath.Join(home, ".gemini"), 0o755); err != nil { + t.Fatal(err) + } + + got, err := (gemini{}).Install() + if err != nil { + t.Fatalf("Install() = %v", err) + } + want := filepath.Join(home, ".gemini", geminiInstructionsFile) + if got != want { + t.Errorf("Install() path = %q, want %q", got, want) + } + + body, err := os.ReadFile(want) + if err != nil { + t.Fatal(err) + } + for _, must := range []string{ + sectionBeginPrefix + skills.Version + sectionBeginSuffix, + sectionEndMarker, + filepath.Join(home, ".gemini", geminiRefsDir, "SKILL.md"), + } { + if !strings.Contains(string(body), must) { + t.Errorf("GEMINI.md missing %q\n%s", must, body) + } + } + + if _, err := os.Stat(filepath.Join(home, ".gemini", geminiRefsDir, "SKILL.md")); err != nil { + t.Errorf("expected SKILL.md under refs root: %v", err) + } + refs, err := os.ReadDir(filepath.Join(home, ".gemini", geminiRefsDir, "references")) + if err != nil || len(refs) == 0 { + t.Errorf("expected references/*.md: err=%v entries=%d", err, len(refs)) + } +} + +func TestGemini_Install_PreservesUserContent(t *testing.T) { + home := t.TempDir() + withHomeDir(t, home) + cfg := filepath.Join(home, ".gemini") + if err := os.MkdirAll(cfg, 0o755); err != nil { + t.Fatal(err) + } + userContent := "# My Gemini rules\n\nBe concise.\n" + if err := os.WriteFile(filepath.Join(cfg, geminiInstructionsFile), []byte(userContent), 0o644); err != nil { + t.Fatal(err) + } + + if _, err := (gemini{}).Install(); err != nil { + t.Fatalf("Install() = %v", err) + } + got, err := os.ReadFile(filepath.Join(cfg, geminiInstructionsFile)) + if err != nil { + t.Fatal(err) + } + if !strings.Contains(string(got), "Be concise.") { + t.Errorf("user content lost:\n%s", got) + } + if !strings.Contains(string(got), sectionEndMarker) { + t.Errorf("section not appended:\n%s", got) + } +} + +func TestGemini_Install_Idempotent(t *testing.T) { + home := t.TempDir() + withHomeDir(t, home) + if err := os.MkdirAll(filepath.Join(home, ".gemini"), 0o755); err != nil { + t.Fatal(err) + } + if _, err := (gemini{}).Install(); err != nil { + t.Fatal(err) + } + first, err := os.ReadFile(filepath.Join(home, ".gemini", geminiInstructionsFile)) + if err != nil { + t.Fatal(err) + } + if _, err := (gemini{}).Install(); err != nil { + t.Fatalf("second Install() = %v", err) + } + second, err := os.ReadFile(filepath.Join(home, ".gemini", geminiInstructionsFile)) + if err != nil { + t.Fatal(err) + } + if string(first) != string(second) { + t.Errorf("idempotence broken\n--- first ---\n%s\n--- second ---\n%s", first, second) + } +} + +func TestGemini_Detect_Outdated(t *testing.T) { + home := t.TempDir() + withHomeDir(t, home) + if err := os.MkdirAll(filepath.Join(home, ".gemini"), 0o755); err != nil { + t.Fatal(err) + } + if _, err := (gemini{}).Install(); err != nil { + t.Fatal(err) + } + if got := (gemini{}).Detect(); got != StatusInstalled { + t.Fatalf("post-install Detect() = %v, want StatusInstalled", got) + } + + path := filepath.Join(home, ".gemini", geminiInstructionsFile) + body, err := os.ReadFile(path) + if err != nil { + t.Fatal(err) + } + stale := strings.Replace(string(body), sectionBeginPrefix+skills.Version, sectionBeginPrefix+"0", 1) + if err := os.WriteFile(path, []byte(stale), 0o644); err != nil { + t.Fatal(err) + } + if got := (gemini{}).Detect(); got != StatusOutdated { + t.Fatalf("Detect() stale marker = %v, want StatusOutdated", got) + } +} + +func TestGemini_Scope_IsUser(t *testing.T) { + if got := (gemini{}).Scope(); got != ScopeUser { + t.Errorf("Scope() = %v, want ScopeUser", got) + } +} + +func TestRegistry_ContainsGemini(t *testing.T) { + if Find("gemini") == nil { + t.Fatal("gemini target not registered") + } +} diff --git a/internal/skillinstaller/installer.go b/internal/skillinstaller/installer.go new file mode 100644 index 0000000..62c6c65 --- /dev/null +++ b/internal/skillinstaller/installer.go @@ -0,0 +1,178 @@ +// Package skillinstaller detects locally-installed AI coding agents and writes +// the bundled DeployHQ skill into each agent's config directory. +// +// This is the Wrangler-style onboarding pattern: after `dhq hello` logs the +// user in, we check which agents are present on disk and offer to install the +// skill so the agent can drive the CLI competently. +// +// Each agent has a different idea of what a "skill" is (Claude Code has a +// formal skills/ directory; Cursor uses .cursor/rules/*.mdc; Copilot uses a +// single .github/copilot-instructions.md; etc.). Each one is implemented as a +// Target so the quirks stay isolated. +package skillinstaller + +import ( + "fmt" + "sort" +) + +// Scope tells the post-login prompt how aggressive it should be about +// installing a target without explicit user consent. +// +// User-scope targets write only to the user's home directory and never +// touch project files — safe to install during 'dhq hello' after a Y/n +// prompt, or silently for the runtime agent. +// +// Project-scope targets modify the current repo (e.g. .github/). They are +// always opt-in via 'dhq skills install --agent ' so we never mutate +// a user's project as a side effect of logging in. +type Scope int + +const ( + ScopeUser Scope = iota + ScopeProject +) + +// Status describes the install state of a Target on this machine. +type Status int + +const ( + // StatusNotInstalled means the target agent is not installed on this + // machine (or we can't tell — we treat absence of the config dir as + // "not present" rather than fail-loud). + StatusNotInstalled Status = iota + // StatusAvailable means the agent is installed but the skill is not. + StatusAvailable + // StatusInstalled means the skill is already installed at the same or + // newer version. Callers should skip re-installing. + StatusInstalled + // StatusOutdated means the skill is installed but at an older version. + // Callers may upgrade without re-prompting. + StatusOutdated +) + +func (s Status) String() string { + switch s { + case StatusNotInstalled: + return "not-installed" + case StatusAvailable: + return "available" + case StatusInstalled: + return "installed" + case StatusOutdated: + return "outdated" + default: + return "unknown" + } +} + +// Target is one AI agent's skill-install integration. +// +// Targets are expected to be cheap to construct and side-effect-free until +// Install is called. Detect should never error — return StatusNotInstalled +// when in doubt. +type Target interface { + // Name is a stable identifier ("claude-code", "cursor", etc.) used in + // CLI output, telemetry, and matching against harness.Detect(). + Name() string + + // DisplayName is a human-friendly label ("Claude Code") for prompts. + DisplayName() string + + // Scope is User for targets that only touch ~/, Project for targets + // that write into the current repo. The hello-flow prompt only + // auto-offers User-scope targets. + Scope() Scope + + // Detect reports whether this agent is installed locally and the + // current install state of the skill. + Detect() Status + + // Install writes the skill into the agent's config directory. It must + // be idempotent — safe to re-run over an existing install. Returns a + // short human-readable summary on success (e.g. the path written to) + // for use in CLI status output. + Install() (string, error) +} + +// registry is the global list of known targets. Each target self-registers in +// an init() so adding a new agent is a single-file change. +var registry []Target + +// Register adds a Target to the global registry. Intended for use from +// per-target init() functions. +func Register(t Target) { + registry = append(registry, t) +} + +// All returns every registered target, sorted by Name for stable output. +func All() []Target { + out := make([]Target, len(registry)) + copy(out, registry) + sort.Slice(out, func(i, j int) bool { return out[i].Name() < out[j].Name() }) + return out +} + +// DetectResult pairs a Target with its current Status. +type DetectResult struct { + Target Target + Status Status +} + +// Noter is an optional Target capability. A target that implements it gets +// a chance to append a free-form note after install — useful when the agent +// needs the user to wire something up manually (e.g. add a `read:` entry to +// a config file we can't safely auto-edit). +// +// Callers (the hello hook, `dhq skills install`) detect this via a type +// assertion and print the note to stderr after the install line. +type Noter interface { + PostInstallNote() string +} + +// DetectInstalled probes every registered target and returns those whose +// agent appears to be installed locally (Status != StatusNotInstalled). +// The runtime agent — the one the user is currently running `dhq` from, +// as reported by harness.Detect — is included even if Status would +// otherwise be StatusInstalled, so the caller can decide whether to +// upgrade silently or skip. +func DetectInstalled() []DetectResult { + var out []DetectResult + for _, t := range All() { + if s := t.Detect(); s != StatusNotInstalled { + out = append(out, DetectResult{Target: t, Status: s}) + } + } + return out +} + +// Find returns the Target with the given name, or nil if no such target is +// registered. Matching is case-insensitive on the canonical name. +func Find(name string) Target { + for _, t := range All() { + if t.Name() == name { + return t + } + } + return nil +} + +// Needed reports whether the skill needs to be (re)installed for this target. +// Returns true for StatusAvailable and StatusOutdated; false for +// StatusInstalled and StatusNotInstalled. +func Needed(s Status) bool { + return s == StatusAvailable || s == StatusOutdated +} + +// InstallError wraps a target-specific failure with the target name so +// callers can surface partial failures without losing context. +type InstallError struct { + Target string + Cause error +} + +func (e *InstallError) Error() string { + return fmt.Sprintf("install %s: %v", e.Target, e.Cause) +} + +func (e *InstallError) Unwrap() error { return e.Cause } diff --git a/internal/skillinstaller/installer_test.go b/internal/skillinstaller/installer_test.go new file mode 100644 index 0000000..363d3d4 --- /dev/null +++ b/internal/skillinstaller/installer_test.go @@ -0,0 +1,171 @@ +package skillinstaller + +import ( + "os" + "path/filepath" + "strings" + "testing" + + "github.com/deployhq/deployhq-cli/skills" +) + +// withHomeDir swaps homeDir for the duration of the test. The Claude target +// reads ~/.claude — we point it at a t.TempDir so tests don't touch the real +// user home. +func withHomeDir(t *testing.T, dir string) { + t.Helper() + orig := homeDir + homeDir = func() (string, error) { return dir, nil } + t.Cleanup(func() { homeDir = orig }) +} + +func TestClaudeCode_Detect_NoConfigDir(t *testing.T) { + withHomeDir(t, t.TempDir()) + if got := (claudeCode{}).Detect(); got != StatusNotInstalled { + t.Fatalf("Detect() = %v, want StatusNotInstalled", got) + } +} + +func TestClaudeCode_Detect_AgentInstalledSkillMissing(t *testing.T) { + home := t.TempDir() + withHomeDir(t, home) + // Simulate Claude Code installed (just the config dir exists). + if err := os.MkdirAll(filepath.Join(home, ".claude"), 0o755); err != nil { + t.Fatal(err) + } + if got := (claudeCode{}).Detect(); got != StatusAvailable { + t.Fatalf("Detect() = %v, want StatusAvailable", got) + } +} + +func TestClaudeCode_Install_WritesTreeAndVersion(t *testing.T) { + home := t.TempDir() + withHomeDir(t, home) + if err := os.MkdirAll(filepath.Join(home, ".claude"), 0o755); err != nil { + t.Fatal(err) + } + + path, err := (claudeCode{}).Install() + if err != nil { + t.Fatalf("Install() error = %v", err) + } + + wantDir := filepath.Join(home, ".claude", "skills", "deployhq") + if path != wantDir { + t.Errorf("Install() path = %q, want %q", path, wantDir) + } + + // SKILL.md must exist and be non-empty. + skill, err := os.ReadFile(filepath.Join(wantDir, "SKILL.md")) + if err != nil { + t.Fatalf("read SKILL.md: %v", err) + } + if !strings.Contains(string(skill), "name: deployhq") { + t.Errorf("SKILL.md content unexpected: %.80q", string(skill)) + } + + // At least one reference file must have been written. + refs, err := os.ReadDir(filepath.Join(wantDir, "references")) + if err != nil { + t.Fatalf("read references/: %v", err) + } + if len(refs) == 0 { + t.Error("references/ is empty after install") + } + + // Version marker must match skills.Version. + v, err := os.ReadFile(filepath.Join(wantDir, versionMarker)) + if err != nil { + t.Fatalf("read version marker: %v", err) + } + if strings.TrimSpace(string(v)) != skills.Version { + t.Errorf("version marker = %q, want %q", string(v), skills.Version) + } +} + +func TestClaudeCode_Detect_InstalledMatchesVersion(t *testing.T) { + home := t.TempDir() + withHomeDir(t, home) + if err := os.MkdirAll(filepath.Join(home, ".claude"), 0o755); err != nil { + t.Fatal(err) + } + if _, err := (claudeCode{}).Install(); err != nil { + t.Fatal(err) + } + if got := (claudeCode{}).Detect(); got != StatusInstalled { + t.Fatalf("Detect() after install = %v, want StatusInstalled", got) + } +} + +func TestClaudeCode_Detect_OutdatedVersionMarker(t *testing.T) { + home := t.TempDir() + withHomeDir(t, home) + if err := os.MkdirAll(filepath.Join(home, ".claude"), 0o755); err != nil { + t.Fatal(err) + } + if _, err := (claudeCode{}).Install(); err != nil { + t.Fatal(err) + } + // Overwrite version marker with an older version. + if err := os.WriteFile( + filepath.Join(home, ".claude", "skills", "deployhq", versionMarker), + []byte("0\n"), 0o644, + ); err != nil { + t.Fatal(err) + } + if got := (claudeCode{}).Detect(); got != StatusOutdated { + t.Fatalf("Detect() with stale marker = %v, want StatusOutdated", got) + } +} + +func TestClaudeCode_Install_Idempotent(t *testing.T) { + home := t.TempDir() + withHomeDir(t, home) + if err := os.MkdirAll(filepath.Join(home, ".claude"), 0o755); err != nil { + t.Fatal(err) + } + if _, err := (claudeCode{}).Install(); err != nil { + t.Fatal(err) + } + // Second install must not error and must leave the same files in place. + if _, err := (claudeCode{}).Install(); err != nil { + t.Fatalf("second Install() = %v", err) + } + if got := (claudeCode{}).Detect(); got != StatusInstalled { + t.Fatalf("after re-install Detect() = %v, want StatusInstalled", got) + } +} + +func TestRegistry_ContainsClaudeCode(t *testing.T) { + if Find("claude-code") == nil { + t.Fatal("claude-code target not registered") + } +} + +func TestDetectInstalled_FiltersNotInstalled(t *testing.T) { + withHomeDir(t, t.TempDir()) + // With an empty home, claude target should not appear in DetectInstalled. + got := DetectInstalled() + for _, r := range got { + if r.Target.Name() == "claude-code" { + t.Errorf("claude-code returned by DetectInstalled() with empty home: status=%v", r.Status) + } + } +} + +func TestNeeded(t *testing.T) { + cases := []struct { + s Status + want bool + }{ + {StatusNotInstalled, false}, + {StatusAvailable, true}, + {StatusOutdated, true}, + {StatusInstalled, false}, + } + for _, tc := range cases { + if got := Needed(tc.s); got != tc.want { + t.Errorf("Needed(%v) = %v, want %v", tc.s, got, tc.want) + } + } +} diff --git a/internal/skillinstaller/kiro.go b/internal/skillinstaller/kiro.go new file mode 100644 index 0000000..4cce333 --- /dev/null +++ b/internal/skillinstaller/kiro.go @@ -0,0 +1,106 @@ +package skillinstaller + +import ( + "fmt" + "os" + "path/filepath" + + "github.com/deployhq/deployhq-cli/skills" +) + +func init() { Register(&kiro{}) } + +// kiro installs the skill as an AWS Kiro CLI steering file. +// +// Kiro discovers per-project context via "steering" files — every *.md +// under /.kiro/steering/ is loaded into the agent's context. We +// own a single file in that directory; the user's other steering files +// are untouched. +// +// Layout: /.kiro/steering/deployhq.md +// +// ScopeProject — modifying the user's repo as a side effect of `dhq hello` +// would be hostile. Opt in via: +// +// dhq skills install --agent kiro +type kiro struct{} + +func (kiro) Name() string { return "kiro" } +func (kiro) DisplayName() string { return "Kiro CLI" } +func (kiro) Scope() Scope { return ScopeProject } + +const ( + kiroSteeringDir = ".kiro/steering" + kiroSkillFile = "deployhq.md" +) + +// inRepo mirrors copilot/cline's ancestor-walk for .git detection. +func (k kiro) inRepo() bool { + dir, err := getCwd() + if err != nil { + return false + } + for { + if _, err := os.Stat(filepath.Join(dir, ".git")); err == nil { + return true + } + parent := filepath.Dir(dir) + if parent == dir { + return false + } + dir = parent + } +} + +func (k kiro) skillPath() (string, error) { + cwd, err := getCwd() + if err != nil { + return "", err + } + return filepath.Join(cwd, kiroSteeringDir, kiroSkillFile), nil +} + +func (k kiro) Detect() Status { + if !k.inRepo() { + return StatusNotInstalled + } + p, err := k.skillPath() + if err != nil { + return StatusNotInstalled + } + data, err := os.ReadFile(p) + if err != nil { + return StatusAvailable + } + switch parseOwnedFileVersion(string(data)) { + case "": + return StatusAvailable + case skills.Version: + return StatusInstalled + default: + return StatusOutdated + } +} + +func (k kiro) Install() (string, error) { + if !k.inRepo() { + cwd, _ := getCwd() + return "", fmt.Errorf("not a git repository (cwd=%s); run from inside a repo", cwd) + } + p, err := k.skillPath() + if err != nil { + return "", err + } + if err := os.MkdirAll(filepath.Dir(p), 0o755); err != nil { + return "", err + } + + body, err := buildOwnedSkillFile(skills.FS, "deployhq") + if err != nil { + return "", err + } + if err := os.WriteFile(p, body, 0o644); err != nil { + return "", err + } + return p, nil +} diff --git a/internal/skillinstaller/kiro_test.go b/internal/skillinstaller/kiro_test.go new file mode 100644 index 0000000..6fbd213 --- /dev/null +++ b/internal/skillinstaller/kiro_test.go @@ -0,0 +1,142 @@ +package skillinstaller + +import ( + "os" + "path/filepath" + "strings" + "testing" + + "github.com/deployhq/deployhq-cli/skills" +) + +func TestKiro_Detect_NotInRepo(t *testing.T) { + withCwd(t, t.TempDir()) + if got := (kiro{}).Detect(); got != StatusNotInstalled { + t.Fatalf("Detect() outside repo = %v, want StatusNotInstalled", got) + } +} + +func TestKiro_Detect_InRepoNoSkill(t *testing.T) { + withCwd(t, fakeRepo(t)) + if got := (kiro{}).Detect(); got != StatusAvailable { + t.Fatalf("Detect() = %v, want StatusAvailable", got) + } +} + +func TestKiro_Install_OutsideRepo_Errors(t *testing.T) { + withCwd(t, t.TempDir()) + if _, err := (kiro{}).Install(); err == nil { + t.Fatal("expected error outside repo") + } +} + +func TestKiro_Install_WritesSkillFile(t *testing.T) { + dir := fakeRepo(t) + withCwd(t, dir) + + got, err := (kiro{}).Install() + if err != nil { + t.Fatalf("Install() = %v", err) + } + want := filepath.Join(dir, kiroSteeringDir, kiroSkillFile) + if got != want { + t.Errorf("Install() path = %q, want %q", got, want) + } + + body, err := os.ReadFile(want) + if err != nil { + t.Fatal(err) + } + s := string(body) + if !strings.HasPrefix(s, ownedFileVersionPrefix+skills.Version+ownedFileVersionSuffix+"\n") { + t.Errorf("file must start with version marker; got: %.120q", s) + } + if !strings.Contains(s, "DeployHQ CLI — Agent Skill Guide") { + t.Error("SKILL.md body missing") + } +} + +func TestKiro_Install_CoexistsWithOtherSteeringFiles(t *testing.T) { + dir := fakeRepo(t) + withCwd(t, dir) + // User already has steering files; ours must land alongside without + // disturbing theirs. + steering := filepath.Join(dir, kiroSteeringDir) + if err := os.MkdirAll(steering, 0o755); err != nil { + t.Fatal(err) + } + other := filepath.Join(steering, "team-rules.md") + if err := os.WriteFile(other, []byte("# Team rules\nbe nice\n"), 0o644); err != nil { + t.Fatal(err) + } + + if _, err := (kiro{}).Install(); err != nil { + t.Fatal(err) + } + got, err := os.ReadFile(other) + if err != nil { + t.Fatal(err) + } + if string(got) != "# Team rules\nbe nice\n" { + t.Errorf("user steering file was disturbed: %q", got) + } +} + +func TestKiro_Detect_InstalledThenOutdated(t *testing.T) { + dir := fakeRepo(t) + withCwd(t, dir) + if _, err := (kiro{}).Install(); err != nil { + t.Fatal(err) + } + if got := (kiro{}).Detect(); got != StatusInstalled { + t.Fatalf("post-install Detect() = %v, want StatusInstalled", got) + } + + path := filepath.Join(dir, kiroSteeringDir, kiroSkillFile) + body, err := os.ReadFile(path) + if err != nil { + t.Fatal(err) + } + stale := strings.Replace(string(body), ownedFileVersionPrefix+skills.Version, ownedFileVersionPrefix+"0", 1) + if err := os.WriteFile(path, []byte(stale), 0o644); err != nil { + t.Fatal(err) + } + if got := (kiro{}).Detect(); got != StatusOutdated { + t.Fatalf("Detect() stale marker = %v, want StatusOutdated", got) + } +} + +func TestKiro_Install_Idempotent(t *testing.T) { + dir := fakeRepo(t) + withCwd(t, dir) + + if _, err := (kiro{}).Install(); err != nil { + t.Fatal(err) + } + first, err := os.ReadFile(filepath.Join(dir, kiroSteeringDir, kiroSkillFile)) + if err != nil { + t.Fatal(err) + } + if _, err := (kiro{}).Install(); err != nil { + t.Fatalf("second Install() = %v", err) + } + second, err := os.ReadFile(filepath.Join(dir, kiroSteeringDir, kiroSkillFile)) + if err != nil { + t.Fatal(err) + } + if string(first) != string(second) { + t.Errorf("idempotence broken\n--- first ---\n%s\n--- second ---\n%s", first, second) + } +} + +func TestKiro_Scope_IsProject(t *testing.T) { + if got := (kiro{}).Scope(); got != ScopeProject { + t.Errorf("Scope() = %v, want ScopeProject", got) + } +} + +func TestRegistry_ContainsKiro(t *testing.T) { + if Find("kiro") == nil { + t.Fatal("kiro target not registered") + } +} diff --git a/internal/skillinstaller/opencode.go b/internal/skillinstaller/opencode.go new file mode 100644 index 0000000..d33f628 --- /dev/null +++ b/internal/skillinstaller/opencode.go @@ -0,0 +1,119 @@ +package skillinstaller + +import ( + "fmt" + "os" + "path/filepath" + "strings" + + "github.com/deployhq/deployhq-cli/skills" +) + +func init() { Register(&opencode{}) } + +// opencode installs the skill into OpenCode's user-level instructions file. +// +// Layout (same shape as Codex CLI, but XDG-aware): +// - $XDG_CONFIG_HOME/opencode/AGENTS.md +// (falls back to ~/.config/opencode/AGENTS.md when XDG_CONFIG_HOME is unset) +// - /opencode/deployhq-references/SKILL.md + references/*.md +type opencode struct{} + +func (opencode) Name() string { return "opencode" } +func (opencode) DisplayName() string { return "OpenCode" } +func (opencode) Scope() Scope { return ScopeUser } + +const ( + opencodeInstructionsFile = "AGENTS.md" + opencodeRefsDir = "deployhq-references" +) + +// xdgConfigDir returns $XDG_CONFIG_HOME, or ~/.config as the XDG spec +// fallback. Lives at file scope so future XDG-aware targets can reuse it. +func xdgConfigDir() (string, error) { + if x := os.Getenv("XDG_CONFIG_HOME"); x != "" { + return x, nil + } + home, err := homeDir() + if err != nil { + return "", err + } + return filepath.Join(home, ".config"), nil +} + +func (o opencode) configDir() (string, error) { + base, err := xdgConfigDir() + if err != nil { + return "", err + } + return filepath.Join(base, "opencode"), nil +} + +func (o opencode) Detect() Status { + cfg, err := o.configDir() + if err != nil { + return StatusNotInstalled + } + if _, err := os.Stat(cfg); err != nil { + return StatusNotInstalled + } + + data, err := os.ReadFile(filepath.Join(cfg, opencodeInstructionsFile)) + if err != nil { + return StatusAvailable + } + switch parseSectionVersion(string(data)) { + case "": + return StatusAvailable + case skills.Version: + return StatusInstalled + default: + return StatusOutdated + } +} + +func (o opencode) Install() (string, error) { + cfg, err := o.configDir() + if err != nil { + return "", err + } + if err := os.MkdirAll(cfg, 0o755); err != nil { + return "", err + } + + refsRoot := filepath.Join(cfg, opencodeRefsDir) + if err := os.RemoveAll(refsRoot); err != nil { + return "", err + } + if err := writeEmbeddedTree(skills.FS, "deployhq", refsRoot); err != nil { + return "", err + } + + instrPath := filepath.Join(cfg, opencodeInstructionsFile) + existing, err := os.ReadFile(instrPath) + if err != nil && !os.IsNotExist(err) { + return "", err + } + + section := buildOpenCodeSection(refsRoot) + merged := mergeSection(string(existing), section) + if err := os.WriteFile(instrPath, []byte(merged), 0o644); err != nil { + return "", err + } + return instrPath, nil +} + +func buildOpenCodeSection(refsAbsPath string) string { + var b strings.Builder + fmt.Fprintf(&b, "%s%s%s\n", sectionBeginPrefix, skills.Version, sectionBeginSuffix) + b.WriteString("DeployHQ CLI (`dhq`) is available. Use it to deploy code, manage projects, ") + b.WriteString("servers, and repos via the DeployHQ platform.\n\n") + b.WriteString("Skill guide and per-domain references (read before suggesting `dhq` commands):\n") + fmt.Fprintf(&b, " - %s/SKILL.md\n", refsAbsPath) + fmt.Fprintf(&b, " - %s/references/*.md\n\n", refsAbsPath) + b.WriteString("Domains: deployments, projects, servers, repos, configuration, operations, ") + b.WriteString("global-resources, auth-setup. When the user asks to deploy, prefer `dhq deploy` ") + b.WriteString("with its flags over raw API calls.\n") + b.WriteString(sectionEndMarker) + return b.String() +} diff --git a/internal/skillinstaller/opencode_test.go b/internal/skillinstaller/opencode_test.go new file mode 100644 index 0000000..d0c10c5 --- /dev/null +++ b/internal/skillinstaller/opencode_test.go @@ -0,0 +1,146 @@ +package skillinstaller + +import ( + "os" + "path/filepath" + "strings" + "testing" + + "github.com/deployhq/deployhq-cli/skills" +) + +func TestOpenCode_Detect_NoConfigDir(t *testing.T) { + withHomeDir(t, t.TempDir()) + t.Setenv("XDG_CONFIG_HOME", "") + if got := (opencode{}).Detect(); got != StatusNotInstalled { + t.Fatalf("Detect() = %v, want StatusNotInstalled", got) + } +} + +func TestOpenCode_Detect_RespectsXDG(t *testing.T) { + home := t.TempDir() + xdg := t.TempDir() + withHomeDir(t, home) + t.Setenv("XDG_CONFIG_HOME", xdg) + + // Without the XDG opencode/ dir, detect is NotInstalled even though + // ~/.config/opencode might exist somewhere — XDG_CONFIG_HOME wins. + if got := (opencode{}).Detect(); got != StatusNotInstalled { + t.Fatalf("Detect() with empty XDG dir = %v, want StatusNotInstalled", got) + } + + // Create the OpenCode config dir under XDG_CONFIG_HOME → Available. + if err := os.MkdirAll(filepath.Join(xdg, "opencode"), 0o755); err != nil { + t.Fatal(err) + } + if got := (opencode{}).Detect(); got != StatusAvailable { + t.Fatalf("Detect() with XDG opencode dir = %v, want StatusAvailable", got) + } +} + +func TestOpenCode_Detect_FallsBackTo_DotConfig(t *testing.T) { + home := t.TempDir() + withHomeDir(t, home) + t.Setenv("XDG_CONFIG_HOME", "") + + if err := os.MkdirAll(filepath.Join(home, ".config", "opencode"), 0o755); err != nil { + t.Fatal(err) + } + if got := (opencode{}).Detect(); got != StatusAvailable { + t.Fatalf("Detect() in ~/.config/opencode = %v, want StatusAvailable", got) + } +} + +func TestOpenCode_Install_WritesSectionAndRefs(t *testing.T) { + home := t.TempDir() + withHomeDir(t, home) + t.Setenv("XDG_CONFIG_HOME", "") + cfg := filepath.Join(home, ".config", "opencode") + if err := os.MkdirAll(cfg, 0o755); err != nil { + t.Fatal(err) + } + + got, err := (opencode{}).Install() + if err != nil { + t.Fatalf("Install() = %v", err) + } + want := filepath.Join(cfg, opencodeInstructionsFile) + if got != want { + t.Errorf("Install() path = %q, want %q", got, want) + } + + body, err := os.ReadFile(want) + if err != nil { + t.Fatal(err) + } + if !strings.Contains(string(body), sectionBeginPrefix+skills.Version+sectionBeginSuffix) { + t.Errorf("AGENTS.md missing BEGIN marker:\n%s", body) + } + if !strings.Contains(string(body), sectionEndMarker) { + t.Errorf("AGENTS.md missing END marker:\n%s", body) + } + if _, err := os.Stat(filepath.Join(cfg, opencodeRefsDir, "SKILL.md")); err != nil { + t.Errorf("expected SKILL.md under refs root: %v", err) + } +} + +func TestOpenCode_Install_Idempotent(t *testing.T) { + home := t.TempDir() + withHomeDir(t, home) + t.Setenv("XDG_CONFIG_HOME", "") + if err := os.MkdirAll(filepath.Join(home, ".config", "opencode"), 0o755); err != nil { + t.Fatal(err) + } + if _, err := (opencode{}).Install(); err != nil { + t.Fatal(err) + } + first, err := os.ReadFile(filepath.Join(home, ".config", "opencode", opencodeInstructionsFile)) + if err != nil { + t.Fatal(err) + } + if _, err := (opencode{}).Install(); err != nil { + t.Fatalf("second Install() = %v", err) + } + second, err := os.ReadFile(filepath.Join(home, ".config", "opencode", opencodeInstructionsFile)) + if err != nil { + t.Fatal(err) + } + if string(first) != string(second) { + t.Errorf("idempotence broken\n--- first ---\n%s\n--- second ---\n%s", first, second) + } +} + +func TestOpenCode_Scope_IsUser(t *testing.T) { + if got := (opencode{}).Scope(); got != ScopeUser { + t.Errorf("Scope() = %v, want ScopeUser", got) + } +} + +func TestRegistry_ContainsOpenCode(t *testing.T) { + if Find("opencode") == nil { + t.Fatal("opencode target not registered") + } +} + +func TestXDGConfigDir(t *testing.T) { + home := t.TempDir() + withHomeDir(t, home) + + t.Setenv("XDG_CONFIG_HOME", "") + got, err := xdgConfigDir() + if err != nil { + t.Fatal(err) + } + if got != filepath.Join(home, ".config") { + t.Errorf("default fallback = %q, want %q", got, filepath.Join(home, ".config")) + } + + t.Setenv("XDG_CONFIG_HOME", "/some/path") + got, err = xdgConfigDir() + if err != nil { + t.Fatal(err) + } + if got != "/some/path" { + t.Errorf("XDG override = %q, want %q", got, "/some/path") + } +} diff --git a/internal/skillinstaller/section.go b/internal/skillinstaller/section.go new file mode 100644 index 0000000..a1a593d --- /dev/null +++ b/internal/skillinstaller/section.go @@ -0,0 +1,58 @@ +package skillinstaller + +import ( + "regexp" + "strings" +) + +// Several agents (Windsurf, GitHub Copilot, Codex CLI) all want to share a +// single instructions file with the user's own content. We solve that by +// owning only a sentinel-bounded section in the file and preserving +// everything outside it. +// +// The markers, regex, and merge logic are all identical across those +// targets, so they live here. Per-target files just supply the body of +// the section (intros, ref paths, etc.) and call mergeSection. +const ( + sectionBeginPrefix = "" + sectionEndMarker = "" +) + +// sectionRE matches our owned block including both markers, plus any +// surrounding blank lines so repeated installs don't leave growing gaps in +// the file. (?s) makes . match newlines. +var sectionRE = regexp.MustCompile( + `(?s)\n*.*?\n*`, +) + +// parseSectionVersion returns the version string embedded in the BEGIN +// marker (the bare token between "v" and " -->"), or "" if no section is +// present in the body. +func parseSectionVersion(body string) string { + idx := strings.Index(body, sectionBeginPrefix) + if idx < 0 { + return "" + } + rest := body[idx+len(sectionBeginPrefix):] + end := strings.Index(rest, sectionBeginSuffix) + if end < 0 { + return "" + } + return strings.TrimSpace(rest[:end]) +} + +// mergeSection returns existing with the DeployHQ section replaced (if +// present) or appended (if not). The user's own content — anything outside +// the sentinel markers — is preserved byte-for-byte. +// +// Re-runs converge: mergeSection(mergeSection(x, s), s) == mergeSection(x, s). +// That's what makes 'dhq skills install' safely idempotent. +func mergeSection(existing, section string) string { + if loc := sectionRE.FindStringIndex(existing); loc != nil { + pre := strings.TrimRight(existing[:loc[0]], "\n") + post := strings.TrimLeft(existing[loc[1]:], "\n") + return joinAround(pre, section, post) + } + return joinAround(strings.TrimRight(existing, "\n"), section, "") +} diff --git a/internal/skillinstaller/section_test.go b/internal/skillinstaller/section_test.go new file mode 100644 index 0000000..ce55971 --- /dev/null +++ b/internal/skillinstaller/section_test.go @@ -0,0 +1,65 @@ +package skillinstaller + +import "testing" + +func TestParseSectionVersion(t *testing.T) { + cases := []struct { + name string + in string + want string + }{ + {"empty", "", ""}, + {"no marker", "# Just user rules\n", ""}, + {"v1", "prefix\n\nbody\n\nsuffix\n", "1"}, + {"v42", "\nx\n\n", "42"}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + if got := parseSectionVersion(tc.in); got != tc.want { + t.Errorf("got %q, want %q", got, tc.want) + } + }) + } +} + +func TestMergeSection_AppendsToEmpty(t *testing.T) { + got := mergeSection("", "\nbody\n") + want := "\nbody\n\n" + if got != want { + t.Errorf("got %q, want %q", got, want) + } +} + +func TestMergeSection_PreservesPreAndPost(t *testing.T) { + pre := "# top\n" + post := "# bottom\n" + existing := pre + "\n\nold\n\n\n" + post + got := mergeSection(existing, "\nnew\n") + + for _, must := range []string{"# top", "# bottom", "v1 -->", "new"} { + if !contains(got, must) { + t.Errorf("missing %q in result:\n%s", must, got) + } + } + if contains(got, "old") || contains(got, "v0 -->") { + t.Errorf("stale content survived:\n%s", got) + } +} + +func TestMergeSection_Idempotent(t *testing.T) { + section := "\nbody\n" + first := mergeSection("# user\n", section) + second := mergeSection(first, section) + if first != second { + t.Errorf("merge not idempotent\n--- first ---\n%s\n--- second ---\n%s", first, second) + } +} + +func contains(s, sub string) bool { + for i := 0; i+len(sub) <= len(s); i++ { + if s[i:i+len(sub)] == sub { + return true + } + } + return false +} diff --git a/internal/skillinstaller/windsurf.go b/internal/skillinstaller/windsurf.go new file mode 100644 index 0000000..c7249b9 --- /dev/null +++ b/internal/skillinstaller/windsurf.go @@ -0,0 +1,161 @@ +package skillinstaller + +import ( + "fmt" + "os" + "path/filepath" + "strings" + + "github.com/deployhq/deployhq-cli/skills" +) + +func init() { Register(&windsurf{}) } + +// windsurf installs the skill into Windsurf's global rules file. +// +// Layout: +// - ~/.codeium/windsurf/memories/global_rules.md +// A single file shared with the user's own rules. We own only a +// sentinel-bounded section; everything outside it is preserved +// verbatim across (re)installs. +// - ~/.codeium/windsurf/memories/deployhq-references/*.md +// The full reference tree, written so the agent can read deep docs on +// demand. global_rules.md has a length budget historically — keeping +// the rules section terse and parking detail in a side directory keeps +// it inside that budget. +// +// Note: Windows users keep Codeium data under %APPDATA%\codeium, not +// %USERPROFILE%\.codeium. Until we add Windows-specific path handling, +// Detect() will return StatusNotInstalled on Windows even when Windsurf is +// installed — that's the correct safe default (no false-positive prompts). +type windsurf struct{} + +func (windsurf) Name() string { return "windsurf" } +func (windsurf) DisplayName() string { return "Windsurf" } +func (windsurf) Scope() Scope { return ScopeUser } + +const ( + windsurfRulesFile = "global_rules.md" + windsurfRefsDir = "deployhq-references" +) + +func (w windsurf) memoriesDir() (string, error) { + home, err := homeDir() + if err != nil { + return "", err + } + return filepath.Join(home, ".codeium", "windsurf", "memories"), nil +} + +func (w windsurf) installedAtDir() (string, error) { + home, err := homeDir() + if err != nil { + return "", err + } + // We treat ~/.codeium/windsurf as the "Windsurf is installed" signal. + return filepath.Join(home, ".codeium", "windsurf"), nil +} + +func (w windsurf) Detect() Status { + dir, err := w.installedAtDir() + if err != nil { + return StatusNotInstalled + } + if _, err := os.Stat(dir); err != nil { + return StatusNotInstalled + } + + mem, err := w.memoriesDir() + if err != nil { + return StatusNotInstalled + } + rulesPath := filepath.Join(mem, windsurfRulesFile) + data, err := os.ReadFile(rulesPath) + if err != nil { + // Windsurf installed but no rules file yet — skill is available. + return StatusAvailable + } + + version := parseSectionVersion(string(data)) + switch version { + case "": + return StatusAvailable + case skills.Version: + return StatusInstalled + default: + return StatusOutdated + } +} + +func (w windsurf) Install() (string, error) { + mem, err := w.memoriesDir() + if err != nil { + return "", err + } + if err := os.MkdirAll(mem, 0o755); err != nil { + return "", err + } + + // Write the deep-reference tree alongside global_rules.md so the agent + // can pull in detail on demand. SKILL.md is intentionally included + // here too — the rules-section pointer can address it by relative path. + refsRoot := filepath.Join(mem, windsurfRefsDir) + if err := os.RemoveAll(refsRoot); err != nil { + return "", err + } + if err := writeEmbeddedTree(skills.FS, "deployhq", refsRoot); err != nil { + return "", err + } + + rulesPath := filepath.Join(mem, windsurfRulesFile) + existing, err := os.ReadFile(rulesPath) + if err != nil && !os.IsNotExist(err) { + return "", err + } + + section := buildWindsurfSection(refsRoot) + merged := mergeSection(string(existing), section) + + if err := os.WriteFile(rulesPath, []byte(merged), 0o644); err != nil { + return "", err + } + return rulesPath, nil +} + +// buildWindsurfSection produces the sentinel-bounded block we own in +// global_rules.md. It is intentionally short — Windsurf's global rules +// budget is tight, so we keep this to a pointer + key behaviours and rely +// on the agent reading deployhq-references/ for detail. +func buildWindsurfSection(refsAbsPath string) string { + var b strings.Builder + fmt.Fprintf(&b, "%s%s%s\n", sectionBeginPrefix, skills.Version, sectionBeginSuffix) + b.WriteString("DeployHQ CLI (`dhq`) is available. Use it to deploy code, manage projects, ") + b.WriteString("servers, and repos via the DeployHQ platform.\n\n") + b.WriteString("Full skill guide and per-domain references live at:\n") + fmt.Fprintf(&b, " %s/SKILL.md\n", refsAbsPath) + fmt.Fprintf(&b, " %s/references/*.md\n\n", refsAbsPath) + b.WriteString("When the user wants to deploy, check deployment status, manage projects/") + b.WriteString("servers, or interact with the DeployHQ platform, read SKILL.md first, then ") + b.WriteString("the relevant reference file for the domain (deployments, projects, servers, ") + b.WriteString("repos, configuration, operations, global-resources, auth-setup).\n") + b.WriteString(sectionEndMarker) + return b.String() +} + +// joinAround stitches pre + section + post with a single blank line between +// non-empty halves, and exactly one trailing newline. Shared with copilot +// and codex via mergeSection in section.go. +func joinAround(pre, section, post string) string { + var b strings.Builder + if pre != "" { + b.WriteString(pre) + b.WriteString("\n\n") + } + b.WriteString(section) + if post != "" { + b.WriteString("\n\n") + b.WriteString(strings.TrimRight(post, "\n")) + } + b.WriteString("\n") + return b.String() +} diff --git a/internal/skillinstaller/windsurf_test.go b/internal/skillinstaller/windsurf_test.go new file mode 100644 index 0000000..c99b8ad --- /dev/null +++ b/internal/skillinstaller/windsurf_test.go @@ -0,0 +1,211 @@ +package skillinstaller + +import ( + "os" + "path/filepath" + "strings" + "testing" + + "github.com/deployhq/deployhq-cli/skills" +) + +func TestWindsurf_Detect_NoConfigDir(t *testing.T) { + withHomeDir(t, t.TempDir()) + if got := (windsurf{}).Detect(); got != StatusNotInstalled { + t.Fatalf("Detect() = %v, want StatusNotInstalled", got) + } +} + +func TestWindsurf_Detect_InstalledNoRulesFile(t *testing.T) { + home := t.TempDir() + withHomeDir(t, home) + if err := os.MkdirAll(filepath.Join(home, ".codeium", "windsurf"), 0o755); err != nil { + t.Fatal(err) + } + if got := (windsurf{}).Detect(); got != StatusAvailable { + t.Fatalf("Detect() = %v, want StatusAvailable", got) + } +} + +func TestWindsurf_Detect_RulesFileWithoutSection(t *testing.T) { + home := t.TempDir() + withHomeDir(t, home) + memDir := filepath.Join(home, ".codeium", "windsurf", "memories") + if err := os.MkdirAll(memDir, 0o755); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(memDir, windsurfRulesFile), []byte("# My user rules\n\nBe concise.\n"), 0o644); err != nil { + t.Fatal(err) + } + if got := (windsurf{}).Detect(); got != StatusAvailable { + t.Fatalf("Detect() = %v, want StatusAvailable", got) + } +} + +func TestWindsurf_Install_WritesSectionAndRefs(t *testing.T) { + home := t.TempDir() + withHomeDir(t, home) + if err := os.MkdirAll(filepath.Join(home, ".codeium", "windsurf"), 0o755); err != nil { + t.Fatal(err) + } + + got, err := (windsurf{}).Install() + if err != nil { + t.Fatalf("Install() error = %v", err) + } + want := filepath.Join(home, ".codeium", "windsurf", "memories", windsurfRulesFile) + if got != want { + t.Errorf("Install() path = %q, want %q", got, want) + } + + rules, err := os.ReadFile(want) + if err != nil { + t.Fatalf("read rules: %v", err) + } + body := string(rules) + if !strings.Contains(body, sectionBeginPrefix+skills.Version+sectionBeginSuffix) { + t.Errorf("rules missing BEGIN marker with current version; body=%q", body) + } + if !strings.Contains(body, sectionEndMarker) { + t.Errorf("rules missing END marker; body=%q", body) + } + + // References tree should land alongside. + refsRoot := filepath.Join(home, ".codeium", "windsurf", "memories", windsurfRefsDir) + if _, err := os.Stat(filepath.Join(refsRoot, "SKILL.md")); err != nil { + t.Errorf("expected SKILL.md under refs root: %v", err) + } + entries, err := os.ReadDir(filepath.Join(refsRoot, "references")) + if err != nil || len(entries) == 0 { + t.Errorf("expected references/*.md under refs root: err=%v entries=%d", err, len(entries)) + } +} + +func TestWindsurf_Install_PreservesUserRules(t *testing.T) { + home := t.TempDir() + withHomeDir(t, home) + memDir := filepath.Join(home, ".codeium", "windsurf", "memories") + if err := os.MkdirAll(memDir, 0o755); err != nil { + t.Fatal(err) + } + userRules := "# My rules\n\nAlways write tests.\n" + if err := os.WriteFile(filepath.Join(memDir, windsurfRulesFile), []byte(userRules), 0o644); err != nil { + t.Fatal(err) + } + + if _, err := (windsurf{}).Install(); err != nil { + t.Fatalf("Install() error = %v", err) + } + + got, err := os.ReadFile(filepath.Join(memDir, windsurfRulesFile)) + if err != nil { + t.Fatal(err) + } + body := string(got) + if !strings.Contains(body, "Always write tests.") { + t.Errorf("user rules were lost; body=%q", body) + } + if !strings.Contains(body, sectionEndMarker) { + t.Errorf("DeployHQ section not appended; body=%q", body) + } +} + +func TestWindsurf_Install_ReplacesExistingSection(t *testing.T) { + home := t.TempDir() + withHomeDir(t, home) + memDir := filepath.Join(home, ".codeium", "windsurf", "memories") + if err := os.MkdirAll(memDir, 0o755); err != nil { + t.Fatal(err) + } + // Pre-existing rules with a stale DeployHQ section sandwiched between + // user rules. Both halves must survive; the section in the middle gets + // rewritten. + pre := "# Top rule\n\n\nold stale content here\n\n\n# Bottom rule\n" + if err := os.WriteFile(filepath.Join(memDir, windsurfRulesFile), []byte(pre), 0o644); err != nil { + t.Fatal(err) + } + + if _, err := (windsurf{}).Install(); err != nil { + t.Fatalf("Install() error = %v", err) + } + + got, err := os.ReadFile(filepath.Join(memDir, windsurfRulesFile)) + if err != nil { + t.Fatal(err) + } + body := string(got) + for _, must := range []string{"# Top rule", "# Bottom rule", sectionBeginPrefix + skills.Version + sectionBeginSuffix} { + if !strings.Contains(body, must) { + t.Errorf("missing %q in merged body:\n%s", must, body) + } + } + if strings.Contains(body, "old stale content here") { + t.Errorf("stale section content survived:\n%s", body) + } + if strings.Contains(body, "v0 -->") { + t.Errorf("stale BEGIN marker survived:\n%s", body) + } +} + +func TestWindsurf_Detect_InstalledAndOutdated(t *testing.T) { + home := t.TempDir() + withHomeDir(t, home) + if err := os.MkdirAll(filepath.Join(home, ".codeium", "windsurf"), 0o755); err != nil { + t.Fatal(err) + } + if _, err := (windsurf{}).Install(); err != nil { + t.Fatal(err) + } + if got := (windsurf{}).Detect(); got != StatusInstalled { + t.Fatalf("after install Detect() = %v, want StatusInstalled", got) + } + + // Replace BEGIN marker with an older version. + rulesPath := filepath.Join(home, ".codeium", "windsurf", "memories", windsurfRulesFile) + body, err := os.ReadFile(rulesPath) + if err != nil { + t.Fatal(err) + } + stale := strings.Replace(string(body), sectionBeginPrefix+skills.Version, sectionBeginPrefix+"0", 1) + if err := os.WriteFile(rulesPath, []byte(stale), 0o644); err != nil { + t.Fatal(err) + } + if got := (windsurf{}).Detect(); got != StatusOutdated { + t.Fatalf("stale version Detect() = %v, want StatusOutdated", got) + } +} + +func TestWindsurf_Install_Idempotent(t *testing.T) { + home := t.TempDir() + withHomeDir(t, home) + if err := os.MkdirAll(filepath.Join(home, ".codeium", "windsurf"), 0o755); err != nil { + t.Fatal(err) + } + if _, err := (windsurf{}).Install(); err != nil { + t.Fatal(err) + } + rulesPath := filepath.Join(home, ".codeium", "windsurf", "memories", windsurfRulesFile) + first, err := os.ReadFile(rulesPath) + if err != nil { + t.Fatal(err) + } + if _, err := (windsurf{}).Install(); err != nil { + t.Fatalf("second Install() = %v", err) + } + second, err := os.ReadFile(rulesPath) + if err != nil { + t.Fatal(err) + } + if string(first) != string(second) { + t.Errorf("re-install produced a different file\n--- first ---\n%s\n--- second ---\n%s", first, second) + } + if got := (windsurf{}).Detect(); got != StatusInstalled { + t.Fatalf("after re-install Detect() = %v, want StatusInstalled", got) + } +} + +func TestRegistry_ContainsWindsurf(t *testing.T) { + if Find("windsurf") == nil { + t.Fatal("windsurf target not registered") + } +} diff --git a/skills/embed.go b/skills/embed.go new file mode 100644 index 0000000..2d0db4a --- /dev/null +++ b/skills/embed.go @@ -0,0 +1,22 @@ +// Package skills exposes the bundled DeployHQ skill files as an embed.FS so +// that other packages (notably internal/skillinstaller) can write them into +// each AI agent's local config directory. +// +// The on-disk layout under skills/deployhq/ is the canonical Claude Code +// skill format: SKILL.md at the root, supporting docs under references/. +// Other agents reshape this into their own format at install time. +package skills + +import "embed" + +// FS contains the deployhq/ skill directory tree, rooted at "deployhq". +// Iterate with fs.WalkDir or read individual files with FS.ReadFile. +// +//go:embed deployhq +var FS embed.FS + +// Version is the schema version of the embedded skill. Installers write this +// alongside the installed files so a future `dhq` can detect stale installs +// and update without re-prompting. Bump when the skill content changes +// meaningfully (new commands, restructured references, etc.). +const Version = "1" From bff3879f72fa03fc7937cf5754573e18cb746f3c Mon Sep 17 00:00:00 2001 From: Facundo Farias Date: Mon, 22 Jun 2026 08:26:25 +0200 Subject: [PATCH 2/6] fix(skills): respect project-scope boundary in bulk install and use repo root Codex review (PR #26) flagged two bugs in the skill-installer: 1. `dhq skills install` (no --agent) iterated every detected target, silently mutating .github/, AGENTS.md, .clinerules/, and .kiro/ in the user's current repo. That contradicts the PR's own contract that project-scope targets are opt-in. Filter bulk install to ScopeUser; print a hint listing skipped project-scope agents and the --agent invocation to install each. 2. Copilot's repoRoot() returned cwd, not the .git-bearing ancestor. Running `dhq` from a subdirectory wrote to `subdir/.github/copilot-instructions.md`, where Copilot doesn't look. Same pattern existed in Cline, Kiro, and Antigravity. Extract a shared findRepoRoot() helper and route all four project-scope targets through it for both Detect() and Install(). Adds regression tests in repo_test.go: each project-scope target, when invoked from a nested subdirectory, must write at the repo root and must NOT create anything in the subdirectory. Co-Authored-By: Claude Opus 4.7 (1M context) --- internal/commands/skills.go | 19 ++++- internal/skillinstaller/antigravity.go | 37 ++------ internal/skillinstaller/cline.go | 66 +++------------ internal/skillinstaller/copilot.go | 54 ++---------- internal/skillinstaller/kiro.go | 43 ++-------- internal/skillinstaller/repo.go | 33 ++++++++ internal/skillinstaller/repo_test.go | 113 +++++++++++++++++++++++++ 7 files changed, 197 insertions(+), 168 deletions(-) create mode 100644 internal/skillinstaller/repo.go create mode 100644 internal/skillinstaller/repo_test.go diff --git a/internal/commands/skills.go b/internal/commands/skills.go index c73d8fc..02b25f8 100644 --- a/internal/commands/skills.go +++ b/internal/commands/skills.go @@ -65,6 +65,7 @@ func newSkillsInstallCmd() *cobra.Command { env := cliCtx.Envelope var targets []skillinstaller.Target + var skippedProject []string if agentFlag != "" { t := skillinstaller.Find(agentFlag) if t == nil { @@ -75,8 +76,17 @@ func newSkillsInstallCmd() *cobra.Command { } targets = []skillinstaller.Target{t} } else { + // Bulk install only touches user-scope targets — installing + // project-scope ones (Copilot, Cline, Kiro, Antigravity) + // would silently mutate the current repo, which we promise + // not to do without --agent. Defer those to an explicit + // `dhq skills install --agent ` invocation. for _, d := range skillinstaller.DetectInstalled() { - targets = append(targets, d.Target) + if d.Target.Scope() == skillinstaller.ScopeUser { + targets = append(targets, d.Target) + } else { + skippedProject = append(skippedProject, d.Target.Name()) + } } } @@ -103,6 +113,13 @@ func newSkillsInstallCmd() *cobra.Command { } } + if len(skippedProject) > 0 { + env.Status("Skipped project-scope agents (modify the current repo, opt-in only):") + for _, name := range skippedProject { + env.Status(" - %s — install with: dhq skills install --agent %s", name, name) + } + } + if failed > 0 { return &output.InternalError{Message: fmt.Sprintf("%d install(s) failed", failed)} } diff --git a/internal/skillinstaller/antigravity.go b/internal/skillinstaller/antigravity.go index 2903571..1299df1 100644 --- a/internal/skillinstaller/antigravity.go +++ b/internal/skillinstaller/antigravity.go @@ -41,32 +41,12 @@ const ( antigravityRefsDir = ".antigravity/deployhq" ) -func (a antigravity) inRepo() bool { - dir, err := getCwd() - if err != nil { - return false - } - for { - if _, err := os.Stat(filepath.Join(dir, ".git")); err == nil { - return true - } - parent := filepath.Dir(dir) - if parent == dir { - return false - } - dir = parent - } -} - func (a antigravity) Detect() Status { - if !a.inRepo() { + root, ok := findRepoRoot() + if !ok { return StatusNotInstalled } - cwd, err := getCwd() - if err != nil { - return StatusNotInstalled - } - data, err := os.ReadFile(filepath.Join(cwd, antigravityInstructionsFile)) + data, err := os.ReadFile(filepath.Join(root, antigravityInstructionsFile)) if err != nil { return StatusAvailable } @@ -81,17 +61,14 @@ func (a antigravity) Detect() Status { } func (a antigravity) Install() (string, error) { - if !a.inRepo() { + root, ok := findRepoRoot() + if !ok { cwd, _ := getCwd() return "", fmt.Errorf("not a git repository (cwd=%s); run from inside a repo", cwd) } - cwd, err := getCwd() - if err != nil { - return "", err - } // Refresh the reference tree at /.antigravity/deployhq/. - refsRoot := filepath.Join(cwd, antigravityRefsDir) + refsRoot := filepath.Join(root, antigravityRefsDir) if err := os.RemoveAll(refsRoot); err != nil { return "", err } @@ -99,7 +76,7 @@ func (a antigravity) Install() (string, error) { return "", err } - instrPath := filepath.Join(cwd, antigravityInstructionsFile) + instrPath := filepath.Join(root, antigravityInstructionsFile) existing, err := os.ReadFile(instrPath) if err != nil && !os.IsNotExist(err) { return "", err diff --git a/internal/skillinstaller/cline.go b/internal/skillinstaller/cline.go index da15142..1b74e3e 100644 --- a/internal/skillinstaller/cline.go +++ b/internal/skillinstaller/cline.go @@ -34,62 +34,20 @@ const ( clineSkillFile = "deployhq.md" ) -// inRepo mirrors copilot's ancestor-walk: any .git/ in the cwd or an -// ancestor means we're inside a project where installing makes sense. -func (c cline) inRepo() bool { - dir, err := getCwd() - if err != nil { - return false - } - for { - if _, err := os.Stat(filepath.Join(dir, ".git")); err == nil { - return true - } - parent := filepath.Dir(dir) - if parent == dir { - return false - } - dir = parent - } -} - -// skillFile returns /.clinerules/deployhq.md. -func (c cline) skillFile() (string, error) { - cwd, err := getCwd() - if err != nil { - return "", err - } - return filepath.Join(cwd, clineRulesDir, clineSkillFile), nil -} - -// rulesPathStat returns info about /.clinerules so callers can tell -// the difference between "absent", "directory" (ours), and "file" (legacy -// single-file form, which would conflict with our directory writes). -func (c cline) rulesPathStat() (os.FileInfo, error) { - cwd, err := getCwd() - if err != nil { - return nil, err - } - return os.Stat(filepath.Join(cwd, clineRulesDir)) -} - func (c cline) Detect() Status { - if !c.inRepo() { + root, ok := findRepoRoot() + if !ok { return StatusNotInstalled } - // If .clinerules exists as a file (legacy shape), we can't safely - // proceed. Report Available so it shows up in listings, and let - // Install() fail loudly with an actionable message. - if info, err := c.rulesPathStat(); err == nil && !info.IsDir() { + // If .clinerules exists as a file at the repo root (legacy shape), + // we can't safely proceed. Report Available so it shows up in + // listings, and let Install() fail loudly with an actionable message. + if info, err := os.Stat(filepath.Join(root, clineRulesDir)); err == nil && !info.IsDir() { return StatusAvailable } - p, err := c.skillFile() - if err != nil { - return StatusNotInstalled - } - data, err := os.ReadFile(p) + data, err := os.ReadFile(filepath.Join(root, clineRulesDir, clineSkillFile)) if err != nil { return StatusAvailable } @@ -104,11 +62,12 @@ func (c cline) Detect() Status { } func (c cline) Install() (string, error) { - if !c.inRepo() { + root, ok := findRepoRoot() + if !ok { cwd, _ := getCwd() return "", fmt.Errorf("not a git repository (cwd=%s); run from inside a repo", cwd) } - if info, err := c.rulesPathStat(); err == nil && !info.IsDir() { + if info, err := os.Stat(filepath.Join(root, clineRulesDir)); err == nil && !info.IsDir() { return "", fmt.Errorf( "%s exists as a file (legacy Cline single-rule shape); "+ "move its contents into %s/main.md (or any *.md name), delete the file, "+ @@ -117,10 +76,7 @@ func (c cline) Install() (string, error) { ) } - p, err := c.skillFile() - if err != nil { - return "", err - } + p := filepath.Join(root, clineRulesDir, clineSkillFile) if err := os.MkdirAll(filepath.Dir(p), 0o755); err != nil { return "", err } diff --git a/internal/skillinstaller/copilot.go b/internal/skillinstaller/copilot.go index 24f8630..85939d5 100644 --- a/internal/skillinstaller/copilot.go +++ b/internal/skillinstaller/copilot.go @@ -43,50 +43,16 @@ const ( copilotRefsDir = ".github/copilot/deployhq" ) -// getCwd is the working-directory lookup. Overridable in tests. -var getCwd = os.Getwd - -func (c copilot) repoRoot() (string, error) { - cwd, err := getCwd() - if err != nil { - return "", err - } - return cwd, nil -} - -// inRepo reports whether the cwd looks like a git repo. We accept any -// ancestor having .git/, which matches how someone running 'dhq' inside a -// subdirectory of their project would expect this to behave. -func (c copilot) inRepo() bool { - dir, err := c.repoRoot() - if err != nil { - return false - } - for { - if _, err := os.Stat(filepath.Join(dir, ".git")); err == nil { - return true - } - parent := filepath.Dir(dir) - if parent == dir { - return false - } - dir = parent - } -} - func (c copilot) Detect() Status { - if !c.inRepo() { + root, ok := findRepoRoot() + if !ok { // Not a git repo — nothing to install into. We treat this as // "not installed" so 'dhq skills list' stays informative without // implying we'd write to a random directory. return StatusNotInstalled } - cwd, err := c.repoRoot() - if err != nil { - return StatusNotInstalled - } - data, err := os.ReadFile(filepath.Join(cwd, copilotInstructionsFile)) + data, err := os.ReadFile(filepath.Join(root, copilotInstructionsFile)) if err != nil { return StatusAvailable } @@ -101,16 +67,14 @@ func (c copilot) Detect() Status { } func (c copilot) Install() (string, error) { - cwd, err := c.repoRoot() - if err != nil { - return "", err - } - if !c.inRepo() { + root, ok := findRepoRoot() + if !ok { + cwd, _ := getCwd() return "", fmt.Errorf("not a git repository (cwd=%s); run from inside a repo or use a user-scope target", cwd) } - // Refresh the reference tree at .github/copilot/deployhq/. - refsRoot := filepath.Join(cwd, copilotRefsDir) + // Refresh the reference tree at /.github/copilot/deployhq/. + refsRoot := filepath.Join(root, copilotRefsDir) if err := os.RemoveAll(refsRoot); err != nil { return "", err } @@ -118,7 +82,7 @@ func (c copilot) Install() (string, error) { return "", err } - instrPath := filepath.Join(cwd, copilotInstructionsFile) + instrPath := filepath.Join(root, copilotInstructionsFile) if err := os.MkdirAll(filepath.Dir(instrPath), 0o755); err != nil { return "", err } diff --git a/internal/skillinstaller/kiro.go b/internal/skillinstaller/kiro.go index 4cce333..c07eb04 100644 --- a/internal/skillinstaller/kiro.go +++ b/internal/skillinstaller/kiro.go @@ -34,41 +34,12 @@ const ( kiroSkillFile = "deployhq.md" ) -// inRepo mirrors copilot/cline's ancestor-walk for .git detection. -func (k kiro) inRepo() bool { - dir, err := getCwd() - if err != nil { - return false - } - for { - if _, err := os.Stat(filepath.Join(dir, ".git")); err == nil { - return true - } - parent := filepath.Dir(dir) - if parent == dir { - return false - } - dir = parent - } -} - -func (k kiro) skillPath() (string, error) { - cwd, err := getCwd() - if err != nil { - return "", err - } - return filepath.Join(cwd, kiroSteeringDir, kiroSkillFile), nil -} - func (k kiro) Detect() Status { - if !k.inRepo() { - return StatusNotInstalled - } - p, err := k.skillPath() - if err != nil { + root, ok := findRepoRoot() + if !ok { return StatusNotInstalled } - data, err := os.ReadFile(p) + data, err := os.ReadFile(filepath.Join(root, kiroSteeringDir, kiroSkillFile)) if err != nil { return StatusAvailable } @@ -83,14 +54,12 @@ func (k kiro) Detect() Status { } func (k kiro) Install() (string, error) { - if !k.inRepo() { + root, ok := findRepoRoot() + if !ok { cwd, _ := getCwd() return "", fmt.Errorf("not a git repository (cwd=%s); run from inside a repo", cwd) } - p, err := k.skillPath() - if err != nil { - return "", err - } + p := filepath.Join(root, kiroSteeringDir, kiroSkillFile) if err := os.MkdirAll(filepath.Dir(p), 0o755); err != nil { return "", err } diff --git a/internal/skillinstaller/repo.go b/internal/skillinstaller/repo.go new file mode 100644 index 0000000..2af878b --- /dev/null +++ b/internal/skillinstaller/repo.go @@ -0,0 +1,33 @@ +package skillinstaller + +import ( + "os" + "path/filepath" +) + +// getCwd is the working-directory lookup. Overridable in tests so they +// don't depend on the dev box's real cwd. Project-scope targets use this +// as the starting point for findRepoRoot. +var getCwd = os.Getwd + +// findRepoRoot walks up from the current working directory looking for a +// .git ancestor and returns (rootPath, true) when found. Project-scope +// targets must use this — never cwd — for both detection and writes, so +// that running `dhq` from a subdirectory still installs into the actual +// repo root (Copilot won't load `subdir/.github/copilot-instructions.md`). +func findRepoRoot() (string, bool) { + dir, err := getCwd() + if err != nil { + return "", false + } + for { + if _, err := os.Stat(filepath.Join(dir, ".git")); err == nil { + return dir, true + } + parent := filepath.Dir(dir) + if parent == dir { + return "", false + } + dir = parent + } +} diff --git a/internal/skillinstaller/repo_test.go b/internal/skillinstaller/repo_test.go new file mode 100644 index 0000000..b616e68 --- /dev/null +++ b/internal/skillinstaller/repo_test.go @@ -0,0 +1,113 @@ +package skillinstaller + +import ( + "os" + "path/filepath" + "testing" +) + +func TestFindRepoRoot_WalksAncestors(t *testing.T) { + root := fakeRepo(t) + sub := filepath.Join(root, "a", "b", "c") + if err := os.MkdirAll(sub, 0o755); err != nil { + t.Fatal(err) + } + withCwd(t, sub) + + got, ok := findRepoRoot() + if !ok { + t.Fatal("findRepoRoot() returned !ok inside a repo subdirectory") + } + if got != root { + t.Errorf("findRepoRoot() = %q, want %q", got, root) + } +} + +func TestFindRepoRoot_OutsideRepo(t *testing.T) { + withCwd(t, t.TempDir()) + if _, ok := findRepoRoot(); ok { + t.Fatal("findRepoRoot() returned ok outside a git repo") + } +} + +// Regression: each project-scope target must write at the repo root, not +// the cwd, when invoked from a subdirectory. Bug surfaced by Codex review +// on copilot.go; the same pattern existed in cline/kiro/antigravity. + +func TestCopilot_Install_WritesAtRepoRoot_FromSubdirectory(t *testing.T) { + root := fakeRepo(t) + sub := filepath.Join(root, "deep", "sub") + if err := os.MkdirAll(sub, 0o755); err != nil { + t.Fatal(err) + } + withCwd(t, sub) + + got, err := (copilot{}).Install() + if err != nil { + t.Fatalf("Install() = %v", err) + } + want := filepath.Join(root, copilotInstructionsFile) + if got != want { + t.Errorf("Install() path = %q, want %q (must be at repo root, not subdir)", got, want) + } + if _, err := os.Stat(filepath.Join(sub, copilotInstructionsFile)); err == nil { + t.Errorf("Install() wrote to subdir %s — must be at repo root", sub) + } +} + +func TestCline_Install_WritesAtRepoRoot_FromSubdirectory(t *testing.T) { + root := fakeRepo(t) + sub := filepath.Join(root, "deep", "sub") + if err := os.MkdirAll(sub, 0o755); err != nil { + t.Fatal(err) + } + withCwd(t, sub) + + got, err := (cline{}).Install() + if err != nil { + t.Fatalf("Install() = %v", err) + } + want := filepath.Join(root, clineRulesDir, clineSkillFile) + if got != want { + t.Errorf("Install() path = %q, want %q (must be at repo root)", got, want) + } + if _, err := os.Stat(filepath.Join(sub, clineRulesDir)); err == nil { + t.Errorf("Install() created %s/.clinerules — must be at repo root", sub) + } +} + +func TestKiro_Install_WritesAtRepoRoot_FromSubdirectory(t *testing.T) { + root := fakeRepo(t) + sub := filepath.Join(root, "deep", "sub") + if err := os.MkdirAll(sub, 0o755); err != nil { + t.Fatal(err) + } + withCwd(t, sub) + + got, err := (kiro{}).Install() + if err != nil { + t.Fatalf("Install() = %v", err) + } + want := filepath.Join(root, kiroSteeringDir, kiroSkillFile) + if got != want { + t.Errorf("Install() path = %q, want %q (must be at repo root)", got, want) + } +} + +func TestAntigravity_Install_WritesAtRepoRoot_FromSubdirectory(t *testing.T) { + root := fakeRepo(t) + sub := filepath.Join(root, "deep", "sub") + if err := os.MkdirAll(sub, 0o755); err != nil { + t.Fatal(err) + } + withCwd(t, sub) + + got, err := (antigravity{}).Install() + if err != nil { + t.Fatalf("Install() = %v", err) + } + want := filepath.Join(root, antigravityInstructionsFile) + if got != want { + t.Errorf("Install() path = %q, want %q (must be at repo root)", got, want) + } +} From fa86d7dd0348761d36769ff5e0b58c8306a49bcd Mon Sep 17 00:00:00 2001 From: Facundo Farias Date: Mon, 22 Jun 2026 08:40:27 +0200 Subject: [PATCH 3/6] fix(skills): address CodeRabbit review nits and code/doc mismatches MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Addresses CodeRabbit findings on PR #26 that survived the first fixup: - aider: PostInstallNote double-quotes the path so the suggested `read: ["..."]` snippet and `--read "..."` invocation are safe to paste verbatim when the user's home contains spaces or other characters that would otherwise need shell/YAML escaping. - opencode: xdgConfigDir requires XDG_CONFIG_HOME to be absolute, per the XDG Base Directory spec. Falls through to ~/.config when the variable holds a relative path, so a misconfigured env never causes writes into the dev's cwd. - installer: tighten two docstrings that lied about behavior. DetectInstalled no longer claims runtime-agent special handling (that lives in offerSkillInstall, not here); Find documents its actual case-sensitive matching and explains why canonical names are always lowercase. - flatten: parseOwnedFileVersion now requires the marker to be the first thing in the body. Owned-file targets (Aider/Cline/Continue/ Kiro) write the marker at the top via buildOwnedSkillFile, so any content above it means the file isn't ours — Detect now reports Available so the next install rewrites cleanly. aider_test flips the corresponding case from "v42 mid file → 42" to "rejected". - skills list: Short text reads "List supported AI agents and their skill install status" — the output enumerates every registered target, not just those detected on disk. - section_test: drops the hand-rolled `contains` helper in favour of strings.Contains. Co-Authored-By: Claude Opus 4.7 (1M context) --- internal/commands/skills.go | 2 +- internal/skillinstaller/aider.go | 19 ++++++++++++++++++- internal/skillinstaller/aider_test.go | 7 +++++-- internal/skillinstaller/flatten.go | 12 ++++++++---- internal/skillinstaller/installer.go | 15 +++++++++------ internal/skillinstaller/opencode.go | 7 ++++++- internal/skillinstaller/section_test.go | 17 ++++++----------- 7 files changed, 53 insertions(+), 26 deletions(-) diff --git a/internal/commands/skills.go b/internal/commands/skills.go index 02b25f8..9656f30 100644 --- a/internal/commands/skills.go +++ b/internal/commands/skills.go @@ -35,7 +35,7 @@ type skillRow struct { func newSkillsListCmd() *cobra.Command { return &cobra.Command{ Use: "list", - Short: "List detected AI agents and skill install status", + Short: "List supported AI agents and their skill install status", RunE: func(cmd *cobra.Command, args []string) error { env := cliCtx.Envelope diff --git a/internal/skillinstaller/aider.go b/internal/skillinstaller/aider.go index e3ff7c4..60d31b3 100644 --- a/internal/skillinstaller/aider.go +++ b/internal/skillinstaller/aider.go @@ -5,6 +5,7 @@ import ( "os" "os/exec" "path/filepath" + "strings" "github.com/deployhq/deployhq-cli/skills" ) @@ -116,15 +117,31 @@ func (a aider) Install() (string, error) { // PostInstallNote tells the user how to wire the skill into Aider, since // we can't safely auto-edit .aider.conf.yml. Surfaced by both the hello // hook and `dhq skills install` via the Noter interface. +// +// The path is double-quoted so the snippet is safe to paste verbatim into +// both YAML (read: ["..."]) and a shell command (--read "..."), even when +// the user's home contains spaces or other characters that would otherwise +// need escaping. func (a aider) PostInstallNote() string { p, err := a.skillPath() if err != nil { return "" } + q := quotePathForYAMLAndShell(p) return fmt.Sprintf( "To load on every Aider run: add `read: [%s]` to ~/.aider.conf.yml "+ "(or pass `--read %s` ad-hoc).", - p, p, + q, q, ) } +// quotePathForYAMLAndShell wraps a path in double quotes with internal +// backslashes and double quotes escaped. The result is valid in both a +// YAML double-quoted scalar and a POSIX shell double-quoted string, which +// is the only quoting context the PostInstallNote needs to support. +func quotePathForYAMLAndShell(p string) string { + p = strings.ReplaceAll(p, `\`, `\\`) + p = strings.ReplaceAll(p, `"`, `\"`) + return `"` + p + `"` +} + diff --git a/internal/skillinstaller/aider_test.go b/internal/skillinstaller/aider_test.go index 9b7fc7c..c9db212 100644 --- a/internal/skillinstaller/aider_test.go +++ b/internal/skillinstaller/aider_test.go @@ -178,8 +178,11 @@ func TestParseOwnedFileVersion(t *testing.T) { }{ {"empty", "", ""}, {"no marker", "# user content\n", ""}, - {"v1", "\nbody\n", "1"}, - {"v42 mid file", "x\n\nbody\n", "42"}, + {"v1 at start", "\nbody\n", "1"}, + // Owned-file contract: marker must be the first thing in the + // file. Anything above it means the file isn't ours and we + // should rewrite, not trust it. + {"marker not on first line is rejected", "x\n\nbody\n", ""}, } for _, tc := range cases { t.Run(tc.name, func(t *testing.T) { diff --git a/internal/skillinstaller/flatten.go b/internal/skillinstaller/flatten.go index 4156ab8..0bf97fd 100644 --- a/internal/skillinstaller/flatten.go +++ b/internal/skillinstaller/flatten.go @@ -43,13 +43,17 @@ func buildOwnedSkillFile(efs fs.FS, root string) ([]byte, error) { } // parseOwnedFileVersion returns the version embedded in our top-of-file -// HTML comment, or "" if no marker is present. +// HTML comment, or "" if no marker is present at the start of the body. +// +// The marker MUST be the first thing in the file. We write it that way +// via buildOwnedSkillFile, and if a future read finds content above the +// marker we treat the file as "not ours" — reporting StatusAvailable so +// the next install rewrites cleanly rather than appending to user edits. func parseOwnedFileVersion(body string) string { - idx := strings.Index(body, ownedFileVersionPrefix) - if idx < 0 { + if !strings.HasPrefix(body, ownedFileVersionPrefix) { return "" } - rest := body[idx+len(ownedFileVersionPrefix):] + rest := body[len(ownedFileVersionPrefix):] end := strings.Index(rest, ownedFileVersionSuffix) if end < 0 { return "" diff --git a/internal/skillinstaller/installer.go b/internal/skillinstaller/installer.go index 62c6c65..073b838 100644 --- a/internal/skillinstaller/installer.go +++ b/internal/skillinstaller/installer.go @@ -132,10 +132,10 @@ type Noter interface { // DetectInstalled probes every registered target and returns those whose // agent appears to be installed locally (Status != StatusNotInstalled). -// The runtime agent — the one the user is currently running `dhq` from, -// as reported by harness.Detect — is included even if Status would -// otherwise be StatusInstalled, so the caller can decide whether to -// upgrade silently or skip. +// +// Decisions about how to act on the result — whether to auto-install for +// the runtime agent, prompt for others, or skip ScopeProject — are the +// caller's responsibility (see internal/commands/hello_skills.go). func DetectInstalled() []DetectResult { var out []DetectResult for _, t := range All() { @@ -146,8 +146,11 @@ func DetectInstalled() []DetectResult { return out } -// Find returns the Target with the given name, or nil if no such target is -// registered. Matching is case-insensitive on the canonical name. +// Find returns the Target with the given name, or nil if no such target +// is registered. Names are exact-match — every registered target's +// canonical name is lowercase (claude-code, cursor, …), so a CLI flag +// like --agent CURSOR will not match. Normalise at the call site if +// case-insensitive lookup is desired. func Find(name string) Target { for _, t := range All() { if t.Name() == name { diff --git a/internal/skillinstaller/opencode.go b/internal/skillinstaller/opencode.go index d33f628..e7e0b2a 100644 --- a/internal/skillinstaller/opencode.go +++ b/internal/skillinstaller/opencode.go @@ -30,8 +30,13 @@ const ( // xdgConfigDir returns $XDG_CONFIG_HOME, or ~/.config as the XDG spec // fallback. Lives at file scope so future XDG-aware targets can reuse it. +// +// The XDG Base Directory spec requires the variable to hold an absolute +// path; if it's set to a relative path, the spec says to ignore it and +// use the default. Honouring that here avoids accidentally writing into +// the dev's cwd when XDG_CONFIG_HOME is misconfigured. func xdgConfigDir() (string, error) { - if x := os.Getenv("XDG_CONFIG_HOME"); x != "" { + if x := os.Getenv("XDG_CONFIG_HOME"); x != "" && filepath.IsAbs(x) { return x, nil } home, err := homeDir() diff --git a/internal/skillinstaller/section_test.go b/internal/skillinstaller/section_test.go index ce55971..d5ab00f 100644 --- a/internal/skillinstaller/section_test.go +++ b/internal/skillinstaller/section_test.go @@ -1,6 +1,9 @@ package skillinstaller -import "testing" +import ( + "strings" + "testing" +) func TestParseSectionVersion(t *testing.T) { cases := []struct { @@ -37,11 +40,11 @@ func TestMergeSection_PreservesPreAndPost(t *testing.T) { got := mergeSection(existing, "\nnew\n") for _, must := range []string{"# top", "# bottom", "v1 -->", "new"} { - if !contains(got, must) { + if !strings.Contains(got, must) { t.Errorf("missing %q in result:\n%s", must, got) } } - if contains(got, "old") || contains(got, "v0 -->") { + if strings.Contains(got, "old") || strings.Contains(got, "v0 -->") { t.Errorf("stale content survived:\n%s", got) } } @@ -55,11 +58,3 @@ func TestMergeSection_Idempotent(t *testing.T) { } } -func contains(s, sub string) bool { - for i := 0; i+len(sub) <= len(s); i++ { - if s[i:i+len(sub)] == sub { - return true - } - } - return false -} From 3e50c7a9d436360d7f538c48b7b084e73c6e11ce Mon Sep 17 00:00:00 2001 From: Facundo Farias Date: Mon, 22 Jun 2026 08:48:40 +0200 Subject: [PATCH 4/6] fix(skills): address CodeRabbit nitpicks on the first fixup - repo_test: cover the getCwd-error path in findRepoRoot so a future refactor that silently turns the error into "no repo found" is caught. - skills.go: when bulk install is invoked in a context where every detected agent is project-scope (e.g. inside a git repo with only Copilot/Cline/etc. installed), the previous "No supported AI agents detected on this machine" message was a lie. Distinguish the case and surface the exact `dhq skills install --agent ` line for each filtered agent. Co-Authored-By: Claude Opus 4.7 (1M context) --- internal/commands/skills.go | 14 ++++++++++++++ internal/skillinstaller/repo_test.go | 18 ++++++++++++++++++ 2 files changed, 32 insertions(+) diff --git a/internal/commands/skills.go b/internal/commands/skills.go index 9656f30..3755938 100644 --- a/internal/commands/skills.go +++ b/internal/commands/skills.go @@ -91,6 +91,20 @@ func newSkillsInstallCmd() *cobra.Command { } if len(targets) == 0 { + if len(skippedProject) > 0 { + // Detected agents exist, they're just project-scope — + // hide them from the bare command to avoid silently + // writing into the user's repo. Tell the user exactly + // which ones can be opted in. + hint := "Available project-scope agents (run with --agent to install):\n" + for _, name := range skippedProject { + hint += fmt.Sprintf(" - dhq skills install --agent %s\n", name) + } + return &output.UserError{ + Message: "No user-scope AI agents available for bulk install", + Hint: hint, + } + } return &output.UserError{ Message: "No supported AI agents detected on this machine", Hint: "Install one (e.g. Claude Code) and re-run, or pass --agent .", diff --git a/internal/skillinstaller/repo_test.go b/internal/skillinstaller/repo_test.go index b616e68..38dd143 100644 --- a/internal/skillinstaller/repo_test.go +++ b/internal/skillinstaller/repo_test.go @@ -30,6 +30,24 @@ func TestFindRepoRoot_OutsideRepo(t *testing.T) { } } +func TestFindRepoRoot_GetCwdError(t *testing.T) { + // If the OS can't tell us the cwd at all (e.g. it was unlinked under + // us, or a permission boundary blocks lookup), we treat it the same + // as "no repo found" rather than panicking or returning a half-true + // path that downstream code would join filenames onto. + orig := getCwd + getCwd = func() (string, error) { return "", os.ErrPermission } + t.Cleanup(func() { getCwd = orig }) + + got, ok := findRepoRoot() + if ok { + t.Errorf("findRepoRoot() ok=true when getCwd errored, want false") + } + if got != "" { + t.Errorf("findRepoRoot() path = %q, want empty string", got) + } +} + // Regression: each project-scope target must write at the repo root, not // the cwd, when invoked from a subdirectory. Bug surfaced by Codex review // on copilot.go; the same pattern existed in cline/kiro/antigravity. From b6fe41cb059f53fb51a79213f83b296787e9c994 Mon Sep 17 00:00:00 2001 From: Facundo Farias Date: Mon, 22 Jun 2026 09:23:24 +0200 Subject: [PATCH 5/6] fix(skills): address CodeRabbit full-review findings MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Five clear wins from the second-round full review on PR #26: - skills.go: command Examples docstring claimed `dhq skills install` installs for "every detected agent" — out of date since the earlier fixup taught the bulk path to skip project-scope targets. Replace with examples that show the user-scope default plus explicit --agent invocations for project-scope agents. - flatten.go: references-tree loop now skips non-*.md entries. The embedded skill is markdown by contract, but a stray editor backup or future binary asset shouldn't get inlined verbatim into a single-file target's output. - cursor.go: write the .mdc description as a YAML-quoted scalar (`description: %q` instead of `%s`). Unquoted scalars containing `:` or `#` would corrupt the frontmatter; %q's Go-quoted form is also valid YAML double-quoted syntax. Test expectation updated to match the new quoted output. - opencode_test: add a regression case for relative XDG_CONFIG_HOME to lock in the spec-mandated fallback to ~/.config and prevent a future refactor from accidentally honouring relative paths and writing into the dev's cwd. - aider_test: the path-mention assertion now compares against the output of quotePathForYAMLAndShell rather than the raw path. Passed on Linux/macOS because the quoting is a no-op without metacharacters, but would have broken on Windows where backslashes in the path get escaped — making the substring search miss. Skipping by design: - Separating quotePathForYAML and quotePathForShell — paths with $/backtick in $HOME aren't realistic and the note is informational. - Replacing `if err != nil → StatusAvailable` with IsNotExist checks across Detect functions — without a StatusError/Unknown enum, the alternatives misreport non-existence harder than the status quo; Install will still surface the real error if Detect misclassifies. Co-Authored-By: Claude Opus 4.7 (1M context) --- internal/commands/skills.go | 11 ++++++++--- internal/skillinstaller/aider_test.go | 11 ++++++++--- internal/skillinstaller/cursor.go | 5 ++++- internal/skillinstaller/cursor_test.go | 6 +++--- internal/skillinstaller/flatten.go | 6 ++++++ internal/skillinstaller/opencode_test.go | 12 ++++++++++++ 6 files changed, 41 insertions(+), 10 deletions(-) diff --git a/internal/commands/skills.go b/internal/commands/skills.go index 3755938..9e51a84 100644 --- a/internal/commands/skills.go +++ b/internal/commands/skills.go @@ -16,9 +16,14 @@ func newSkillsCmd() *cobra.Command { (Claude Code, Cursor, etc.). Run during 'dhq hello' or standalone. Examples: - dhq skills list Show detected agents and skill status - dhq skills install Install for every detected agent - dhq skills install --agent claude-code Install for a single agent`, + dhq skills list Show every supported agent and status + dhq skills install Install for detected user-scope agents + (Claude Code, Cursor, etc.). Skips + project-scope agents to avoid silently + mutating the current repo. + dhq skills install --agent copilot Install for a project-scope agent + (writes into the current repo) + dhq skills install --agent claude-code Install for any specific agent`, } cmd.AddCommand(newSkillsListCmd()) cmd.AddCommand(newSkillsInstallCmd()) diff --git a/internal/skillinstaller/aider_test.go b/internal/skillinstaller/aider_test.go index c9db212..5c78d15 100644 --- a/internal/skillinstaller/aider_test.go +++ b/internal/skillinstaller/aider_test.go @@ -145,9 +145,14 @@ func TestAider_PostInstallNote_MentionsExactPath(t *testing.T) { if note == "" { t.Fatal("PostInstallNote() returned empty string") } - want := filepath.Join(home, aiderSkillDir, aiderSkillFile) - if !strings.Contains(note, want) { - t.Errorf("note doesn't mention exact path %q: %s", want, note) + // Compare against the quoted form the note actually emits. On Linux + // the raw path would also be a substring (quotePathForYAMLAndShell is + // a no-op without metacharacters), but on Windows the path has + // backslashes that get escaped to `\\` — so we'd never find the raw + // path. Always compare against the quoted form to stay portable. + wantQuoted := quotePathForYAMLAndShell(filepath.Join(home, aiderSkillDir, aiderSkillFile)) + if !strings.Contains(note, wantQuoted) { + t.Errorf("note doesn't mention quoted path %s in %s", wantQuoted, note) } if !strings.Contains(note, ".aider.conf.yml") { t.Errorf("note should point at .aider.conf.yml: %s", note) diff --git a/internal/skillinstaller/cursor.go b/internal/skillinstaller/cursor.go index 4c2a322..5aa3701 100644 --- a/internal/skillinstaller/cursor.go +++ b/internal/skillinstaller/cursor.go @@ -121,7 +121,10 @@ func buildCursorMDC(efs fs.FS, root string) ([]byte, error) { var buf strings.Builder buf.WriteString("---\n") - fmt.Fprintf(&buf, "description: %s\n", oneLine(description)) + // %q produces a Go-quoted string with " and \ escaped — also a valid + // YAML double-quoted scalar. Required because the description may + // contain `:` or `#`, which would corrupt an unquoted scalar. + fmt.Fprintf(&buf, "description: %q\n", oneLine(description)) buf.WriteString("alwaysApply: false\n") buf.WriteString("---\n\n") buf.Write(body) diff --git a/internal/skillinstaller/cursor_test.go b/internal/skillinstaller/cursor_test.go index db55207..51ad106 100644 --- a/internal/skillinstaller/cursor_test.go +++ b/internal/skillinstaller/cursor_test.go @@ -57,9 +57,9 @@ func TestCursor_Install_WritesMDC(t *testing.T) { t.Error("mdc missing alwaysApply: false") } // The description from SKILL.md ("Deploy code…") should make it into - // the Cursor frontmatter, collapsed to one line. - if !strings.Contains(body, "description: Deploy code") { - t.Errorf("mdc missing description extracted from SKILL.md\n--- body ---\n%s", body[:min(400, len(body))]) + // the Cursor frontmatter, collapsed to one line and YAML-quoted. + if !strings.Contains(body, `description: "Deploy code`) { + t.Errorf("mdc missing quoted description extracted from SKILL.md\n--- body ---\n%s", body[:min(400, len(body))]) } // SKILL.md body content should appear (post-frontmatter heading). if !strings.Contains(body, "DeployHQ CLI — Agent Skill Guide") { diff --git a/internal/skillinstaller/flatten.go b/internal/skillinstaller/flatten.go index 0bf97fd..2e46cf6 100644 --- a/internal/skillinstaller/flatten.go +++ b/internal/skillinstaller/flatten.go @@ -103,6 +103,12 @@ func flattenSkill(efs fs.FS, root string) (description string, body []byte, err if e.IsDir() { continue } + // References are markdown by contract. Skip anything else so a + // stray editor backup (e.g. .swp) or future binary asset doesn't + // land verbatim in a single-file target's output. + if !strings.HasSuffix(e.Name(), ".md") { + continue + } data, err := fs.ReadFile(efs, path.Join(refsDir, e.Name())) if err != nil { return "", nil, fmt.Errorf("read %s: %w", e.Name(), err) diff --git a/internal/skillinstaller/opencode_test.go b/internal/skillinstaller/opencode_test.go index d0c10c5..7efd991 100644 --- a/internal/skillinstaller/opencode_test.go +++ b/internal/skillinstaller/opencode_test.go @@ -143,4 +143,16 @@ func TestXDGConfigDir(t *testing.T) { if got != "/some/path" { t.Errorf("XDG override = %q, want %q", got, "/some/path") } + + // Per the XDG Base Directory spec, a relative XDG_CONFIG_HOME must + // be ignored — falling back to ~/.config rather than potentially + // writing into the dev's cwd via a misconfigured env. + t.Setenv("XDG_CONFIG_HOME", "relative/path") + got, err = xdgConfigDir() + if err != nil { + t.Fatal(err) + } + if got != filepath.Join(home, ".config") { + t.Errorf("relative XDG should fall back to ~/.config = %q, want %q", got, filepath.Join(home, ".config")) + } } From 8a047c92c90900378f2be6360b9d01fa91e24f2f Mon Sep 17 00:00:00 2001 From: Facundo Farias Date: Fri, 26 Jun 2026 18:40:58 +0200 Subject: [PATCH 6/6] fix(skills): harden writes against symlink redirect + test the hello hook MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Council review (security + tests) on PR #26 surfaced three items worth fixing before merge: 1. Symlink-redirect on writes (MEDIUM, security council). All 12 targets plus writeEmbeddedTree used plain os.WriteFile, which follows symlinks on the final path component. A local actor pre-planting a symlink at any of dhq's predictable install paths (~/.codex/AGENTS.md, .github/copilot-instructions.md, ~/.aider/deployhq-skill.md, …) pointing at a victim file (e.g. ~/.bashrc) would have caused dhq to corrupt that file during install. Self-LPE only — never network reachable — but cheap to close. Add internal/skillinstaller/safewrite.go with safeWriteFile (Lstat- refuse symlinks before WriteFile) and ensureNotSymlinkDir (the same check for refs-tree roots). Route every production write through safeWriteFile, and gate writeEmbeddedTree on ensureNotSymlinkDir before walking. Add safewrite_test.go covering: refuses symlink to victim (asserts victim bytes are not corrupted), writes missing path, overwrites a regular file, refuses symlinked refs root in writeEmbeddedTree. Not race-free against an attacker who swaps in a symlink between the Lstat and the WriteFile. That window is irrelevant under the local-only threat model and could be tightened with O_NOFOLLOW in a future per-OS pass. 2. Zero coverage on offerSkillInstall (P1, test council). The runtime entry point for the entire feature has real branching — runtime auto-install, project-scope filter, NonInteractive suppression, the batched prompt — and none of it was tested. Extract three test seams as package vars (detectInstalledFn, detectRuntimeAgentFn, confirmInstallFn) and add hello_skills_test.go with 8 tests covering: empty detection no-op, runtime auto-install, project- scope skip, non-interactive prompt suppression, prompt-yes, prompt-no, already-installed not re-installed, install-error non-fatal. Production code does not reassign the seams. 3. Mutable-globals + future t.Parallel footgun (all three reviewers flagged this independently). homeDir, getCwd, findAider are all package-level vars overridden by tests. Today's tests are serial so it works; the next contributor to add t.Parallel produces flaky races across the whole package. Add doc comments on each of the three vars explicitly forbidding t.Parallel in this package and pointing at homeDir's note as the rationale. Co-Authored-By: Claude Opus 4.7 (1M context) --- internal/commands/hello_skills.go | 37 +++- internal/commands/hello_skills_test.go | 256 ++++++++++++++++++++++ internal/skillinstaller/aider.go | 5 +- internal/skillinstaller/antigravity.go | 2 +- internal/skillinstaller/claude.go | 20 +- internal/skillinstaller/cline.go | 2 +- internal/skillinstaller/codex.go | 2 +- internal/skillinstaller/continue.go | 2 +- internal/skillinstaller/copilot.go | 2 +- internal/skillinstaller/cursor.go | 4 +- internal/skillinstaller/gemini.go | 2 +- internal/skillinstaller/kiro.go | 2 +- internal/skillinstaller/opencode.go | 2 +- internal/skillinstaller/repo.go | 3 + internal/skillinstaller/safewrite.go | 47 ++++ internal/skillinstaller/safewrite_test.go | 140 ++++++++++++ internal/skillinstaller/windsurf.go | 2 +- 17 files changed, 506 insertions(+), 24 deletions(-) create mode 100644 internal/commands/hello_skills_test.go create mode 100644 internal/skillinstaller/safewrite.go create mode 100644 internal/skillinstaller/safewrite_test.go diff --git a/internal/commands/hello_skills.go b/internal/commands/hello_skills.go index e586981..f9eec3c 100644 --- a/internal/commands/hello_skills.go +++ b/internal/commands/hello_skills.go @@ -10,6 +10,31 @@ import ( "github.com/manifoldco/promptui" ) +// Test seams for offerSkillInstall. These three vars are the only external +// state the function depends on; overriding them lets tests exercise every +// branch (no targets / runtime auto-install / prompt yes / prompt no / +// project-scope skip) without a live TTY or real agent installs. +// +// Production code MUST NOT reassign these — only the test files do. +var ( + detectInstalledFn = skillinstaller.DetectInstalled + detectRuntimeAgentFn = harness.Detect + confirmInstallFn = defaultConfirmInstall +) + +// defaultConfirmInstall runs the Y/n promptui prompt and returns true on +// "yes". Extracted from offerSkillInstall so tests can substitute a +// deterministic answer. +func defaultConfirmInstall(label string) bool { + prompt := promptui.Prompt{ + Label: label, + IsConfirm: true, + Default: "Y", + } + _, err := prompt.Run() + return err == nil +} + // offerSkillInstall is the Wrangler-style post-login hook that detects locally // installed AI agents and offers to install the DeployHQ skill for them. // @@ -24,12 +49,12 @@ import ( // The function is a no-op when nothing is detected, when nothing needs // installing, or when env.NonInteractive is set. func offerSkillInstall(env *output.Envelope) { - detected := skillinstaller.DetectInstalled() + detected := detectInstalledFn() if len(detected) == 0 { return } - runtimeName := harness.Detect().Name + runtimeName := detectRuntimeAgentFn().Name var runtime *skillinstaller.DetectResult var others []skillinstaller.DetectResult @@ -64,13 +89,7 @@ func offerSkillInstall(env *output.Envelope) { } label := fmt.Sprintf("Detected AI agents that could use the DeployHQ skill: %s.\n Install for them now?", strings.Join(names, ", ")) - prompt := promptui.Prompt{ - Label: label, - IsConfirm: true, - Default: "Y", - } - if _, err := prompt.Run(); err != nil { - // User said no, or aborted — fine. + if !confirmInstallFn(label) { env.Status("Skipping. Run `dhq skills install` later if you change your mind.") return } diff --git a/internal/commands/hello_skills_test.go b/internal/commands/hello_skills_test.go new file mode 100644 index 0000000..1664b89 --- /dev/null +++ b/internal/commands/hello_skills_test.go @@ -0,0 +1,256 @@ +package commands + +import ( + "bytes" + "errors" + "strings" + "testing" + + "github.com/deployhq/deployhq-cli/internal/harness" + "github.com/deployhq/deployhq-cli/internal/output" + "github.com/deployhq/deployhq-cli/internal/skillinstaller" +) + +// fakeTarget is a minimal Target the hello-flow tests can fully control. +// It records whether Install was called so each test can assert exactly +// which targets were touched. +type fakeTarget struct { + name string + displayName string + scope skillinstaller.Scope + status skillinstaller.Status + installErr error + installPath string + installed bool +} + +func (f *fakeTarget) Name() string { return f.name } +func (f *fakeTarget) DisplayName() string { return f.displayName } +func (f *fakeTarget) Scope() skillinstaller.Scope { return f.scope } +func (f *fakeTarget) Detect() skillinstaller.Status { return f.status } +func (f *fakeTarget) Install() (string, error) { + f.installed = true + if f.installErr != nil { + return "", f.installErr + } + return f.installPath, nil +} + +// withFakeDeps swaps the three test seams in hello_skills.go for the +// duration of the test. Pass runtimeName="" to simulate "no agent +// detected at runtime". confirmAnswer is only consulted when the prompt +// path is reached. +func withFakeDeps(t *testing.T, detected []skillinstaller.DetectResult, runtimeName string, confirmAnswer bool) { + t.Helper() + origDetect := detectInstalledFn + origRuntime := detectRuntimeAgentFn + origConfirm := confirmInstallFn + + detectInstalledFn = func() []skillinstaller.DetectResult { return detected } + detectRuntimeAgentFn = func() harness.AgentInfo { + return harness.AgentInfo{Detected: runtimeName != "", Name: runtimeName} + } + confirmInstallFn = func(string) bool { return confirmAnswer } + + t.Cleanup(func() { + detectInstalledFn = origDetect + detectRuntimeAgentFn = origRuntime + confirmInstallFn = origConfirm + }) +} + +// newTestEnv returns an Envelope writing to the returned buffer. +// NonInteractive is configurable via the second arg. +func newTestEnv(nonInteractive bool) (*output.Envelope, *bytes.Buffer) { + var buf bytes.Buffer + env := &output.Envelope{ + Stdout: &buf, + Stderr: &buf, + Logger: output.NewLogger(), + NonInteractive: nonInteractive, + } + return env, &buf +} + +func TestOfferSkillInstall_NoDetected_NoOp(t *testing.T) { + withFakeDeps(t, nil, "", true) + env, buf := newTestEnv(false) + + offerSkillInstall(env) + + if buf.Len() != 0 { + t.Errorf("expected no output for empty detection, got: %q", buf.String()) + } +} + +func TestOfferSkillInstall_RuntimeAgent_AutoInstallsWithoutPrompt(t *testing.T) { + runtime := &fakeTarget{name: "claude-code", displayName: "Claude Code", scope: skillinstaller.ScopeUser, status: skillinstaller.StatusAvailable, installPath: "/fake/claude"} + other := &fakeTarget{name: "cursor", displayName: "Cursor", scope: skillinstaller.ScopeUser, status: skillinstaller.StatusAvailable, installPath: "/fake/cursor"} + + confirmCalled := false + withFakeDeps(t, []skillinstaller.DetectResult{ + {Target: runtime, Status: runtime.status}, + {Target: other, Status: other.status}, + }, "claude-code", false) + confirmInstallFn = func(string) bool { + confirmCalled = true + return false + } + env, buf := newTestEnv(false) + + offerSkillInstall(env) + + if !runtime.installed { + t.Error("runtime agent (claude-code) was not auto-installed") + } + if !confirmCalled { + t.Error("expected confirmation prompt for the non-runtime agent") + } + if other.installed { + t.Error("non-runtime agent should not be installed after user declines") + } + // Sanity: status output mentions the runtime agent line. + if !strings.Contains(buf.String(), "Claude Code") { + t.Errorf("expected runtime install status in output: %q", buf.String()) + } +} + +func TestOfferSkillInstall_ProjectScopeTargets_AreSkipped(t *testing.T) { + // Only project-scope targets detected. None should install — those are + // reserved for explicit `dhq skills install --agent `. + copilot := &fakeTarget{name: "copilot", displayName: "GitHub Copilot", scope: skillinstaller.ScopeProject, status: skillinstaller.StatusAvailable} + cline := &fakeTarget{name: "cline", displayName: "Cline", scope: skillinstaller.ScopeProject, status: skillinstaller.StatusAvailable} + + confirmCalled := false + withFakeDeps(t, []skillinstaller.DetectResult{ + {Target: copilot, Status: copilot.status}, + {Target: cline, Status: cline.status}, + }, "", false) + confirmInstallFn = func(string) bool { + confirmCalled = true + return true + } + env, _ := newTestEnv(false) + + offerSkillInstall(env) + + if copilot.installed { + t.Error("Copilot (project-scope) should not be installed from hello hook") + } + if cline.installed { + t.Error("Cline (project-scope) should not be installed from hello hook") + } + if confirmCalled { + t.Error("no prompt should appear when only project-scope targets are detected") + } +} + +func TestOfferSkillInstall_NonInteractive_InstallsRuntime_SkipsPrompt(t *testing.T) { + runtime := &fakeTarget{name: "claude-code", displayName: "Claude Code", scope: skillinstaller.ScopeUser, status: skillinstaller.StatusAvailable, installPath: "/fake/claude"} + other := &fakeTarget{name: "cursor", displayName: "Cursor", scope: skillinstaller.ScopeUser, status: skillinstaller.StatusAvailable, installPath: "/fake/cursor"} + + confirmCalled := false + withFakeDeps(t, []skillinstaller.DetectResult{ + {Target: runtime, Status: runtime.status}, + {Target: other, Status: other.status}, + }, "claude-code", true) + confirmInstallFn = func(string) bool { + confirmCalled = true + return true + } + env, _ := newTestEnv(true) // NonInteractive + + offerSkillInstall(env) + + if !runtime.installed { + t.Error("runtime auto-install should still happen in non-interactive mode") + } + if other.installed { + t.Error("non-runtime install must not happen in non-interactive mode") + } + if confirmCalled { + t.Error("no confirm prompt allowed in non-interactive mode") + } +} + +func TestOfferSkillInstall_PromptYes_InstallsOthers(t *testing.T) { + a := &fakeTarget{name: "cursor", displayName: "Cursor", scope: skillinstaller.ScopeUser, status: skillinstaller.StatusAvailable, installPath: "/fake/cursor"} + b := &fakeTarget{name: "windsurf", displayName: "Windsurf", scope: skillinstaller.ScopeUser, status: skillinstaller.StatusAvailable, installPath: "/fake/windsurf"} + + withFakeDeps(t, []skillinstaller.DetectResult{ + {Target: a, Status: a.status}, + {Target: b, Status: b.status}, + }, "", true) // no runtime, confirm: yes + env, _ := newTestEnv(false) + + offerSkillInstall(env) + + if !a.installed || !b.installed { + t.Errorf("expected both targets installed after user confirmed; a=%v b=%v", a.installed, b.installed) + } +} + +func TestOfferSkillInstall_PromptNo_NoInstalls(t *testing.T) { + a := &fakeTarget{name: "cursor", displayName: "Cursor", scope: skillinstaller.ScopeUser, status: skillinstaller.StatusAvailable, installPath: "/fake/cursor"} + + withFakeDeps(t, []skillinstaller.DetectResult{ + {Target: a, Status: a.status}, + }, "", false) // no runtime, confirm: no + env, buf := newTestEnv(false) + + offerSkillInstall(env) + + if a.installed { + t.Error("target installed despite user declining the prompt") + } + if !strings.Contains(buf.String(), "Skipping") { + t.Errorf("expected a 'Skipping' message after decline: %q", buf.String()) + } +} + +func TestOfferSkillInstall_AlreadyInstalled_NotReinstalled(t *testing.T) { + // StatusInstalled means Needed() returns false; we must not Install + // again on every `dhq hello`. + t1 := &fakeTarget{name: "cursor", displayName: "Cursor", scope: skillinstaller.ScopeUser, status: skillinstaller.StatusInstalled, installPath: "/fake/cursor"} + + confirmCalled := false + withFakeDeps(t, []skillinstaller.DetectResult{ + {Target: t1, Status: t1.status}, + }, "", true) + confirmInstallFn = func(string) bool { + confirmCalled = true + return true + } + env, _ := newTestEnv(false) + + offerSkillInstall(env) + + if t1.installed { + t.Error("already-installed target was re-installed (Needed() should have filtered it)") + } + if confirmCalled { + t.Error("no prompt should appear when the only target is already installed") + } +} + +func TestOfferSkillInstall_InstallError_IsNonFatal(t *testing.T) { + // Install errors are surfaced via env.Warn but must not panic or + // halt processing of the remaining targets. + failing := &fakeTarget{name: "cursor", displayName: "Cursor", scope: skillinstaller.ScopeUser, status: skillinstaller.StatusAvailable, installErr: errors.New("disk full")} + ok := &fakeTarget{name: "windsurf", displayName: "Windsurf", scope: skillinstaller.ScopeUser, status: skillinstaller.StatusAvailable, installPath: "/fake/windsurf"} + + withFakeDeps(t, []skillinstaller.DetectResult{ + {Target: failing, Status: failing.status}, + {Target: ok, Status: ok.status}, + }, "", true) + env, buf := newTestEnv(false) + + offerSkillInstall(env) + + if !ok.installed { + t.Error("second target should have installed even after the first one errored") + } + if !strings.Contains(buf.String(), "Warning") || !strings.Contains(buf.String(), "Cursor") { + t.Errorf("expected a warning about the failing Cursor install: %q", buf.String()) + } +} diff --git a/internal/skillinstaller/aider.go b/internal/skillinstaller/aider.go index 60d31b3..9fd328d 100644 --- a/internal/skillinstaller/aider.go +++ b/internal/skillinstaller/aider.go @@ -47,6 +47,9 @@ const ( // findAider is the binary-on-PATH lookup. Overridable in tests so they // don't depend on whether the dev box actually has aider installed. +// +// Tests using this var must run serially — see the note on homeDir in +// claude.go for why this package forbids t.Parallel(). var findAider = func() bool { _, err := exec.LookPath("aider") return err == nil @@ -108,7 +111,7 @@ func (a aider) Install() (string, error) { } dst := filepath.Join(dir, aiderSkillFile) - if err := os.WriteFile(dst, body, 0o644); err != nil { + if err := safeWriteFile(dst, body, 0o644); err != nil { return "", err } return dst, nil diff --git a/internal/skillinstaller/antigravity.go b/internal/skillinstaller/antigravity.go index 1299df1..e4f2f00 100644 --- a/internal/skillinstaller/antigravity.go +++ b/internal/skillinstaller/antigravity.go @@ -84,7 +84,7 @@ func (a antigravity) Install() (string, error) { section := buildAntigravitySection(antigravityRefsDir) merged := mergeSection(string(existing), section) - if err := os.WriteFile(instrPath, []byte(merged), 0o644); err != nil { + if err := safeWriteFile(instrPath, []byte(merged), 0o644); err != nil { return "", err } return instrPath, nil diff --git a/internal/skillinstaller/claude.go b/internal/skillinstaller/claude.go index efaa5e8..9eccfa4 100644 --- a/internal/skillinstaller/claude.go +++ b/internal/skillinstaller/claude.go @@ -11,7 +11,13 @@ import ( func init() { Register(&claudeCode{}) } -// homeDir is the user home-directory lookup. Overridable in tests. +// homeDir is the user home-directory lookup. Overridable in tests so we +// don't write into the dev's real home. +// +// Tests in this package MUST NOT call t.Parallel(): homeDir (along with +// getCwd in repo.go and findAider in aider.go) is mutable global state. +// Concurrent tests would race on these vars and produce flaky output. The +// install/detect paths are fast enough that serial execution costs nothing. var homeDir = os.UserHomeDir // claudeCode installs the skill into Claude Code's user-level skills directory. @@ -87,7 +93,7 @@ func (c claudeCode) Install() (string, error) { if err := writeEmbeddedTree(skills.FS, "deployhq", dir); err != nil { return "", err } - if err := os.WriteFile(filepath.Join(dir, versionMarker), []byte(skills.Version+"\n"), 0o644); err != nil { + if err := safeWriteFile(filepath.Join(dir, versionMarker), []byte(skills.Version+"\n"), 0o644); err != nil { return "", err } return dir, nil @@ -97,7 +103,15 @@ func (c claudeCode) Install() (string, error) { // creating directories as needed. Existing files are overwritten — this is // the simplest definition of idempotent and matches what users expect from // "re-install" or "upgrade". +// +// Refuses when dst is a symlink: otherwise a planted symlink at the refs +// root would silently redirect every write into a victim directory. +// Per-file writes go through safeWriteFile, which adds the same check on +// the final path. func writeEmbeddedTree(efs fs.FS, srcRoot, dst string) error { + if err := ensureNotSymlinkDir(dst); err != nil { + return err + } return fs.WalkDir(efs, srcRoot, func(p string, d fs.DirEntry, err error) error { if err != nil { return err @@ -116,7 +130,7 @@ func writeEmbeddedTree(efs fs.FS, srcRoot, dst string) error { if err := os.MkdirAll(filepath.Dir(out), 0o755); err != nil { return err } - return os.WriteFile(out, data, 0o644) + return safeWriteFile(out, data, 0o644) }) } diff --git a/internal/skillinstaller/cline.go b/internal/skillinstaller/cline.go index 1b74e3e..95bfbca 100644 --- a/internal/skillinstaller/cline.go +++ b/internal/skillinstaller/cline.go @@ -85,7 +85,7 @@ func (c cline) Install() (string, error) { if err != nil { return "", err } - if err := os.WriteFile(p, body, 0o644); err != nil { + if err := safeWriteFile(p, body, 0o644); err != nil { return "", err } return p, nil diff --git a/internal/skillinstaller/codex.go b/internal/skillinstaller/codex.go index f1c2dc8..a98107a 100644 --- a/internal/skillinstaller/codex.go +++ b/internal/skillinstaller/codex.go @@ -96,7 +96,7 @@ func (c codex) Install() (string, error) { section := buildCodexSection(refsRoot) merged := mergeSection(string(existing), section) - if err := os.WriteFile(agentsPath, []byte(merged), 0o644); err != nil { + if err := safeWriteFile(agentsPath, []byte(merged), 0o644); err != nil { return "", err } return agentsPath, nil diff --git a/internal/skillinstaller/continue.go b/internal/skillinstaller/continue.go index 19eb67d..81107a0 100644 --- a/internal/skillinstaller/continue.go +++ b/internal/skillinstaller/continue.go @@ -88,7 +88,7 @@ func (c continueDev) Install() (string, error) { if err != nil { return "", err } - if err := os.WriteFile(p, body, 0o644); err != nil { + if err := safeWriteFile(p, body, 0o644); err != nil { return "", err } return p, nil diff --git a/internal/skillinstaller/copilot.go b/internal/skillinstaller/copilot.go index 85939d5..1ed247c 100644 --- a/internal/skillinstaller/copilot.go +++ b/internal/skillinstaller/copilot.go @@ -93,7 +93,7 @@ func (c copilot) Install() (string, error) { section := buildCopilotSection(copilotRefsDir) merged := mergeSection(string(existing), section) - if err := os.WriteFile(instrPath, []byte(merged), 0o644); err != nil { + if err := safeWriteFile(instrPath, []byte(merged), 0o644); err != nil { return "", err } return instrPath, nil diff --git a/internal/skillinstaller/cursor.go b/internal/skillinstaller/cursor.go index 5aa3701..daa2ebb 100644 --- a/internal/skillinstaller/cursor.go +++ b/internal/skillinstaller/cursor.go @@ -87,10 +87,10 @@ func (c cursor) Install() (string, error) { } dst := filepath.Join(rules, cursorSkillFile) - if err := os.WriteFile(dst, content, 0o644); err != nil { + if err := safeWriteFile(dst, content, 0o644); err != nil { return "", err } - if err := os.WriteFile(filepath.Join(rules, versionMarker), []byte(skills.Version+"\n"), 0o644); err != nil { + if err := safeWriteFile(filepath.Join(rules, versionMarker), []byte(skills.Version+"\n"), 0o644); err != nil { return "", err } return dst, nil diff --git a/internal/skillinstaller/gemini.go b/internal/skillinstaller/gemini.go index a0b3484..175723d 100644 --- a/internal/skillinstaller/gemini.go +++ b/internal/skillinstaller/gemini.go @@ -87,7 +87,7 @@ func (g gemini) Install() (string, error) { section := buildGeminiSection(refsRoot) merged := mergeSection(string(existing), section) - if err := os.WriteFile(instrPath, []byte(merged), 0o644); err != nil { + if err := safeWriteFile(instrPath, []byte(merged), 0o644); err != nil { return "", err } return instrPath, nil diff --git a/internal/skillinstaller/kiro.go b/internal/skillinstaller/kiro.go index c07eb04..41dc250 100644 --- a/internal/skillinstaller/kiro.go +++ b/internal/skillinstaller/kiro.go @@ -68,7 +68,7 @@ func (k kiro) Install() (string, error) { if err != nil { return "", err } - if err := os.WriteFile(p, body, 0o644); err != nil { + if err := safeWriteFile(p, body, 0o644); err != nil { return "", err } return p, nil diff --git a/internal/skillinstaller/opencode.go b/internal/skillinstaller/opencode.go index e7e0b2a..3f98147 100644 --- a/internal/skillinstaller/opencode.go +++ b/internal/skillinstaller/opencode.go @@ -102,7 +102,7 @@ func (o opencode) Install() (string, error) { section := buildOpenCodeSection(refsRoot) merged := mergeSection(string(existing), section) - if err := os.WriteFile(instrPath, []byte(merged), 0o644); err != nil { + if err := safeWriteFile(instrPath, []byte(merged), 0o644); err != nil { return "", err } return instrPath, nil diff --git a/internal/skillinstaller/repo.go b/internal/skillinstaller/repo.go index 2af878b..f5b7b58 100644 --- a/internal/skillinstaller/repo.go +++ b/internal/skillinstaller/repo.go @@ -8,6 +8,9 @@ import ( // getCwd is the working-directory lookup. Overridable in tests so they // don't depend on the dev box's real cwd. Project-scope targets use this // as the starting point for findRepoRoot. +// +// Tests using this var must run serially — see the note on homeDir in +// claude.go for why this package forbids t.Parallel(). var getCwd = os.Getwd // findRepoRoot walks up from the current working directory looking for a diff --git a/internal/skillinstaller/safewrite.go b/internal/skillinstaller/safewrite.go new file mode 100644 index 0000000..c32cc72 --- /dev/null +++ b/internal/skillinstaller/safewrite.go @@ -0,0 +1,47 @@ +package skillinstaller + +import ( + "fmt" + "os" +) + +// safeWriteFile is os.WriteFile with a symlink-refusal Lstat check on path. +// If path already exists as a symlink, the call returns an error rather than +// following it. +// +// Threat model: a local actor pre-plants a symlink at one of dhq's +// predictable install targets (~/.codex/AGENTS.md, .github/copilot-instructions.md, +// ~/.aider/deployhq-skill.md, …) pointing at a victim file (e.g. ~/.bashrc). +// Without this check, our subsequent os.WriteFile follows the symlink and +// corrupts the victim. This is self-LPE, not a network threat — but cheap +// to close given how predictable our install paths are. +// +// Not race-free against an attacker who swaps in a symlink between the +// Lstat and the WriteFile. That window is irrelevant under the local-only +// threat model. Where the OS supports it, a follow-up could open with +// O_NOFOLLOW for full closure; the Lstat approach is portable and good +// enough for the defense-in-depth goal here. +func safeWriteFile(path string, data []byte, perm os.FileMode) error { + if info, err := os.Lstat(path); err == nil { + if info.Mode()&os.ModeSymlink != 0 { + return fmt.Errorf("refusing to write through symlink: %s", path) + } + } + return os.WriteFile(path, data, perm) +} + +// ensureNotSymlinkDir returns an error if path exists and is a symlink. A +// missing path is fine — the caller is about to create it. Used by +// writeEmbeddedTree to refuse a symlinked destination root before walking +// and writing into it; without this, a planted ~/.codex/deployhq-references +// → /tmp/attacker symlink would redirect every reference file dhq writes. +func ensureNotSymlinkDir(path string) error { + info, err := os.Lstat(path) + if err != nil { + return nil + } + if info.Mode()&os.ModeSymlink != 0 { + return fmt.Errorf("refusing to write into symlinked directory: %s", path) + } + return nil +} diff --git a/internal/skillinstaller/safewrite_test.go b/internal/skillinstaller/safewrite_test.go new file mode 100644 index 0000000..e12105a --- /dev/null +++ b/internal/skillinstaller/safewrite_test.go @@ -0,0 +1,140 @@ +package skillinstaller + +import ( + "os" + "path/filepath" + "strings" + "testing" + + "github.com/deployhq/deployhq-cli/skills" +) + +func TestSafeWriteFile_RefusesSymlinkTarget(t *testing.T) { + dir := t.TempDir() + victim := filepath.Join(dir, "victim.txt") + if err := os.WriteFile(victim, []byte("important user content\n"), 0o644); err != nil { + t.Fatal(err) + } + // Attacker pre-plants `target` as a symlink to victim — the simulated + // "~/.codex/AGENTS.md → ~/.bashrc" pattern. + target := filepath.Join(dir, "target.md") + if err := os.Symlink(victim, target); err != nil { + t.Fatal(err) + } + + err := safeWriteFile(target, []byte("payload\n"), 0o644) + if err == nil { + t.Fatal("safeWriteFile via symlink should have errored") + } + if !strings.Contains(err.Error(), "symlink") { + t.Errorf("error should mention symlink: %v", err) + } + + // Critical assertion: the victim was NOT modified. + got, err := os.ReadFile(victim) + if err != nil { + t.Fatal(err) + } + if string(got) != "important user content\n" { + t.Errorf("victim file was overwritten through symlink: %q", got) + } +} + +func TestSafeWriteFile_WritesToMissingPath(t *testing.T) { + dir := t.TempDir() + target := filepath.Join(dir, "new.md") + + if err := safeWriteFile(target, []byte("hello\n"), 0o644); err != nil { + t.Fatalf("safeWriteFile on missing path = %v, want nil", err) + } + got, err := os.ReadFile(target) + if err != nil { + t.Fatal(err) + } + if string(got) != "hello\n" { + t.Errorf("file content = %q, want %q", got, "hello\n") + } +} + +func TestSafeWriteFile_OverwritesNonSymlinkFile(t *testing.T) { + dir := t.TempDir() + target := filepath.Join(dir, "exists.md") + if err := os.WriteFile(target, []byte("old\n"), 0o644); err != nil { + t.Fatal(err) + } + + if err := safeWriteFile(target, []byte("new\n"), 0o644); err != nil { + t.Fatalf("safeWriteFile overwriting regular file = %v, want nil", err) + } + got, err := os.ReadFile(target) + if err != nil { + t.Fatal(err) + } + if string(got) != "new\n" { + t.Errorf("file content = %q, want %q", got, "new\n") + } +} + +func TestEnsureNotSymlinkDir_RefusesSymlink(t *testing.T) { + dir := t.TempDir() + victim := t.TempDir() + link := filepath.Join(dir, "linked-refs") + if err := os.Symlink(victim, link); err != nil { + t.Fatal(err) + } + + err := ensureNotSymlinkDir(link) + if err == nil { + t.Fatal("ensureNotSymlinkDir on symlink should have errored") + } + if !strings.Contains(err.Error(), "symlink") { + t.Errorf("error should mention symlink: %v", err) + } +} + +func TestEnsureNotSymlinkDir_AllowsMissingPath(t *testing.T) { + dir := t.TempDir() + if err := ensureNotSymlinkDir(filepath.Join(dir, "does-not-exist")); err != nil { + t.Errorf("ensureNotSymlinkDir on missing path = %v, want nil", err) + } +} + +func TestEnsureNotSymlinkDir_AllowsRealDirectory(t *testing.T) { + if err := ensureNotSymlinkDir(t.TempDir()); err != nil { + t.Errorf("ensureNotSymlinkDir on real dir = %v, want nil", err) + } +} + +func TestWriteEmbeddedTree_RefusesSymlinkedRoot(t *testing.T) { + // Regression for the security finding: planted symlink at the refs + // root must not let writeEmbeddedTree silently redirect every file + // write into the victim directory. + dir := t.TempDir() + victim := t.TempDir() + refsRoot := filepath.Join(dir, "deployhq-references") + if err := os.Symlink(victim, refsRoot); err != nil { + t.Fatal(err) + } + + // Use the real embedded skill FS to mirror the production call path. + err := writeEmbeddedTree(skills.FS, "deployhq", refsRoot) + if err == nil { + t.Fatal("writeEmbeddedTree into symlinked root should have errored") + } + if !strings.Contains(err.Error(), "symlink") { + t.Errorf("error should mention symlink: %v", err) + } + + // Victim must be untouched. + entries, err := os.ReadDir(victim) + if err != nil { + t.Fatal(err) + } + if len(entries) != 0 { + names := make([]string, len(entries)) + for i, e := range entries { + names[i] = e.Name() + } + t.Errorf("victim directory has %d entries after refused write: %v", len(entries), names) + } +} diff --git a/internal/skillinstaller/windsurf.go b/internal/skillinstaller/windsurf.go index c7249b9..52d1820 100644 --- a/internal/skillinstaller/windsurf.go +++ b/internal/skillinstaller/windsurf.go @@ -116,7 +116,7 @@ func (w windsurf) Install() (string, error) { section := buildWindsurfSection(refsRoot) merged := mergeSection(string(existing), section) - if err := os.WriteFile(rulesPath, []byte(merged), 0o644); err != nil { + if err := safeWriteFile(rulesPath, []byte(merged), 0o644); err != nil { return "", err } return rulesPath, nil