From 57a87caeacd2c6802ea406d9e5dd11d47963ca69 Mon Sep 17 00:00:00 2001 From: Dibbayajyoti Date: Tue, 27 Oct 2026 12:25:12 +0530 Subject: [PATCH] init and status: onboarding fixes from QA - init checks the key, plans and confirms before saving it, so a refused plan saves and writes nothing; the key is saved after the changes are applied. - An existing spicrawl entry that differs only in its bearer token (a key or a ${SPICRAWL_API_KEY} reference we wrote) is replaced without --force, in JSON and TOML configs; the .bak is kept. - With --global=false the .bak of a project key file is gitignored too, and a key file that git tracks is flagged (warning, and the result's reason). - status flags an embedded key that is not the key in use (fingerprint compare) as mcp.stale; its repair, like needs_env's, is init --client --yes without --force. - README: the prompt names the scrape command; the status output wording is fixed. npm launcher: SPICRAWL_BINARY was read twice. --- CHANGELOG.md | 3 + README.md | 4 +- internal/cmd/agents.go | 44 +- internal/cmd/auth.go | 33 +- internal/cmd/initcmd.go | 33 +- internal/cmd/mcpcmd.go | 86 +++- internal/cmd/onboarding_qa_test.go | 403 ++++++++++++++++++ internal/cmd/onboarding_test.go | 25 +- internal/cmd/ops_golden_test.go | 10 + internal/cmd/status.go | 79 ++-- ...it_all_yes_api_key_project.file..gitignore | 2 + .../init_all_yes_api_key_project.stdout | 2 +- .../init_all_yes_api_key_project.stdout | 2 +- .../golden/status/key_invalid_exit3.stdout | 3 +- .../status/key_without_read_scope.stdout | 3 +- .../golden/status/mcp_connected.stdout | 3 +- .../golden/status/mcp_needs_env_exit1.stdout | 7 +- .../status/mcp_not_configured_exit1.stdout | 3 +- .../golden/status/mcp_stale_key_exit1.exit | 1 + .../golden/status/mcp_stale_key_exit1.stdout | 44 ++ .../golden/status/no_key_exit3.stdout | 3 +- .../testdata/golden/status/not_ready.stdout | 3 +- internal/cmd/testdata/golden/status/ok.stdout | 3 +- .../golden/status/unreachable_exit10.stdout | 3 +- .../status/worker_kind_down_exit1.stdout | 3 +- .../golden/status/workers_error.stdout | 3 +- .../status_human/mcp_needs_env_exit1.stdout | 4 +- .../status_human/mcp_stale_key_exit1.exit | 1 + .../status_human/mcp_stale_key_exit1.stdout | 15 + npm/bin/spicrawl.js | 4 +- 30 files changed, 747 insertions(+), 85 deletions(-) create mode 100644 internal/cmd/onboarding_qa_test.go create mode 100644 internal/cmd/testdata/golden/status/mcp_stale_key_exit1.exit create mode 100644 internal/cmd/testdata/golden/status/mcp_stale_key_exit1.stdout create mode 100644 internal/cmd/testdata/golden/status_human/mcp_stale_key_exit1.exit create mode 100644 internal/cmd/testdata/golden/status_human/mcp_stale_key_exit1.stdout diff --git a/CHANGELOG.md b/CHANGELOG.md index e843539..03c617b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -23,6 +23,9 @@ Bug fixes from a review of 0.1.3, and live progress on a terminal. No new comman - **init:** `--all` connects every detected client (conflicts with `--client`). `--api-key` is checked with the API and saved as `login` does, before any client file is written; a rejected key writes nothing. The key is then written into each client's MCP config as is, in the user-level files (`--global` is implied; `--global=false` picks the project files, which are added to the project's `.gitignore` when it has one; `--use-env=true` keeps the `${SPICRAWL_API_KEY}` reference), so the agents work with nothing exported. A key already saved in the config file (by `login` or an earlier `--api-key`) is embedded the same way when `init` runs without `--api-key`, with no new check; a key that is only in `$SPICRAWL_API_KEY` keeps today's `${SPICRAWL_API_KEY}` references. The key is never printed, only masked, and the error output masks it too. After the result list (stdout, unchanged) init prints a report on stderr in human mode (like every message, it is silent in JSON mode, where the list has the `action` and `reason` of each file): `connected:`, `skipped:` with the reason, `api key:` and the `next:` steps. `$SPICRAWL_API_KEY` is used as before and is not checked or saved. - **status** (**behaviour change**): it prints a repair command for each failed check (`reachable`, `ready`, `key`, `workers.`, `mcp`), each marked `unattended_safe: true|false`; `--json` has them in a `repairs` array of `{check, command, unattended_safe, why}`, plus an `mcp` object (`detected`, `connected`, `needs_env`). It checks that each agent client found in this directory or your home directory has a Spicrawl MCP server that can authenticate: an entry that reads `${SPICRAWL_API_KEY}` does not count while that variable is unset in the environment `status` runs in (`mcp.needs_env`, with a repair that rewrites the entry using the saved key), and a project-level entry shadows a user-level one. **It now exits 1 when any check fails** (the API reports not ready, a worker kind has none ready, an MCP server is missing), where a not-ready API used to exit 0. Key and network failures still exit 3 and 10. +- **init, fixes from QA.** (1) The plan is made and the key checked before anything is saved: a plan that is refused (a customised `spicrawl` entry without `--force`) saves and writes nothing, where it used to leave the new key saved. The key is saved after the changes are confirmed (or with `--yes`) and applied; the plan lists it as `save key`. (2) An existing `spicrawl` entry that differs from the one to write only in its `Authorization: Bearer ...` token (same URL and shape, in `.mcp.json`, `.cursor/mcp.json`, VS Code's `mcp.json` and Codex's `config.toml`) is replaced without `--force`, so a new `--api-key` or a switch from a reference to the key works on a re-run; the old file is kept as `.bak`. (3) With `--global=false`, the `.bak` copy of each project file that holds the key is added to `.gitignore` as well (it can hold the previous key), and a key file that git already tracks is flagged: a warning for a person, and `tracked by git: git rm --cached ` in the result's `reason`. `git` is only asked when it is installed. +- **status, fixes from QA.** An MCP entry with a key written into it that is not the key in use (compared by fingerprint, neither printed) is reported as `mcp.stale` and counts as a failed `mcp` check. The repair for `needs_env` and `stale` entries is now `init --client --yes` (plus `--global=false` for a project entry), without `--force`. +- **README, npm launcher.** The one-prompt text names the command that proves it works: `run npx -y @spicrawl/cli@latest scrape https://news.ycombinator.com`. The `status` text says which output has the repairs where. `npm/bin/spicrawl.js` read `SPICRAWL_BINARY` twice; once now. ## 0.1.3 — 2026-10-05 diff --git a/README.md b/README.md index ee627db..0d38967 100644 --- a/README.md +++ b/README.md @@ -146,7 +146,7 @@ For other clients (Gemini CLI, OpenCode, Factory Droid, Devin and more) use the Paste this into a coding agent that runs shell commands on your computer (Claude Code, Cursor, Codex, VS Code with Copilot agent mode), and put your key in place of ``. Create a key at [app.spicrawl.com/dashboard/keys](https://app.spicrawl.com/dashboard/keys). ```text -Set up Spicrawl for yourself. My API key is . Only if your shell runs on my computer, run: npx -y @spicrawl/cli@latest init --all --yes --api-key — it finds every coding agent installed here, adds the Spicrawl MCP server and installs the Spicrawl skill. Check it: npx -y @spicrawl/cli@latest status --json. If a check failed, run the repairs it lists in order; repairs with unattended_safe: false are mine, so show me the command instead of running it. Prove it works: scrape https://news.ycombinator.com with Spicrawl and tell me today's top story. Report which agents connected and anything you could not finish. Never repeat my API key back to me. If you have no shell on my computer (a chat app or cloud sandbox), stop and send me to https://docs.spicrawl.com/agents/quickstart instead. New MCP tools appear only in a session started after the install. +Set up Spicrawl for yourself. My API key is . Only if your shell runs on my computer, run: npx -y @spicrawl/cli@latest init --all --yes --api-key — it finds every coding agent installed here, adds the Spicrawl MCP server and installs the Spicrawl skill. Check it: npx -y @spicrawl/cli@latest status --json. If a check failed, run the repairs it lists in order; repairs with unattended_safe: false are mine, so show me the command instead of running it. Prove it works: run npx -y @spicrawl/cli@latest scrape https://news.ycombinator.com and tell me today's top story. Report which agents connected and anything you could not finish. Never repeat my API key back to me. If you have no shell on my computer (a chat app or cloud sandbox), stop and send me to https://docs.spicrawl.com/agents/quickstart instead. New MCP tools appear only in a session started after the install. ``` What the two commands do, with no questions asked: @@ -171,7 +171,7 @@ What the two commands do, with no questions asked: ] ``` - `unattended_safe: false` means the user must act (supply a key) or decide; the command holds a placeholder, never a key. The checks are `reachable`, `ready`, `key`, `workers.` and `mcp`: each agent client found in this directory or your home directory needs a Spicrawl MCP server that can authenticate (skipped when no client is found). A server whose key is `${SPICRAWL_API_KEY}` does not count while that variable is unset where `status` runs (`mcp.needs_env`); its repair rewrites the entry with the saved key, or asks for a key if none is saved. A project-level entry shadows a user-level one. Without `--json`, `status` prints the same repairs as `check:`, `command:`, `unattended_safe:` and `why:` lines. + `unattended_safe: false` means the user must act (supply a key) or decide; the command holds a placeholder, never a key. The checks are `reachable`, `ready`, `key`, `workers.` and `mcp`: each agent client found in this directory or your home directory needs a Spicrawl MCP server that can authenticate (skipped when no client is found). A server whose key is `${SPICRAWL_API_KEY}` does not count while that variable is unset where `status` runs (`mcp.needs_env`), and neither does one with another key written into it than the one in use (`mcp.stale`, compared by fingerprint; neither key is printed). The repair, `init --client --yes`, rewrites the entry with the saved key, or asks for a key if none is saved. A project-level entry shadows a user-level one. On a terminal, `status` draws a checklist with the repairs under it; with `NO_COLOR`, `TERM=dumb` or `CI` set it prints them as `check:`, `command:`, `unattended_safe:` and `why:` lines. Piped (as an agent runs it), it prints the JSON above. The key is passed as an argument, so other users on the machine can see it in the process list while the command runs, and it ends up in plain text in the client configs. To avoid both, export `SPICRAWL_API_KEY` and drop `--api-key`: as long as no key is saved in the config file, `init` then writes `$SPICRAWL_API_KEY` references and does not check or save the key (run `spicrawl login` to save it, after which `init` embeds it). Claude Code rewrites `~/.claude.json` itself, so if a running session overwrites the entry, `status` reports the `mcp` check as failed and its repair writes it again (with the saved key, as a literal). diff --git a/internal/cmd/agents.go b/internal/cmd/agents.go index 9dedaa3..d88b6a9 100644 --- a/internal/cmd/agents.go +++ b/internal/cmd/agents.go @@ -24,15 +24,18 @@ package cmd import ( "bytes" + "context" "encoding/json" "errors" "fmt" "io" "net/url" "os" + "os/exec" "path/filepath" "reflect" "strings" + "time" "github.com/OfficialSpicrawl/cli/internal/config" "github.com/OfficialSpicrawl/cli/internal/output" @@ -521,14 +524,49 @@ func agentSecretGuidance(p *output.Printer, rs []*agentResult, s agentScope) { } else { p.Warn("%s now contains your API key in plain text; its permissions are 0600 (owner only)", r.File) } - if rel, err := filepath.Rel(s.Dir, r.File); err == nil && !strings.HasPrefix(rel, "..") { - if ignored, _ := os.ReadFile(filepath.Join(s.Dir, ".gitignore")); !gitignoreHas(ignored, filepath.ToSlash(rel)) { - p.Warn("keep it out of version control: echo %s >> .gitignore", filepath.ToSlash(rel)) + // A project file also leaves a .bak that can hold the previous key. + names := []string{agentProjectRel(s, r.File)} + if r.Backup != "" { + names = append(names, agentProjectRel(s, r.Backup)) + } + ignored, _ := os.ReadFile(filepath.Join(s.Dir, ".gitignore")) + for _, rel := range names { + if rel == "" { + continue + } + if !gitignoreHas(ignored, rel) { + p.Warn("keep it out of version control: echo %s >> .gitignore", rel) + } + if gitTracked(s.Dir, rel) { + p.Warn("%s is tracked by git, so the key in it will be committed: run git rm --cached %s, and treat the key as exposed if it was ever committed", rel, rel) } } } } +// agentProjectRel is file relative to the project directory, slash-separated, +// or "" when it is outside it (a user-level file). +func agentProjectRel(s agentScope, file string) string { + rel, err := filepath.Rel(s.Dir, file) + if err != nil || strings.HasPrefix(rel, "..") { + return "" + } + return filepath.ToSlash(rel) +} + +// gitTracked reports whether git tracks rel in dir. It asks git when git is +// installed and dir is a repository, and says no otherwise: a best-effort +// warning, not a dependency. +func gitTracked(dir, rel string) bool { + git, err := exec.LookPath("git") + if err != nil { + return false + } + ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second) + defer cancel() + return exec.CommandContext(ctx, git, "-C", dir, "ls-files", "--error-unmatch", "--", rel).Run() == nil +} + func agentDisplayPath(p string, s agentScope) string { if rel, err := filepath.Rel(s.Dir, p); err == nil && !strings.HasPrefix(rel, "..") { return rel diff --git a/internal/cmd/auth.go b/internal/cmd/auth.go index c2af037..6c716ec 100644 --- a/internal/cmd/auth.go +++ b/internal/cmd/auth.go @@ -146,11 +146,9 @@ func authRunLogin(cmd *cobra.Command, _ []string) error { return err } -// authSaveKey checks key against the API and saves it to the config file (and -// --base-url, when given) exactly as "login" does; "init --api-key" shares it. -// Nothing is saved when the check fails. It returns the settings the key was -// checked against. -func authSaveKey(p *output.Printer, key string) (config.Resolved, error) { +// authCheck resolves the settings for key and checks the key against the API. +// Nothing is saved. +func authCheck(p *output.Printer, key string) (config.Resolved, error) { r, err := config.Resolve(key, flagBaseURL) if err != nil { return r, err @@ -158,18 +156,33 @@ func authSaveKey(p *output.Printer, key string) (config.Resolved, error) { sp := spin(p, "Checking the key with %s", r.BaseURL) err = authCheckKey(rootCmd.Context(), r.BaseURL, key) sp.Stop() - if err != nil { - return r, err - } + return r, err +} + +// authStore saves key to the config file, and --base-url when given, as +// "login" does. +func authStore(key string) error { f, err := config.Load() if err != nil { - return r, err + return err } f.APIKey = key if flagBaseURL != "" { f.BaseURL = strings.TrimRight(flagBaseURL, "/") } - return r, config.Save(f) + return config.Save(f) +} + +// authSaveKey checks key against the API and saves it, exactly as "login" does. +// Nothing is saved when the check fails. It returns the settings the key was +// checked against. "init --api-key" checks first and saves only after its plan +// is confirmed and applied. +func authSaveKey(p *output.Printer, key string) (config.Resolved, error) { + r, err := authCheck(p, key) + if err != nil { + return r, err + } + return r, authStore(key) } // authPromptKey asks for the key on the terminal. Without golang.org/x/term diff --git a/internal/cmd/initcmd.go b/internal/cmd/initcmd.go index e13fdc1..ed50674 100644 --- a/internal/cmd/initcmd.go +++ b/internal/cmd/initcmd.go @@ -139,12 +139,17 @@ func runInit(cmd *cobra.Command, _ []string) error { } keyLine += ")" } + var saveKey func() error // set when --api-key is to be saved once the changes are applied + saveNote := "" // the same, for the plan if keySource == "flag" { - // Check it and save it before writing any client file: a bad key must not leave a half-done setup. - r, err := authSaveKey(p, key) + // Check it now, so a rejected key stops the run before anything is planned + // or written; it is saved only after the plan was confirmed and applied. + r, err := authCheck(p, key) if err != nil { return err } + saveKey = func() error { return authStore(key) } + saveNote = config.MaskKey(key) + " to " + r.Path keyLine = config.MaskKey(key) + " (checked, saved to " + r.Path if embed { keyLine += "; written into the client configs" @@ -190,7 +195,7 @@ func runInit(cmd *cobra.Command, _ []string) error { if pending > 0 && !initFlags.yes { // JSON mode keeps stderr for the problem document, unless a person is about to be asked. if !p.JSON || initStdinIsTerminal() { - initPrintPlan(p.Err, errUI(p), rs, s) + initPrintPlan(p.Err, errUI(p), rs, s, saveNote) } if !initStdinIsTerminal() { if p.JSON { @@ -216,6 +221,11 @@ func runInit(cmd *cobra.Command, _ []string) error { if err := agentApply(rs); err != nil { return err } + if saveKey != nil { + if err := saveKey(); err != nil { + return err + } + } if err := agentReport(p, rs, s); err != nil { return err } @@ -304,13 +314,15 @@ func initPlanGitignore(s agentScope, rs []*agentResult) (*agentResult, error) { if r.Kind != "mcp" || !r.Secret || r.Action == agentActionSkipped { continue } - rel, err := filepath.Rel(s.Dir, r.File) - if err != nil || strings.HasPrefix(rel, "..") { + rel := agentProjectRel(s, r.File) + if rel == "" { continue } - rel = filepath.ToSlash(rel) - if !gitignoreHas(old, rel) && !slices.Contains(add, rel) { - add = append(add, rel) + // The .bak copy of a config that held a key holds it too. + for _, name := range []string{rel, rel + ".bak"} { + if !gitignoreHas(old, name) && !slices.Contains(add, name) { + add = append(add, name) + } } } if len(add) == 0 { @@ -336,7 +348,7 @@ func gitignoreHas(content []byte, rel string) bool { return false } -func initPrintPlan(w io.Writer, u *ui.UI, rs []*agentResult, s agentScope) { +func initPrintPlan(w io.Writer, u *ui.UI, rs []*agentResult, s agentScope, saveNote string) { fmt.Fprintln(w, u.Bold("plan:")) for _, r := range rs { verb := map[string]string{ @@ -357,6 +369,9 @@ func initPrintPlan(w io.Writer, u *ui.UI, rs []*agentResult, s agentScope) { } fmt.Fprintln(w, line) } + if saveNote != "" { + fmt.Fprintf(w, " %s %-7s %-6s %s\n", u.Green("save "), "", "key", saveNote) + } } func initConfirm(w io.Writer, r io.Reader) (bool, error) { diff --git a/internal/cmd/mcpcmd.go b/internal/cmd/mcpcmd.go index 6437e72..6595c54 100644 --- a/internal/cmd/mcpcmd.go +++ b/internal/cmd/mcpcmd.go @@ -2,11 +2,13 @@ package cmd import ( "bytes" + "crypto/sha256" "encoding/json" "errors" "fmt" "io" "os" + "reflect" "regexp" "strconv" "strings" @@ -198,6 +200,22 @@ func mcpConfigured(c *agentClient, path string) bool { // bearer_token_env_var (Codex). A ${VAR:-default} form has a fallback and does not match. var mcpEnvRefRE = regexp.MustCompile(`\$\{(?:env:)?` + config.EnvAPIKey + `\}|bearer_token_env_var\s*=\s*["']` + config.EnvAPIKey + `["']`) +// mcpEntryKeyRE finds the token of an Authorization header in an entry. +var mcpEntryKeyRE = regexp.MustCompile(`Bearer\s+([^"'\s]+)`) + +// mcpEntryStale reports whether an entry has a literal key that is not key. +// The keys are compared by fingerprint and never printed. An entry with a +// reference, or no key at all, is not stale; with no key in use nothing is. +func mcpEntryStale(entry, key string) bool { + m := mcpEntryKeyRE.FindStringSubmatch(entry) + if m == nil || key == "" || strings.Contains(m[1], "${") { + return false + } + return keyFingerprint(m[1]) != keyFingerprint(key) +} + +func keyFingerprint(k string) [sha256.Size]byte { return sha256.Sum256([]byte(k)) } + // mcpEntryNeedsEnv reports whether an entry's key comes from the environment. func mcpEntryNeedsEnv(entry string) bool { return mcpEnvRefRE.MatchString(entry) } @@ -299,6 +317,12 @@ func mcpPlan(c *agentClient, s agentScope, opt mcpOptions) (*agentResult, error) if secret && r.Action != agentActionUnchange && r.Reason == "" { r.Reason = "contains the literal API key" } + if rel := agentProjectRel(s, r.File); secret && rel != "" && gitTracked(s.Dir, rel) { + if r.Reason != "" { + r.Reason += "; " + } + r.Reason += "tracked by git: git rm --cached " + rel + } return r, nil } @@ -330,7 +354,7 @@ func mcpMergeJSON(c *agentClient, path string, s agentScope, opt mcpOptions) ([] desired := agentRaw(entry) current, had := servers.get(agentServerName) changed := !had || !agentJSONEqual(current, desired) - if had && changed && !opt.Force { + if had && changed && !opt.Force && !mcpOnlyAuthDiffers(current, desired) { o, _ := agentIndent(current) n, _ := agentIndent(desired) return nil, secret, false, mcpRefuse(path, string(o), string(n)) @@ -371,6 +395,33 @@ func mcpMergeJSON(c *agentClient, path string, s agentScope, opt mcpOptions) ([] return out, secret, true, err } +// mcpOnlyAuthDiffers reports whether an existing server entry is the one we +// would write except for its bearer token: same keys, same URL, and an +// Authorization header of the form "Bearer " on both sides (a literal +// key, or the ${...} reference we write). Such an entry is ours from an +// earlier run, so a new key or a switch between key and reference replaces it +// without --force; the file is still copied to .bak first. +func mcpOnlyAuthDiffers(current, desired json.RawMessage) bool { + var c, d map[string]any + if json.Unmarshal(current, &c) != nil || json.Unmarshal(desired, &d) != nil { + return false + } + bearer := func(m map[string]any) (string, bool) { + h, _ := m["headers"].(map[string]any) + a, _ := h["Authorization"].(string) + tok, ok := strings.CutPrefix(a, "Bearer ") + return tok, ok && tok != "" && !strings.ContainsAny(tok, " \t") + } + if _, ok := bearer(c); !ok { + return false + } + if _, ok := bearer(d); !ok { + return false + } + c["headers"].(map[string]any)["Authorization"] = d["headers"].(map[string]any)["Authorization"] + return reflect.DeepEqual(c, d) +} + // mcpParseConfig parses a client's JSON config. It accepts a UTF-8 BOM and, // as VS Code does for mcp.json, comments and trailing commas. lossy reports // that comments were present: rewriting the file drops them. @@ -997,6 +1048,34 @@ func mcpTOMLUpdate(entries []tomlEntry, opt mcpOptions) ([]tomlEntry, error) { return res, nil } +// mcpTOMLAuthOnly reports whether the update res of an entry's pairs changes +// only how the key is passed (bearer_token_env_var, http_headers): the url and +// every other pair stay as they were. +func mcpTOMLAuthOnly(entries, res []tomlEntry) bool { + isAuth := func(k []string) bool { + return tomlPathIs(k, "bearer_token_env_var") || len(k) > 0 && k[0] == "http_headers" + } + kept := map[int]bool{} + for _, e := range res { + switch { + case e.orig < 0: + if !strings.HasPrefix(e.text, "bearer_token_env_var") && !strings.HasPrefix(e.text, "http_headers") { + return false + } + case e.text != "" && !isAuth(e.key): + return false + default: + kept[e.orig] = true + } + } + for i, e := range entries { + if !kept[i] && !isAuth(e.key) { + return false + } + } + return true +} + // mcpMergeTOML sets the spicrawl server in a Codex config.toml, editing an // existing entry in place (whatever form it takes) or appending a // [mcp_servers.spicrawl] table. The rest of the file is left byte-for-byte. @@ -1028,6 +1107,7 @@ func mcpMergeTOML(path string, _ agentScope, opt mcpOptions) ([]byte, bool, bool } var out string + authOnly := false // the update changes nothing but the token (see mcpOnlyAuthDiffers) switch { case table < 0 && inline < 0: var b strings.Builder @@ -1049,6 +1129,7 @@ func mcpMergeTOML(path string, _ agentScope, opt mcpOptions) ([]byte, bool, bool if err != nil { return refuse(err) } + authOnly = mcpTOMLAuthOnly(entries, res) line := st.indent + st.rawKey + " = " + tomlRenderInline(res) if st.comment != "" { line += " " + st.comment @@ -1073,6 +1154,7 @@ func mcpMergeTOML(path string, _ agentScope, opt mcpOptions) ([]byte, bool, bool if err != nil { return refuse(err) } + authOnly = mcpTOMLAuthOnly(entries, res) // Where each source pair goes (or nothing, if dropped), and the new // pairs that follow it; new pairs before any source pair follow the header. emit := map[int]*tomlEntry{} @@ -1124,7 +1206,7 @@ func mcpMergeTOML(path string, _ agentScope, opt mcpOptions) ([]byte, bool, bool if out == string(old) { return old, secret, false, nil } - if (table >= 0 || inline >= 0) && !opt.Force && out != strings.TrimSuffix(text, "\n")+"\n" { + if (table >= 0 || inline >= 0) && !opt.Force && !authOnly && out != strings.TrimSuffix(text, "\n")+"\n" { return nil, secret, false, mcpRefuse(path, text, out) } // Never write something we would refuse to read back. diff --git a/internal/cmd/onboarding_qa_test.go b/internal/cmd/onboarding_qa_test.go new file mode 100644 index 0000000..8f7b0e0 --- /dev/null +++ b/internal/cmd/onboarding_qa_test.go @@ -0,0 +1,403 @@ +package cmd + +import ( + "encoding/json" + "os" + "os/exec" + "path/filepath" + "strings" + "testing" + + "github.com/OfficialSpicrawl/cli/internal/config" +) + +// Re-running init with another key, saving only once the plan is accepted, the +// .bak copies of key files, and the docs of the one-prompt flow. + +// persistConfig keeps the CLI config file across the runCLI calls of a test. +func persistConfig(t *testing.T) { + t.Helper() + testConfig = filepath.Join(t.TempDir(), "config.json") + testKey = "" // $SPICRAWL_API_KEY is not exported in this shell + t.Cleanup(func() { testConfig, testKey = "", "spicrawl_test_key" }) +} + +func readFile(t *testing.T, path string) string { + t.Helper() + b, err := os.ReadFile(path) + if err != nil { + t.Fatal(err) + } + return string(b) +} + +// ---- B2: a new key over the old one ---- + +// Entries we wrote earlier differ from the new ones only in the token: init +// replaces them without --force, keeps a .bak, and saves the new key. +func TestInitNewKeyReplacesOldKeyWithoutForce(t *testing.T) { + persistConfig(t) + base := onboardAPI(t) + dir := onboardProject(t) + home := os.Getenv("HOME") + run := func(key string) (string, string, int) { + return runCLI(t, base, "init", "--all", "--yes", "--api-key", key, "--dir", dir) + } + if _, errb, code := run(onboardOldKey); code != 0 { + t.Fatalf("first run: exit %d: %s", code, errb) + } + out, errb, code := run(onboardKey) + if code != 0 { + t.Fatalf("second run: exit %d: %s", code, errb) + } + var rs []agentResult + if err := json.Unmarshal([]byte(out), &rs); err != nil { + t.Fatal(err) + } + for _, r := range rs { + if r.Kind == "mcp" && r.Action != agentActionUpdated { + t.Errorf("%+v", r) + } + } + for _, f := range []string{".claude.json", ".cursor/mcp.json"} { + now, bak := readFile(t, filepath.Join(home, f)), readFile(t, filepath.Join(home, f+".bak")) + if !strings.Contains(now, onboardKey) || strings.Contains(now, onboardOldKey) { + t.Errorf("%s:\n%s", f, now) + } + if !strings.Contains(bak, onboardOldKey) { + t.Errorf("%s.bak does not keep the old file:\n%s", f, bak) + } + } + if savedKey(t) != onboardKey { + t.Error("the new key was not saved") + } + for _, k := range []string{onboardKey, onboardOldKey} { + noKeyIn(t, k, "stdout", out) + noKeyIn(t, k, "stderr", errb) + } +} + +// The same for a Codex config, where the pairs are edited in place; a changed +// URL is still a refusal. +func TestInitNewKeyCodexWithoutForce(t *testing.T) { + persistConfig(t) + base := onboardAPI(t) + dir, home := onboardProject(t), os.Getenv("HOME") + toml := filepath.Join(home, ".codex", "config.toml") + args := func(key string) []string { + return []string{"init", "--client", "codex", "--yes", "--api-key", key, "--dir", dir} + } + // A reference we wrote becomes the key; so does an older key. + agentWrite(t, toml, "[mcp_servers.spicrawl]\nurl = \""+base+"/mcp\"\nbearer_token_env_var = \"SPICRAWL_API_KEY\"\n") + if _, errb, code := runCLI(t, base, args(onboardOldKey)...); code != 0 { + t.Fatalf("reference to key: exit %d: %s", code, errb) + } + if got := readFile(t, toml); !strings.Contains(got, onboardOldKey) || strings.Contains(got, "bearer_token_env_var") { + t.Errorf("config.toml:\n%s", got) + } + if _, errb, code := runCLI(t, base, args(onboardKey)...); code != 0 { + t.Fatalf("key to key: exit %d: %s", code, errb) + } + if got := readFile(t, toml); !strings.Contains(got, onboardKey) || strings.Contains(got, onboardOldKey) { + t.Errorf("config.toml:\n%s", got) + } + // Another URL is the user's own entry. + agentWrite(t, toml, "[mcp_servers.spicrawl]\nurl = \"https://mine.example/mcp\"\nbearer_token_env_var = \"SPICRAWL_API_KEY\"\n") + _, errb, code := runCLI(t, base, args(onboardKey)...) + if code != 2 || !strings.Contains(errb, "--force") { + t.Errorf("a customised URL: exit %d: %s", code, errb) + } + noKeyIn(t, onboardKey, "stderr", errb) +} + +func TestMCPOnlyAuthDiffers(t *testing.T) { + entry := func(url, auth string, extra string) json.RawMessage { + return json.RawMessage(`{"type":"http","url":"` + url + `","headers":{"Authorization":"` + auth + `"` + extra + `}}`) + } + want := entry("https://x/mcp", "Bearer spicrawl_live_NEW", "") + for _, tc := range []struct { + name string + cur json.RawMessage + want bool + }{ + {"another key", entry("https://x/mcp", "Bearer spicrawl_live_OLD", ""), true}, + {"a reference", entry("https://x/mcp", "Bearer ${SPICRAWL_API_KEY}", ""), true}, + {"a Cursor reference", entry("https://x/mcp", "Bearer ${env:SPICRAWL_API_KEY}", ""), true}, + {"another URL", entry("https://mine/mcp", "Bearer spicrawl_live_OLD", ""), false}, + {"another header too", entry("https://x/mcp", "Bearer spicrawl_live_OLD", `,"X-Mine":"1"`), false}, + {"not a bearer token", entry("https://x/mcp", "Basic abc", ""), false}, + {"a token with spaces", entry("https://x/mcp", "Bearer a b", ""), false}, + {"another shape", json.RawMessage(`{"command":"x"}`), false}, + } { + if got := mcpOnlyAuthDiffers(tc.cur, want); got != tc.want { + t.Errorf("%s: %v, want %v", tc.name, got, tc.want) + } + } +} + +// A refused plan writes and saves nothing, not even the key. +func TestInitRefusedPlanSavesNothing(t *testing.T) { + persistConfig(t) + base := onboardAPI(t) + dir, home := onboardProject(t), os.Getenv("HOME") + mine := `{"mcpServers":{"spicrawl":{"url":"https://mine.example/mcp","headers":{"Authorization":"Bearer ` + onboardOldKey + `"}}}}` + agentWrite(t, filepath.Join(home, ".cursor", "mcp.json"), mine) + out, errb, code := runCLI(t, base, "init", "--all", "--yes", "--api-key", onboardKey, "--dir", dir) + if code != 2 || !strings.Contains(errb, "--force") { + t.Fatalf("exit %d: %s", code, errb) + } + if savedKey(t) != "" { + t.Error("the key was saved although the plan was refused") + } + if got := readFile(t, filepath.Join(home, ".cursor", "mcp.json")); got != mine { + t.Errorf("the refused file changed:\n%s", got) + } + for _, f := range []string{".claude.json", ".claude/skills/spicrawl/SKILL.md"} { + if _, err := os.Stat(filepath.Join(home, f)); err == nil { + t.Errorf("%s was written although the plan was refused", f) + } + } + for _, k := range []string{onboardKey, onboardOldKey} { + noKeyIn(t, k, "stdout", out) + noKeyIn(t, k, "stderr", errb) + } +} + +// status sees an embedded key that is not the one in use, never prints either, +// and the repair fixes it. +func TestStatusStaleEmbeddedKey(t *testing.T) { + usesSavedKey(t) // onboardKey is saved and in use + base := onboardAPI(t) + dir, home := onboardProject(t), os.Getenv("HOME") + t.Chdir(dir) + entry := func(key string) string { + return `{"mcpServers":{"spicrawl":{"type":"http","url":"` + base + `/mcp","headers":{"Authorization":"Bearer ` + key + `"}}}}` + } + agentWrite(t, filepath.Join(home, ".claude.json"), entry(onboardOldKey)) + agentWrite(t, filepath.Join(home, ".cursor", "mcp.json"), strings.Replace(entry(onboardKey), `"type":"http",`, "", 1)) + + out, errb, code := runCLI(t, base, "status") + var rep statusReport + if err := json.Unmarshal([]byte(out), &rep); err != nil { + t.Fatal(err) + } + if code != 1 || strings.Join(rep.MCP.Stale, ",") != "claude" || strings.Join(rep.MCP.Connected, ",") != "cursor" { + t.Fatalf("exit %d, mcp %+v\n%s", code, rep.MCP, errb) + } + r := repairFor(rep, "mcp") + if r == nil || !r.UnattendedSafe || r.Command != "spicrawl init --client claude --yes" { + t.Fatalf("repair %+v", r) + } + for _, k := range []string{onboardKey, onboardOldKey} { + noKeyIn(t, k, "stdout", out) + noKeyIn(t, k, "stderr", errb) + } + if _, errb, code := runCLI(t, base, strings.Fields(r.Command)[1:]...); code != 0 { + t.Fatalf("%s: exit %d: %s", r.Command, code, errb) + } + if rep, _, code := statusJSON(t, base); code != 0 || len(rep.Repairs) != 0 { + t.Errorf("after the repair: exit %d, %+v", code, rep) + } + + // With no key saved the repair needs the user's, and is not safe to run unattended. + agentWrite(t, filepath.Join(home, ".claude.json"), entry(onboardOldKey)) + testConfig = filepath.Join(t.TempDir(), "config.json") + out, _, _ = runCLI(t, base, "status", "--api-key", onboardKey) + _ = json.Unmarshal([]byte(out), &rep) + r = repairFor(rep, "mcp") + if r == nil || r.UnattendedSafe || r.Command != "spicrawl init --client claude --yes --api-key " { + t.Errorf("repair without a saved key: %+v", r) + } + noKeyIn(t, onboardKey, "stdout", out) +} + +// ---- B3: .bak files and tracked files ---- + +// A project file that held a key leaves a .bak that holds it too: it is +// ignored from the start, and a second run keeps .gitignore as it is. +func TestInitProjectBackupIsGitignored(t *testing.T) { + persistConfig(t) + base := onboardAPI(t) + dir := onboardProject(t) + agentWrite(t, filepath.Join(dir, ".gitignore"), "node_modules/\n") + run := func(key string) { + t.Helper() + if _, errb, code := runCLI(t, base, "init", "--all", "--yes", "--api-key", key, "--global=false", "--dir", dir); code != 0 { + t.Fatalf("exit %d: %s", code, errb) + } + } + run(onboardOldKey) + gi := readFile(t, filepath.Join(dir, ".gitignore")) + run(onboardKey) + if bak := readFile(t, filepath.Join(dir, ".mcp.json.bak")); !strings.Contains(bak, onboardOldKey) { + t.Errorf(".mcp.json.bak:\n%s", bak) + } + for _, want := range []string{".mcp.json\n", ".mcp.json.bak\n", ".cursor/mcp.json.bak\n"} { + if !strings.Contains(gi, want) { + t.Errorf(".gitignore lacks %q:\n%s", want, gi) + } + } + if again := readFile(t, filepath.Join(dir, ".gitignore")); again != gi { + t.Errorf("the second run changed .gitignore:\n%s", again) + } +} + +func gitInDir(t *testing.T, dir string, args ...string) { + t.Helper() + if out, err := exec.Command("git", append([]string{"-C", dir}, args...)...).CombinedOutput(); err != nil { + t.Fatalf("git %v: %v\n%s", args, err, out) + } +} + +// A key file git already tracks would be committed with the key: say so, in the +// warning for a person and in the result's reason for a script. +func TestInitWarnsWhenTheKeyFileIsTracked(t *testing.T) { + if _, err := exec.LookPath("git"); err != nil { + t.Skip("git is not installed") + } + persistConfig(t) + base := onboardAPI(t) + setup := func() string { + dir := onboardProject(t) + gitInDir(t, dir, "init", "-q") + agentWrite(t, filepath.Join(dir, ".mcp.json"), + `{"mcpServers":{"spicrawl":{"type":"http","url":"`+base+`/mcp","headers":{"Authorization":"Bearer ${SPICRAWL_API_KEY}"}}}}`) + gitInDir(t, dir, "add", ".mcp.json") + return dir + } + args := func(dir string) []string { + return []string{"init", "--client", "claude", "--yes", "--api-key", onboardKey, "--global=false", "--dir", dir} + } + + dir := setup() + _, errb, code := runCLIHuman(t, base, args(dir)...) + if code != 0 { + t.Fatalf("exit %d: %s", code, errb) + } + if !strings.Contains(errb, ".mcp.json is tracked by git") || !strings.Contains(errb, "git rm --cached .mcp.json") { + t.Errorf("no warning:\n%s", errb) + } + noKeyIn(t, onboardKey, "stderr", errb) + + out, errb, code := runCLI(t, base, args(setup())...) + if code != 0 || errb != "" { + t.Fatalf("JSON: exit %d, stderr %q", code, errb) + } + var rs []agentResult + _ = json.Unmarshal([]byte(out), &rs) + if len(rs) == 0 || !strings.Contains(rs[0].Reason, "tracked by git: git rm --cached .mcp.json") { + t.Errorf("reason: %+v", rs) + } + + // A file git does not track gets no warning. + untracked := onboardProject(t) + gitInDir(t, untracked, "init", "-q") + _, errb, code = runCLIHuman(t, base, args(untracked)...) + if code != 0 || strings.Contains(errb, "tracked by git") { + t.Errorf("untracked: exit %d\n%s", code, errb) + } +} + +// ---- B4: the key is saved once the changes are accepted ---- + +func TestInitKeyIsSavedOnlyAfterConfirmation(t *testing.T) { + persistConfig(t) + base := onboardAPI(t) + + // No terminal and no --yes: refused, nothing saved or written. + dir := onboardProject(t) + home := os.Getenv("HOME") + _, errb, code := runCLI(t, base, "init", "--all", "--api-key", onboardKey, "--dir", dir) + if code != 2 || !strings.Contains(errb, "--yes") { + t.Fatalf("exit %d: %s", code, errb) + } + if savedKey(t) != "" { + t.Error("the key was saved before the confirmation") + } + if _, err := os.Stat(filepath.Join(home, ".claude.json")); err == nil { + t.Error("a config was written without --yes") + } + + // At a terminal: "n" saves nothing, "y" saves and writes. + orig, origIn := initStdinIsTerminal, initStdin + initStdinIsTerminal = func() bool { return true } + t.Cleanup(func() { initStdinIsTerminal, initStdin = orig, origIn }) + for _, tc := range []struct { + answer string + saved bool + }{{"n\n", false}, {"y\n", true}} { + initStdin = strings.NewReader(tc.answer) + out, errb, code := runCLIHuman(t, base, "init", "--all", "--api-key", onboardKey, "--dir", dir) + if code != 0 { + t.Fatalf("%q: exit %d: %s", tc.answer, code, errb) + } + if !strings.Contains(errb, "save ") || !strings.Contains(errb, "key") { + t.Errorf("%q: the plan does not mention saving the key:\n%s", tc.answer, errb) + } + if got := savedKey(t) == onboardKey; got != tc.saved { + t.Errorf("%q: key saved = %v, want %v", tc.answer, got, tc.saved) + } + _, err := os.Stat(filepath.Join(home, ".claude.json")) + if (err == nil) != tc.saved { + t.Errorf("%q: config written = %v, want %v\n%s\n%s", tc.answer, err == nil, tc.saved, out, errb) + } + noKeyIn(t, onboardKey, "stdout", out) + noKeyIn(t, onboardKey, "stderr", errb) + } +} + +// Nothing to change, but a key to save: it is saved. +func TestInitSavesTheKeyWhenNothingElseChanges(t *testing.T) { + persistConfig(t) + base := onboardAPI(t) + dir := onboardProject(t) + if _, errb, code := runCLI(t, base, "init", "--all", "--yes", "--api-key", onboardKey, "--dir", dir); code != 0 { + t.Fatalf("exit %d: %s", code, errb) + } + if err := os.Remove(testConfig); err != nil { + t.Fatal(err) + } + if _, errb, code := runCLI(t, base, "init", "--all", "--api-key", onboardKey, "--dir", dir); code != 0 { + t.Fatalf("exit %d: %s", code, errb) // no --yes: nothing is pending + } + if savedKey(t) != onboardKey { + t.Error("the key was not saved") + } +} + +// ---- B5: the docs of the flow ---- + +func TestOnePromptDocs(t *testing.T) { + readme := readFile(t, filepath.Join("..", "..", "README.md")) + i := strings.Index(readme, "Set up Spicrawl for yourself.") + if i < 0 { + t.Fatal("the prompt is not in the README") + } + prompt := readme[i : i+strings.Index(readme[i:], "\n")] + for _, want := range []string{ + "npx -y @spicrawl/cli@latest init --all --yes --api-key ", + "npx -y @spicrawl/cli@latest status --json", + "run npx -y @spicrawl/cli@latest scrape https://news.ycombinator.com and tell me today's top story", + "Never repeat my API key back to me.", + "New MCP tools appear only in a session started after the install.", + } { + if !strings.Contains(prompt, want) { + t.Errorf("the prompt lacks %q", want) + } + } + if strings.Contains(prompt, "spicrawl_live_") || strings.Contains(prompt, config.EnvAPIKey) { + t.Error("the prompt holds a key, not the placeholder") + } + if strings.Contains(readme, "Without `--json`, `status` prints") { + t.Error("the README claims status prints lines without --json; piped it prints JSON") + } + + // The npm launcher looks SPICRAWL_BINARY up once. + js := readFile(t, filepath.Join("..", "..", "npm", "bin", "spicrawl.js")) + if n := strings.Count(js, "process.env.SPICRAWL_BINARY) return"); n != 1 { + t.Errorf("the launcher reads SPICRAWL_BINARY %d times", n) + } + if strings.Contains(js, "fallback") { + t.Error("the launcher comment still mentions a fallback name") + } +} diff --git a/internal/cmd/onboarding_test.go b/internal/cmd/onboarding_test.go index d4625d5..7c820ac 100644 --- a/internal/cmd/onboarding_test.go +++ b/internal/cmd/onboarding_test.go @@ -13,7 +13,10 @@ import ( // The one-prompt setup: "init --all --yes --api-key K", then "status --json". -const onboardKey = "spicrawl_live_ONBOARDSECRET0123456789abcd" +const ( + onboardKey = "spicrawl_live_ONBOARDSECRET0123456789abcd" + onboardOldKey = "spicrawl_live_OLDSECRETKEY0123456789abcdef" +) // onboardAPI answers like a healthy API that accepts onboardKey only. func onboardAPI(t *testing.T) string { @@ -25,7 +28,7 @@ func onboardAPI(t *testing.T) string { case "/v1/workers": _, _ = w.Write([]byte(`{"kinds":[{"kind":"browser","present":1,"ready":1}]}`)) case "/v1/requests": - if r.Header.Get("Authorization") != "Bearer "+onboardKey && r.Header.Get("Authorization") != "Bearer spicrawl_test_key" { + if a := r.Header.Get("Authorization"); a != "Bearer "+onboardKey && a != "Bearer "+onboardOldKey && a != "Bearer spicrawl_test_key" { writeProblem(w, 401, "ERR::AUTH::INVALID_KEY", false) return } @@ -193,9 +196,10 @@ func TestInitAPIKeyProjectFilesAreGitignored(t *testing.T) { if fi, _ := os.Stat(filepath.Join(dir, ".mcp.json")); fi.Mode().Perm() != 0o600 { t.Errorf(".mcp.json mode %v", fi.Mode().Perm()) } - // Only the file that was not ignored yet is added; no .bak is left behind. + // What was not ignored yet is added, with the .bak copies (they can hold a + // previous key); no .gitignore.bak is left behind. gi, _ := os.ReadFile(filepath.Join(dir, ".gitignore")) - if string(gi) != "node_modules/\n/.cursor/mcp.json\n.mcp.json\n" { + if string(gi) != "node_modules/\n/.cursor/mcp.json\n.mcp.json\n.mcp.json.bak\n.cursor/mcp.json.bak\n" { t.Errorf(".gitignore:\n%s", gi) } if _, err := os.Stat(filepath.Join(dir, ".gitignore.bak")); err == nil { @@ -335,7 +339,7 @@ func TestInitNeverEchoesTheKeyInErrors(t *testing.T) { func TestInitRefusalMasksBothKeys(t *testing.T) { base := onboardAPI(t) dir := onboardProject(t) - oldKey := "spicrawl_live_OLDSECRETKEY0123456789abcdef" + oldKey := onboardOldKey agentWrite(t, filepath.Join(os.Getenv("HOME"), ".cursor", "mcp.json"), `{"mcpServers":{"spicrawl":{"url":"https://mine.example/mcp","headers":{"Authorization":"Bearer `+oldKey+`"}}}}`) out, errb, code := runCLI(t, base, "init", "--all", "--yes", "--api-key", onboardKey, "--dir", dir) @@ -575,6 +579,7 @@ func TestStatusRepairsUseNpx(t *testing.T) { // The loop the prompt relies on: status fails with an mcp repair, running that // repair fixes it, status passes and exits 0. func TestStatusPassesAfterInit(t *testing.T) { + usesSavedKey(t) // the key status uses is the one init embeds base := onboardAPI(t) dir := onboardProject(t) t.Chdir(dir) @@ -671,7 +676,7 @@ func TestStatusMCPNeedsEnv(t *testing.T) { t.Fatalf("env unset: exit %d, mcp %+v\n%s", code, bad.MCP, errb) } r := repairFor(bad, "mcp") - if r == nil || r.UnattendedSafe || r.Command != "spicrawl init --client claude,cursor --yes --force --global=false --api-key " { + if r == nil || r.UnattendedSafe || r.Command != "spicrawl init --client claude,cursor --yes --global=false --api-key " { t.Errorf("repair %+v", r) } noKeyIn(t, onboardKey, "stdout", out) @@ -686,9 +691,11 @@ func TestStatusMCPNeedsEnvRepairLoop(t *testing.T) { base := onboardAPI(t) dir := onboardProject(t) t.Chdir(dir) - agentWrite(t, filepath.Join(dir, ".mcp.json"), envRefEntry) + // The entries are the ones init wrote for this server, with the key as a reference. + agentWrite(t, filepath.Join(dir, ".mcp.json"), + `{"mcpServers": {"spicrawl": {"type": "http", "url": "`+base+`/mcp", "headers": {"Authorization": "Bearer ${SPICRAWL_API_KEY}"}}}}`) agentWrite(t, filepath.Join(os.Getenv("HOME"), ".cursor", "mcp.json"), - `{"mcpServers": {"spicrawl": {"url": "https://mcp.spicrawl.com/mcp", "headers": {"Authorization": "Bearer ${env:SPICRAWL_API_KEY}"}}}}`) + `{"mcpServers": {"spicrawl": {"url": "`+base+`/mcp", "headers": {"Authorization": "Bearer ${env:SPICRAWL_API_KEY}"}}}}`) // Claude's entry is in the project, Cursor's at user level. rep, _, code := statusJSON(t, base) if code != 1 || strings.Join(rep.MCP.NeedsEnv, ",") != "claude,cursor" { @@ -701,7 +708,7 @@ func TestStatusMCPNeedsEnvRepairLoop(t *testing.T) { } cmds = append(cmds, r.Command) } - want := []string{"spicrawl init --client cursor --yes --force", "spicrawl init --client claude --yes --force --global=false"} + want := []string{"spicrawl init --client cursor --yes", "spicrawl init --client claude --yes --global=false"} if strings.Join(cmds, "|") != strings.Join(want, "|") { t.Fatalf("repairs %q, want %q", cmds, want) } diff --git a/internal/cmd/ops_golden_test.go b/internal/cmd/ops_golden_test.go index 9c7f656..9b45258 100644 --- a/internal/cmd/ops_golden_test.go +++ b/internal/cmd/ops_golden_test.go @@ -67,6 +67,14 @@ func gStatusEnvRefClient(t *testing.T, e *goldenEnv) { goldenWrite(t, e.CfgDir, "config.json", `{"api_key": "`+goldenKey+`"}`) } +// gStatusStaleClient is Claude Code with a Spicrawl server that holds another +// key than the one saved in the CLI config. +func gStatusStaleClient(t *testing.T, e *goldenEnv) { + goldenWrite(t, e.Home, ".claude/settings.json", `{}`) + goldenWrite(t, e.Home, ".claude.json", `{"mcpServers": {"spicrawl": {"type": "http", "url": "https://mcp.spicrawl.com/mcp", "headers": {"Authorization": "Bearer spicrawl_live_OLDOLDOLD0123456789"}}}}`) + goldenWrite(t, e.CfgDir, "config.json", `{"api_key": "`+goldenKey+`"}`) +} + // gStatusAPIWorkersDown is a healthy API whose fleet endpoint fails. func gStatusAPIWorkersDown() func() http.HandlerFunc { ok := gStatusAPI(true, 200, "")() @@ -155,6 +163,7 @@ func init() { goldenCase{name: "status/worker_kind_down_exit1", args: []string{"status"}, server: gStatusAPI(true, 200, gWorkers)}, goldenCase{name: "status/mcp_not_configured_exit1", args: []string{"status"}, server: gStatusAPI(true, 200, gWorkersOK), setup: gStatusClient(false)}, goldenCase{name: "status/mcp_needs_env_exit1", args: []string{"status"}, noKey: true, server: gStatusAPI(true, 200, gWorkersOK), setup: gStatusEnvRefClient}, + goldenCase{name: "status/mcp_stale_key_exit1", args: []string{"status"}, noKey: true, server: gStatusAPI(true, 200, gWorkersOK), setup: gStatusStaleClient}, goldenCase{name: "status/mcp_connected", args: []string{"status"}, server: gStatusAPI(true, 200, gWorkersOK), setup: gStatusClient(true)}, goldenCase{name: "status/key_invalid_exit3", args: []string{"status"}, stderr: true, server: gStatusAPI(true, 401, gWorkers)}, goldenCase{name: "status/key_without_read_scope", args: []string{"status"}, server: gStatusAPI(true, 403, gWorkersOK)}, @@ -164,6 +173,7 @@ func init() { goldenCase{name: "status_human/ok", human: true, args: []string{"status"}, server: gStatusAPI(true, 200, gWorkersOK)}, goldenCase{name: "status_human/not_ready", human: true, args: []string{"status"}, server: gStatusAPI(false, 200, gWorkers)}, goldenCase{name: "status_human/mcp_needs_env_exit1", human: true, args: []string{"status"}, noKey: true, server: gStatusAPI(true, 200, gWorkersOK), setup: gStatusEnvRefClient}, + goldenCase{name: "status_human/mcp_stale_key_exit1", human: true, args: []string{"status"}, noKey: true, server: gStatusAPI(true, 200, gWorkersOK), setup: gStatusStaleClient}, goldenCase{name: "status_human/mcp_not_configured_exit1", human: true, args: []string{"status"}, server: gStatusAPI(true, 200, gWorkersOK), setup: gStatusClient(false)}, goldenCase{name: "status_human/key_invalid_exit3", human: true, args: []string{"status"}, server: gStatusAPI(true, 401, gWorkers)}, goldenCase{name: "status_human/no_key_exit3", human: true, args: []string{"status"}, noKey: true, server: gStatusAPI(true, 200, gWorkers)}, diff --git a/internal/cmd/status.go b/internal/cmd/status.go index 1ee0584..82a9b7e 100644 --- a/internal/cmd/status.go +++ b/internal/cmd/status.go @@ -54,8 +54,11 @@ type statusMCP struct { // NeedsEnv has a server whose key is $SPICRAWL_API_KEY, which is not set // here: the client cannot authenticate. NeedsEnv []string `json:"needs_env"` + // Stale has a server with a key written into it that is not the key in use + // (compared by fingerprint; neither is ever printed). + Stale []string `json:"stale"` - needsEnvAt map[string]string // client id -> where that entry is: "user" or "project" + entryAt map[string]string // client id -> where its NeedsEnv / Stale entry is: "user" or "project" } // statusRepair is a command that fixes a failed check. UnattendedSafe says an @@ -83,7 +86,7 @@ It also looks for agent clients in this directory and your home directory configured and able to authenticate; with no client found that check is skipped. A project-level entry shadows a user-level one, and an entry whose key is $SPICRAWL_API_KEY ("needs_env" in --json) does not count while that variable -is unset here. +is unset here, nor does one that holds another key than the one in use ("stale"). For every failed check it prints a repair command, marked unattended_safe true (an agent may run it) or false (the user must act, for example supply an @@ -107,7 +110,7 @@ worker kind has none ready, no MCP server configured for a client) exits 1; read } ctx := cmd.Context() c := api.New(r.BaseURL, r.APIKey, flagTimeout) - rep := statusReport{BaseURL: r.BaseURL, KeySource: r.APIKeySource, MCP: statusMCPCheck(), Repairs: []statusRepair{}} + rep := statusReport{BaseURL: r.BaseURL, KeySource: r.APIKeySource, MCP: statusMCPCheck(r.APIKey), Repairs: []statusRepair{}} p := Printer() sp := spin(p, "Checking the API") @@ -300,8 +303,8 @@ func cliInvocation() string { // server that can authenticate. A project entry shadows the user-level one of // the same name. An entry that takes the key from $SPICRAWL_API_KEY works only // while that is set, which is taken from this process's environment. -func statusMCPCheck() statusMCP { - m := statusMCP{Detected: []string{}, Connected: []string{}, NeedsEnv: []string{}, needsEnvAt: map[string]string{}} +func statusMCPCheck(key string) statusMCP { + m := statusMCP{Detected: []string{}, Connected: []string{}, NeedsEnv: []string{}, Stale: []string{}, entryAt: map[string]string{}} s, err := agentResolveScope("", false) if err != nil { return m @@ -320,7 +323,10 @@ func statusMCPCheck() statusMCP { case !ok: case mcpEntryNeedsEnv(entry) && !envSet: m.NeedsEnv = append(m.NeedsEnv, c.ID) - m.needsEnvAt[c.ID] = where + m.entryAt[c.ID] = where + case mcpEntryStale(entry, key): + m.Stale = append(m.Stale, c.ID) + m.entryAt[c.ID] = where default: m.Connected = append(m.Connected, c.ID) } @@ -332,7 +338,7 @@ func statusMCPCheck() statusMCP { func (m statusMCP) missing() []string { var out []string for _, id := range m.Detected { - if !slices.Contains(m.Connected, id) && !slices.Contains(m.NeedsEnv, id) { + if !slices.Contains(m.Connected, id) && !slices.Contains(m.NeedsEnv, id) && !slices.Contains(m.Stale, id) { out = append(out, id) } } @@ -361,6 +367,9 @@ func statusMCPText(m statusMCP) string { if len(m.NeedsEnv) > 0 { parts = append(parts, "needs $"+config.EnvAPIKey+", which is not set here: "+strings.Join(clientNames(m.NeedsEnv), ", ")) } + if len(m.Stale) > 0 { + parts = append(parts, "has a different key than the one in use: "+strings.Join(clientNames(m.Stale), ", ")) + } if miss := m.missing(); len(miss) > 0 { parts = append(parts, "not configured: "+strings.Join(clientNames(miss), ", ")) } @@ -422,32 +431,42 @@ func statusRepairs(rep *statusReport) []statusRepair { add("mcp", cli+" init --all --yes", true, "no Spicrawl MCP server is configured for "+strings.Join(clientNames(missing), ", ")+"; this writes the MCP entry and the skill for every client found here") } - // An entry that reads $SPICRAWL_API_KEY, unset here, is rewritten with the - // key saved in the CLI config (init embeds it); without a saved key the - // user has to supply one. + // An entry that cannot authenticate (it reads $SPICRAWL_API_KEY, unset here, + // or holds another key than the one in use) is rewritten by init, which + // replaces a stale token without --force. With a key saved in the CLI config + // init embeds that; otherwise the user has to supply one. Entries are + // rewritten where they are: a project entry shadows a user-level one. saved := rep.KeySource == "file" - for _, where := range []string{"user", "project"} { - var ids []string - for _, id := range rep.MCP.NeedsEnv { - if rep.MCP.needsEnvAt[id] == where { - ids = append(ids, id) + for _, state := range []struct { + ids []string + why string + }{ + {rep.MCP.NeedsEnv, " reads the key from $" + config.EnvAPIKey + ", which is not set here, so the MCP server cannot authenticate; "}, + {rep.MCP.Stale, " holds a different key than the one in use, so the MCP server may reject it; "}, + } { + for _, where := range []string{"user", "project"} { + var ids []string + for _, id := range state.ids { + if rep.MCP.entryAt[id] == where { + ids = append(ids, id) + } } + if len(ids) == 0 { + continue + } + command := cli + " init --client " + strings.Join(ids, ",") + " --yes" + if where == "project" { + command += " --global=false" + } + why := "The MCP config for " + strings.Join(clientNames(ids), ", ") + state.why + if saved { + why += "this rewrites the entry with the key saved in the CLI config (the old file is kept as .bak)" + } else { + command += " --api-key " + why += "this checks your key, saves it and writes it into the entry (the old file is kept as .bak)" + } + add("mcp", command, saved, why) } - if len(ids) == 0 { - continue - } - command := cli + " init --client " + strings.Join(ids, ",") + " --yes --force" - if where == "project" { - command += " --global=false" - } - why := strings.Join(clientNames(ids), ", ") + " read the key from $" + config.EnvAPIKey + ", which is not set here, so the MCP server cannot authenticate; " - if saved { - why += "this rewrites the entry with the key saved in the CLI config (the old file is kept as .bak)" - } else { - command += " --api-key " - why += "this checks your key, saves it and writes it into the entry (the old file is kept as .bak)" - } - add("mcp", command, saved, why) } return out } diff --git a/internal/cmd/testdata/golden/agents/init_all_yes_api_key_project.file..gitignore b/internal/cmd/testdata/golden/agents/init_all_yes_api_key_project.file..gitignore index 288c9cd..ffe896e 100644 --- a/internal/cmd/testdata/golden/agents/init_all_yes_api_key_project.file..gitignore +++ b/internal/cmd/testdata/golden/agents/init_all_yes_api_key_project.file..gitignore @@ -1,3 +1,5 @@ node_modules/ .mcp.json +.mcp.json.bak .cursor/mcp.json +.cursor/mcp.json.bak diff --git a/internal/cmd/testdata/golden/agents/init_all_yes_api_key_project.stdout b/internal/cmd/testdata/golden/agents/init_all_yes_api_key_project.stdout index c2015ae..488b2c2 100644 --- a/internal/cmd/testdata/golden/agents/init_all_yes_api_key_project.stdout +++ b/internal/cmd/testdata/golden/agents/init_all_yes_api_key_project.stdout @@ -33,6 +33,6 @@ "kind": "gitignore", "file": "/.gitignore", "action": "updated", - "reason": "keeps .mcp.json, .cursor/mcp.json (holds the API key) out of git" + "reason": "keeps .mcp.json, .mcp.json.bak, .cursor/mcp.json, .cursor/mcp.json.bak (holds the API key) out of git" } ] diff --git a/internal/cmd/testdata/golden/agents_human/init_all_yes_api_key_project.stdout b/internal/cmd/testdata/golden/agents_human/init_all_yes_api_key_project.stdout index f23bdaa..4b71dba 100644 --- a/internal/cmd/testdata/golden/agents_human/init_all_yes_api_key_project.stdout +++ b/internal/cmd/testdata/golden/agents_human/init_all_yes_api_key_project.stdout @@ -3,4 +3,4 @@ claude mcp created .mcp.json contains the liter cursor mcp created .cursor/mcp.json contains the literal API key claude skill created .claude/skills/spicrawl/SKILL.md cursor skill skipped .claude/skills/spicrawl/SKILL.md already reads this skill - gitignore updated .gitignore keeps .mcp.json, .cursor/mcp.json (holds the API key) out of git + gitignore updated .gitignore keeps .mcp.json, .mcp.json.bak, .cursor/mcp.json, .cursor/mcp.json.bak (holds the API key) out of git diff --git a/internal/cmd/testdata/golden/status/key_invalid_exit3.stdout b/internal/cmd/testdata/golden/status/key_invalid_exit3.stdout index d39f22e..a209c9a 100644 --- a/internal/cmd/testdata/golden/status/key_invalid_exit3.stdout +++ b/internal/cmd/testdata/golden/status/key_invalid_exit3.stdout @@ -14,7 +14,8 @@ "mcp": { "detected": [], "connected": [], - "needs_env": [] + "needs_env": [], + "stale": [] }, "repairs": [ { diff --git a/internal/cmd/testdata/golden/status/key_without_read_scope.stdout b/internal/cmd/testdata/golden/status/key_without_read_scope.stdout index b853e74..78c18cc 100644 --- a/internal/cmd/testdata/golden/status/key_without_read_scope.stdout +++ b/internal/cmd/testdata/golden/status/key_without_read_scope.stdout @@ -27,7 +27,8 @@ "mcp": { "detected": [], "connected": [], - "needs_env": [] + "needs_env": [], + "stale": [] }, "repairs": [] } diff --git a/internal/cmd/testdata/golden/status/mcp_connected.stdout b/internal/cmd/testdata/golden/status/mcp_connected.stdout index 843c2e0..cfc9a0a 100644 --- a/internal/cmd/testdata/golden/status/mcp_connected.stdout +++ b/internal/cmd/testdata/golden/status/mcp_connected.stdout @@ -30,7 +30,8 @@ "connected": [ "claude" ], - "needs_env": [] + "needs_env": [], + "stale": [] }, "repairs": [] } diff --git a/internal/cmd/testdata/golden/status/mcp_needs_env_exit1.stdout b/internal/cmd/testdata/golden/status/mcp_needs_env_exit1.stdout index 94d6b7a..046465f 100644 --- a/internal/cmd/testdata/golden/status/mcp_needs_env_exit1.stdout +++ b/internal/cmd/testdata/golden/status/mcp_needs_env_exit1.stdout @@ -30,14 +30,15 @@ "connected": [], "needs_env": [ "claude" - ] + ], + "stale": [] }, "repairs": [ { "check": "mcp", - "command": "spicrawl init --client claude --yes --force", + "command": "spicrawl init --client claude --yes", "unattended_safe": true, - "why": "Claude Code read the key from $SPICRAWL_API_KEY, which is not set here, so the MCP server cannot authenticate; this rewrites the entry with the key saved in the CLI config (the old file is kept as .bak)" + "why": "The MCP config for Claude Code reads the key from $SPICRAWL_API_KEY, which is not set here, so the MCP server cannot authenticate; this rewrites the entry with the key saved in the CLI config (the old file is kept as .bak)" } ] } diff --git a/internal/cmd/testdata/golden/status/mcp_not_configured_exit1.stdout b/internal/cmd/testdata/golden/status/mcp_not_configured_exit1.stdout index 6cdfcc2..1d14e43 100644 --- a/internal/cmd/testdata/golden/status/mcp_not_configured_exit1.stdout +++ b/internal/cmd/testdata/golden/status/mcp_not_configured_exit1.stdout @@ -28,7 +28,8 @@ "claude" ], "connected": [], - "needs_env": [] + "needs_env": [], + "stale": [] }, "repairs": [ { diff --git a/internal/cmd/testdata/golden/status/mcp_stale_key_exit1.exit b/internal/cmd/testdata/golden/status/mcp_stale_key_exit1.exit new file mode 100644 index 0000000..d00491f --- /dev/null +++ b/internal/cmd/testdata/golden/status/mcp_stale_key_exit1.exit @@ -0,0 +1 @@ +1 diff --git a/internal/cmd/testdata/golden/status/mcp_stale_key_exit1.stdout b/internal/cmd/testdata/golden/status/mcp_stale_key_exit1.stdout new file mode 100644 index 0000000..941a667 --- /dev/null +++ b/internal/cmd/testdata/golden/status/mcp_stale_key_exit1.stdout @@ -0,0 +1,44 @@ +{ + "api_reachable": true, + "ready": true, + "checks": { + "postgres": "ok", + "redis": "ok" + }, + "cdp": "enabled", + "key_valid": true, + "key_source": "file", + "base_url": "http://SERVER", + "workers": { + "batch": { + "present": 2, + "ready": 2 + }, + "browser": { + "present": 3, + "ready": 3 + }, + "egress": { + "present": 1, + "ready": 1 + } + }, + "mcp": { + "detected": [ + "claude" + ], + "connected": [], + "needs_env": [], + "stale": [ + "claude" + ] + }, + "repairs": [ + { + "check": "mcp", + "command": "spicrawl init --client claude --yes", + "unattended_safe": true, + "why": "The MCP config for Claude Code holds a different key than the one in use, so the MCP server may reject it; this rewrites the entry with the key saved in the CLI config (the old file is kept as .bak)" + } + ] +} diff --git a/internal/cmd/testdata/golden/status/no_key_exit3.stdout b/internal/cmd/testdata/golden/status/no_key_exit3.stdout index 04390ee..f52b2d4 100644 --- a/internal/cmd/testdata/golden/status/no_key_exit3.stdout +++ b/internal/cmd/testdata/golden/status/no_key_exit3.stdout @@ -14,7 +14,8 @@ "mcp": { "detected": [], "connected": [], - "needs_env": [] + "needs_env": [], + "stale": [] }, "repairs": [ { diff --git a/internal/cmd/testdata/golden/status/not_ready.stdout b/internal/cmd/testdata/golden/status/not_ready.stdout index a521bd1..e2748db 100644 --- a/internal/cmd/testdata/golden/status/not_ready.stdout +++ b/internal/cmd/testdata/golden/status/not_ready.stdout @@ -26,7 +26,8 @@ "mcp": { "detected": [], "connected": [], - "needs_env": [] + "needs_env": [], + "stale": [] }, "repairs": [ { diff --git a/internal/cmd/testdata/golden/status/ok.stdout b/internal/cmd/testdata/golden/status/ok.stdout index 1933e83..0bc5b1b 100644 --- a/internal/cmd/testdata/golden/status/ok.stdout +++ b/internal/cmd/testdata/golden/status/ok.stdout @@ -26,7 +26,8 @@ "mcp": { "detected": [], "connected": [], - "needs_env": [] + "needs_env": [], + "stale": [] }, "repairs": [] } diff --git a/internal/cmd/testdata/golden/status/unreachable_exit10.stdout b/internal/cmd/testdata/golden/status/unreachable_exit10.stdout index b489b0b..8482251 100644 --- a/internal/cmd/testdata/golden/status/unreachable_exit10.stdout +++ b/internal/cmd/testdata/golden/status/unreachable_exit10.stdout @@ -9,7 +9,8 @@ "mcp": { "detected": [], "connected": [], - "needs_env": [] + "needs_env": [], + "stale": [] }, "repairs": [ { diff --git a/internal/cmd/testdata/golden/status/worker_kind_down_exit1.stdout b/internal/cmd/testdata/golden/status/worker_kind_down_exit1.stdout index 5226ede..f08eee1 100644 --- a/internal/cmd/testdata/golden/status/worker_kind_down_exit1.stdout +++ b/internal/cmd/testdata/golden/status/worker_kind_down_exit1.stdout @@ -26,7 +26,8 @@ "mcp": { "detected": [], "connected": [], - "needs_env": [] + "needs_env": [], + "stale": [] }, "repairs": [ { diff --git a/internal/cmd/testdata/golden/status/workers_error.stdout b/internal/cmd/testdata/golden/status/workers_error.stdout index 64376b4..ab33579 100644 --- a/internal/cmd/testdata/golden/status/workers_error.stdout +++ b/internal/cmd/testdata/golden/status/workers_error.stdout @@ -14,7 +14,8 @@ "mcp": { "detected": [], "connected": [], - "needs_env": [] + "needs_env": [], + "stale": [] }, "repairs": [] } diff --git a/internal/cmd/testdata/golden/status_human/mcp_needs_env_exit1.stdout b/internal/cmd/testdata/golden/status_human/mcp_needs_env_exit1.stdout index df6094b..3045b72 100644 --- a/internal/cmd/testdata/golden/status_human/mcp_needs_env_exit1.stdout +++ b/internal/cmd/testdata/golden/status_human/mcp_needs_env_exit1.stdout @@ -10,6 +10,6 @@ workers: batch 2 ready of 2 repairs: - check: mcp - command: spicrawl init --client claude --yes --force + command: spicrawl init --client claude --yes unattended_safe: true - why: Claude Code read the key from $SPICRAWL_API_KEY, which is not set here, so the MCP server cannot authenticate; this rewrites the entry with the key saved in the CLI config (the old file is kept as .bak) + why: The MCP config for Claude Code reads the key from $SPICRAWL_API_KEY, which is not set here, so the MCP server cannot authenticate; this rewrites the entry with the key saved in the CLI config (the old file is kept as .bak) diff --git a/internal/cmd/testdata/golden/status_human/mcp_stale_key_exit1.exit b/internal/cmd/testdata/golden/status_human/mcp_stale_key_exit1.exit new file mode 100644 index 0000000..d00491f --- /dev/null +++ b/internal/cmd/testdata/golden/status_human/mcp_stale_key_exit1.exit @@ -0,0 +1 @@ +1 diff --git a/internal/cmd/testdata/golden/status_human/mcp_stale_key_exit1.stdout b/internal/cmd/testdata/golden/status_human/mcp_stale_key_exit1.stdout new file mode 100644 index 0000000..64ceb2f --- /dev/null +++ b/internal/cmd/testdata/golden/status_human/mcp_stale_key_exit1.stdout @@ -0,0 +1,15 @@ +base url: http://SERVER +reachable: yes +ready: yes +cdp: enabled +key: valid (from file) +mcp: has a different key than the one in use: Claude Code +workers: + browser 3 ready of 3 + egress 1 ready of 1 + batch 2 ready of 2 +repairs: + - check: mcp + command: spicrawl init --client claude --yes + unattended_safe: true + why: The MCP config for Claude Code holds a different key than the one in use, so the MCP server may reject it; this rewrites the entry with the key saved in the CLI config (the old file is kept as .bak) diff --git a/npm/bin/spicrawl.js b/npm/bin/spicrawl.js index bc4d9cb..00dec6a 100755 --- a/npm/bin/spicrawl.js +++ b/npm/bin/spicrawl.js @@ -7,8 +7,7 @@ // runs it with the same argv, stdio and exit code. There is no postinstall // step and nothing is downloaded at install time. // -// SPICRAWL_BINARY=/path/to/spicrawl overrides the lookup (SPICRAWL_BINARY is -// still honoured as a fallback). +// SPICRAWL_BINARY=/path/to/spicrawl overrides the lookup. "use strict"; const { spawn } = require("child_process"); @@ -19,7 +18,6 @@ const INSTALL_DOCS = "https://docs.spicrawl.com/cli/install"; function binaryPath() { if (process.env.SPICRAWL_BINARY) return process.env.SPICRAWL_BINARY; - if (process.env.SPICRAWL_BINARY) return process.env.SPICRAWL_BINARY; const target = `${process.platform}-${process.arch}`; const exe = process.platform === "win32" ? "spicrawl.exe" : "spicrawl";