From 637b4db1dd420aacf42dddab9c54563137fd94c4 Mon Sep 17 00:00:00 2001 From: Shawn Yeager Date: Wed, 26 Aug 2026 11:40:22 -0500 Subject: [PATCH 1/3] Connect Grok Build as a first-class coding agent Register Grok in the agent harness, add hey setup grok, and include Grok in skill install, doctor, setup agents, and --remove. No Grok plugin yet. Grok, like Codex since #385, discovers the shared ~/.agents/skills/hey skill directly (xAI's docs list ~/.agents/skills/ among the user-level skill locations), so hey setup grok installs the shared skill and confirms it is healthy rather than copying it into $GROK_HOME/skills. Detection uses $GROK_HOME or ~/.grok, or a grok binary on PATH, ~/.local/bin, or $GROK_HOME/bin, where Grok Build's installers put it. --- .surface | 1 + AGENTS.md | 6 +- README.md | 1 + docs/agents.md | 15 ++-- docs/cli.md | 2 +- internal/cmd/help_topics.go | 2 +- internal/cmd/setup_agent.go | 50 +++++++++++- internal/cmd/setup_agents.go | 12 +-- internal/cmd/setup_agents_test.go | 81 +++++++++++++++++-- internal/cmd/setup_test.go | 11 +-- internal/cmd/skill_install.go | 2 +- internal/cmd/skill_install_test.go | 2 + internal/cmd/skill_refresh_test.go | 2 + internal/harness/agent.go | 2 +- internal/harness/agent_test.go | 7 +- internal/harness/grok.go | 122 +++++++++++++++++++++++++++++ internal/harness/grok_test.go | 68 ++++++++++++++++ scripts/install.ps1 | 2 +- scripts/install.sh | 4 +- tests/e2e/installer.bats | 3 +- 20 files changed, 358 insertions(+), 37 deletions(-) create mode 100644 internal/harness/grok.go create mode 100644 internal/harness/grok_test.go diff --git a/.surface b/.surface index 4ebe280a..65da2ed1 100644 --- a/.surface +++ b/.surface @@ -334,6 +334,7 @@ hey setup agents hey setup agents --remove hey setup claude hey setup codex +hey setup grok hey setup omarchy hey setup omarchy --no-notify hey setup omarchy --notify diff --git a/AGENTS.md b/AGENTS.md index 73bd9092..c909ea5b 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -71,10 +71,10 @@ and fails loudly (`setup_failed`) on any incomplete outcome. `--remove` writes i tombstone first, disables, and keeps the checkout. Details and the state model are in docs/omarchy.md. -Coding-agent integration lives in `internal/harness` (agent registry, Claude Code / Codex +Coding-agent integration lives in `internal/harness` (agent registry, Claude Code / Codex / Grok detection, plugin and skill health checks) and `internal/cmd/setup_agent*.go` (`hey setup -claude|codex|agents`). Claude Code gets the `hey@37signals` plugin from `basecamp/claude-plugins` -plus a skill link; Codex discovers the shared `~/.agents` skill directly. `HEY_SETUP_AGENT` +claude|codex|grok|agents`). Claude Code gets the `hey@37signals` plugin from `basecamp/claude-plugins` +plus a skill link; Codex and Grok discover the shared `~/.agents` skill directly. `HEY_SETUP_AGENT` selects the target for `hey setup agents`; `hey setup agents --remove` uninstalls the Claude plugin and removes only hey-cli-managed skill files. `hey doctor` reports per-agent diagnostics, and a `PersistentPostRunE` hook (`skill_refresh.go`) re-syncs installed skill copies once per release diff --git a/README.md b/README.md index 0c27d406..00f3cd4e 100644 --- a/README.md +++ b/README.md @@ -89,6 +89,7 @@ The setup wizard connects the agents it finds. To do it yourself: ```bash hey setup claude # skill + the hey@37signals plugin for Claude Code hey setup codex # skill for Codex +hey setup grok # skill for Grok claude mcp add hey -- hey mcp # HEY as MCP tools, on your signed-in account ``` diff --git a/docs/agents.md b/docs/agents.md index 497321ac..8d416fcc 100644 --- a/docs/agents.md +++ b/docs/agents.md @@ -3,7 +3,7 @@ hey-cli is built to be driven by an agent as readily as by a person: every command that returns data answers `--json`, the exit codes are stable (`hey help exit-codes`), and `hey commands --json` describes the whole surface. This page covers the two integrations that ship with it: an -agent skill for Claude Code and Codex, and an MCP server. +agent skill for Claude Code, Codex and Grok, and an MCP server. ## Agent skill and Claude Code plugin @@ -15,6 +15,7 @@ manage the integrations on their own: ```bash hey setup claude # install the skill and the hey@37signals plugin for Claude Code hey setup codex # install the shared skill for Codex +hey setup grok # install the shared skill for Grok hey skill install # install the skill only (~/.agents/skills/hey, linked for detected agents) hey setup agents # non-interactive: skill + a single detected agent (the installer uses this) hey setup agents --remove # remove HEY's managed skills and Claude Code plugin @@ -22,16 +23,16 @@ hey doctor # check skill and plugin health per detected agent ``` `hey setup agents` never prompts and never guesses: with several agents detected it installs -the skill only and lists the `hey setup ` choices. `HEY_SETUP_AGENT=claude|codex|all|none` +the skill only and lists the `hey setup ` choices. `HEY_SETUP_AGENT=claude|codex|grok|all|none` picks explicitly. `HEY_NONINTERACTIVE=1` disables interactive sign-in for harnesses that run hey under a pseudo-terminal. The installed skill is refreshed automatically the first time a new hey release runs. -Codex discovers the shared `~/.agents/skills/hey` skill directly. hey-cli does -not also copy it to `~/.codex/skills/hey`, which would make Codex list the same -skill twice. Setup and version refresh remove that legacy path only when its -ownership marker proves an older hey-cli created it; user-authored files are -left untouched. +Codex and Grok discover the shared `~/.agents/skills/hey` skill directly. hey-cli does +not also copy it into their own skills directories (`~/.codex/skills/hey`, `~/.grok/skills/hey`), +which would make an agent list the same skill twice. Setup and version refresh remove the +legacy Codex copy only when its ownership marker proves an older hey-cli created it; +user-authored files are left untouched. hey only ever writes skill directories it owns: each one it creates carries a `.managed-by-hey-cli` marker, and install, replacement and automatic refresh all refuse a diff --git a/docs/cli.md b/docs/cli.md index 3e66e59a..80a31448 100644 --- a/docs/cli.md +++ b/docs/cli.md @@ -27,7 +27,7 @@ hey help linked-accounts # account selection and precedence ## Setup `hey setup` runs the first-run wizard again at any time: browser sign-in, a check of who -you are signed in as, and connecting the coding agents it detects (Claude Code, Codex). +you are signed in as, and connecting the coding agents it detects (Claude Code, Codex, Grok). `--skip-agents` leaves agent integrations unchanged and `--skip-omarchy` leaves the Omarchy integration unchanged. `--silent-success` keeps any required sign-in visible, shows an installation spinner, and ends a successful run with `SETUP COMPLETE`; failure diff --git a/internal/cmd/help_topics.go b/internal/cmd/help_topics.go index 3f70731a..dcf0f782 100644 --- a/internal/cmd/help_topics.go +++ b/internal/cmd/help_topics.go @@ -66,7 +66,7 @@ INTERACTION & DIAGNOSTICS TUI & SETUP HEY_THEME Load a TUI theme overlay from a TOML file. HEY_CABLE_URL Override the Action Cable websocket URL. - HEY_SETUP_AGENT Select claude, codex, all, or none during agent setup. + HEY_SETUP_AGENT Select claude, codex, grok, all, or none during agent setup. Command-line flags take precedence over environment values.`, }, diff --git a/internal/cmd/setup_agent.go b/internal/cmd/setup_agent.go index 55c46baf..03881aba 100644 --- a/internal/cmd/setup_agent.go +++ b/internal/cmd/setup_agent.go @@ -77,9 +77,16 @@ var agentSetupHandlers = map[string]agentSetupHandler{ Run: runCodexSetup, RunNonInteractive: runCodexSetupNonInteractive, }, + "grok": { + Labels: []string{ + "Install the shared HEY skill for Grok", + }, + Run: runGrokSetup, + RunNonInteractive: runGrokSetupNonInteractive, + }, } -// runAgentCommand is the subprocess seam for agent CLIs (claude, codex) so +// runAgentCommand is the subprocess seam for agent CLIs (claude, codex, grok) so // tests never spawn a real one. Output is captured, not streamed: the wizard // prints its own status lines and surfaces the tool's output only on failure. var runAgentCommand = func(ctx context.Context, name string, args ...string) ([]byte, error) { @@ -434,6 +441,47 @@ func installCodexSkill() (string, error) { return path, nil } +// --- Grok --- + +// runGrokSetup connects Grok to the shared agent skill. +func runGrokSetup(cmd *cobra.Command) error { + w := cmd.OutOrStdout() + path, err := installGrokSkill() + if err != nil { + fmt.Fprintln(w, warning.format("Grok skill install failed: "+err.Error())) + fmt.Fprintln(w, "Then verify with: hey doctor") + return nil //nolint:nilerr // warn and continue; the post-setup snapshot reports the failure + } + fmt.Fprintln(w, statusLine(true, "Grok skill installed ("+path+")")) + return nil +} + +func runGrokSetupNonInteractive(*cobra.Command) error { + _, err := installGrokSkill() + return err +} + +// installGrokSkill is the Grok handler's one step. The caller installs the +// shared baseline first, and Grok reads it from ~/.agents/skills directly, +// so this only confirms the baseline is healthy. Like Claude, it never +// fabricates the agent. +func installGrokSkill() (string, error) { + if !harness.DetectGrok() { + return "", &agentSetupError{ + Summary: "Grok not detected — install Grok, then run: hey setup grok", + Manual: []string{"hey setup grok"}, + } + } + path := harness.AgentSkillPath() + if path == "" { + return "", fmt.Errorf("cannot determine shared Agent Skills directory") + } + if !baselineSkillInstalled() { + return "", fmt.Errorf("shared HEY skill is not installed") + } + return path, nil +} + // --- Shared helpers --- // statusLine renders a ✓/✗ checklist line. diff --git a/internal/cmd/setup_agents.go b/internal/cmd/setup_agents.go index d30a8336..3feeec6e 100644 --- a/internal/cmd/setup_agents.go +++ b/internal/cmd/setup_agents.go @@ -14,7 +14,7 @@ import ( ) // agentSetupEnv selects which coding agents `setup agents` targets. -// Values: claude | codex | all | none. Empty (unset) means auto-detect. +// Values: claude | codex | grok | all | none. Empty (unset) means auto-detect. const agentSetupEnv = "HEY_SETUP_AGENT" // newSetupAgentsCommand builds `hey setup agents`. It always runs @@ -28,7 +28,7 @@ func newSetupAgentsCommand() *cobra.Command { Use: "agents", Short: "Install or remove HEY coding-agent integrations", Long: "Install the baseline HEY agent skill and attempt to connect coding agents.\n\n" + - "Selection is controlled by " + agentSetupEnv + ": claude, codex, all, or none. When\n" + + "Selection is controlled by " + agentSetupEnv + ": claude, codex, grok, all, or none. When\n" + "unset, a single detected agent is connected; when several are detected none is\n" + "guessed — the per-agent `hey setup ` commands are surfaced instead. Use\n" + "--remove to uninstall the HEY integrations and managed skill files.", @@ -36,7 +36,7 @@ func newSetupAgentsCommand() *cobra.Command { // or confusion with `setup `). Reject them rather than silently ignore. Args: cobra.NoArgs, Annotations: map[string]string{ - "agent_notes": "Never prompts. Set " + agentSetupEnv + "=claude|codex|all|none to choose; unset auto-detects a single agent. --remove uninstalls HEY's managed agent integrations.", + "agent_notes": "Never prompts. Set " + agentSetupEnv + "=claude|codex|grok|all|none to choose; unset auto-detects a single agent. --remove uninstalls HEY's managed agent integrations.", }, RunE: func(cmd *cobra.Command, _ []string) error { if remove { @@ -102,13 +102,13 @@ func runNonInteractiveAgentSetup(cmd *cobra.Command) error { targets = harness.AllAgents() case "none": // baseline skill only - case "claude", "codex": + case "claude", "codex", "grok": if a := harness.FindAgent(selector); a != nil { targets = []harness.AgentInfo{*a} } default: selector = "invalid" - warnings = append(warnings, fmt.Sprintf("Unknown %s value %q; installed the baseline skill only (expected claude, codex, all, or none)", agentSetupEnv, selectorRaw)) + warnings = append(warnings, fmt.Sprintf("Unknown %s value %q; installed the baseline skill only (expected claude, codex, grok, all, or none)", agentSetupEnv, selectorRaw)) } // Run handlers in id order so aggregation is deterministic. @@ -243,6 +243,8 @@ func agentBinaryPresent(id string) bool { return harness.FindClaudeBinary() != "" case "codex": return harness.FindCodexBinary() != "" + case "grok": + return harness.FindGrokBinary() != "" default: return true } diff --git a/internal/cmd/setup_agents_test.go b/internal/cmd/setup_agents_test.go index 6e8c15fd..5115c976 100644 --- a/internal/cmd/setup_agents_test.go +++ b/internal/cmd/setup_agents_test.go @@ -101,6 +101,23 @@ func TestSetupAgentsSingleDetectedAgentIsConnected(t *testing.T) { } } +func TestSetupAgentsSingleDetectedGrokIsConnected(t *testing.T) { + data, response := runSetupAgents(t, "", ".grok") + if got := stringList(t, data["attempted_agents"]); len(got) != 1 || got[0] != "grok" { + t.Errorf("attempted = %v", got) + } + if got := stringList(t, data["errors"]); len(got) != 0 { + t.Errorf("errors = %v", got) + } + agents := data["agents"].([]any) + if len(agents) != 1 || agents[0].(map[string]any)["plugin_installed"] != true { + t.Errorf("agents = %v", agents) + } + if response.Summary != "Installed baseline skill; connected Grok" { + t.Errorf("summary = %q", response.Summary) + } +} + func TestSetupAgentsAmbiguousDetectionNeverGuesses(t *testing.T) { data, response := runSetupAgents(t, "", ".claude", ".codex") if data["ambiguous"] != true { @@ -120,24 +137,34 @@ func TestSetupAgentsAmbiguousDetectionNeverGuesses(t *testing.T) { func TestSetupAgentsAllAttemptsEveryAgent(t *testing.T) { data, response := runSetupAgents(t, "all", ".claude", ".codex") - if got := stringList(t, data["attempted_agents"]); len(got) != 2 || got[0] != "claude" || got[1] != "codex" { + if got := stringList(t, data["attempted_agents"]); len(got) != 3 || got[0] != "claude" || got[1] != "codex" || got[2] != "grok" { t.Errorf("attempted = %v", got) } // Claude cannot be connected without its binary: an error, a warning and - // manual remediation, never a silent success. + // manual remediation, never a silent success. Grok is not detected here + // (no ~/.grok), so its handler also fails closed. errs := stringList(t, data["errors"]) - if len(errs) == 0 || !strings.HasPrefix(errs[0], "claude: ") { + if len(errs) < 2 || !strings.HasPrefix(errs[0], "claude: ") { t.Errorf("errors = %v", errs) } + var sawGrok bool + for _, e := range errs { + if strings.HasPrefix(e, "grok: ") { + sawGrok = true + } + } + if !sawGrok { + t.Errorf("errors = %v, want a grok: failure", errs) + } warnings := stringList(t, data["warnings"]) if len(warnings) == 0 || !strings.Contains(warnings[0], "Claude Code binary not found") { t.Errorf("warnings = %v", warnings) } manual := stringList(t, data["manual_commands"]) - if !contains(manual, "claude plugin install hey@37signals") || !contains(manual, "hey setup claude") { + if !contains(manual, "claude plugin install hey@37signals") || !contains(manual, "hey setup claude") || !contains(manual, "hey setup grok") { t.Errorf("manual_commands = %v", manual) } - if response.Summary != "Installed baseline skill; attempted Claude Code and Codex" { + if response.Summary != "Installed baseline skill; attempted Claude Code, Codex, and Grok" { t.Errorf("summary = %q", response.Summary) } } @@ -160,6 +187,14 @@ func TestSetupAgentsExplicitSelectorTargetsThatAgent(t *testing.T) { if got := stringList(t, data["attempted_agents"]); len(got) != 1 || got[0] != "codex" { t.Errorf("attempted = %v", got) } + + data, _ = runSetupAgents(t, "Grok", ".claude", ".grok") + if data["selector"] != "grok" { + t.Errorf("selector = %v", data["selector"]) + } + if got := stringList(t, data["attempted_agents"]); len(got) != 1 || got[0] != "grok" { + t.Errorf("attempted = %v", got) + } } func TestSetupAgentsInvalidSelectorWarns(t *testing.T) { @@ -200,6 +235,21 @@ func TestSetupAgentCommandEnvelope(t *testing.T) { t.Errorf("summary = %q", response.Summary) } + if err := os.MkdirAll(filepath.Join(home, ".grok"), 0o755); err != nil { + t.Fatal(err) + } + _, response, err = runAuthCommand(t, home, server.URL, "", true, "setup", "grok") + if err != nil { + t.Fatalf("setup grok: %v", err) + } + data = response.Data.(map[string]any) + if data["agent_detected"] != true || data["plugin_installed"] != true { + t.Errorf("grok data = %v", data) + } + if response.Summary != "Grok connected" { + t.Errorf("summary = %q", response.Summary) + } + // An explicitly requested integration that is not detected is a failed // command: error envelope, nonzero exit. _, _, err = runAuthCommand(t, home, server.URL, "", true, "setup", "claude") @@ -281,6 +331,24 @@ func TestSetupCodexDoesNotFabricateCodex(t *testing.T) { } } +// `hey setup grok` on a machine without Grok must not create ~/.grok and +// then count its own creation as detection. +func TestSetupGrokDoesNotFabricateGrok(t *testing.T) { + isolateAgents(t) + home := t.TempDir() + server := httptest.NewServer(http.NotFoundHandler()) + defer server.Close() + + _, _, err := runAuthCommand(t, home, server.URL, "", true, "setup", "grok") + var cliErr *apierr.Error + if !errors.As(err, &cliErr) || cliErr.Code != "setup_incomplete" || cliErr.Message != "Grok not detected" { + t.Fatalf("error = %v, want setup_incomplete/Grok not detected", err) + } + if _, err := os.Stat(filepath.Join(home, ".grok")); !os.IsNotExist(err) { + t.Error("~/.grok was fabricated") + } +} + // A styled `hey setup ` that did not connect must say so and exit // nonzero — never "start a new session" over a failed integration. func TestSetupAgentStyledReportsNotConnected(t *testing.T) { @@ -580,7 +648,7 @@ func TestSetupAgentsRemoveDeletesManagedSkillsAndPreservesUserFiles(t *testing.T home := t.TempDir() t.Setenv("HOME", home) t.Setenv("USERPROFILE", home) - for _, dir := range []string{".claude", ".codex"} { + for _, dir := range []string{".claude", ".codex", ".grok"} { if err := os.MkdirAll(filepath.Join(home, dir), 0o755); err != nil { t.Fatal(err) } @@ -631,6 +699,7 @@ func TestSetupAgentsRemovePreservesUnmanagedSkills(t *testing.T) { filepath.Join(home, ".agents", "skills", "hey"), filepath.Join(home, ".claude", "skills", "hey"), filepath.Join(home, ".codex", "skills", "hey"), + filepath.Join(home, ".grok", "skills", "hey"), } for _, path := range paths { if err := os.MkdirAll(path, 0o755); err != nil { diff --git a/internal/cmd/setup_test.go b/internal/cmd/setup_test.go index b904aa61..bd2eeaf2 100644 --- a/internal/cmd/setup_test.go +++ b/internal/cmd/setup_test.go @@ -20,13 +20,14 @@ import ( "github.com/basecamp/hey-cli/internal/output" ) -// isolateAgents makes agent detection deterministic: no claude/codex binary -// on PATH and no ~/.local/bin, so only the ~/.claude and ~/.codex directories -// a test creates count. Agent CLIs are never spawned. +// isolateAgents makes agent detection deterministic: no claude/codex/grok +// binary on PATH and no ~/.local/bin, so only the ~/.claude, ~/.codex and +// ~/.grok directories a test creates count. Agent CLIs are never spawned. func isolateAgents(t *testing.T) { t.Helper() t.Setenv("PATH", t.TempDir()) t.Setenv("CODEX_HOME", "") + t.Setenv("GROK_HOME", "") // The wizard installs shell completions too; without this it would read // the shell of whoever runs the tests. stubCompletionEnv(t, testCompletionEnv(t, "bash")) @@ -88,7 +89,7 @@ func wizardData(t *testing.T, response output.Response) map[string]any { func TestSetupCommandRegistersAgentSubcommands(t *testing.T) { root := newRootCmd() - for _, path := range [][]string{{"setup", "agents"}, {"setup", "claude"}, {"setup", "codex"}} { + for _, path := range [][]string{{"setup", "agents"}, {"setup", "claude"}, {"setup", "codex"}, {"setup", "grok"}} { command, _, err := root.Find(path) if err != nil || command.Name() != path[1] { t.Errorf("%v not registered: %v", path, err) @@ -895,7 +896,7 @@ func TestSetupRejectsListOnlyFormatsBeforeSideEffects(t *testing.T) { isolateAgents(t) server := quietServer(t) for _, flag := range []string{"--ids-only", "--count"} { - for _, args := range [][]string{{"setup"}, {"setup", "agents"}, {"setup", "codex"}} { + for _, args := range [][]string{{"setup"}, {"setup", "agents"}, {"setup", "codex"}, {"setup", "grok"}} { configHome := t.TempDir() _, _, err := runAuthCommand(t, configHome, server.URL, "", false, append(args, flag)...) if err == nil || !strings.Contains(err.Error(), flag+" is not supported") { diff --git a/internal/cmd/skill_install.go b/internal/cmd/skill_install.go index 556c285f..a13670c2 100644 --- a/internal/cmd/skill_install.go +++ b/internal/cmd/skill_install.go @@ -100,7 +100,7 @@ func newSkillInstallCommand() *cobra.Command { return &cobra.Command{ Use: "install", Short: "Install the hey skill globally for your coding agents", - Long: "Copies the embedded SKILL.md to ~/.agents/skills/hey/ and links it into ~/.claude/skills/hey when Claude Code is installed. Codex discovers the shared skill directly.", + Long: "Copies the embedded SKILL.md to ~/.agents/skills/hey/ and links it into ~/.claude/skills/hey when Claude Code is installed. Codex and Grok discover the shared skill directly.", RunE: runSkillInstall, } } diff --git a/internal/cmd/skill_install_test.go b/internal/cmd/skill_install_test.go index d511c18a..d7786f35 100644 --- a/internal/cmd/skill_install_test.go +++ b/internal/cmd/skill_install_test.go @@ -79,6 +79,7 @@ func TestSkillInstallCopyFallbackIsIdempotent(t *testing.T) { t.Setenv("USERPROFILE", home) t.Setenv("PATH", t.TempDir()) t.Setenv("CODEX_HOME", "") + t.Setenv("GROK_HOME", "") if err := os.MkdirAll(filepath.Join(home, ".claude"), 0o755); err != nil { t.Fatal(err) } @@ -156,6 +157,7 @@ func agentHome(t *testing.T, dirs ...string) string { t.Setenv("USERPROFILE", home) t.Setenv("PATH", t.TempDir()) t.Setenv("CODEX_HOME", "") + t.Setenv("GROK_HOME", "") for _, dir := range dirs { if err := os.MkdirAll(filepath.Join(home, dir), 0o755); err != nil { t.Fatal(err) diff --git a/internal/cmd/skill_refresh_test.go b/internal/cmd/skill_refresh_test.go index 8c2688ee..30680746 100644 --- a/internal/cmd/skill_refresh_test.go +++ b/internal/cmd/skill_refresh_test.go @@ -17,6 +17,7 @@ func refreshFixture(t *testing.T) (home string) { t.Setenv("USERPROFILE", home) t.Setenv("XDG_CONFIG_HOME", filepath.Join(home, ".config")) t.Setenv("CODEX_HOME", "") + t.Setenv("GROK_HOME", "") return home } @@ -244,6 +245,7 @@ func TestRefreshSkillsPreservesUnmanagedSkills(t *testing.T) { writeSkillFixture(t, filepath.Join(home, ".agents", "skills", "hey"), custom, false), writeSkillFixture(t, filepath.Join(home, ".claude", "skills", "hey"), custom, false), writeSkillFixture(t, filepath.Join(home, ".codex", "skills", "hey"), custom, false), + writeSkillFixture(t, filepath.Join(home, ".grok", "skills", "hey"), custom, false), } if refreshSkillsIfVersionChanged() { diff --git a/internal/harness/agent.go b/internal/harness/agent.go index f02bc809..553c9da5 100644 --- a/internal/harness/agent.go +++ b/internal/harness/agent.go @@ -7,7 +7,7 @@ import ( // AgentInfo describes a coding agent integration. type AgentInfo struct { - Name string // "Claude Code" + Name string // "Claude Code", "Codex", "Grok" ID string // "claude" Detect func() bool // reports whether the agent is installed Checks func() []*StatusCheck // cheap health checks gating setup wizard behavior diff --git a/internal/harness/agent_test.go b/internal/harness/agent_test.go index aadae6f9..852e5b0f 100644 --- a/internal/harness/agent_test.go +++ b/internal/harness/agent_test.go @@ -3,7 +3,7 @@ package harness import "testing" // withCleanRegistry empties the registry for a test and restores the real -// claude/codex registrations from init() afterwards. +// claude/codex/grok registrations from init() afterwards. func withCleanRegistry(t *testing.T) { t.Helper() registryMu.Lock() @@ -76,11 +76,14 @@ func TestRegisterAgentPanicsOnBadIDs(t *testing.T) { assertPanics("duplicate ID", func() { RegisterAgent(AgentInfo{ID: "dup", Name: "Second"}) }) } -func TestDefaultRegistryHasClaudeAndCodex(t *testing.T) { +func TestDefaultRegistryHasClaudeCodexAndGrok(t *testing.T) { if FindAgent("claude") == nil { t.Error("claude agent not registered") } if FindAgent("codex") == nil { t.Error("codex agent not registered") } + if FindAgent("grok") == nil { + t.Error("grok agent not registered") + } } diff --git a/internal/harness/grok.go b/internal/harness/grok.go new file mode 100644 index 00000000..b80369b1 --- /dev/null +++ b/internal/harness/grok.go @@ -0,0 +1,122 @@ +package harness + +import ( + "context" + "os" + "os/exec" + "path/filepath" + "strings" +) + +func init() { + RegisterAgent(AgentInfo{ + Name: "Grok", + ID: "grok", + Detect: DetectGrok, + // Grok discovers the shared ~/.agents skill directly, so health is + // skill-presence only. When a native plugin lands, this grows the + // plugin/version checks Claude has. + Checks: func() []*StatusCheck { + return []*StatusCheck{CheckGrokSkill()} + }, + Diagnostics: func(_ context.Context) []*StatusCheck { + return []*StatusCheck{CheckGrokSkill()} + }, + }) +} + +// DetectGrok returns true when Grok has a home directory or executable. +func DetectGrok() bool { + if info, err := os.Stat(GrokHome()); err == nil && info.IsDir() { + return true + } + return FindGrokBinary() != "" +} + +// FindGrokBinary returns the Grok executable path, or an empty string. Grok +// Build's installers put it in $GROK_HOME/bin, which may not be on PATH yet. +func FindGrokBinary() string { + if path, err := exec.LookPath("grok"); err == nil { + return path + } + if home := GrokHome(); home != "" { + candidate := filepath.Join(home, "bin", "grok") + if _, err := os.Stat(candidate); err == nil { + return candidate + } + } + home, err := os.UserHomeDir() + if err != nil || home == "" { + return "" + } + candidate := filepath.Join(filepath.Clean(home), ".local", "bin", "grok") + if _, err := os.Stat(candidate); err == nil { + return candidate + } + return "" +} + +// GrokHome returns Grok's home directory: $GROK_HOME or ~/.grok. +func GrokHome() string { + if grokHome := strings.TrimSpace(os.Getenv("GROK_HOME")); grokHome != "" { + return grokHome + } + home, err := os.UserHomeDir() + if err != nil || home == "" { + return "" + } + return filepath.Join(filepath.Clean(home), ".grok") +} + +// CheckGrokSkill checks whether the shared hey skill is installed for Grok. +func CheckGrokSkill() *StatusCheck { + skillPath := AgentSkillPath() + if skillPath == "" { + return &StatusCheck{ + Name: "Grok Skill", + Status: "warn", + Message: "Cannot determine shared Agent Skills directory", + } + } + if _, err := os.Stat(skillPath); err != nil { + if os.IsNotExist(err) { + return &StatusCheck{ + Name: "Grok Skill", + Status: "fail", + Message: "Skill not installed", + Hint: "Run: hey setup grok", + } + } + return &StatusCheck{ + Name: "Grok Skill", + Status: "warn", + Message: "Cannot check Grok skill", + Hint: "Unable to stat " + skillPath, + } + } + // Presence is not health: the file must be a regular file (a symlinked + // SKILL.md points somewhere never inspected)... + if !RegularSkillFile(skillPath) { + return &StatusCheck{ + Name: "Grok Skill", + Status: "fail", + Message: "SKILL.md at " + filepath.Dir(skillPath) + " is not a regular file", + Hint: "Move it aside, then run: hey setup grok", + } + } + // ...written by hey-cli — anything else is somebody's work occupying + // the path, not a connected integration. + if skillDir := filepath.Dir(skillPath); !SkillDirOwned(skillDir) { + return &StatusCheck{ + Name: "Grok Skill", + Status: "fail", + Message: "A skill not written by hey-cli occupies " + skillDir, + Hint: "Move it aside, then run: hey setup grok", + } + } + return &StatusCheck{ + Name: "Grok Skill", + Status: "pass", + Message: "Installed", + } +} diff --git a/internal/harness/grok_test.go b/internal/harness/grok_test.go new file mode 100644 index 00000000..cc760150 --- /dev/null +++ b/internal/harness/grok_test.go @@ -0,0 +1,68 @@ +package harness + +import ( + "os" + "path/filepath" + "testing" +) + +func TestDetectGrokByHomeDirectory(t *testing.T) { + home := tempHome(t) + t.Setenv("PATH", t.TempDir()) + t.Setenv("GROK_HOME", "") + + if DetectGrok() { + t.Error("no ~/.grok and no binary should not detect Grok") + } + if err := os.MkdirAll(filepath.Join(home, ".grok"), 0o755); err != nil { + t.Fatal(err) + } + if !DetectGrok() { + t.Error("~/.grok directory should detect Grok") + } +} + +func TestGrokHomeHonorsEnvOverride(t *testing.T) { + home := tempHome(t) + + t.Setenv("GROK_HOME", "") + if got, want := GrokHome(), filepath.Join(home, ".grok"); got != want { + t.Errorf("GrokHome() = %q, want %q", got, want) + } + + override := t.TempDir() + t.Setenv("GROK_HOME", override) + if got := GrokHome(); got != override { + t.Errorf("GrokHome() = %q, want %q", got, override) + } +} + +func TestCheckGrokSkill(t *testing.T) { + home := tempHome(t) + t.Setenv("GROK_HOME", "") + + check := CheckGrokSkill() + if check.Status != "fail" || check.Hint != "Run: hey setup grok" { + t.Errorf("missing skill: %+v", check) + } + + skillDir := filepath.Join(home, ".agents", "skills", "hey") + if err := os.MkdirAll(skillDir, 0o755); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(skillDir, "SKILL.md"), []byte("# hey"), 0o644); err != nil { + t.Fatal(err) + } + // Present but unmarked is somebody else's skill occupying the path — + // never reported as a connected integration. + if check := CheckGrokSkill(); check.Status != "fail" || check.Hint != "Move it aside, then run: hey setup grok" { + t.Errorf("unmanaged skill: %+v", check) + } + + if err := os.WriteFile(filepath.Join(skillDir, SkillOwnershipMarker), []byte("hey-cli"), 0o644); err != nil { + t.Fatal(err) + } + if check := CheckGrokSkill(); check.Status != "pass" { + t.Errorf("managed skill: %+v", check) + } +} diff --git a/scripts/install.ps1 b/scripts/install.ps1 index 701094ec..3448735d 100644 --- a/scripts/install.ps1 +++ b/scripts/install.ps1 @@ -20,7 +20,7 @@ try { # `hey setup agents` to install the agent skill and connect # coding agents without prompting) # HEY_SETUP_AGENT Which coding agent(s) `setup agents` connects: -# claude | codex | all | none (default: auto-detect) +# claude | codex | grok | all | none (default: auto-detect) # # This file must stay pure ASCII: the release pipeline stages and # Authenticode-signs a CRLF copy of it, and Windows PowerShell 5.1 decodes a diff --git a/scripts/install.sh b/scripts/install.sh index fa7de829..f9f95c23 100755 --- a/scripts/install.sh +++ b/scripts/install.sh @@ -14,7 +14,7 @@ # and connect coding agents without prompting) # HEY_SETUP_AGENT # Which coding agent(s) `setup agents` connects: -# claude | codex | all | none (default: auto-detect a single +# claude | codex | grok | all | none (default: auto-detect a single # agent; several detected connects none and lists them) # # Verification: the SHA-256 checksum is always verified against the release's @@ -499,7 +499,7 @@ binary_supports_setup_agents() { # post_install_setup connects coding agents without prompting, but only when # the installed binary has the ownership-aware `setup agents` (it honors -# HEY_SETUP_AGENT itself: claude|codex|all|none, unset = auto-detect one). +# HEY_SETUP_AGENT itself: claude|codex|grok|all|none, unset = auto-detect one). # The jq selector keeps the machine-readable command's envelope out of the # human-facing installer while preserving its concise outcome. # diff --git a/tests/e2e/installer.bats b/tests/e2e/installer.bats index 1e33656a..c490671a 100644 --- a/tests/e2e/installer.bats +++ b/tests/e2e/installer.bats @@ -130,7 +130,7 @@ run_post_install_setup() { # printed next steps. @test "old binary: nothing is invoked beyond the capability probe" { write_stub old - for selector in "" claude codex all none; do + for selector in "" claude codex grok all none; do : > "$LOG" if [[ -n "$selector" ]]; then run_post_install_setup "export HEY_SETUP_AGENT=$selector" @@ -142,6 +142,7 @@ run_post_install_setup() { [[ "$output" != *"skill install"* ]] [[ "$output" != *"setup claude"* ]] [[ "$output" != *"setup codex"* ]] + [[ "$output" != *"setup grok"* ]] [[ "$output" != *"setup agents"$'\n'* ]] done } From debe4684b5fdaf57b963f7c2b34bc69288f14fa7 Mon Sep 17 00:00:00 2001 From: Shawn Yeager Date: Wed, 26 Aug 2026 20:09:44 -0500 Subject: [PATCH 2/3] Cover DetectGrok when only a grok binary is on PATH --- internal/harness/grok_test.go | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/internal/harness/grok_test.go b/internal/harness/grok_test.go index cc760150..225e2c8c 100644 --- a/internal/harness/grok_test.go +++ b/internal/harness/grok_test.go @@ -22,6 +22,21 @@ func TestDetectGrokByHomeDirectory(t *testing.T) { } } +func TestDetectGrokByBinary(t *testing.T) { + tempHome(t) + t.Setenv("GROK_HOME", "") + bin := t.TempDir() + stub := filepath.Join(bin, "grok") + if err := os.WriteFile(stub, []byte("#!/bin/sh\n"), 0o755); err != nil { + t.Fatal(err) + } + t.Setenv("PATH", bin) + + if !DetectGrok() { + t.Error("grok executable on PATH should detect Grok without a home directory") + } +} + func TestGrokHomeHonorsEnvOverride(t *testing.T) { home := tempHome(t) From 38aed687c85769e169c4d3ccb5150db6047739aa Mon Sep 17 00:00:00 2001 From: Jeremy Daer Date: Thu, 10 Sep 2026 12:35:36 -0700 Subject: [PATCH 3/3] Describe Codex and Grok as rows of one shared-skill agent table MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Grok arrived as a copy of Codex: a second harness file and a second block in every command that touches a shared-skill agent (setup handlers, hey skill install, refresh, --remove, the HEY_SETUP_AGENT values), differing only in name, id, home env var, home directory and binary. Two copies of one shape drift — the binary-only detection test existed for Grok but not for Codex, the ~//bin lookup existed for Grok but not for Codex, and the legacy-copy migration #385 added existed for Codex but not for Grok. Say it once. harness.SkillAgent carries those five fields; Codex and Grok are two values in a table that init() registers, and Detect, Home, FindBinary, LegacySkillPath and CheckSkill are its methods. The cmd package loops over harness.SkillAgents() wherever it used to name each agent, so a third shared-skill agent is a new row and the prose lists in the help topic, installers and docs — nothing else. AgentInfo gains FindBinary so setup's "binary not found" remediation reads the registry rather than a switch on ids. FindBinary looks on PATH, then ~/.local/bin, then the agent's own home's bin for every row: Grok Build's installers write ~/.grok/bin/grok ($GROK_HOME/bin/grok for the npm package), and a uniform rule costs Codex one stat. Legacy migration is uniform too: a hey-cli-marked copy in any row's own skills directory is a duplicate the agent would list twice, and only hey-cli could have written a marked one, so doctor flags it, and setup, hey skill install and refresh remove it once the shared skill is healthy, for Grok as for Codex. The twin tests fold the same way: one table-driven test per behavior in harness and cmd, run once per row, so a behavior one agent has and the other lacks is a failing test rather than a gap. --- AGENTS.md | 8 +- docs/agents.md | 8 +- internal/cmd/local_config_trust.go | 4 +- internal/cmd/setup_agent.go | 148 +++++++----------- internal/cmd/setup_agents.go | 51 ++++--- internal/cmd/setup_agents_remove.go | 8 +- internal/cmd/setup_agents_test.go | 176 +++++++++------------- internal/cmd/setup_test.go | 2 +- internal/cmd/skill_install.go | 37 ++--- internal/cmd/skill_install_test.go | 41 ++--- internal/cmd/skill_refresh.go | 18 ++- internal/cmd/skill_refresh_test.go | 59 ++++---- internal/cmd/testdata/sink_manifest.txt | 1 + internal/harness/agent.go | 6 +- internal/harness/agent_test.go | 16 +- internal/harness/claude.go | 9 +- internal/harness/codex.go | 135 ----------------- internal/harness/codex_test.go | 103 ------------- internal/harness/grok.go | 122 --------------- internal/harness/grok_test.go | 83 ---------- internal/harness/skill_agent.go | 178 ++++++++++++++++++++++ internal/harness/skill_agent_test.go | 192 ++++++++++++++++++++++++ 22 files changed, 651 insertions(+), 754 deletions(-) delete mode 100644 internal/harness/codex.go delete mode 100644 internal/harness/codex_test.go delete mode 100644 internal/harness/grok.go delete mode 100644 internal/harness/grok_test.go create mode 100644 internal/harness/skill_agent.go create mode 100644 internal/harness/skill_agent_test.go diff --git a/AGENTS.md b/AGENTS.md index c909ea5b..9039fcb1 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -74,7 +74,13 @@ docs/omarchy.md. Coding-agent integration lives in `internal/harness` (agent registry, Claude Code / Codex / Grok detection, plugin and skill health checks) and `internal/cmd/setup_agent*.go` (`hey setup claude|codex|grok|agents`). Claude Code gets the `hey@37signals` plugin from `basecamp/claude-plugins` -plus a skill link; Codex and Grok discover the shared `~/.agents` skill directly. `HEY_SETUP_AGENT` +plus a skill link; Codex and Grok discover the shared `~/.agents` skill directly. Those two are +rows of one table, `harness.SkillAgent` (name, id, home env var, home directory, binary), and +everything in `internal/cmd` that touches them — setup handlers, `hey skill install`, refresh, +`--remove`, the `HEY_SETUP_AGENT` values, the migration of a copy an older hey-cli made in the +agent's own skills directory — loops over `harness.SkillAgents()` rather than naming either. A new +shared-skill agent is a new row; the prose lists in `help_topics.go`, the installers and the docs +are the places to update by hand. `HEY_SETUP_AGENT` selects the target for `hey setup agents`; `hey setup agents --remove` uninstalls the Claude plugin and removes only hey-cli-managed skill files. `hey doctor` reports per-agent diagnostics, and a `PersistentPostRunE` hook (`skill_refresh.go`) re-syncs installed skill copies once per release diff --git a/docs/agents.md b/docs/agents.md index 8d416fcc..59923218 100644 --- a/docs/agents.md +++ b/docs/agents.md @@ -29,10 +29,10 @@ run hey under a pseudo-terminal. The installed skill is refreshed automatically time a new hey release runs. Codex and Grok discover the shared `~/.agents/skills/hey` skill directly. hey-cli does -not also copy it into their own skills directories (`~/.codex/skills/hey`, `~/.grok/skills/hey`), -which would make an agent list the same skill twice. Setup and version refresh remove the -legacy Codex copy only when its ownership marker proves an older hey-cli created it; -user-authored files are left untouched. +not also copy it into their own skills directories (`~/.codex/skills/hey`, `~/.grok/skills/hey`): +Codex would list the same skill twice, and either would be a second copy to keep current. +Setup, `hey skill install` and version refresh remove such a copy only when its ownership +marker proves an older hey-cli created it; user-authored files are left untouched. hey only ever writes skill directories it owns: each one it creates carries a `.managed-by-hey-cli` marker, and install, replacement and automatic refresh all refuse a diff --git a/internal/cmd/local_config_trust.go b/internal/cmd/local_config_trust.go index 7ed38245..243b2ae3 100644 --- a/internal/cmd/local_config_trust.go +++ b/internal/cmd/local_config_trust.go @@ -62,7 +62,7 @@ func commandIgnoresLocalConfig(cmd *cobra.Command) bool { return true case "setup": // The wizard itself uses the effective server; its subcommands - // (agents, claude, codex, omarchy) touch only local files and must + // (agents, claude, codex, grok, omarchy) touch only local files and must // work from any directory — the installer pipes curl from wherever // the user happens to be, malformed .hey/config.json included. return len(parts) >= 3 @@ -85,7 +85,7 @@ func commandUsesRuntimeConfig(cmd *cobra.Command) bool { return false case "setup": // `hey setup` itself signs in against the effective server, but its - // subcommands (agents, claude, codex) only touch local agent files. + // subcommands (agents, claude, codex, grok) only touch local agent files. // The installer's non-TTY handoff runs `setup agents` from whatever // directory the user piped curl in — possibly a repository with an // untrusted .hey/config.json — and must not be blocked by it. diff --git a/internal/cmd/setup_agent.go b/internal/cmd/setup_agent.go index 03881aba..d6b31ee6 100644 --- a/internal/cmd/setup_agent.go +++ b/internal/cmd/setup_agent.go @@ -59,34 +59,29 @@ type agentSetupOutcome struct { Issues []agentIssue } -// agentSetupHandlers maps agent ID → setup handler. -var agentSetupHandlers = map[string]agentSetupHandler{ - "claude": { - Labels: []string{ - "Add the " + harness.ClaudeMarketplaceSource + " marketplace to Claude Code", - "Install the " + harness.ClaudeExpectedPluginKey + " plugin for Claude Code", - "Link the skill into ~/.claude/skills/hey", - }, - Run: runClaudeSetup, - RunNonInteractive: runClaudeSetupNonInteractive, - }, - "codex": { - Labels: []string{ - "Install the shared HEY skill for Codex", - }, - Run: runCodexSetup, - RunNonInteractive: runCodexSetupNonInteractive, - }, - "grok": { - Labels: []string{ - "Install the shared HEY skill for Grok", +// agentSetupHandlers maps agent ID → setup handler. Claude Code's is written +// out; every shared-skill agent's comes from the harness table. +var agentSetupHandlers = agentSetupHandlersFor(harness.SkillAgents()) + +func agentSetupHandlersFor(skillAgents []harness.SkillAgent) map[string]agentSetupHandler { + handlers := map[string]agentSetupHandler{ + "claude": { + Labels: []string{ + "Add the " + harness.ClaudeMarketplaceSource + " marketplace to Claude Code", + "Install the " + harness.ClaudeExpectedPluginKey + " plugin for Claude Code", + "Link the skill into ~/.claude/skills/hey", + }, + Run: runClaudeSetup, + RunNonInteractive: runClaudeSetupNonInteractive, }, - Run: runGrokSetup, - RunNonInteractive: runGrokSetupNonInteractive, - }, + } + for _, agent := range skillAgents { + handlers[agent.ID] = skillAgentSetupHandler(agent) + } + return handlers } -// runAgentCommand is the subprocess seam for agent CLIs (claude, codex, grok) so +// runAgentCommand is the subprocess seam for agent CLIs (claude, codex, …) so // tests never spawn a real one. Output is captured, not streamed: the wizard // prints its own status lines and surfaces the tool's output only on failure. var runAgentCommand = func(ctx context.Context, name string, args ...string) ([]byte, error) { @@ -398,34 +393,48 @@ func agentCommandFailure(out []byte, err error) string { return message } -// --- Codex --- +// --- Shared-skill agents (Codex, Grok) --- -// runCodexSetup connects Codex to the shared agent skill. -func runCodexSetup(cmd *cobra.Command) error { - w := cmd.OutOrStdout() - path, err := installCodexSkill() - if err != nil { - fmt.Fprintln(w, warning.format("Codex skill install failed: "+err.Error())) - fmt.Fprintln(w, "Then verify with: hey doctor") - return nil //nolint:nilerr // warn and continue; the post-setup snapshot reports the failure +// skillAgentSetupHandler builds the handler for an agent that reads the +// shared skill directly: hey has no plugin for it, so the one step is +// confirming the shared skill and clearing any copy it once made. +func skillAgentSetupHandler(agent harness.SkillAgent) agentSetupHandler { + return agentSetupHandler{ + Labels: []string{ + "Install the shared HEY skill for " + agent.Name, + }, + // Interactive: print progress, warn and continue. + Run: func(cmd *cobra.Command) error { + w := cmd.OutOrStdout() + path, err := installSkillAgentSkill(agent) + if err != nil { + fmt.Fprintln(w, warning.format(agent.Name+" skill install failed: "+err.Error())) + fmt.Fprintln(w, "Then verify with: hey doctor") + return nil //nolint:nilerr // warn and continue; the post-setup snapshot reports the failure + } + fmt.Fprintln(w, statusLine(true, agent.Name+" skill installed ("+path+")")) + return nil + }, + RunNonInteractive: func(*cobra.Command) error { + _, err := installSkillAgentSkill(agent) + return err + }, } - fmt.Fprintln(w, statusLine(true, "Codex skill installed ("+path+")")) - return nil -} - -func runCodexSetupNonInteractive(*cobra.Command) error { - _, err := installCodexSkill() - return err } -// installCodexSkill is the Codex handler's one step. The caller installs the -// shared baseline first; this removes any older hey-cli-managed Codex copy so -// Codex discovers only one skill. Like Claude, it never fabricates the agent. -func installCodexSkill() (string, error) { - if !harness.DetectCodex() { +// installSkillAgentSkill is a shared-skill agent's one step. The caller +// installs the shared baseline first; this confirms it is healthy and +// removes any older hey-cli-managed copy in the agent's own skills +// directory so the agent discovers only one skill. Like Claude, it never +// fabricates the agent: creating its home on a machine without it would +// make every later detection — and this command's own verdict — report it +// installed. +func installSkillAgentSkill(agent harness.SkillAgent) (string, error) { + if !agent.Detect() { + setup := "hey setup " + agent.ID return "", &agentSetupError{ - Summary: "Codex not detected — install Codex, then run: hey setup codex", - Manual: []string{"hey setup codex"}, + Summary: agent.Name + " not detected — install " + agent.Name + ", then run: " + setup, + Manual: []string{setup}, } } path := harness.AgentSkillPath() @@ -435,53 +444,12 @@ func installCodexSkill() (string, error) { if !baselineSkillInstalled() { return "", fmt.Errorf("shared HEY skill is not installed") } - if _, err := migrateLegacyCodexSkill(); err != nil { + if _, err := migrateLegacySkill(agent); err != nil { return "", err } return path, nil } -// --- Grok --- - -// runGrokSetup connects Grok to the shared agent skill. -func runGrokSetup(cmd *cobra.Command) error { - w := cmd.OutOrStdout() - path, err := installGrokSkill() - if err != nil { - fmt.Fprintln(w, warning.format("Grok skill install failed: "+err.Error())) - fmt.Fprintln(w, "Then verify with: hey doctor") - return nil //nolint:nilerr // warn and continue; the post-setup snapshot reports the failure - } - fmt.Fprintln(w, statusLine(true, "Grok skill installed ("+path+")")) - return nil -} - -func runGrokSetupNonInteractive(*cobra.Command) error { - _, err := installGrokSkill() - return err -} - -// installGrokSkill is the Grok handler's one step. The caller installs the -// shared baseline first, and Grok reads it from ~/.agents/skills directly, -// so this only confirms the baseline is healthy. Like Claude, it never -// fabricates the agent. -func installGrokSkill() (string, error) { - if !harness.DetectGrok() { - return "", &agentSetupError{ - Summary: "Grok not detected — install Grok, then run: hey setup grok", - Manual: []string{"hey setup grok"}, - } - } - path := harness.AgentSkillPath() - if path == "" { - return "", fmt.Errorf("cannot determine shared Agent Skills directory") - } - if !baselineSkillInstalled() { - return "", fmt.Errorf("shared HEY skill is not installed") - } - return path, nil -} - // --- Shared helpers --- // statusLine renders a ✓/✗ checklist line. diff --git a/internal/cmd/setup_agents.go b/internal/cmd/setup_agents.go index 3feeec6e..1ff1a3de 100644 --- a/internal/cmd/setup_agents.go +++ b/internal/cmd/setup_agents.go @@ -14,9 +14,26 @@ import ( ) // agentSetupEnv selects which coding agents `setup agents` targets. -// Values: claude | codex | grok | all | none. Empty (unset) means auto-detect. +// Values: an agent id (claude | codex | grok) | all | none. Empty (unset) +// means auto-detect. const agentSetupEnv = "HEY_SETUP_AGENT" +// agentSelectorValues lists what agentSetupEnv accepts, for help and +// diagnostics: every registered agent id, then all and none. +func agentSelectorValues() []string { + var values []string + for _, agent := range harness.AllAgents() { + values = append(values, agent.ID) + } + return append(values, "all", "none") +} + +// agentSelectorProse renders agentSelectorValues as "claude, codex, grok, all, or none". +func agentSelectorProse() string { + values := agentSelectorValues() + return strings.Join(values[:len(values)-1], ", ") + ", or " + values[len(values)-1] +} + // newSetupAgentsCommand builds `hey setup agents`. It always runs // non-interactively: it installs the baseline skill, connects agents per the // HEY_SETUP_AGENT selector (or auto-detection), and emits a structured @@ -28,7 +45,7 @@ func newSetupAgentsCommand() *cobra.Command { Use: "agents", Short: "Install or remove HEY coding-agent integrations", Long: "Install the baseline HEY agent skill and attempt to connect coding agents.\n\n" + - "Selection is controlled by " + agentSetupEnv + ": claude, codex, grok, all, or none. When\n" + + "Selection is controlled by " + agentSetupEnv + ": " + agentSelectorProse() + ". When\n" + "unset, a single detected agent is connected; when several are detected none is\n" + "guessed — the per-agent `hey setup ` commands are surfaced instead. Use\n" + "--remove to uninstall the HEY integrations and managed skill files.", @@ -36,7 +53,7 @@ func newSetupAgentsCommand() *cobra.Command { // or confusion with `setup `). Reject them rather than silently ignore. Args: cobra.NoArgs, Annotations: map[string]string{ - "agent_notes": "Never prompts. Set " + agentSetupEnv + "=claude|codex|grok|all|none to choose; unset auto-detects a single agent. --remove uninstalls HEY's managed agent integrations.", + "agent_notes": "Never prompts. Set " + agentSetupEnv + "=" + strings.Join(agentSelectorValues(), "|") + " to choose; unset auto-detects a single agent. --remove uninstalls HEY's managed agent integrations.", }, RunE: func(cmd *cobra.Command, _ []string) error { if remove { @@ -102,13 +119,13 @@ func runNonInteractiveAgentSetup(cmd *cobra.Command) error { targets = harness.AllAgents() case "none": // baseline skill only - case "claude", "codex", "grok": + default: if a := harness.FindAgent(selector); a != nil { targets = []harness.AgentInfo{*a} + } else { + selector = "invalid" + warnings = append(warnings, fmt.Sprintf("Unknown %s value %q; installed the baseline skill only (expected %s)", agentSetupEnv, selectorRaw, agentSelectorProse())) } - default: - selector = "invalid" - warnings = append(warnings, fmt.Sprintf("Unknown %s value %q; installed the baseline skill only (expected claude, codex, grok, all, or none)", agentSetupEnv, selectorRaw)) } // Run handlers in id order so aggregation is deterministic. @@ -213,7 +230,7 @@ func runAgentSetupHandler(cmd *cobra.Command, agent harness.AgentInfo) agentSetu id: agent.ID, name: agent.Name, detectedBefore: agent.Detect != nil && agent.Detect(), - binaryAbsent: !agentBinaryPresent(agent.ID), + binaryAbsent: !agentBinaryPresent(agent), } if handler, ok := agentSetupHandlers[agent.ID]; ok && handler.RunNonInteractive != nil { @@ -235,19 +252,11 @@ func runAgentSetupHandler(cmd *cobra.Command, agent harness.AgentInfo) agentSetu return rec } -// agentBinaryPresent reports whether the agent's executable is on disk. -// Unknown agents are assumed present so no bogus remediation is synthesized. -func agentBinaryPresent(id string) bool { - switch id { - case "claude": - return harness.FindClaudeBinary() != "" - case "codex": - return harness.FindCodexBinary() != "" - case "grok": - return harness.FindGrokBinary() != "" - default: - return true - } +// agentBinaryPresent reports whether the agent's executable is on disk. An +// agent with no executable to look for is assumed present so no bogus +// remediation is synthesized. +func agentBinaryPresent(agent harness.AgentInfo) bool { + return agent.FindBinary == nil || agent.FindBinary() != "" } // detectedAgentIDs returns the ids of currently detected agents, sorted. diff --git a/internal/cmd/setup_agents_remove.go b/internal/cmd/setup_agents_remove.go index 36d54d34..06e60937 100644 --- a/internal/cmd/setup_agents_remove.go +++ b/internal/cmd/setup_agents_remove.go @@ -47,11 +47,11 @@ func runRemoveAgentSetup(cmd *cobra.Command) error { } } - if codexSkill := harness.LegacyCodexSkillPath(); codexSkill != "" { - if didRemove, removeErr := removeOwnedSkillFiles(filepath.Dir(codexSkill)); removeErr != nil { - failures = append(failures, "Codex skill: "+removeErr.Error()) + for _, agent := range harness.SkillAgents() { + if didRemove, removeErr := removeLegacySkill(agent); removeErr != nil { + failures = append(failures, agent.Name+" skill: "+removeErr.Error()) } else if didRemove { - removed = append(removed, "Codex skill") + removed = append(removed, agent.Name+" skill") } } diff --git a/internal/cmd/setup_agents_test.go b/internal/cmd/setup_agents_test.go index 5115c976..b57672e1 100644 --- a/internal/cmd/setup_agents_test.go +++ b/internal/cmd/setup_agents_test.go @@ -13,6 +13,7 @@ import ( "time" "github.com/basecamp/hey-cli/internal/apierr" + "github.com/basecamp/hey-cli/internal/harness" "github.com/basecamp/hey-cli/internal/output" ) @@ -84,38 +85,32 @@ func TestSetupAgentsNoAgentsDetectedInstallsSkillOnly(t *testing.T) { } } -func TestSetupAgentsSingleDetectedAgentIsConnected(t *testing.T) { - data, response := runSetupAgents(t, "", ".codex") - if got := stringList(t, data["attempted_agents"]); len(got) != 1 || got[0] != "codex" { - t.Errorf("attempted = %v", got) - } - if got := stringList(t, data["errors"]); len(got) != 0 { - t.Errorf("errors = %v", got) - } - agents := data["agents"].([]any) - if len(agents) != 1 || agents[0].(map[string]any)["plugin_installed"] != true { - t.Errorf("agents = %v", agents) - } - if response.Summary != "Installed baseline skill; connected Codex" { - t.Errorf("summary = %q", response.Summary) +// forEachSkillAgent runs a test once per shared-skill agent (Codex, Grok): +// their setup is one code path, so their coverage is one test. +func forEachSkillAgent(t *testing.T, test func(t *testing.T, agent harness.SkillAgent)) { + t.Helper() + for _, agent := range harness.SkillAgents() { + t.Run(agent.ID, func(t *testing.T) { test(t, agent) }) } } -func TestSetupAgentsSingleDetectedGrokIsConnected(t *testing.T) { - data, response := runSetupAgents(t, "", ".grok") - if got := stringList(t, data["attempted_agents"]); len(got) != 1 || got[0] != "grok" { - t.Errorf("attempted = %v", got) - } - if got := stringList(t, data["errors"]); len(got) != 0 { - t.Errorf("errors = %v", got) - } - agents := data["agents"].([]any) - if len(agents) != 1 || agents[0].(map[string]any)["plugin_installed"] != true { - t.Errorf("agents = %v", agents) - } - if response.Summary != "Installed baseline skill; connected Grok" { - t.Errorf("summary = %q", response.Summary) - } +func TestSetupAgentsSingleDetectedAgentIsConnected(t *testing.T) { + forEachSkillAgent(t, func(t *testing.T, agent harness.SkillAgent) { + data, response := runSetupAgents(t, "", agent.HomeDir) + if got := stringList(t, data["attempted_agents"]); len(got) != 1 || got[0] != agent.ID { + t.Errorf("attempted = %v", got) + } + if got := stringList(t, data["errors"]); len(got) != 0 { + t.Errorf("errors = %v", got) + } + agents := data["agents"].([]any) + if len(agents) != 1 || agents[0].(map[string]any)["plugin_installed"] != true { + t.Errorf("agents = %v", agents) + } + if response.Summary != "Installed baseline skill; connected "+agent.Name { + t.Errorf("summary = %q", response.Summary) + } + }) } func TestSetupAgentsAmbiguousDetectionNeverGuesses(t *testing.T) { @@ -215,48 +210,35 @@ func TestSetupAgentsInvalidSelectorWarns(t *testing.T) { } func TestSetupAgentCommandEnvelope(t *testing.T) { - isolateAgents(t) - home := t.TempDir() - if err := os.MkdirAll(filepath.Join(home, ".codex"), 0o755); err != nil { - t.Fatal(err) - } - server := httptest.NewServer(http.NotFoundHandler()) - defer server.Close() - - _, response, err := runAuthCommand(t, home, server.URL, "", true, "setup", "codex") - if err != nil { - t.Fatalf("setup codex: %v", err) - } - data := response.Data.(map[string]any) - if data["agent_detected"] != true || data["plugin_installed"] != true { - t.Errorf("data = %v", data) - } - if response.Summary != "Codex connected" { - t.Errorf("summary = %q", response.Summary) - } + forEachSkillAgent(t, func(t *testing.T, agent harness.SkillAgent) { + isolateAgents(t) + home := t.TempDir() + if err := os.MkdirAll(filepath.Join(home, agent.HomeDir), 0o755); err != nil { + t.Fatal(err) + } + server := httptest.NewServer(http.NotFoundHandler()) + defer server.Close() - if err := os.MkdirAll(filepath.Join(home, ".grok"), 0o755); err != nil { - t.Fatal(err) - } - _, response, err = runAuthCommand(t, home, server.URL, "", true, "setup", "grok") - if err != nil { - t.Fatalf("setup grok: %v", err) - } - data = response.Data.(map[string]any) - if data["agent_detected"] != true || data["plugin_installed"] != true { - t.Errorf("grok data = %v", data) - } - if response.Summary != "Grok connected" { - t.Errorf("summary = %q", response.Summary) - } + _, response, err := runAuthCommand(t, home, server.URL, "", true, "setup", agent.ID) + if err != nil { + t.Fatalf("setup %s: %v", agent.ID, err) + } + data := response.Data.(map[string]any) + if data["agent_detected"] != true || data["plugin_installed"] != true { + t.Errorf("data = %v", data) + } + if response.Summary != agent.Name+" connected" { + t.Errorf("summary = %q", response.Summary) + } - // An explicitly requested integration that is not detected is a failed - // command: error envelope, nonzero exit. - _, _, err = runAuthCommand(t, home, server.URL, "", true, "setup", "claude") - var cliErr *apierr.Error - if !errors.As(err, &cliErr) || cliErr.Code != "setup_incomplete" || cliErr.Message != "Claude Code not detected" { - t.Fatalf("error = %v, want setup_incomplete/Claude Code not detected", err) - } + // An explicitly requested integration that is not detected is a failed + // command: error envelope, nonzero exit. + _, _, err = runAuthCommand(t, home, server.URL, "", true, "setup", "claude") + var cliErr *apierr.Error + if !errors.As(err, &cliErr) || cliErr.Code != "setup_incomplete" || cliErr.Message != "Claude Code not detected" { + t.Fatalf("error = %v, want setup_incomplete/Claude Code not detected", err) + } + }) } func TestJoinNames(t *testing.T) { @@ -313,40 +295,24 @@ func TestSetupAgentsPreservesUnmarkedBaselineSkill(t *testing.T) { } } -// `hey setup codex` on a machine without Codex must not create ~/.codex and -// then count its own creation as detection. -func TestSetupCodexDoesNotFabricateCodex(t *testing.T) { - isolateAgents(t) - home := t.TempDir() - server := httptest.NewServer(http.NotFoundHandler()) - defer server.Close() - - _, _, err := runAuthCommand(t, home, server.URL, "", true, "setup", "codex") - var cliErr *apierr.Error - if !errors.As(err, &cliErr) || cliErr.Code != "setup_incomplete" || cliErr.Message != "Codex not detected" { - t.Fatalf("error = %v, want setup_incomplete/Codex not detected", err) - } - if _, err := os.Stat(filepath.Join(home, ".codex")); !os.IsNotExist(err) { - t.Error("~/.codex was fabricated") - } -} - -// `hey setup grok` on a machine without Grok must not create ~/.grok and -// then count its own creation as detection. -func TestSetupGrokDoesNotFabricateGrok(t *testing.T) { - isolateAgents(t) - home := t.TempDir() - server := httptest.NewServer(http.NotFoundHandler()) - defer server.Close() +// `hey setup ` on a machine without the agent must not create its +// home and then count its own creation as detection. +func TestSetupSkillAgentDoesNotFabricateAgent(t *testing.T) { + forEachSkillAgent(t, func(t *testing.T, agent harness.SkillAgent) { + isolateAgents(t) + home := t.TempDir() + server := httptest.NewServer(http.NotFoundHandler()) + defer server.Close() - _, _, err := runAuthCommand(t, home, server.URL, "", true, "setup", "grok") - var cliErr *apierr.Error - if !errors.As(err, &cliErr) || cliErr.Code != "setup_incomplete" || cliErr.Message != "Grok not detected" { - t.Fatalf("error = %v, want setup_incomplete/Grok not detected", err) - } - if _, err := os.Stat(filepath.Join(home, ".grok")); !os.IsNotExist(err) { - t.Error("~/.grok was fabricated") - } + _, _, err := runAuthCommand(t, home, server.URL, "", true, "setup", agent.ID) + var cliErr *apierr.Error + if !errors.As(err, &cliErr) || cliErr.Code != "setup_incomplete" || cliErr.Message != agent.Name+" not detected" { + t.Fatalf("error = %v, want setup_incomplete/%s not detected", err, agent.Name) + } + if _, err := os.Stat(filepath.Join(home, agent.HomeDir)); !os.IsNotExist(err) { + t.Errorf("~/%s was fabricated", agent.HomeDir) + } + }) } // A styled `hey setup ` that did not connect must say so and exit @@ -659,8 +625,9 @@ func TestSetupAgentsRemoveDeletesManagedSkillsAndPreservesUserFiles(t *testing.T if _, err := linkSkillToClaude(); err != nil { t.Fatal(err) } - legacy := filepath.Join(home, ".codex", "skills", "hey") - writeSkillFixture(t, legacy, "# legacy managed skill", true) + for _, agent := range harness.SkillAgents() { + writeSkillFixture(t, filepath.Join(home, agent.HomeDir, "skills", "hey"), "# legacy managed skill", true) + } baseline := filepath.Join(home, ".agents", "skills", "hey") if err := os.WriteFile(filepath.Join(baseline, "notes.txt"), []byte("keep me"), 0o600); err != nil { t.Fatal(err) @@ -678,6 +645,7 @@ func TestSetupAgentsRemoveDeletesManagedSkillsAndPreservesUserFiles(t *testing.T for _, path := range []string{ filepath.Join(home, ".claude", "skills", "hey"), filepath.Join(home, ".codex", "skills", "hey"), + filepath.Join(home, ".grok", "skills", "hey"), filepath.Join(baseline, skillFilename), filepath.Join(baseline, ownershipMarkerFile), } { diff --git a/internal/cmd/setup_test.go b/internal/cmd/setup_test.go index bd2eeaf2..728157bc 100644 --- a/internal/cmd/setup_test.go +++ b/internal/cmd/setup_test.go @@ -755,7 +755,7 @@ func TestSetupRepeatMigratesManagedLegacyCodexSkill(t *testing.T) { legacy := filepath.Join(home, ".codex", "skills", "hey") writeSkillFixture(t, legacy, "# managed legacy duplicate", true) - if check := harness.CheckCodexSkill(); check.Status != "fail" { + if check := harness.Codex.CheckSkill(); check.Status != "fail" { t.Fatalf("preflight did not notice managed duplicate: %+v", check) } diff --git a/internal/cmd/skill_install.go b/internal/cmd/skill_install.go index a13670c2..d584cb2a 100644 --- a/internal/cmd/skill_install.go +++ b/internal/cmd/skill_install.go @@ -116,12 +116,14 @@ func runSkillInstall(cmd *cobra.Command, args []string) error { // Once the shared replacement is installed, remove a managed legacy copy // before optional agent-specific setup. A later Claude failure must not - // leave Codex discovering both copies. - if removed, cleanupErr := migrateLegacyCodexSkill(); cleanupErr != nil { - return apierr.ErrAPI(0, cleanupErr.Error()) - } else if removed { - result["removed_legacy_codex_skill"] = "true" - lines = append(lines, "Removed the redundant managed Codex skill copy") + // leave an agent discovering both copies. + for _, agent := range harness.SkillAgents() { + if removed, cleanupErr := migrateLegacySkill(agent); cleanupErr != nil { + return apierr.ErrAPI(0, cleanupErr.Error()) + } else if removed { + result["removed_legacy_"+agent.ID+"_skill"] = "true" + lines = append(lines, "Removed the redundant managed "+agent.Name+" skill copy") + } } if harness.DetectClaude() { @@ -290,28 +292,29 @@ func isManagedSkillCopy(path string) bool { return sawMarker } -// removeLegacyCodexSkill removes only the redundant Codex-specific copy -// written by an older hey-cli. An unmarked directory is user-owned and stays -// untouched; Codex will continue to discover it alongside the shared skill. -func removeLegacyCodexSkill() (bool, error) { - skillPath := harness.LegacyCodexSkillPath() +// removeLegacySkill removes only the redundant agent-specific copy written +// by an older hey-cli. An unmarked directory is user-owned and stays +// untouched; the agent will continue to discover it alongside the shared +// skill. +func removeLegacySkill(agent harness.SkillAgent) (bool, error) { + skillPath := agent.LegacySkillPath() if skillPath == "" { return false, nil } return removeOwnedSkillFiles(filepath.Dir(skillPath)) } -// migrateLegacyCodexSkill removes the old Codex-specific copy only after the -// shared skill is known healthy. Until then the legacy copy may be the user's -// only working Codex integration and must remain available. -func migrateLegacyCodexSkill() (bool, error) { +// migrateLegacySkill removes the old agent-specific copy only after the +// shared skill is known healthy. Until then the legacy copy may be the +// user's only working integration and must remain available. +func migrateLegacySkill(agent harness.SkillAgent) (bool, error) { if !baselineSkillInstalled() { return false, nil } - if harness.SameFile(harness.AgentSkillPath(), harness.LegacyCodexSkillPath()) { + if harness.SameFile(harness.AgentSkillPath(), agent.LegacySkillPath()) { return false, nil } - return removeLegacyCodexSkill() + return removeLegacySkill(agent) } // baselineSkillInstalled reports whether ~/.agents/skills/hey/SKILL.md is a diff --git a/internal/cmd/skill_install_test.go b/internal/cmd/skill_install_test.go index d7786f35..7f597efe 100644 --- a/internal/cmd/skill_install_test.go +++ b/internal/cmd/skill_install_test.go @@ -8,6 +8,7 @@ import ( "strings" "testing" + "github.com/basecamp/hey-cli/internal/harness" "github.com/basecamp/hey-cli/internal/output" ) @@ -206,28 +207,32 @@ func TestSkillInstallPreservesUnmanagedLegacyCodexSkill(t *testing.T) { } } -func TestCodexMigrationPreservesLegacySkillWithoutSharedBaseline(t *testing.T) { - home := agentHome(t, ".codex") - legacy := filepath.Join(home, ".codex", "skills", "hey") - writeSkillFixture(t, legacy, "# only working skill", true) +func TestSkillAgentMigrationPreservesLegacySkillWithoutSharedBaseline(t *testing.T) { + forEachSkillAgent(t, func(t *testing.T, agent harness.SkillAgent) { + home := agentHome(t, agent.HomeDir) + legacy := filepath.Join(home, agent.HomeDir, "skills", "hey") + writeSkillFixture(t, legacy, "# only working skill", true) - if _, err := installCodexSkill(); err == nil || !strings.Contains(err.Error(), "shared HEY skill is not installed") { - t.Fatalf("installCodexSkill error = %v", err) - } - if got, err := os.ReadFile(filepath.Join(legacy, skillFilename)); err != nil || string(got) != "# only working skill" { - t.Fatalf("legacy-only skill changed: %q, %v", got, err) - } + if _, err := installSkillAgentSkill(agent); err == nil || !strings.Contains(err.Error(), "shared HEY skill is not installed") { + t.Fatalf("installSkillAgentSkill error = %v", err) + } + if got, err := os.ReadFile(filepath.Join(legacy, skillFilename)); err != nil || string(got) != "# only working skill" { + t.Fatalf("legacy-only skill changed: %q, %v", got, err) + } + }) } -func TestCodexInstallReportsMissingSharedAgentSkillsHome(t *testing.T) { - t.Setenv("HOME", "") - t.Setenv("USERPROFILE", "") - t.Setenv("CODEX_HOME", t.TempDir()) - t.Setenv("PATH", t.TempDir()) +func TestSkillAgentInstallReportsMissingSharedAgentSkillsHome(t *testing.T) { + forEachSkillAgent(t, func(t *testing.T, agent harness.SkillAgent) { + t.Setenv("HOME", "") + t.Setenv("USERPROFILE", "") + t.Setenv(agent.HomeEnv, t.TempDir()) + t.Setenv("PATH", t.TempDir()) - if _, err := installCodexSkill(); err == nil || err.Error() != "cannot determine shared Agent Skills directory" { - t.Fatalf("installCodexSkill error = %v", err) - } + if _, err := installSkillAgentSkill(agent); err == nil || err.Error() != "cannot determine shared Agent Skills directory" { + t.Fatalf("installSkillAgentSkill error = %v", err) + } + }) } func TestSkillInstallFailurePreservesManagedLegacyCodexSkill(t *testing.T) { diff --git a/internal/cmd/skill_refresh.go b/internal/cmd/skill_refresh.go index 75a11296..30ae5412 100644 --- a/internal/cmd/skill_refresh.go +++ b/internal/cmd/skill_refresh.go @@ -48,9 +48,12 @@ func refreshSkillsIfVersionChanged() bool { } sentinelPath := filepath.Join(configDir, ".last-run-version") - // Legacy cleanup still depends on the active Codex home, so switching - // CODEX_HOME gets one migration pass per release. - sentinelState := version.Version + "\n" + harness.CodexHome() + "\n" + // Legacy cleanup still depends on each agent's active home, so switching + // CODEX_HOME or GROK_HOME gets one migration pass per release. + sentinelState := version.Version + "\n" + for _, agent := range harness.SkillAgents() { + sentinelState += agent.Home() + "\n" + } data, err := os.ReadFile(sentinelPath) // #nosec G304 -- fixed path under the user config dir if err == nil && string(data) == sentinelState { return false @@ -128,11 +131,14 @@ func refreshInstalledSkills() (updated, failed int) { } } - // Current Codex reads the shared ~/.agents skill. A copy from an older + // Shared-skill agents read ~/.agents directly. A copy from an older // release would produce a duplicate entry, so migrate it away when its // ownership marker proves hey-cli created it. - if failed == 0 { - if removed, err := migrateLegacyCodexSkill(); err != nil { + for _, agent := range harness.SkillAgents() { + if failed != 0 { + break + } + if removed, err := migrateLegacySkill(agent); err != nil { failed++ } else if removed { updated++ diff --git a/internal/cmd/skill_refresh_test.go b/internal/cmd/skill_refresh_test.go index 30680746..ffd80f92 100644 --- a/internal/cmd/skill_refresh_test.go +++ b/internal/cmd/skill_refresh_test.go @@ -7,6 +7,7 @@ import ( "strings" "testing" + "github.com/basecamp/hey-cli/internal/harness" "github.com/basecamp/hey-cli/skills" ) @@ -327,35 +328,37 @@ func TestRefreshSkillsSkipsWithoutConfigDir(t *testing.T) { } } -// The sentinel tracks the active Codex home: a marked legacy copy in a home -// that was inactive during the first post-upgrade run is removed as soon as -// that home becomes active, not at the next release. -func TestRefreshSkillsRescansWhenCodexHomeChanges(t *testing.T) { - home := refreshFixture(t) - stubVersion(t, "9.9.9") - installStaleSkill(t, home) - - homeA := t.TempDir() - t.Setenv("CODEX_HOME", homeA) - if !refreshSkillsIfVersionChanged() { - t.Fatal("first run should refresh") - } - if refreshSkillsIfVersionChanged() { - t.Fatal("same home: second run is a no-op") - } +// The sentinel tracks each agent's active home: a marked legacy copy in a +// home that was inactive during the first post-upgrade run is removed as +// soon as that home becomes active, not at the next release. +func TestRefreshSkillsRescansWhenAgentHomeChanges(t *testing.T) { + forEachSkillAgent(t, func(t *testing.T, agent harness.SkillAgent) { + home := refreshFixture(t) + stubVersion(t, "9.9.9") + installStaleSkill(t, home) + + homeA := t.TempDir() + t.Setenv(agent.HomeEnv, homeA) + if !refreshSkillsIfVersionChanged() { + t.Fatal("first run should refresh") + } + if refreshSkillsIfVersionChanged() { + t.Fatal("same home: second run is a no-op") + } - homeB := t.TempDir() - legacyB := writeSkillFixture(t, filepath.Join(homeB, "skills", "hey"), "# stale skill", true) - t.Setenv("CODEX_HOME", homeB) - if !refreshSkillsIfVersionChanged() { - t.Fatal("switching Codex homes should rescan") - } - if _, err := os.Stat(legacyB); !os.IsNotExist(err) { - t.Errorf("legacy skill in the newly active Codex home was not removed: %v", err) - } - if refreshSkillsIfVersionChanged() { - t.Error("stable again: refresh must be a no-op") - } + homeB := t.TempDir() + legacyB := writeSkillFixture(t, filepath.Join(homeB, "skills", "hey"), "# stale skill", true) + t.Setenv(agent.HomeEnv, homeB) + if !refreshSkillsIfVersionChanged() { + t.Fatalf("switching %s homes should rescan", agent.Name) + } + if _, err := os.Stat(legacyB); !os.IsNotExist(err) { + t.Errorf("legacy skill in the newly active %s home was not removed: %v", agent.Name, err) + } + if refreshSkillsIfVersionChanged() { + t.Error("stable again: refresh must be a no-op") + } + }) } // The sentinel gets the same no-follow rule as every other file this feature diff --git a/internal/cmd/testdata/sink_manifest.txt b/internal/cmd/testdata/sink_manifest.txt index 844891f3..1adca9af 100644 --- a/internal/cmd/testdata/sink_manifest.txt +++ b/internal/cmd/testdata/sink_manifest.txt @@ -57,6 +57,7 @@ exempt internal/cmd/attachment_upload.go:appendUploadedAttachments HTML-escaped exempt internal/cmd/omarchy.go:run the step names and statuses are this program's own constants exempt internal/cmd/setup_agent.go:newSetupAgentCommands agent names are this program's own constants exempt internal/cmd/setup_agent.go:runSetupAgent agent names are this program's own constants +exempt internal/cmd/setup_agent.go:skillAgentSetupHandler agent names are this program's own constants exempt internal/cmd/setup.go:setupAgents agent names are this program's own constants exempt internal/cmd/setup.go:showWizardSuccess check names and statuses are this program's own constants exempt internal/tui/habit_form.go:iconField the icon names and emoji are this program's own constants, from internal/habit's list of what HEY accepts, never a name HEY served diff --git a/internal/harness/agent.go b/internal/harness/agent.go index 553c9da5..e3a27927 100644 --- a/internal/harness/agent.go +++ b/internal/harness/agent.go @@ -7,11 +7,15 @@ import ( // AgentInfo describes a coding agent integration. type AgentInfo struct { - Name string // "Claude Code", "Codex", "Grok" + Name string // "Claude Code" ID string // "claude" Detect func() bool // reports whether the agent is installed Checks func() []*StatusCheck // cheap health checks gating setup wizard behavior + // FindBinary returns the path to the agent's executable, or "" when it + // is not on disk. Nil means the agent has no executable to look for. + FindBinary func() string + // Diagnostics returns the full doctor check suite, including checks that // are too slow or noisy for the wizard (e.g. version comparisons). // When nil, doctor falls back to Checks. diff --git a/internal/harness/agent_test.go b/internal/harness/agent_test.go index 852e5b0f..b44afb78 100644 --- a/internal/harness/agent_test.go +++ b/internal/harness/agent_test.go @@ -3,7 +3,7 @@ package harness import "testing" // withCleanRegistry empties the registry for a test and restores the real -// claude/codex/grok registrations from init() afterwards. +// registrations from init() afterwards. func withCleanRegistry(t *testing.T) { t.Helper() registryMu.Lock() @@ -76,14 +76,10 @@ func TestRegisterAgentPanicsOnBadIDs(t *testing.T) { assertPanics("duplicate ID", func() { RegisterAgent(AgentInfo{ID: "dup", Name: "Second"}) }) } -func TestDefaultRegistryHasClaudeCodexAndGrok(t *testing.T) { - if FindAgent("claude") == nil { - t.Error("claude agent not registered") - } - if FindAgent("codex") == nil { - t.Error("codex agent not registered") - } - if FindAgent("grok") == nil { - t.Error("grok agent not registered") +func TestDefaultRegistryHasEveryAgent(t *testing.T) { + for _, id := range []string{"claude", "codex", "grok"} { + if FindAgent(id) == nil { + t.Errorf("%s agent not registered", id) + } } } diff --git a/internal/harness/claude.go b/internal/harness/claude.go index 2cfcd1dd..854bec05 100644 --- a/internal/harness/claude.go +++ b/internal/harness/claude.go @@ -24,10 +24,11 @@ const ( func init() { RegisterAgent(AgentInfo{ - Name: "Claude Code", - ID: "claude", - Detect: DetectClaude, - Checks: claudeChecks, + Name: "Claude Code", + ID: "claude", + Detect: DetectClaude, + FindBinary: FindClaudeBinary, + Checks: claudeChecks, Diagnostics: func(_ context.Context) []*StatusCheck { return append(claudeChecks(), CheckClaudePluginVersion()) }, diff --git a/internal/harness/codex.go b/internal/harness/codex.go deleted file mode 100644 index 92d4fc9e..00000000 --- a/internal/harness/codex.go +++ /dev/null @@ -1,135 +0,0 @@ -package harness - -import ( - "context" - "os" - "os/exec" - "path/filepath" - "strings" -) - -func init() { - RegisterAgent(AgentInfo{ - Name: "Codex", - ID: "codex", - Detect: DetectCodex, - // Codex discovers the shared ~/.agents skill directly, so health is - // skill-presence only. When a native plugin lands, this grows the - // plugin/version checks basecamp-cli has. - Checks: func() []*StatusCheck { - return []*StatusCheck{CheckCodexSkill()} - }, - Diagnostics: func(_ context.Context) []*StatusCheck { - return []*StatusCheck{CheckCodexSkill()} - }, - }) -} - -// DetectCodex returns true when Codex has a home directory or executable. -func DetectCodex() bool { - if info, err := os.Stat(CodexHome()); err == nil && info.IsDir() { - return true - } - return FindCodexBinary() != "" -} - -// FindCodexBinary returns the Codex executable path, or an empty string. -func FindCodexBinary() string { - if path, err := exec.LookPath("codex"); err == nil { - return path - } - home, err := os.UserHomeDir() - if err != nil || home == "" { - return "" - } - candidate := filepath.Join(filepath.Clean(home), ".local", "bin", "codex") - if _, err := os.Stat(candidate); err == nil { - return candidate - } - return "" -} - -// CodexHome returns Codex's home directory: $CODEX_HOME or ~/.codex. -func CodexHome() string { - if codexHome := strings.TrimSpace(os.Getenv("CODEX_HOME")); codexHome != "" { - return codexHome - } - home, err := os.UserHomeDir() - if err != nil || home == "" { - return "" - } - return filepath.Join(filepath.Clean(home), ".codex") -} - -// LegacyCodexSkillPath returns the old Codex-specific skill path. Current Codex -// discovers AgentSkillPath directly; this remains only so hey-cli can safely -// migrate and remove copies written by older releases. -func LegacyCodexSkillPath() string { - codexHome := CodexHome() - if codexHome == "" { - return "" - } - return filepath.Join(codexHome, "skills", "hey", "SKILL.md") -} - -// CheckCodexSkill checks whether the shared hey skill is installed for Codex. -func CheckCodexSkill() *StatusCheck { - skillPath := AgentSkillPath() - if skillPath == "" { - return &StatusCheck{ - Name: "Codex Skill", - Status: "warn", - Message: "Cannot determine shared Agent Skills directory", - } - } - if _, err := os.Stat(skillPath); err != nil { - if os.IsNotExist(err) { - return &StatusCheck{ - Name: "Codex Skill", - Status: "fail", - Message: "Skill not installed", - Hint: "Run: hey setup codex", - } - } - return &StatusCheck{ - Name: "Codex Skill", - Status: "warn", - Message: "Cannot check Codex skill", - Hint: "Unable to stat " + skillPath, - } - } - // Presence is not health: the file must be a regular file (a symlinked - // SKILL.md points somewhere never inspected)... - if !RegularSkillFile(skillPath) { - return &StatusCheck{ - Name: "Codex Skill", - Status: "fail", - Message: "SKILL.md at " + filepath.Dir(skillPath) + " is not a regular file", - Hint: "Move it aside, then run: hey setup codex", - } - } - // ...written by hey-cli — anything else is somebody's work occupying - // the path, not a connected integration. - if skillDir := filepath.Dir(skillPath); !SkillDirOwned(skillDir) { - return &StatusCheck{ - Name: "Codex Skill", - Status: "fail", - Message: "A skill not written by hey-cli occupies " + skillDir, - Hint: "Move it aside, then run: hey setup codex", - } - } - legacyPath := LegacyCodexSkillPath() - if !SameFile(skillPath, legacyPath) && RegularSkillFile(legacyPath) && SkillDirOwned(filepath.Dir(legacyPath)) { - return &StatusCheck{ - Name: "Codex Skill", - Status: "fail", - Message: "Redundant managed skill installed at " + filepath.Dir(legacyPath), - Hint: "Run: hey setup codex", - } - } - return &StatusCheck{ - Name: "Codex Skill", - Status: "pass", - Message: "Installed", - } -} diff --git a/internal/harness/codex_test.go b/internal/harness/codex_test.go deleted file mode 100644 index fde3b42a..00000000 --- a/internal/harness/codex_test.go +++ /dev/null @@ -1,103 +0,0 @@ -package harness - -import ( - "os" - "path/filepath" - "testing" -) - -func TestDetectCodexByHomeDirectory(t *testing.T) { - home := tempHome(t) - t.Setenv("PATH", t.TempDir()) - t.Setenv("CODEX_HOME", "") - - if DetectCodex() { - t.Error("no ~/.codex and no binary should not detect Codex") - } - if err := os.MkdirAll(filepath.Join(home, ".codex"), 0o755); err != nil { - t.Fatal(err) - } - if !DetectCodex() { - t.Error("~/.codex directory should detect Codex") - } -} - -func TestCodexHomeHonorsEnvOverride(t *testing.T) { - home := tempHome(t) - - t.Setenv("CODEX_HOME", "") - if got, want := CodexHome(), filepath.Join(home, ".codex"); got != want { - t.Errorf("CodexHome() = %q, want %q", got, want) - } - - override := t.TempDir() - t.Setenv("CODEX_HOME", override) - if got := CodexHome(); got != override { - t.Errorf("CodexHome() = %q, want %q", got, override) - } - if got, want := LegacyCodexSkillPath(), filepath.Join(override, "skills", "hey", "SKILL.md"); got != want { - t.Errorf("LegacyCodexSkillPath() = %q, want %q", got, want) - } -} - -func TestCheckCodexSkill(t *testing.T) { - home := tempHome(t) - t.Setenv("CODEX_HOME", "") - - check := CheckCodexSkill() - if check.Status != "fail" || check.Hint != "Run: hey setup codex" { - t.Errorf("missing skill: %+v", check) - } - - skillDir := filepath.Join(home, ".agents", "skills", "hey") - if err := os.MkdirAll(skillDir, 0o755); err != nil { - t.Fatal(err) - } - if err := os.WriteFile(filepath.Join(skillDir, "SKILL.md"), []byte("# hey"), 0o644); err != nil { - t.Fatal(err) - } - // Present but unmarked is somebody else's skill occupying the path — - // never reported as a connected integration. - if check := CheckCodexSkill(); check.Status != "fail" || check.Hint != "Move it aside, then run: hey setup codex" { - t.Errorf("unmanaged skill: %+v", check) - } - - if err := os.WriteFile(filepath.Join(skillDir, SkillOwnershipMarker), []byte("hey-cli"), 0o644); err != nil { - t.Fatal(err) - } - if check := CheckCodexSkill(); check.Status != "pass" { - t.Errorf("managed skill: %+v", check) - } - - legacyDir := filepath.Join(home, ".codex", "skills", "hey") - if err := os.MkdirAll(legacyDir, 0o755); err != nil { - t.Fatal(err) - } - for name, content := range map[string]string{ - "SKILL.md": "# legacy hey", - SkillOwnershipMarker: "hey-cli", - } { - if err := os.WriteFile(filepath.Join(legacyDir, name), []byte(content), 0o644); err != nil { - t.Fatal(err) - } - } - if check := CheckCodexSkill(); check.Status != "fail" || check.Hint != "Run: hey setup codex" { - t.Errorf("managed duplicate skill: %+v", check) - } - - // A CODEX_HOME that aliases ~/.agents makes the old and current paths - // identical. That is one skill, not a duplicate. - t.Setenv("CODEX_HOME", filepath.Join(home, ".agents")) - if check := CheckCodexSkill(); check.Status != "pass" { - t.Errorf("aliased current skill: %+v", check) - } -} - -func TestCheckCodexSkillReportsMissingAgentSkillsHome(t *testing.T) { - t.Setenv("HOME", "") - t.Setenv("USERPROFILE", "") - check := CheckCodexSkill() - if check.Status != "warn" || check.Message != "Cannot determine shared Agent Skills directory" { - t.Errorf("check = %+v", check) - } -} diff --git a/internal/harness/grok.go b/internal/harness/grok.go deleted file mode 100644 index b80369b1..00000000 --- a/internal/harness/grok.go +++ /dev/null @@ -1,122 +0,0 @@ -package harness - -import ( - "context" - "os" - "os/exec" - "path/filepath" - "strings" -) - -func init() { - RegisterAgent(AgentInfo{ - Name: "Grok", - ID: "grok", - Detect: DetectGrok, - // Grok discovers the shared ~/.agents skill directly, so health is - // skill-presence only. When a native plugin lands, this grows the - // plugin/version checks Claude has. - Checks: func() []*StatusCheck { - return []*StatusCheck{CheckGrokSkill()} - }, - Diagnostics: func(_ context.Context) []*StatusCheck { - return []*StatusCheck{CheckGrokSkill()} - }, - }) -} - -// DetectGrok returns true when Grok has a home directory or executable. -func DetectGrok() bool { - if info, err := os.Stat(GrokHome()); err == nil && info.IsDir() { - return true - } - return FindGrokBinary() != "" -} - -// FindGrokBinary returns the Grok executable path, or an empty string. Grok -// Build's installers put it in $GROK_HOME/bin, which may not be on PATH yet. -func FindGrokBinary() string { - if path, err := exec.LookPath("grok"); err == nil { - return path - } - if home := GrokHome(); home != "" { - candidate := filepath.Join(home, "bin", "grok") - if _, err := os.Stat(candidate); err == nil { - return candidate - } - } - home, err := os.UserHomeDir() - if err != nil || home == "" { - return "" - } - candidate := filepath.Join(filepath.Clean(home), ".local", "bin", "grok") - if _, err := os.Stat(candidate); err == nil { - return candidate - } - return "" -} - -// GrokHome returns Grok's home directory: $GROK_HOME or ~/.grok. -func GrokHome() string { - if grokHome := strings.TrimSpace(os.Getenv("GROK_HOME")); grokHome != "" { - return grokHome - } - home, err := os.UserHomeDir() - if err != nil || home == "" { - return "" - } - return filepath.Join(filepath.Clean(home), ".grok") -} - -// CheckGrokSkill checks whether the shared hey skill is installed for Grok. -func CheckGrokSkill() *StatusCheck { - skillPath := AgentSkillPath() - if skillPath == "" { - return &StatusCheck{ - Name: "Grok Skill", - Status: "warn", - Message: "Cannot determine shared Agent Skills directory", - } - } - if _, err := os.Stat(skillPath); err != nil { - if os.IsNotExist(err) { - return &StatusCheck{ - Name: "Grok Skill", - Status: "fail", - Message: "Skill not installed", - Hint: "Run: hey setup grok", - } - } - return &StatusCheck{ - Name: "Grok Skill", - Status: "warn", - Message: "Cannot check Grok skill", - Hint: "Unable to stat " + skillPath, - } - } - // Presence is not health: the file must be a regular file (a symlinked - // SKILL.md points somewhere never inspected)... - if !RegularSkillFile(skillPath) { - return &StatusCheck{ - Name: "Grok Skill", - Status: "fail", - Message: "SKILL.md at " + filepath.Dir(skillPath) + " is not a regular file", - Hint: "Move it aside, then run: hey setup grok", - } - } - // ...written by hey-cli — anything else is somebody's work occupying - // the path, not a connected integration. - if skillDir := filepath.Dir(skillPath); !SkillDirOwned(skillDir) { - return &StatusCheck{ - Name: "Grok Skill", - Status: "fail", - Message: "A skill not written by hey-cli occupies " + skillDir, - Hint: "Move it aside, then run: hey setup grok", - } - } - return &StatusCheck{ - Name: "Grok Skill", - Status: "pass", - Message: "Installed", - } -} diff --git a/internal/harness/grok_test.go b/internal/harness/grok_test.go deleted file mode 100644 index 225e2c8c..00000000 --- a/internal/harness/grok_test.go +++ /dev/null @@ -1,83 +0,0 @@ -package harness - -import ( - "os" - "path/filepath" - "testing" -) - -func TestDetectGrokByHomeDirectory(t *testing.T) { - home := tempHome(t) - t.Setenv("PATH", t.TempDir()) - t.Setenv("GROK_HOME", "") - - if DetectGrok() { - t.Error("no ~/.grok and no binary should not detect Grok") - } - if err := os.MkdirAll(filepath.Join(home, ".grok"), 0o755); err != nil { - t.Fatal(err) - } - if !DetectGrok() { - t.Error("~/.grok directory should detect Grok") - } -} - -func TestDetectGrokByBinary(t *testing.T) { - tempHome(t) - t.Setenv("GROK_HOME", "") - bin := t.TempDir() - stub := filepath.Join(bin, "grok") - if err := os.WriteFile(stub, []byte("#!/bin/sh\n"), 0o755); err != nil { - t.Fatal(err) - } - t.Setenv("PATH", bin) - - if !DetectGrok() { - t.Error("grok executable on PATH should detect Grok without a home directory") - } -} - -func TestGrokHomeHonorsEnvOverride(t *testing.T) { - home := tempHome(t) - - t.Setenv("GROK_HOME", "") - if got, want := GrokHome(), filepath.Join(home, ".grok"); got != want { - t.Errorf("GrokHome() = %q, want %q", got, want) - } - - override := t.TempDir() - t.Setenv("GROK_HOME", override) - if got := GrokHome(); got != override { - t.Errorf("GrokHome() = %q, want %q", got, override) - } -} - -func TestCheckGrokSkill(t *testing.T) { - home := tempHome(t) - t.Setenv("GROK_HOME", "") - - check := CheckGrokSkill() - if check.Status != "fail" || check.Hint != "Run: hey setup grok" { - t.Errorf("missing skill: %+v", check) - } - - skillDir := filepath.Join(home, ".agents", "skills", "hey") - if err := os.MkdirAll(skillDir, 0o755); err != nil { - t.Fatal(err) - } - if err := os.WriteFile(filepath.Join(skillDir, "SKILL.md"), []byte("# hey"), 0o644); err != nil { - t.Fatal(err) - } - // Present but unmarked is somebody else's skill occupying the path — - // never reported as a connected integration. - if check := CheckGrokSkill(); check.Status != "fail" || check.Hint != "Move it aside, then run: hey setup grok" { - t.Errorf("unmanaged skill: %+v", check) - } - - if err := os.WriteFile(filepath.Join(skillDir, SkillOwnershipMarker), []byte("hey-cli"), 0o644); err != nil { - t.Fatal(err) - } - if check := CheckGrokSkill(); check.Status != "pass" { - t.Errorf("managed skill: %+v", check) - } -} diff --git a/internal/harness/skill_agent.go b/internal/harness/skill_agent.go new file mode 100644 index 00000000..5b2b1d16 --- /dev/null +++ b/internal/harness/skill_agent.go @@ -0,0 +1,178 @@ +package harness + +import ( + "context" + "os" + "os/exec" + "path/filepath" + "strings" +) + +// SkillAgent describes a coding agent that reads the shared ~/.agents skill +// directly: hey has no plugin for it, so its whole integration is the +// baseline skill and health is skill presence only. Codex and Grok are both +// this shape and differ only in the five fields below, which is why they are +// rows in a table rather than two files. When one of them grows a native +// plugin, it leaves the table for a registration of its own, the way Claude +// Code has. +type SkillAgent struct { + Name string // "Codex" + ID string // "codex"; the `hey setup ` subcommand and HEY_SETUP_AGENT value + HomeEnv string // "CODEX_HOME"; overrides HomeDir when set + HomeDir string // ".codex"; under the user's home directory + Binary string // "codex"; the executable's name +} + +// Codex is OpenAI's Codex CLI. +var Codex = SkillAgent{Name: "Codex", ID: "codex", HomeEnv: "CODEX_HOME", HomeDir: ".codex", Binary: "codex"} + +// Grok is xAI's Grok Build CLI. +var Grok = SkillAgent{Name: "Grok", ID: "grok", HomeEnv: "GROK_HOME", HomeDir: ".grok", Binary: "grok"} + +// skillAgents is the registration table: every agent that reads the shared +// skill, in the order they register. +var skillAgents = []SkillAgent{Codex, Grok} + +func init() { + for _, agent := range skillAgents { + RegisterAgent(agent.agentInfo()) + } +} + +// SkillAgents returns every shared-skill agent, in registration order. +func SkillAgents() []SkillAgent { + return append([]SkillAgent(nil), skillAgents...) +} + +func (a SkillAgent) agentInfo() AgentInfo { + checks := func() []*StatusCheck { return []*StatusCheck{a.CheckSkill()} } + return AgentInfo{ + Name: a.Name, + ID: a.ID, + Detect: a.Detect, + FindBinary: a.FindBinary, + Checks: checks, + Diagnostics: func(context.Context) []*StatusCheck { return checks() }, + } +} + +// Detect reports whether the agent has a home directory or an executable. +func (a SkillAgent) Detect() bool { + if info, err := os.Stat(a.Home()); err == nil && info.IsDir() { + return true + } + return a.FindBinary() != "" +} + +// FindBinary returns the agent's executable path, or an empty string. It +// looks on PATH first, then where an installer puts the binary when the +// shell has not picked up the PATH change yet: ~/.local/bin, and the agent's +// own home's bin (Grok Build's installers write ~/.grok/bin/grok, or +// $GROK_HOME/bin/grok for the npm package). +func (a SkillAgent) FindBinary() string { + if path, err := exec.LookPath(a.Binary); err == nil { + return path + } + var candidates []string + if home, err := os.UserHomeDir(); err == nil && home != "" { + candidates = append(candidates, filepath.Join(filepath.Clean(home), ".local", "bin", a.Binary)) + } + if agentHome := a.Home(); agentHome != "" { + candidates = append(candidates, filepath.Join(agentHome, "bin", a.Binary)) + } + for _, candidate := range candidates { + if _, err := os.Stat(candidate); err == nil { + return candidate + } + } + return "" +} + +// Home returns the agent's home directory: $HomeEnv, or HomeDir under the +// user's home. +func (a SkillAgent) Home() string { + if home := strings.TrimSpace(os.Getenv(a.HomeEnv)); home != "" { + return home + } + home, err := os.UserHomeDir() + if err != nil || home == "" { + return "" + } + return filepath.Join(filepath.Clean(home), a.HomeDir) +} + +// LegacySkillPath returns the agent-specific path an older hey-cli copied the +// skill to, or an empty string when the agent's home cannot be determined. +// The agent reads AgentSkillPath directly, so a copy here is a duplicate; +// this remains only so hey-cli can migrate and remove copies it wrote. +func (a SkillAgent) LegacySkillPath() string { + home := a.Home() + if home == "" { + return "" + } + return filepath.Join(home, "skills", "hey", "SKILL.md") +} + +// CheckSkill checks whether the shared hey skill is installed for the agent. +func (a SkillAgent) CheckSkill() *StatusCheck { + name := a.Name + " Skill" + setup := "hey setup " + a.ID + skillPath := AgentSkillPath() + if skillPath == "" { + return &StatusCheck{ + Name: name, + Status: "warn", + Message: "Cannot determine shared Agent Skills directory", + } + } + if _, err := os.Stat(skillPath); err != nil { + if os.IsNotExist(err) { + return &StatusCheck{ + Name: name, + Status: "fail", + Message: "Skill not installed", + Hint: "Run: " + setup, + } + } + return &StatusCheck{ + Name: name, + Status: "warn", + Message: "Cannot check " + a.Name + " skill", + Hint: "Unable to stat " + skillPath, + } + } + // Presence is not health: the file must be a regular file (a symlinked + // SKILL.md points somewhere never inspected)... + if !RegularSkillFile(skillPath) { + return &StatusCheck{ + Name: name, + Status: "fail", + Message: "SKILL.md at " + filepath.Dir(skillPath) + " is not a regular file", + Hint: "Move it aside, then run: " + setup, + } + } + // ...written by hey-cli — anything else is somebody's work occupying + // the path, not a connected integration. + if skillDir := filepath.Dir(skillPath); !SkillDirOwned(skillDir) { + return &StatusCheck{ + Name: name, + Status: "fail", + Message: "A skill not written by hey-cli occupies " + skillDir, + Hint: "Move it aside, then run: " + setup, + } + } + legacyPath := a.LegacySkillPath() + if !SameFile(skillPath, legacyPath) && RegularSkillFile(legacyPath) && SkillDirOwned(filepath.Dir(legacyPath)) { + return &StatusCheck{ + Name: name, + Status: "fail", + Message: "Redundant managed skill installed at " + filepath.Dir(legacyPath), + Hint: "Run: " + setup, + } + } + return &StatusCheck{ + Name: name, + Status: "pass", + Message: "Installed", + } +} diff --git a/internal/harness/skill_agent_test.go b/internal/harness/skill_agent_test.go new file mode 100644 index 00000000..6f6066d2 --- /dev/null +++ b/internal/harness/skill_agent_test.go @@ -0,0 +1,192 @@ +package harness + +import ( + "os" + "path/filepath" + "testing" +) + +// Every shared-skill agent is one row of the same table, so every test here +// runs once per row: a behavior Codex has that Grok lacks is a bug in the +// table, not a difference between them. +func forEachSkillAgent(t *testing.T, test func(t *testing.T, agent SkillAgent)) { + t.Helper() + for _, agent := range SkillAgents() { + t.Run(agent.ID, func(t *testing.T) { test(t, agent) }) + } +} + +func TestSkillAgentsAreRegistered(t *testing.T) { + forEachSkillAgent(t, func(t *testing.T, agent SkillAgent) { + info := FindAgent(agent.ID) + if info == nil { + t.Fatalf("%s agent not registered", agent.ID) + } + if info.Name != agent.Name || info.Detect == nil || info.FindBinary == nil || info.Checks == nil || info.Diagnostics == nil { + t.Errorf("registration incomplete: %+v", info) + } + }) +} + +func TestSkillAgentDetectByHomeDirectory(t *testing.T) { + forEachSkillAgent(t, func(t *testing.T, agent SkillAgent) { + home := tempHome(t) + t.Setenv("PATH", t.TempDir()) + t.Setenv(agent.HomeEnv, "") + + if agent.Detect() { + t.Errorf("no ~/%s and no binary should not detect %s", agent.HomeDir, agent.Name) + } + if err := os.MkdirAll(filepath.Join(home, agent.HomeDir), 0o755); err != nil { + t.Fatal(err) + } + if !agent.Detect() { + t.Errorf("~/%s directory should detect %s", agent.HomeDir, agent.Name) + } + }) +} + +func TestSkillAgentDetectByBinary(t *testing.T) { + forEachSkillAgent(t, func(t *testing.T, agent SkillAgent) { + tempHome(t) + t.Setenv(agent.HomeEnv, "") + bin := t.TempDir() + stub := filepath.Join(bin, agent.Binary) + if err := os.WriteFile(stub, []byte("#!/bin/sh\n"), 0o755); err != nil { + t.Fatal(err) + } + t.Setenv("PATH", bin) + + if !agent.Detect() { + t.Errorf("%s executable on PATH should detect %s without a home directory", agent.Binary, agent.Name) + } + if got := agent.FindBinary(); got != stub { + t.Errorf("FindBinary() = %q, want %q", got, stub) + } + }) +} + +// Off PATH, the binary is found where an installer leaves it: ~/.local/bin, +// or the bin directory of the agent's own home — a relocated one included. +func TestSkillAgentFindBinaryOffPath(t *testing.T) { + forEachSkillAgent(t, func(t *testing.T, agent SkillAgent) { + cases := map[string]func(t *testing.T, home string) string{ + "local bin": func(_ *testing.T, home string) string { return filepath.Join(home, ".local", "bin") }, + "home bin": func(_ *testing.T, home string) string { return filepath.Join(home, agent.HomeDir, "bin") }, + "env home bin": func(t *testing.T, _ string) string { + override := t.TempDir() + t.Setenv(agent.HomeEnv, override) + return filepath.Join(override, "bin") + }, + } + for name, binDir := range cases { + t.Run(name, func(t *testing.T) { + home := tempHome(t) + t.Setenv("PATH", t.TempDir()) + t.Setenv(agent.HomeEnv, "") + if got := agent.FindBinary(); got != "" { + t.Fatalf("FindBinary() = %q before any install, want none", got) + } + stub := filepath.Join(binDir(t, home), agent.Binary) + if err := os.MkdirAll(filepath.Dir(stub), 0o755); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(stub, []byte("#!/bin/sh\n"), 0o755); err != nil { + t.Fatal(err) + } + if got := agent.FindBinary(); got != stub { + t.Errorf("FindBinary() = %q, want %q", got, stub) + } + }) + } + }) +} + +func TestSkillAgentHomeHonorsEnvOverride(t *testing.T) { + forEachSkillAgent(t, func(t *testing.T, agent SkillAgent) { + home := tempHome(t) + + t.Setenv(agent.HomeEnv, "") + if got, want := agent.Home(), filepath.Join(home, agent.HomeDir); got != want { + t.Errorf("Home() = %q, want %q", got, want) + } + + override := t.TempDir() + t.Setenv(agent.HomeEnv, override) + if got := agent.Home(); got != override { + t.Errorf("Home() = %q, want %q", got, override) + } + if got, want := agent.LegacySkillPath(), filepath.Join(override, "skills", "hey", "SKILL.md"); got != want { + t.Errorf("LegacySkillPath() = %q, want %q", got, want) + } + }) +} + +func TestSkillAgentCheckSkill(t *testing.T) { + forEachSkillAgent(t, func(t *testing.T, agent SkillAgent) { + home := tempHome(t) + t.Setenv(agent.HomeEnv, "") + setup := "hey setup " + agent.ID + + check := agent.CheckSkill() + if check.Name != agent.Name+" Skill" || check.Status != "fail" || check.Hint != "Run: "+setup { + t.Errorf("missing skill: %+v", check) + } + + skillDir := filepath.Join(home, ".agents", "skills", "hey") + if err := os.MkdirAll(skillDir, 0o755); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(skillDir, "SKILL.md"), []byte("# hey"), 0o644); err != nil { + t.Fatal(err) + } + // Present but unmarked is somebody else's skill occupying the path — + // never reported as a connected integration. + if check := agent.CheckSkill(); check.Status != "fail" || check.Hint != "Move it aside, then run: "+setup { + t.Errorf("unmanaged skill: %+v", check) + } + + if err := os.WriteFile(filepath.Join(skillDir, SkillOwnershipMarker), []byte("hey-cli"), 0o644); err != nil { + t.Fatal(err) + } + if check := agent.CheckSkill(); check.Status != "pass" { + t.Errorf("managed skill: %+v", check) + } + + // A managed copy in the agent's own skills directory is a duplicate + // the agent would list twice. + legacyDir := filepath.Join(home, agent.HomeDir, "skills", "hey") + if err := os.MkdirAll(legacyDir, 0o755); err != nil { + t.Fatal(err) + } + for name, content := range map[string]string{ + "SKILL.md": "# legacy hey", + SkillOwnershipMarker: "hey-cli", + } { + if err := os.WriteFile(filepath.Join(legacyDir, name), []byte(content), 0o644); err != nil { + t.Fatal(err) + } + } + if check := agent.CheckSkill(); check.Status != "fail" || check.Hint != "Run: "+setup { + t.Errorf("managed duplicate skill: %+v", check) + } + + // A home that aliases ~/.agents makes the old and current paths + // identical. That is one skill, not a duplicate. + t.Setenv(agent.HomeEnv, filepath.Join(home, ".agents")) + if check := agent.CheckSkill(); check.Status != "pass" { + t.Errorf("aliased current skill: %+v", check) + } + }) +} + +func TestSkillAgentCheckSkillReportsMissingAgentSkillsHome(t *testing.T) { + forEachSkillAgent(t, func(t *testing.T, agent SkillAgent) { + t.Setenv("HOME", "") + t.Setenv("USERPROFILE", "") + check := agent.CheckSkill() + if check.Status != "warn" || check.Message != "Cannot determine shared Agent Skills directory" { + t.Errorf("check = %+v", check) + } + }) +}