diff --git a/.claude-plugin/marketplace.json b/.claude-plugin/marketplace.json new file mode 100644 index 0000000..c9aa470 --- /dev/null +++ b/.claude-plugin/marketplace.json @@ -0,0 +1,21 @@ +{ + "name": "context-guru", + "description": "Run context-guru in front of your Claude Code sessions: install once, route per repo.", + "owner": { + "name": "rossoctl", + "url": "https://github.com/rossoctl/context-guru" + }, + "plugins": [ + { + "name": "context-guru", + "source": "./context-guru-plugin", + "displayName": "context-guru", + "description": "Install, route, inspect and remove a local context-guru proxy for Claude Code. Recovers prompt-cache misses on long sessions; no API key needed on a Pro/Max subscription.", + "homepage": "https://github.com/rossoctl/context-guru", + "repository": "https://github.com/rossoctl/context-guru", + "license": "Apache-2.0", + "keywords": ["cache", "cost", "proxy", "tokens", "prompt-caching"], + "category": "productivity" + } + ] +} diff --git a/README.md b/README.md index 582ae3a..cf33f93 100644 --- a/README.md +++ b/README.md @@ -108,7 +108,25 @@ docker build -t context-guru:local . ## Quickstart (60 seconds) -Download a release binary — statically linked, **no Go and no C compiler needed** — or build +**Claude Code users — install once per machine, route once per repo; no toolchain, and no API key +needed on a Pro/Max subscription** ([details](docs/how-to/install-plugin.md)): + +``` +/plugin marketplace add rossoctl/context-guru +/plugin install context-guru@context-guru +/reload-plugins +/context-guru:install +``` + +`/reload-plugins` is what makes the `/context-guru:*` skills exist in this session; without it the +last line answers `Unknown command`. A new session does the same thing. + +That installs a statically-linked binary (no Go, no C compiler), routes **this project only** by +default, starts the proxy on demand and lets it exit when idle. `/context-guru:uninstall` undoes it, +restoring any base URL it replaced. The plugin installs with `--preset cache` — the prompt-cache +split and nothing else. (The proxy's own default is `house`; `--preset` is how you change it.) + +Or by hand — a release binary is statically linked, **no Go and no C compiler needed** — or build from source: ```sh diff --git a/context-guru-plugin/.claude-plugin/plugin.json b/context-guru-plugin/.claude-plugin/plugin.json new file mode 100644 index 0000000..3531302 --- /dev/null +++ b/context-guru-plugin/.claude-plugin/plugin.json @@ -0,0 +1,48 @@ +{ + "name": "context-guru", + "displayName": "context-guru", + "description": "Installs a local context-guru proxy, routes this project's Claude Code sessions through it, and reports what it saved. Reversible: /context-guru:uninstall removes the one settings key it adds and stops the proxy.", + "version": "0.1.0", + "author": { + "name": "rossoctl", + "url": "https://github.com/rossoctl/context-guru" + }, + "homepage": "https://github.com/rossoctl/context-guru", + "repository": "https://github.com/rossoctl/context-guru", + "license": "Apache-2.0", + "keywords": [ + "cache", + "cost", + "proxy", + "tokens", + "prompt-caching" + ], + "userConfig": { + "port": { + "type": "number", + "title": "Proxy port", + "description": "Local port the proxy listens on. The port must be FIXED rather than negotiated: the ANTHROPIC_BASE_URL written into your settings and the session hook that starts the proxy have to agree, and a URL cannot be renegotiated after it is written. Default 8787 \u2014 deliberately not 4000, which collides with litellm.", + "default": 8787, + "min": 1024, + "max": 65535 + }, + "preset": { + "type": "string", + "title": "Preset", + "description": "Compaction pipeline. `cache` (the default) runs the prompt-cache split and NOTHING else: no content dropped, no markers, no extra tool, no model calls. `codesmart` adds the offloaders once you want them.", + "default": "cache" + }, + "idle_exit": { + "type": "string", + "title": "Idle exit", + "description": "Exit the proxy after this long with no requests and no keep-alive ping pending, so nothing is left running on your machine. Must be at least 2x the store's entry lifetime (~5h34m at the default), because exiting clears in-memory cache state.", + "default": "24h" + }, + "upstream": { + "title": "Upstream base URL", + "type": "string", + "default": "", + "description": "Forward to this Anthropic-compatible base URL instead of api.anthropic.com. Set it when something else is already the gateway \u2014 a hosted agent pod, a corporate gateway \u2014 so the proxy CHAINS behind it rather than replacing it. That gateway keeps holding the credential and, on at least one pod, rewriting model names: bypassing it sends model ids the real API has never heard of and every request fails. Empty means go straight to Anthropic." + } + } +} diff --git a/context-guru-plugin/hooks/hooks.json b/context-guru-plugin/hooks/hooks.json new file mode 100644 index 0000000..ca94fe2 --- /dev/null +++ b/context-guru-plugin/hooks/hooks.json @@ -0,0 +1,26 @@ +{ + "hooks": { + "UserPromptSubmit": [ + { + "hooks": [ + { + "type": "command", + "command": "\"${CLAUDE_PLUGIN_ROOT}\"/scripts/check-proxy.sh", + "timeout": 30 + } + ] + } + ], + "SessionStart": [ + { + "hooks": [ + { + "type": "command", + "command": "\"${CLAUDE_PLUGIN_ROOT}\"/scripts/start-proxy.sh", + "timeout": 60 + } + ] + } + ] + } +} diff --git a/context-guru-plugin/plugin_test.go b/context-guru-plugin/plugin_test.go new file mode 100644 index 0000000..f7a0652 --- /dev/null +++ b/context-guru-plugin/plugin_test.go @@ -0,0 +1,1750 @@ +// Package plugin holds tests for the Claude Code plugin's shell/Python helpers. +// +// The scripts are not Go, but their failure modes are the most expensive in this repo: they +// edit the user's real settings.json, and the SessionStart hook runs in EVERY project the user +// has. A regression here does not degrade compaction — it breaks Claude Code on a stranger's +// machine, in projects that have nothing to do with context-guru. So they are tested from Go, +// where `go test ./...` and CI already look. +package plugin + +import ( + "encoding/json" + "fmt" + "net" + "net/http" + "os" + "os/exec" + "path/filepath" + "runtime" + "strconv" + "strings" + "syscall" + "testing" + "time" +) + +func scriptsDir(t *testing.T) string { + t.Helper() + abs, err := filepath.Abs("scripts") + if err != nil { + t.Fatal(err) + } + if _, err := os.Stat(abs); err != nil { + t.Fatalf("plugin scripts missing: %v", err) + } + return abs +} + +// requireTool fails rather than skips for the two interpreters these tests are built on. +// +// A skip and a pass are indistinguishable in CI output, and this whole package exists because +// these scripts warrant coverage their blast radius demands — so on a runner without python3 or +// bash, EVERY test in this file used to skip and `go test` was green. ubuntu-latest has both, so +// an absence means the image changed, and that is something to hear about rather than sail past. +// Genuinely optional tools keep using t.Skipf via requireOptionalTool. +func requireTool(t *testing.T, name string) string { + t.Helper() + p, err := exec.LookPath(name) + if err != nil { + switch name { + case "python3", "bash": + t.Fatalf("%s is required to test the plugin scripts and is not on PATH: %v", name, err) + default: + t.Skipf("%s not available: %v", name, err) + } + } + return p +} + +// settings runs settings.py and returns its key=value output as a map, plus the exit code. +func settings(t *testing.T, args ...string) (map[string]string, int) { + t.Helper() + py := requireTool(t, "python3") + cmd := exec.Command(py, append([]string{filepath.Join(scriptsDir(t), "settings.py")}, args...)...) + out, err := cmd.CombinedOutput() + code := 0 + if ee, ok := err.(*exec.ExitError); ok { + code = ee.ExitCode() + } else if err != nil { + t.Fatalf("running settings.py: %v (%s)", err, out) + } + facts := map[string]string{} + for _, line := range strings.Split(string(out), "\n") { + if k, v, ok := strings.Cut(strings.TrimSpace(line), "="); ok { + facts[k] = v + } + } + t.Logf("settings.py %v -> exit %d, %v", args, code, facts) + return facts, code +} + +func writeJSON(t *testing.T, path string, v any) { + t.Helper() + b, err := json.MarshalIndent(v, "", " ") + if err != nil { + t.Fatal(err) + } + if err := os.WriteFile(path, b, 0o644); err != nil { + t.Fatal(err) + } +} + +func readJSON(t *testing.T, path string) map[string]any { + t.Helper() + b, err := os.ReadFile(path) + if err != nil { + t.Fatal(err) + } + var m map[string]any + if err := json.Unmarshal(b, &m); err != nil { + t.Fatalf("settings file is not valid JSON after the edit: %v\n%s", err, b) + } + return m +} + +const ourURL = "http://127.0.0.1:8787/anthropic" + +// TestSettingsAddPreservesEverythingElse is the whole reason a script does this rather than a +// one-line `jq`: the target is a file the user depends on, holding their theme, model, +// permission rules and their own env vars. Exactly one key may appear, and nothing may be lost. +func TestSettingsAddPreservesEverythingElse(t *testing.T) { + dir := t.TempDir() + path := filepath.Join(dir, "settings.json") + writeJSON(t, path, map[string]any{ + "theme": "dark", + "model": "opus", + "env": map[string]any{ + "SOME_OTHER_VAR": "keep me", + "ANTHROPIC_SMALL_FAST": "also keep me", + }, + "permissions": map[string]any{"allow": []string{"Bash(ls:*)"}}, + }) + + facts, code := settings(t, "add", "--file", path, "--url", ourURL) + if code != 0 || facts["result"] != "added" { + t.Fatalf("add failed: exit %d, %v", code, facts) + } + if facts["backup"] == "" || facts["backup"] == "(new file)" { + t.Errorf("no backup was taken of an existing settings file: %v", facts) + } else if _, err := os.Stat(facts["backup"]); err != nil { + t.Errorf("reported backup %q does not exist: %v", facts["backup"], err) + } + + got := readJSON(t, path) + if got["theme"] != "dark" || got["model"] != "opus" { + t.Errorf("top-level settings were lost: %v", got) + } + if got["permissions"] == nil { + t.Error("permissions block was lost") + } + env, _ := got["env"].(map[string]any) + if env["ANTHROPIC_BASE_URL"] != ourURL { + t.Errorf("env.ANTHROPIC_BASE_URL = %v, want %q", env["ANTHROPIC_BASE_URL"], ourURL) + } + if env["SOME_OTHER_VAR"] != "keep me" || env["ANTHROPIC_SMALL_FAST"] != "also keep me" { + t.Errorf("the user's own env vars were lost: %v", env) + } + if len(env) != 3 { + t.Errorf("env has %d keys, want the 2 originals plus ours: %v", len(env), env) + } +} + +// TestSettingsAddRefusesToStealAnExistingBaseURL covers the one conflict the install has to +// reason about. A base URL already in the file may be the user's company gateway or a benchmark +// endpoint; taking it over would break their setup while reporting success. +func TestSettingsAddRefusesToStealAnExistingBaseURL(t *testing.T) { + dir := t.TempDir() + path := filepath.Join(dir, "settings.json") + theirs := "https://gateway.corp.example/anthropic" + writeJSON(t, path, map[string]any{"env": map[string]any{"ANTHROPIC_BASE_URL": theirs}}) + + facts, code := settings(t, "add", "--file", path, "--url", ourURL) + if code != 2 || facts["result"] != "conflict" { + t.Fatalf("expected a conflict (exit 2), got exit %d, %v", code, facts) + } + if facts["existing"] != theirs { + t.Errorf("conflict did not report the existing value: %v", facts) + } + if env := readJSON(t, path)["env"].(map[string]any); env["ANTHROPIC_BASE_URL"] != theirs { + t.Fatalf("the file was modified despite the conflict: %v", env) + } + + // --force is the user's explicit decision, and it must report what it replaced so the old + // value is recoverable from the transcript as well as the backup. + facts, code = settings(t, "add", "--file", path, "--url", ourURL, "--force") + if code != 0 || facts["result"] != "added" || facts["replaced"] != theirs { + t.Fatalf("--force did not replace and report: exit %d, %v", code, facts) + } + + // Re-adding the same URL is a no-op, so a re-run of the install skill is free. + facts, code = settings(t, "add", "--file", path, "--url", ourURL) + if code != 0 || facts["result"] != "unchanged" { + t.Fatalf("re-adding the same URL should be unchanged: exit %d, %v", code, facts) + } +} + +// TestSettingsRemoveTakesOnlyOurKey: uninstall must be exact. It removes our base URL and +// nothing else, refuses to remove one that is not ours, and leaves no empty `env: {}` behind. +func TestSettingsRemoveTakesOnlyOurKey(t *testing.T) { + dir := t.TempDir() + + // (a) our key alongside the user's own env vars. + path := filepath.Join(dir, "a.json") + writeJSON(t, path, map[string]any{"theme": "dark", "env": map[string]any{ + "ANTHROPIC_BASE_URL": ourURL, "KEEP": "yes"}}) + facts, code := settings(t, "remove", "--file", path, "--url", ourURL) + if code != 0 || facts["result"] != "removed" { + t.Fatalf("remove failed: exit %d, %v", code, facts) + } + got := readJSON(t, path) + env, _ := got["env"].(map[string]any) + if _, still := env["ANTHROPIC_BASE_URL"]; still { + t.Error("the key survived removal") + } + if env["KEEP"] != "yes" || got["theme"] != "dark" { + t.Errorf("removal took more than its own key: %v", got) + } + + // (b) our key alone: the env block we created goes with it, leaving no litter. + path = filepath.Join(dir, "b.json") + writeJSON(t, path, map[string]any{"theme": "dark", "env": map[string]any{ + "ANTHROPIC_BASE_URL": ourURL}}) + if _, code := settings(t, "remove", "--file", path, "--url", ourURL); code != 0 { + t.Fatalf("remove exit %d", code) + } + if got := readJSON(t, path); got["env"] != nil { + t.Errorf("an empty env block was left behind: %v", got) + } + + // (c) a base URL that is NOT ours must survive an uninstall untouched. + path = filepath.Join(dir, "c.json") + theirs := "http://127.0.0.1:4000/anthropic" // e.g. litellm + writeJSON(t, path, map[string]any{"env": map[string]any{"ANTHROPIC_BASE_URL": theirs}}) + facts, code = settings(t, "remove", "--file", path, "--url", ourURL) + if code != 2 || facts["result"] != "conflict" { + t.Fatalf("uninstall must not remove a base URL it did not install: exit %d, %v", code, facts) + } + if env := readJSON(t, path)["env"].(map[string]any); env["ANTHROPIC_BASE_URL"] != theirs { + t.Fatalf("someone else's base URL was removed: %v", env) + } +} + +// TestSettingsRefusesToRewriteABrokenFile: if the file will not parse, the only safe move is to +// stop. Treating it as empty and writing a fresh one would discard every setting in it. +func TestSettingsRefusesToRewriteABrokenFile(t *testing.T) { + dir := t.TempDir() + path := filepath.Join(dir, "settings.json") + broken := "{\n \"theme\": \"dark\",,,\n}\n" + if err := os.WriteFile(path, []byte(broken), 0o644); err != nil { + t.Fatal(err) + } + facts, code := settings(t, "add", "--file", path, "--url", ourURL) + if code != 3 || facts["reason"] != "unparseable_json" { + t.Fatalf("expected a refusal on unparseable JSON, got exit %d, %v", code, facts) + } + b, _ := os.ReadFile(path) + if string(b) != broken { + t.Fatalf("the broken file was modified:\n%s", b) + } +} + +// --- the SessionStart hook ----------------------------------------------------------------- + +// runStart runs start-proxy.sh with a controlled environment and returns its output. +// +// CONTEXT_GURU_BIN points at a sentinel script: if the hook decides to launch a proxy, the +// sentinel file appears. That is how "did it start something?" is asserted, rather than by +// looking for a process. +func runStart(t *testing.T, env map[string]string) (out string, code int, startedSentinel string) { + t.Helper() + requireTool(t, "bash") + dir := t.TempDir() + sentinel := filepath.Join(dir, "started") + fake := filepath.Join(dir, "fake-proxy") + if err := os.WriteFile(fake, []byte("#!/usr/bin/env bash\ntouch \""+sentinel+"\"\nsleep 30\n"), 0o755); err != nil { + t.Fatal(err) + } + + cmd := exec.Command("bash", filepath.Join(scriptsDir(t), "start-proxy.sh")) + cmd.Env = append(os.Environ(), "CONTEXT_GURU_BIN="+fake, "TMPDIR="+dir) + for k, v := range env { + cmd.Env = append(cmd.Env, k+"="+v) + } + b, err := cmd.CombinedOutput() + if ee, ok := err.(*exec.ExitError); ok { + code = ee.ExitCode() + } else if err != nil { + t.Fatalf("running start-proxy.sh: %v (%s)", err, b) + } + t.Logf("start-proxy.sh env=%v -> exit %d, output:\n%s", env, code, b) + return string(b), code, sentinel +} + +// TestHookIsSilentAndInertWhereRoutingIsNotConfigured is the property that makes a user-scope +// plugin acceptable at all. +// +// The plugin installs globally, so this hook runs on EVERY session in EVERY project — including +// all the ones the user never routed. In those it must do nothing and say nothing: starting a +// proxy would be waste, and printing anything would put context-guru noise in sessions that have +// nothing to do with it. It also must not hijack a user who routes to a different local proxy on +// another port, which is why the gate matches the port and not merely "localhost". +func TestHookIsSilentAndInertWhereRoutingIsNotConfigured(t *testing.T) { + // The last row is a POSITIVE CONTROL and it is not decoration. + // + // Every other assertion here is an absence, so without it this test cannot distinguish "the + // gate declined" from "the script exited before reaching the gate" — gut start-proxy.sh to a + // bare `exit 0` and every silent row still passes. The control did exist, in + // TestHookStartsTheProxyAndWaitsForHealthz, but a control in a different test is one that a + // later change can narrow or skip without anything here failing. Keep it local to the test + // whose meaning depends on it. + for _, c := range []struct { + name, baseURL string + routed bool + }{ + {name: "unset"}, + {name: "another local proxy on a different port (e.g. litellm)", baseURL: "http://localhost:4000/anthropic"}, + {name: "a remote gateway", baseURL: "https://gateway.corp.example/anthropic"}, + {name: "our port number appearing in a REMOTE host", baseURL: "https://8787.example.com/anthropic"}, + // The gate matched on the port as a PREFIX, so 8787 also matched 87871 — and this hook + // would start our proxy on 8787 under a user routed to a different local proxy there. + {name: "our port as a PREFIX of a longer port", baseURL: "http://127.0.0.1:87871/anthropic"}, + {name: "POSITIVE CONTROL: a routed project, where it must act", routed: true}, + } { + t.Run(c.name, func(t *testing.T) { + port := "8787" + baseURL := c.baseURL + if c.routed { + // A port of our own, so this never probes or starts anything on a developer's + // real 8787, and a short budget because the stand-in never answers /healthz. + port = freePort(t) + baseURL = "http://127.0.0.1:" + port + "/anthropic" + } + env := map[string]string{ + "CLAUDE_PLUGIN_OPTION_PORT": port, + "ANTHROPIC_BASE_URL": baseURL, + "CONTEXT_GURU_HEALTH_BUDGET": "1", + "XDG_STATE_HOME": t.TempDir(), + } + out, code, sentinel := runStart(t, env) + if code != 0 { + t.Errorf("exit %d; the hook must never fail a session", code) + } + _, started := os.Stat(sentinel) + if c.routed { + if started != nil { + t.Errorf("the hook did NOT start a proxy in a routed project — so every "+ + "silent row above proves nothing: %v\noutput:\n%s", started, out) + } + if strings.TrimSpace(out) == "" { + t.Errorf("the hook said nothing in a routed project where the proxy never " + + "came up; the failure report is the only diagnostic that path has") + } + return + } + if strings.TrimSpace(out) != "" { + t.Errorf("the hook printed output in an unrouted project: %q", out) + } + if started == nil { + t.Error("the hook started a proxy in a project that is not routed to it") + } + }) + } +} + +// TestHookIsIdempotentWhenTheProxyIsAlreadyUp: SessionStart also fires on clear, compact, +// resume and fork, so a long session re-runs this repeatedly. A second proxy must never be +// launched — it would fail to bind, or worse, bind a different port and split the state. +func TestHookIsIdempotentWhenTheProxyIsAlreadyUp(t *testing.T) { + ln, err := net.Listen("tcp", "127.0.0.1:0") + if err != nil { + t.Fatal(err) + } + defer ln.Close() + mux := http.NewServeMux() + mux.HandleFunc("/healthz", func(w http.ResponseWriter, _ *http.Request) { w.Write([]byte("ok")) }) + srv := &http.Server{Handler: mux, ReadHeaderTimeout: 5 * time.Second} + go srv.Serve(ln) //nolint:errcheck // returns ErrServerClosed on Close + defer srv.Close() + port := fmt.Sprint(ln.Addr().(*net.TCPAddr).Port) + + out, code, sentinel := runStart(t, map[string]string{ + "CLAUDE_PLUGIN_OPTION_PORT": port, + "ANTHROPIC_BASE_URL": "http://127.0.0.1:" + port + "/anthropic", + }) + if code != 0 { + t.Errorf("exit %d, output %q", code, out) + } + if _, err := os.Stat(sentinel); err == nil { + t.Error("a second proxy was started even though /healthz already answered") + } + if strings.TrimSpace(out) != "" { + t.Errorf("nothing to do, but the hook printed %q", out) + } +} + +// TestHookNeverFailsTheSessionWhenTheBinaryIsMissing: routed, but the binary is gone (the user +// deleted it, or PATH differs under the hook). The session must still start, with an +// explanation — a hook that exits non-zero here is a plugin that can brick every session on the +// machine, which is the biggest risk in this whole feature. +func TestHookNeverFailsTheSessionWhenTheBinaryIsMissing(t *testing.T) { + requireTool(t, "bash") + dir := t.TempDir() + cmd := exec.Command("bash", filepath.Join(scriptsDir(t), "start-proxy.sh")) + // An unused high port: nothing answers /healthz, and the binary does not exist. + cmd.Env = append(os.Environ(), + "CLAUDE_PLUGIN_OPTION_PORT=8799", + "ANTHROPIC_BASE_URL=http://127.0.0.1:8799/anthropic", + "CONTEXT_GURU_BIN="+filepath.Join(dir, "does-not-exist"), + "TMPDIR="+dir) + b, err := cmd.CombinedOutput() + code := 0 + if ee, ok := err.(*exec.ExitError); ok { + code = ee.ExitCode() + } else if err != nil { + t.Fatal(err) + } + if code != 0 { + t.Fatalf("exit %d — the hook must never fail a session: %s", code, b) + } + out := string(b) + for _, want := range []string{"not on PATH", "/context-guru:install"} { + if !strings.Contains(out, want) { + t.Errorf("the explanation omits %q:\n%s", want, out) + } + } +} + +// TestHookStartsTheProxyAndWaitsForHealthz is the positive path, and specifically the WAIT: the +// hook is synchronous on purpose, so the session's first API request cannot beat the proxy up. +// A hook that returned before /healthz answered would leave that race in place. +func TestHookStartsTheProxyAndWaitsForHealthz(t *testing.T) { + requireTool(t, "bash") + py := requireTool(t, "python3") + if runtime.GOOS == "windows" { + t.Skip("shell hook is POSIX-only") + } + ln, err := net.Listen("tcp", "127.0.0.1:0") + if err != nil { + t.Fatal(err) + } + port := fmt.Sprint(ln.Addr().(*net.TCPAddr).Port) + ln.Close() // free it for the fake proxy to bind + + dir := t.TempDir() + // A stand-in proxy: takes ~1s to come up, then answers /healthz. The delay is the point — + // it is what a hook that does not wait would skip past. + fake := filepath.Join(dir, "fake-proxy") + script := "#!/usr/bin/env bash\nsleep 1\nexec " + py + " -c '\n" + + "import http.server\n" + + "class H(http.server.BaseHTTPRequestHandler):\n" + + " def do_GET(self):\n" + + " self.send_response(200); self.end_headers(); self.wfile.write(b\"ok\")\n" + + " def log_message(self, *a): pass\n" + + "http.server.HTTPServer((\"127.0.0.1\", " + port + "), H).serve_forever()\n'\n" + if err := os.WriteFile(fake, []byte(script), 0o755); err != nil { + t.Fatal(err) + } + + cmd := exec.Command("bash", filepath.Join(scriptsDir(t), "start-proxy.sh")) + cmd.Env = append(os.Environ(), + "CLAUDE_PLUGIN_OPTION_PORT="+port, + "ANTHROPIC_BASE_URL=http://127.0.0.1:"+port+"/anthropic", + "CONTEXT_GURU_BIN="+fake, + "TMPDIR="+dir) + b, err := cmd.CombinedOutput() + t.Cleanup(func() { exec.Command("pkill", "-f", "127.0.0.1\", "+port).Run() }) //nolint:errcheck + if err != nil { + t.Fatalf("start-proxy.sh failed: %v\n%s", err, b) + } + if !strings.Contains(string(b), "proxy up on 127.0.0.1:"+port) { + t.Fatalf("the hook returned without reporting a healthy proxy:\n%s", b) + } + // The claim is that it returned only AFTER /healthz answered, so it must answer now. + resp, err := http.Get("http://127.0.0.1:" + port + "/healthz") + if err != nil { + t.Fatalf("the hook reported the proxy up, but /healthz does not answer: %v", err) + } + resp.Body.Close() +} + +// --- fixes from the review of #141 --------------------------------------------------------- + +// TestBackupsDoNotClobberEachOther is the defect that destroyed the user's undo. +// +// The stamp was second-granularity with a plain copy2, so an install-then-uninstall round trip — +// well inside one second — wrote both backups to the SAME filename. The survivor held the +// POST-install state, and the install skill tells the user to keep that path as their undo. The +// value it was supposed to protect was gone from both the file and the backup. +func TestBackupsDoNotClobberEachOther(t *testing.T) { + dir := t.TempDir() + path := filepath.Join(dir, "settings.json") + theirs := "https://gateway.corp.example/anthropic" + writeJSON(t, path, map[string]any{"env": map[string]any{"ANTHROPIC_BASE_URL": theirs}}) + + // Back to back, deliberately: the bug needed only that both land in the same second. + add, code := settings(t, "add", "--file", path, "--url", ourURL, "--force") + if code != 0 { + t.Fatalf("add: exit %d, %v", code, add) + } + rm, code := settings(t, "remove", "--file", path, "--url", ourURL) + if code != 0 { + t.Fatalf("remove: exit %d, %v", code, rm) + } + if add["backup"] == rm["backup"] { + t.Fatalf("both operations reported the same backup path %q, so one overwrote the other", + add["backup"]) + } + // The install backup must still hold what was there BEFORE we touched it. + b, err := os.ReadFile(add["backup"]) + if err != nil { + t.Fatalf("the install backup is gone: %v", err) + } + if !strings.Contains(string(b), theirs) { + t.Errorf("the install backup does not contain the value it was meant to preserve:\n%s", b) + } +} + +// TestUninstallRestoresTheBaseURLItReplaced: after a --force install over somebody's own gateway, +// uninstall must hand it back. Deleting the key left them with NO base URL at all — a worse state +// than before they installed, and (with the backup defect above) unrecoverable from anything the +// tool produced. +func TestUninstallRestoresTheBaseURLItReplaced(t *testing.T) { + dir := t.TempDir() + path := filepath.Join(dir, "settings.json") + theirs := "https://gateway.corp.example/anthropic" + writeJSON(t, path, map[string]any{"env": map[string]any{ + "ANTHROPIC_BASE_URL": theirs, "ANTHROPIC_AUTH_TOKEN": "keep"}}) + + if _, code := settings(t, "add", "--file", path, "--url", ourURL, "--force"); code != 0 { + t.Fatal("add --force failed") + } + facts, code := settings(t, "remove", "--file", path, "--url", ourURL) + if code != 0 { + t.Fatalf("remove: exit %d, %v", code, facts) + } + if facts["restored"] != theirs { + t.Errorf("remove reported restored=%q, want %q", facts["restored"], theirs) + } + env, _ := readJSON(t, path)["env"].(map[string]any) + if env["ANTHROPIC_BASE_URL"] != theirs { + t.Fatalf("the user's own base URL was not restored: %v", env) + } + if env["ANTHROPIC_AUTH_TOKEN"] != "keep" { + t.Errorf("an unrelated env var was lost: %v", env) + } + // And no bookkeeping left behind. + if _, ok := readJSON(t, path)["$context-guru"]; ok { + t.Errorf("uninstall left its own bookkeeping key in the user's settings") + } +} + +// TestSettingsPreservesFileMode: the file holds a credential often enough that widening its mode +// is a real leak. The temp file is created fresh, so os.replace took the UMASK mode rather than +// the replaced file's — a 600 settings file came back 644 under the common default. +func TestSettingsPreservesFileMode(t *testing.T) { + if runtime.GOOS == "windows" { + t.Skip("POSIX modes only") + } + dir := t.TempDir() + path := filepath.Join(dir, "settings.json") + writeJSON(t, path, map[string]any{"env": map[string]any{"ANTHROPIC_AUTH_TOKEN": "secret"}}) + if err := os.Chmod(path, 0o600); err != nil { + t.Fatal(err) + } + if _, code := settings(t, "add", "--file", path, "--url", ourURL); code != 0 { + t.Fatal("add failed") + } + fi, err := os.Stat(path) + if err != nil { + t.Fatal(err) + } + if got := fi.Mode().Perm(); got != 0o600 { + t.Errorf("mode after add = %o, want 600: this file holds a credential", got) + } +} + +// TestSettingsFollowsASymlink: a dotfile-managed settings.json is commonly a symlink into a +// repository. os.replace onto the link path replaces the LINK with a regular file, so the edit +// never reaches the file the user manages and their dotfiles still hold the old content — while +// the tool reports success. +func TestSettingsFollowsASymlink(t *testing.T) { + if runtime.GOOS == "windows" { + t.Skip("symlink semantics differ") + } + dir := t.TempDir() + real := filepath.Join(dir, "dotfiles", "settings.json") + if err := os.MkdirAll(filepath.Dir(real), 0o755); err != nil { + t.Fatal(err) + } + writeJSON(t, real, map[string]any{"theme": "dark"}) + link := filepath.Join(dir, "settings.json") + if err := os.Symlink(real, link); err != nil { + t.Skipf("cannot symlink here: %v", err) + } + + if _, code := settings(t, "add", "--file", link, "--url", ourURL); code != 0 { + t.Fatal("add failed") + } + fi, err := os.Lstat(link) + if err != nil { + t.Fatal(err) + } + if fi.Mode()&os.ModeSymlink == 0 { + t.Error("the symlink was replaced by a regular file, so the user's dotfiles repo never saw the edit") + } + env, _ := readJSON(t, real)["env"].(map[string]any) + if env["ANTHROPIC_BASE_URL"] != ourURL { + t.Errorf("the edit did not reach the real file: %v", readJSON(t, real)) + } +} + +// TestSettingsRecognisesItsOwnURLOnAnotherPort: changing the configured port and re-running install +// used to report a conflict against context-guru itself, telling the user something else owned +// their routing. +func TestSettingsRecognisesItsOwnURLOnAnotherPort(t *testing.T) { + dir := t.TempDir() + path := filepath.Join(dir, "settings.json") + // The state a previous install leaves: the URL it wrote, recorded. + writeJSON(t, path, map[string]any{ + "env": map[string]any{"ANTHROPIC_BASE_URL": "http://localhost:9999/anthropic"}, + "$context-guru": map[string]any{"installed_base_url": "http://localhost:9999/anthropic"}, + }) + + facts, code := settings(t, "add", "--file", path, "--url", ourURL) + if code != 0 || facts["result"] != "repointed" { + t.Fatalf("expected a clean repoint, got exit %d, %v", code, facts) + } + if env, _ := readJSON(t, path)["env"].(map[string]any); env["ANTHROPIC_BASE_URL"] != ourURL { + t.Errorf("not repointed: %v", env) + } + // Anything we did NOT record stays a conflict — including another LOCAL proxy, which is the + // case a URL-shape rule got wrong: litellm's default is http://127.0.0.1:4000/anthropic, and + // treating that as ours would have let uninstall delete somebody else's routing. + for _, theirs := range []string{ + "https://8787.example.com/anthropic", // remote host that merely contains our port + "http://127.0.0.1:4000/anthropic", // another local proxy (litellm's default) + } { + p2 := filepath.Join(dir, "conflict.json") + writeJSON(t, p2, map[string]any{"env": map[string]any{"ANTHROPIC_BASE_URL": theirs}}) + facts, code = settings(t, "add", "--file", p2, "--url", ourURL) + if code != 2 || facts["result"] != "conflict" { + t.Errorf("%s must be a conflict, not ours: exit %d, %v", theirs, code, facts) + } + if _, code := settings(t, "remove", "--file", p2, "--url", ourURL); code != 2 { + t.Errorf("uninstall must refuse to remove %s", theirs) + } + } +} + +// TestInstallRefusesAnUnverifiedDownload is the security fix, and it is the one to keep. +// +// A checksum MISMATCH was fatal, but an absent or unfetchable checksums.txt printed one advisory +// line and fell through to `install -m 755`. An unverified binary landed on a PATH directory and +// ran — a binary that handles all of the user's LLM traffic and holds their API key. The script's +// own comment said "a failure here is fatal, never a warning" while the code did the opposite. +func TestInstallRefusesAnUnverifiedDownload(t *testing.T) { + requireTool(t, "bash") + dir := t.TempDir() + + // A stub `curl` that serves a tarball and 404s the checksum file — exactly the shape of a + // release whose checksums.txt is missing. + bin := filepath.Join(dir, "bin") + if err := os.MkdirAll(bin, 0o755); err != nil { + t.Fatal(err) + } + payload := filepath.Join(dir, "context-guru-proxy") + if err := os.WriteFile(payload, []byte("#!/bin/sh\necho THIS BINARY WAS NEVER VERIFIED\n"), 0o755); err != nil { + t.Fatal(err) + } + tarball := filepath.Join(dir, "payload.tar.gz") + if out, err := exec.Command("tar", "czf", tarball, "-C", dir, "context-guru-proxy").CombinedOutput(); err != nil { + t.Fatalf("tar: %v (%s)", err, out) + } + stub := "#!/usr/bin/env bash\n" + + "# args end with the URL; -o gives the destination\n" + + "dest=\"\"; url=\"\"\n" + + "while [ $# -gt 0 ]; do case \"$1\" in -o) dest=$2; shift 2;; -*) shift;; *) url=$1; shift;; esac; done\n" + + "case \"$url\" in\n" + + " *checksums.txt) exit 22;;\n" + + " *api.github.com*) printf '{\"tag_name\": \"v9.9.9\"}' ${dest:+> \"$dest\"}; exit 0;;\n" + + " *.tar.gz) cp " + tarball + " \"$dest\"; exit 0;;\n" + + "esac\nexit 22\n" + if err := os.WriteFile(filepath.Join(bin, "curl"), []byte(stub), 0o755); err != nil { + t.Fatal(err) + } + + dest := filepath.Join(dir, "dest") + cmd := exec.Command("bash", filepath.Join(scriptsDir(t), "install.sh")) + cmd.Env = append(os.Environ(), + "PATH="+bin+":"+os.Getenv("PATH"), + "CONTEXT_GURU_DEST="+dest, + "HOME="+dir, + ) + out, err := cmd.CombinedOutput() + t.Logf("install.sh output:\n%s", out) + if err == nil { + t.Error("install.sh succeeded without verifying the download") + } + if !strings.Contains(string(out), "checksum_unavailable") { + t.Errorf("the refusal does not name the reason: %s", out) + } + if _, err := os.Stat(filepath.Join(dest, "context-guru-proxy")); err == nil { + t.Fatal("an unverified binary was installed onto a PATH directory") + } +} + +// TestHookMakesTheProxyIdentifiable is the other half of the uninstall fix. +// +// The uninstall skill used to stop the proxy with `pkill -f "context-guru-proxy.*$PORT"`, which +// could not work: the starter passed the port through LISTEN_ADDR in the environment, so it +// appeared nowhere in the proxy's command line. The pattern matched no proxy — and did match the +// shell running it, i.e. the Bash tool of the user's own session, killing it mid-command while the +// proxy kept the port. +// +// So the starter now has to leave two handles behind, and this asserts both: +// +// 1. the port in `argv`, so `ps` and a human can tell instances apart; +// 2. a pidfile, which is what uninstall actually uses — no pattern matching at all. +func TestHookMakesTheProxyIdentifiable(t *testing.T) { + requireTool(t, "bash") + dir := t.TempDir() + + // A stand-in proxy that records its own argv and then holds the port, so the starter's + // health probe succeeds and the script runs to completion. + argvFile := filepath.Join(dir, "argv") + py := requireTool(t, "python3") + port := freePort(t) + fake := filepath.Join(dir, "fake-proxy") + script := "#!/usr/bin/env bash\n" + + "printf '%s\\n' \"$*\" > " + argvFile + "\n" + + "exec " + py + " -c '\n" + + "import http.server\n" + + "class H(http.server.BaseHTTPRequestHandler):\n" + + " def do_GET(self):\n" + + " self.send_response(200); self.end_headers(); self.wfile.write(b\"ok\")\n" + + " def log_message(self, *a): pass\n" + + "http.server.HTTPServer((\"127.0.0.1\", " + port + "), H).serve_forever()\n'\n" + if err := os.WriteFile(fake, []byte(script), 0o755); err != nil { + t.Fatal(err) + } + + state := filepath.Join(dir, "state") + cmd := exec.Command("bash", filepath.Join(scriptsDir(t), "start-proxy.sh")) + cmd.Env = append(os.Environ(), + "CLAUDE_PLUGIN_OPTION_PORT="+port, + "ANTHROPIC_BASE_URL=http://127.0.0.1:"+port+"/anthropic", + "CONTEXT_GURU_BIN="+fake, + "XDG_STATE_HOME="+state, + "TMPDIR="+dir) + out, err := cmd.CombinedOutput() + t.Logf("start-proxy.sh:\n%s", out) + t.Cleanup(func() { + if b, e := os.ReadFile(filepath.Join(state, "context-guru", "proxy-"+port+".pid")); e == nil { + exec.Command("kill", strings.TrimSpace(string(b))).Run() //nolint:errcheck + } + }) + if err != nil { + t.Fatalf("start-proxy.sh failed: %v", err) + } + + // (1) the port is on the command line. + argv, err := os.ReadFile(argvFile) + if err != nil { + t.Fatalf("the fake proxy never ran: %v", err) + } + if !strings.Contains(string(argv), "--listen") || !strings.Contains(string(argv), port) { + t.Errorf("the proxy's argv does not carry its port (%q); nothing can identify this "+ + "instance among others, which is what made the old pkill pattern match the caller's "+ + "own shell instead", strings.TrimSpace(string(argv))) + } + // The dashboard must not be written into whatever directory the proxy started in — that is + // the user's repository. + if !strings.Contains(string(argv), "--dashboard-db") { + t.Errorf("no explicit --dashboard-db, so the database lands in the current directory: %q", argv) + } + + // (2) the pidfile exists, names a live process, and that process is ours. + pidfile := filepath.Join(state, "context-guru", "proxy-"+port+".pid") + b, err := os.ReadFile(pidfile) + if err != nil { + t.Fatalf("no pidfile at %s: uninstall has no handle but a pattern match: %v", pidfile, err) + } + pid, err := strconv.Atoi(strings.TrimSpace(string(b))) + if err != nil || pid <= 0 { + t.Fatalf("pidfile does not contain a pid: %q", b) + } + if err := syscall.Kill(pid, 0); err != nil { + t.Errorf("pidfile names pid %d, which is not running: %v", pid, err) + } +} + +// freePort asks the kernel for a port and gives it back, so the fake proxy can bind it. +func freePort(t *testing.T) string { + t.Helper() + ln, err := net.Listen("tcp", "127.0.0.1:0") + if err != nil { + t.Fatal(err) + } + p := fmt.Sprint(ln.Addr().(*net.TCPAddr).Port) + ln.Close() + return p +} + +// --- fixes from the review of #160 --------------------------------------------------------- + +// hookTimeout reads a hook's timeout out of hooks.json rather than hardcoding it here. +// +// Read from the config on purpose: the defect below was a mismatch BETWEEN this file's budget and +// that file's timeout, so a test with the number retyped into it could pass while the pair drifted +// apart again. This way, lowering the timeout in hooks.json fails the test that depends on it. +func hookTimeout(t *testing.T, event string) time.Duration { + t.Helper() + b, err := os.ReadFile(filepath.Join("hooks", "hooks.json")) + if err != nil { + t.Fatalf("reading hooks.json: %v", err) + } + var cfg struct { + Hooks map[string][]struct { + Hooks []struct { + Command string `json:"command"` + Timeout int `json:"timeout"` + } `json:"hooks"` + } `json:"hooks"` + } + if err := json.Unmarshal(b, &cfg); err != nil { + t.Fatalf("hooks.json does not parse: %v", err) + } + group, ok := cfg.Hooks[event] + if !ok || len(group) == 0 || len(group[0].Hooks) == 0 { + t.Fatalf("hooks.json has no %s hook", event) + } + secs := group[0].Hooks[0].Timeout + if secs <= 0 { + t.Fatalf("%s hook has no timeout in hooks.json", event) + } + return time.Duration(secs) * time.Second +} + +// stallingPort returns a port with a listener that ACCEPTS connections and never answers. +// +// This is the shape that made the old iteration-counted health loop pathological: curl cannot +// return early, so every probe burns its full --max-time instead of failing instantly the way a +// refused port does. A hung proxy, a half-open socket, and an unrelated service holding the port +// all look like this, and none of them are exotic. +func stallingPort(t *testing.T) string { + t.Helper() + ln, err := net.Listen("tcp", "127.0.0.1:0") + if err != nil { + t.Fatal(err) + } + // Closing the listener is registered from the TEST goroutine; that is what unblocks Accept + // below and lets the goroutine tear down its own connections. + t.Cleanup(func() { ln.Close() }) + go func() { + // `held` is touched by this goroutine only — appended here, closed by this deferred func. + // An earlier version registered that cleanup with t.Cleanup from inside here, which reads + // the slice from the test goroutine while this one appends to it: a data race that -race + // caught in CI and a non-race run cannot see. + var held []net.Conn + defer func() { + for _, c := range held { + c.Close() + } + }() + for { + c, err := ln.Accept() + if err != nil { // the listener was closed by the cleanup above + return + } + held = append(held, c) // hold it open, answer nothing + } + }() + return fmt.Sprint(ln.Addr().(*net.TCPAddr).Port) +} + +// runCheck runs check-proxy.sh the way the UserPromptSubmit hook does, with a stand-in binary. +// +// `bin` is the script body of the fake proxy; the caller decides whether it ever binds the port. +func runCheck(t *testing.T, port, binBody string) (out string, code int, elapsed time.Duration) { + t.Helper() + requireTool(t, "bash") + dir := t.TempDir() + fake := filepath.Join(dir, "fake-proxy") + if err := os.WriteFile(fake, []byte(binBody), 0o755); err != nil { + t.Fatal(err) + } + root, err := filepath.Abs(".") + if err != nil { + t.Fatal(err) + } + cmd := exec.Command("bash", filepath.Join(scriptsDir(t), "check-proxy.sh")) + cmd.Env = append(os.Environ(), + "CLAUDE_PLUGIN_ROOT="+root, + "CLAUDE_PLUGIN_OPTION_PORT="+port, + "ANTHROPIC_BASE_URL=http://127.0.0.1:"+port+"/anthropic", + "CONTEXT_GURU_BIN="+fake, + "XDG_STATE_HOME="+filepath.Join(dir, "state"), + "TMPDIR="+dir) + start := time.Now() + b, err := cmd.CombinedOutput() + elapsed = time.Since(start) + if ee, ok := err.(*exec.ExitError); ok { + code = ee.ExitCode() + } else if err != nil { + t.Fatalf("running check-proxy.sh: %v (%s)", err, b) + } + t.Logf("check-proxy.sh -> exit %d in %v, output:\n%s", code, elapsed.Round(time.Millisecond), b) + return string(b), code, elapsed +} + +// TestCheckHookFinishesInsideItsOwnTimeout is the test whose absence hid a defect that defeated +// the hook's entire purpose. +// +// check-proxy.sh exists for one case: routing configured, nothing listening, and therefore a +// prompt that produces NOTHING — no error, no timeout the user can read. The hook replaces that +// silence with an explanation. But the explanation is the LAST thing the script prints, after it +// has tried to recover, and recovery called start-proxy.sh with its default 15s health wait. The +// measured total was 19s against a 10s hook timeout, so on exactly the path the hook was written +// for, Claude Code killed it first and the user saw nothing at all — the same symptom, now with a +// hook that was supposed to have fixed it. +// +// Asserted with real margin rather than "under the timeout": a check that only just fits is a +// check that fails on a loaded CI runner, and a flaky test here would get muted, which is how the +// property would be lost a second time. +func TestCheckHookFinishesInsideItsOwnTimeout(t *testing.T) { + if runtime.GOOS == "windows" { + t.Skip("shell hook is POSIX-only") + } + limit := hookTimeout(t, "UserPromptSubmit") + // The WORST shape, deliberately: a port that accepts and never answers, so each of the three + // probes costs its full --max-time on top of the health budget. Measuring the cheap shape (a + // refused port, where curl returns instantly) would have let this assertion pass at almost any + // timeout, which is the opposite of what it is for. + port := stallingPort(t) + out, code, elapsed := runCheck(t, port, "#!/usr/bin/env bash\nsleep 60\n") + + if code != 0 { + t.Errorf("the hook exited %d; it must never fail a prompt", code) + } + if margin := limit / 2; elapsed > margin { + t.Errorf("check-proxy.sh took %v on the dead-proxy path; hooks.json allows %v for "+ + "UserPromptSubmit, and this must finish inside half of that so a loaded machine "+ + "still gets the diagnostic (it is printed last, so a kill means the user sees nothing)", + elapsed.Round(time.Millisecond), limit) + } + // The whole point of surviving is what it says. Compare on collapsed whitespace: the note is + // hard-wrapped for a terminal, so a plain substring match depends on where the wrap lands. + flat := strings.Join(strings.Fields(out), " ") + for _, want := range []string{ + "nothing is answering there", + "Your request will hang with no error message", + "--dashboard", // the printed recovery command must not recreate the 404 dashboard + } { + if !strings.Contains(flat, want) { + t.Errorf("the diagnostic is missing %q; output was:\n%s", want, out) + } + } +} + +// TestCheckHookIsSilentWhereRoutingIsNotConfigured mirrors the SessionStart property, and matters +// more here: this hook runs on EVERY PROMPT in every project on the machine, not once per session. +// Anything it prints lands in the model's context for that turn, so a regression is noise on every +// turn the user takes, in projects that have nothing to do with context-guru. +func TestCheckHookIsSilentWhereRoutingIsNotConfigured(t *testing.T) { + if runtime.GOOS == "windows" { + t.Skip("shell hook is POSIX-only") + } + requireTool(t, "bash") + // Last row is a POSITIVE CONTROL: without it every assertion here is an absence, and a + // check-proxy.sh gutted to `exit 0` passes all of them. See the note in the SessionStart + // equivalent — a control living in another test is one this test cannot rely on. + for _, c := range []struct { + name, baseURL string + routed bool + }{ + {name: "unset"}, + {name: "another local proxy on a different port (e.g. litellm)", baseURL: "http://localhost:4000/anthropic"}, + {name: "a remote gateway", baseURL: "https://gateway.corp.example/anthropic"}, + // The gate was a PREFIX match, so port 8787 also matched 87871 — and this hook would then + // probe and start our proxy underneath a user routed elsewhere on that port. + {name: "our port as a PREFIX of a longer port", baseURL: "http://127.0.0.1:87871/anthropic"}, + {name: "POSITIVE CONTROL: a routed project with a dead proxy, where it must speak", routed: true}, + } { + t.Run(c.name, func(t *testing.T) { + dir := t.TempDir() + port := "8787" + baseURL := c.baseURL + if c.routed { + port = freePort(t) // our own port, so a developer's real 8787 is never touched + baseURL = "http://127.0.0.1:" + port + "/anthropic" + } + cmd := exec.Command("bash", filepath.Join(scriptsDir(t), "check-proxy.sh")) + cmd.Env = append(os.Environ(), + "CLAUDE_PLUGIN_OPTION_PORT="+port, + "ANTHROPIC_BASE_URL="+baseURL, + // No recovery attempt: the binary is absent, so this exercises the gate and the + // diagnostic without waiting on a health budget. + "CONTEXT_GURU_BIN=/nonexistent/never-run-me", + "XDG_STATE_HOME="+dir, + "TMPDIR="+dir) + b, err := cmd.CombinedOutput() + if err != nil { + t.Fatalf("the hook must exit 0 everywhere: %v\n%s", err, b) + } + spoke := len(strings.TrimSpace(string(b))) != 0 + if c.routed { + if !spoke { + t.Error("the hook said nothing about a dead proxy in a ROUTED project — so " + + "every silent row above is consistent with the script doing nothing at all") + } + return + } + if spoke { + t.Errorf("the hook spoke in an unrouted project (base URL %q):\n%s", c.baseURL, b) + } + }) + } +} + +// TestCheckHookRecoversSilently pins the deliberate choice to keep the diagnostic LAST. +// +// The common case for this hook is an --idle-exit between two prompts: the proxy is gone, it comes +// back, and the user should never know. Printing the note up front would guarantee it is seen when +// the hook is killed, but it would also put a paragraph about a dead proxy into the context of +// every successful recovery. Silence on success is what makes the ordering worth defending — and +// it is only safe because the recovery is now budgeted, which the timeout test above enforces. +func TestCheckHookRecoversSilently(t *testing.T) { + if runtime.GOOS == "windows" { + t.Skip("shell hook is POSIX-only") + } + py := requireTool(t, "python3") + port := freePort(t) + fake := "#!/usr/bin/env bash\nexec " + py + " -c '\n" + + "import http.server\n" + + "class H(http.server.BaseHTTPRequestHandler):\n" + + " def do_GET(self):\n" + + " self.send_response(200); self.end_headers(); self.wfile.write(b\"ok\")\n" + + " def log_message(self, *a): pass\n" + + "http.server.HTTPServer((\"127.0.0.1\", " + port + "), H).serve_forever()\n'\n" + + t.Cleanup(func() { exec.Command("pkill", "-f", "127.0.0.1\", "+port).Run() }) //nolint:errcheck + out, code, _ := runCheck(t, port, fake) + if code != 0 { + t.Errorf("exit %d; the hook must never fail a prompt", code) + } + if strings.TrimSpace(out) != "" { + t.Errorf("the hook recovered the proxy but still spoke — this output goes into the "+ + "model's context on a turn where nothing is wrong:\n%s", out) + } + resp, err := http.Get("http://127.0.0.1:" + port + "/healthz") + if err != nil { + t.Fatalf("the hook was silent but the proxy is not up — silence must mean success: %v", err) + } + resp.Body.Close() +} + +// TestStartHookBudgetsOnWallClockNotIterations covers the shape that made the failure report +// unreachable. +// +// The health wait was `for _ in $(seq 1 60)` with `--max-time 2`, commented "up to ~15s". That is +// only true when the port is REFUSED, where curl returns instantly. Against a socket that accepts +// and never answers — a hung proxy, or an unrelated service on the port — each probe burned its +// full timeout: measured 2046ms, so ~122s against a 60s SessionStart timeout. The hook was killed, +// so the block that prints the log path and the /context-guru:status pointer never ran. A hung +// port is one of the likeliest reasons to need that block, and it was the one case that never +// produced it. +func TestStartHookBudgetsOnWallClockNotIterations(t *testing.T) { + if runtime.GOOS == "windows" { + t.Skip("shell hook is POSIX-only") + } + requireTool(t, "bash") + port := stallingPort(t) + dir := t.TempDir() + fake := filepath.Join(dir, "fake-proxy") + if err := os.WriteFile(fake, []byte("#!/usr/bin/env bash\nsleep 60\n"), 0o755); err != nil { + t.Fatal(err) + } + limit := hookTimeout(t, "SessionStart") + cmd := exec.Command("bash", filepath.Join(scriptsDir(t), "start-proxy.sh")) + cmd.Env = append(os.Environ(), + "CLAUDE_PLUGIN_OPTION_PORT="+port, + "ANTHROPIC_BASE_URL=http://127.0.0.1:"+port+"/anthropic", + "CONTEXT_GURU_BIN="+fake, + "CONTEXT_GURU_HEALTH_BUDGET=3", + "XDG_STATE_HOME="+filepath.Join(dir, "state"), + "TMPDIR="+dir) + start := time.Now() + b, err := cmd.CombinedOutput() + elapsed := time.Since(start) + t.Logf("start-proxy.sh against an accept-and-stall port -> %v in %v, output:\n%s", + err, elapsed.Round(time.Millisecond), b) + if err != nil { + t.Fatalf("the hook must exit 0 even here: %v", err) + } + if elapsed > limit/2 { + t.Errorf("took %v against a stalling port; SessionStart allows %v, and the budget was 3s "+ + "— an iteration-counted loop reaches ~122s here and gets killed", elapsed, limit) + } + // The point of finishing early is that this actually gets said. + if !strings.Contains(string(b), "did not come up") { + t.Errorf("the failure report never ran — that is the whole defect:\n%s", b) + } +} + +// TestUninstallRefusesAForeignBaseURLEvenWithNoURLGiven is the regression test for the worst +// defect this plugin has had. +// +// `remove` guarded its conflict check with `if args.url and ...`, so the documented invocation +// with no --url skipped the check entirely and deleted whatever base URL was configured. Measured +// before the fix: a corporate gateway with no context-guru record came out `result=removed`, +// `restored=` empty, exit 0 — the user's gateway silently gone, reported as success. +// +// The file must come back BYTE-IDENTICAL, not merely "still have a base URL": a refusal that +// rewrites the file has already done the thing it refused. +func TestUninstallRefusesAForeignBaseURLEvenWithNoURLGiven(t *testing.T) { + dir := t.TempDir() + path := filepath.Join(dir, "settings.json") + const foreign = `{ + "env": { + "ANTHROPIC_BASE_URL": "https://gateway.corp.example.com", + "ANTHROPIC_AUTH_TOKEN": "sk-corp-secret" + }, + "permissions": {"allow": ["Bash(ls:*)"]} +} +` + if err := os.WriteFile(path, []byte(foreign), 0o600); err != nil { + t.Fatal(err) + } + + facts, code := settings(t, "remove", "--file", path) + if code != 2 { + t.Errorf("exit %d for a base URL we never installed; want 2 (conflict). facts=%v", code, facts) + } + if facts["result"] != "conflict" { + t.Errorf("result=%q, want conflict", facts["result"]) + } + got, err := os.ReadFile(path) + if err != nil { + t.Fatal(err) + } + if string(got) != foreign { + t.Errorf("the file was rewritten while refusing to change it:\n--- got ---\n%s\n--- want ---\n%s", + got, foreign) + } + // And with no --url, a URL we DID record must still be removable, or uninstall is broken. + facts, code = settings(t, "add", "--file", path, "--url", "http://127.0.0.1:8787/anthropic", "--force") + if code != 0 { + t.Fatalf("add --force failed: %v", facts) + } + facts, code = settings(t, "remove", "--file", path) + if code != 0 || facts["result"] != "removed" { + t.Errorf("a recorded URL must be removable with no --url: exit %d, facts=%v", code, facts) + } +} + +// TestInstallReportsPATHFromTheSourceFallbackToo covers a silence, which is why it went unnoticed. +// +// try_source_build returned success and the script exited right there, bypassing the shared tail — +// so a user who landed on the `go install` fallback got `result=installed` and NO `on_path` line. +// ~/.local/bin frequently is not on PATH, and install/SKILL.md only warns when it reads on_path=false, +// so nothing said anything. The failure surfaced later, in a different session, as the SessionStart +// hook reporting "the proxy binary is not on PATH", with nothing tying it back to the install. +func TestInstallReportsPATHFromTheSourceFallbackToo(t *testing.T) { + requireTool(t, "bash") + dir := t.TempDir() + bin := filepath.Join(dir, "bin") + if err := os.MkdirAll(bin, 0o755); err != nil { + t.Fatal(err) + } + + // curl: resolve a release, then 404 the tarball — a published tag with no asset for this + // platform, which is exactly what sends the script to the source fallback. + stub := "#!/usr/bin/env bash\n" + + "dest=\"\"; url=\"\"\n" + + "while [ $# -gt 0 ]; do case \"$1\" in -o) dest=$2; shift 2;; -*) shift;; *) url=$1; shift;; esac; done\n" + + "case \"$url\" in\n" + + " *api.github.com*) printf '{\"tag_name\": \"v9.9.9\"}' ${dest:+> \"$dest\"}; exit 0;;\n" + + "esac\nexit 22\n" + if err := os.WriteFile(filepath.Join(bin, "curl"), []byte(stub), 0o755); err != nil { + t.Fatal(err) + } + // A stand-in `go` that installs into GOBIN the way the real one does, without a toolchain. + goStub := "#!/usr/bin/env bash\n" + + "[ \"$1\" = install ] || exit 1\n" + + "mkdir -p \"$GOBIN\" && printf '#!/bin/sh\\ntrue\\n' > \"$GOBIN/context-guru-proxy\"\n" + + "chmod 755 \"$GOBIN/context-guru-proxy\"\n" + if err := os.WriteFile(filepath.Join(bin, "go"), []byte(goStub), 0o755); err != nil { + t.Fatal(err) + } + + dest := filepath.Join(dir, "dest") + run := func(pathHasDest bool) map[string]string { + path := bin + ":" + os.Getenv("PATH") + if pathHasDest { + path = dest + ":" + path + } + cmd := exec.Command("bash", filepath.Join(scriptsDir(t), "install.sh")) + cmd.Env = append(os.Environ(), "PATH="+path, "CONTEXT_GURU_DEST="+dest, "HOME="+dir) + out, err := cmd.CombinedOutput() + t.Logf("install.sh (dest on PATH=%v) -> %v, output:\n%s", pathHasDest, err, out) + if err != nil { + t.Fatalf("the source fallback should have succeeded: %v", err) + } + facts := map[string]string{} + for _, line := range strings.Split(string(out), "\n") { + if k, v, ok := strings.Cut(strings.TrimSpace(line), "="); ok { + facts[k] = v + } + } + return facts + } + + facts := run(false) + if facts["result"] != "installed" || facts["built_from"] != "source" { + t.Fatalf("the fallback did not report a source install: %v", facts) + } + if facts["on_path"] != "false" { + t.Errorf("on_path=%q with $DEST off PATH; the fallback must report this or the user only "+ + "finds out from a hook in a later session: %v", facts["on_path"], facts) + } + if facts["note"] == "" { + t.Errorf("no actionable note accompanied on_path=false: %v", facts) + } + if facts["fallback"] != "go_install_attempted" { + t.Errorf("fallback=%q — the line is printed before the build, so it must not read as "+ + "proof the build worked: %v", facts["fallback"], facts) + } + + if facts := run(true); facts["on_path"] != "true" { + t.Errorf("on_path=%q with $DEST on PATH: %v", facts["on_path"], facts) + } +} + +// TestBackupPruningSurvivesAGlobbyPath: `glob.glob` reads `[`, `?` and `*` in the PATH as pattern +// syntax, so for a settings file under a directory like `foo[1]` the prune matched nothing and +// silently did nothing — forever. Invisible by construction, because pruning is best-effort, and +// the backups KEEP_BACKUPS exists to bound then grow without limit in the user's ~/.claude. +func TestBackupPruningSurvivesAGlobbyPath(t *testing.T) { + dir := filepath.Join(t.TempDir(), "proj[1]", ".claude") + if err := os.MkdirAll(dir, 0o755); err != nil { + t.Fatal(err) + } + path := filepath.Join(dir, "settings.json") + writeJSON(t, path, map[string]any{"env": map[string]any{"KEEP": "yes"}}) + + // Each add/remove pair takes a backup, so this comfortably exceeds KEEP_BACKUPS (10). + for i := 0; i < 8; i++ { + if _, code := settings(t, "add", "--file", path, "--url", ourURL); code != 0 { + t.Fatalf("add %d failed", i) + } + if _, code := settings(t, "remove", "--file", path, "--url", ourURL); code != 0 { + t.Fatalf("remove %d failed", i) + } + } + entries, err := os.ReadDir(dir) + if err != nil { + t.Fatal(err) + } + backups := 0 + for _, e := range entries { + if strings.Contains(e.Name(), ".context-guru-backup-") { + backups++ + } + } + t.Logf("%d backups left under a path containing [ ]", backups) + if backups > 11 { // KEEP_BACKUPS plus the one just written + t.Errorf("%d backups accumulated under a globby path — pruning never matched anything", backups) + } +} + +// skillBlock returns the fenced ```bash block in a skill file that contains `needle`. +// +// The skills are prompts, but the destructive steps in them are shell that gets run verbatim. So +// the ones that can hurt somebody are extracted and EXECUTED here rather than reviewed by reading: +// reading is exactly what missed the ordering defect this covers, twice. +func skillBlock(t *testing.T, skill, needle string) string { + t.Helper() + b, err := os.ReadFile(filepath.Join("skills", skill, "SKILL.md")) + if err != nil { + t.Fatalf("reading skill: %v", err) + } + var blocks []string + var cur []string + in := false + for _, line := range strings.Split(string(b), "\n") { + switch { + case !in && strings.HasPrefix(line, "```bash"): + in, cur = true, nil + case in && strings.HasPrefix(line, "```"): + in = false + blocks = append(blocks, strings.Join(cur, "\n")) + case in: + cur = append(cur, line) + } + } + found := "" + for _, blk := range blocks { + if strings.Contains(blk, needle) { + if found != "" { + t.Fatalf("%s/SKILL.md has more than one bash block containing %q; the test cannot "+ + "tell which one is the destructive path", skill, needle) + } + found = blk + } + } + if found == "" { + t.Fatalf("no bash block in %s/SKILL.md contains %q", skill, needle) + } + return found +} + +// TestUninstallDoesNotSignalAProcessThatIsNotOurs executes the uninstall skill's stop-the-proxy +// block against a PID that is NOT a context-guru proxy. +// +// The block used to send the signal first and document the ownership check as a SEPARATE snippet +// below it — so executed the way it reads, top to bottom, `kill "$pid"` had already run by the time +// the guard was reached. That matters most on the lsof/ss fallback, which exists precisely for a +// stale pidfile or a hand-started proxy: the cases where the PID may belong to something else. A +// recycled PID satisfies `kill -0` perfectly well. +// +// This is the same shape as the defect that had uninstall killing the user's own Claude Code +// session, which is the reason it gets an executing test rather than careful prose. +func TestUninstallDoesNotSignalAProcessThatIsNotOurs(t *testing.T) { + if runtime.GOOS == "windows" { + t.Skip("shell snippet is POSIX-only") + } + requireTool(t, "bash") + block := skillBlock(t, "uninstall", `kill "$pid"`) + + for _, c := range []struct { + name string + psReports string // what `ps -p -o command=` prints + wantKilled bool + }{ + {"a stranger's process on our port", "/usr/bin/postgres -D /var/lib/postgres", false}, + {"a proxy of ours", "context-guru-proxy --listen 127.0.0.1:8787 --preset cache", true}, + } { + t.Run(c.name, func(t *testing.T) { + dir := t.TempDir() + stubs := filepath.Join(dir, "bin") + if err := os.MkdirAll(stubs, 0o755); err != nil { + t.Fatal(err) + } + killLog := filepath.Join(dir, "kill.log") + write := func(name, body string) { + if err := os.WriteFile(filepath.Join(stubs, name), []byte(body), 0o755); err != nil { + t.Fatal(err) + } + } + write("ps", "#!/usr/bin/env bash\nprintf '%s\\n' "+strconv.Quote(c.psReports)+"\n") + // No socket-owner lookup: the pidfile below is what supplies the PID. + write("lsof", "#!/usr/bin/env bash\nexit 1\n") + write("ss", "#!/usr/bin/env bash\nexit 1\n") + + state := filepath.Join(dir, "state", "context-guru") + if err := os.MkdirAll(state, 0o755); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(state, "proxy-8787.pid"), []byte("424242\n"), 0o644); err != nil { + t.Fatal(err) + } + + // `kill` must be intercepted by a FUNCTION, not a file on PATH: it is a bash builtin, + // so a PATH stub is never consulted — the first version of this test stubbed a file, + // the block's `kill -0` liveness probe therefore failed for a pid that does not exist, + // it fell through to the socket-owner lookup and the run exercised none of the branch + // under test. It records the signal rather than sending one, and answers `kill -0` as + // "alive" so the pidfile path is the one taken. + preamble := "kill() {\n" + + " if [ \"$1\" = -0 ]; then return 0; fi\n" + + " printf '%s\\n' \"$*\" >> " + strconv.Quote(killLog) + "\n" + + "}\n" + cmd := exec.Command("bash", "-c", preamble+block) + cmd.Env = append(os.Environ(), + "PATH="+stubs+":"+os.Getenv("PATH"), + "CLAUDE_PLUGIN_OPTION_PORT=8787", + "XDG_STATE_HOME="+filepath.Join(dir, "state")) + out, err := cmd.CombinedOutput() + t.Logf("uninstall stop-block -> %v, output:\n%s", err, out) + + logged, _ := os.ReadFile(killLog) + killed := strings.Contains(string(logged), "424242") + if killed != c.wantKilled { + t.Errorf("kill invoked = %v, want %v (ps reported %q). kill log: %q\nblock output:\n%s", + killed, c.wantKilled, c.psReports, logged, out) + } + if !c.wantKilled && !strings.Contains(string(out), "NOT OURS") { + t.Errorf("the block signalled nothing but also said nothing about why:\n%s", out) + } + // A pidfile belonging to someone else's process must survive: removing it would strand + // a proxy of ours that is still running under a different pid. + _, statErr := os.Stat(filepath.Join(state, "proxy-8787.pid")) + if !c.wantKilled && statErr != nil { + t.Errorf("the pidfile was removed for a process we refused to touch: %v", statErr) + } + }) + } +} + +// TestChainingUpstreamSurvivesIntoLaterSessions covers the gap the first hosted-agent install hit. +// +// On a platform whose own gateway holds the credential and rewrites model names, the proxy has to +// chain behind it — and the SessionStart hook reads its configuration from the settings env block. +// With ANTHROPIC_UPSTREAM set only in the installing shell's environment, chaining worked until the +// proxy idled out; the next session's hook then started one aimed at api.anthropic.com, where every +// request fails. So `add --upstream` writes both keys in one atomic save. +// +// The removal half matters as much: uninstall must take back only an upstream it recorded writing. +// Deleting one the user set themselves is the same overreach as deleting a base URL we never +// installed. +func TestChainingUpstreamSurvivesIntoLaterSessions(t *testing.T) { + const theirGateway = "http://127.0.0.1:24180" + + t.Run("written and removed as a pair", func(t *testing.T) { + dir := t.TempDir() + path := filepath.Join(dir, "settings.json") + writeJSON(t, path, map[string]any{"env": map[string]any{"MINE": "keep"}}) + + if facts, code := settings(t, "add", "--file", path, "--url", ourURL, + "--upstream", theirGateway); code != 0 || facts["result"] != "added" { + t.Fatalf("add --upstream failed: %v", facts) + } + env := readJSON(t, path)["env"].(map[string]any) + if env["ANTHROPIC_UPSTREAM"] != theirGateway { + t.Errorf("ANTHROPIC_UPSTREAM = %v, want %q — without it the hook starts an unchained "+ + "proxy in every later session", env["ANTHROPIC_UPSTREAM"], theirGateway) + } + if env["MINE"] != "keep" { + t.Error("the user's own env var was lost") + } + + if facts, code := settings(t, "remove", "--file", path); code != 0 { + t.Fatalf("remove failed: %v", facts) + } + env = readJSON(t, path)["env"].(map[string]any) + if _, still := env["ANTHROPIC_UPSTREAM"]; still { + t.Error("uninstall left our upstream key behind") + } + if env["MINE"] != "keep" { + t.Error("removal took the user's own env var with it") + } + }) + + t.Run("an upstream we did NOT write is left alone", func(t *testing.T) { + dir := t.TempDir() + path := filepath.Join(dir, "settings.json") + writeJSON(t, path, map[string]any{"env": map[string]any{"MINE": "keep"}}) + + // Routed by us, but the upstream is the user's own — no --upstream on the add. + if _, code := settings(t, "add", "--file", path, "--url", ourURL); code != 0 { + t.Fatal("add failed") + } + data := readJSON(t, path) + env := data["env"].(map[string]any) + env["ANTHROPIC_UPSTREAM"] = "https://their-own-choice.example" + writeJSON(t, path, data) + + if _, code := settings(t, "remove", "--file", path); code != 0 { + t.Fatal("remove failed") + } + env = readJSON(t, path)["env"].(map[string]any) + if env["ANTHROPIC_UPSTREAM"] != "https://their-own-choice.example" { + t.Errorf("uninstall deleted an upstream it never wrote: %v", env) + } + }) +} + +// TestBinPathSurvivesAMachineWhereItIsNotOnPATH is the hosted-agent case, and it is about a +// SILENT failure rather than a visible one. +// +// The SessionStart hook resolves the proxy by NAME. On a machine where the install directory is not +// on PATH — `~/.local/bin` very often is not, and on an agent pod the writable dirs reset on restart +// so "edit your shell profile" does not survive — the install succeeds, routing works for the session +// that set it up, and the auto-restart hook then never finds the binary again. The failure mode it +// exists to catch is a hang with no error, so nothing announces that the safety net is gone. +// +// `--bin` writes the absolute path into the env block the hook inherits. As with the upstream, +// uninstall must take back only a path it recorded writing. +func TestBinPathSurvivesAMachineWhereItIsNotOnPATH(t *testing.T) { + const absPath = "/home/agent/.local/bin/context-guru-proxy" + + dir := t.TempDir() + path := filepath.Join(dir, "settings.json") + writeJSON(t, path, map[string]any{"env": map[string]any{"MINE": "keep"}}) + + if facts, code := settings(t, "add", "--file", path, "--url", ourURL, "--bin", absPath); code != 0 { + t.Fatalf("add --bin failed: %v", facts) + } + env := readJSON(t, path)["env"].(map[string]any) + if env["CONTEXT_GURU_BIN"] != absPath { + t.Errorf("CONTEXT_GURU_BIN = %v, want %q — without it the hook cannot find the proxy on a "+ + "machine where its directory is not on PATH, and nothing says so", env["CONTEXT_GURU_BIN"], absPath) + } + + if _, code := settings(t, "remove", "--file", path); code != 0 { + t.Fatal("remove failed") + } + env = readJSON(t, path)["env"].(map[string]any) + if _, still := env["CONTEXT_GURU_BIN"]; still { + t.Error("uninstall left our binary path behind") + } + if env["MINE"] != "keep" { + t.Error("removal took the user's own env var with it") + } + + // A CONTEXT_GURU_BIN the user set themselves is theirs to keep. + writeJSON(t, path, map[string]any{"env": map[string]any{"MINE": "keep"}}) + if _, code := settings(t, "add", "--file", path, "--url", ourURL); code != 0 { + t.Fatal("add failed") + } + data := readJSON(t, path) + env = data["env"].(map[string]any) + env["CONTEXT_GURU_BIN"] = "/opt/their/own/build" + writeJSON(t, path, data) + if _, code := settings(t, "remove", "--file", path); code != 0 { + t.Fatal("remove failed") + } + if got := readJSON(t, path)["env"].(map[string]any)["CONTEXT_GURU_BIN"]; got != "/opt/their/own/build" { + t.Errorf("uninstall deleted a binary path it never wrote: %v", got) + } +} + +// --- fixes from the second review of #160 ----------------------------------------------------- + +// TestReRunningTheInstallFillsInMissingKeys is the defect that defeated the remedy for every other +// failure in this flow. +// +// `unchanged` used to mean "the base URL matches", and the two early returns in cmd_add wrote nothing +// else — so `add --url --upstream … --bin …` reported success and wrote NEITHER new key. Since +// re-running the install is the obvious thing to do after an attempt dies partway (which is how every +// hosted-agent attempt ended), the repair was a silent no-op. The repointed path had the same hole, so +// changing the configured port un-chained the proxy without a word. +func TestReRunningTheInstallFillsInMissingKeys(t *testing.T) { + const gw = "http://gw.example:4000" + const bin = "/opt/cg/context-guru-proxy" + + ourKeys := func(path string) map[string]any { + t.Helper() + env, _ := readJSON(t, path)["env"].(map[string]any) + out := map[string]any{} + for _, k := range []string{"ANTHROPIC_BASE_URL", "ANTHROPIC_UPSTREAM", "CONTEXT_GURU_BIN"} { + if v, ok := env[k]; ok { + out[k] = v + } + } + return out + } + + t.Run("re-run adds the keys the first run did not have", func(t *testing.T) { + dir := t.TempDir() + path := filepath.Join(dir, "settings.json") + writeJSON(t, path, map[string]any{"env": map[string]any{}}) + + if _, code := settings(t, "add", "--file", path, "--url", ourURL); code != 0 { + t.Fatal("first add failed") + } + facts, code := settings(t, "add", "--file", path, "--url", ourURL, "--upstream", gw, "--bin", bin) + if code != 0 { + t.Fatalf("re-run failed: %v", facts) + } + if facts["result"] == "unchanged" { + t.Errorf("result=unchanged on a re-run that had two keys to add — this is the no-op that "+ + "made re-running the install useless as a repair: %v", facts) + } + got := ourKeys(path) + if got["ANTHROPIC_UPSTREAM"] != gw || got["CONTEXT_GURU_BIN"] != bin { + t.Errorf("re-run did not write the missing keys: %v", got) + } + }) + + t.Run("unchanged only when there is genuinely nothing to add", func(t *testing.T) { + dir := t.TempDir() + path := filepath.Join(dir, "settings.json") + writeJSON(t, path, map[string]any{"env": map[string]any{}}) + if _, code := settings(t, "add", "--file", path, "--url", ourURL, + "--upstream", gw, "--bin", bin); code != 0 { + t.Fatal("add failed") + } + facts, code := settings(t, "add", "--file", path, "--url", ourURL, "--upstream", gw, "--bin", bin) + if code != 0 || facts["result"] != "unchanged" { + t.Errorf("a complete install re-run should be unchanged: %v", facts) + } + }) + + t.Run("a port change keeps the chaining keys", func(t *testing.T) { + dir := t.TempDir() + path := filepath.Join(dir, "settings.json") + writeJSON(t, path, map[string]any{"env": map[string]any{}}) + if _, code := settings(t, "add", "--file", path, "--url", ourURL, + "--upstream", gw, "--bin", bin); code != 0 { + t.Fatal("add failed") + } + const moved = "http://127.0.0.1:9999/anthropic" + facts, code := settings(t, "add", "--file", path, "--url", moved, "--upstream", gw, "--bin", bin) + if code != 0 || facts["result"] != "repointed" { + t.Fatalf("expected repointed: %v", facts) + } + got := ourKeys(path) + if got["ANTHROPIC_BASE_URL"] != moved { + t.Errorf("port change did not move the base URL: %v", got) + } + if got["ANTHROPIC_UPSTREAM"] != gw || got["CONTEXT_GURU_BIN"] != bin { + t.Errorf("a port change silently un-chained the proxy: %v", got) + } + }) + + t.Run("other_env_keys counts the user's keys, not ours", func(t *testing.T) { + dir := t.TempDir() + path := filepath.Join(dir, "settings.json") + writeJSON(t, path, map[string]any{"env": map[string]any{"MINE": "one"}}) + facts, _ := settings(t, "add", "--file", path, "--url", ourURL, "--upstream", gw, "--bin", bin) + if facts["other_env_keys"] != "1" { + t.Errorf("other_env_keys=%q; the user owns exactly one env var, and reporting our own "+ + "keys as theirs made /context-guru:status repeat the wrong number back to them", + facts["other_env_keys"]) + } + }) +} + +// TestStartProxyReportsArgumentsItCannotUse: three malformed invocations launched a proxy and reported +// success while doing the wrong thing, because the argument loop had no default branch and took the +// next word as a value on faith. +// +// --upsteam one transposed letter -> no upstream at all +// --upstream missing value -> no upstream at all +// --upstream --bin

swallowed the flag -> upstream="--bin", and --bin lost too +// +// The first two leave a proxy forwarding to api.anthropic.com, which on a platform whose gateway +// rewrites model names makes every request fail. Exiting non-zero is not the fix — this script must +// never fail a session — so it says what it discarded, for the same reason the declined gate leaves a +// breadcrumb: silence is indistinguishable from "never ran". +func TestStartProxyReportsArgumentsItCannotUse(t *testing.T) { + if runtime.GOOS == "windows" { + t.Skip("shell hook is POSIX-only") + } + requireTool(t, "bash") + + for _, c := range []struct { + name string + args []string + wantSaid string + wantUp string // expected --anthropic-upstream value in argv, "" for none + }{ + {"good", []string{"--upstream", "http://gw:4000"}, "", "http://gw:4000"}, + {"typo", []string{"--upsteam", "http://gw:4000"}, "unrecognised argument '--upsteam'", ""}, + {"no value", []string{"--upstream"}, "needs a value", ""}, + {"nonsense", []string{"--nonsense"}, "unrecognised argument '--nonsense'", ""}, + // The empty `=` forms, which are how a caller interpolating an unset variable arrives here. + {"empty =value", []string{"--upstream="}, "needs a value", ""}, + } { + t.Run(c.name, func(t *testing.T) { + dir := t.TempDir() + argv := filepath.Join(dir, "argv.log") + fake := filepath.Join(dir, "fake") + body := "#!/usr/bin/env bash\nprintf '%s\\n' \"$*\" >> " + argv + "\nsleep 30\n" + if err := os.WriteFile(fake, []byte(body), 0o755); err != nil { + t.Fatal(err) + } + port := freePort(t) + cmd := exec.Command("bash", append([]string{ + filepath.Join(scriptsDir(t), "start-proxy.sh"), "--unrouted", "--bin", fake, + }, c.args...)...) + cmd.Env = append(os.Environ(), + "CLAUDE_PLUGIN_OPTION_PORT="+port, + "ANTHROPIC_BASE_URL=", + "ANTHROPIC_UPSTREAM=", + "CLAUDE_PLUGIN_OPTION_UPSTREAM=", + "CONTEXT_GURU_HEALTH_BUDGET=1", + "XDG_STATE_HOME="+filepath.Join(dir, "state"), + "TMPDIR="+dir) + out, err := cmd.CombinedOutput() + t.Cleanup(func() { exec.Command("pkill", "-f", fake).Run() }) //nolint:errcheck + if err != nil { + t.Fatalf("must never fail a session: %v\n%s", err, out) + } + said := string(out) + if c.wantSaid == "" { + if strings.Contains(said, "ignoring") { + t.Errorf("complained about a valid invocation:\n%s", said) + } + } else if !strings.Contains(said, c.wantSaid) { + t.Errorf("did not report the discarded argument (want %q):\n%s", c.wantSaid, said) + } + launched, _ := os.ReadFile(argv) + if c.wantUp == "" { + if strings.Contains(string(launched), "--anthropic-upstream") { + t.Errorf("an upstream reached argv from a malformed argument: %s", launched) + } + } else if !strings.Contains(string(launched), "--anthropic-upstream "+c.wantUp) { + t.Errorf("argv missing the upstream: %s", launched) + } + }) + } +} + +// TestRejectingAValueDoesNotEatTheNextFlag is the row that used to prove nothing. +// +// `--upstream --bin ` was in the table above with assertions "it says 'needs a value'" and "no +// upstream reaches argv" — and the BUGGY parser satisfies both. Reconstructed and measured by the +// reviewer: with the shift outside the accepted branch, rejecting the value still consumed `--bin`, so +// the message appeared, no upstream reached argv, and the row was green either way. +// +// What actually separates the two versions is `--bin`: the buggy parser eats it (falling back to +// resolving the binary by name, which fails on the machines --bin exists for), the fixed one rejects +// `--upstream` alone and then parses `--bin ` normally. +// +// So this asserts the POSITIVE — the proxy was launched via the binary that --bin named. A test whose +// only assertions are absences cannot distinguish "handled correctly" from "handled wrongly in a way +// that happens to be quiet", which is the lesson three of this week's defects share. +func TestRejectingAValueDoesNotEatTheNextFlag(t *testing.T) { + if runtime.GOOS == "windows" { + t.Skip("shell hook is POSIX-only") + } + requireTool(t, "bash") + dir := t.TempDir() + argv := filepath.Join(dir, "argv.log") + fake := filepath.Join(dir, "fake-proxy") + body := "#!/usr/bin/env bash\nprintf '%s\\n' \"$*\" >> " + argv + "\nsleep 30\n" + if err := os.WriteFile(fake, []byte(body), 0o755); err != nil { + t.Fatal(err) + } + port := freePort(t) + + // --upstream has no value; --bin follows it and must survive. + cmd := exec.Command("bash", filepath.Join(scriptsDir(t), "start-proxy.sh"), + "--unrouted", "--upstream", "--bin", fake) + cmd.Env = append(os.Environ(), + "CLAUDE_PLUGIN_OPTION_PORT="+port, + "ANTHROPIC_BASE_URL=", + "ANTHROPIC_UPSTREAM=", + "CLAUDE_PLUGIN_OPTION_UPSTREAM=", + "CONTEXT_GURU_BIN=", + "CONTEXT_GURU_HEALTH_BUDGET=1", + "XDG_STATE_HOME="+filepath.Join(dir, "state"), + "TMPDIR="+dir) + out, err := cmd.CombinedOutput() + t.Cleanup(func() { exec.Command("pkill", "-f", fake).Run() }) //nolint:errcheck + if err != nil { + t.Fatalf("must never fail a session: %v\n%s", err, out) + } + t.Logf("output:\n%s", out) + + // The positive: --bin was honoured, so the proxy actually started from that path. + launched, _ := os.ReadFile(argv) + if len(launched) == 0 { + t.Errorf("--bin was eaten as --upstream's rejected value, so the binary fell back to name "+ + "resolution and nothing started. Output was:\n%s", out) + } + // And the rejection still had to be reported. + if !strings.Contains(string(out), "needs a value") { + t.Errorf("the rejected --upstream was not reported:\n%s", out) + } + // The buggy parser reports the path as a stray argument; the fixed one consumes it as --bin's value. + if strings.Contains(string(out), "unrecognised argument '"+fake+"'") { + t.Errorf("the path after --bin was treated as a stray argument, which means --bin was "+ + "consumed by the rejected --upstream:\n%s", out) + } + if strings.Contains(string(launched), "--anthropic-upstream") { + t.Errorf("an upstream reached argv from a rejected value: %s", launched) + } +} diff --git a/context-guru-plugin/scripts/check-proxy.sh b/context-guru-plugin/scripts/check-proxy.sh new file mode 100755 index 0000000..8778876 --- /dev/null +++ b/context-guru-plugin/scripts/check-proxy.sh @@ -0,0 +1,80 @@ +#!/usr/bin/env bash +# UserPromptSubmit hook: if this project is routed and the proxy is NOT answering, say so before +# the request goes out. +# +# This exists because of the worst failure mode in the whole plugin: with routing configured and no +# proxy listening, a prompt produces **nothing at all**. No error on stdout, no error on stderr, +# no timeout the user can interpret — the session simply hangs. That is the state after any crash, +# any reboot, and after every `--idle-exit`. +# +# `/context-guru:status` diagnoses it correctly and cannot be reached: invoking a skill needs Claude +# to respond, which needs an API call, which is the broken thing. A hook is the only thing left +# that runs without a model turn, and UserPromptSubmit is the last moment before the request. +# +# Every exit is 0 and this never blocks a prompt. It is a note, not a gate: the user may be about to +# ask something that does not need the API, and a hook that refuses prompts would be a worse +# failure than the one it reports. +set -uo pipefail + +PORT="${CLAUDE_PLUGIN_OPTION_PORT:-8787}" + +# Same gate as the starter, for the same reason: this plugin is installed at user scope, so this +# hook runs in every project the user has. Unrouted projects must never hear from it. +# The trailing "/" makes this an exact port match rather than a prefix one -- see the same gate +# in start-proxy.sh. PORT=8787 must not match a URL on 87871. +case "${ANTHROPIC_BASE_URL:-}" in + *"127.0.0.1:${PORT}/"* | *"localhost:${PORT}/"* | *"[::1]:${PORT}/"*) ;; + *) exit 0 ;; +esac + +if curl -fsS --max-time 2 "http://127.0.0.1:${PORT}/healthz" >/dev/null 2>&1; then + exit 0 +fi + +# Try to start it first — the common case is an idle-exit between prompts, and recovering silently +# is better than reporting a problem the user then has to act on. +# +# THE WHOLE PATH MUST FIT IN THIS HOOK'S TIMEOUT, which is the thing that was wrong here: with +# start-proxy.sh's default 15s health wait, this script measured 19s against a 10s timeout. So the +# hook was killed before reaching the note below and the user saw NOTHING — the identical symptom +# this hook exists to replace with an explanation. +# +# Two changes, because either alone leaves a hole: +# * ask start-proxy.sh for a 5s wait (it honours CONTEXT_GURU_HEALTH_BUDGET). A proxy that has +# not answered in 5s is not going to answer inside a prompt's patience anyway, and the note +# below is a better outcome than a longer silence. +# * hooks.json allows 30s, so even the slow shapes — a port that accepts and stalls costs a full +# --max-time on each of the three probes here — finish with room to spare. +# +# The note is deliberately still LAST rather than printed up front. Printing first would guarantee +# the user sees it, but this path's common case is a silent successful recovery, and a scary note +# on every idle-exit recovery is noise on a path that is working. The budget is what makes the +# ordering safe; measured end to end at ~6s for the dead-proxy path, and there is a test that +# reads the timeout out of hooks.json so the two cannot drift apart again. +if [ -x "${CLAUDE_PLUGIN_ROOT:-}/scripts/start-proxy.sh" ]; then + CONTEXT_GURU_HEALTH_BUDGET="${CONTEXT_GURU_HEALTH_BUDGET:-5}" \ + "${CLAUDE_PLUGIN_ROOT}/scripts/start-proxy.sh" >/dev/null 2>&1 || true + if curl -fsS --max-time 2 "http://127.0.0.1:${PORT}/healthz" >/dev/null 2>&1; then + exit 0 + fi +fi + +LOG="${TMPDIR:-/tmp}/context-guru-proxy-${PORT}.log" +# Same state directory the starter uses, so the command printed below writes its dashboard DB +# where the hook-started proxy would have, and not into the user's repository. +STATE_DIR="${XDG_STATE_HOME:-$HOME/.local/state}/context-guru" +cat </dev/null | awk '{print $2; exit}' +} + +if command -v "$BIN" >/dev/null 2>&1; then + have_path=$(command -v "$BIN") + have=$(installed_version "$have_path") + emit "path=${have_path}" + emit "version=${have:-unknown}" + # An explicit CONTEXT_GURU_VERSION means "I want that one" — honour it even when something is + # already installed. `latest` resolves below and is compared there. + if [ "$VERSION" != latest ] && [ "$VERSION" = "$have" ]; then + emit "result=present" + exit 0 + fi + if [ "$VERSION" = latest ] && [ -n "$have" ] && [ "${CONTEXT_GURU_UPGRADE:-}" != 1 ]; then + # Do not silently re-download on every install run; say what is there and how to move. + emit "result=present" + emit "note=set CONTEXT_GURU_UPGRADE=1 to check for and install a newer release" + exit 0 + fi + if [ -z "$have" ]; then + emit "note=the installed binary does not support --version; it predates the release channel" + fi + emit "note=upgrading from ${have:-unknown}" +fi + +case "$(uname -s)" in + Darwin) OS=darwin ;; + Linux) OS=linux ;; + *) die "unsupported_os_$(uname -s): build from source, see docs/get-started/quickstart-proxy.md" ;; +esac +case "$(uname -m)" in + x86_64|amd64) ARCH=amd64 ;; + arm64|aarch64) ARCH=arm64 ;; + *) die "unsupported_arch_$(uname -m)" ;; +esac +emit "platform=${OS}/${ARCH}" + +command -v curl >/dev/null 2>&1 || die "no_curl" + +# --- 2. release tarball ------------------------------------------------------------------- +if [ "$VERSION" = latest ]; then + # Resolve to a CONCRETE tag once, then use it for both the tarball and checksums.txt — the two + # must come from the same release, and two independent /latest/download follows could straddle a + # release published between them. + # + # The web redirect FIRST, not the API. `api.github.com` allows 60 requests/hour for unauthenticated + # callers, counted PER IP — so the budget is shared by everyone behind the same address: a corporate + # NAT, a CI fleet, a shared dev box. Exhausted, it answers 403, this resolution produced the empty + # string, and the script then reported `no_release_found: no published release yet`. That message is + # not just unhelpful, it is FALSE — it sent people off to build from source while a perfectly good + # release sat published. Observed on a corporate IP: `{"limit":60,"remaining":0,"used":60}` with + # v0.1.1 released and downloadable. + # + # The releases/latest web redirect carries no such budget and lands on /releases/tag/. + VERSION=$(curl -fsSLI -o /dev/null -w '%{url_effective}' \ + "https://github.com/${REPO}/releases/latest" 2>/dev/null) + VERSION="${VERSION##*/}" + # With no releases at all the redirect lands on /releases, so guard against taking that as a tag. + case "$VERSION" in + ''|releases|latest) VERSION="" ;; + esac + if [ -z "$VERSION" ]; then + # Only now the API, and report WHICH failure it was: "rate limited" and "no release" call for + # completely different actions, and conflating them is what made the old message misleading. + api=$(curl -sSL -w '\n%{http_code}' "https://api.github.com/repos/${REPO}/releases/latest" 2>/dev/null) + code=$(printf '%s' "$api" | tail -1) + VERSION=$(printf '%s' "$api" | sed -n 's/.*"tag_name": *"\([^"]*\)".*/\1/p' | head -1) + if [ -z "$VERSION" ]; then + case "$code" in + 403|429) die "github_rate_limited: GitHub's API is rate limited for this IP (60/hour, shared with everything behind the same address), so the latest version could not be resolved. This says NOTHING about whether a release exists. Wait for the window to reset, or set CONTEXT_GURU_VERSION=vX.Y.Z to skip resolution entirely." ;; + *) die "no_release_found: no published release for ${REPO} (HTTP ${code}); build from source or set CONTEXT_GURU_VERSION" ;; + esac + fi + fi +fi +NUM="${VERSION#v}" +TARBALL="context-guru_${NUM}_${OS}_${ARCH}.tar.gz" +BASE="https://github.com/${REPO}/releases/download/${VERSION}" +emit "version=${VERSION}" + +# try_source_build is distribution option 3 from the header comment, which the first version of +# this script documented and never implemented — on a machine that had Go 1.26.4 on PATH. +# +# It is a FALLBACK, not a path anyone is steered to: it needs a toolchain, which is the gate this +# whole change exists to remove. But when there is no downloadable asset and a toolchain is right +# there, refusing to use it is worse than using it. +# report_path emits the two facts every successful install owes the caller, from BOTH install +# paths. The `go install` fallback used to return straight out of the script, so a user who landed +# on it got no `on_path` line at all — and ~/.local/bin frequently is not on PATH. The install +# looked clean, then a LATER session's hook said "the proxy binary is not on PATH", with nothing +# connecting the two. install/SKILL.md reads on_path to warn them, so the skill was silent too. +report_path() { + emit "result=installed" + emit "path=${DEST}/${BIN}" + # Report — do not fix — a PATH that will not find it. Editing the user's shell rc is a bigger + # intrusion than this script is entitled to, and the skill can tell them in context. + case ":${PATH}:" in + *":${DEST}:"*) emit "on_path=true" ;; + *) emit "on_path=false" + emit "note=add ${DEST} to your PATH, or the session hook will not find the proxy" ;; + esac +} + +try_source_build() { + command -v go >/dev/null 2>&1 || return 1 + # "attempted", because this line is printed BEFORE the build runs: it appears even when the + # build then fails, and `result=` is what says whether anything was installed. + emit "fallback=go_install_attempted" + # CGO off: the binary is pure Go, and requiring a C toolchain here would reintroduce the gate. + # GOBIN does not need creating first: checked on Linux with Go 1.26.4 — `go install` creates a + # missing GOBIN directory itself, so the tarball path's `mkdir -p` is not needed here. + if CGO_ENABLED=0 GOBIN="$DEST" go install "github.com/${REPO}/cmd/context-guru-proxy@${VERSION}" 2>"$TMP/go.err"; then + emit "built_from=source" + report_path + return 0 + fi + emit "go_install_failed=$(tail -1 "$TMP/go.err" 2>/dev/null | tr -d '\n')" + return 1 +} + +TMP=$(mktemp -d) || die "no_tmpdir" +trap 'rm -rf "$TMP"' EXIT + +# The raw curl error used to reach stdout and break this script's "every fact is a key=value +# line" contract, which the calling skill parses. Keep curl quiet and report the failure as data. +if ! curl -fsSL -o "$TMP/$TARBALL" "$BASE/$TARBALL" 2>"$TMP/curl.err"; then + emit "download_url=$BASE/$TARBALL" + # A published tag with no assets reaches exactly here — the release exists, the artifact does + # not — which is what a pre-release repository looks like before the first build is attached. + if try_source_build; then + exit 0 + fi + die "download_failed: $BASE/$TARBALL (no asset for this platform, and no Go toolchain to build from source)" +fi + +# Checksum. The download is unsigned, there is no signature anywhere yet, and this script strips +# macOS quarantine from the file below — so this is the ONLY integrity check in the path. +# +# It is therefore fail-CLOSED, in every branch. The first version of this was fail-open: a missing +# or unfetchable checksums.txt printed one advisory line and installed anyway, which meant an +# unverified binary landed on a PATH directory and ran — a binary that then handles all of the +# user's LLM traffic and holds their API key. The comment above it said "a failure here is fatal, +# never a warning" while the code did the opposite. +# +# CONTEXT_GURU_INSECURE=1 exists for the one legitimate case (a local build served from a file +# path with no checksums file) and says what it is in its name. +verify_checksum() { + curl -fsSL -o "$TMP/checksums.txt" "$BASE/checksums.txt" 2>/dev/null || + die "checksum_unavailable: could not fetch $BASE/checksums.txt, so the download cannot be verified. Set CONTEXT_GURU_INSECURE=1 to install anyway (not recommended)." + want=$(awk -v f="$TARBALL" '$2 == f || $2 == "*"f {print $1}' "$TMP/checksums.txt" | head -1) + [ -n "$want" ] || + die "checksum_absent: $TARBALL is not listed in checksums.txt, so the download cannot be verified. Set CONTEXT_GURU_INSECURE=1 to install anyway (not recommended)." + if command -v sha256sum >/dev/null 2>&1; then + got=$(sha256sum "$TMP/$TARBALL" | awk '{print $1}') + else + got=$(shasum -a 256 "$TMP/$TARBALL" | awk '{print $1}') + fi + [ "$want" = "$got" ] || die "checksum_mismatch: expected $want got $got" + emit "checksum=verified" +} + +if [ "${CONTEXT_GURU_INSECURE:-}" = 1 ]; then + emit "checksum=SKIPPED_BY_CONTEXT_GURU_INSECURE" +else + verify_checksum +fi + +tar xzf "$TMP/$TARBALL" -C "$TMP" || die "untar_failed" + +# FIND the binary rather than assuming where it sits. +# +# The release archive wraps its contents in a directory (goreleaser `wrap_in_directory: true`), so +# the binary is at `/context-guru-proxy` and not at the root. It is wrapped for a +# reason worth not undoing: a flat archive plus the documented `tar xzf` with no `-C` overwrites the +# README.md and LICENSE of whatever directory the user is standing in. +# +# Searching handles both layouts, so this script does not break the next time the packaging changes +# — and the failure it avoids is the worst-placed one there is: a stranger's first install, reporting +# `binary_not_in_tarball`, which reads as a broken release rather than a moved file. +found=$(find "$TMP" -type f -name "$BIN" 2>/dev/null | head -1) +[ -n "$found" ] || die "binary_not_in_tarball: no $BIN anywhere in $TARBALL" + +mkdir -p "$DEST" || die "cannot_create_$DEST" +# Install to a temp name and RENAME into place, so the destination is never absent or partial. +# +# Not for the reason it was suggested: the review's premise was ETXTBSY on Linux when writing over +# a running binary, and that was tested on Linux and does NOT happen — coreutils `install` unlinks +# the destination first, so the upgrade succeeds. But that unlink is itself the window worth +# closing: between it and the new file appearing, a SessionStart hook firing in another project +# finds no binary and reports "not on PATH". rename(2) swaps the directory entry in one step, so +# there is no instant at which $DEST/$BIN does not exist. +install -m 755 "$found" "$DEST/$BIN.new" || die "install_failed_to_$DEST" +mv -f "$DEST/$BIN.new" "$DEST/$BIN" || die "install_failed_to_$DEST" + +# macOS: without this, the first run dies with "cannot be verified" and the evaluator concludes +# the project is broken. Notarization would remove the need and requires a paid Apple account. +if [ "$OS" = darwin ] && command -v xattr >/dev/null 2>&1; then + xattr -d com.apple.quarantine "$DEST/$BIN" 2>/dev/null || true + emit "quarantine=cleared" +fi + +report_path diff --git a/context-guru-plugin/scripts/settings.py b/context-guru-plugin/scripts/settings.py new file mode 100755 index 0000000..ee32494 --- /dev/null +++ b/context-guru-plugin/scripts/settings.py @@ -0,0 +1,443 @@ +#!/usr/bin/env python3 +"""Add or remove exactly ONE key in a Claude Code settings file: env.ANTHROPIC_BASE_URL. + +This is the deterministic half of the install. The skill decides WHICH file and what to do +about a conflict; this script does the edit and refuses to guess. + +Why a script rather than `jq` in the skill's prompt: the target file is the user's real +`settings.json`, holding their theme, model, permission rules, statusline and possibly their +own base URL. Every operation here is therefore conservative to the point of being boring: + +* the file is read, parsed, and written back whole — never patched textually; +* a timestamped backup is written BEFORE the file is touched, and its path is reported; +* an existing ANTHROPIC_BASE_URL that is not ours is a CONFLICT and exits non-zero, because + overwriting somebody's gateway or benchmark endpoint is not a thing to do quietly; +* anything unparseable is refused rather than replaced with a fresh file, which would throw + away settings the user cannot get back. + +Output is one `key=value` line per fact on stdout, so the skill can act on the result without +re-reading the file or parsing prose. + +Usage: + settings.py add --file PATH --url URL [--force] + settings.py remove --file PATH [--url URL] + settings.py show --file PATH +""" + +from __future__ import annotations + +import argparse +import datetime as _dt +import json +import os +import shutil +import sys +import tempfile + +KEY = "ANTHROPIC_BASE_URL" +# The second key, written only when the proxy has to chain behind an existing gateway. It lives in +# the same env block because that block is what the SessionStart hook inherits — see cmd_add. +UPSTREAM_KEY = "ANTHROPIC_UPSTREAM" +# The absolute path to the proxy, for machines where its directory is not on PATH — the hook +# resolves the binary by name, so on those machines this is what makes auto-start work at all. +BIN_KEY = "CONTEXT_GURU_BIN" + +# The keys this script owns. `other_env_keys` counts what the USER has in the env block, so it must +# exclude all three of ours — it excluded only the base URL, so a chained install reported 3 "other" +# keys where the user owned 1, and /context-guru:status repeated the wrong number back to them. +OURS = (KEY, UPSTREAM_KEY, BIN_KEY) + +# Where this script records what it did, so a later run can tell its own work from the user's. +META = "$context-guru" + + +def is_ours(data: dict, url: str) -> bool: + """Did WE write this base URL? Answered from a record, never from the URL's shape. + + The tempting version of this is a regex over loopback `/anthropic` URLs, and it is wrong in a + way a test caught: litellm's default is `http://127.0.0.1:4000/anthropic`, so "any local + /anthropic URL is ours" would make uninstall delete somebody else's routing. Two local proxies + are indistinguishable by URL — so instead `add` records the URL it wrote, and this reads it. + + A file with no record predates that (or was hand-edited), and then only an exact match against + the URL the caller passed counts. Fail toward leaving the user's configuration alone. + """ + meta = data.get(META) + if isinstance(meta, dict) and meta.get("installed_base_url"): + return url == meta["installed_base_url"] + return False + + +def emit(**facts: object) -> None: + for k, v in facts.items(): + print(f"{k}={v}") + + +def load(path: str) -> tuple[dict, bool]: + """Return (settings, existed). Refuses to proceed on anything it cannot parse.""" + if not os.path.exists(path): + return {}, False + with open(path, encoding="utf-8") as fh: + text = fh.read() + if not text.strip(): + return {}, True + try: + data = json.loads(text) + except json.JSONDecodeError as exc: + emit(result="error", reason="unparseable_json", detail=f"{exc}") + # Deliberately fatal. The alternative — treating a broken file as empty — would + # silently discard every setting in it. + sys.exit(3) + if not isinstance(data, dict): + emit(result="error", reason="not_an_object") + sys.exit(3) + return data, True + + +def backup(path: str) -> str: + """Copy `path` aside and return the copy's name. Never overwrites an existing backup. + + The stamp used to be second-granularity with a plain `copy2`, which meant an + install-then-uninstall round trip — well inside one second — wrote both backups to the SAME + filename, and the survivor held the POST-install state. The user was then told to keep that + path as their undo, and it was a copy of the change, not of what preceded it. + + Microseconds plus O_EXCL: the exclusive create is what actually guarantees it, since two + writes in the same microsecond are merely unlikely rather than impossible. + """ + stamp = _dt.datetime.now().strftime("%Y%m%d-%H%M%S-%f") + for attempt in range(100): + dest = f"{path}.context-guru-backup-{stamp}" + (f".{attempt}" if attempt else "") + try: + fd = os.open(dest, os.O_CREAT | os.O_EXCL | os.O_WRONLY, 0o600) + except FileExistsError: + continue + with os.fdopen(fd, "wb") as out, open(path, "rb") as src: + shutil.copyfileobj(src, out) + shutil.copystat(path, dest) + prune_backups(path) + return dest + raise RuntimeError(f"could not create a backup for {path}") + + +# How many backups of one settings file to keep. Each add and each remove writes one, so a user +# who installs and uninstalls a few times accumulated them forever in ~/.claude — 40 files after +# 20 cycles, in a directory they read by hand. +KEEP_BACKUPS = 10 + + +def prune_backups(path: str) -> None: + """Delete all but the newest KEEP_BACKUPS backups of `path`. Best effort.""" + import glob + + try: + # glob.escape on the PATH: `[`, `?` and `*` in a directory name are pattern syntax, so + # for a settings file under e.g. `~/projects/foo[1]/.claude/` this matched nothing and + # pruning silently did nothing forever — invisible, because pruning is best-effort by + # design, and the backups it exists to bound then accumulate without limit. + found = sorted(glob.glob(glob.escape(path) + ".context-guru-backup-*"), + key=os.path.getmtime) + except OSError: + return + for old in found[:-KEEP_BACKUPS]: + try: + os.remove(old) + except OSError: + pass + + +def save(path: str, data: dict) -> None: + """Write `data` to `path` atomically, preserving the file's identity and permissions. + + Three things here are each a defect that was found rather than anticipated: + + * **Follow symlinks.** A dotfile-managed `settings.json` is commonly a symlink into a + repository. `os.replace` onto the link path REPLACES THE LINK with a regular file, so the + edit silently never reaches the file the user actually manages and their dotfiles still + hold the old content. Resolve first, then write to the real path. + * **Preserve the mode.** The temp file is created fresh, so the replaced file's mode was + taken from the umask: a `600` settings file holding `ANTHROPIC_AUTH_TOKEN` came back + world-readable `644` under the common default umask. + * **Atomic.** An interrupted write must not leave half a settings file, which would break + every session in that scope rather than only ours. + """ + real = os.path.realpath(path) + os.makedirs(os.path.dirname(os.path.abspath(real)) or ".", exist_ok=True) + # 0600 from the instant the file exists, not from copymode() below. + # + # `open(tmp, "w")` creates with 0666 & ~umask — typically 0644 — and the mode was corrected + # only after the entire file had been written and flushed. For a settings.json holding + # ANTHROPIC_AUTH_TOKEN (the reason the mode is preserved at all, per the docstring above) that + # is a window in which the replacement sits world-readable on disk. Start private, then widen + # with copymode; never the other way round. + # + # mkstemp rather than O_EXCL on the fixed name `.context-guru-tmp`: O_EXCL would close the + # mode window too, but a leftover temp file from a crash or a full disk would then make every + # later save fail until somebody deleted it by hand — trading a mode window for a permanent + # lockout of the file this script exists to edit. A unique name has neither problem. + fd, tmp = tempfile.mkstemp(dir=os.path.dirname(os.path.abspath(real)) or ".", + prefix=os.path.basename(real) + ".context-guru-tmp-") + try: + with os.fdopen(fd, "w", encoding="utf-8") as fh: + json.dump(data, fh, indent=2, ensure_ascii=False) + fh.write("\n") + if os.path.exists(real): + shutil.copymode(real, tmp) + else: + os.chmod(tmp, 0o600) + os.replace(tmp, real) + except BaseException: + # Leave no litter on the failure paths — this directory is the user's ~/.claude. + try: + os.unlink(tmp) + except OSError: + pass + raise + + +def cmd_show(args: argparse.Namespace) -> int: + data, existed = load(args.file) + current = (data.get("env") or {}).get(KEY) + emit( + result="ok", + file=args.file, + exists=str(existed).lower(), + base_url=current if current else "(unset)", + other_env_keys=len([k for k in (data.get("env") or {}) if k not in OURS]), + top_level_keys=len(data), + ) + return 0 + + +def cmd_add(args: argparse.Namespace) -> int: + data, existed = load(args.file) + env = data.get("env") + if env is None: + env = {} + if not isinstance(env, dict): + emit(result="error", reason="env_not_an_object") + return 3 + + # What a COMPLETE install looks like in this file. Every exit below is judged against this whole + # set, not against the base URL alone. + # + # That distinction is the fix for a defect worth spelling out, because it defeated the remedy for + # every other failure in this flow. `unchanged` used to mean "the base URL matches", and the two + # early returns below wrote nothing else — so: + # + # add --url --upstream http://gw:4000 --bin /opt/cg/proxy + # -> result=unchanged, exit 0, and NEITHER new key written + # + # Re-running the install is the obvious thing to do after an attempt dies partway, which is how + # every hosted-agent attempt ended — and it was a no-op that reported success. There was no way to + # add the upstream to an already-routed project at all. The repointed path had the same hole, so + # changing the configured port silently un-chained the proxy. + desired = {KEY: args.url} + if getattr(args, "upstream", ""): + desired[UPSTREAM_KEY] = args.upstream + if getattr(args, "bin", ""): + desired[BIN_KEY] = args.bin + + current = env.get(KEY) + if current == args.url and all(env.get(k) == v for k, v in desired.items()): + emit(result="unchanged", file=args.file, base_url=current, + upstream=env.get(UPSTREAM_KEY, ""), bin=env.get(BIN_KEY, ""), + note="already routed to this proxy, with nothing left to add") + return 0 + if current == args.url: + # Routed already, but missing one of the other keys — the repair case. Fill in only what is + # absent or different, and say which, since "added" would misdescribe it. + saved = backup(args.file) if existed else "" + changed = [k for k, v in desired.items() if env.get(k) != v] + env.update(desired) + data["env"] = env + meta = data.setdefault(META, {}) + meta["installed_base_url"] = args.url + if getattr(args, "upstream", ""): + meta["installed_upstream"] = args.upstream + if getattr(args, "bin", ""): + meta["installed_bin"] = args.bin + save(args.file, data) + emit(result="completed", file=args.file, base_url=args.url, added_keys=",".join(changed), + upstream=env.get(UPSTREAM_KEY, ""), bin=env.get(BIN_KEY, ""), backup=saved, + note="already routed; filled in the keys that were missing") + return 0 + if current and is_ours(data, current) and not args.force: + # Our own URL on a different port — the user changed the configured port and re-ran. + # Reporting a conflict here told them somebody else owned their routing, which was + # wrong and alarming. Move it, and keep the note so the change is visible. + saved = backup(args.file) + env.update(desired) # the whole set: a port change must not drop the chaining keys + data["env"] = env + meta = data.setdefault(META, {}) + meta["installed_base_url"] = args.url + if getattr(args, "upstream", ""): + meta["installed_upstream"] = args.upstream + if getattr(args, "bin", ""): + meta["installed_bin"] = args.bin + save(args.file, data) + emit(result="repointed", file=args.file, base_url=args.url, previous=current, + upstream=env.get(UPSTREAM_KEY, ""), bin=env.get(BIN_KEY, ""), + backup=saved, note="this was our own URL on another port; moved") + return 0 + if current and not args.force: + # The one conflict this has to reason about. `env` blocks merge per key across + # scopes, so a user-scope install is not clobbered by a project that ships its own + # env block — what is left is a base URL the USER set, which may be their company + # gateway or a benchmark endpoint, and taking it over would break their setup while + # looking like it worked. + emit(result="conflict", file=args.file, existing=current, proposed=args.url, + note="ANTHROPIC_BASE_URL is already set here; ask before replacing it, " + "then re-run with --force") + return 2 + + saved = backup(args.file) if existed else "" + env[KEY] = args.url + # --upstream writes the SECOND key that chaining needs, in the same atomic save. + # + # Not scope creep: without it, chaining survives only as long as the proxy that is already + # running. A later session's SessionStart hook reads its configuration from this env block, so a + # missing ANTHROPIC_UPSTREAM means that hook starts a proxy aimed at api.anthropic.com — and on a + # platform whose gateway holds the credential and rewrites model names, every request in that + # session fails. The first real install on a hosted agent ended exactly here, with the skill + # telling the user to paste the key in by hand. + # + # Recorded in our own metadata as well, so uninstall removes only an upstream WE wrote. + if getattr(args, "upstream", ""): + env[UPSTREAM_KEY] = args.upstream + # --bin persists an ABSOLUTE path to the proxy, for machines where $DEST is not on PATH. + # + # `~/.local/bin` frequently is not, and on a hosted agent it is worse than an inconvenience: the + # writable directories there reset on pod restart, so "add it to your shell profile" is advice + # that does not survive. The SessionStart hook resolves the binary BY NAME, so without this the + # install succeeds, routing works for the session that set it up, and the auto-restart safety net + # silently never fires afterwards — and the failure mode it exists to catch is a hang with no + # error. start-proxy.sh already honours CONTEXT_GURU_BIN and accepts an absolute path, so the fix + # is to write it where the hook will inherit it rather than to ask for a PATH change. + if getattr(args, "bin", ""): + env[BIN_KEY] = args.bin + data["env"] = env + # Remember what we took over, so uninstall can hand it back. + # + # `replaced` used to be reported and then forgotten. After a --force install over somebody's + # own gateway, uninstall deleted the key and left them with NO base URL at all — and because + # the backup filename collided with the install's own, the copy that held it was gone too. + # Their setup was unrecoverable from anything the tool produced. + # Record what we wrote, so a re-run can recognise its own work, and what we took over, so + # uninstall can hand it back. + meta = data.setdefault(META, {}) + meta["installed_base_url"] = args.url + if getattr(args, "upstream", ""): + meta["installed_upstream"] = args.upstream + if getattr(args, "bin", ""): + meta["installed_bin"] = args.bin + if current: + meta["previous_base_url"] = current + save(args.file, data) + emit(result="added", file=args.file, base_url=args.url, + replaced=current if current else "", backup=saved or "(new file)", + other_env_keys=len([k for k in env if k not in OURS])) + return 0 + + +def cmd_remove(args: argparse.Namespace) -> int: + data, existed = load(args.file) + if not existed: + emit(result="unchanged", file=args.file, note="no such file") + return 0 + env = data.get("env") + if not isinstance(env, dict) or KEY not in env: + emit(result="unchanged", file=args.file, note=f"no env.{KEY} here") + return 0 + current = env[KEY] + # Ours is the URL passed in, or the one we recorded at install time — which covers the case + # where the configured port changed since. It is NOT "any loopback /anthropic URL": litellm's + # default is one of those, and uninstall must not delete somebody else's routing. + # + # This check is UNCONDITIONAL, and that is the fix for the worst defect this script has had. + # It used to read `if args.url and ...`, so omitting --url — an invocation this module's own + # docstring advertises as supported — skipped the check entirely and deleted whatever was + # there. Measured against a corporate gateway with no context-guru record: `result=removed`, + # `restored=` empty, exit 0. The user's gateway was gone and the exit code said success. + # + # What made that severe was not the branch, it was WHERE the safety lived: uninstall/SKILL.md + # passes --url and its prose said that flag "is what keeps this safe". So the property + # protecting the user's configuration depended on a model remembering a flag in a prompt. + # This script is the deterministic half precisely so that it does not have to. + # + # With no --url, `current != args.url` is trivially true and is_ours() decides alone — i.e. + # remove only what we recorded installing, which is what is_ours() is documented to be for. + # The escape hatch for a record we never wrote (hand-edited settings, or an install predating + # the record) is to pass --url naming the URL to delete, which the skill already does. + if current != args.url and not is_ours(data, current): + # Refuse to remove a base URL that is not ours: the user may have pointed this at + # something else since, and uninstall must not take that with it. + emit(result="conflict", file=args.file, existing=current, expected=args.url, + note="this base URL is not the one context-guru installed; left untouched") + return 2 + saved = backup(args.file) + del env[KEY] + # Take our upstream key with it, but ONLY the value we recorded writing. An ANTHROPIC_UPSTREAM + # the user set themselves is theirs, and uninstall removing it would be the same class of + # overreach as deleting a base URL we never installed. + recorded_upstream = "" + _meta = data.get(META) + if isinstance(_meta, dict): + recorded_upstream = _meta.get("installed_upstream") or "" + if recorded_upstream and env.get(UPSTREAM_KEY) == recorded_upstream: + del env[UPSTREAM_KEY] + recorded_bin = "" + if isinstance(_meta, dict): + recorded_bin = _meta.get("installed_bin") or "" + if recorded_bin and env.get(BIN_KEY) == recorded_bin: + del env[BIN_KEY] + # Put back whatever we took over at install time. Deleting the key was leaving a user who had + # a gateway configured with nothing at all — a worse state than before they installed. + restored = "" + meta = data.get(META) + if isinstance(meta, dict): + if meta.get("previous_base_url"): + restored = meta["previous_base_url"] + env[KEY] = restored + # Our bookkeeping goes with our key: leaving it behind would make a later install think it + # had written a URL it did not. + meta.pop("previous_base_url", None) + meta.pop("installed_base_url", None) + meta.pop("installed_upstream", None) + meta.pop("installed_bin", None) + if not meta: + data.pop(META, None) + # Leave no litter: an `env: {}` we created is removed with the key. An env block that + # still holds the user's own variables stays exactly as it is. + if not env: + del data["env"] + else: + data["env"] = env + save(args.file, data) + emit(result="removed", file=args.file, was=current, backup=saved, + restored=restored, env_block_left=str(bool(env)).lower()) + return 0 + + +def main() -> int: + ap = argparse.ArgumentParser(description=__doc__) + sub = ap.add_subparsers(dest="cmd", required=True) + for name in ("add", "remove", "show"): + p = sub.add_parser(name) + p.add_argument("--file", required=True) + p.add_argument("--url", default="") + p.add_argument("--force", action="store_true") + p.add_argument("--bin", default="", + help="also write env.CONTEXT_GURU_BIN (absolute path to the proxy), for " + "machines where its directory is not on PATH") + p.add_argument("--upstream", default="", + help="also write env.ANTHROPIC_UPSTREAM, so the proxy chains behind an " + "existing gateway in LATER sessions too (the hook reads this block)") + args = ap.parse_args() + if args.cmd == "add" and not args.url: + ap.error("add needs --url") + return {"add": cmd_add, "remove": cmd_remove, "show": cmd_show}[args.cmd](args) + + +if __name__ == "__main__": + sys.exit(main()) diff --git a/context-guru-plugin/scripts/start-proxy.sh b/context-guru-plugin/scripts/start-proxy.sh new file mode 100755 index 0000000..21ec30d --- /dev/null +++ b/context-guru-plugin/scripts/start-proxy.sh @@ -0,0 +1,288 @@ +#!/usr/bin/env bash +# SessionStart hook: make sure the proxy this project routes to is actually listening. +# +# Five properties, each of which is a way this can be wrong: +# +# 1. IT SELF-GATES ON $ANTHROPIC_BASE_URL. The plugin installs at USER scope, so this hook +# runs in every project the user has — including every project they never routed, where +# starting a proxy is pure waste. Settings `env` values are written into the process +# environment and hook processes inherit it, so the variable naming our port IS the +# per-project enablement signal. No second config to keep in sync, and it degrades +# correctly: delete the env key by hand and this hook stops doing anything, by itself. +# +# 2. IT IS IDEMPOTENT. SessionStart also fires on `clear`, `compact`, `resume` and `fork`, not +# just `startup` — a long session re-fires it repeatedly. So: probe /healthz, and start +# something only if nothing answers. +# +# 3. IT IS SYNCHRONOUS. The hook is deliberately NOT marked async: it returns only once +# /healthz answers, which is what closes the race with the session's first API request. +# A request that beats the proxy up gets a connection error, and Claude Code's retry does +# not make that invisible. +# +# 4. THE PORT IS FIXED, not negotiated. The URL in settings was written before this ran and +# cannot be renegotiated, so both sides read the same configured value. +# +# 5. IT NEVER FAILS THE SESSION. Every exit is 0. A proxy that will not start must leave the +# user with a working Claude Code and a note about it — the alternative is a plugin that +# can brick every session on the machine, which is the biggest risk in this whole feature. +set -uo pipefail + +PORT="${CLAUDE_PLUGIN_OPTION_PORT:-8787}" +PRESET="${CLAUDE_PLUGIN_OPTION_PRESET:-cache}" +IDLE_EXIT="${CLAUDE_PLUGIN_OPTION_IDLE_EXIT:-24h}" +BIN="${CONTEXT_GURU_BIN:-context-guru-proxy}" +LOG="${TMPDIR:-/tmp}/context-guru-proxy-${PORT}.log" +HEALTH="http://127.0.0.1:${PORT}/healthz" + +note() { printf 'context-guru: %s\n' "$*"; } + +# --- (1) the gate ------------------------------------------------------------------------ +# Match the port, not merely the word "localhost": a user routing to a DIFFERENT local proxy +# (litellm, their own gateway) must not have ours started underneath them. +# The trailing "/" is load-bearing: without it this is a PREFIX match, so PORT=8787 also matches +# a URL on 87871 -- and this hook would start our proxy on 8787 underneath a user routed to a +# different local proxy on that port, which is the exact case this gate exists to prevent. Every +# base URL we write ends in /anthropic, so the delimiter is always there to match. +# CONTEXT_GURU_FORCE=1 starts the proxy without routing being configured yet. +# +# The install needs the proxy up BEFORE it writes the routing key, so at that moment the gate below +# is false by definition. The skill used to satisfy it by prefixing the invocation with +# `ANTHROPIC_BASE_URL="http://127.0.0.1:/anthropic"`, and that turned out to be a bad idea for +# two independent reasons: +# +# * Claude Code's auto-mode classifier denied the command as [Traffic Redirection] — reasonably, +# since the command text literally repoints ANTHROPIC_BASE_URL at a local interceptor. Observed +# on a hosted agent, and it blocked the install at the last step. +# * Bash permission rules match by command PREFIX, so an env-prefixed command cannot be covered by +# a rule naming this script. Users could not grant it even if they wanted to. +# +# So the install asks for what it means — "start the proxy" — and nothing in the command line +# reassigns the variable that routes traffic. +# --force as an ARGUMENT, not only an environment variable. +# +# Bash permission rules match by command PREFIX, so `FOO=1 /path/to/start-proxy.sh` cannot be covered +# by any rule naming this script — the command does not begin with it. Every env-prefixed form is +# therefore ungrantable: a user who wants to approve "this plugin may start its proxy" has no way to +# say so. As an argument it is `