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..9039fcb1 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -71,10 +71,16 @@ 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. 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/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..59923218 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`): +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/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/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 55c46baf..d6b31ee6 100644 --- a/internal/cmd/setup_agent.go +++ b/internal/cmd/setup_agent.go @@ -59,27 +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", +// 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: runCodexSetup, - RunNonInteractive: runCodexSetupNonInteractive, - }, + } + for _, agent := range skillAgents { + handlers[agent.ID] = skillAgentSetupHandler(agent) + } + return handlers } -// runAgentCommand is the subprocess seam for agent CLIs (claude, codex) 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) { @@ -391,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() @@ -428,7 +444,7 @@ 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 diff --git a/internal/cmd/setup_agents.go b/internal/cmd/setup_agents.go index d30a8336..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 | 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, 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|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": + 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, 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,17 +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() != "" - 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 6e8c15fd..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,23 +85,34 @@ 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 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) { data, response := runSetupAgents(t, "", ".claude", ".codex") if data["ambiguous"] != true { @@ -120,24 +132,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 +182,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) { @@ -180,33 +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() + 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() - _, 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) - } + _, 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) { @@ -263,22 +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 ` 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", 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 @@ -580,7 +614,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) } @@ -591,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) @@ -610,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), } { @@ -631,6 +667,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..728157bc 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) @@ -754,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) } @@ -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..d584cb2a 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, } } @@ -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 d511c18a..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" ) @@ -79,6 +80,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 +158,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) @@ -204,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 8c2688ee..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" ) @@ -17,6 +18,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 +246,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() { @@ -325,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 f02bc809..e3a27927 100644 --- a/internal/harness/agent.go +++ b/internal/harness/agent.go @@ -12,6 +12,10 @@ type AgentInfo struct { 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 aadae6f9..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 registrations from init() afterwards. +// registrations from init() afterwards. func withCleanRegistry(t *testing.T) { t.Helper() registryMu.Lock() @@ -76,11 +76,10 @@ func TestRegisterAgentPanicsOnBadIDs(t *testing.T) { assertPanics("duplicate ID", func() { RegisterAgent(AgentInfo{ID: "dup", Name: "Second"}) }) } -func TestDefaultRegistryHasClaudeAndCodex(t *testing.T) { - if FindAgent("claude") == nil { - t.Error("claude agent not registered") - } - if FindAgent("codex") == nil { - t.Error("codex 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/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) + } + }) +} 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 }