diff --git a/.surface b/.surface index 64f42a04f..7f7b23b32 100644 --- a/.surface +++ b/.surface @@ -143,6 +143,11 @@ hey label remove --from hey labels hey labels --all hey labels --limit +hey login +hey login --cookie +hey login --no-browser +hey login --token +hey logout hey move hey move --to hey recordings @@ -184,6 +189,9 @@ hey search --to hey search filters hey seen hey setup +hey setup agents +hey setup claude +hey setup codex hey setup omarchy hey setup omarchy --no-notify hey setup omarchy --notify diff --git a/AGENTS.md b/AGENTS.md index 935bbb559..7522c2db7 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -39,11 +39,40 @@ Authentication supports four methods, all managed through `internal/auth/`: The auth Manager (`internal/auth/auth.go`) proactively refreshes tokens with a 5-minute expiry buffer. The SDK uses the Manager to authenticate requests via a bridge in `internal/cmd/sdk.go`. -All data-access commands call `requireAuth()` before making API calls. Auth subcommands (`hey auth login`, `hey auth logout`, `hey auth status`) work without authentication. +All data-access commands call `requireAuth()` before making API calls. At an interactive +terminal with styled output, `requireAuth()` offers to sign in on the spot ("Not logged in. +Sign in now?" → OAuth → the command continues); piped, machine-output or declined runs get +`Error: Not logged in` / `Run: hey auth login` with exit code 3. Auth subcommands (`hey auth +login`, `hey auth logout`, `hey auth status`), the `hey login`/`hey logout` shortcuts, +`hey setup` and `hey doctor` work without authentication. + +Bare `hey` at an interactive terminal runs the setup wizard when logged out +(`runSetupWizard` in `internal/cmd/setup.go` — welcome, OAuth sign-in, linked-account +greeting, coding-agent setup, summary) and stops at the summary; in every other case it +prints help. The TUI lives at `hey tui` (plus the hidden `hey hey`). `config.json`'s +`onboarded` flag only trims a later logged-out run to the sign-in step. `HEY_NONINTERACTIVE=1` +disables every prompt regardless of TTY detection: the wizard skips OAuth and answers its +own confirmations with their defaults. + +Coding-agent integration lives in `internal/harness` (agent registry, Claude Code / Codex +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 gets the skill only until a `.codex-plugin` ships. `HEY_SETUP_AGENT` +selects the target for `hey setup agents`. `hey doctor` reports per-agent diagnostics, and a +`PersistentPostRunE` hook (`skill_refresh.go`) re-syncs installed skill copies once per release +version change. Ownership is explicit: every skill write goes through `claimSkillDir`, which +marks a directory it creates (`.managed-by-hey-cli`) and refuses a populated one without the +marker; removal (`removeExistingSkillLink`) touches only our canonical symlink or a marked +copy; refresh writes only marked, non-symlinked files. Do not add a write path that bypasses +the gate. + +New top-level commands and subcommands need the `.surface` snapshot updated +(`go test ./internal/cmd -run TestSurfaceSnapshot -update-surface`), and anything listed in +root help needs the byte-exact literal in `help_test.go` updated. ### State storage -Configuration (the base URL, the default linked account, and the Imbox's cover) is stored in `~/.config/hey-cli/config.json`. Credentials are stored in the system keyring (service name: `hey`) with automatic fallback to `~/.config/hey-cli/credentials.json` when the keyring is unavailable. Set `HEY_NO_KEYRING=1` to force file storage. +Configuration (the base URL, the default linked account, the Imbox's cover, and the `onboarded` flag) is stored in `~/.config/hey-cli/config.json`; `onboarded` is read from the global file only, never from a repository's `.hey/config.json`. Credentials are stored in the system keyring (service name: `hey`) with automatic fallback to `~/.config/hey-cli/credentials.json` when the keyring is unavailable. Set `HEY_NO_KEYRING=1` to force file storage. ### CLI diff --git a/README.md b/README.md index 49f5ebbc4..db63b2356 100644 --- a/README.md +++ b/README.md @@ -39,6 +39,22 @@ irm https://hey.com/install-cli.ps1 | iex On Windows 11 with Smart App Control, see [Troubleshooting](#windows-smart-app-control-and-smartscreen) if the install is blocked. +## Getting started + +```bash +hey +``` + +The first time you run `hey` at a terminal it walks you through setup: it signs you in +(browser-based OAuth), shows the mail accounts linked to your HEY identity, and offers to +connect your coding agents (Claude Code, Codex). After that, `hey tui` opens the app and +bare `hey` prints the help. `hey setup` reruns the wizard any time; `hey login` and +`hey logout` are shortcuts for `hey auth login` and `hey auth logout`. + +Logged-out data commands at a terminal (`hey boxes`, say) offer to sign you in on the spot. +Piped or `--json` runs never prompt: they fail with `Not logged in` (exit 3) so scripts and +agents can handle it. + Both scripts download the release for your platform, verify its SHA-256 checksum, and — when `cosign` is installed — verify the release's keyless Sigstore signature (cosign v3 as-is, v2.6+ with `--new-bundle-format=true`; older versions skip signature verification with a warning). Set `HEY_VERSION` to pin a release and `HEY_BIN_DIR` to choose the install directory.
@@ -145,6 +161,8 @@ hey auth refresh # force token refresh hey auth logout # clear credentials ``` +`hey login` and `hey logout` are top-level shortcuts for `hey auth login` and `hey auth logout`. + ### Linked accounts One HEY login exposes every mail account linked to that identity. List the available @@ -176,9 +194,10 @@ identity-wide. ## TUI -Run `hey tui` to launch the interactive terminal UI. Bare `hey` prints the help. For -identities with multiple linked mail accounts, press Ctrl+A to switch between All Accounts -and individual email addresses. +Run `hey tui` to launch the interactive terminal UI (it offers to sign you in first if +needed). Bare `hey` prints the help — or, logged out at a terminal, runs first-time setup. +For identities with multiple linked mail accounts, press Ctrl+A to switch between All +Accounts and individual email addresses. Switching cancels requests from the previous account and reloads the active section; Calendar and Journal remain identity-wide. @@ -431,14 +450,32 @@ Imbox` for more — replacing the previous toast rather than stacking, and click focuses the TUI. Omarchy's notification silencing (SUPER+CTRL+comma) mutes them like any other app. See [docs/omarchy.md](docs/omarchy.md) for the details and what is planned next. -## Agent Skill +## AI agent integration -hey-cli ships with an embedded agent skill so your agent can interact with HEY on your behalf. +hey-cli ships with an embedded agent skill so your coding agent can work with HEY on your +behalf, and a Claude Code plugin (`hey@37signals` from the `basecamp/claude-plugins` +marketplace). The setup wizard offers to connect detected agents; these commands do it on +their own: ```bash -hey skill install # install the skill globally for your agent +hey setup claude # install the skill and the hey@37signals plugin for Claude Code +hey setup codex # install the skill for Codex +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 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` +picks explicitly. `HEY_NONINTERACTIVE=1` disables every prompt (the sign-in offer, the +wizard's confirmations) for harnesses that run hey under a pseudo-terminal. The installed +skill is refreshed automatically the first time a new hey release runs. + +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 +`hey` skill directory (or symlink) without it — a hand-authored skill at one of those paths +is never overwritten or claimed. `hey doctor` flags an unmanaged baseline and how to adopt it. + ## Troubleshooting ```bash diff --git a/internal/auth/auth.go b/internal/auth/auth.go index 7875935e3..b5f0494e3 100644 --- a/internal/auth/auth.go +++ b/internal/auth/auth.go @@ -142,6 +142,21 @@ func (m *Manager) IsAuthenticated() bool { // LoginOptions configures the login flow. type LoginOptions struct { NoBrowser bool + + // Logger receives login progress messages (browser hand-off, the + // authorization URL, waiting notice). Nil keeps the default os.Stderr + // output unchanged. Messages may span multiple lines. + Logger func(msg string) +} + +// log routes a progress message to the configured Logger, or to os.Stderr +// verbatim when none is set so `hey auth login` output stays as it was. +func (o LoginOptions) log(msg string) { + if o.Logger != nil { + o.Logger(msg) + return + } + fmt.Fprint(os.Stderr, msg) } // Login initiates the browser-based OAuth login flow with PKCE. @@ -351,12 +366,12 @@ func (m *Manager) waitForCallback(ctx context.Context, expectedState, authURL, c if !opts.NoBrowser { if err := openBrowser(authURL); err != nil { - fmt.Fprintf(os.Stderr, "\nCouldn't open browser automatically.\nOpen this URL in your browser:\n%s\n\nWaiting for authentication...\n", authURL) + opts.log(fmt.Sprintf("\nCouldn't open browser automatically.\nOpen this URL in your browser:\n%s\n\nWaiting for authentication...\n", authURL)) } else { - fmt.Fprintf(os.Stderr, "\nOpening browser for authentication...\nIf the browser doesn't open, visit: %s\n\nWaiting for authentication...\n", authURL) + opts.log(fmt.Sprintf("\nOpening browser for authentication...\nIf the browser doesn't open, visit: %s\n\nWaiting for authentication...\n", authURL)) } } else { - fmt.Fprintf(os.Stderr, "\nOpen this URL in your browser:\n%s\n\nWaiting for authentication...\n", authURL) + opts.log(fmt.Sprintf("\nOpen this URL in your browser:\n%s\n\nWaiting for authentication...\n", authURL)) } select { diff --git a/internal/auth/auth_test.go b/internal/auth/auth_test.go index 6510885e7..2f4771f4e 100644 --- a/internal/auth/auth_test.go +++ b/internal/auth/auth_test.go @@ -10,6 +10,7 @@ import ( "net/http" "net/http/httptest" "net/url" + "os" "strings" "testing" "time" @@ -602,3 +603,60 @@ func TestRefreshFailuresPreserveCredentials(t *testing.T) { }) } } + +func TestLoginOptionsLoggerReceivesProgress(t *testing.T) { + listenConfig := &net.ListenConfig{} + listener, err := listenConfig.Listen(t.Context(), "tcp", "127.0.0.1:0") + if err != nil { + t.Fatalf("Listen: %v", err) + } + mgr := NewManager("http://example.test", http.DefaultClient, t.TempDir()) + mgr.listen = func(context.Context, string, string) (net.Listener, error) { + return listener, nil + } + + // Capture stderr: a configured Logger must own all progress output. + origStderr := os.Stderr + r, w, err := os.Pipe() + if err != nil { + t.Fatal(err) + } + os.Stderr = w + t.Cleanup(func() { os.Stderr = origStderr }) + + var logged []string + opts := LoginOptions{NoBrowser: true, Logger: func(msg string) { logged = append(logged, msg) }} + + done := make(chan struct{}) + go func() { + defer close(done) + _, _ = mgr.waitForCallback(t.Context(), "expected", "http://example.test/authorize", "addr", opts) + }() + + client := &http.Client{Timeout: 2 * time.Second} + response, err := client.Get("http://" + listener.Addr().String() + "/callback?state=expected&code=abc123") //nolint:noctx // local one-shot test request + if err != nil { + t.Fatalf("GET callback: %v", err) + } + _ = response.Body.Close() + select { + case <-done: + case <-time.After(2 * time.Second): + t.Fatal("waitForCallback did not return") + } + + _ = w.Close() + os.Stderr = origStderr + captured, _ := io.ReadAll(r) + + all := strings.Join(logged, "\n") + if !strings.Contains(all, "http://example.test/authorize") { + t.Errorf("Logger did not receive the authorization URL: %q", all) + } + if !strings.Contains(all, "Waiting for authentication") { + t.Errorf("Logger did not receive the waiting notice: %q", all) + } + if len(captured) != 0 { + t.Errorf("stderr should stay silent when a Logger is set, got %q", captured) + } +} diff --git a/internal/cmd/auth.go b/internal/cmd/auth.go index d8c9274eb..9805d17d4 100644 --- a/internal/cmd/auth.go +++ b/internal/cmd/auth.go @@ -3,12 +3,16 @@ package cmd import ( "context" "fmt" + "io" "os" + "sort" + "strings" "time" "github.com/spf13/cobra" "github.com/basecamp/hey-cli/internal/auth" + "github.com/basecamp/hey-cli/internal/harness" "github.com/basecamp/hey-cli/internal/output" ) @@ -39,6 +43,12 @@ func newAuthCommand() *authCommand { // login subcommand func newAuthLoginCommand() *cobra.Command { + return buildLoginCommand("hey auth login") +} + +// buildLoginCommand constructs a login command whose examples read as the +// given path. Shared by `hey auth login` and the top-level `hey login`. +func buildLoginCommand(path string) *cobra.Command { var ( token string cookie string @@ -51,10 +61,12 @@ func newAuthLoginCommand() *cobra.Command { Long: `Authenticate with the HEY server via Launchpad OAuth. Opens a browser for OAuth authentication. Use --token or --cookie for non-interactive login.`, - Example: ` hey auth login - hey auth login --token YOUR_BEARER_TOKEN - hey auth login --cookie SESSION_COOKIE_VALUE - hey auth login --no-browser`, + Example: strings.Join([]string{ + " " + path, + " " + path + " --token YOUR_BEARER_TOKEN", + " " + path + " --cookie SESSION_COOKIE_VALUE", + " " + path + " --no-browser", + }, "\n"), RunE: func(cmd *cobra.Command, args []string) error { if token != "" { if err := authMgr.LoginWithToken(token); err != nil { @@ -86,7 +98,12 @@ Opens a browser for OAuth authentication. Use --token or --cookie for non-intera } if writer.IsStyled() { - fmt.Fprintln(cmd.OutOrStdout(), "Logged in successfully.") + w := cmd.OutOrStdout() + fmt.Fprintln(w, "Logged in successfully.") + if identity, err := rootSDK.Identity().GetIdentity(cmd.Context()); err == nil && identity != nil { + fmt.Fprintln(w, identityGreeting(identity)) + } + printAgentNudge(w) return nil } return writeOK(map[string]string{"method": "oauth"}, output.WithSummary("Logged in successfully")) @@ -103,9 +120,16 @@ Opens a browser for OAuth authentication. Use --token or --cookie for non-intera // logout subcommand func newAuthLogoutCommand() *cobra.Command { + return buildLogoutCommand("hey auth logout") +} + +// buildLogoutCommand constructs a logout command whose example reads as the +// given path. Shared by `hey auth logout` and the top-level `hey logout`. +func buildLogoutCommand(path string) *cobra.Command { return &cobra.Command{ - Use: "logout", - Short: "Clear stored credentials", + Use: "logout", + Short: "Clear stored credentials", + Example: " " + path, RunE: func(cmd *cobra.Command, args []string) error { if err := authMgr.Logout(); err != nil { return output.ErrAuth(fmt.Sprintf("could not clear credentials: %v", err)) @@ -280,3 +304,41 @@ func newAuthTokenCommand() *cobra.Command { return cmd } + +// printAgentNudge prints a hint about coding agent setup after login. +// +// Detection proves presence, not intent: with a single detected-unhealthy +// agent it points at that agent; with several, it never guesses — it prints +// every `hey setup ` choice so the user picks. It never suggests +// `hey setup agents`, which is the installer's non-interactive path. +func printAgentNudge(w io.Writer) { + type nudgeAgent struct{ id, name string } + var unhealthy []nudgeAgent + for _, agent := range harness.DetectedAgents() { + if agent.Checks == nil { + continue + } + for _, c := range agent.Checks() { + if c.Status != "pass" { + unhealthy = append(unhealthy, nudgeAgent{id: agent.ID, name: agent.Name}) + break + } + } + } + sort.Slice(unhealthy, func(i, j int) bool { return unhealthy[i].id < unhealthy[j].id }) + + switch len(unhealthy) { + case 0: + return + case 1: + fmt.Fprintln(w) + fmt.Fprintln(w, muted.format(fmt.Sprintf(" %s detected. Connect it to HEY:", unhealthy[0].name))) + fmt.Fprintln(w, bold.format(" hey setup "+unhealthy[0].id)) + default: + fmt.Fprintln(w) + fmt.Fprintln(w, muted.format(" Multiple coding agents detected. Choose one:")) + for _, a := range unhealthy { + fmt.Fprintln(w, bold.format(" hey setup "+a.id)) + } + } +} diff --git a/internal/cmd/auth_commands_test.go b/internal/cmd/auth_commands_test.go index 8c81df748..80c38dcec 100644 --- a/internal/cmd/auth_commands_test.go +++ b/internal/cmd/auth_commands_test.go @@ -190,6 +190,7 @@ func TestDoctorCommandReportsEnvironment(t *testing.T) { if err := os.WriteFile(skillPath, []byte("# HEY"), 0600); err != nil { t.Fatalf("write skill: %v", err) } + writeOwnershipMarker(filepath.Dir(skillPath)) // a hey-cli-written install _, response, err := runAuthCommand(t, configHome, server.URL, "environment-token", true, "doctor") if err != nil { @@ -220,8 +221,8 @@ func TestDoctorCommandReportsEnvironment(t *testing.T) { if got := byName["Shell"]["message"]; got != "/bin/zsh" { t.Errorf("shell = %q", got) } - if got := byName["Claude Skill"]["status"]; got != "ok" { - t.Errorf("Claude Skill status = %q", got) + if got := byName["Agent Skill"]["status"]; got != "ok" { + t.Errorf("Agent Skill status = %q", got) } if _, ok := byName["CLI Version"]; !ok { t.Error("CLI Version check missing") @@ -253,3 +254,87 @@ func TestDoctorCommandReportsMissingAuthentication(t *testing.T) { } t.Fatal("Authentication check missing") } + +func TestLoginLogoutShortcutsMirrorAuthCommands(t *testing.T) { + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + t.Errorf("unexpected HTTP request: %s %s", r.Method, r.URL.Path) + })) + defer server.Close() + configHome := t.TempDir() + + _, login, err := runAuthCommand(t, configHome, server.URL, "", true, "login", "--cookie", "session-cookie") + if err != nil { + t.Fatalf("login: %v", err) + } + if login.Summary != "Logged in with session cookie" { + t.Errorf("login summary = %q", login.Summary) + } + + _, status, err := runAuthCommand(t, configHome, server.URL, "", true, "auth", "status") + if err != nil { + t.Fatalf("auth status: %v", err) + } + if data, _ := status.Data.(map[string]any); data["authenticated"] != true { + t.Errorf("status after hey login = %#v", status.Data) + } + + _, logout, err := runAuthCommand(t, configHome, server.URL, "", true, "logout") + if err != nil { + t.Fatalf("logout: %v", err) + } + if logout.Summary != "Logged out" { + t.Errorf("logout summary = %q", logout.Summary) + } + + _, status, err = runAuthCommand(t, configHome, server.URL, "", true, "auth", "status") + if err != nil { + t.Fatalf("auth status: %v", err) + } + if data, _ := status.Data.(map[string]any); data["authenticated"] != false { + t.Errorf("status after hey logout = %#v", status.Data) + } + + root := newRootCmd() + login2, _, err := root.Find([]string{"login"}) + if err != nil || !strings.Contains(login2.Example, "hey login --token") || strings.Contains(login2.Example, "hey auth login") { + t.Errorf("hey login example = %q", login2.Example) + } +} + +func TestPrintAgentNudge(t *testing.T) { + isolateAgents(t) + home := t.TempDir() + t.Setenv("HOME", home) + t.Setenv("USERPROFILE", home) + origColor := colorDisabled + colorDisabled = true + t.Cleanup(func() { colorDisabled = origColor }) + + var out bytes.Buffer + printAgentNudge(&out) + if out.Len() != 0 { + t.Errorf("no agents: nudge should be silent, got %q", out.String()) + } + + if err := os.MkdirAll(filepath.Join(home, ".claude"), 0o755); err != nil { + t.Fatal(err) + } + out.Reset() + printAgentNudge(&out) + if !strings.Contains(out.String(), "Claude Code detected. Connect it to HEY:") || !strings.Contains(out.String(), "hey setup claude") { + t.Errorf("single agent nudge = %q", out.String()) + } + if strings.Contains(out.String(), "setup agents") { + t.Errorf("nudge must never suggest setup agents: %q", out.String()) + } + + if err := os.MkdirAll(filepath.Join(home, ".codex"), 0o755); err != nil { + t.Fatal(err) + } + out.Reset() + printAgentNudge(&out) + text := out.String() + if !strings.Contains(text, "Multiple coding agents detected. Choose one:") || !strings.Contains(text, "hey setup claude") || !strings.Contains(text, "hey setup codex") { + t.Errorf("multiple agent nudge = %q", text) + } +} diff --git a/internal/cmd/config.go b/internal/cmd/config.go index a22f3a18b..d4c289273 100644 --- a/internal/cmd/config.go +++ b/internal/cmd/config.go @@ -2,6 +2,7 @@ package cmd import ( "fmt" + "strconv" "github.com/spf13/cobra" @@ -44,12 +45,19 @@ func newConfigSetCommand() *cobra.Command { if err := cfg.SetFromFlag(key, value); err != nil { return err } + if err := cfg.SaveBaseURL(cfg.BaseURL); err != nil { + return err + } + case "onboarded": + onboarded, err := strconv.ParseBool(value) + if err != nil { + return output.ErrUsage(fmt.Sprintf("onboarded must be true or false (got %q)", value)) + } + if err := cfg.SaveOnboarded(onboarded); err != nil { + return err + } default: - return output.ErrUsage(fmt.Sprintf("unknown config key: %s (available: base_url)", key)) - } - - if err := cfg.SaveBaseURL(cfg.BaseURL); err != nil { - return err + return output.ErrUsage(fmt.Sprintf("unknown config key: %s (available: base_url, onboarded)", key)) } if writer.IsStyled() { @@ -156,6 +164,11 @@ func newConfigShowCommand() *cobra.Command { "value": cfg.AccountID, "source": string(cfg.SourceOf("account_id")), }, + { + "key": "onboarded", + "value": strconv.FormatBool(cfg.Onboarded), + "source": "global", + }, } if local := cfg.LocalConfig(); local != nil { trust := "untrusted" diff --git a/internal/cmd/doctor.go b/internal/cmd/doctor.go index 7c389e0c6..b3ba1adab 100644 --- a/internal/cmd/doctor.go +++ b/internal/cmd/doctor.go @@ -12,6 +12,7 @@ import ( "github.com/spf13/cobra" "github.com/basecamp/hey-cli/internal/config" + "github.com/basecamp/hey-cli/internal/harness" "github.com/basecamp/hey-cli/internal/output" "github.com/basecamp/hey-cli/internal/version" ) @@ -132,7 +133,8 @@ func runDoctorChecks(ctx context.Context) []map[string]string { checks = append(checks, map[string]string{ "name": "Authentication", "status": "error", - "message": "Not authenticated — run `hey auth login`", + "message": "Not authenticated", + "hint": "hey auth login", }) } @@ -146,21 +148,21 @@ func runDoctorChecks(ctx context.Context) []map[string]string { }) } - // Claude Plugin - if _, err := os.Stat(".claude-plugin/plugin.json"); err == nil { - checks = append(checks, map[string]string{ - "name": "Claude Plugin", - "status": "ok", - "message": "Found .claude-plugin/plugin.json", - }) - } else { - // Check if installed globally - home, _ := os.UserHomeDir() - if _, err := os.Stat(home + "/.agents/skills/hey/SKILL.md"); err == nil { + // Agent skill + detected coding agents + checks = append(checks, checkBaselineSkill()) + for _, agent := range harness.DetectedAgents() { + var agentChecks []*harness.StatusCheck + if agent.Diagnostics != nil { + agentChecks = agent.Diagnostics(ctx) + } else if agent.Checks != nil { + agentChecks = agent.Checks() + } + for _, c := range agentChecks { checks = append(checks, map[string]string{ - "name": "Claude Skill", - "status": "ok", - "message": "Installed at ~/.agents/skills/hey/", + "name": c.Name, + "status": doctorStatus(c.Status), + "message": c.Message, + "hint": c.Hint, }) } } @@ -177,6 +179,72 @@ func runDoctorChecks(ctx context.Context) []map[string]string { return checks } +// doctorStatus maps a harness check status onto doctor's status vocabulary. +func doctorStatus(status string) string { + switch status { + case "pass": + return "ok" + case "warn": + return "warning" + case "fail": + return "error" + default: + return status + } +} + +// checkBaselineSkill reports whether the shared agent skill is installed and +// current. Not installed is a warning, not an error: the CLI works without it, +// coding agents just get no HEY skill. +func checkBaselineSkill() map[string]string { + home, err := os.UserHomeDir() + if err != nil { + return map[string]string{ + "name": "Agent Skill", + "status": "warning", + "message": "Cannot determine home directory", + } + } + skillPath := filepath.Join(home, ".agents", "skills", "hey", "SKILL.md") + if _, statErr := os.Lstat(skillPath); statErr != nil { + return map[string]string{ + "name": "Agent Skill", + "status": "warning", + "message": "Not installed", + "hint": "hey skill install", + } + } + // Something occupies the path but fails the health rules (unmarked + // hand-authored skill, pre-ownership hey-cli install, symlinked file): + // hey skill install would refuse it, so the remediation is move-aside. + if !baselineSkillInstalled() { + return map[string]string{ + "name": "Agent Skill", + "status": "warning", + "message": "Present, but not a hey-cli-managed install (hand-authored, or from a release before ownership tracking); it will not be kept up to date", + "hint": "Move it aside and run: hey skill install", + } + } + check := map[string]string{ + "name": "Agent Skill", + "status": "ok", + } + installed := installedSkillVersion() + switch { + case installed == "": + check["message"] = "Installed (version not tracked)" + case version.IsDev(): + check["message"] = fmt.Sprintf("Installed (%s, dev build)", installed) + case installed == version.Version: + check["message"] = fmt.Sprintf("Up to date (%s)", installed) + default: + check["status"] = "warning" + check["message"] = fmt.Sprintf("Stale (installed: %s, current: %s)", installed, version.Version) + check["hint"] = "hey skill install" + } + return check +} + // checkVersion reports the build and, for release builds, whether a newer // release exists. The lookup is best-effort and bounded: a slow or offline // api.github.com must not stall doctor. Dev and other non-semver builds have diff --git a/internal/cmd/doctor_test.go b/internal/cmd/doctor_test.go index 259952f16..007e14a53 100644 --- a/internal/cmd/doctor_test.go +++ b/internal/cmd/doctor_test.go @@ -3,6 +3,8 @@ package cmd import ( "context" "errors" + "os" + "path/filepath" "strings" "testing" ) @@ -118,3 +120,51 @@ func TestDoctorVersionIgnoresNonReleaseLatestTag(t *testing.T) { } } } + +// Presence and health are reported separately: an unmanaged skill gets the +// move-aside remediation (hey skill install would refuse it), a missing one +// gets the install hint, and a marker that is not a regular file confers no +// ownership. +func TestDoctorBaselineSkillDiagnostics(t *testing.T) { + home := t.TempDir() + t.Setenv("HOME", home) + t.Setenv("USERPROFILE", home) + + check := checkBaselineSkill() + if check["message"] != "Not installed" || check["hint"] != "hey skill install" { + t.Errorf("missing skill: %v", check) + } + + dir := filepath.Join(home, ".agents", "skills", "hey") + if err := os.MkdirAll(dir, 0o755); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(dir, "SKILL.md"), []byte("# mine"), 0o644); err != nil { + t.Fatal(err) + } + check = checkBaselineSkill() + if check["status"] != "warning" || !strings.Contains(check["message"], "not a hey-cli-managed install") { + t.Errorf("unmanaged skill: %v", check) + } + if check["hint"] != "Move it aside and run: hey skill install" { + t.Errorf("unmanaged hint = %q", check["hint"]) + } + + // A directory planted in the marker's name confers no ownership. + if err := os.MkdirAll(filepath.Join(dir, ".managed-by-hey-cli"), 0o755); err != nil { + t.Fatal(err) + } + check = checkBaselineSkill() + if !strings.Contains(check["message"], "not a hey-cli-managed install") { + t.Errorf("non-regular marker treated as ownership: %v", check) + } + + if err := os.RemoveAll(filepath.Join(dir, ".managed-by-hey-cli")); err != nil { + t.Fatal(err) + } + writeOwnershipMarker(dir) + check = checkBaselineSkill() + if check["status"] != "ok" { + t.Errorf("managed skill: %v", check) + } +} diff --git a/internal/cmd/formatting.go b/internal/cmd/formatting.go index 497c95511..d1f858a67 100644 --- a/internal/cmd/formatting.go +++ b/internal/cmd/formatting.go @@ -12,6 +12,7 @@ import ( "github.com/mattn/go-runewidth" "golang.org/x/term" + "github.com/basecamp/hey-cli/internal/config" "github.com/basecamp/hey-cli/internal/markdown" "github.com/basecamp/hey-cli/internal/output" ) @@ -72,9 +73,12 @@ func (t *table) updateColumnWidths(row []string) { type style string const ( - plain style = "" - bold style = "1;34" - italic style = "3;94" + plain style = "" + bold style = "1;34" + italic style = "3;94" + success style = "32" + warning style = "33" + muted style = "90" ) func (s style) format(value string) string { @@ -119,11 +123,13 @@ func threadNoun(count int) string { return "threads" } -func stdinIsTerminal() bool { +// stdinIsTerminal and stdoutIsTerminal are seam variables (house pattern: +// askLocalConfigTrust) so tests can simulate an interactive terminal. +var stdinIsTerminal = func() bool { return term.IsTerminal(int(os.Stdin.Fd())) //nolint:gosec // G115: fd fits in int } -func stdoutIsTerminal() bool { +var stdoutIsTerminal = func() bool { return term.IsTerminal(int(os.Stdout.Fd())) //nolint:gosec // G115: fd fits in int } @@ -137,6 +143,18 @@ func stdoutWidth() int { return min(width-2, 100) } +var stderrIsTerminal = func() bool { + return term.IsTerminal(int(os.Stderr.Fd())) //nolint:gosec // G115: fd fits in int +} + +// interactiveStdio reports whether the CLI may prompt: stdin, stdout AND +// stderr are terminals — prompts render on stderr so stdout stays data, and +// an invisible prompt must never sit waiting for input — and the +// HEY_NONINTERACTIVE escape hatch is not engaged. +var interactiveStdio = func() bool { + return stdinIsTerminal() && stdoutIsTerminal() && stderrIsTerminal() && !config.NonInteractiveEnv() +} + func readStdin() (string, error) { data, err := io.ReadAll(os.Stdin) if err != nil { diff --git a/internal/cmd/help_test.go b/internal/cmd/help_test.go index b184c13ae..8226b8bb0 100644 --- a/internal/cmd/help_test.go +++ b/internal/cmd/help_test.go @@ -12,7 +12,7 @@ func TestCuratedCommandHelpUsesUserFacingLanguage(t *testing.T) { root := newRootCmd() tests := map[string]string{ "auth": "Sign in to HEY, sign out, or check your login status.", - "setup": "Sign in and prepare HEY for first use.", + "setup": "Sign in and connect your coding agents.", } for name, expected := range tests { diff --git a/internal/cmd/local_config_trust.go b/internal/cmd/local_config_trust.go index 969fa6b28..52ca175e4 100644 --- a/internal/cmd/local_config_trust.go +++ b/internal/cmd/local_config_trust.go @@ -26,7 +26,7 @@ func ensureLocalConfigTrusted(cmd *cobra.Command) error { if local == nil || !commandUsesRuntimeConfig(cmd) { return nil } - if machineReadableOutput(cmd) || !stdinIsTerminal() || !stdoutIsTerminal() { + if machineReadableOutput(cmd) || !interactiveStdio() { return untrustedLocalConfigError(local) } @@ -59,7 +59,18 @@ func commandIgnoresLocalConfig(cmd *cobra.Command) bool { if len(parts) < 2 { return false } - return parts[1] == "omarchy" || (len(parts) >= 3 && parts[1] == "setup" && parts[2] == "omarchy") + switch parts[1] { + case "omarchy", "skill": + return true + case "setup": + // The wizard itself uses the effective server; its subcommands + // (agents, claude, codex, 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 + default: + return false + } } // commandUsesRuntimeConfig reports whether a command reads the effective @@ -74,6 +85,13 @@ func commandUsesRuntimeConfig(cmd *cobra.Command) bool { switch parts[1] { case "commands", "completion", "config", "skill", "upgrade", "version": return false + case "setup": + // `hey setup` itself signs in against the effective server, but its + // subcommands (agents, claude, codex) 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. + return len(parts) == 2 default: return true } diff --git a/internal/cmd/local_config_trust_test.go b/internal/cmd/local_config_trust_test.go index 2901ce9c1..aa53990b2 100644 --- a/internal/cmd/local_config_trust_test.go +++ b/internal/cmd/local_config_trust_test.go @@ -222,3 +222,72 @@ func TestVersionRunsWithUntrustedLocalConfig(t *testing.T) { t.Fatalf("version with an untrusted local config: %v", err) } } + +// The agent-facing setup subcommands never touch the HEY server or account, +// and the installer's non-TTY handoff runs `setup agents` from an arbitrary +// directory — an untrusted .hey/config.json there must not block it. The +// wizard itself signs in against the effective server, so it stays gated. +func TestSetupSubcommandsSkipLocalConfigTrust(t *testing.T) { + root := newRootCmd() + for _, test := range []struct { + args []string + want bool + }{ + {args: []string{"setup"}, want: true}, + {args: []string{"setup", "agents"}, want: false}, + {args: []string{"setup", "claude"}, want: false}, + {args: []string{"setup", "codex"}, want: false}, + } { + command, _, err := root.Find(test.args) + if err != nil { + t.Fatalf("Find(%v): %v", test.args, err) + } + if got := commandUsesRuntimeConfig(command); got != test.want { + t.Errorf("commandUsesRuntimeConfig(%v) = %v, want %v", test.args, got, test.want) + } + } +} + +func TestSetupAgentsRunsWithUntrustedLocalConfig(t *testing.T) { + isolateAgents(t) + t.Setenv(agentSetupEnv, "none") + setupLocalTrustTest(t, "https://mail.example.com", "all") + home := t.TempDir() + t.Setenv("HOME", home) + t.Setenv("USERPROFILE", home) + + if err := runLocalTrustCLI(t, "--json", "setup", "agents"); err != nil { + t.Fatalf("setup agents must not be blocked by an untrusted local config: %v", err) + } + if _, err := os.Stat(filepath.Join(home, ".agents", "skills", "hey", "SKILL.md")); err != nil { + t.Errorf("skill was not installed: %v", err) + } +} + +// The installer pipes curl from an arbitrary directory: a malformed +// .hey/config.json there must not stop a local-only command before its +// trust exemption is even consulted. Runtime-config commands still surface +// the parse error. +func TestSetupAgentsRunsWithMalformedLocalConfig(t *testing.T) { + isolateAgents(t) + t.Setenv(agentSetupEnv, "none") + path := setupLocalTrustTest(t, "https://mail.example.com", "all") + if err := os.WriteFile(path, []byte("{not json"), 0600); err != nil { + t.Fatal(err) + } + home := t.TempDir() + t.Setenv("HOME", home) + t.Setenv("USERPROFILE", home) + + if err := runLocalTrustCLI(t, "--json", "setup", "agents"); err != nil { + t.Fatalf("setup agents must survive a malformed local config: %v", err) + } + if _, err := os.Stat(filepath.Join(home, ".agents", "skills", "hey", "SKILL.md")); err != nil { + t.Errorf("skill was not installed: %v", err) + } + + err := runLocalTrustCLI(t, "--json", "boxes") + if err == nil || !strings.Contains(err.Error(), "could not parse config") { + t.Fatalf("runtime-config commands must still report the parse error, got %v", err) + } +} diff --git a/internal/cmd/login.go b/internal/cmd/login.go new file mode 100644 index 000000000..6ee7af7a2 --- /dev/null +++ b/internal/cmd/login.go @@ -0,0 +1,8 @@ +package cmd + +import "github.com/spf13/cobra" + +// newLoginCommand is the top-level `hey login` shortcut for `hey auth login`. +func newLoginCommand() *cobra.Command { + return buildLoginCommand("hey login") +} diff --git a/internal/cmd/logout.go b/internal/cmd/logout.go new file mode 100644 index 000000000..fd6561a09 --- /dev/null +++ b/internal/cmd/logout.go @@ -0,0 +1,8 @@ +package cmd + +import "github.com/spf13/cobra" + +// newLogoutCommand is the top-level `hey logout` shortcut for `hey auth logout`. +func newLogoutCommand() *cobra.Command { + return buildLogoutCommand("hey logout") +} diff --git a/internal/cmd/root.go b/internal/cmd/root.go index c343db019..5d21cb010 100644 --- a/internal/cmd/root.go +++ b/internal/cmd/root.go @@ -15,6 +15,7 @@ import ( "github.com/basecamp/hey-cli/internal/auth" "github.com/basecamp/hey-cli/internal/config" "github.com/basecamp/hey-cli/internal/output" + "github.com/basecamp/hey-cli/internal/tui" "github.com/basecamp/hey-cli/internal/version" ) @@ -38,20 +39,14 @@ var ( writer *output.Writer ) +// runTUI is a seam so tests can observe root routing without a terminal. +var runTUI = tui.Run + func newRootCmd() *cobra.Command { root := &cobra.Command{ - Use: "hey", - Short: "Read, send, and organize HEY email, contacts, and calendars from your terminal.", - Long: `A CLI for HEY -⠀⠀⠀⠀⠀⠀⣰⠲⣄⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀ -⠀⠀⠀⡟⢳⡀⣏⠀⠘⣆⠀⠀⠀⠀⠀⣤⣤⡄⠀⠀⢠⣤⣤⣤⣤⣤⣤⣤⣤⠀⠀⢀⣤⣤⡄⠀ -⠀⣴⢄⢳⠀⠹⣿⠀⠀⠸⣆⠴⠒⢢⡀⢻⣿⡇⠀⠀⢸⣿⣿⡟⠛⠛⠛⢿⣿⣇⠀⣼⣿⡟⠀⠀ -⠀⢻⠈⠻⣧⠀⠹⣇⠀⢰⣿⠀⠀⠀⡇⢸⣿⣷⣶⣶⣾⣿⣿⣷⣶⣶⠀⠈⢿⣿⣼⣿⡟⠀⠀⠀ -⣶⠺⣧⡀⠙⢧⠀⠉⠀⣸⢸⡆⠀⢸⠁⣼⣿⡏⠉⠉⢹⣿⣿⡏⠉⠉⠀⠀⠈⣿⣿⡟⠀⠀⠀⠀ -⠘⣆⠈⠳⠀⠀⠀⠀⠀⢻⢸⠇⢀⡏⠀⣿⣿⡇⠀⠀⢸⣿⣿⣿⣶⣶⣶⡆⠀⣿⣿⡇⠀⠀⠀⠀ -⠀⠈⠳⣄⡀⠀⠀⠀⠀⠈⠉⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀ -⠀⠀⠀⠀⠉⠙⠚⠃⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀ - `, + Use: "hey", + Short: "Read, send, and organize HEY email, contacts, and calendars from your terminal.", + Long: "A CLI for HEY\n" + tui.Wordmark, SilenceUsage: true, SilenceErrors: true, PersistentPreRunE: func(cmd *cobra.Command, args []string) error { @@ -115,7 +110,17 @@ func newRootCmd() *cobra.Command { authMgr = auth.NewManager(cfg.BaseURL, httpClient, configDir) initSDK(authMgr, cfg.BaseURL) - migrateOldCredentials(configDir) + // The agent-local setup subcommands and skill commands never read + // credentials and never rewrite the global config, so they defer + // migration — the installer runs them with HEY_NO_KEYRING=1, which + // would otherwise move legacy tokens into plaintext. Every other + // command migrates first: anything that rewrites config.json + // (config set, trust-local, the wizard's onboarded flag) would + // silently DELETE legacy embedded credentials otherwise, because + // fileConfig has no fields to carry them through the rewrite. + if !commandSkipsCredentialMigration(cmd) { + migrateOldCredentials(configDir) + } if authMgr.IsAuthenticated() && commandUsesAccountScope(cmd) { if err := selectConfiguredAccount(cmd.Context()); err != nil { @@ -125,11 +130,21 @@ func newRootCmd() *cobra.Command { return nil }, + PersistentPostRunE: func(cmd *cobra.Command, args []string) error { + maybeRefreshSkills(cmd) + return nil + }, RunE: func(cmd *cobra.Command, args []string) error { if versionFlag { fmt.Fprintf(cmd.OutOrStdout(), "hey version %s\n", version.Version) return nil } + // First run: an interactive, logged-out bare `hey` gets the + // onboarding wizard (lite once onboarded — sign-in only). Every + // other bare `hey` prints help; the app lives at `hey tui`. + if interactiveStdio() && !machineReadableOutput(cmd) && !authMgr.IsAuthenticated() { + return runSetupWizard(cmd, wizardOptions{full: !cfg.Onboarded}) + } return cmd.Help() }, } @@ -156,6 +171,8 @@ func newRootCmd() *cobra.Command { root.SetHelpFunc(customHelpFunc(root.HelpFunc())) root.AddCommand(newAuthCommand().cmd) + root.AddCommand(newLoginCommand()) + root.AddCommand(newLogoutCommand()) root.AddCommand(newAccountsCommand().cmd) root.AddCommand(newBoxesCommand().cmd) root.AddCommand(newBoxCommand().cmd) @@ -189,7 +206,7 @@ func newRootCmd() *cobra.Command { root.AddCommand(newSpamCommand().cmd) root.AddCommand(newIgnoreCommand().cmd) root.AddCommand(newStopIgnoringCommand().cmd) - root.AddCommand(newSetupCommand()) + root.AddCommand(newSetupCommand().cmd) root.AddCommand(newOmarchyCommand().cmd) root.AddCommand(newTuiCommand().cmd) root.AddCommand(newHeyCommand().cmd) @@ -204,6 +221,24 @@ func newRootCmd() *cobra.Command { return root } +// commandSkipsCredentialMigration lists the commands safe to run without +// first migrating legacy config.json credentials: they neither use +// credentials nor rewrite the global config file. +func commandSkipsCredentialMigration(cmd *cobra.Command) bool { + parts := strings.Fields(cmd.CommandPath()) + if len(parts) < 2 { + return false + } + switch parts[1] { + case "skill": + return true + case "setup": + return len(parts) > 2 // the wizard itself persists onboarded; subcommands touch only agent files + default: + return false + } +} + func commandUsesAccountScope(cmd *cobra.Command) bool { parts := strings.Fields(cmd.CommandPath()) if len(parts) < 2 { @@ -212,7 +247,7 @@ func commandUsesAccountScope(cmd *cobra.Command) bool { switch parts[1] { // omarchy is exempt because its bar-status must never fail: it selects the // configured account itself and treats a failed selection as a dark indicator. - case "accounts", "auth", "commands", "completion", "config", "doctor", "omarchy", "setup", "skill", "upgrade", "version": + case "accounts", "auth", "commands", "completion", "config", "doctor", "login", "logout", "omarchy", "setup", "skill", "upgrade", "version": return false default: return true @@ -262,6 +297,8 @@ func validateJQFlags(cmd *cobra.Command, filter string, requested, ids, count bo return output.ErrJQNotSupported("the auth token command") case "hey completion": return output.ErrJQNotSupported("the completion command") + case "hey setup": + return output.ErrJQNotSupported("the setup wizard") case "hey skill": return output.ErrJQNotSupported("the skill display command") case "hey tui", "hey hey": @@ -271,11 +308,26 @@ func validateJQFlags(cmd *cobra.Command, filter string, requested, ids, count bo } } +// askToSignIn is the prompt requireAuth shows a logged-out user at a +// terminal. A seam so tests can answer it. +var askToSignIn = func() (bool, error) { + return tui.Confirm("Not logged in. Sign in now?", true) +} + +// requireAuth gates data commands on a login. At an interactive terminal it +// offers to sign in on the spot (prompt and OAuth progress on stderr, so +// stdout stays data); declined, piped or machine-output runs get the auth +// error and exit code 3 instead. func requireAuth() error { - if !authMgr.IsAuthenticated() { - return output.ErrAuth("not logged in — run `hey auth login` first") + if authMgr.IsAuthenticated() { + return nil + } + if writer.IsStyled() && interactiveStdio() { + if yes, err := askToSignIn(); err == nil && yes { + return loginInteractively(os.Stderr) + } } - return nil + return output.ErrAuth("Not logged in") } // migrateOldCredentials migrates credentials from the old config.json format @@ -293,7 +345,30 @@ func migrateOldCredentials(_ string) { store := authMgr.GetStore() credKey := authMgr.CredentialKey() - if _, err := store.Load(credKey); err == nil { + // The legacy fields carry their own server: migrate (and later scrub) + // only when it is the server this run is talking to. With HEY_BASE_URL + // pointed elsewhere, the current store key says nothing about the legacy + // credential — saving it there would misfile it, and scrubbing it would + // delete the only copy for its real origin. + legacyOrigin := strings.TrimRight(old.BaseURL, "/") + if legacyOrigin == "" { + legacyOrigin = strings.TrimRight(config.Defaults().BaseURL, "/") + } + if legacyOrigin != credKey { + return + } + + if existing, err := store.Load(credKey); err == nil { + // A prior run already migrated. If its scrub failed, the legacy + // fields are still in config.json (we just loaded them above) — + // retry the scrub now, but only when the stored record is actually + // usable: an empty record must never destroy the one recoverable + // copy of the credentials. + if existing.AccessToken != "" || existing.SessionCookie != "" { + if scrubErr := config.ScrubLegacyCredentials(); scrubErr != nil { + fmt.Fprintf(os.Stderr, "warning: could not remove migrated credentials from config.json: %v\n", scrubErr) + } + } return } @@ -309,10 +384,18 @@ func migrateOldCredentials(_ string) { } if err := store.Save(credKey, creds); err != nil { + // Leave the legacy fields in place: config rewrites preserve keys + // they do not own, so the credentials survive until a later run + // migrates them successfully. fmt.Fprintf(os.Stderr, "warning: could not migrate credentials: %v\n", err) return } + // Only after the store confirmed the save do the embedded secrets leave + // the config file. + if err := config.ScrubLegacyCredentials(); err != nil { + fmt.Fprintf(os.Stderr, "warning: could not remove migrated credentials from config.json: %v\n", err) + } if err := cfg.SaveBaseURL(old.BaseURL); err != nil { fmt.Fprintf(os.Stderr, "warning: could not update config after migration: %v\n", err) } diff --git a/internal/cmd/root_test.go b/internal/cmd/root_test.go new file mode 100644 index 000000000..e1a351799 --- /dev/null +++ b/internal/cmd/root_test.go @@ -0,0 +1,222 @@ +package cmd + +import ( + "bytes" + "errors" + "net/http" + "net/http/httptest" + "strings" + "testing" + + hey "github.com/basecamp/hey-sdk/go/pkg/hey" + + "github.com/basecamp/hey-cli/internal/output" + "github.com/basecamp/hey-cli/internal/tui" +) + +func stubRunTUI(t *testing.T) *int { + t.Helper() + calls := 0 + orig := runTUI + runTUI = func(*hey.Client, *hey.Client, string, tui.Watchers) error { + calls++ + return nil + } + t.Cleanup(func() { runTUI = orig }) + return &calls +} + +func stubAskToSignIn(t *testing.T, answer bool) *int { + t.Helper() + calls := 0 + orig := askToSignIn + askToSignIn = func() (bool, error) { + calls++ + return answer, nil + } + t.Cleanup(func() { askToSignIn = orig }) + return &calls +} + +func quietServer(t *testing.T) *httptest.Server { + t.Helper() + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + t.Errorf("unexpected HTTP request: %s %s", r.Method, r.URL.Path) + })) + t.Cleanup(server.Close) + return server +} + +func TestBareHeyInteractiveUnauthenticatedRunsWizard(t *testing.T) { + isolateAgents(t) + stubInteractive(t, true) + tuiCalls := stubRunTUI(t) + server := quietServer(t) + + stdout, _, err := runAuthCommand(t, t.TempDir(), server.URL, "", false) + if err != nil { + t.Fatalf("bare hey: %v", err) + } + if *tuiCalls != 0 { + t.Error("logged out: the TUI must not open") + } + // The wizard ran (envelope form, since stdout is a buffer), not help. + if !strings.Contains(stdout, `"status"`) || strings.Contains(stdout, "USAGE") { + t.Errorf("expected the wizard, got:\n%s", stdout) + } +} + +// Bare `hey` never opens the TUI — that contract moved to `hey tui` — so an +// authenticated interactive run prints help. +func TestBareHeyInteractiveAuthenticatedShowsHelp(t *testing.T) { + isolateAgents(t) + stubInteractive(t, true) + tuiCalls := stubRunTUI(t) + server := quietServer(t) + + stdout, _, err := runAuthCommand(t, t.TempDir(), server.URL, "environment-token", false) + if err != nil { + t.Fatalf("bare hey: %v", err) + } + if *tuiCalls != 0 { + t.Error("bare hey must never open the TUI") + } + if !strings.Contains(stdout, "USAGE") || !strings.Contains(stdout, "hey tui") { + t.Errorf("expected help pointing at hey tui, got:\n%s", stdout) + } +} + +// The hidden `hey hey` and `hey tui` share the TUI runner. +func TestHeyTuiOpensTUIWhenAuthenticated(t *testing.T) { + isolateAgents(t) + tuiCalls := stubRunTUI(t) + server := quietServer(t) + + for _, command := range []string{"tui", "hey"} { + if _, _, err := runAuthCommand(t, t.TempDir(), server.URL, "environment-token", false, command); err != nil { + t.Fatalf("hey %s: %v", command, err) + } + } + if *tuiCalls != 2 { + t.Errorf("TUI opened %d times, want 2", *tuiCalls) + } +} + +func TestBareHeyMachineFlagsShowHelpEvenOnTerminal(t *testing.T) { + isolateAgents(t) + stubInteractive(t, true) + tuiCalls := stubRunTUI(t) + server := quietServer(t) + + stdout, _, err := runAuthCommand(t, t.TempDir(), server.URL, "environment-token", false, "--json") + if err != nil { + t.Fatalf("bare hey --json: %v", err) + } + if *tuiCalls != 0 { + t.Error("--json must never open the TUI") + } + if !strings.Contains(stdout, "USAGE") { + t.Errorf("expected help, got:\n%s", stdout) + } +} + +func TestBareHeyNonInteractiveShowsHelp(t *testing.T) { + isolateAgents(t) + stubInteractive(t, false) + tuiCalls := stubRunTUI(t) + server := quietServer(t) + + stdout, _, err := runAuthCommand(t, t.TempDir(), server.URL, "environment-token", false) + if err != nil { + t.Fatalf("bare hey: %v", err) + } + if *tuiCalls != 0 || !strings.Contains(stdout, "USAGE") { + t.Errorf("expected help without a terminal, got tui=%d:\n%s", *tuiCalls, stdout) + } +} + +func TestRequireAuthPromptsOnlyWhenInteractiveAndStyled(t *testing.T) { + isolateAgents(t) + server := quietServer(t) + t.Setenv("HEY_TOKEN", "") + t.Setenv("HEY_NO_KEYRING", "1") + home := t.TempDir() + t.Setenv("HOME", home) + t.Setenv("XDG_CONFIG_HOME", home) + + root := newRootCmd() + root.SetOut(&bytes.Buffer{}) + root.SetArgs([]string{"--base-url", server.URL, "auth", "status"}) + if err := root.Execute(); err != nil { + t.Fatalf("prime globals: %v", err) + } + + t.Run("declined at a terminal", func(t *testing.T) { + stubInteractive(t, true) + asked := stubAskToSignIn(t, false) + prev := writer + writer = output.New(output.Options{Format: output.FormatStyled, Stdout: &bytes.Buffer{}, Stderr: &bytes.Buffer{}}) + t.Cleanup(func() { writer = prev }) + + err := requireAuth() + var cliErr *output.Error + if !errors.As(err, &cliErr) || cliErr.Code != "auth" || cliErr.Message != "Not logged in" { + t.Fatalf("error = %v, want auth/Not logged in", err) + } + if cliErr.Hint != "Run: hey auth login" { + t.Errorf("hint = %q", cliErr.Hint) + } + if output.ExitCodeFor(err) != output.ExitAuth { + t.Errorf("exit code = %d, want %d", output.ExitCodeFor(err), output.ExitAuth) + } + if *asked != 1 { + t.Errorf("prompt shown %d times, want 1", *asked) + } + }) + + t.Run("never prompts without a terminal", func(t *testing.T) { + stubInteractive(t, false) + asked := stubAskToSignIn(t, true) + prev := writer + writer = output.New(output.Options{Format: output.FormatStyled, Stdout: &bytes.Buffer{}, Stderr: &bytes.Buffer{}}) + t.Cleanup(func() { writer = prev }) + + if err := requireAuth(); err == nil { + t.Fatal("expected an auth error") + } + if *asked != 0 { + t.Errorf("prompt shown %d times without a terminal", *asked) + } + }) + + t.Run("never prompts for machine output", func(t *testing.T) { + stubInteractive(t, true) + asked := stubAskToSignIn(t, true) + prev := writer + writer = output.New(output.Options{Format: output.FormatJSON, Stdout: &bytes.Buffer{}, Stderr: &bytes.Buffer{}}) + t.Cleanup(func() { writer = prev }) + + if err := requireAuth(); err == nil { + t.Fatal("expected an auth error") + } + if *asked != 0 { + t.Errorf("prompt shown %d times for machine output", *asked) + } + }) +} + +func TestDataCommandWithoutAuthReturnsAuthErrorWhenPiped(t *testing.T) { + isolateAgents(t) + stubInteractive(t, false) + asked := stubAskToSignIn(t, true) + server := quietServer(t) + + _, _, err := runAuthCommand(t, t.TempDir(), server.URL, "", true, "boxes") + var cliErr *output.Error + if !errors.As(err, &cliErr) || cliErr.Code != "auth" { + t.Fatalf("error = %v, want auth", err) + } + if *asked != 0 { + t.Error("piped data command must not prompt") + } +} diff --git a/internal/cmd/setup.go b/internal/cmd/setup.go index 5ff2d5d95..739e0e62e 100644 --- a/internal/cmd/setup.go +++ b/internal/cmd/setup.go @@ -2,67 +2,577 @@ package cmd import ( "context" + "errors" "fmt" + "io" + "os" + "strings" "time" "github.com/spf13/cobra" + "github.com/basecamp/hey-sdk/go/pkg/generated" + "github.com/basecamp/hey-cli/internal/auth" + "github.com/basecamp/hey-cli/internal/config" + "github.com/basecamp/hey-cli/internal/harness" "github.com/basecamp/hey-cli/internal/output" + "github.com/basecamp/hey-cli/internal/tui" + "github.com/basecamp/hey-cli/internal/version" ) -func newSetupCommand() *cobra.Command { - setup := &cobra.Command{ +type setupCommand struct { + cmd *cobra.Command +} + +func newSetupCommand() *setupCommand { + setupCommand := &setupCommand{} + setupCommand.cmd = &cobra.Command{ Use: "setup", Short: "Set up HEY for first use", - Long: "Sign in and prepare HEY for first use.", + Long: "Sign in and connect your coding agents.", + Args: cobra.NoArgs, Annotations: map[string]string{ - "agent_notes": "Run this on first use. Performs OAuth login. Equivalent to hey auth login.", + "agent_notes": "Runs the first-run wizard: OAuth sign-in, a look at the linked accounts, and coding-agent setup. HEY_NONINTERACTIVE=1 suppresses OAuth and every prompt, but the wizard still connects detected agents (writing skill files) and persists the onboarded flag — there is no read-only mode; use `hey doctor` to inspect without changing anything. --json changes only the output format: a terminal on stdin (an allocated PTY included) still starts browser OAuth and waits for it. Logged out and non-interactive reports status incomplete with a remediation breadcrumb.", }, - RunE: func(cmd *cobra.Command, args []string) error { - if authMgr.IsAuthenticated() { - if writer.IsStyled() { - fmt.Fprintln(cmd.OutOrStdout(), "Already authenticated. Run `hey auth status` for details.") - return nil - } - return writeOK(map[string]string{"status": "already_authenticated"}, - output.WithSummary("Already authenticated"), - ) + RunE: setupCommand.run, + } + + for _, sub := range newSetupAgentCommands() { + setupCommand.cmd.AddCommand(sub) + } + setupCommand.cmd.AddCommand(newSetupAgentsCommand()) + setupCommand.cmd.AddCommand(newSetupOmarchyCommand().cmd) + + return setupCommand +} + +func (c *setupCommand) run(cmd *cobra.Command, _ []string) error { + if err := rejectListOnlyFormats("the setup wizard"); err != nil { + return err + } + return runSetupWizard(cmd, wizardOptions{full: true}) +} + +// rejectListOnlyFormats fails fast on --ids-only and --count: setup results +// are not list data, and the writer's late refusal would otherwise land +// after OAuth ran, agents were connected and onboarded was persisted — +// side effects dressed up as a failed command. +func rejectListOnlyFormats(command string) error { + switch writer.EffectiveFormat() { + case output.FormatIDs: + return output.ErrUsage("--ids-only is not supported by " + command) + case output.FormatCount: + return output.ErrUsage("--count is not supported by " + command) + default: + return nil + } +} + +// wizardOptions tunes the first-run wizard. full runs every step; a lite run +// (a later logged-out bare `hey` once onboarded) only signs in. +type wizardOptions struct { + full bool +} + +// wizardIdentity is the signed-in HEY identity reported in the envelope. +type wizardIdentity struct { + Name string `json:"name,omitempty"` + Email string `json:"email,omitempty"` +} + +// wizardResult is the wizard's outcome, rendered as prose in a terminal and +// as the envelope otherwise. +type wizardResult struct { + Version string `json:"version"` + Status string `json:"status"` // "complete" or "incomplete" + Identity *wizardIdentity `json:"identity,omitempty"` + Accounts []accountListItem `json:"accounts,omitempty"` + SkillInstalled bool `json:"skill_installed"` + Agents []agentCheck `json:"agents"` + Issues []agentIssue `json:"issues"` +} + +// setupWizard carries one wizard run: the command it prints through and what +// it has learned so far. +type setupWizard struct { + cmd *cobra.Command + opts wizardOptions + styled bool + result wizardResult + outcome agentSetupOutcome +} + +// confirmAgentSetup is the wizard's one prompt, a seam so tests can answer it. +var confirmAgentSetup = func() (bool, error) { + return tui.Confirm(" Set up HEY for your coding agents?", true) +} + +// runSetupWizard is the entry point shared by `hey setup` and bare `hey`. +func runSetupWizard(cmd *cobra.Command, opts wizardOptions) error { + wizard := &setupWizard{ + cmd: cmd, + opts: opts, + styled: writer.IsStyled(), + result: wizardResult{Version: version.Version, Status: "complete"}, + } + return wizard.run() +} + +func (s *setupWizard) run() error { + if s.styled { + s.welcome(s.cmd.OutOrStdout()) + } + + signedIn, err := s.signIn() + if err != nil { + return err + } + if signedIn { + s.greet() + } else { + s.result.Status = "incomplete" + s.result.Issues = append(s.result.Issues, agentIssue{Check: "Not logged in", Hint: "Run: hey auth login"}) + } + + if s.opts.full { + s.outcome = s.setupAgents() + } + s.result.SkillInstalled = baselineSkillInstalled() + s.result.Agents = s.outcome.Checks + s.result.Issues = append(s.result.Issues, s.outcome.Issues...) + if statusFromOutcome(s.outcome) == "incomplete" { + s.result.Status = "incomplete" + } + + s.persistOnboarded() + return s.summary() +} + +// welcome prints the wordmark and a short intro (styled runs only). +func (s *setupWizard) welcome(w io.Writer) { + fmt.Fprintln(w, tui.RenderWordmark(!colorDisabled)) + fmt.Fprintln(w) + fmt.Fprintln(w, bold.format("Welcome to HEY")) + fmt.Fprintln(w) + fmt.Fprintf(w, "The command-line interface for HEY (v%s).\n", version.Version) + fmt.Fprintln(w, "Let's get you set up. This will only take a moment.") + fmt.Fprintln(w) +} + +// signIn makes sure we are authenticated. Reports whether we are. Sign-in +// runs only when somebody can see it through: stdin is a terminal and +// HEY_NONINTERACTIVE is not engaged. Otherwise — a piped `hey setup --json`, +// an agent harness on a pseudo-terminal — it reports "not logged in" instead +// of parking a six-minute browser wait. +func (s *setupWizard) signIn() (bool, error) { + if authMgr.IsAuthenticated() { + return true, nil + } + if !canSignInInteractively() { + return false, nil + } + + if s.styled { + w := s.cmd.OutOrStdout() + fmt.Fprintln(w, bold.format(" Step 1: Sign in")) + fmt.Fprintln(w) + if err := loginInteractively(w); err != nil { + return false, err + } + fmt.Fprintln(w) + return true, nil + } + + if err := loginInteractively(s.cmd.ErrOrStderr()); err != nil { + return false, err + } + return true, nil +} + +// canSignInInteractively reports whether the OAuth flow may start: a human on +// stdin, prompts not disabled. Deliberately looser than interactiveStdio() — +// `hey setup --json` from a terminal still signs in, with progress on stderr. +func canSignInInteractively() bool { + return stdinIsTerminal() && !config.NonInteractiveEnv() +} + +// loginInteractively runs the OAuth flow with progress on out, then selects +// the configured mail account (PersistentPreRunE skipped that while we were +// logged out). Shared with requireAuth's sign-in prompt. +func loginInteractively(out io.Writer) error { + ctx, cancel := context.WithTimeout(context.Background(), 6*time.Minute) + defer cancel() + + logger := func(msg string) { + for _, line := range strings.Split(strings.Trim(msg, "\n"), "\n") { + if line == "" { + fmt.Fprintln(out) + } else { + fmt.Fprintln(out, " "+line) } + } + } + if err := authMgr.Login(ctx, auth.LoginOptions{Logger: logger}); err != nil { + return output.ErrAuth(fmt.Sprintf("login failed: %v", err)) + } + return selectConfiguredAccount(context.Background()) +} - if writer.IsStyled() { - w := cmd.OutOrStdout() - fmt.Fprintln(w, "Welcome to hey CLI!") - fmt.Fprintln(w) - fmt.Fprintln(w, "Let's get you logged in...") +// greet looks up who signed in and shows the linked accounts. Read-only: +// HEY is natively multi-account and "all" is the default, so nothing is +// chosen or persisted here. +func (s *setupWizard) greet() { + w := s.cmd.OutOrStdout() + + identity, err := rootSDK.Identity().GetIdentity(s.cmd.Context()) + if err != nil || identity == nil { + // Stored credentials that HEY rejects mean we are not signed in at + // all, however present they are — surface that instead of reporting + // a complete setup that cannot run a single data command. Other + // failures (offline, transient) keep the best-effort greeting. + if isAuthError(err) { + // The remediation must match the credential actually in use: + // HEY_TOKEN outranks anything hey auth login saves, so sending + // the user there would repeat the same failure forever. + issue := agentIssue{Check: "Stored sign-in rejected", Hint: "Run: hey auth login"} + if os.Getenv("HEY_TOKEN") != "" { + issue = agentIssue{Check: "HEY_TOKEN rejected", Hint: "Update or unset HEY_TOKEN"} + } + if s.styled { + fmt.Fprintln(w, warning.format(" "+issue.Check+" by HEY — "+issue.Hint)) fmt.Fprintln(w) } + s.result.Status = "incomplete" + s.result.Issues = append(s.result.Issues, issue) + return + } + if s.styled { + fmt.Fprintln(w, success.format(" Signed in.")) + fmt.Fprintln(w) + } + return + } + + s.result.Identity = &wizardIdentity{Name: identity.Name, Email: identity.PrimaryContact.EmailAddress} + s.result.Accounts = linkedAccountList(identity, cfg.AccountID) + + if !s.styled { + return + } + fmt.Fprintln(w, success.format(" "+identityGreeting(identity))) + // accounts[0] is the "All Accounts" filter; a single linked account + // needs no list. + if len(s.result.Accounts) > 2 { + for _, account := range s.result.Accounts[1:] { + label := terminalSafeText(account.Name) + if account.Email != "" { + label += " (" + terminalSafeText(account.Email) + ")" + } + fmt.Fprintln(w, muted.format(" • "+label)) + } + if cfg.AccountID == config.AllAccounts { + fmt.Fprintln(w, muted.format(" Using All Accounts — hey accounts use to default to one")) + } else { + fmt.Fprintln(w, muted.format(" Default mail account: "+cfg.AccountID)) + } + } + fmt.Fprintln(w) +} + +// isAuthError reports whether err is HEY rejecting our credentials. +func isAuthError(err error) bool { + if err == nil { + return false + } + converted := convertSDKError(err) + var cliErr *output.Error + return errors.As(converted, &cliErr) && cliErr.Code == "auth" +} + +func identityGreeting(identity *generated.Identity) string { + name := terminalSafeText(identity.Name) + email := terminalSafeText(identity.PrimaryContact.EmailAddress) + switch { + case name != "" && email != "": + return fmt.Sprintf("Signed in as %s (%s)", name, email) + case email != "": + return "Signed in as " + email + case name != "": + return "Signed in as " + name + default: + return "Signed in." + } +} + +// setupAgents offers to connect detected coding agents. In a styled run the +// user confirms first; a machine run just does it — there is nobody to ask. +func (s *setupWizard) setupAgents() agentSetupOutcome { + agents := harness.DetectedAgents() + if len(agents) == 0 { + return agentSetupOutcome{} + } + + w := s.cmd.OutOrStdout() + + // One pre-setup snapshot drives both the all-good gate and the checklist + // rendered in the summary for the paths that do not run setup. + preChecks := snapshotAgentChecks(agents) + if baselineSkillInstalled() && len(issuesFromChecks(preChecks)) == 0 { + if s.styled { + for _, a := range agents { + fmt.Fprintln(w, statusLine(true, a.Name+" connected")) + } + fmt.Fprintln(w) + } + return agentSetupOutcome{Checks: preChecks} + } - ctx, cancel := context.WithTimeout(context.Background(), 6*time.Minute) - defer cancel() + if s.styled { + fmt.Fprintln(w, bold.format(" Step 2: Coding agents")) + fmt.Fprintln(w) - if err := authMgr.Login(ctx, auth.LoginOptions{}); err != nil { - return output.ErrAuth(fmt.Sprintf("login failed: %v", err)) + var names []string + for _, a := range agents { + names = append(names, a.Name) + } + fmt.Fprintf(w, " Detected: %s\n", joinNames(names)) + fmt.Fprintln(w) + fmt.Fprintln(w, " This will:") + step := 1 + fmt.Fprintln(w, muted.format(fmt.Sprintf(" %d. Install the HEY agent skill to ~/.agents/skills/hey/", step))) + step++ + for _, a := range agents { + handler, ok := agentSetupHandlers[a.ID] + if !ok { + continue + } + for _, label := range handler.Labels { + fmt.Fprintln(w, muted.format(fmt.Sprintf(" %d. %s", step, label))) + step++ } + } + fmt.Fprintln(w) - if writer.IsStyled() { - w := cmd.OutOrStdout() - fmt.Fprintln(w, "Setup complete! You're ready to use hey.") + // The prompt runs only when it can be answered: styled output alone + // does not prove a human (HEY_NONINTERACTIVE on a PTY, --styled while + // piped). Without one, proceed with the prompt's default answer — + // exactly what the machine-mode wizard does. + if interactiveStdio() { + install, confirmErr := confirmAgentSetup() + if confirmErr != nil || !install { + fmt.Fprintln(w, muted.format(" You can set up agents later:")) + for _, a := range agents { + if _, ok := agentSetupHandlers[a.ID]; ok { + fmt.Fprintln(w, bold.format(" hey setup "+a.ID)) + } + } fmt.Fprintln(w) - fmt.Fprintln(w, "Try: hey boxes") - return nil + // Skipped carries the snapshot for the checklist but records no + // issues, so a deliberate skip stays "complete". + return agentSetupOutcome{Skipped: true, Checks: preChecks} } + } + fmt.Fprintln(w) + } - return writeOK(map[string]string{"status": "setup_complete"}, - output.WithSummary("Setup complete"), - output.WithBreadcrumbs(output.Breadcrumb{ - Action: "start", - Command: "hey boxes", - Description: "List your mailboxes", - }), - ) - }, + var issues []agentIssue + if _, err := installSkillFiles(); err != nil { + if s.styled { + fmt.Fprintln(w, warning.format(fmt.Sprintf(" Skill install failed: %s", err))) + } + issues = append(issues, agentIssue{Check: "Agent skill", Hint: "Run: hey skill install"}) + } else if s.styled { + fmt.Fprintln(w, statusLine(true, "Agent skill installed")) + } + + for _, a := range agents { + handler, ok := agentSetupHandlers[a.ID] + if !ok { + continue + } + var handlerErr error + if s.styled { + if handler.Run != nil { + handlerErr = handler.Run(s.cmd) // interactive handlers warn and continue + } + } else if handler.RunNonInteractive != nil { + handlerErr = handler.RunNonInteractive(s.cmd) + } + // A handler failure is an issue in its own right: the snapshot below + // catches most of them, but not every refusal leaves a failing check. + if handlerErr != nil { + issues = append(issues, agentIssue{Agent: a.Name, Check: a.Name + " setup failed", Hint: handlerErr.Error()}) + } + } + + // Re-snapshot after setup ran so failed installs surface as issues rather + // than a silent "complete". The same snapshot renders the checklist, so + // status and checklist can never disagree. + postChecks := snapshotAgentChecks(agents) + issues = append(issues, issuesFromChecks(postChecks)...) + + if s.styled { + fmt.Fprintln(w) + } + return agentSetupOutcome{Checks: postChecks, Issues: issues} +} + +// persistOnboarded records that the wizard ran, so a later logged-out bare +// `hey` runs the lite wizard. Best-effort: a read-only config dir only costs +// a repeat of the agent step next time. +func (s *setupWizard) persistOnboarded() { + if cfg.Onboarded { + return + } + if err := cfg.SaveOnboarded(true); err != nil { + fmt.Fprintf(s.cmd.ErrOrStderr(), "warning: could not save onboarding state: %v\n", err) + } +} + +// summary closes the wizard: a checklist and next steps in a terminal, the +// envelope otherwise. +func (s *setupWizard) summary() error { + if s.styled { + showWizardSuccess(s.cmd.OutOrStdout(), s.result, s.outcome) + return nil + } + if s.result.Agents == nil { + s.result.Agents = []agentCheck{} + } + if s.result.Issues == nil { + s.result.Issues = []agentIssue{} + } + return writeOK(s.result, + output.WithSummary(wizardSummaryLine(s.result)), + output.WithBreadcrumbs(wizardBreadcrumbs(s.result)...), + ) +} + +// successHeadline returns the completion banner. When a step left unresolved +// issues the banner is honest about it rather than claiming "Setup complete!". +func successHeadline(status string, issueCount int) string { + if status != "incomplete" { + return "Setup complete!" + } + if issueCount == 1 { + return "Setup finished — 1 step needs attention" + } + return fmt.Sprintf("Setup finished — %d steps need attention", issueCount) +} + +// showWizardSuccess renders the completion checklist, remediation for +// anything that did not complete, and example commands. +func showWizardSuccess(w io.Writer, result wizardResult, outcome agentSetupOutcome) { + divider := muted.format("─────────────────────────────────") + + headline := success + if result.Status == "incomplete" { + headline = warning + } + + fmt.Fprintln(w, divider) + fmt.Fprintln(w, headline.format(" "+successHeadline(result.Status, len(result.Issues)))) + fmt.Fprintln(w, divider) + fmt.Fprintln(w) + + fmt.Fprintln(w, statusLine(!hasAuthIssue(result.Issues), "Signed in")) + if outcome.Skipped { + fmt.Fprintln(w, muted.format(" Coding agent setup skipped — run: hey setup")) + } else { + for _, check := range outcome.Checks { + fmt.Fprintln(w, statusLine(check.Status == "pass", check.Name)) + } + } + fmt.Fprintln(w) + + if len(result.Issues) > 0 { + fmt.Fprintln(w, " Some steps need attention:") + for _, issue := range result.Issues { + // Check names usually already carry the agent (e.g. "Claude Code + // Plugin"); only prefix when they don't. + label := issue.Check + if issue.Agent != "" && !strings.HasPrefix(issue.Check, issue.Agent) { + label = issue.Agent + " — " + issue.Check + } + line := " " + label + if issue.Hint != "" { + line += ": " + issue.Hint + } + fmt.Fprintln(w, warning.format(line)) + } + fmt.Fprintln(w, muted.format(" Then verify with: hey doctor")) + fmt.Fprintln(w) + } + + fmt.Fprintln(w, " Try these commands:") + fmt.Fprintln(w) + examples := []struct{ cmd, desc string }{ + {"hey tui", "Open the app"}, + {"hey boxes", "List your boxes"}, + {"hey box imbox", "Read your Imbox"}, + {`hey search "quarterly planning"`, "Search your mail"}, + } + width := 0 + for _, ex := range examples { + width = max(width, len(ex.cmd)) + } + for _, ex := range examples { + fmt.Fprintf(w, " %s%s %s\n", bold.format(ex.cmd), strings.Repeat(" ", width-len(ex.cmd)), muted.format(ex.desc)) + } + fmt.Fprintln(w) +} + +func hasIssue(issues []agentIssue, check string) bool { + for _, issue := range issues { + if issue.Check == check { + return true + } + } + return false +} + +// hasAuthIssue reports whether the wizard could not establish a working +// login — missing or rejected. Both mean the same thing to a caller: repair +// authentication before anything else. +func hasAuthIssue(issues []agentIssue) bool { + return hasIssue(issues, "Not logged in") || hasIssue(issues, "Stored sign-in rejected") || hasIssue(issues, "HEY_TOKEN rejected") +} + +// wizardSummaryLine builds a concise summary for the output envelope. +func wizardSummaryLine(result wizardResult) string { + headline := "Setup complete" + if result.Status == "incomplete" { + headline = "Setup finished with issues" + } + if result.Identity != nil && result.Identity.Email != "" { + return fmt.Sprintf("%s - %s", headline, result.Identity.Email) + } + return headline +} + +// wizardBreadcrumbs returns next-step breadcrumbs based on wizard outcome. +func wizardBreadcrumbs(result wizardResult) []output.Breadcrumb { + if hasIssue(result.Issues, "HEY_TOKEN rejected") { + // HEY_TOKEN outranks anything hey auth login saves: the structured + // remediation must point at the environment or it loops forever. + return []output.Breadcrumb{ + {Action: "fix_token", Command: "unset HEY_TOKEN", Description: "Remove or replace the rejected environment token"}, + {Action: "doctor", Command: "hey doctor", Description: "Check CLI health"}, + } + } + if hasAuthIssue(result.Issues) { + return []output.Breadcrumb{ + {Action: "login", Command: "hey auth login", Description: "Authenticate with HEY"}, + {Action: "doctor", Command: "hey doctor", Description: "Check CLI health"}, + } + } + crumbs := []output.Breadcrumb{ + {Action: "open", Command: "hey tui", Description: "Open the app"}, + {Action: "boxes", Command: "hey boxes", Description: "List your boxes"}, + } + if result.Status == "incomplete" { + crumbs = append(crumbs, output.Breadcrumb{Action: "doctor", Command: "hey doctor", Description: "Check CLI health"}) } - setup.AddCommand(newSetupOmarchyCommand().cmd) - return setup + return crumbs } diff --git a/internal/cmd/setup_agent.go b/internal/cmd/setup_agent.go new file mode 100644 index 000000000..35a16b28f --- /dev/null +++ b/internal/cmd/setup_agent.go @@ -0,0 +1,453 @@ +package cmd + +import ( + "context" + "errors" + "fmt" + "os/exec" + "strings" + "time" + + "github.com/spf13/cobra" + + "github.com/basecamp/hey-cli/internal/harness" + "github.com/basecamp/hey-cli/internal/output" +) + +// agentSetupHandler describes what a single agent's setup step does and how to run it. +type agentSetupHandler struct { + Labels []string // what this will do, for the wizard's "This will:" list + Run func(*cobra.Command) error // interactive setup: prints progress, warns and continues + RunNonInteractive func(*cobra.Command) error // non-interactive setup: silent, returns the failure +} + +// agentSetupError is a setup failure with manual remediation commands the +// user (or agent) can run themselves. +type agentSetupError struct { + Summary string + Manual []string +} + +func (e *agentSetupError) Error() string { return e.Summary } + +// agentCheck is one agent health check captured in a single post-setup +// snapshot. Both the completion status and the rendered checklist derive +// from the same snapshot so they can never disagree. +type agentCheck struct { + Agent string `json:"agent"` + Name string `json:"name"` + Status string `json:"status"` + Hint string `json:"hint,omitempty"` +} + +// agentIssue is a single unresolved problem after agent setup ran. Hint +// carries the failing check's own remediation so reporting stays specific. +type agentIssue struct { + Agent string `json:"agent,omitempty"` + Check string `json:"check"` + Hint string `json:"hint,omitempty"` +} + +// agentSetupOutcome reports what happened during the agent-setup step. +// Issues are authoritative for the wizard's completion status; Skipped is +// metadata only (a deliberate skip records no issues, so it stays complete). +type agentSetupOutcome struct { + Skipped bool + Checks []agentCheck + 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{ + "Copy the HEY skill into Codex's skills directory", + }, + Run: runCodexSetup, + RunNonInteractive: runCodexSetupNonInteractive, + }, +} + +// 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) { + command := exec.CommandContext(ctx, name, args...) // #nosec G204 -- name comes from harness.Find*Binary + // Bound Wait, not just the process: a wrapper script's grandchild can hold + // the output pipes open after the timeout kills the child. + command.WaitDelay = time.Second + return command.CombinedOutput() +} + +const ( + // claudeMarketplaceTimeout bounds marketplace add/update so a hung clone + // can't stall setup indefinitely. + claudeMarketplaceTimeout = 60 * time.Second + claudeInstallTimeout = 120 * time.Second +) + +// newSetupAgentCommands generates `hey setup ` from the registry. +func newSetupAgentCommands() []*cobra.Command { + var cmds []*cobra.Command + for _, a := range harness.AllAgents() { + agent := a + handler, ok := agentSetupHandlers[agent.ID] + if !ok { + continue + } + cmds = append(cmds, &cobra.Command{ + Use: agent.ID, + Short: fmt.Sprintf("Connect %s to HEY", agent.Name), + Long: fmt.Sprintf("Install the HEY agent skill and set up the %s integration so %s can work with your HEY mail.", agent.Name, agent.Name), + Args: cobra.NoArgs, + Annotations: map[string]string{ + "agent_notes": "Non-interactive when output is piped or --json is set. Succeeds with {plugin_installed, agent_detected}; a not-detected or not-connected outcome is an error envelope (code setup_incomplete, manual steps in the hint) with a nonzero exit.", + }, + RunE: func(cmd *cobra.Command, _ []string) error { + return runSetupAgent(cmd, agent, handler) + }, + }) + } + return cmds +} + +func runSetupAgent(cmd *cobra.Command, agent harness.AgentInfo, handler agentSetupHandler) error { + if err := rejectListOnlyFormats("hey setup " + agent.ID); err != nil { + return err + } + _, skillErr := installSkillFiles() + + var setupErrors, manualCommands []string + if skillErr != nil { + setupErrors = append(setupErrors, fmt.Sprintf("skill install: %s", skillErr)) + } + + if writer.IsStyled() { + w := cmd.OutOrStdout() + if skillErr != nil { + fmt.Fprintln(w, warning.format(fmt.Sprintf(" Skill install failed: %s", skillErr))) + } else { + fmt.Fprintln(w, statusLine(true, "Agent skill installed")) + } + if handler.Run != nil { + if err := handler.Run(cmd); err != nil { + return err + } + } + // Interactive handlers warn and continue (the wizard needs that), so + // the verdict comes from a fresh health snapshot, not from the + // handler's return value: never tell the user to start a session + // against an integration that is not connected. + if skillErr == nil && agent.Detect != nil && agent.Detect() && agentChecksPass(agent) { + fmt.Fprintln(w, muted.format(" Start a new "+agent.Name+" session to use HEY commands.")) + return nil + } + return &output.Error{ + Code: "setup_incomplete", + Message: agent.Name + " not connected", + Hint: "Run: hey doctor", + } + } + + if handler.RunNonInteractive != nil { + if err := handler.RunNonInteractive(cmd); err != nil { + setupErrors = append(setupErrors, err.Error()) + var setupErr *agentSetupError + if errors.As(err, &setupErr) { + manualCommands = setupErr.Manual + } + } + } + + detected := agent.Detect != nil && agent.Detect() + installed := detected && agentChecksPass(agent) + + summary := agent.Name + " connected" + switch { + case !detected: + summary = agent.Name + " not detected" + case !installed: + summary = agent.Name + " not connected" + } + + result := map[string]any{ + "plugin_installed": installed, + "agent_detected": detected, + } + if len(setupErrors) > 0 { + result["errors"] = setupErrors + // Setup errors mean we cannot claim success even if checks pass. + if installed { + result["plugin_installed"] = false + summary = agent.Name + " not connected" + } + } + if len(manualCommands) > 0 { + result["manual_commands"] = manualCommands + } + + // An explicitly requested integration that did not connect is a failed + // command: automation keys off the exit status, not just the fields. + // The aggregate `setup agents` keeps its own in-band contract — its core + // outcome is the skill, and per-agent detail rides in the envelope. + if !installed || len(setupErrors) > 0 { + hint := "Run: hey doctor" + if len(manualCommands) > 0 { + hint = strings.Join(manualCommands, "; ") + } + return &output.Error{Code: "setup_incomplete", Message: summary, Hint: hint} + } + + breadcrumbs := []output.Breadcrumb{ + {Action: "doctor", Command: "hey doctor", Description: "Check CLI health"}, + } + for i, manual := range manualCommands { + breadcrumbs = append(breadcrumbs, output.Breadcrumb{ + Action: fmt.Sprintf("manual_step_%d", i+1), + Command: manual, + Description: "Manual setup step", + }) + } + + return writeOK(result, + output.WithSummary(summary), + output.WithBreadcrumbs(breadcrumbs...), + ) +} + +// --- Claude Code --- + +// runClaudeSetup performs the Claude Code setup for the interactive wizard. +// Failures warn and continue — the post-setup snapshot turns them into issues. +func runClaudeSetup(cmd *cobra.Command) error { + w := cmd.OutOrStdout() + progress := func(message string) { + fmt.Fprintln(w, muted.format(" "+message)) + } + + if err := installClaudePlugin(cmd.Context(), progress); err != nil { + fmt.Fprintln(w, warning.format(" Claude Code setup failed: "+err.Error())) + var setupErr *agentSetupError + if errors.As(err, &setupErr) && len(setupErr.Manual) > 0 { + fmt.Fprintln(w, muted.format(" Try manually:")) + for _, manual := range setupErr.Manual { + fmt.Fprintln(w, bold.format(" "+manual)) + } + } + fmt.Fprintln(w, muted.format(" Then verify with: hey doctor")) + return nil + } + + fmt.Fprintln(w, statusLine(true, "Claude Code plugin installed")) + fmt.Fprintln(w, statusLine(true, "Claude Code skill linked")) + fmt.Fprintln(w) + fmt.Fprintln(w, muted.format(" Tip: enable auto-update to stay current with new CLI releases:")) + fmt.Fprintln(w, muted.format(" "+harness.AutoUpdateHint)) + return nil +} + +func runClaudeSetupNonInteractive(cmd *cobra.Command) error { + return installClaudePlugin(cmd.Context(), func(string) {}) +} + +// installClaudePlugin registers the 37signals marketplace, installs the hey +// plugin and links the skill. The marketplace refresh matters: `marketplace +// add` no-ops on an already-registered marketplace, so a stale cached entry +// would otherwise strand `plugin install` on an old source. +func installClaudePlugin(parent context.Context, progress func(string)) error { + // Never fabricate ~/.claude: linking the skill on a machine without Claude + // Code would make every later detection report it installed. + if !harness.DetectClaude() { + return &agentSetupError{ + Summary: "Claude Code not detected — install Claude Code, then run: hey setup claude", + Manual: claudeManualCommands(), + } + } + + // The skill link first: it needs no binary, and a repaired link is worth + // having even when the plugin install cannot run. + var linkErr error + if _, err := linkSkillToClaude(); err != nil { + linkErr = fmt.Errorf("skill link: %w", err) + } + + if harness.CheckClaudePlugin().Status != "pass" { + claudePath := harness.FindClaudeBinary() + if claudePath == "" { + return &agentSetupError{ + Summary: "Claude Code binary not found — install the plugin from inside Claude Code", + Manual: claudeManualCommands(), + } + } + + progress("Registering " + harness.ClaudeMarketplaceName + " marketplace…") + // Best-effort: already registered is the common case and not an error. + _, _ = runClaudeStep(parent, claudeMarketplaceTimeout, claudePath, "plugin", "marketplace", "add", harness.ClaudeMarketplaceSource) + progress("Refreshing " + harness.ClaudeMarketplaceName + " marketplace…") + _, _ = runClaudeStep(parent, claudeMarketplaceTimeout, claudePath, "plugin", "marketplace", "update", harness.ClaudeMarketplaceName) + + progress("Installing " + harness.ClaudeExpectedPluginKey + " plugin…") + out, err := runClaudeStep(parent, claudeInstallTimeout, claudePath, "plugin", "install", harness.ClaudeExpectedPluginKey) + if err != nil { + return claudeSetupError("plugin install failed: " + agentCommandFailure(out, err)) + } + if harness.CheckClaudePlugin().Status != "pass" { + return claudeSetupError("plugin install did not register " + harness.ClaudeExpectedPluginKey) + } + } + + return linkErr +} + +func runClaudeStep(parent context.Context, timeout time.Duration, path string, args ...string) ([]byte, error) { + ctx, cancel := context.WithTimeout(parent, timeout) + defer cancel() + return runAgentCommand(ctx, path, args...) +} + +func claudeSetupError(summary string) *agentSetupError { + return &agentSetupError{Summary: summary, Manual: claudeManualCommands()} +} + +// claudeManualCommands are the manual install steps. The `marketplace +// update` step is essential: `marketplace add` no-ops on an already- +// registered marketplace, so without it a stale entry would survive. +func claudeManualCommands() []string { + return []string{ + "claude plugin marketplace add " + harness.ClaudeMarketplaceSource, + "claude plugin marketplace update " + harness.ClaudeMarketplaceName, + "claude plugin install " + harness.ClaudeExpectedPluginKey, + } +} + +func agentCommandFailure(out []byte, err error) string { + message := strings.TrimSpace(string(out)) + if len(message) > 500 { + message = message[:500] + } + if message == "" { + return err.Error() + } + return message +} + +// --- Codex --- + +// runCodexSetup copies the skill for Codex in the interactive wizard. +// hey has no Codex plugin yet, so the skill is the whole integration. +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, muted.format(" Then verify with: hey doctor")) + return nil //nolint:nilerr // warn and continue; the post-setup snapshot reports the failure + } + 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. Like Claude, it never +// fabricates the agent: creating ~/.codex on a machine without Codex would +// make every later detection — and this command's own verdict — report it +// installed. +func installCodexSkill() (string, error) { + if !harness.DetectCodex() { + return "", &agentSetupError{ + Summary: "Codex not detected — install Codex, then run: hey setup codex", + Manual: []string{"hey setup codex"}, + } + } + return installSkillToCodex() +} + +// --- Shared helpers --- + +// statusLine renders a ✓/✗ checklist line. +func statusLine(ok bool, label string) string { + if ok { + return success.format(" ✓ " + label) + } + return warning.format(" ✗ " + label) +} + +// snapshotAgentChecks captures every check across the given agents in one pass. +func snapshotAgentChecks(agents []harness.AgentInfo) []agentCheck { + var out []agentCheck + for _, a := range agents { + if a.Checks == nil { + continue + } + for _, c := range a.Checks() { + out = append(out, agentCheck{Agent: a.Name, Name: c.Name, Status: c.Status, Hint: c.Hint}) + } + } + return out +} + +// issuesFromChecks returns one issue per non-passing check in the snapshot. +func issuesFromChecks(checks []agentCheck) []agentIssue { + var issues []agentIssue + for _, c := range checks { + if c.Status != "pass" { + issues = append(issues, agentIssue{Agent: c.Agent, Check: c.Name, Hint: c.Hint}) + } + } + return issues +} + +// statusFromOutcome maps an agent-setup outcome to a wizard status. Issues +// are authoritative: any observed failure means "incomplete", and Skipped +// never suppresses one. A deliberate skip records no issues, so it stays +// "complete". +func statusFromOutcome(o agentSetupOutcome) string { + if len(o.Issues) > 0 { + return "incomplete" + } + return "complete" +} + +// agentChecksPass reports whether every health check for the agent passes. +func agentChecksPass(agent harness.AgentInfo) bool { + if agent.Checks == nil { + return false + } + checks := agent.Checks() + if len(checks) == 0 { + return false + } + for _, c := range checks { + if c.Status != "pass" { + return false + } + } + return true +} + +// joinNames joins names with commas and "and". +func joinNames(names []string) string { + switch len(names) { + case 0: + return "" + case 1: + return names[0] + case 2: + return names[0] + " and " + names[1] + default: + return strings.Join(names[:len(names)-1], ", ") + ", and " + names[len(names)-1] + } +} diff --git a/internal/cmd/setup_agents.go b/internal/cmd/setup_agents.go new file mode 100644 index 000000000..04e4c83d4 --- /dev/null +++ b/internal/cmd/setup_agents.go @@ -0,0 +1,321 @@ +package cmd + +import ( + "errors" + "fmt" + "os" + "sort" + "strings" + + "github.com/spf13/cobra" + + "github.com/basecamp/hey-cli/internal/harness" + "github.com/basecamp/hey-cli/internal/output" +) + +// agentSetupEnv selects which coding agents `setup agents` targets. +// Values: claude | codex | all | none. Empty (unset) means auto-detect. +const agentSetupEnv = "HEY_SETUP_AGENT" + +// 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 +// envelope. It never prompts, so it is safe for the piped installer and +// coding-agent shells. +func newSetupAgentsCommand() *cobra.Command { + return &cobra.Command{ + Use: "agents", + Short: "Install the HEY skill and connect detected coding agents", + 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" + + "unset, a single detected agent is connected; when several are detected none is\n" + + "guessed — the per-agent `hey setup ` commands are surfaced instead.", + // Selection is env-driven; positional args are always a mistake (typo, + // 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.", + }, + RunE: func(cmd *cobra.Command, _ []string) error { + return runNonInteractiveAgentSetup(cmd) + }, + } +} + +// agentSetupRecord is the per-agent outcome captured while running handlers. +// errors/manualCommands are stored bare (not id-prefixed); the top-level union +// prefixes errors with the agent id. +type agentSetupRecord struct { + id, name string + detectedBefore bool + detectedAfter bool + pluginInstalled bool + binaryAbsent bool + errors []string + manualCommands []string +} + +// runNonInteractiveAgentSetup installs the baseline skill, resolves the agent +// selector, runs each targeted handler, and returns a structured envelope +// where every safety-critical outcome lives in a top-level flat field (the +// `agents` array is per-agent detail). +func runNonInteractiveAgentSetup(cmd *cobra.Command) error { + if err := rejectListOnlyFormats("hey setup agents"); err != nil { + return err + } + // Baseline skill: installed regardless of selector. + _, skillErr := installSkillFiles() + skillInstalled := skillErr == nil + + detectedBefore := detectedAgentIDs() + + selectorRaw := strings.TrimSpace(os.Getenv(agentSetupEnv)) + selector := strings.ToLower(selectorRaw) + + var warnings []string + var ambiguous bool + var ambiguousManual []string + var targets []harness.AgentInfo + + switch selector { + case "", "auto": + selector = "auto" + detected := harness.DetectedAgents() + switch len(detected) { + case 0: + // baseline skill only + case 1: + targets = detected + default: + ambiguous = true + ambiguousManual = agentChoiceCommands(detected) + warnings = append(warnings, "Multiple coding agents detected; installed the baseline skill only. Choose one: "+strings.Join(ambiguousManual, ", ")) + } + case "all": + targets = harness.AllAgents() + case "none": + // baseline skill only + case "claude", "codex": + 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)) + } + + // Run handlers in id order so aggregation is deterministic. + sort.Slice(targets, func(i, j int) bool { return targets[i].ID < targets[j].ID }) + records := make([]agentSetupRecord, 0, len(targets)) + for _, agent := range targets { + records = append(records, runAgentSetupHandler(cmd, agent)) + } + + attempted := make([]string, 0, len(records)) + for _, r := range records { + attempted = append(attempted, r.id) + } + + // errors: union of baseline + every per-agent error (id-prefixed), stable + // first-seen dedup, in sorted-agent order. + errUnion := newOrderedStringSet() + if skillErr != nil { + errUnion.add(fmt.Sprintf("skill: %s", skillErr)) + } + for _, r := range records { + for _, e := range r.errors { + errUnion.add(fmt.Sprintf("%s: %s", r.id, e)) + } + } + + // manual_commands: ambiguous → every `setup `; else union of each + // handler's own ordered sequence plus a synthesized hint for absent + // binaries. + manualUnion := newOrderedStringSet() + if ambiguous { + for _, m := range ambiguousManual { + manualUnion.add(m) + } + } else { + for _, r := range records { + for _, m := range r.manualCommands { + manualUnion.add(m) + } + if r.binaryAbsent && !r.pluginInstalled { + manualUnion.add("hey setup " + r.id) + } + } + } + + // warnings: synthesized missing-binary remediation, sorted-agent order. + // Only when the absence actually prevented the connection — Codex is + // routinely detected by its home directory alone and its skill-only + // setup succeeds without a binary. + for _, r := range records { + if r.binaryAbsent && !r.pluginInstalled { + warnings = append(warnings, fmt.Sprintf("%s: %s binary not found; install %s, then run: hey setup %s", r.id, r.name, r.name, r.id)) + } + } + + agentsDetail := make([]map[string]any, 0, len(records)) + for _, r := range records { + agentsDetail = append(agentsDetail, map[string]any{ + "id": r.id, + "name": r.name, + "detected_before": r.detectedBefore, + "detected_after": r.detectedAfter, + "plugin_installed": r.pluginInstalled, + "errors": orEmptyStrings(r.errors), + "manual_commands": orEmptyStrings(r.manualCommands), + }) + } + + result := map[string]any{ + "skill_installed": skillInstalled, + "selector": selector, + "ambiguous": ambiguous, + "detected_before": detectedBefore, + "attempted_agents": attempted, + "errors": errUnion.slice(), + "warnings": orEmptyStrings(warnings), + "manual_commands": manualUnion.slice(), + "agents": agentsDetail, + } + + manual := manualUnion.slice() + breadcrumbs := make([]output.Breadcrumb, 0, 1+len(manual)) + breadcrumbs = append(breadcrumbs, output.Breadcrumb{Action: "doctor", Command: "hey doctor", Description: "Check CLI health"}) + for i, m := range manual { + breadcrumbs = append(breadcrumbs, output.Breadcrumb{ + Action: fmt.Sprintf("manual_step_%d", i+1), + Command: m, + Description: "Manual setup step", + }) + } + + return writeOK(result, + output.WithSummary(agentSetupSummary(selector, ambiguous, skillInstalled, records)), + output.WithBreadcrumbs(breadcrumbs...), + ) +} + +// runAgentSetupHandler runs one agent's non-interactive handler and captures +// its before/after detection, plugin health, and remediation. +func runAgentSetupHandler(cmd *cobra.Command, agent harness.AgentInfo) agentSetupRecord { + rec := agentSetupRecord{ + id: agent.ID, + name: agent.Name, + detectedBefore: agent.Detect != nil && agent.Detect(), + binaryAbsent: !agentBinaryPresent(agent.ID), + } + + if handler, ok := agentSetupHandlers[agent.ID]; ok && handler.RunNonInteractive != nil { + if err := handler.RunNonInteractive(cmd); err != nil { + rec.errors = append(rec.errors, err.Error()) + var setupErr *agentSetupError + if errors.As(err, &setupErr) { + rec.manualCommands = append(rec.manualCommands, setupErr.Manual...) + } + } + } + + rec.detectedAfter = agent.Detect != nil && agent.Detect() + // Connected means the handler succeeded AND health checks pass. Checks + // alone are not enough: an unmanaged skill at the canonical path makes + // the handler refuse while the presence check still passes — that is a + // conflict, not a connection. + rec.pluginInstalled = len(rec.errors) == 0 && agentChecksPass(agent) + 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 + } +} + +// detectedAgentIDs returns the ids of currently detected agents, sorted. +func detectedAgentIDs() []string { + agents := harness.DetectedAgents() + ids := make([]string, 0, len(agents)) + for _, a := range agents { + ids = append(ids, a.ID) + } + sort.Strings(ids) + return ids +} + +// agentChoiceCommands returns `hey setup ` for each agent, sorted by id. +func agentChoiceCommands(agents []harness.AgentInfo) []string { + ids := make([]string, 0, len(agents)) + for _, a := range agents { + ids = append(ids, a.ID) + } + sort.Strings(ids) + cmds := make([]string, 0, len(ids)) + for _, id := range ids { + cmds = append(cmds, "hey setup "+id) + } + return cmds +} + +// agentSetupSummary names the resulting state in one line. +func agentSetupSummary(selector string, ambiguous, skillInstalled bool, records []agentSetupRecord) string { + switch { + case !skillInstalled: + return "Baseline skill installation failed" + case selector == "invalid": + return "Unknown " + agentSetupEnv + " value; installed baseline skill only" + case ambiguous: + return "Multiple coding agents detected; installed baseline skill only" + case len(records) == 0: + return "Installed baseline skill; no coding agents connected" + } + names := make([]string, 0, len(records)) + connected := 0 + for _, r := range records { + names = append(names, r.name) + if r.pluginInstalled { + connected++ + } + } + if connected == len(records) { + return "Installed baseline skill; connected " + joinNames(names) + } + return "Installed baseline skill; attempted " + joinNames(names) +} + +// orderedStringSet accumulates strings with stable first-seen dedup. +type orderedStringSet struct { + seen map[string]bool + items []string +} + +func newOrderedStringSet() *orderedStringSet { + return &orderedStringSet{seen: map[string]bool{}, items: []string{}} +} + +func (s *orderedStringSet) add(v string) { + if !s.seen[v] { + s.seen[v] = true + s.items = append(s.items, v) + } +} + +func (s *orderedStringSet) slice() []string { return s.items } + +// orEmptyStrings replaces a nil slice with a non-nil empty one so JSON +// renders `[]` rather than `null`. +func orEmptyStrings(ss []string) []string { + if ss == nil { + return []string{} + } + return ss +} diff --git a/internal/cmd/setup_agents_test.go b/internal/cmd/setup_agents_test.go new file mode 100644 index 000000000..d6aedc44d --- /dev/null +++ b/internal/cmd/setup_agents_test.go @@ -0,0 +1,625 @@ +package cmd + +import ( + "encoding/json" + "errors" + "net/http" + "net/http/httptest" + "os" + "path/filepath" + "strings" + "testing" + + "github.com/basecamp/hey-cli/internal/output" +) + +// runSetupAgents runs `hey setup agents --json` against an isolated HOME with +// the given agent directories present and the selector set. +func runSetupAgents(t *testing.T, selector string, agentDirs ...string) (map[string]any, output.Response) { + t.Helper() + isolateAgents(t) + t.Setenv(agentSetupEnv, selector) + home := t.TempDir() + for _, dir := range agentDirs { + if err := os.MkdirAll(filepath.Join(home, dir), 0o755); err != nil { + t.Fatal(err) + } + } + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + t.Errorf("unexpected HTTP request: %s %s", r.Method, r.URL.Path) + })) + t.Cleanup(server.Close) + + _, response, err := runAuthCommand(t, home, server.URL, "", true, "setup", "agents") + if err != nil { + t.Fatalf("setup agents: %v", err) + } + data, ok := response.Data.(map[string]any) + if !ok { + t.Fatalf("data = %T", response.Data) + } + if data["skill_installed"] != true { + t.Errorf("baseline skill must always be installed: %v", data) + } + if _, err := os.Stat(filepath.Join(home, ".agents", "skills", "hey", "SKILL.md")); err != nil { + t.Errorf("baseline skill file missing: %v", err) + } + return data, response +} + +func stringList(t *testing.T, value any) []string { + t.Helper() + items, ok := value.([]any) + if !ok { + t.Fatalf("expected a list, got %T (%v)", value, value) + } + out := make([]string, 0, len(items)) + for _, item := range items { + out = append(out, item.(string)) + } + return out +} + +func TestSetupAgentsRejectsPositionalArgs(t *testing.T) { + isolateAgents(t) + _, _, err := runAuthCommand(t, t.TempDir(), "http://app.hey.localhost:3003", "", true, "setup", "agents", "claude") + if err == nil || !strings.Contains(err.Error(), "unknown command") && !strings.Contains(err.Error(), "arg") { + t.Fatalf("positional args must be rejected, got %v", err) + } +} + +func TestSetupAgentsNoAgentsDetectedInstallsSkillOnly(t *testing.T) { + data, response := runSetupAgents(t, "") + if data["selector"] != "auto" || data["ambiguous"] != false { + t.Errorf("selector/ambiguous = %v/%v", data["selector"], data["ambiguous"]) + } + if got := stringList(t, data["attempted_agents"]); len(got) != 0 { + t.Errorf("attempted = %v", got) + } + if response.Summary != "Installed baseline skill; no coding agents connected" { + t.Errorf("summary = %q", response.Summary) + } +} + +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) + } +} + +func TestSetupAgentsAmbiguousDetectionNeverGuesses(t *testing.T) { + data, response := runSetupAgents(t, "", ".claude", ".codex") + if data["ambiguous"] != true { + t.Errorf("ambiguous = %v", data["ambiguous"]) + } + if got := stringList(t, data["attempted_agents"]); len(got) != 0 { + t.Errorf("attempted = %v, ambiguity must not connect anyone", got) + } + manual := stringList(t, data["manual_commands"]) + if len(manual) != 2 || manual[0] != "hey setup claude" || manual[1] != "hey setup codex" { + t.Errorf("manual_commands = %v", manual) + } + if response.Summary != "Multiple coding agents detected; installed baseline skill only" { + t.Errorf("summary = %q", response.Summary) + } +} + +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" { + t.Errorf("attempted = %v", got) + } + // Claude cannot be connected without its binary: an error, a warning and + // manual remediation, never a silent success. + errs := stringList(t, data["errors"]) + if len(errs) == 0 || !strings.HasPrefix(errs[0], "claude: ") { + t.Errorf("errors = %v", 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") { + t.Errorf("manual_commands = %v", manual) + } + if response.Summary != "Installed baseline skill; attempted Claude Code and Codex" { + t.Errorf("summary = %q", response.Summary) + } +} + +func TestSetupAgentsNoneInstallsSkillOnly(t *testing.T) { + data, _ := runSetupAgents(t, "none", ".claude", ".codex") + if data["selector"] != "none" { + t.Errorf("selector = %v", data["selector"]) + } + if got := stringList(t, data["attempted_agents"]); len(got) != 0 { + t.Errorf("attempted = %v", got) + } +} + +func TestSetupAgentsExplicitSelectorTargetsThatAgent(t *testing.T) { + data, _ := runSetupAgents(t, "Codex", ".claude", ".codex") + if data["selector"] != "codex" { + t.Errorf("selector = %v", data["selector"]) + } + if got := stringList(t, data["attempted_agents"]); len(got) != 1 || got[0] != "codex" { + t.Errorf("attempted = %v", got) + } +} + +func TestSetupAgentsInvalidSelectorWarns(t *testing.T) { + data, response := runSetupAgents(t, "bogus", ".codex") + if data["selector"] != "invalid" { + t.Errorf("selector = %v", data["selector"]) + } + if got := stringList(t, data["attempted_agents"]); len(got) != 0 { + t.Errorf("attempted = %v", got) + } + warnings := stringList(t, data["warnings"]) + if len(warnings) != 1 || !strings.Contains(warnings[0], `Unknown HEY_SETUP_AGENT value "bogus"`) { + t.Errorf("warnings = %v", warnings) + } + if response.Summary != "Unknown HEY_SETUP_AGENT value; installed baseline skill only" { + t.Errorf("summary = %q", response.Summary) + } +} + +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) + } + + // 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 *output.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) { + tests := map[string][]string{ + "": nil, + "Claude Code": {"Claude Code"}, + "Claude Code and Codex": {"Claude Code", "Codex"}, + "Claude Code, Codex, and Cursor": {"Claude Code", "Codex", "Cursor"}, + } + for want, names := range tests { + if got := joinNames(names); got != want { + t.Errorf("joinNames(%v) = %q, want %q", names, got, want) + } + } +} + +// The installer's automatic handoff must never destroy a hand-authored +// baseline skill, whatever the selector: it reports the refusal instead. +func TestSetupAgentsPreservesUnmarkedBaselineSkill(t *testing.T) { + isolateAgents(t) + t.Setenv(agentSetupEnv, "none") + home := t.TempDir() + custom := "# my own hey skill\n" + dir := filepath.Join(home, ".agents", "skills", "hey") + if err := os.MkdirAll(dir, 0o755); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(dir, "SKILL.md"), []byte(custom), 0o600); err != nil { + t.Fatal(err) + } + server := httptest.NewServer(http.NotFoundHandler()) + defer server.Close() + + _, response, err := runAuthCommand(t, home, server.URL, "", true, "setup", "agents") + if err != nil { + t.Fatalf("setup agents: %v", err) + } + data := response.Data.(map[string]any) + if data["skill_installed"] != false { + t.Error("must not report the skill installed over a refused directory") + } + errs := stringList(t, data["errors"]) + if len(errs) != 1 || !strings.Contains(errs[0], "not written by hey-cli") { + t.Errorf("errors = %v", errs) + } + if got, _ := os.ReadFile(filepath.Join(dir, "SKILL.md")); string(got) != custom { + t.Errorf("user skill changed: %q", got) + } + if ownedSkillDir(dir) { + t.Error("user directory was claimed") + } + if response.Summary != "Baseline skill installation failed" { + t.Errorf("summary = %q", response.Summary) + } +} + +// `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 *output.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") + } +} + +// 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) { + isolateAgents(t) + home := t.TempDir() + if err := os.MkdirAll(filepath.Join(home, ".claude"), 0o755); err != nil { + t.Fatal(err) + } + server := httptest.NewServer(http.NotFoundHandler()) + defer server.Close() + origColor := colorDisabled + colorDisabled = true + t.Cleanup(func() { colorDisabled = origColor }) + + stdout, _, err := runAuthCommand(t, home, server.URL, "", false, "setup", "claude", "--styled") + var cliErr *output.Error + if !errors.As(err, &cliErr) || cliErr.Code != "setup_incomplete" { + t.Fatalf("error = %v, want setup_incomplete", err) + } + if strings.Contains(stdout, "Start a new Claude Code session") { + t.Errorf("claimed success over a failed setup:\n%s", stdout) + } + if !strings.Contains(stdout, "Claude Code setup failed") { + t.Errorf("failure not surfaced:\n%s", stdout) + } +} + +// Commands that never touch the server must not move secrets around: the +// installer runs `setup agents` with HEY_NO_KEYRING=1, which would otherwise +// migrate legacy config.json tokens into plaintext credentials.json. +func TestSetupAgentsDoesNotMigrateLegacyCredentials(t *testing.T) { + isolateAgents(t) + t.Setenv(agentSetupEnv, "none") + home := t.TempDir() + configDir := filepath.Join(home, "hey-cli") + if err := os.MkdirAll(configDir, 0o700); err != nil { + t.Fatal(err) + } + server := httptest.NewServer(http.NotFoundHandler()) + defer server.Close() + legacy := `{"base_url":"` + server.URL + `","access_token":"legacy-token"}` + if err := os.WriteFile(filepath.Join(configDir, "config.json"), []byte(legacy), 0o600); err != nil { + t.Fatal(err) + } + + if _, _, err := runAuthCommand(t, home, server.URL, "", true, "setup", "agents"); err != nil { + t.Fatalf("setup agents: %v", err) + } + if _, err := os.Stat(filepath.Join(configDir, "credentials.json")); !os.IsNotExist(err) { + t.Fatal("setup agents migrated legacy credentials into credentials.json") + } + + // A command that does use credentials still migrates them. + if _, _, err := runAuthCommand(t, home, server.URL, "", true, "auth", "status"); err != nil { + t.Fatalf("auth status: %v", err) + } + if _, err := os.Stat(filepath.Join(configDir, "credentials.json")); err != nil { + t.Errorf("auth status should have migrated legacy credentials: %v", err) + } +} + +// A targeted agent whose skill path is occupied by an unmanaged skill is a +// conflict, not a connection: the presence check alone must not flip +// plugin_installed back to true over the handler's refusal. +func TestSetupAgentsConflictedInstallIsNotConnected(t *testing.T) { + isolateAgents(t) + t.Setenv(agentSetupEnv, "codex") + home := t.TempDir() + custom := "# my own hey skill\n" + codexSkill := filepath.Join(home, ".codex", "skills", "hey") + if err := os.MkdirAll(codexSkill, 0o755); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(codexSkill, "SKILL.md"), []byte(custom), 0o600); err != nil { + t.Fatal(err) + } + server := httptest.NewServer(http.NotFoundHandler()) + defer server.Close() + + _, response, err := runAuthCommand(t, home, server.URL, "", true, "setup", "agents") + if err != nil { + t.Fatalf("setup agents: %v", err) + } + data := response.Data.(map[string]any) + errs := stringList(t, data["errors"]) + if len(errs) == 0 || !strings.Contains(errs[0], "not written by hey-cli") { + t.Fatalf("errors = %v", errs) + } + agents := data["agents"].([]any) + if agents[0].(map[string]any)["plugin_installed"] != false { + t.Error("a refused install must not report plugin_installed") + } + if response.Summary != "Installed baseline skill; attempted Codex" { + t.Errorf("summary = %q", response.Summary) + } + if got, _ := os.ReadFile(filepath.Join(codexSkill, "SKILL.md")); string(got) != custom { + t.Errorf("user skill changed: %q", got) + } +} + +// Config-writing commands rewrite config.json through a struct with no +// credential fields, so they must migrate legacy credentials FIRST — skipping +// migration there would delete the tokens instead of moving them. +func TestConfigSetMigratesLegacyCredentialsBeforeRewriting(t *testing.T) { + isolateAgents(t) + home := t.TempDir() + configDir := filepath.Join(home, "hey-cli") + if err := os.MkdirAll(configDir, 0o700); err != nil { + t.Fatal(err) + } + server := httptest.NewServer(http.NotFoundHandler()) + defer server.Close() + legacy := `{"base_url":"` + server.URL + `","access_token":"legacy-token","refresh_token":"legacy-refresh"}` + if err := os.WriteFile(filepath.Join(configDir, "config.json"), []byte(legacy), 0o600); err != nil { + t.Fatal(err) + } + + if _, _, err := runAuthCommand(t, home, server.URL, "", true, "config", "set", "onboarded", "true"); err != nil { + t.Fatalf("config set: %v", err) + } + creds, err := os.ReadFile(filepath.Join(configDir, "credentials.json")) + if err != nil { + t.Fatalf("legacy credentials were not migrated before the rewrite: %v", err) + } + if !strings.Contains(string(creds), "legacy-token") { + t.Errorf("migrated credentials missing the token: %s", creds) + } +} + +// When migration itself fails, the legacy credentials must survive every +// config rewrite — including the wizard's onboarded flag — until a later run +// migrates them. +func TestConfigRewritesPreserveLegacyCredentialsWhenMigrationFails(t *testing.T) { + isolateAgents(t) + home := t.TempDir() + configDir := filepath.Join(home, "hey-cli") + if err := os.MkdirAll(configDir, 0o700); err != nil { + t.Fatal(err) + } + server := httptest.NewServer(http.NotFoundHandler()) + defer server.Close() + legacy := `{"base_url":"` + server.URL + `","access_token":"legacy-token"}` + if err := os.WriteFile(filepath.Join(configDir, "config.json"), []byte(legacy), 0o600); err != nil { + t.Fatal(err) + } + // A directory where credentials.json should be makes the file store's + // load and save both fail, so migration cannot complete. + if err := os.MkdirAll(filepath.Join(configDir, "credentials.json"), 0o700); err != nil { + t.Fatal(err) + } + + if _, _, err := runAuthCommand(t, home, server.URL, "", true, "config", "set", "onboarded", "true"); err != nil { + t.Fatalf("config set: %v", err) + } + raw, err := os.ReadFile(filepath.Join(configDir, "config.json")) + if err != nil { + t.Fatal(err) + } + if !strings.Contains(string(raw), "legacy-token") { + t.Fatalf("failed migration + rewrite deleted the only credentials: %s", raw) + } + if !strings.Contains(string(raw), `"onboarded": true`) { + t.Errorf("rewrite lost its own change: %s", raw) + } +} + +// An agent detected by its home directory alone, whose setup succeeded, +// gets no missing-binary remediation: the warning is for absences that +// actually prevented the connection. +func TestSetupAgentsNoBinaryWarningAfterSuccessfulSetup(t *testing.T) { + data, response := runSetupAgents(t, "", ".codex") + if got := stringList(t, data["warnings"]); len(got) != 0 { + t.Errorf("warnings = %v, want none after a successful skill-only setup", got) + } + if got := stringList(t, data["manual_commands"]); len(got) != 0 { + t.Errorf("manual_commands = %v, want none after success", got) + } + if response.Summary != "Installed baseline skill; connected Codex" { + t.Errorf("summary = %q", response.Summary) + } +} + +// A symlinked baseline SKILL.md is the state every write path refuses, so +// the read-side predicates must not report it healthy. +func TestBaselineSkillInstalledRequiresRegularFile(t *testing.T) { + isolateAgents(t) + home := t.TempDir() + t.Setenv("HOME", home) + t.Setenv("USERPROFILE", home) + dir := filepath.Join(home, ".agents", "skills", "hey") + if err := os.MkdirAll(dir, 0o755); err != nil { + t.Fatal(err) + } + elsewhere := filepath.Join(home, "elsewhere.md") + if err := os.WriteFile(elsewhere, []byte("# elsewhere"), 0o644); err != nil { + t.Fatal(err) + } + if err := os.Symlink(elsewhere, filepath.Join(dir, "SKILL.md")); err != nil { + t.Fatal(err) + } + writeOwnershipMarker(dir) + if baselineSkillInstalled() { + t.Error("a symlinked SKILL.md must not count as installed, marker or not") + } + + // A regular file without the marker is a hand-authored skill: refused by + // install, so never reported as a healthy installation either. + if err := os.Remove(filepath.Join(dir, "SKILL.md")); err != nil { + t.Fatal(err) + } + if err := os.Remove(filepath.Join(dir, ownershipMarkerFile)); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(dir, "SKILL.md"), []byte("# mine"), 0o644); err != nil { + t.Fatal(err) + } + if baselineSkillInstalled() { + t.Error("an unmarked SKILL.md must not count as installed") + } + writeOwnershipMarker(dir) + if !baselineSkillInstalled() { + t.Error("a marked regular SKILL.md is installed") + } +} + +// A prior run's save-success/scrub-failure must not strand secrets in +// config.json forever: with a usable stored credential, the next run retries +// the scrub — removing the legacy fields, preserving unrelated keys, and +// leaving the stored credentials untouched. +func TestMigrationRetriesScrubWhenStoreAlreadyPopulated(t *testing.T) { + isolateAgents(t) + home := t.TempDir() + configDir := filepath.Join(home, "hey-cli") + if err := os.MkdirAll(configDir, 0o700); err != nil { + t.Fatal(err) + } + server := httptest.NewServer(http.NotFoundHandler()) + defer server.Close() + + // A populated credential store, as a completed earlier migration leaves it. + if _, _, err := runAuthCommand(t, home, server.URL, "", true, "auth", "login", "--token", "stored-token"); err != nil { + t.Fatalf("auth login: %v", err) + } + storedBefore, err := os.ReadFile(filepath.Join(configDir, "credentials.json")) + if err != nil { + t.Fatal(err) + } + + // The scrub failed back then: legacy fields still sit in config.json, + // alongside a key from some future version. + raw, err := os.ReadFile(filepath.Join(configDir, "config.json")) + if os.IsNotExist(err) { + raw = []byte("{}") + } else if err != nil { + t.Fatal(err) + } + var cfgMap map[string]any + if err := json.Unmarshal(raw, &cfgMap); err != nil { + t.Fatal(err) + } + cfgMap["base_url"] = server.URL + cfgMap["access_token"] = "legacy-token" + cfgMap["session_cookie"] = "legacy-cookie" + cfgMap["future_setting"] = map[string]any{"nested": true} + seeded, _ := json.Marshal(cfgMap) + if err := os.WriteFile(filepath.Join(configDir, "config.json"), seeded, 0o600); err != nil { + t.Fatal(err) + } + + if _, _, err := runAuthCommand(t, home, server.URL, "", true, "auth", "status"); err != nil { + t.Fatalf("auth status: %v", err) + } + + after, err := os.ReadFile(filepath.Join(configDir, "config.json")) + if err != nil { + t.Fatal(err) + } + var saved map[string]any + if err := json.Unmarshal(after, &saved); err != nil { + t.Fatal(err) + } + if _, ok := saved["access_token"]; ok { + t.Errorf("legacy token not scrubbed on retry: %s", after) + } + if _, ok := saved["session_cookie"]; ok { + t.Errorf("legacy cookie not scrubbed on retry: %s", after) + } + if _, ok := saved["future_setting"]; !ok { + t.Errorf("unrelated key destroyed by the retry: %s", after) + } + storedAfter, err := os.ReadFile(filepath.Join(configDir, "credentials.json")) + if err != nil || string(storedAfter) != string(storedBefore) { + t.Errorf("stored credentials changed during the retry: %v", err) + } +} + +// Legacy credentials belong to the server recorded beside them. With the +// effective base URL pointed elsewhere, migration must neither misfile them +// under the wrong store key nor scrub the only copy for their real origin. +func TestMigrationLeavesForeignOriginLegacyCredentialsAlone(t *testing.T) { + isolateAgents(t) + home := t.TempDir() + configDir := filepath.Join(home, "hey-cli") + if err := os.MkdirAll(configDir, 0o700); err != nil { + t.Fatal(err) + } + server := httptest.NewServer(http.NotFoundHandler()) + defer server.Close() + + // Store already holds credentials for the dev server this run targets. + if _, _, err := runAuthCommand(t, home, server.URL, "", true, "auth", "login", "--token", "dev-token"); err != nil { + t.Fatalf("auth login: %v", err) + } + // The legacy fields target production. + raw, err := os.ReadFile(filepath.Join(configDir, "config.json")) + if os.IsNotExist(err) { + raw = []byte("{}") + } else if err != nil { + t.Fatal(err) + } + var cfgMap map[string]any + if err := json.Unmarshal(raw, &cfgMap); err != nil { + t.Fatal(err) + } + cfgMap["access_token"] = "production-token" + seeded, _ := json.Marshal(cfgMap) + if err := os.WriteFile(filepath.Join(configDir, "config.json"), seeded, 0o600); err != nil { + t.Fatal(err) + } + + if _, _, err := runAuthCommand(t, home, server.URL, "", true, "auth", "status"); err != nil { + t.Fatalf("auth status: %v", err) + } + after, err := os.ReadFile(filepath.Join(configDir, "config.json")) + if err != nil { + t.Fatal(err) + } + if !strings.Contains(string(after), "production-token") { + t.Fatalf("foreign-origin legacy credential scrubbed: %s", after) + } + creds, err := os.ReadFile(filepath.Join(configDir, "credentials.json")) + if err != nil || strings.Contains(string(creds), "production-token") { + t.Errorf("foreign-origin legacy credential misfiled into the store: %v", err) + } +} diff --git a/internal/cmd/setup_test.go b/internal/cmd/setup_test.go new file mode 100644 index 000000000..fad8a3935 --- /dev/null +++ b/internal/cmd/setup_test.go @@ -0,0 +1,597 @@ +package cmd + +import ( + "bytes" + "context" + "encoding/json" + "errors" + "net/http" + "net/http/httptest" + "os" + "path/filepath" + "strings" + "testing" + + "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. +func isolateAgents(t *testing.T) { + t.Helper() + t.Setenv("PATH", t.TempDir()) + t.Setenv("CODEX_HOME", "") + stubRunAgentCommand(t, func(_ context.Context, name string, args ...string) ([]byte, error) { + t.Errorf("unexpected agent command: %s %v", name, args) + return nil, errors.New("unexpected agent command") + }) +} + +func stubRunAgentCommand(t *testing.T, fn func(context.Context, string, ...string) ([]byte, error)) { + t.Helper() + orig := runAgentCommand + runAgentCommand = fn + t.Cleanup(func() { runAgentCommand = orig }) +} + +func stubInteractive(t *testing.T, interactive bool) { + t.Helper() + orig := interactiveStdio + interactiveStdio = func() bool { return interactive } + t.Cleanup(func() { interactiveStdio = orig }) +} + +func identityServer(t *testing.T) *httptest.Server { + t.Helper() + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.URL.Path != "/identity.json" { + t.Errorf("unexpected HTTP request: %s %s", r.Method, r.URL.Path) + http.NotFound(w, r) + return + } + w.Header().Set("Content-Type", "application/json") + _, _ = w.Write([]byte(`{ + "id":1, + "name":"Jane Doe", + "primary_contact":{"id":101,"email_address":"jane@example.com"}, + "accounts":[ + {"id":1,"name":"Personal","purpose":"home","status":"active"}, + {"id":2,"name":"Work","purpose":"work","status":"active"} + ], + "all_users":[ + {"id":11,"account_id":1,"contact":{"id":101,"email_address":"jane@example.com"}}, + {"id":22,"account_id":2,"contact":{"id":202,"email_address":"jane@company.example"}} + ] + }`)) + })) + t.Cleanup(server.Close) + return server +} + +func wizardData(t *testing.T, response output.Response) map[string]any { + t.Helper() + data, ok := response.Data.(map[string]any) + if !ok { + t.Fatalf("wizard data = %T", response.Data) + } + return data +} + +func TestSetupCommandRegistersAgentSubcommands(t *testing.T) { + root := newRootCmd() + for _, path := range [][]string{{"setup", "agents"}, {"setup", "claude"}, {"setup", "codex"}} { + command, _, err := root.Find(path) + if err != nil || command.Name() != path[1] { + t.Errorf("%v not registered: %v", path, err) + } + } +} + +func TestSetupRejectsJQ(t *testing.T) { + isolateAgents(t) + _, _, err := runAuthCommand(t, t.TempDir(), "http://app.hey.localhost:3003", "", false, "setup", "--jq", ".") + if err == nil || !strings.Contains(err.Error(), "--jq is not supported by the setup wizard") { + t.Fatalf("error = %v", err) + } +} + +// A piped `hey setup --json` with nobody at a terminal must not wait for a +// browser: it reports "not logged in" and points at `hey auth login`. +func TestSetupJSONNotLoggedInWithoutTerminalReportsIncomplete(t *testing.T) { + isolateAgents(t) + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + t.Errorf("unexpected HTTP request: %s %s", r.Method, r.URL.Path) + })) + defer server.Close() + configHome := t.TempDir() + + _, response, err := runAuthCommand(t, configHome, server.URL, "", true, "setup") + if err != nil { + t.Fatalf("setup: %v", err) + } + data := wizardData(t, response) + if data["status"] != "incomplete" { + t.Errorf("status = %v, want incomplete", data["status"]) + } + issues, _ := data["issues"].([]any) + if len(issues) != 1 { + t.Fatalf("issues = %v", data["issues"]) + } + if issue, _ := issues[0].(map[string]any); issue["check"] != "Not logged in" { + t.Errorf("issue = %v", issues[0]) + } + if len(response.Breadcrumbs) == 0 || response.Breadcrumbs[0].Command != "hey auth login" { + t.Errorf("breadcrumbs = %+v", response.Breadcrumbs) + } + if response.Summary != "Setup finished with issues" { + t.Errorf("summary = %q", response.Summary) + } +} + +func TestSetupJSONSignedInReportsIdentityAndPersistsOnboarded(t *testing.T) { + isolateAgents(t) + server := identityServer(t) + configHome := t.TempDir() + + if _, _, err := runAuthCommand(t, configHome, server.URL, "", true, "auth", "login", "--cookie", "session-cookie"); err != nil { + t.Fatalf("auth login: %v", err) + } + + stdout, response, err := runAuthCommand(t, configHome, server.URL, "", true, "setup") + if err != nil { + t.Fatalf("setup: %v", err) + } + // Pure JSON on stdout: exactly one envelope, nothing before or after. + var probe map[string]any + if decodeErr := json.NewDecoder(strings.NewReader(stdout)).Decode(&probe); decodeErr != nil || !strings.HasPrefix(strings.TrimSpace(stdout), "{") { + t.Fatalf("stdout is not a single JSON envelope: %q", stdout) + } + + data := wizardData(t, response) + if data["status"] != "complete" { + t.Errorf("status = %v, want complete", data["status"]) + } + identity, _ := data["identity"].(map[string]any) + if identity["name"] != "Jane Doe" || identity["email"] != "jane@example.com" { + t.Errorf("identity = %v", data["identity"]) + } + accounts, _ := data["accounts"].([]any) + if len(accounts) != 3 { // All Accounts + two linked + t.Errorf("accounts = %v", data["accounts"]) + } + if response.Summary != "Setup complete - jane@example.com" { + t.Errorf("summary = %q", response.Summary) + } + if len(response.Breadcrumbs) == 0 || response.Breadcrumbs[0].Command != "hey tui" { + t.Errorf("breadcrumbs = %+v", response.Breadcrumbs) + } + + raw, err := os.ReadFile(filepath.Join(configHome, "hey-cli", "config.json")) + if err != nil { + t.Fatalf("read config: %v", err) + } + var saved map[string]any + if err := json.Unmarshal(raw, &saved); err != nil { + t.Fatal(err) + } + if saved["onboarded"] != true { + t.Errorf("onboarded not persisted: %s", raw) + } +} + +// With Claude Code detected but no binary to drive, the machine-mode wizard +// still installs the skill, never prompts, and reports the plugin as an issue. +func TestSetupJSONRunsAgentStepNonInteractively(t *testing.T) { + isolateAgents(t) + server := identityServer(t) + configHome := t.TempDir() + if err := os.MkdirAll(filepath.Join(configHome, ".claude"), 0o755); err != nil { + t.Fatal(err) + } + if _, _, err := runAuthCommand(t, configHome, server.URL, "", true, "auth", "login", "--cookie", "session-cookie"); err != nil { + t.Fatalf("auth login: %v", err) + } + + _, response, err := runAuthCommand(t, configHome, server.URL, "", true, "setup") + if err != nil { + t.Fatalf("setup: %v", err) + } + data := wizardData(t, response) + if data["skill_installed"] != true { + t.Error("baseline skill should be installed for a detected agent") + } + if _, err := os.Stat(filepath.Join(configHome, ".claude", "skills", "hey", "SKILL.md")); err != nil { + t.Errorf("Claude skill link missing: %v", err) + } + if data["status"] != "incomplete" { + t.Errorf("status = %v, want incomplete (plugin cannot install without a binary)", data["status"]) + } + rawIssues := data["issues"].([]any) + issueChecks := make([]string, 0, len(rawIssues)) + for _, raw := range rawIssues { + issue := raw.(map[string]any) + issueChecks = append(issueChecks, issue["check"].(string)) + } + if !contains(issueChecks, "Claude Code Plugin") || contains(issueChecks, "Claude Code Skill") { + t.Errorf("issues = %v", issueChecks) + } + agents, _ := data["agents"].([]any) + if len(agents) == 0 { + t.Error("agents checklist should be reported") + } +} + +// Once onboarded, a logged-out bare `hey` runs the lite wizard: sign-in only, +// no agent step even when an agent is detected. +func TestBareHeyLiteWizardSkipsAgentsWhenOnboarded(t *testing.T) { + isolateAgents(t) + stubInteractive(t, true) + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + t.Errorf("unexpected HTTP request: %s %s", r.Method, r.URL.Path) + })) + defer server.Close() + configHome := t.TempDir() + if err := os.MkdirAll(filepath.Join(configHome, ".claude"), 0o755); err != nil { + t.Fatal(err) + } + if _, _, err := runAuthCommand(t, configHome, server.URL, "", true, "config", "set", "onboarded", "true"); err != nil { + t.Fatalf("config set: %v", err) + } + + // No --json: that is the help route. Stdout is a buffer, so the wizard + // renders its envelope rather than prose — and with no terminal on stdin + // it cannot sign in. + stdout, _, err := runAuthCommand(t, configHome, server.URL, "", false) + if err != nil { + t.Fatalf("bare hey: %v", err) + } + var response output.Response + if err := json.Unmarshal([]byte(stdout), &response); err != nil { + t.Fatalf("bare hey did not render the wizard envelope: %q", stdout) + } + data := wizardData(t, response) + if data["skill_installed"] != false { + t.Error("lite wizard must not run the agent step") + } + if agents, _ := data["agents"].([]any); len(agents) != 0 { + t.Errorf("lite wizard reported agents: %v", agents) + } +} + +func contains(items []string, want string) bool { + for _, item := range items { + if item == want { + return true + } + } + return false +} + +func TestSuccessHeadline(t *testing.T) { + tests := []struct { + status string + issues int + want string + }{ + {"complete", 0, "Setup complete!"}, + {"incomplete", 1, "Setup finished — 1 step needs attention"}, + {"incomplete", 3, "Setup finished — 3 steps need attention"}, + } + for _, tt := range tests { + if got := successHeadline(tt.status, tt.issues); got != tt.want { + t.Errorf("successHeadline(%q, %d) = %q, want %q", tt.status, tt.issues, got, tt.want) + } + } +} + +func TestStatusFromOutcome(t *testing.T) { + if got := statusFromOutcome(agentSetupOutcome{}); got != "complete" { + t.Errorf("empty outcome = %q", got) + } + if got := statusFromOutcome(agentSetupOutcome{Skipped: true}); got != "complete" { + t.Errorf("skipped outcome = %q, a deliberate skip is complete", got) + } + if got := statusFromOutcome(agentSetupOutcome{Issues: []agentIssue{{Check: "Claude Code Plugin"}}}); got != "incomplete" { + t.Errorf("outcome with issues = %q", got) + } +} + +func TestShowWizardSuccessText(t *testing.T) { + origColor := colorDisabled + colorDisabled = true + t.Cleanup(func() { colorDisabled = origColor }) + + var out bytes.Buffer + showWizardSuccess(&out, wizardResult{Status: "complete", Identity: &wizardIdentity{Email: "jane@example.com"}}, agentSetupOutcome{ + Checks: []agentCheck{{Agent: "Claude Code", Name: "Claude Code Plugin", Status: "pass"}}, + }) + text := out.String() + for _, want := range []string{"Setup complete!", "✓ Signed in", "✓ Claude Code Plugin", "Try these commands:", "hey boxes", `hey search "quarterly planning"`} { + if !strings.Contains(text, want) { + t.Errorf("complete summary missing %q:\n%s", want, text) + } + } + if strings.Contains(text, "Some steps need attention") { + t.Errorf("complete summary should not list remediation:\n%s", text) + } + + out.Reset() + issues := []agentIssue{{Agent: "Claude Code", Check: "Claude Code Plugin", Hint: "Run: hey setup claude"}} + showWizardSuccess(&out, wizardResult{Status: "incomplete", Issues: issues}, agentSetupOutcome{ + Checks: []agentCheck{{Agent: "Claude Code", Name: "Claude Code Plugin", Status: "fail"}}, + Issues: issues, + }) + text = out.String() + for _, want := range []string{"Setup finished — 1 step needs attention", "✗ Claude Code Plugin", "Some steps need attention:", "Claude Code Plugin: Run: hey setup claude", "Then verify with: hey doctor"} { + if !strings.Contains(text, want) { + t.Errorf("incomplete summary missing %q:\n%s", want, text) + } + } + + out.Reset() + showWizardSuccess(&out, wizardResult{Status: "complete"}, agentSetupOutcome{Skipped: true}) + if !strings.Contains(out.String(), "Coding agent setup skipped — run: hey setup") { + t.Errorf("skipped summary:\n%s", out.String()) + } +} + +func stubStdinTerminal(t *testing.T, isTerminal bool) { + t.Helper() + orig := stdinIsTerminal + stdinIsTerminal = func() bool { return isTerminal } + t.Cleanup(func() { stdinIsTerminal = orig }) +} + +func stubConfirmAgentSetup(t *testing.T, answer bool, err error) *int { + t.Helper() + calls := 0 + orig := confirmAgentSetup + confirmAgentSetup = func() (bool, error) { + calls++ + return answer, err + } + t.Cleanup(func() { confirmAgentSetup = orig }) + return &calls +} + +// HEY_NONINTERACTIVE must disable every wizard interaction even on a real +// PTY: no agent-setup prompt (the default answer applies) and no OAuth wait. +func TestSetupStyledNonInteractiveNeverPromptsNorSignsIn(t *testing.T) { + isolateAgents(t) + stubStdinTerminal(t, true) // a PTY — but HEY_NONINTERACTIVE wins + t.Setenv("HEY_NONINTERACTIVE", "1") + confirms := stubConfirmAgentSetup(t, false, nil) + server := quietServer(t) + configHome := t.TempDir() + if err := os.MkdirAll(filepath.Join(configHome, ".claude"), 0o755); err != nil { + t.Fatal(err) + } + + origColor := colorDisabled + colorDisabled = true + t.Cleanup(func() { colorDisabled = origColor }) + + stdout, _, err := runAuthCommand(t, configHome, server.URL, "", false, "setup", "--styled") + if err != nil { + t.Fatalf("setup --styled: %v", err) + } + if *confirms != 0 { + t.Errorf("the agent-setup prompt ran %d times with HEY_NONINTERACTIVE=1", *confirms) + } + // No OAuth wait: the run completed and reported the missing login. + if !strings.Contains(stdout, "Setup finished") || !strings.Contains(stdout, "Not logged in") { + t.Errorf("expected an incomplete summary, got:\n%s", stdout) + } + // The agent step proceeded with the prompt's default answer. + if _, err := os.Stat(filepath.Join(configHome, ".agents", "skills", "hey", "SKILL.md")); err != nil { + t.Errorf("agent step should auto-proceed without a prompt: %v", err) + } +} + +// The interactive path still prompts, and declining skips the agent step. +func TestSetupStyledInteractiveDeclineSkipsAgents(t *testing.T) { + isolateAgents(t) + stubInteractive(t, true) + confirms := stubConfirmAgentSetup(t, false, nil) + server := identityServer(t) + configHome := t.TempDir() + if err := os.MkdirAll(filepath.Join(configHome, ".claude"), 0o755); err != nil { + t.Fatal(err) + } + if _, _, err := runAuthCommand(t, configHome, server.URL, "", true, "auth", "login", "--cookie", "session-cookie"); err != nil { + t.Fatalf("auth login: %v", err) + } + + origColor := colorDisabled + colorDisabled = true + t.Cleanup(func() { colorDisabled = origColor }) + + stdout, _, err := runAuthCommand(t, configHome, server.URL, "", false, "setup", "--styled") + if err != nil { + t.Fatalf("setup --styled: %v", err) + } + if *confirms != 1 { + t.Errorf("prompt ran %d times, want 1", *confirms) + } + if !strings.Contains(stdout, "You can set up agents later:") || !strings.Contains(stdout, "Coding agent setup skipped") { + t.Errorf("declined setup should skip:\n%s", stdout) + } + if _, err := os.Stat(filepath.Join(configHome, ".agents", "skills", "hey", "SKILL.md")); !os.IsNotExist(err) { + t.Error("declined setup must not install the skill") + } +} + +// Machine output plus HEY_NONINTERACTIVE on a terminal must not start OAuth. +func TestSetupJSONNonInteractiveEnvSkipsSignIn(t *testing.T) { + isolateAgents(t) + stubStdinTerminal(t, true) + t.Setenv("HEY_NONINTERACTIVE", "1") + server := quietServer(t) + + _, response, err := runAuthCommand(t, t.TempDir(), server.URL, "", true, "setup") + if err != nil { + t.Fatalf("setup: %v", err) + } + data := wizardData(t, response) + if data["status"] != "incomplete" { + t.Errorf("status = %v, want incomplete", data["status"]) + } +} + +// Credentials that merely exist are not a login: when HEY rejects them, the +// wizard must not skip OAuth and then report a complete, signed-in setup. +func TestSetupJSONStaleCredentialsReportIncomplete(t *testing.T) { + isolateAgents(t) + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + http.Error(w, `{"error":"unauthorized"}`, http.StatusUnauthorized) + })) + defer server.Close() + configHome := t.TempDir() + if _, _, err := runAuthCommand(t, configHome, server.URL, "", true, "auth", "login", "--cookie", "stale-cookie"); err != nil { + t.Fatalf("auth login: %v", err) + } + + _, response, err := runAuthCommand(t, configHome, server.URL, "", true, "setup") + if err != nil { + t.Fatalf("setup: %v", err) + } + data := wizardData(t, response) + if data["status"] != "incomplete" { + t.Errorf("status = %v, want incomplete for rejected credentials", data["status"]) + } + issues, _ := data["issues"].([]any) + if len(issues) != 1 { + t.Fatalf("issues = %v", data["issues"]) + } + if issue, _ := issues[0].(map[string]any); issue["check"] != "Stored sign-in rejected" { + t.Errorf("issue = %v", issues[0]) + } + // Machine clients must be routed to repair auth, not to hey tui. + if len(response.Breadcrumbs) == 0 || response.Breadcrumbs[0].Command != "hey auth login" { + t.Errorf("breadcrumbs = %+v", response.Breadcrumbs) + } +} + +// The wizard records a handler refusal as an issue even when the health +// snapshot alone would miss it, so "Setup complete!" can never follow an +// installation warning. +func TestSetupWizardRecordsHandlerFailures(t *testing.T) { + isolateAgents(t) + server := identityServer(t) + configHome := t.TempDir() + custom := "# my own hey skill\n" + codexSkill := filepath.Join(configHome, ".codex", "skills", "hey") + if err := os.MkdirAll(codexSkill, 0o755); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(codexSkill, "SKILL.md"), []byte(custom), 0o600); err != nil { + t.Fatal(err) + } + if _, _, err := runAuthCommand(t, configHome, server.URL, "", true, "auth", "login", "--cookie", "session-cookie"); err != nil { + t.Fatalf("auth login: %v", err) + } + + _, response, err := runAuthCommand(t, configHome, server.URL, "", true, "setup") + if err != nil { + t.Fatalf("setup: %v", err) + } + data := wizardData(t, response) + if data["status"] != "incomplete" { + t.Errorf("status = %v, want incomplete over a refused install", data["status"]) + } + rawIssues := data["issues"].([]any) + checks := make([]string, 0, len(rawIssues)) + for _, raw := range rawIssues { + checks = append(checks, raw.(map[string]any)["check"].(string)) + } + if !contains(checks, "Codex setup failed") { + t.Errorf("handler failure not recorded: %v", checks) + } + if got, _ := os.ReadFile(filepath.Join(codexSkill, "SKILL.md")); string(got) != custom { + t.Errorf("user skill changed: %q", got) + } +} + +// The checklist and the issue list must agree: rejected stored credentials +// render as not signed in. +func TestShowWizardSuccessRejectedCredentialsChecklist(t *testing.T) { + origColor := colorDisabled + colorDisabled = true + t.Cleanup(func() { colorDisabled = origColor }) + + var out bytes.Buffer + issues := []agentIssue{{Check: "Stored sign-in rejected", Hint: "Run: hey auth login"}} + showWizardSuccess(&out, wizardResult{Status: "incomplete", Issues: issues}, agentSetupOutcome{}) + if !strings.Contains(out.String(), "✗ Signed in") { + t.Errorf("rejected credentials must render as not signed in:\n%s", out.String()) + } +} + +// Prompts render on stderr, so a redirected stderr means no prompting: an +// invisible prompt must never sit waiting for input. +func TestInteractiveStdioRequiresStderrTerminal(t *testing.T) { + stubTerminal := func(v *func() bool, value bool) { + orig := *v + *v = func() bool { return value } + t.Cleanup(func() { *v = orig }) + } + stubTerminal(&stdinIsTerminal, true) + stubTerminal(&stdoutIsTerminal, true) + stubTerminal(&stderrIsTerminal, false) + t.Setenv("HEY_NONINTERACTIVE", "") + + if interactiveStdio() { + t.Error("a redirected stderr must disable prompting") + } + stubTerminal(&stderrIsTerminal, true) + if !interactiveStdio() { + t.Error("three terminals and no escape hatch should allow prompting") + } +} + +// --ids-only and --count cannot render a wizard result, so they are rejected +// before any side effect — never OAuth-then-"requires list data". +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"}} { + configHome := t.TempDir() + _, _, err := runAuthCommand(t, configHome, server.URL, "", false, append(args, flag)...) + if err == nil || !strings.Contains(err.Error(), flag+" is not supported") { + t.Fatalf("%v %s: error = %v", args, flag, err) + } + if _, statErr := os.Stat(filepath.Join(configHome, ".agents")); !os.IsNotExist(statErr) { + t.Errorf("%v %s ran side effects before rejecting the format", args, flag) + } + } + } +} + +// A rejected HEY_TOKEN outranks anything hey auth login saves, so the +// remediation must point at the environment, not at a login that cannot win. +func TestSetupJSONRejectedEnvTokenPointsAtEnvironment(t *testing.T) { + isolateAgents(t) + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + http.Error(w, `{"error":"unauthorized"}`, http.StatusUnauthorized) + })) + defer server.Close() + + _, response, err := runAuthCommand(t, t.TempDir(), server.URL, "revoked-env-token", true, "setup") + if err != nil { + t.Fatalf("setup: %v", err) + } + data := wizardData(t, response) + if data["status"] != "incomplete" { + t.Errorf("status = %v", data["status"]) + } + issues := data["issues"].([]any) + issue := issues[0].(map[string]any) + if issue["check"] != "HEY_TOKEN rejected" || issue["hint"] != "Update or unset HEY_TOKEN" { + t.Errorf("issue = %v", issue) + } + if len(response.Breadcrumbs) == 0 || response.Breadcrumbs[0].Command != "unset HEY_TOKEN" { + t.Errorf("breadcrumbs must point at the environment, got %+v", response.Breadcrumbs) + } +} diff --git a/internal/cmd/skill_install.go b/internal/cmd/skill_install.go index 5c3c4aa3c..ba6bf7732 100644 --- a/internal/cmd/skill_install.go +++ b/internal/cmd/skill_install.go @@ -4,67 +4,363 @@ import ( "fmt" "os" "path/filepath" + "strings" "github.com/spf13/cobra" + "github.com/basecamp/hey-cli/internal/harness" "github.com/basecamp/hey-cli/internal/output" + "github.com/basecamp/hey-cli/internal/version" "github.com/basecamp/hey-cli/skills" ) +const ( + skillFilename = "SKILL.md" + installedVersionFile = ".installed-version" + + // ownershipMarkerFile marks a skill directory as written by hey-cli. + // Replacement, automatic refresh, and the harness health checks all + // require it: a directory that merely looks like ours (a user-authored + // skill happens to be one SKILL.md too) is never destroyed, rewritten, + // or reported as a working integration. + ownershipMarkerFile = harness.SkillOwnershipMarker +) + +// unmanagedSkillDirError reports a skill directory hey-cli did not write. +type unmanagedSkillDirError struct{ dir string } + +func (e *unmanagedSkillDirError) Error() string { + return fmt.Sprintf("%s exists but was not written by hey-cli; move it aside to let hey install its skill there", e.dir) +} + +// claimSkillDir is the one gate every skill write goes through. It creates +// and marks a missing (or empty) directory, accepts one that already carries +// our marker, and refuses anything else — a populated directory without the +// marker is somebody's hand-authored skill, and neither an explicit install +// nor the installer's automatic handoff may overwrite it or claim it. +func claimSkillDir(dir string) error { + info, err := os.Lstat(dir) + switch { + case os.IsNotExist(err): + if mkErr := os.MkdirAll(dir, 0o755); mkErr != nil { // #nosec G301 -- installed skills contain public documentation + return fmt.Errorf("creating skill directory: %w", mkErr) + } + case err != nil: + return fmt.Errorf("inspecting skill directory: %w", err) + case info.Mode()&os.ModeSymlink != 0: + // A symlink here is a user's arrangement (or our own Claude link, + // which is never a write target). Writing through it would land in + // a directory we never inspected. + return &unmanagedSkillDirError{dir: dir} + case !info.IsDir(): + return &unmanagedSkillDirError{dir: dir} + case !ownedSkillDir(dir): + entries, err := os.ReadDir(dir) + if err != nil { + return fmt.Errorf("inspecting skill directory: %w", err) + } + if len(entries) > 0 { + return &unmanagedSkillDirError{dir: dir} + } + } + writeOwnershipMarker(dir) + return nil +} + +// writeOwnershipMarker stamps dir as hey-cli-managed. Only claimSkillDir +// calls it, after proving the directory is ours to claim. +func writeOwnershipMarker(dir string) { + content := []byte("This skill is managed by hey-cli. Manual edits will be overwritten on upgrade.\n") + _ = writeSkillFile(filepath.Join(dir, ownershipMarkerFile), content) +} + +// writeSkillFile writes one skill file, refusing to write through a symlink +// or any other non-regular file: a link's target was never inspected by the +// ownership gate, so following it could truncate a file we do not own — even +// inside a directory that carries our marker. +func writeSkillFile(path string, data []byte) error { + if info, err := os.Lstat(path); err == nil { + if !info.Mode().IsRegular() { + return &unmanagedSkillDirError{dir: path} + } + } else if !os.IsNotExist(err) { + return fmt.Errorf("inspecting %s: %w", path, err) + } + return os.WriteFile(path, data, 0o644) // #nosec G306 G703 -- fixed skill locations under the user's own home, gated by claimSkillDir and the Lstat above +} + +// ownedSkillDir reports whether hey-cli wrote the skill directory: the +// harness predicate, which requires the marker to be a regular file. +func ownedSkillDir(dir string) bool { + return harness.SkillDirOwned(dir) +} + func newSkillInstallCommand() *cobra.Command { return &cobra.Command{ Use: "install", - Short: "Install the hey skill globally for Claude", - Long: "Copies the embedded SKILL.md to ~/.agents/skills/hey/ and creates a symlink in ~/.claude/skills/hey.", + Short: "Install the hey skill globally for your coding agents", + Long: "Copies the embedded SKILL.md to ~/.agents/skills/hey/, links it into ~/.claude/skills/hey when Claude Code is installed, and copies it for Codex when Codex is installed.", RunE: runSkillInstall, } } func runSkillInstall(cmd *cobra.Command, args []string) error { + skillPath, err := installSkillFiles() + if err != nil { + return output.ErrAPI(0, err.Error()) + } + + result := map[string]string{"skill_path": skillPath} + lines := []string{"Installed hey skill to ~/.agents/skills/hey/SKILL.md"} + + if harness.DetectClaude() { + notice, linkErr := linkSkillToClaude() + if linkErr != nil { + return output.ErrAPI(0, linkErr.Error()) + } + home, _ := os.UserHomeDir() + result["symlink_path"] = filepath.Join(home, ".claude", "skills", harness.ClaudePluginName) + if notice != "" { + result["notice"] = notice + lines = append(lines, "Copied skill to ~/.claude/skills/hey ("+notice+")") + } else { + lines = append(lines, "Symlinked ~/.claude/skills/hey → ../../.agents/skills/hey") + } + } + + if harness.DetectCodex() { + codexPath, codexErr := installSkillToCodex() + if codexErr != nil { + return output.ErrAPI(0, codexErr.Error()) + } + result["codex_skill_path"] = codexPath + lines = append(lines, "Copied skill to "+codexPath) + } + + if writer.IsStyled() { + for _, line := range lines { + fmt.Fprintln(cmd.OutOrStdout(), line) + } + return nil + } + + return writeOK(result, output.WithSummary("hey skill installed")) +} + +// installSkillFiles writes the embedded SKILL.md to ~/.agents/skills/hey/ and +// stamps the installed version. Returns the path to the installed file. +func installSkillFiles() (string, error) { home, err := os.UserHomeDir() if err != nil { - return output.ErrAPI(0, fmt.Sprintf("getting home directory: %v", err)) + return "", fmt.Errorf("getting home directory: %w", err) } skillDir := filepath.Join(home, ".agents", "skills", "hey") - skillFile := filepath.Join(skillDir, "SKILL.md") - symlinkDir := filepath.Join(home, ".claude", "skills") - symlinkPath := filepath.Join(symlinkDir, "hey") + skillFile := filepath.Join(skillDir, skillFilename) - // Read embedded SKILL.md data, err := skills.FS.ReadFile("hey/SKILL.md") if err != nil { - return output.ErrAPI(0, fmt.Sprintf("reading embedded skill: %v", err)) + return "", fmt.Errorf("reading embedded skill: %w", err) } - // Create skill directory and write file - if err := os.MkdirAll(skillDir, 0o755); err != nil { // #nosec G301 -- installed skills contain public documentation - return output.ErrAPI(0, fmt.Sprintf("creating skill directory: %v", err)) + if err := claimSkillDir(skillDir); err != nil { + return "", err } - if err := os.WriteFile(skillFile, data, 0o644); err != nil { // #nosec G306 -- installed skills are intentionally user-readable - return output.ErrAPI(0, fmt.Sprintf("writing skill file: %v", err)) + if err := writeSkillFile(skillFile, data); err != nil { + return "", fmt.Errorf("writing skill file: %w", err) + } + + // Best-effort: stamp installed version so doctor can spot staleness. + _ = writeSkillFile(filepath.Join(skillDir, installedVersionFile), []byte(version.Version)) + + return skillFile, nil +} + +// linkSkillToClaude creates a symlink at ~/.claude/skills/hey pointing to the +// baseline skill directory, falling back to a file copy where symlinks are +// unavailable. A populated real directory at that path is an error: user +// content is never overwritten. Returns a human-readable notice when the copy +// fallback was used. +func linkSkillToClaude() (string, error) { + home, err := os.UserHomeDir() + if err != nil { + return "", fmt.Errorf("getting home directory: %w", err) + } + + skillDir := filepath.Join(home, ".agents", "skills", "hey") + symlinkDir := filepath.Join(home, ".claude", "skills") + symlinkPath := filepath.Join(symlinkDir, harness.ClaudePluginName) + + // Never link Claude to a baseline the install paths refused to claim: a + // link to unmanaged content would modify Claude while the same command + // reports the integration unhealthy. + if !baselineSkillInstalled() { + return "", &unmanagedSkillDirError{dir: skillDir} } - // Create symlink directory and symlink if err := os.MkdirAll(symlinkDir, 0o755); err != nil { // #nosec G301 -- standard user-level skills directory - return output.ErrAPI(0, fmt.Sprintf("creating symlink directory: %v", err)) + return "", fmt.Errorf("creating symlink directory: %w", err) } - // Remove existing symlink/file if present - if err := os.Remove(symlinkPath); err != nil && !os.IsNotExist(err) { - return output.ErrAPI(0, fmt.Sprintf("removing existing skill link: %v", err)) + if err := removeExistingSkillLink(symlinkPath); err != nil { + return "", err } - if err := os.Symlink(filepath.Join("..", "..", ".agents", "skills", "hey"), symlinkPath); err != nil { - return output.ErrAPI(0, fmt.Sprintf("creating symlink: %v", err)) + + if err := makeSkillSymlink(claudeSkillLinkTarget, symlinkPath); err != nil { + // Fallback (e.g. Windows without symlink privilege): copy the files. + notice := fmt.Sprintf("symlink failed (%v), copied files instead", err) + if copyErr := copySkillFiles(skillDir, symlinkPath); copyErr != nil { + return "", fmt.Errorf("creating symlink: %w (copy fallback also failed: %w)", err, copyErr) + } + return notice, nil } + return "", nil +} - if writer.IsStyled() { - fmt.Fprintln(cmd.OutOrStdout(), "Installed hey skill to ~/.agents/skills/hey/SKILL.md") - fmt.Fprintln(cmd.OutOrStdout(), "Symlinked ~/.claude/skills/hey → ../../.agents/skills/hey") +// claudeSkillLinkTarget is the relative target every hey-cli-written +// ~/.claude/skills/hey symlink points at. It doubles as provenance: a link +// with any other target was not written by hey-cli. +var claudeSkillLinkTarget = filepath.Join("..", "..", ".agents", "skills", "hey") + +// makeSkillSymlink is a seam so tests can exercise the symlink-less fallback. +var makeSkillSymlink = os.Symlink + +// removeExistingSkillLink clears the way for a fresh Claude skill link, +// removing only what hey-cli itself wrote: our canonical symlink, or the +// directory a prior copy fallback left (marker plus allowlisted files). A +// user's own symlink, a regular file, or a populated unmarked directory is +// their state and errors instead — shape is not ownership. +func removeExistingSkillLink(path string) error { + info, err := os.Lstat(path) + if os.IsNotExist(err) { return nil } + if err != nil { + return fmt.Errorf("inspecting existing skill link: %w", err) + } + switch { + case info.Mode()&os.ModeSymlink != 0: + if target, err := os.Readlink(path); err != nil || target != claudeSkillLinkTarget { + return &unmanagedSkillDirError{dir: path} + } + if err := os.Remove(path); err != nil { + return fmt.Errorf("removing existing skill link: %w", err) + } + case info.IsDir(): + if !isManagedSkillCopy(path) { + return &unmanagedSkillDirError{dir: path} + } + if err := os.RemoveAll(path); err != nil { + return fmt.Errorf("removing existing skill copy: %w", err) + } + default: + return &unmanagedSkillDirError{dir: path} + } + return nil +} - return writeOK(map[string]string{ - "skill_path": skillFile, - "symlink_path": symlinkPath, - }, output.WithSummary("hey skill installed")) +// isManagedSkillCopy reports whether path is a plain directory hey-cli's copy +// fallback wrote: it carries the ownership marker and nothing but the files +// we write, every one a regular file. All three conditions are required — +// the marker proves provenance (and a symlink planted in its name proves +// nothing), and the allowlist keeps anything a user added alongside it safe. +func isManagedSkillCopy(path string) bool { + info, err := os.Lstat(path) + if err != nil || !info.IsDir() { + return false + } + entries, err := os.ReadDir(path) + if err != nil { + return false + } + sawMarker := false + for _, entry := range entries { + if !entry.Type().IsRegular() { + return false + } + switch entry.Name() { + case ownershipMarkerFile: + sawMarker = true + case skillFilename, installedVersionFile: + default: + return false + } + } + return sawMarker +} + +// installSkillToCodex copies the embedded SKILL.md into Codex's skills +// directory ($CODEX_HOME or ~/.codex). Codex does not follow the shared +// ~/.agents layout, so this is a copy rather than a link. +func installSkillToCodex() (string, error) { + skillPath := harness.CodexSkillPath() + if skillPath == "" { + return "", fmt.Errorf("cannot determine Codex home directory") + } + + data, err := skills.FS.ReadFile("hey/SKILL.md") + if err != nil { + return "", fmt.Errorf("reading embedded skill: %w", err) + } + if err := claimSkillDir(filepath.Dir(skillPath)); err != nil { + return "", err + } + if err := writeSkillFile(skillPath, data); err != nil { + return "", fmt.Errorf("writing Codex skill file: %w", err) + } + return skillPath, nil +} + +// baselineSkillInstalled reports whether ~/.agents/skills/hey/SKILL.md is a +// healthy install: a regular file in a directory hey-cli owns. These are the +// final-component rules the write paths enforce (a symlinked SKILL.md or an +// unmarked hand-authored skill is neither written nor reported healthy); +// like every skill read, intermediate directory symlinks resolve, so a +// user-symlinked skill directory can pass here while writes refuse it — +// bounded and non-destructive, see harness.RegularSkillFile. +func baselineSkillInstalled() bool { + home, err := os.UserHomeDir() + if err != nil { + return false + } + skillDir := filepath.Join(home, ".agents", "skills", "hey") + return harness.RegularSkillFile(filepath.Join(skillDir, skillFilename)) && ownedSkillDir(skillDir) +} + +// installedSkillVersion reads the .installed-version stamp from the baseline +// skill directory. Returns "" if absent or unreadable. +func installedSkillVersion() string { + home, err := os.UserHomeDir() + if err != nil { + return "" + } + data, err := os.ReadFile(filepath.Join(home, ".agents", "skills", "hey", installedVersionFile)) // #nosec G304 -- the fixed baseline version stamp under the user's home + if err != nil { + return "" + } + return strings.TrimSpace(string(data)) +} + +func copySkillFiles(src, dst string) error { + // Claim the directory first: removeExistingSkillLink already proved dst is + // ours to (re)create, and marking before copying means a partially copied + // directory is still recognized and replaced on the next attempt. + if err := claimSkillDir(dst); err != nil { + return err + } + // Copy only the files hey-cli owns. A user may add their own files to + // the managed baseline; carrying them into the copy would make it fail + // its own allowlist on the next run and strand the fallback. + for _, name := range []string{skillFilename, installedVersionFile, ownershipMarkerFile} { + data, err := os.ReadFile(filepath.Join(src, name)) // #nosec G304 -- fixed filenames in the baseline skill directory we wrote + if err != nil { + if os.IsNotExist(err) { + continue // the version stamp and marker are best-effort + } + return err + } + if err := writeSkillFile(filepath.Join(dst, name), data); err != nil { + return err + } + } + return nil } diff --git a/internal/cmd/skill_install_test.go b/internal/cmd/skill_install_test.go index 51985a2f8..18c30d3d3 100644 --- a/internal/cmd/skill_install_test.go +++ b/internal/cmd/skill_install_test.go @@ -2,6 +2,7 @@ package cmd import ( "bytes" + "errors" "os" "path/filepath" "testing" @@ -9,6 +10,8 @@ import ( "github.com/basecamp/hey-cli/internal/output" ) +// A stale hey-cli link (canonical target, baseline since removed) is ours to +// replace. A link to anywhere else is the user's — see the refusal test below. func TestSkillInstallReplacesExistingLink(t *testing.T) { home := t.TempDir() t.Setenv("HOME", home) @@ -18,7 +21,7 @@ func TestSkillInstallReplacesExistingLink(t *testing.T) { t.Fatal(err) } symlinkPath := filepath.Join(symlinkDir, "hey") - if err := os.Symlink("old-target", symlinkPath); err != nil { + if err := os.Symlink(claudeSkillLinkTarget, symlinkPath); err != nil { t.Fatal(err) } @@ -64,3 +67,326 @@ func TestSkillInstallReportsExistingDirectory(t *testing.T) { t.Fatalf("existing skill content changed: %v", err) } } + +// Where symlinks are unavailable (Windows without the privilege), the copy +// fallback leaves a real, populated directory at ~/.claude/skills/hey. The +// next install must replace that directory — our own files only — while the +// existing test above pins that unknown user content is still refused. +func TestSkillInstallCopyFallbackIsIdempotent(t *testing.T) { + home := t.TempDir() + t.Setenv("HOME", home) + t.Setenv("USERPROFILE", home) + t.Setenv("PATH", t.TempDir()) + t.Setenv("CODEX_HOME", "") + if err := os.MkdirAll(filepath.Join(home, ".claude"), 0o755); err != nil { + t.Fatal(err) + } + + origSymlink := makeSkillSymlink + makeSkillSymlink = func(string, string) error { return errors.New("symlinks unavailable") } + t.Cleanup(func() { makeSkillSymlink = origSymlink }) + + previousWriter := writer + t.Cleanup(func() { writer = previousWriter }) + writer = output.New(output.Options{Format: output.FormatJSON, Stdout: &bytes.Buffer{}}) + + for run := 1; run <= 2; run++ { + if err := runSkillInstall(newSkillInstallCommand(), nil); err != nil { + t.Fatalf("run %d: %v", run, err) + } + copyPath := filepath.Join(home, ".claude", "skills", "hey") + info, err := os.Lstat(copyPath) + if err != nil { + t.Fatalf("run %d: %v", run, err) + } + if info.Mode()&os.ModeSymlink != 0 || !info.IsDir() { + t.Fatalf("run %d: expected a copied directory, got mode %v", run, info.Mode()) + } + if _, err := os.Stat(filepath.Join(copyPath, "SKILL.md")); err != nil { + t.Fatalf("run %d: copied skill missing: %v", run, err) + } + } +} + +// Shape is not ownership: a user-authored ~/.claude/skills/hey holding only a +// SKILL.md — the exact shape our copy fallback produces, minus the marker — +// must be refused and preserved, not silently replaced by a symlink. +func TestSkillInstallRefusesUnmarkedSkillOnlyDirectory(t *testing.T) { + home := t.TempDir() + t.Setenv("HOME", home) + t.Setenv("USERPROFILE", home) + skillDir := filepath.Join(home, ".claude", "skills", "hey") + if err := os.MkdirAll(skillDir, 0o755); err != nil { + t.Fatal(err) + } + custom := "# my own hey skill" + if err := os.WriteFile(filepath.Join(skillDir, "SKILL.md"), []byte(custom), 0o600); err != nil { + t.Fatal(err) + } + + previousWriter := writer + t.Cleanup(func() { writer = previousWriter }) + writer = output.New(output.Options{Format: output.FormatJSON, Stdout: &bytes.Buffer{}}) + + if err := runSkillInstall(newSkillInstallCommand(), nil); err == nil { + t.Fatal("install replaced a user-authored skill directory") + } + info, err := os.Lstat(skillDir) + if err != nil || !info.IsDir() || info.Mode()&os.ModeSymlink != 0 { + t.Fatalf("user directory replaced: %v %v", info, err) + } + data, err := os.ReadFile(filepath.Join(skillDir, "SKILL.md")) + if err != nil || string(data) != custom { + t.Fatalf("user skill content changed: %q, %v", data, err) + } +} + +func jsonWriter(t *testing.T) { + t.Helper() + previousWriter := writer + t.Cleanup(func() { writer = previousWriter }) + writer = output.New(output.Options{Format: output.FormatJSON, Stdout: &bytes.Buffer{}}) +} + +func agentHome(t *testing.T, dirs ...string) string { + t.Helper() + home := t.TempDir() + t.Setenv("HOME", home) + t.Setenv("USERPROFILE", home) + t.Setenv("PATH", t.TempDir()) + t.Setenv("CODEX_HOME", "") + for _, dir := range dirs { + if err := os.MkdirAll(filepath.Join(home, dir), 0o755); err != nil { + t.Fatal(err) + } + } + return home +} + +// Only hey-cli's own canonical symlink may be replaced. A user's symlink to +// their own skill, or a regular file, is refused and left exactly as found. +func TestSkillInstallRefusesForeignLinkOrFile(t *testing.T) { + for name, plant := range map[string]func(path string) error{ + "foreign symlink": func(path string) error { return os.Symlink("../../my-skills/hey", path) }, + "regular file": func(path string) error { return os.WriteFile(path, []byte("# mine"), 0o600) }, + } { + t.Run(name, func(t *testing.T) { + home := agentHome(t, ".claude/skills") + path := filepath.Join(home, ".claude", "skills", "hey") + if err := plant(path); err != nil { + t.Fatal(err) + } + before, _ := os.Lstat(path) + jsonWriter(t) + + if err := runSkillInstall(newSkillInstallCommand(), nil); err == nil { + t.Fatal("install replaced user state") + } + after, err := os.Lstat(path) + if err != nil || after.Mode() != before.Mode() { + t.Fatalf("user state disturbed: %v, %v", after, err) + } + if before.Mode()&os.ModeSymlink != 0 { + if target, _ := os.Readlink(path); target != "../../my-skills/hey" { + t.Errorf("symlink retargeted to %q", target) + } + } + }) + } +} + +// The canonical baseline and Codex paths are conventions agents share, so a +// hand-authored skill can legitimately live there. Every install path — the +// explicit command and the installer's automatic `setup agents` alike — must +// refuse an unmarked one rather than overwrite it and then claim it. +func TestSkillInstallRefusesUnmarkedBaselineAndCodexSkills(t *testing.T) { + home := agentHome(t, ".codex") + custom := "# my own hey skill\n" + baseline := filepath.Join(home, ".agents", "skills", "hey") + codex := filepath.Join(home, ".codex", "skills", "hey") + for _, dir := range []string{baseline, codex} { + if err := os.MkdirAll(dir, 0o755); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(dir, "SKILL.md"), []byte(custom), 0o600); err != nil { + t.Fatal(err) + } + } + + var unmanaged *unmanagedSkillDirError + if _, err := installSkillFiles(); !errors.As(err, &unmanaged) { + t.Fatalf("installSkillFiles error = %v, want unmanaged refusal", err) + } + if _, err := installSkillToCodex(); !errors.As(err, &unmanaged) { + t.Fatalf("installSkillToCodex error = %v, want unmanaged refusal", err) + } + for _, dir := range []string{baseline, codex} { + data, err := os.ReadFile(filepath.Join(dir, "SKILL.md")) + if err != nil || string(data) != custom { + t.Errorf("%s changed: %q, %v", dir, data, err) + } + if ownedSkillDir(dir) { + t.Errorf("%s was claimed", dir) + } + } +} + +// An empty directory at the install path is not user content and is claimed. +func TestClaimSkillDirAcceptsEmptyAndMarkedDirectories(t *testing.T) { + home := agentHome(t) + empty := filepath.Join(home, "empty") + if err := os.MkdirAll(empty, 0o755); err != nil { + t.Fatal(err) + } + if err := claimSkillDir(empty); err != nil || !ownedSkillDir(empty) { + t.Errorf("empty dir: %v, owned=%v", err, ownedSkillDir(empty)) + } + if err := claimSkillDir(empty); err != nil { + t.Errorf("re-claiming our own dir: %v", err) + } + missing := filepath.Join(home, "missing", "hey") + if err := claimSkillDir(missing); err != nil || !ownedSkillDir(missing) { + t.Errorf("missing dir: %v, owned=%v", err, ownedSkillDir(missing)) + } +} + +// A marker only proves the directory; each file write still refuses to +// follow a symlink planted inside it — a link's target was never inspected, +// and truncating it would destroy a file we do not own. +func TestSkillInstallRefusesSymlinkedSkillFileInManagedDir(t *testing.T) { + home := agentHome(t, ".codex") + precious := filepath.Join(home, "precious.md") + if err := os.WriteFile(precious, []byte("# do not truncate"), 0o644); err != nil { + t.Fatal(err) + } + for _, dir := range []string{ + filepath.Join(home, ".agents", "skills", "hey"), + filepath.Join(home, ".codex", "skills", "hey"), + } { + if err := os.MkdirAll(dir, 0o755); err != nil { + t.Fatal(err) + } + writeOwnershipMarker(dir) + if err := os.Symlink(precious, filepath.Join(dir, "SKILL.md")); err != nil { + t.Fatal(err) + } + } + + if _, err := installSkillFiles(); err == nil { + t.Error("baseline install wrote through a symlinked SKILL.md") + } + if _, err := installSkillToCodex(); err == nil { + t.Error("Codex install wrote through a symlinked SKILL.md") + } + if data, _ := os.ReadFile(precious); string(data) != "# do not truncate" { + t.Errorf("symlink target was truncated: %q", data) + } +} + +// A marker planted as a symlink or directory confers no ownership, so the +// unmanaged-content refusal still protects the regular SKILL.md next to it. +func TestClaimSkillDirRejectsNonRegularMarker(t *testing.T) { + home := agentHome(t) + dir := filepath.Join(home, "skills", "hey") + if err := os.MkdirAll(dir, 0o755); err != nil { + t.Fatal(err) + } + custom := "# my own hey skill" + if err := os.WriteFile(filepath.Join(dir, "SKILL.md"), []byte(custom), 0o600); err != nil { + t.Fatal(err) + } + if err := os.Mkdir(filepath.Join(dir, ownershipMarkerFile), 0o755); err != nil { + t.Fatal(err) + } + + var unmanaged *unmanagedSkillDirError + if err := claimSkillDir(dir); !errors.As(err, &unmanaged) { + t.Fatalf("claimSkillDir = %v, want unmanaged refusal", err) + } + if got, _ := os.ReadFile(filepath.Join(dir, "SKILL.md")); string(got) != custom { + t.Errorf("user skill changed: %q", got) + } +} + +// The copy-fallback removal predicate applies the regular-file rule to every +// entry: a symlink planted in the marker's name authorizes nothing, and the +// user's SKILL.md beside it survives Claude setup. +func TestRemoveExistingSkillLinkRejectsSymlinkedMarker(t *testing.T) { + home := agentHome(t) + dir := filepath.Join(home, ".claude", "skills", "hey") + if err := os.MkdirAll(dir, 0o755); err != nil { + t.Fatal(err) + } + custom := "# my own hey skill" + if err := os.WriteFile(filepath.Join(dir, "SKILL.md"), []byte(custom), 0o600); err != nil { + t.Fatal(err) + } + target := filepath.Join(home, "marker-target") + if err := os.WriteFile(target, []byte("x"), 0o644); err != nil { + t.Fatal(err) + } + if err := os.Symlink(target, filepath.Join(dir, ownershipMarkerFile)); err != nil { + t.Fatal(err) + } + + var unmanaged *unmanagedSkillDirError + if err := removeExistingSkillLink(dir); !errors.As(err, &unmanaged) { + t.Fatalf("removeExistingSkillLink = %v, want unmanaged refusal", err) + } + if got, _ := os.ReadFile(filepath.Join(dir, "SKILL.md")); string(got) != custom { + t.Errorf("user skill deleted: %q", got) + } +} + +// A user file added to the managed baseline is not carried into the Claude +// copy, so consecutive fallback installs stay repeatable. +func TestCopyFallbackSkipsUserFilesInBaseline(t *testing.T) { + home := agentHome(t) + if err := os.MkdirAll(filepath.Join(home, ".claude"), 0o755); err != nil { + t.Fatal(err) + } + origSymlink := makeSkillSymlink + makeSkillSymlink = func(string, string) error { return errors.New("symlinks unavailable") } + t.Cleanup(func() { makeSkillSymlink = origSymlink }) + jsonWriter(t) + + if err := runSkillInstall(newSkillInstallCommand(), nil); err != nil { + t.Fatalf("first install: %v", err) + } + // The user adds their own file to the managed baseline. + baseline := filepath.Join(home, ".agents", "skills", "hey") + if err := os.WriteFile(filepath.Join(baseline, "notes.txt"), []byte("mine"), 0o644); err != nil { + t.Fatal(err) + } + for run := 2; run <= 3; run++ { + if err := runSkillInstall(newSkillInstallCommand(), nil); err != nil { + t.Fatalf("run %d: %v", run, err) + } + } + if _, err := os.Stat(filepath.Join(home, ".claude", "skills", "hey", "notes.txt")); !os.IsNotExist(err) { + t.Error("user file was copied into the Claude skill directory") + } +} + +// Claude is never linked to a baseline the install paths refused to claim. +func TestLinkSkillToClaudeRequiresManagedBaseline(t *testing.T) { + home := agentHome(t) + if err := os.MkdirAll(filepath.Join(home, ".claude"), 0o755); err != nil { + t.Fatal(err) + } + dir := filepath.Join(home, ".agents", "skills", "hey") + if err := os.MkdirAll(dir, 0o755); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(dir, "SKILL.md"), []byte("# mine"), 0o600); err != nil { + t.Fatal(err) + } + + var unmanaged *unmanagedSkillDirError + if _, err := linkSkillToClaude(); !errors.As(err, &unmanaged) { + t.Fatalf("linkSkillToClaude = %v, want unmanaged refusal", err) + } + if _, err := os.Lstat(filepath.Join(home, ".claude", "skills", "hey")); !os.IsNotExist(err) { + t.Error("a link to unmanaged content was created") + } +} diff --git a/internal/cmd/skill_refresh.go b/internal/cmd/skill_refresh.go new file mode 100644 index 000000000..41d743202 --- /dev/null +++ b/internal/cmd/skill_refresh.go @@ -0,0 +1,151 @@ +package cmd + +import ( + "fmt" + "os" + "path/filepath" + + "github.com/spf13/cobra" + + "github.com/basecamp/hey-cli/internal/config" + "github.com/basecamp/hey-cli/internal/harness" + "github.com/basecamp/hey-cli/internal/version" + "github.com/basecamp/hey-cli/skills" +) + +// maybeRefreshSkills runs after every command: when the CLI version changed +// since the last run, installed skill copies are silently brought up to date +// so agents never read instructions for a binary that no longer matches. +// The notice is stderr-only and suppressed for machine output. +func maybeRefreshSkills(cmd *cobra.Command) { + if !refreshSkillsIfVersionChanged() { + return + } + // Machine consumers get no stderr chatter — whether they asked with a + // flag or got JSON automatically because stdout is not a terminal. + if machineReadableOutput(cmd) || (writer != nil && !writer.IsStyled()) { + return + } + fmt.Fprintf(os.Stderr, "Agent skill updated to match CLI %s\n", version.Version) + + // One-time hint: a plugin/CLI version mismatch right after an upgrade + // means auto-update is off in Claude Code. + if pv := harness.InstalledPluginVersion(); pv != "" && pv != version.Version && !version.IsDev() { + fmt.Fprintf(os.Stderr, "hey plugin version mismatch (plugin %s, CLI %s) — %s\n", pv, version.Version, harness.AutoUpdateHint) + } +} + +// refreshSkillsIfVersionChanged checks the CLI version sentinel and refreshes +// installed skills when the version changed. Reports whether a refresh ran. +func refreshSkillsIfVersionChanged() bool { + if version.IsDev() { + return false + } + + configDir := config.ConfigDir() + if configDir == "" { + return false // no home to keep state in; never drop a sentinel in the cwd + } + sentinelPath := filepath.Join(configDir, ".last-run-version") + + // The sentinel records the active Codex home alongside the version: + // skill locations depend on CODEX_HOME, so switching homes mid-release + // triggers one rescan instead of leaving the other home's marked skill + // stale until the next release. + sentinelState := version.Version + "\n" + harness.CodexHome() + "\n" + data, err := os.ReadFile(sentinelPath) // #nosec G304 -- fixed path under the user config dir + if err == nil && string(data) == sentinelState { + return false + } + + updated, failed := refreshInstalledSkills() + + // Advance the sentinel unless something failed: nothing owned to refresh + // and a fully successful refresh both mean this version is done. On + // transient failure, leave it stale so the next run retries. + if failed != 0 { + return false + } + // 0o700: ConfigDir can hold credentials.json; keep it owner-only. + // writeSkillFile refuses to follow a symlink planted at the sentinel + // path, the same rule as every other file this feature writes. + _ = os.MkdirAll(filepath.Dir(sentinelPath), 0o700) + if err := writeSkillFile(sentinelPath, []byte(sentinelState)); err != nil { + // Without the sentinel every later run rescans; report no completed + // refresh so the update notice cannot repeat on each of them. + return false + } + + return updated > 0 +} + +// skillRefreshLocations lists every absolute path an agent reads the hey +// skill from. Only existing files in directories hey-cli owns (per the +// ownership marker) are refreshed — refresh never installs, and never +// rewrites a skill it cannot prove it wrote. +func skillRefreshLocations() []string { + var locations []string + if home, err := os.UserHomeDir(); err == nil && home != "" { + locations = append(locations, + filepath.Join(home, ".agents", "skills", "hey", skillFilename), + filepath.Join(home, ".claude", "skills", "hey", skillFilename), + ) + } + if codexPath := harness.CodexSkillPath(); codexPath != "" { + locations = append(locations, codexPath) + } + return locations +} + +func refreshInstalledSkills() (updated, failed int) { + embedded, err := skills.FS.ReadFile("hey/SKILL.md") + if err != nil { + return 0, 1 + } + + for _, location := range skillRefreshLocations() { + info, statErr := os.Lstat(location) + if statErr != nil { + if !os.IsNotExist(statErr) { + failed++ // permission or IO error on a known location + } + continue + } + // Never write through a symlink — neither a linked SKILL.md nor a + // linked directory (our own Claude link resolves to the baseline, + // which is refreshed as itself; a user's link resolves to somewhere + // we never inspected). + if info.Mode()&os.ModeSymlink != 0 || !info.Mode().IsRegular() || isSymlink(filepath.Dir(location)) { + continue + } + // Ownership gate: a SKILL.md in an unmarked directory is somebody + // else's work — a hand-authored skill that happens to share the + // name — and is left exactly as found. + if !ownedSkillDir(filepath.Dir(location)) { + continue + } + if writeErr := os.WriteFile(location, embedded, 0o644); writeErr == nil { // #nosec G306 -- installed skills are intentionally user-readable + updated++ + } else { + failed++ + } + } + + // Stamp the installed version in the baseline directory only on full + // success, and only when that directory is ours to stamp. + if failed == 0 && updated > 0 { + if home, err := os.UserHomeDir(); err == nil { + baselineDir := filepath.Join(home, ".agents", "skills", "hey") + if ownedSkillDir(baselineDir) { + _ = writeSkillFile(filepath.Join(baselineDir, installedVersionFile), []byte(version.Version)) + } + } + } + + return updated, failed +} + +func isSymlink(path string) bool { + info, err := os.Lstat(path) + return err == nil && info.Mode()&os.ModeSymlink != 0 +} diff --git a/internal/cmd/skill_refresh_test.go b/internal/cmd/skill_refresh_test.go new file mode 100644 index 000000000..a893a89d8 --- /dev/null +++ b/internal/cmd/skill_refresh_test.go @@ -0,0 +1,334 @@ +package cmd + +import ( + "os" + "path/filepath" + "strings" + "testing" + + "github.com/basecamp/hey-cli/skills" +) + +func refreshFixture(t *testing.T) (home string) { + t.Helper() + home = t.TempDir() + t.Setenv("HOME", home) + t.Setenv("USERPROFILE", home) + t.Setenv("XDG_CONFIG_HOME", filepath.Join(home, ".config")) + t.Setenv("CODEX_HOME", "") + return home +} + +// installStaleSkill lays down a hey-cli-written baseline install (marker and +// all) whose content is stale. +func installStaleSkill(t *testing.T, home string) string { + t.Helper() + return writeSkillFixture(t, filepath.Join(home, ".agents", "skills", "hey"), "# stale skill", true) +} + +// writeSkillFixture writes dir/SKILL.md with the given content, marked as +// hey-cli-managed or not. +func writeSkillFixture(t *testing.T, dir, content string, managed bool) string { + t.Helper() + if err := os.MkdirAll(dir, 0o755); err != nil { + t.Fatal(err) + } + skillPath := filepath.Join(dir, "SKILL.md") + if err := os.WriteFile(skillPath, []byte(content), 0o644); err != nil { + t.Fatal(err) + } + if managed { + writeOwnershipMarker(dir) + } + return skillPath +} + +func TestRefreshSkillsSkipsDevBuilds(t *testing.T) { + refreshFixture(t) + stubVersion(t, "dev") + if refreshSkillsIfVersionChanged() { + t.Error("dev builds must never refresh skills") + } +} + +func TestRefreshSkillsUpdatesInstalledCopiesOnce(t *testing.T) { + home := refreshFixture(t) + stubVersion(t, "1.2.3") + skillPath := installStaleSkill(t, home) + + // A Codex copy must be refreshed too. + codexSkill := writeSkillFixture(t, filepath.Join(home, ".codex", "skills", "hey"), "# stale skill", true) + + if !refreshSkillsIfVersionChanged() { + t.Fatal("first run on a new version should refresh") + } + + embedded, err := skills.FS.ReadFile("hey/SKILL.md") + if err != nil { + t.Fatal(err) + } + for _, path := range []string{skillPath, codexSkill} { + data, err := os.ReadFile(path) + if err != nil { + t.Fatal(err) + } + if string(data) != string(embedded) { + t.Errorf("%s was not refreshed", path) + } + } + + stamp, err := os.ReadFile(filepath.Join(filepath.Dir(skillPath), installedVersionFile)) + if err != nil || strings.TrimSpace(string(stamp)) != "1.2.3" { + t.Errorf("installed version stamp = %q, %v", stamp, err) + } + + sentinel, err := os.ReadFile(filepath.Join(home, ".config", "hey-cli", ".last-run-version")) + if err != nil || !strings.HasPrefix(string(sentinel), "1.2.3\n") { + t.Errorf("sentinel = %q, %v", sentinel, err) + } + + if refreshSkillsIfVersionChanged() { + t.Error("second run on the same version must be a no-op") + } +} + +func TestRefreshSkillsWithoutInstallNeverInstalls(t *testing.T) { + home := refreshFixture(t) + stubVersion(t, "1.2.3") + + if refreshSkillsIfVersionChanged() { + t.Error("nothing installed, nothing to refresh") + } + if _, err := os.Stat(filepath.Join(home, ".agents", "skills", "hey", "SKILL.md")); !os.IsNotExist(err) { + t.Error("refresh must never install the skill") + } + // The sentinel still advances so every future run doesn't rescan. + if _, err := os.Stat(filepath.Join(home, ".config", "hey-cli", ".last-run-version")); err != nil { + t.Errorf("sentinel not written: %v", err) + } +} + +// Refresh never touches symlinks: a dangling ~/.claude/skills/hey — a user's +// link to a volume that is merely unmounted right now — survives untouched, +// even when a marked baseline exists next to it. +func TestRefreshSkillsPreservesForeignBrokenClaudeLink(t *testing.T) { + home := refreshFixture(t) + stubVersion(t, "9.9.9") + installStaleSkill(t, home) + + claudeSkills := filepath.Join(home, ".claude", "skills") + if err := os.MkdirAll(claudeSkills, 0o755); err != nil { + t.Fatal(err) + } + linkPath := filepath.Join(claudeSkills, "hey") + foreign := "../../../Volumes/offline/custom-hey-skill" + if err := os.Symlink(foreign, linkPath); err != nil { + t.Fatal(err) + } + + if !refreshSkillsIfVersionChanged() { + t.Fatal("the marked baseline should still refresh") + } + target, err := os.Readlink(linkPath) + if err != nil || target != foreign { + t.Errorf("foreign broken link was replaced: target = %q, %v", target, err) + } +} + +// Our own canonical link over a marked baseline passes through the refresh +// intact and still resolves afterwards. +func TestRefreshSkillsKeepsCanonicalClaudeLink(t *testing.T) { + home := refreshFixture(t) + stubVersion(t, "9.9.9") + installStaleSkill(t, home) + + claudeSkills := filepath.Join(home, ".claude", "skills") + if err := os.MkdirAll(claudeSkills, 0o755); err != nil { + t.Fatal(err) + } + linkPath := filepath.Join(claudeSkills, "hey") + if err := os.Symlink(claudeSkillLinkTarget, linkPath); err != nil { + t.Fatal(err) + } + + if !refreshSkillsIfVersionChanged() { + t.Fatal("refresh should run") + } + if target, err := os.Readlink(linkPath); err != nil || target != claudeSkillLinkTarget { + t.Errorf("canonical link disturbed: %q, %v", target, err) + } + if _, err := os.Stat(filepath.Join(linkPath, "SKILL.md")); err != nil { + t.Errorf("canonical link no longer resolves: %v", err) + } +} + +// Refresh must prove ownership before rewriting: a SKILL.md in a directory +// without our marker is somebody else's skill that shares the name, and a new +// release running any command must leave it byte-for-byte alone. +func TestRefreshSkillsPreservesUnmanagedSkills(t *testing.T) { + home := refreshFixture(t) + stubVersion(t, "9.9.9") + + custom := "# my own hey skill\n\nhand-authored, not hey-cli's\n" + locations := []string{ + 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), + } + + if refreshSkillsIfVersionChanged() { + t.Error("nothing owned: refresh must report no work done") + } + for _, path := range locations { + data, err := os.ReadFile(path) + if err != nil { + t.Fatal(err) + } + if string(data) != custom { + t.Errorf("%s was rewritten without ownership", path) + } + } + // The sentinel still advances — leaving foreign skills alone is this + // version's finished state, not a failure to retry. + sentinel, err := os.ReadFile(filepath.Join(home, ".config", "hey-cli", ".last-run-version")) + if err != nil || !strings.HasPrefix(string(sentinel), "9.9.9\n") { + t.Errorf("sentinel = %q, %v", sentinel, err) + } +} + +// Refresh never writes through a symlink, even inside a marked directory: a +// SKILL.md that is itself a link, or a skill directory that is a link, points +// somewhere hey-cli never inspected. +func TestRefreshSkillsNeverWritesThroughSymlinks(t *testing.T) { + home := refreshFixture(t) + stubVersion(t, "9.9.9") + + // Marked baseline whose SKILL.md is a link to the user's real file. + baseline := filepath.Join(home, ".agents", "skills", "hey") + if err := os.MkdirAll(baseline, 0o755); err != nil { + t.Fatal(err) + } + writeOwnershipMarker(baseline) + custom := "# the user's real skill\n" + realSkill := filepath.Join(home, "real-skill.md") + if err := os.WriteFile(realSkill, []byte(custom), 0o644); err != nil { + t.Fatal(err) + } + if err := os.Symlink(realSkill, filepath.Join(baseline, "SKILL.md")); err != nil { + t.Fatal(err) + } + + // Codex skill directory that is itself a link to a user directory. + elsewhere := writeSkillFixture(t, filepath.Join(home, "elsewhere"), custom, true) + if err := os.MkdirAll(filepath.Join(home, ".codex", "skills"), 0o755); err != nil { + t.Fatal(err) + } + if err := os.Symlink(filepath.Dir(elsewhere), filepath.Join(home, ".codex", "skills", "hey")); err != nil { + t.Fatal(err) + } + + if refreshSkillsIfVersionChanged() { + t.Error("nothing writable: refresh must report no work done") + } + for _, path := range []string{realSkill, elsewhere} { + if got, _ := os.ReadFile(path); string(got) != custom { + t.Errorf("%s was written through a symlink", path) + } + } +} + +// With no config directory at all there is nowhere to keep the sentinel, and +// it must not land in the current directory. +func TestRefreshSkillsSkipsWithoutConfigDir(t *testing.T) { + stubVersion(t, "9.9.9") + t.Setenv("XDG_CONFIG_HOME", "") + t.Setenv("HOME", "") + t.Setenv("USERPROFILE", "") + cwd := t.TempDir() + t.Chdir(cwd) + + if refreshSkillsIfVersionChanged() { + t.Error("refresh should not run without a config dir") + } + if _, err := os.Stat(filepath.Join(cwd, ".last-run-version")); !os.IsNotExist(err) { + t.Error("sentinel written into the current directory") + } +} + +// The sentinel tracks the active Codex home: a marked, stale skill in a +// Codex home that was inactive during the first post-upgrade run is +// refreshed 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") + } + + homeB := t.TempDir() + staleB := writeSkillFixture(t, filepath.Join(homeB, "skills", "hey"), "# stale skill", true) + t.Setenv("CODEX_HOME", homeB) + if !refreshSkillsIfVersionChanged() { + t.Fatal("switching Codex homes should rescan") + } + data, err := os.ReadFile(staleB) + if err != nil || string(data) == "# stale skill" { + t.Errorf("skill in the newly active Codex home was not refreshed: %v", 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 +// writes: a symlink planted at its path is never truncated. +func TestRefreshSkillsNeverWritesThroughSymlinkedSentinel(t *testing.T) { + home := refreshFixture(t) + stubVersion(t, "9.9.9") + + configDir := filepath.Join(home, ".config", "hey-cli") + if err := os.MkdirAll(configDir, 0o700); err != nil { + t.Fatal(err) + } + precious := filepath.Join(home, "precious") + if err := os.WriteFile(precious, []byte("do not truncate"), 0o644); err != nil { + t.Fatal(err) + } + if err := os.Symlink(precious, filepath.Join(configDir, ".last-run-version")); err != nil { + t.Fatal(err) + } + + refreshSkillsIfVersionChanged() + if got, _ := os.ReadFile(precious); string(got) != "do not truncate" { + t.Errorf("sentinel write followed a symlink: %q", got) + } +} + +// A refresh whose sentinel cannot be written is not complete: reporting it +// as one would repeat the update notice on every subsequent command. +func TestRefreshSkillsSentinelFailureIsNotCompletion(t *testing.T) { + home := refreshFixture(t) + stubVersion(t, "9.9.9") + installStaleSkill(t, home) + + // The sentinel's parent exists but is unwritable. + configDir := filepath.Join(home, ".config", "hey-cli") + if err := os.MkdirAll(configDir, 0o755); err != nil { + t.Fatal(err) + } + if err := os.Chmod(configDir, 0o500); err != nil { + t.Fatal(err) + } + t.Cleanup(func() { _ = os.Chmod(configDir, 0o700) }) + + if refreshSkillsIfVersionChanged() { + t.Error("a refresh without a sentinel must not report completion") + } +} diff --git a/internal/cmd/tui.go b/internal/cmd/tui.go index cda4db38a..55606ba5d 100644 --- a/internal/cmd/tui.go +++ b/internal/cmd/tui.go @@ -2,8 +2,6 @@ package cmd import ( "github.com/spf13/cobra" - - "github.com/basecamp/hey-cli/internal/tui" ) type tuiCommand struct { @@ -27,7 +25,7 @@ func newTuiRunner(use string, hidden bool) *cobra.Command { if err := requireAuth(); err != nil { return err } - return tui.Run(rootSDK, sdk, cfg.AccountID, tuiWatchers()) + return runTUI(rootSDK, sdk, cfg.AccountID, tuiWatchers()) }, } } diff --git a/internal/config/config.go b/internal/config/config.go index eb9dd4c7c..7b706785b 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -38,6 +38,7 @@ type Value struct { type Config struct { BaseURL string `json:"base_url"` AccountID string `json:"account_id,omitempty"` + Onboarded bool `json:"onboarded,omitempty"` sources map[string]Source globalConfig fileConfig @@ -48,6 +49,7 @@ type Config struct { type fileConfig struct { BaseURL string `json:"base_url,omitempty"` AccountID string `json:"account_id,omitempty"` + Onboarded *bool `json:"onboarded,omitempty"` Cover string `json:"cover,omitempty"` AccountDefaults map[string]string `json:"account_defaults,omitempty"` TrustedLocalConfigs map[string]trustedLocalConfigRecord `json:"trusted_local_configs,omitempty"` @@ -167,6 +169,11 @@ func load(localPath string) (*Config, error) { cfg.BaseURL = global.BaseURL cfg.sources["base_url"] = SourceGlobal } + // Onboarded is deliberately global-only: a repository's .hey/config.json + // must not be able to suppress (or force) the first-run wizard. + if global.Onboarded != nil { + cfg.Onboarded = *global.Onboarded + } var local fileConfig if localPath != "" { @@ -393,6 +400,21 @@ func SaveCover(preset string) error { return saveGlobalConfig(file) } +// SaveOnboarded stores the onboarding flag in the global config so later +// logged-out runs of bare `hey` skip the full first-run wizard. +func (c *Config) SaveOnboarded(onboarded bool) error { + file, err := loadGlobalFileConfig() + if err != nil { + return err + } + file.Onboarded = &onboarded + if err := saveGlobalConfig(file); err != nil { + return err + } + c.Onboarded = onboarded + return nil +} + // SaveAccountID stores the default linked-account filter for the effective server // without replacing settings for any other server. func (c *Config) SaveAccountID(accountID string) error { @@ -441,7 +463,36 @@ func migrateLegacyAccountDefault(file *fileConfig) error { return nil } +// fileConfigKeys are the keys the fileConfig schema owns. A rewrite replaces +// exactly these; every other key in the file is preserved verbatim. +var fileConfigKeys = []string{"base_url", "account_id", "onboarded", "cover", "account_defaults", "trusted_local_configs"} + +// legacyCredentialKeys are the embedded-credential fields of the pre-store +// config format. They survive every ordinary rewrite (deleting a credential +// is never a side effect) and are removed only by ScrubLegacyCredentials +// after a successful migration. +var legacyCredentialKeys = []string{"access_token", "refresh_token", "token_expiry", "client_id", "client_secret", "install_id", "session_cookie"} + +// ScrubLegacyCredentials removes migrated embedded credentials from the +// global config file. Call only after the credential store confirmed the +// save. +func ScrubLegacyCredentials() error { + file, err := loadGlobalFileConfig() + if err != nil { + return err + } + return saveGlobalConfigScrubbing(file, legacyCredentialKeys) +} + func saveGlobalConfig(file fileConfig) error { + return saveGlobalConfigScrubbing(file, nil) +} + +// saveGlobalConfigScrubbing rewrites the schema-owned keys while carrying +// every unknown key through verbatim — a config rewrite must never delete +// state it does not understand (unmigrated legacy credentials, a newer +// version's settings) — except the keys explicitly listed in scrub. +func saveGlobalConfigScrubbing(file fileConfig, scrub []string) error { path := globalConfigPath() if path == "" { return fmt.Errorf("could not determine config path") @@ -454,7 +505,34 @@ func saveGlobalConfig(file fileConfig) error { return fmt.Errorf("could not secure config directory: %w", err) } - data, err := json.MarshalIndent(file, "", " ") + merged := map[string]json.RawMessage{} + if existing, readErr := os.ReadFile(path); readErr == nil { // #nosec G304 -- the fixed global config path + // Best-effort: an unparsable file is replaced by the schema keys. + _ = json.Unmarshal(existing, &merged) + if merged == nil { + // A literal JSON null decodes to a nil map without error. + merged = map[string]json.RawMessage{} + } + } + for _, key := range fileConfigKeys { + delete(merged, key) + } + for _, key := range scrub { + delete(merged, key) + } + known, err := json.Marshal(file) + if err != nil { + return fmt.Errorf("could not marshal config: %w", err) + } + var knownMap map[string]json.RawMessage + if unmarshalErr := json.Unmarshal(known, &knownMap); unmarshalErr != nil { + return fmt.Errorf("could not merge config: %w", unmarshalErr) + } + for key, value := range knownMap { + merged[key] = value + } + + data, err := json.MarshalIndent(merged, "", " ") if err != nil { return fmt.Errorf("could not marshal config: %w", err) } @@ -481,6 +559,35 @@ func saveGlobalConfig(file fileConfig) error { return nil } +// NonInteractiveEnv reports whether HEY_NONINTERACTIVE is set to a truthy +// value. When true, the CLI must not show interactive prompts regardless of +// TTY detection. This is an escape hatch for agents and harnesses that run +// the CLI under an allocated PTY (where stdio looks like a terminal) and want +// to avoid a prompt wedging the session — without forcing a machine output +// format the way --agent does. +func NonInteractiveEnv() bool { + if v := os.Getenv("HEY_NONINTERACTIVE"); v != "" { + if b, ok := parseEnvBool(v); ok { + return b + } + } + return false +} + +// parseEnvBool parses a boolean environment variable strictly. Returns +// (value, true) for recognized values, (false, false) for unrecognized ones, +// which are ignored rather than guessed at. +func parseEnvBool(v string) (bool, bool) { + switch strings.ToLower(v) { + case "true", "1": + return true, true + case "false", "0": + return false, true + default: + return false, false + } +} + func normalizeAccountID(accountID string) (string, error) { accountID = strings.TrimSpace(accountID) if strings.EqualFold(accountID, AllAccounts) { diff --git a/internal/config/config_test.go b/internal/config/config_test.go index c00b13c08..e2018c086 100644 --- a/internal/config/config_test.go +++ b/internal/config/config_test.go @@ -558,3 +558,179 @@ func TestSaveCoverKeepsOtherSettings(t *testing.T) { t.Errorf("cover = %q, want peace", after.Cover) } } + +func TestOnboardedRoundTrip(t *testing.T) { + tmp := t.TempDir() + t.Setenv("XDG_CONFIG_HOME", tmp) + t.Setenv("HEY_BASE_URL", "") + + cfg, err := Load() + if err != nil { + t.Fatalf("Load: %v", err) + } + if cfg.Onboarded { + t.Error("fresh config should not be onboarded") + } + + if err := cfg.SaveOnboarded(true); err != nil { + t.Fatalf("SaveOnboarded: %v", err) + } + if !cfg.Onboarded { + t.Error("SaveOnboarded should update the in-memory config") + } + + data, err := os.ReadFile(filepath.Join(tmp, configDirName, configFile)) + if err != nil { + t.Fatalf("read config: %v", err) + } + var raw map[string]any + if err := json.Unmarshal(data, &raw); err != nil { + t.Fatalf("parse config: %v", err) + } + if onboarded, ok := raw["onboarded"].(bool); !ok || !onboarded { + t.Errorf("config file onboarded = %v, want JSON bool true", raw["onboarded"]) + } + + reloaded, err := Load() + if err != nil { + t.Fatalf("reload: %v", err) + } + if !reloaded.Onboarded { + t.Error("reloaded config should be onboarded") + } +} + +func TestOnboardedIgnoresLocalConfig(t *testing.T) { + tmp := t.TempDir() + t.Setenv("XDG_CONFIG_HOME", tmp) + t.Setenv("HEY_BASE_URL", "") + + repo := t.TempDir() + if err := os.MkdirAll(filepath.Join(repo, ".hey"), 0700); err != nil { + t.Fatal(err) + } + data, _ := json.Marshal(map[string]any{"onboarded": true}) + if err := os.WriteFile(filepath.Join(repo, ".hey", configFile), data, 0600); err != nil { + t.Fatal(err) + } + cwd, err := os.Getwd() + if err != nil { + t.Fatal(err) + } + if err := os.Chdir(repo); err != nil { + t.Fatal(err) + } + t.Cleanup(func() { _ = os.Chdir(cwd) }) + + cfg, err := Load() + if err != nil { + t.Fatalf("Load: %v", err) + } + if cfg.Onboarded { + t.Error("a repository .hey/config.json must not set onboarded") + } +} + +func TestNonInteractiveEnv(t *testing.T) { + tests := map[string]bool{ + "": false, + "true": true, + "1": true, + "TRUE": true, + "false": false, + "0": false, + "bananas": false, + } + for value, want := range tests { + t.Run("value="+value, func(t *testing.T) { + t.Setenv("HEY_NONINTERACTIVE", value) + if got := NonInteractiveEnv(); got != want { + t.Errorf("NonInteractiveEnv() with %q = %v, want %v", value, got, want) + } + }) + } +} + +// A config rewrite must never delete state it does not understand: legacy +// embedded credentials awaiting migration and any future version's keys ride +// through verbatim. ScrubLegacyCredentials is the one deliberate removal. +func TestSaveGlobalConfigPreservesUnknownKeys(t *testing.T) { + tmp := t.TempDir() + t.Setenv("XDG_CONFIG_HOME", tmp) + t.Setenv("HEY_BASE_URL", "") + + dir := filepath.Join(tmp, configDirName) + if err := os.MkdirAll(dir, 0700); err != nil { + t.Fatal(err) + } + seed := `{"base_url":"https://app.hey.com","access_token":"legacy-token","future_setting":{"nested":true}}` + if err := os.WriteFile(filepath.Join(dir, configFile), []byte(seed), 0600); err != nil { + t.Fatal(err) + } + + cfg, err := Load() + if err != nil { + t.Fatalf("Load: %v", err) + } + if err := cfg.SaveOnboarded(true); err != nil { + t.Fatalf("SaveOnboarded: %v", err) + } + + raw, err := os.ReadFile(filepath.Join(dir, configFile)) + if err != nil { + t.Fatal(err) + } + var saved map[string]any + if err := json.Unmarshal(raw, &saved); err != nil { + t.Fatal(err) + } + if saved["access_token"] != "legacy-token" { + t.Errorf("rewrite deleted unmigrated credentials: %s", raw) + } + if _, ok := saved["future_setting"]; !ok { + t.Errorf("rewrite deleted an unknown key: %s", raw) + } + if saved["onboarded"] != true { + t.Errorf("rewrite lost its own change: %s", raw) + } + + if err := ScrubLegacyCredentials(); err != nil { + t.Fatalf("ScrubLegacyCredentials: %v", err) + } + raw, _ = os.ReadFile(filepath.Join(dir, configFile)) + saved = map[string]any{} + _ = json.Unmarshal(raw, &saved) + if _, ok := saved["access_token"]; ok { + t.Errorf("scrub left migrated credentials behind: %s", raw) + } + if _, ok := saved["future_setting"]; !ok { + t.Errorf("scrub removed an unrelated key: %s", raw) + } +} + +// A config file containing JSON null decodes to a nil map; rewrites must +// recover rather than panic. +func TestSaveGlobalConfigSurvivesNullFile(t *testing.T) { + tmp := t.TempDir() + t.Setenv("XDG_CONFIG_HOME", tmp) + t.Setenv("HEY_BASE_URL", "") + dir := filepath.Join(tmp, configDirName) + if err := os.MkdirAll(dir, 0700); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(dir, configFile), []byte("null"), 0600); err != nil { + t.Fatal(err) + } + + cfg, err := Load() + if err != nil { + t.Fatalf("Load: %v", err) + } + if err := cfg.SaveOnboarded(true); err != nil { + t.Fatalf("SaveOnboarded over a null file: %v", err) + } + reloaded, err := Load() + if err != nil || !reloaded.Onboarded { + t.Fatalf("reload: %v, onboarded=%v", err, reloaded.Onboarded) + } +} diff --git a/internal/harness/agent.go b/internal/harness/agent.go new file mode 100644 index 000000000..f02bc8090 --- /dev/null +++ b/internal/harness/agent.go @@ -0,0 +1,81 @@ +package harness + +import ( + "context" + "sync" +) + +// AgentInfo describes a coding agent integration. +type AgentInfo struct { + 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 + + // 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. + Diagnostics func(ctx context.Context) []*StatusCheck +} + +var ( + registryMu sync.RWMutex + registry []AgentInfo +) + +// RegisterAgent adds an agent to the global registry. Called from init() in +// agent-specific files. Panics on empty or duplicate IDs to keep registry +// state well-defined. +func RegisterAgent(info AgentInfo) { + registryMu.Lock() + defer registryMu.Unlock() + if info.ID == "" { + panic("harness: RegisterAgent called with empty agent ID") + } + for i := range registry { + if registry[i].ID == info.ID { + panic("harness: RegisterAgent called with duplicate agent ID: " + info.ID) + } + } + registry = append(registry, info) +} + +// DetectedAgents returns all agents whose Detect function returns true. +// Copies the registry under the lock before calling Detect callbacks +// to avoid holding the lock during potentially slow I/O. +func DetectedAgents() []AgentInfo { + registryMu.RLock() + snapshot := make([]AgentInfo, len(registry)) + copy(snapshot, registry) + registryMu.RUnlock() + + var detected []AgentInfo + for _, a := range snapshot { + if a.Detect != nil && a.Detect() { + detected = append(detected, a) + } + } + return detected +} + +// AllAgents returns every registered agent. +func AllAgents() []AgentInfo { + registryMu.RLock() + defer registryMu.RUnlock() + out := make([]AgentInfo, len(registry)) + copy(out, registry) + return out +} + +// FindAgent returns the agent with the given ID, or nil. +func FindAgent(id string) *AgentInfo { + registryMu.RLock() + defer registryMu.RUnlock() + for i := range registry { + if registry[i].ID == id { + info := registry[i] + return &info + } + } + return nil +} diff --git a/internal/harness/agent_test.go b/internal/harness/agent_test.go new file mode 100644 index 000000000..aadae6f9a --- /dev/null +++ b/internal/harness/agent_test.go @@ -0,0 +1,86 @@ +package harness + +import "testing" + +// withCleanRegistry empties the registry for a test and restores the real +// claude/codex registrations from init() afterwards. +func withCleanRegistry(t *testing.T) { + t.Helper() + registryMu.Lock() + saved := registry + registry = nil + registryMu.Unlock() + t.Cleanup(func() { + registryMu.Lock() + registry = saved + registryMu.Unlock() + }) +} + +func TestRegisterAndFindAgent(t *testing.T) { + withCleanRegistry(t) + + RegisterAgent(AgentInfo{Name: "Test Agent", ID: "test", Detect: func() bool { return true }}) + + found := FindAgent("test") + if found == nil { + t.Fatal("FindAgent returned nil") + } + if found.Name != "Test Agent" || found.ID != "test" { + t.Errorf("found = %+v", found) + } + if FindAgent("nonexistent") != nil { + t.Error("FindAgent should return nil for unknown IDs") + } +} + +func TestDetectedAgents(t *testing.T) { + withCleanRegistry(t) + + RegisterAgent(AgentInfo{ID: "yes", Detect: func() bool { return true }}) + RegisterAgent(AgentInfo{ID: "no", Detect: func() bool { return false }}) + RegisterAgent(AgentInfo{ID: "also-yes", Detect: func() bool { return true }}) + + detected := DetectedAgents() + if len(detected) != 2 || detected[0].ID != "yes" || detected[1].ID != "also-yes" { + t.Errorf("detected = %+v", detected) + } +} + +func TestAllAgentsReturnsEveryRegistration(t *testing.T) { + withCleanRegistry(t) + + RegisterAgent(AgentInfo{ID: "a"}) + RegisterAgent(AgentInfo{ID: "b"}) + + if all := AllAgents(); len(all) != 2 { + t.Errorf("AllAgents = %+v", all) + } +} + +func TestRegisterAgentPanicsOnBadIDs(t *testing.T) { + withCleanRegistry(t) + + assertPanics := func(name string, fn func()) { + t.Helper() + defer func() { + if recover() == nil { + t.Errorf("%s should panic", name) + } + }() + fn() + } + + assertPanics("empty ID", func() { RegisterAgent(AgentInfo{Name: "Bad Agent"}) }) + RegisterAgent(AgentInfo{ID: "dup", Name: "First"}) + 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") + } +} diff --git a/internal/harness/claude.go b/internal/harness/claude.go new file mode 100644 index 000000000..2cfcd1dd5 --- /dev/null +++ b/internal/harness/claude.go @@ -0,0 +1,340 @@ +package harness + +import ( + "context" + "encoding/json" + "fmt" + "os" + "os/exec" + "path/filepath" + + "github.com/basecamp/hey-cli/internal/version" +) + +const ( + // ClaudeMarketplaceSource is the marketplace repository carrying the hey plugin. + ClaudeMarketplaceSource = "basecamp/claude-plugins" + // ClaudeMarketplaceName is the marketplace name as it appears in plugin keys. + ClaudeMarketplaceName = "37signals" + // ClaudePluginName is the plugin identifier to install. + ClaudePluginName = "hey" + // ClaudeExpectedPluginKey is the fully-qualified key for a correctly installed plugin. + ClaudeExpectedPluginKey = ClaudePluginName + "@" + ClaudeMarketplaceName +) + +func init() { + RegisterAgent(AgentInfo{ + Name: "Claude Code", + ID: "claude", + Detect: DetectClaude, + Checks: claudeChecks, + Diagnostics: func(_ context.Context) []*StatusCheck { + return append(claudeChecks(), CheckClaudePluginVersion()) + }, + }) +} + +func claudeChecks() []*StatusCheck { + checks := []*StatusCheck{CheckClaudePlugin()} + // Only check the skill link if ~/.claude exists (i.e. Claude is dir-detected) + home, err := os.UserHomeDir() + if err == nil { + if info, statErr := os.Stat(filepath.Join(home, ".claude")); statErr == nil && info.IsDir() { + checks = append(checks, CheckClaudeSkillLink()) + } + } + return checks +} + +// DetectClaude returns true if Claude Code is installed. +// Checks ~/.claude/ directory first, then falls back to binary on PATH. +func DetectClaude() bool { + home, err := os.UserHomeDir() + if err == nil { + info, statErr := os.Stat(filepath.Join(filepath.Clean(home), ".claude")) + if statErr == nil && info.IsDir() { + return true + } + } + return FindClaudeBinary() != "" +} + +// FindClaudeBinary returns the path to the claude binary, or "" if not found. +func FindClaudeBinary() string { + if p, err := exec.LookPath("claude"); err == nil { + return p + } + home, err := os.UserHomeDir() + if err != nil || home == "" { + return "" + } + candidate := filepath.Join(filepath.Clean(home), ".local", "bin", "claude") + if _, err := os.Stat(candidate); err == nil { + return candidate + } + return "" +} + +// CheckClaudePlugin checks whether the hey plugin is installed in Claude Code. +func CheckClaudePlugin() *StatusCheck { + home, err := os.UserHomeDir() + if err != nil { + return &StatusCheck{ + Name: "Claude Code Plugin", + Status: "warn", + Message: "Cannot determine home directory", + } + } + + pluginsPath := filepath.Join(filepath.Clean(home), ".claude", "plugins", "installed_plugins.json") + data, err := os.ReadFile(pluginsPath) //nolint:gosec // G304: trusted path + if err != nil { + if os.IsNotExist(err) { + return &StatusCheck{ + Name: "Claude Code Plugin", + Status: "fail", + Message: "Plugin not installed", + Hint: "Run: hey setup claude", + } + } + return &StatusCheck{ + Name: "Claude Code Plugin", + Status: "warn", + Message: "Cannot check Claude Code integration", + Hint: "Unable to read " + pluginsPath, + } + } + + if pluginInstalled(data) { + return &StatusCheck{ + Name: "Claude Code Plugin", + Status: "pass", + Message: "Installed", + } + } + + return &StatusCheck{ + Name: "Claude Code Plugin", + Status: "fail", + Message: "Plugin not installed", + Hint: "Run: hey setup claude", + } +} + +// CheckClaudeSkillLink checks whether ~/.claude/skills/hey contains a valid SKILL.md. +func CheckClaudeSkillLink() *StatusCheck { + home, err := os.UserHomeDir() + if err != nil { + return &StatusCheck{ + Name: "Claude Code Skill", + Status: "warn", + Message: "Cannot determine home directory", + } + } + + skillDir := filepath.Join(filepath.Clean(home), ".claude", "skills", ClaudePluginName) + skillPath := filepath.Join(skillDir, "SKILL.md") + if _, err := os.Stat(skillPath); err != nil { + if os.IsNotExist(err) { + return &StatusCheck{ + Name: "Claude Code Skill", + Status: "fail", + Message: "Skill not linked", + Hint: "Run: hey setup claude", + } + } + return &StatusCheck{ + Name: "Claude Code Skill", + Status: "warn", + Message: "Cannot check skill link", + 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: "Claude Code Skill", + Status: "fail", + Message: "SKILL.md at " + skillDir + " is not a regular file", + Hint: "Move it aside, then run: hey setup claude", + } + } + // ...written by hey-cli — anything else is somebody's work occupying + // the path, not a connected integration. + if !SkillDirOwned(skillDir) { + return &StatusCheck{ + Name: "Claude Code Skill", + Status: "fail", + Message: "A skill not written by hey-cli occupies " + skillDir, + Hint: "Move it aside, then run: hey setup claude", + } + } + + return &StatusCheck{ + Name: "Claude Code Skill", + Status: "pass", + Message: "Linked", + } +} + +// AutoUpdateHint is the user-facing instruction for enabling plugin auto-update. +const AutoUpdateHint = "In Claude Code: /plugins → Marketplaces → 37signals → Enable auto-update" + +// CheckClaudePluginVersion compares the installed plugin version against the +// running CLI version. Returns a warn check when they differ. +func CheckClaudePluginVersion() *StatusCheck { + installed := InstalledPluginVersion() + if installed == "" { + // Can't determine version — don't nag. + return &StatusCheck{ + Name: "Claude Code Plugin Version", + Status: "pass", + Message: "Version not tracked", + } + } + + if version.IsDev() { + return &StatusCheck{ + Name: "Claude Code Plugin Version", + Status: "pass", + Message: fmt.Sprintf("Installed (%s, dev build)", installed), + } + } + + if installed == version.Version { + return &StatusCheck{ + Name: "Claude Code Plugin Version", + Status: "pass", + Message: fmt.Sprintf("Up to date (%s)", installed), + } + } + + return &StatusCheck{ + Name: "Claude Code Plugin Version", + Status: "warn", + Message: fmt.Sprintf("Mismatched (plugin %s, CLI %s)", installed, version.Version), + Hint: AutoUpdateHint, + } +} + +// InstalledPluginVersion reads the installed plugin version from +// ~/.claude/plugins/installed_plugins.json. Returns "" if unreadable. +func InstalledPluginVersion() string { + home, err := os.UserHomeDir() + if err != nil || home == "" { + return "" + } + data, err := os.ReadFile(filepath.Join(filepath.Clean(home), ".claude", "plugins", "installed_plugins.json")) //nolint:gosec // G304: trusted path + if err != nil { + return "" + } + return installedPluginVersion(data) +} + +// installedPluginVersion extracts the hey plugin's version from +// installed_plugins.json data. Handles v2, v1 flat map, and array formats. +func installedPluginVersion(data []byte) string { + // v2 format: {"version": 2, "plugins": {"hey@37signals": [{"version": "1.0.0", ...}]}} + var pluginMap map[string]any + if err := json.Unmarshal(data, &pluginMap); err == nil { + if inner, ok := pluginMap["plugins"].(map[string]any); ok { + for key, val := range inner { + if !matchesPluginKey(key) { + continue + } + if arr, ok := val.([]any); ok { + for _, entry := range arr { + if obj, ok := entry.(map[string]any); ok { + if v, ok := obj["version"].(string); ok { + return v + } + } + } + } + } + } + + // v1 flat map: {"hey@37signals": {"version": "1.0.0"}} + for key, val := range pluginMap { + if !matchesPluginKey(key) { + continue + } + if obj, ok := val.(map[string]any); ok { + if v, ok := obj["version"].(string); ok { + return v + } + } + } + } + + // Array format: [{"name": "hey@37signals", "version": "1.0.0", ...}] + var plugins []map[string]any + if err := json.Unmarshal(data, &plugins); err == nil { + for _, p := range plugins { + if matchesHeyPlugin(p) { + if v, ok := p["version"].(string); ok { + return v + } + } + } + } + + return "" +} + +// pluginInstalled checks if the hey plugin appears as installed. +// Handles multiple possible JSON schemas without panicking. +func pluginInstalled(data []byte) bool { + // Try as array of objects + var plugins []map[string]any + if err := json.Unmarshal(data, &plugins); err == nil { + for _, p := range plugins { + if matchesHeyPlugin(p) { + return true + } + } + return false + } + + // Try as map (key = plugin identifier, or v2 envelope with "plugins" key) + var pluginMap map[string]any + if err := json.Unmarshal(data, &pluginMap); err == nil { + // v2 format: {"version": 2, "plugins": {"hey@37signals": [...]}} + if inner, ok := pluginMap["plugins"].(map[string]any); ok { + for key := range inner { + if matchesPluginKey(key) { + return true + } + } + return false + } + // v1 flat map: {"hey@37signals": {...}} + for key := range pluginMap { + if matchesPluginKey(key) { + return true + } + } + return false + } + + // An unparseable registry proves nothing: Claude itself cannot read it, + // so the plugin is not usable regardless of what raw text it contains. + return false +} + +func matchesHeyPlugin(p map[string]any) bool { + for _, field := range []string{"name", "package", "id"} { + if s, ok := p[field].(string); ok && matchesPluginKey(s) { + return true + } + } + return false +} + +// matchesPluginKey reports whether the key identifies a correctly installed +// hey plugin. Unlike basecamp-cli there is no bare legacy key: hey only ever +// shipped the marketplace-qualified "hey@37signals". +func matchesPluginKey(key string) bool { + return key == ClaudeExpectedPluginKey +} diff --git a/internal/harness/claude_test.go b/internal/harness/claude_test.go new file mode 100644 index 000000000..51b8ed7c0 --- /dev/null +++ b/internal/harness/claude_test.go @@ -0,0 +1,151 @@ +package harness + +import ( + "os" + "path/filepath" + "testing" + + "github.com/basecamp/hey-cli/internal/version" +) + +func tempHome(t *testing.T) string { + t.Helper() + home := t.TempDir() + t.Setenv("HOME", home) + t.Setenv("USERPROFILE", home) + return home +} + +func writeInstalledPlugins(t *testing.T, home, content string) { + t.Helper() + dir := filepath.Join(home, ".claude", "plugins") + if err := os.MkdirAll(dir, 0o755); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(dir, "installed_plugins.json"), []byte(content), 0o644); err != nil { + t.Fatal(err) + } +} + +func TestDetectClaudeByHomeDirectory(t *testing.T) { + home := tempHome(t) + t.Setenv("PATH", t.TempDir()) + + if DetectClaude() { + t.Error("no ~/.claude and no binary should not detect Claude") + } + if err := os.MkdirAll(filepath.Join(home, ".claude"), 0o755); err != nil { + t.Fatal(err) + } + if !DetectClaude() { + t.Error("~/.claude directory should detect Claude") + } +} + +func TestCheckClaudePluginFormats(t *testing.T) { + tests := map[string]struct { + content string + status string + }{ + "missing file": {content: "", status: "fail"}, + "v2 installed": {content: `{"version": 2, "plugins": {"hey@37signals": [{"version": "1.0.0", "scope": "user"}]}}`, status: "pass"}, + "v1 flat map": {content: `{"hey@37signals": {"version": "1.0.0"}}`, status: "pass"}, + "array format": {content: `[{"name": "hey@37signals", "version": "1.0.0"}]`, status: "pass"}, + "other plugin": {content: `{"version": 2, "plugins": {"basecamp@37signals": [{"version": "1.0.0"}]}}`, status: "fail"}, + "bare hey name": {content: `{"hey": {"version": "1.0.0"}}`, status: "fail"}, + } + + for name, tt := range tests { + t.Run(name, func(t *testing.T) { + home := tempHome(t) + if tt.content != "" { + writeInstalledPlugins(t, home, tt.content) + } + check := CheckClaudePlugin() + if check.Status != tt.status { + t.Errorf("status = %q, want %q (message %q)", check.Status, tt.status, check.Message) + } + if check.Status == "fail" && check.Hint != "Run: hey setup claude" { + t.Errorf("hint = %q", check.Hint) + } + }) + } +} + +func TestCheckClaudeSkillLink(t *testing.T) { + home := tempHome(t) + + check := CheckClaudeSkillLink() + if check.Status != "fail" || check.Hint != "Run: hey setup claude" { + t.Errorf("missing skill: %+v", check) + } + + skillDir := filepath.Join(home, ".claude", "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 := CheckClaudeSkillLink(); check.Status != "fail" || check.Hint != "Move it aside, then run: hey setup claude" { + 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 := CheckClaudeSkillLink(); check.Status != "pass" { + t.Errorf("managed skill: %+v", check) + } +} + +func TestInstalledPluginVersionFormats(t *testing.T) { + tests := map[string]string{ + `{"version": 2, "plugins": {"hey@37signals": [{"version": "2.1.0"}]}}`: "2.1.0", + `{"hey@37signals": {"version": "1.5.0"}}`: "1.5.0", + `[{"name": "hey@37signals", "version": "0.9.0"}]`: "0.9.0", + `{"version": 2, "plugins": {"basecamp@37signals": [{"version": "3.0.0"}]}}`: "", + `not json`: "", + } + for content, want := range tests { + if got := installedPluginVersion([]byte(content)); got != want { + t.Errorf("installedPluginVersion(%q) = %q, want %q", content, got, want) + } + } +} + +func TestCheckClaudePluginVersion(t *testing.T) { + home := tempHome(t) + + origVersion := version.Version + version.Version = "1.2.3" + t.Cleanup(func() { version.Version = origVersion }) + + writeInstalledPlugins(t, home, `{"version": 2, "plugins": {"hey@37signals": [{"version": "1.2.3"}]}}`) + if check := CheckClaudePluginVersion(); check.Status != "pass" { + t.Errorf("matching version: %+v", check) + } + + writeInstalledPlugins(t, home, `{"version": 2, "plugins": {"hey@37signals": [{"version": "1.0.0"}]}}`) + check := CheckClaudePluginVersion() + if check.Status != "warn" || check.Hint != AutoUpdateHint { + t.Errorf("mismatched version: %+v", check) + } + + version.Version = "dev" + if check := CheckClaudePluginVersion(); check.Status != "pass" { + t.Errorf("dev build must not nag: %+v", check) + } +} + +// An unparseable registry proves nothing — Claude itself cannot read it — so +// raw text containing the plugin key must not count as installed. +func TestCheckClaudePluginRejectsMalformedRegistry(t *testing.T) { + home := tempHome(t) + writeInstalledPlugins(t, home, `{"version": 2, "plugins": {"hey@37signals": [{"version"`) + if check := CheckClaudePlugin(); check.Status != "fail" { + t.Errorf("malformed registry: %+v", check) + } +} diff --git a/internal/harness/codex.go b/internal/harness/codex.go new file mode 100644 index 000000000..a3a9feef3 --- /dev/null +++ b/internal/harness/codex.go @@ -0,0 +1,124 @@ +package harness + +import ( + "context" + "os" + "os/exec" + "path/filepath" + "strings" +) + +func init() { + RegisterAgent(AgentInfo{ + Name: "Codex", + ID: "codex", + Detect: DetectCodex, + // hey has no Codex plugin yet (no .codex-plugin manifest ships in + // this repo), so Codex 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") +} + +// CodexSkillPath returns where Codex reads the hey skill from. +func CodexSkillPath() string { + codexHome := CodexHome() + if codexHome == "" { + return "" + } + return filepath.Join(codexHome, "skills", "hey", "SKILL.md") +} + +// CheckCodexSkill checks whether the hey skill is installed for Codex. +func CheckCodexSkill() *StatusCheck { + skillPath := CodexSkillPath() + if skillPath == "" { + return &StatusCheck{ + Name: "Codex Skill", + Status: "warn", + Message: "Cannot determine Codex home 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", + } + } + return &StatusCheck{ + Name: "Codex Skill", + Status: "pass", + Message: "Installed", + } +} diff --git a/internal/harness/codex_test.go b/internal/harness/codex_test.go new file mode 100644 index 000000000..491a8c968 --- /dev/null +++ b/internal/harness/codex_test.go @@ -0,0 +1,71 @@ +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 := CodexSkillPath(), filepath.Join(override, "skills", "hey", "SKILL.md"); got != want { + t.Errorf("CodexSkillPath() = %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, ".codex", "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) + } +} diff --git a/internal/harness/harness.go b/internal/harness/harness.go new file mode 100644 index 000000000..5b72185c1 --- /dev/null +++ b/internal/harness/harness.go @@ -0,0 +1,42 @@ +// Package harness detects and checks AI coding agent integration health. +package harness + +import ( + "os" + "path/filepath" +) + +// SkillOwnershipMarker marks a skill directory as written by hey-cli. +// A skill file at a canonical path without it is somebody's hand-authored +// skill: present, but not a working hey integration. +const SkillOwnershipMarker = ".managed-by-hey-cli" + +// SkillDirOwned reports whether hey-cli wrote the skill directory at dir. +// The marker itself must be a regular file — the same shape rule as every +// other skill file, so a planted symlink or directory in the marker's name +// cannot confer ownership. Intermediate directory links still resolve, so +// ownership stays visible through hey-cli's own canonical Claude symlink. +func SkillDirOwned(dir string) bool { + return dir != "" && RegularSkillFile(filepath.Join(dir, SkillOwnershipMarker)) +} + +// RegularSkillFile reports whether path's final component is a regular file +// (Lstat, so a symlinked SKILL.md does not count — its target was never +// inspected). Intermediate directories may still be symlinks: hey-cli's own +// canonical Claude link is a directory link whose SKILL.md is the baseline's +// regular file. That deliberate resolution bounds what this proves: a +// user-symlinked skill *directory* can pass reads that the write paths +// refuse — a non-destructive asymmetry, since every write, removal and +// refresh still declines to touch it. +func RegularSkillFile(path string) bool { + info, err := os.Lstat(path) + return err == nil && info.Mode().IsRegular() +} + +// StatusCheck represents a single agent integration health check result. +type StatusCheck struct { + Name string `json:"name"` + Status string `json:"status"` // "pass", "warn", "fail" + Message string `json:"message"` + Hint string `json:"hint,omitempty"` +} diff --git a/internal/tui/brand.go b/internal/tui/brand.go new file mode 100644 index 000000000..981bd6a02 --- /dev/null +++ b/internal/tui/brand.go @@ -0,0 +1,26 @@ +package tui + +import "strings" + +// Wordmark is the HEY braille-art logo shown in root help and on the setup +// wizard's welcome screen. +const Wordmark = `⠀⠀⠀⠀⠀⠀⣰⠲⣄⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀ +⠀⠀⠀⡟⢳⡀⣏⠀⠘⣆⠀⠀⠀⠀⠀⣤⣤⡄⠀⠀⢠⣤⣤⣤⣤⣤⣤⣤⣤⠀⠀⢀⣤⣤⡄⠀ +⠀⣴⢄⢳⠀⠹⣿⠀⠀⠸⣆⠴⠒⢢⡀⢻⣿⡇⠀⠀⢸⣿⣿⡟⠛⠛⠛⢿⣿⣇⠀⣼⣿⡟⠀⠀ +⠀⢻⠈⠻⣧⠀⠹⣇⠀⢰⣿⠀⠀⠀⡇⢸⣿⣷⣶⣶⣾⣿⣿⣷⣶⣶⠀⠈⢿⣿⣼⣿⡟⠀⠀⠀ +⣶⠺⣧⡀⠙⢧⠀⠉⠀⣸⢸⡆⠀⢸⠁⣼⣿⡏⠉⠉⢹⣿⣿⡏⠉⠉⠀⠀⠈⣿⣿⡟⠀⠀⠀⠀ +⠘⣆⠈⠳⠀⠀⠀⠀⠀⢻⢸⠇⢀⡏⠀⣿⣿⡇⠀⠀⢸⣿⣿⣿⣶⣶⣶⡆⠀⣿⣿⡇⠀⠀⠀⠀ +⠀⠈⠳⣄⡀⠀⠀⠀⠀⠈⠉⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀ +⠀⠀⠀⠀⠉⠙⠚⠃⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀` + +// RenderWordmark returns the wordmark, tinted when color is enabled. +func RenderWordmark(colorEnabled bool) string { + if !colorEnabled { + return Wordmark + } + lines := strings.Split(Wordmark, "\n") + for i, line := range lines { + lines[i] = "\033[94m" + line + "\033[0m" + } + return strings.Join(lines, "\n") +} diff --git a/internal/tui/prompt.go b/internal/tui/prompt.go new file mode 100644 index 000000000..9b84eccb8 --- /dev/null +++ b/internal/tui/prompt.go @@ -0,0 +1,107 @@ +package tui + +import ( + "errors" + "os" + + tea "charm.land/bubbletea/v2" + "charm.land/lipgloss/v2" + "golang.org/x/term" +) + +// ErrCanceled reports that the user dismissed a prompt (esc or ctrl+c). +var ErrCanceled = errors.New("canceled") + +// ErrNotInteractive reports that a prompt was requested without a terminal. +var ErrNotInteractive = errors.New("interactive prompt requires a terminal") + +// Confirm shows a yes/no prompt on the terminal (rendered to stderr so stdout +// stays data) and reports the choice. Enter accepts the highlighted answer, +// y/n answer directly, arrows or h/l move, esc and ctrl+c cancel. +func Confirm(message string, defaultYes bool) (bool, error) { + if !term.IsTerminal(int(os.Stdin.Fd())) { //nolint:gosec // G115: fd fits in int + return false, ErrNotInteractive + } + + p := tea.NewProgram(newConfirmModel(message, defaultYes), tea.WithOutput(os.Stderr)) + final, err := p.Run() + if err != nil { + return false, err + } + m, ok := final.(confirmModel) + if !ok { + return false, ErrCanceled + } + if m.canceled { + return false, ErrCanceled + } + return m.value, nil +} + +type confirmModel struct { + message string + value bool + canceled bool + styles confirmStyles +} + +type confirmStyles struct { + message lipgloss.Style + selected lipgloss.Style + option lipgloss.Style + help lipgloss.Style +} + +func newConfirmModel(message string, defaultYes bool) confirmModel { + return confirmModel{ + message: message, + value: defaultYes, + styles: confirmStyles{ + message: lipgloss.NewStyle().Bold(true), + selected: lipgloss.NewStyle().Foreground(colorPrimary).Bold(true), + option: lipgloss.NewStyle().Foreground(colorMuted), + help: lipgloss.NewStyle().Foreground(colorMuted), + }, + } +} + +func (m confirmModel) Init() tea.Cmd { + return nil +} + +func (m confirmModel) Update(msg tea.Msg) (tea.Model, tea.Cmd) { + key, ok := msg.(tea.KeyPressMsg) + if !ok { + return m, nil + } + switch key.String() { + case "y", "Y": + m.value = true + return m, tea.Quit + case "n", "N": + m.value = false + return m, tea.Quit + case "enter": + return m, tea.Quit + case "left", "right", "h", "l", "tab": + m.value = !m.value + return m, nil + case "esc", "ctrl+c": + m.canceled = true + return m, tea.Quit + } + return m, nil +} + +func (m confirmModel) View() tea.View { + yes, no := m.styles.option.Render("Yes"), m.styles.option.Render("No") + if m.value { + yes = m.styles.selected.Render("▸ Yes") + } else { + no = m.styles.selected.Render("▸ No") + } + + line := m.styles.message.Render(m.message) + " " + yes + " " + no + "\n" + + m.styles.help.Render(" y/n choose · enter accept · esc cancel") + "\n" + return tea.NewView(line) +} diff --git a/internal/tui/prompt_test.go b/internal/tui/prompt_test.go new file mode 100644 index 000000000..fb56ffa1b --- /dev/null +++ b/internal/tui/prompt_test.go @@ -0,0 +1,77 @@ +package tui + +import ( + "strings" + "testing" + + tea "charm.land/bubbletea/v2" +) + +func confirmAfter(t *testing.T, defaultYes bool, keys ...string) confirmModel { + t.Helper() + m := newConfirmModel("Sign in now?", defaultYes) + for _, key := range keys { + updated, _ := m.Update(keyPress(key)) + m = updated.(confirmModel) + } + return m +} + +func TestConfirmDefaultHighlightsAnswer(t *testing.T) { + yes := newConfirmModel("Sign in now?", true) + if !yes.value { + t.Error("defaultYes=true should preselect Yes") + } + no := newConfirmModel("Sign in now?", false) + if no.value { + t.Error("defaultYes=false should preselect No") + } +} + +func TestConfirmDirectAnswers(t *testing.T) { + if m := confirmAfter(t, false, "y"); !m.value || m.canceled { + t.Errorf("y should answer yes, got value=%v canceled=%v", m.value, m.canceled) + } + if m := confirmAfter(t, true, "n"); m.value || m.canceled { + t.Errorf("n should answer no, got value=%v canceled=%v", m.value, m.canceled) + } +} + +func TestConfirmEnterAcceptsHighlighted(t *testing.T) { + if m := confirmAfter(t, true, "enter"); !m.value { + t.Error("enter should accept the default Yes") + } + if m := confirmAfter(t, true, "left", "enter"); m.value { + t.Error("left should toggle to No before enter accepts") + } +} + +func TestConfirmEscapeCancels(t *testing.T) { + if m := confirmAfter(t, true, "esc"); !m.canceled { + t.Error("esc should cancel") + } + if m := confirmAfter(t, true, "ctrl+c"); !m.canceled { + t.Error("ctrl+c should cancel") + } +} + +func TestConfirmQuitsOnAnswer(t *testing.T) { + m := newConfirmModel("Sign in now?", true) + _, cmd := m.Update(keyPress("y")) + if cmd == nil { + t.Fatal("answering should quit the prompt") + } + if _, ok := cmd().(tea.QuitMsg); !ok { + t.Errorf("answer produced %T, want tea.QuitMsg", cmd()) + } +} + +func TestConfirmViewShowsMessageAndOptions(t *testing.T) { + view := newConfirmModel("Sign in now?", true).View() + text := view.Content + for _, want := range []string{"Sign in now?", "Yes", "No", "esc cancel"} { + if !strings.Contains(text, want) { + t.Errorf("confirm view missing %q:\n%s", want, text) + } + } +} diff --git a/scripts/install.ps1 b/scripts/install.ps1 index dedf152de..858306dc6 100644 --- a/scripts/install.ps1 +++ b/scripts/install.ps1 @@ -14,8 +14,13 @@ try { } # Environment options: -# HEY_VERSION Specific version to install (default: latest) -# HEY_BIN_DIR Where to install the binary +# HEY_VERSION Specific version to install (default: latest) +# HEY_BIN_DIR Where to install the binary +# HEY_SKIP_SETUP Set to 1 to skip the interactive setup wizard (still runs +# `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) # # 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 @@ -24,6 +29,7 @@ try { $Repo = 'basecamp/hey-cli' $Version = $env:HEY_VERSION $BinDir = $env:HEY_BIN_DIR +$SkipSetup = $env:HEY_SKIP_SETUP function Step([string]$Message) { Write-Host " -> $Message" @@ -288,6 +294,45 @@ function Ensure-UserPath([string]$Dir) { Info "Added $Dir to your user PATH" } +function Test-InteractiveSession { + if ($Host.Name -ne 'ConsoleHost' -and $Host.Name -ne 'Visual Studio Code Host') { + return $false + } + + try { + return -not [Console]::IsInputRedirected -and -not [Console]::IsOutputRedirected + } catch { + return $false + } +} + +# Invoke-PostInstallSetup connects coding agents without prompting, but only +# when the installed binary has the ownership-aware `setup agents` (it honors +# HEY_SETUP_AGENT itself). Releases that predate `setup agents` also predate +# the ownership gate on `skill install`, so the installer never invokes an +# old binary's legacy setup or skill commands - the printed next steps are +# the fallback. Best-effort: a failure here must not fail the install. +function Invoke-PostInstallSetup([string]$Binary) { + # The call never touches credentials, but a locked headless keychain can + # block the keyring probe forever. Set the escape hatch for the duration + # only, restoring the caller's value (or absence) on the way out. + $savedNoKeyring = $env:HEY_NO_KEYRING + $env:HEY_NO_KEYRING = '1' + try { + try { $help = & $Binary setup --help 2>$null } catch { $help = '' } + + if ($help -match '(?m)^\s+agents\s') { + try { & $Binary setup agents } catch { } + } + } finally { + if ($null -eq $savedNoKeyring) { + Remove-Item Env:HEY_NO_KEYRING -ErrorAction SilentlyContinue + } else { + $env:HEY_NO_KEYRING = $savedNoKeyring + } + } +} + function Main { $arch = Get-PlatformArch if (-not $BinDir) { @@ -355,11 +400,46 @@ function Main { } Info "$installedVersion installed" + $isInteractive = Test-InteractiveSession + Write-Host '' - Write-Host ' Next steps:' - Write-Host ' hey auth login Authenticate with HEY' - Write-Host ' hey --help See what you can do' - Write-Host '' + if ($SkipSetup -eq '1') { + Step 'Skipping setup wizard (HEY_SKIP_SETUP=1)' + # Still install the agent skill and connect coding agents (best-effort). + Invoke-PostInstallSetup $installedBinary + Write-Host '' + Write-Host ' Next steps:' + Write-Host ' hey auth login Authenticate with HEY' + Write-Host ' hey setup Run the setup wizard' + Write-Host ' hey --help See what you can do' + Write-Host '' + } elseif ($isInteractive) { + # Setup is optional: a declined OAuth, a timeout or a busy callback port + # must not turn an already-successful install into a failure. Windows + # PowerShell 5.1 does not throw on native nonzero exits, but strict + # configurations can, so guard the call and check $LASTEXITCODE. + try { & $installedBinary setup } catch { } + if ($LASTEXITCODE -ne 0) { + Write-Host '' + Write-Host " Setup didn't finish - hey is installed; run it again any time:" + Write-Host '' + Write-Host ' Next steps:' + Write-Host ' hey auth login Authenticate with HEY' + Write-Host ' hey setup Run the setup wizard' + Write-Host ' hey --help See what you can do' + Write-Host '' + } + } else { + Info 'Skipping interactive setup because PowerShell is running non-interactively.' + # Install the agent skill and connect coding agents (best-effort). + Invoke-PostInstallSetup $installedBinary + Write-Host '' + Write-Host ' Next steps:' + Write-Host ' hey auth login Authenticate with HEY' + Write-Host ' hey setup Run the setup wizard' + Write-Host ' hey --help See what you can do' + Write-Host '' + } Write-Host ' Installed executable:' Write-Host " $installedBinary" Write-Host '' diff --git a/scripts/install.sh b/scripts/install.sh index 37fa3303e..5aa3210b4 100755 --- a/scripts/install.sh +++ b/scripts/install.sh @@ -9,6 +9,13 @@ # (default: ~/bin if on PATH, else ~/.local/bin if on PATH; # otherwise ~/bin on Windows, ~/.local/bin elsewhere) # HEY_VERSION Specific version to install (default: latest) +# HEY_SKIP_SETUP Set to 1 to skip the interactive setup wizard after install +# (still runs `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 a single +# agent; several detected connects none and lists them) # # Verification: the SHA-256 checksum is always verified against the release's # checksums.txt. When cosign is installed, checksums.txt is additionally @@ -438,17 +445,81 @@ main() { # shellcheck disable=SC2064 trap "rm -rf '${tmp_dir}'" EXIT + local binary_name="hey" + if [[ "$platform" == windows_* ]]; then + binary_name="hey.exe" + fi + download_binary "$version" "$platform" "$tmp_dir" setup_path verify_install "$platform" + echo "" + + # Hand off to the setup wizard only when stdin and stdout are a terminal and + # it was not explicitly skipped. Non-interactive environments (CI, piped + # input, coding agents like Claude Code or Codex) get the agent skill + # installed and a best-effort agent connection via `setup agents`, plus + # next-step instructions — the wizard prompts, and a prompt without a + # terminal would hang. + if [[ "${HEY_SKIP_SETUP:-}" == "1" ]]; then + step "Skipping setup wizard (HEY_SKIP_SETUP=1)" + post_install_setup "$binary_name" + print_next_steps + elif [[ -t 0 ]] && [[ -t 1 ]]; then + # Setup is optional: a declined OAuth, a timeout or a busy callback port + # must not turn an already-successful install into a failure under set -e. + if ! "$BIN_DIR/$binary_name" setup; then + echo "" + echo " Setup didn't finish — hey is installed; run it again any time:" + print_next_steps + fi + else + info "Skipping interactive setup (no terminal detected)." + post_install_setup "$binary_name" + print_next_steps + fi +} + +print_next_steps() { echo "" echo " Next steps:" echo " $(bold "hey auth login") Authenticate with HEY" + echo " $(bold "hey setup") Run the setup wizard" echo " $(bold "hey --help") See what you can do" echo "" } +# binary_supports_setup_agents reports whether the installed binary exposes the +# `setup agents` subcommand. The hosted install.sh from main can outrun the +# latest release, so we probe rather than assume. +binary_supports_setup_agents() { + "$1" setup --help 2>/dev/null | grep -qE '^[[:space:]]+agents[[:space:]]' +} + +# 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). +# +# Releases that predate `setup agents` also predate the ownership gate on +# `skill install` — their legacy implementation overwrites whatever occupies +# the skill paths — so the installer never invokes an old binary's setup or +# skill commands. The printed next steps are the fallback: setup belongs to +# the user, on a binary that refuses to destroy their files. +# +# The real invocation carries HEY_NO_KEYRING=1, per-command rather than +# exported: it never touches credentials, and a locked headless keychain +# (CI, ssh) could otherwise block the keyring probe forever. The +# `setup --help` capability probe stays bare — help short-circuits first. +post_install_setup() { + local binary_name="$1" + local bin="$BIN_DIR/$binary_name" + + if binary_supports_setup_agents "$bin"; then + HEY_NO_KEYRING=1 "$bin" setup agents || true + fi +} + # Guard so sourcing the script (e.g. from tests) doesn't run the installer. # The if-form is required: `[[ … ]] && main` returns 1 when sourced, which # trips `set -e` in the sourcing shell. The `:-$0` fallback is also diff --git a/skills/hey/SKILL.md b/skills/hey/SKILL.md index 254dfb8a2..3d8da83e0 100644 --- a/skills/hey/SKILL.md +++ b/skills/hey/SKILL.md @@ -199,7 +199,7 @@ hey boxes --quiet --jq '.[].name' | Write journal entry | `hey journal write "Today was great"` | | Check auth status | `hey auth status` | | Print access token | `hey auth token` | -| Launch TUI | `hey` (Ctrl+A switches linked mail accounts) | +| Launch TUI | `hey tui` (Ctrl+A switches linked mail accounts) | ## Decision Trees @@ -226,7 +226,7 @@ Want to read email? ├── Mark as spam? → hey spam ├── Ignore future activity? → hey ignore ├── Stop ignoring? → hey stop-ignoring -└── Launch interactive UI? → hey (no args, launches TUI) +└── Launch interactive UI? → hey tui ``` ### Sending Email @@ -512,6 +512,13 @@ hey journal write # Write entry via $EDITOR hey auth login # Log in (browser-based OAuth) hey auth status # Check if authenticated hey auth logout # Log out +hey login / hey logout # Shortcuts for the two above +hey setup # First-run wizard: sign in + connect coding agents +HEY_NONINTERACTIVE=1 hey setup --json # No prompts and no OAuth wait — but still + # installs agent skills and records onboarding; + # use `hey doctor` to inspect without changes. + # (Without HEY_NONINTERACTIVE, a terminal on + # stdin still starts browser sign-in.) ``` If a command fails with an auth error, run `hey auth status` to check, then `hey auth login` to re-authenticate. diff --git a/tests/e2e/installer.bats b/tests/e2e/installer.bats index 23503f5a6..d20556695 100644 --- a/tests/e2e/installer.bats +++ b/tests/e2e/installer.bats @@ -9,11 +9,15 @@ # first run diagnosed rather than swallowed. setup() { + # The installer's setup hand-off keys off these; a leaked value would skew results. + unset HEY_SKIP_SETUP HEY_SETUP_AGENT + INSTALL_SH="${BATS_TEST_DIRNAME}/../../scripts/install.sh" INSTALL_PS1="${BATS_TEST_DIRNAME}/../../scripts/install.ps1" STUB_DIR="$(mktemp -d)" LOG="$STUB_DIR/calls.log" + write_stub new # default: a binary that supports `setup agents` } teardown() { @@ -37,12 +41,229 @@ teardown() { [[ "$output" != *"BASH_SOURCE[0]: unbound variable"* ]] } -# No interactive setup wizard exists: the installer must end with printed next -# steps, never by exec'ing the freshly installed binary into a prompt. -@test "install.sh ends with next steps, not an interactive setup" { - run grep -nE '"\$BIN_DIR/\$binary_name" setup|HEY_SKIP_SETUP|post_install_setup' "$INSTALL_SH" +# write_stub emits a `hey` stub that logs its argv. mode=new advertises the +# `setup agents` subcommand in `setup --help`; mode=old omits it and fails an +# actual `setup agents` invocation, mimicking a release binary that predates +# the setup subcommands. +write_stub() { + local mode="$1" + { + echo '#!/usr/bin/env bash' + echo "echo \"\$@\" >> \"$LOG\"" + echo 'if [[ "$1 $2" == "setup --help" ]]; then' + echo ' echo " claude Connect Claude Code to HEY"' + if [[ "$mode" == "new" ]]; then + echo ' echo " agents Install the HEY skill and connect detected coding agents"' + fi + echo ' exit 0' + echo 'fi' + if [[ "$mode" == "old" ]]; then + # An old binary advertises only `setup claude`. Reject every OTHER + # `setup `: a real old parent would swallow it as a stray arg and + # launch the interactive wizard -- the exact bug the installer must avoid. + echo 'if [[ "$1" == "setup" && "$2" != "claude" && "$2" != "--help" ]]; then echo "unknown command \"$2\"" >&2; exit 1; fi' + fi + echo 'exit 0' + } > "$STUB_DIR/hey" + chmod +x "$STUB_DIR/hey" +} + +# write_nk_stub emits a stub that logs HEY_NO_KEYRING alongside argv, for +# asserting the escape-hatch belt. Separate from write_stub so the argv-shape +# assertions stay byte-exact. mode mirrors write_stub's new/old. +write_nk_stub() { + local mode="$1" + { + echo '#!/usr/bin/env bash' + echo "echo \"nk=\${HEY_NO_KEYRING:-unset} \$@\" >> \"$LOG\"" + echo 'if [[ "$1 $2" == "setup --help" ]]; then' + if [[ "$mode" == "new" ]]; then + echo ' echo " agents Install the HEY skill and connect detected coding agents"' + fi + echo ' exit 0' + echo 'fi' + echo 'exit 0' + } > "$STUB_DIR/hey" + chmod +x "$STUB_DIR/hey" +} + +run_post_install_setup() { + run bash -c " + set -euo pipefail + ${1:-} + source '$INSTALL_SH' + BIN_DIR='$STUB_DIR' + post_install_setup hey + cat '$LOG' + " +} + +# The installer hands off to the setup wizard on a terminal and to the +# non-interactive `setup agents` otherwise. The non-TTY and skip branches must +# run `setup agents` (skill + best-effort agent connection), never a hardcoded +# `setup claude`; old binaries (no `setup agents`) must fall back without +# guessing an agent. + +@test "new binary: post_install_setup dispatches to 'setup agents', never 'setup claude'" { + run_post_install_setup + [[ "$status" -eq 0 ]] + [[ "$output" == *"setup agents"* ]] + [[ "$output" != *"setup claude"* ]] +} + +@test "new binary: HEY_SKIP_SETUP path still runs 'setup agents'" { + run_post_install_setup "export HEY_SKIP_SETUP=1" + [[ "$status" -eq 0 ]] + [[ "$output" == *"setup agents"* ]] + [[ "$output" != *"setup claude"* ]] +} + +# Releases that predate `setup agents` also predate the ownership gate on +# `skill install`: their legacy implementation overwrites whatever occupies +# the skill paths. The installer therefore never invokes an old binary's +# setup or skill commands — whatever the selector — and leaves setup to the +# printed next steps. +@test "old binary: nothing is invoked beyond the capability probe" { + write_stub old + for selector in "" claude codex all none; do + : > "$LOG" + if [[ -n "$selector" ]]; then + run_post_install_setup "export HEY_SETUP_AGENT=$selector" + else + run_post_install_setup + fi + [[ "$status" -eq 0 ]] + [[ "$output" == *"setup --help"* ]] + [[ "$output" != *"skill install"* ]] + [[ "$output" != *"setup claude"* ]] + [[ "$output" != *"setup codex"* ]] + [[ "$output" != *"setup agents"$'\n'* ]] + done +} + +@test "install.sh has no residual 'setup claude' dispatch" { + run grep -n 'setup claude' "$INSTALL_SH" [[ "$status" -ne 0 ]] +} + +@test "install.sh hands off to the wizard on a TTY and to post_install_setup otherwise" { + # The TTY branch execs the freshly installed binary into `setup`. + grep -qF '"$BIN_DIR/$binary_name" setup' "$INSTALL_SH" + grep -qE '\[\[ -t 0 \]\] && \[\[ -t 1 \]\]' "$INSTALL_SH" + # The skip and non-TTY branches both dispatch via post_install_setup. + run grep -c 'post_install_setup "\$binary_name"' "$INSTALL_SH" + [[ "$status" -eq 0 ]] + [[ "$output" -ge 2 ]] + # Next steps keep the literal login hint plus the wizard. grep -q 'hey auth login' "$INSTALL_SH" + grep -q 'hey setup' "$INSTALL_SH" +} + +@test "install.ps1 routes both non-wizard branches through the guarded best-effort helper" { + run grep -c 'Invoke-PostInstallSetup \$installedBinary' "$INSTALL_PS1" + [[ "$status" -eq 0 ]] + [[ "$output" -eq 2 ]] + grep -qF '& $installedBinary setup' "$INSTALL_PS1" +} + +@test "install.ps1 helper is guarded and never invokes legacy setup commands" { + grep -q 'function Invoke-PostInstallSetup' "$INSTALL_PS1" + grep -q 'setup agents' "$INSTALL_PS1" + grep -q 'catch {' "$INSTALL_PS1" + grep -q 'HEY_NO_KEYRING' "$INSTALL_PS1" + # Only the ownership-aware `setup agents` may be invoked automatically: no + # legacy skill-install or per-agent dispatch survives in the helper. + helper=$(awk '/function Invoke-PostInstallSetup/,/^}/' "$INSTALL_PS1") + [[ "$helper" != *"skill install"* ]] + [[ "$helper" != *'setup $'* ]] +} + +# The installer's best-effort children never touch credentials, but a locked +# headless keychain can block the keyring probe forever, so they must carry +# HEY_NO_KEYRING=1 -- per-command, not blanket: the `setup --help` capability +# probe stays bare (help short-circuits before the probe). +@test "new binary: real setup calls carry HEY_NO_KEYRING=1, capability probe does not" { + write_nk_stub new + run bash -c " + set -euo pipefail + unset HEY_NO_KEYRING + source '$INSTALL_SH' + BIN_DIR='$STUB_DIR' + post_install_setup hey + cat '$LOG' + " + [[ "$status" -eq 0 ]] + [[ "$output" == *"nk=unset setup --help"* ]] + [[ "$output" == *"nk=1 setup agents"* ]] +} + +@test "old binary: only the bare capability probe runs" { + write_nk_stub old + run bash -c " + set -euo pipefail + unset HEY_NO_KEYRING + source '$INSTALL_SH' + BIN_DIR='$STUB_DIR' + post_install_setup hey + cat '$LOG' + " + [[ "$status" -eq 0 ]] + [[ "$output" == *"nk=unset setup --help"* ]] + [[ "$output" != *"skill install"* ]] +} + +# The function under test is extracted from install.ps1's AST and evaluated +# alone: Main never runs, and the installer needs no test hooks. +@test "install.ps1 Invoke-PostInstallSetup sets and restores HEY_NO_KEYRING" { + if ! command -v pwsh >/dev/null 2>&1; then + if [[ -n "${CI:-}" ]]; then + echo "pwsh is required in CI for install.ps1 belt coverage" >&2 + return 1 + fi + skip "pwsh not installed" + fi + + PS_LOG="$STUB_DIR/ps-calls.log" + + cat > "$STUB_DIR/hey-stub.ps1" <<'EOF' +$rest = $args -join ' ' +$nk = if ($null -eq $env:HEY_NO_KEYRING) { 'unset' } else { $env:HEY_NO_KEYRING } +Add-Content -LiteralPath $env:PS_LOG "nk=$nk $rest" +if ($rest -eq 'setup --help') { + ' agents Install the HEY skill and connect detected coding agents' +} +EOF + + cat > "$STUB_DIR/driver.ps1" <<'EOF' +$ErrorActionPreference = 'Stop' +$tokens = $null; $parseErrors = $null +$ast = [System.Management.Automation.Language.Parser]::ParseFile($env:INSTALL_PS1_PATH, [ref]$tokens, [ref]$parseErrors) +if ($parseErrors.Count -gt 0) { throw "install.ps1 parse errors: $($parseErrors -join '; ')" } +$fn = $ast.Find({ param($n) $n -is [System.Management.Automation.Language.FunctionDefinitionAst] -and $n.Name -eq 'Invoke-PostInstallSetup' }, $true) +if (-not $fn) { throw 'Invoke-PostInstallSetup not found in install.ps1' } +# Evaluate only the function definition -- the installer's Main never runs. +. ([scriptblock]::Create($fn.Extent.Text)) +Invoke-PostInstallSetup $env:PS_STUB +if ($null -ne $env:HEY_NO_KEYRING) { throw "HEY_NO_KEYRING not restored: '$env:HEY_NO_KEYRING'" } +'restored-ok' +# Second pass: a caller-set value must survive, not just absence. +$env:HEY_NO_KEYRING = '0' +Invoke-PostInstallSetup $env:PS_STUB +if ($env:HEY_NO_KEYRING -ne '0') { throw "existing HEY_NO_KEYRING value not restored: '$env:HEY_NO_KEYRING'" } +'restored-value-ok' +EOF + + run bash -c " + set -euo pipefail + unset HEY_NO_KEYRING + export PS_LOG='$PS_LOG' PS_STUB='$STUB_DIR/hey-stub.ps1' INSTALL_PS1_PATH='$INSTALL_PS1' + pwsh -NoProfile -File '$STUB_DIR/driver.ps1' + cat '$PS_LOG' + " + [[ "$status" -eq 0 ]] + [[ "$output" == *"restored-ok"* ]] + [[ "$output" == *"restored-value-ok"* ]] + [[ "$output" == *"nk=1 setup agents"* ]] } # The release builds darwin_amd64 and darwin_arm64, never a universal @@ -586,3 +807,35 @@ EOF [[ "$status" -eq 0 ]] [[ "$output" == *"iex-scope-ok"* ]] } + +# Setup is optional: a wizard the user cancels (declined OAuth, timeout) must +# not turn a successful install into a failure under set -e. +@test "install.sh TTY wizard failure falls back to next steps, not an install failure" { + grep -qE 'if ! "\$BIN_DIR/\$binary_name" setup; then' "$INSTALL_SH" + run bash -c " + set -euo pipefail + source '$INSTALL_SH' + BIN_DIR='$STUB_DIR' + cat > '$STUB_DIR/hey' <<'STUB' +#!/usr/bin/env bash +if [[ \"\$1\" == \"setup\" && -z \"\${2:-}\" ]]; then exit 3; fi +exit 0 +STUB + chmod +x '$STUB_DIR/hey' + binary_name=hey + if ! \"\$BIN_DIR/\$binary_name\" setup; then + echo 'fallback-ran' + print_next_steps + fi + echo 'installer-continued' + " + [[ "$status" -eq 0 ]] + [[ "$output" == *"fallback-ran"* ]] + [[ "$output" == *"installer-continued"* ]] + [[ "$output" == *"hey auth login"* ]] +} + +@test "install.ps1 treats a failing interactive wizard as optional setup" { + grep -qF 'try { & $installedBinary setup } catch { }' "$INSTALL_PS1" + grep -q 'if ($LASTEXITCODE -ne 0)' "$INSTALL_PS1" +} diff --git a/tests/smoke/auth_test.go b/tests/smoke/auth_test.go index bab03bd7b..1e7f99623 100644 --- a/tests/smoke/auth_test.go +++ b/tests/smoke/auth_test.go @@ -78,3 +78,19 @@ func TestAuthLogoutAndRelogin(t *testing.T) { t.Errorf("expected authenticated=true after re-login") } } + +func TestLoginLogoutShortcuts(t *testing.T) { + // hey logout == hey auth logout. + heyOK(t, "logout") + status := heyJSON(t, "auth", "status") + if data := dataAs[map[string]any](t, status); data["authenticated"] == true { + t.Errorf("expected authenticated=false after hey logout") + } + + // hey login == hey auth login. + heyOK(t, "login", "--cookie", sessionCookie) + status = heyJSON(t, "auth", "status") + if data := dataAs[map[string]any](t, status); data["authenticated"] != true { + t.Errorf("expected authenticated=true after hey login --cookie") + } +}