diff --git a/.github/workflows/ci.yaml b/.github/workflows/ci.yaml index 95e5a1e1..e3bb0623 100644 --- a/.github/workflows/ci.yaml +++ b/.github/workflows/ci.yaml @@ -17,8 +17,10 @@ jobs: - uses: actions/checkout@v4 - uses: actions/setup-go@v5 with: - go-version: '1.25' - check-latest: true + # go.mod is the single source of truth for the toolchain. Pinning a literal here is + # what let CI validate on 1.25 while go.mod declared 1.26.4 and releases built on + # 1.26 (#152) — three numbers that have to agree and no mechanism making them. + go-version-file: go.mod - name: Lint run: make lint - name: Test & coverage @@ -54,8 +56,10 @@ jobs: - uses: actions/checkout@v4 - uses: actions/setup-go@v5 with: - go-version: '1.25' - check-latest: true + # go.mod is the single source of truth for the toolchain. Pinning a literal here is + # what let CI validate on 1.25 while go.mod declared 1.26.4 and releases built on + # 1.26 (#152) — three numbers that have to agree and no mechanism making them. + go-version-file: go.mod - name: Build with no C compiler available run: | go build -o /tmp/cg-purego ./cmd/context-guru-proxy diff --git a/.github/workflows/release.yaml b/.github/workflows/release.yaml new file mode 100644 index 00000000..e93fc89e --- /dev/null +++ b/.github/workflows/release.yaml @@ -0,0 +1,91 @@ +name: Release + +# Tag-driven, so a release is something a maintainer does on purpose. The `workflow_dispatch` +# entry builds the same matrix WITHOUT publishing (snapshot mode), which is how the release +# path gets exercised before there is a tag to regret. +on: + push: + tags: ["v*"] + workflow_dispatch: + +permissions: + contents: read + +jobs: + release: + runs-on: ubuntu-latest + permissions: + # Only the tag path publishes, and only this job needs the write. + contents: write + steps: + - uses: actions/checkout@v4 + with: + # GoReleaser's changelog needs the history the default shallow clone does not have. + fetch-depth: 0 + - uses: actions/setup-go@v5 + with: + # Same source as CI (go.mod), deliberately: an artifact people download must be built + # with the toolchain CI validated, and a literal here is how that drifts apart. + go-version-file: go.mod + + # The claim the release rests on, asserted in CI rather than trusted: the shipped + # binary needs no C toolchain. CGO_ENABLED=0 with no compiler on PATH would fail loudly + # here if a cgo dependency ever escaped the cg_skeleton build tag — which is exactly the + # regression that would otherwise be discovered by an evaluator, at install time. + - name: Assert the binary is pure Go + env: + CGO_ENABLED: "0" + CC: /nonexistent-c-compiler + run: | + go build -o /tmp/cg-purego ./cmd/context-guru-proxy + file /tmp/cg-purego | tee /dev/stderr | grep -q "statically linked" + # And it has to actually start, not just link. + /tmp/cg-purego --listen 127.0.0.1:4471 --preset cache & + for _ in $(seq 1 40); do + sleep 0.25 + curl -fsS http://127.0.0.1:4471/healthz && break + done + curl -fsS http://127.0.0.1:4471/healthz | grep -q ok + # An installer asks the binary what it is; make sure it can answer. + /tmp/cg-purego --version | tee /dev/stderr | grep -q context-guru-proxy + + # Nothing tested the configuration we actually SHIP. + # + # ci.yaml runs the suite only with CGO_ENABLED=1, and a tag push previously published + # without running any tests at all. So the one guard that matters most to a released + # artifact — TestEveryPresetBuilds, which catches a preset naming a component that is not + # registered in a CGO-free binary — was never executed in the CGO-free configuration. That + # is exactly the `preset: coding` / `unknown component "skeleton"` failure, in a build no + # developer runs locally. + # + # The race detector needs cgo, so this cannot be the whole suite; it is the packages whose + # behaviour depends on which components are compiled in. + - name: Test the shipped configuration (CGO off, no race detector) + env: + CGO_ENABLED: "0" + run: go test ./config/... ./components/... ./apply/... ./proxy/... ./store/... + + # A tag must not publish something the full suite has not seen. + - name: Full test suite + env: + CGO_ENABLED: "1" + run: go test ./... + + - name: Release + uses: goreleaser/goreleaser-action@v6 + with: + version: "~> v2" + # A tag publishes; a manual run builds the full matrix and publishes nothing. + args: ${{ startsWith(github.ref, 'refs/tags/v') && 'release --clean' || 'release --clean --snapshot' }} + env: + GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} + + - name: Upload snapshot artifacts + if: ${{ !startsWith(github.ref, 'refs/tags/v') }} + uses: actions/upload-artifact@v4 + with: + name: snapshot-dist + path: | + dist/*.tar.gz + dist/checksums.txt + retention-days: 7 diff --git a/.goreleaser.yaml b/.goreleaser.yaml new file mode 100644 index 00000000..8b568a1a --- /dev/null +++ b/.goreleaser.yaml @@ -0,0 +1,98 @@ +# GoReleaser: the release artifacts an evaluator downloads instead of installing a toolchain. +# +# The whole file is a plain GOOS/GOARCH matrix with no C cross-toolchains, no zig, and no +# libc coupling, because `CGO_ENABLED=0` builds the shipped binary. That is verified rather +# than assumed — the `Assert the binary is pure Go` step in .github/workflows/release.yaml fails +# the release if a cgo dependency ever escapes the cg_skeleton build tag. Measured directly on +# go 1.26.4: +# all four targets build, the artifact is 27–34 MB stripped, `file` reports "statically +# linked" and `ldd` "not a dynamic executable", and the resulting binary serves /healthz. +# +# `cg_skeleton` is the ONE thing that needs cgo (tree-sitter), and it is deliberately not +# built here: it is in no default preset, not in the cache story, and shipping it would mean +# per-platform C cross-compilation for a component this funnel never runs. Source build is +# documented in docs/components/skeleton.md. +# +# There is no `brews:` block yet — the tap repo and release signing are an open ownership +# question (spec §"Open questions", 3). Until it is answered the funnel installs from the +# release tarball, so nothing here depends on a repo that does not exist. Adding the tap +# later is additive and changes none of the below. +version: 2 + +project_name: context-guru + +before: + hooks: + - go mod download + +builds: + - id: context-guru-proxy + main: ./cmd/context-guru-proxy + binary: context-guru-proxy + env: + # The point of the whole file. Not inherited from the Makefile, which sets + # CGO_ENABLED=1 because `go test -race` needs it — a test-time requirement that was + # being read as a shipping requirement. + - CGO_ENABLED=0 + flags: + # Reproducible paths in panics, and no VCS stamping (the checkout is shallow in CI). + - -trimpath + - -buildvcs=false + ldflags: + # Same two symbols the Makefile stamps, so `/stats` build_version is populated in a + # released binary exactly as it is in a locally built one. + - -s -w + - -X github.com/rossoctl/context-guru/internal/buildinfo.Version={{ .Version }} + - -X github.com/rossoctl/context-guru/internal/buildinfo.Commit={{ .ShortCommit }} + goos: [linux, darwin] + goarch: [amd64, arm64] + +archives: + - id: default + ids: [context-guru-proxy] + # An evaluator untars this into ~/.local/bin, so the archive name is what they see and + # the binary inside must be the plain name with no version in it. + name_template: >- + {{ .ProjectName }}_{{ .Version }}_{{ .Os }}_{{ .Arch }} + formats: [tar.gz] + files: + - LICENSE + - README.md + - THIRD-PARTY-NOTICES + +checksums: + # scripts/install.sh verifies the downloaded tarball against this file. macOS quarantines + # an unsigned download and the installer strips the attribute, so a checksum is the only + # integrity check left in that path — it is not optional decoration. + name_template: checksums.txt + algorithm: sha256 + +snapshot: + version_template: "{{ incpatch .Version }}-next" + +changelog: + use: github + sort: asc + filters: + exclude: + - "^docs:" + - "^test:" + - "^chore:" + - "^ci:" + +release: + prerelease: auto + footer: | + ## Install + + No Go toolchain and no C compiler are needed — the binary is statically linked. + + Download the tarball for your platform, untar it, and put `context-guru-proxy` on your + `PATH`: + + ``` + tar xzf context-guru_*_darwin_arm64.tar.gz + install -m 755 context-guru-proxy ~/.local/bin/ + ``` + + Then see `docs/get-started/quickstart-proxy.md`. diff --git a/README.md b/README.md index 3fd159ec..2d1db4fc 100644 --- a/README.md +++ b/README.md @@ -108,6 +108,9 @@ docker build -t context-guru:local . ## Quickstart (60 seconds) +Download a release binary — statically linked, **no Go and no C compiler needed** — or build +from source: + ```sh # 1 — run the proxy (ships with the SWE-bench-winning cache-aware config by default) ./bin/context-guru-proxy # --preset house (the default); listens on :4000 @@ -144,6 +147,8 @@ See [docs/components.md](docs/components.md) and [docs/reference/presets.md](doc | Flag / env | Default | Purpose | |---|---|---| | `--preset` / `PRESET` | `house` | pipeline preset when no `--config` | +| `--idle-exit` / `IDLE_EXIT` | `0` (never) | exit after this long unused; floor `max(2 × store.ttl_seconds, 1h)`, refused with `--upstreams` | +| `--version` | — | print version and commit, then exit | | `--config` / `CONFIG` | — | YAML config (overrides preset) | | `LISTEN_ADDR` | `:4000` | listen address | | `--anthropic-upstream` / `ANTHROPIC_UPSTREAM` | `https://api.anthropic.com` | Anthropic upstream base | diff --git a/cmd/context-guru-proxy/idleexit.go b/cmd/context-guru-proxy/idleexit.go new file mode 100644 index 00000000..023afc13 --- /dev/null +++ b/cmd/context-guru-proxy/idleexit.go @@ -0,0 +1,172 @@ +package main + +import ( + "fmt" + "net/http" + "sync/atomic" + "time" + + "github.com/rossoctl/context-guru/store" +) + +// Idle-exit: a proxy that a Claude Code session started should not outlive the machine's use +// of it. +// +// The funnel installs a SessionStart hook that starts the proxy on demand, so nothing has to +// be left running — but only if the process eventually goes away on its own. That is all this +// is: a clock, a probe, and the SAME graceful shutdown path SIGTERM takes. No new teardown +// logic, because the teardown is the part that is already right (armShutdown releases the +// dashboard's SSE connections, the deferred closes flush the capture batch). +// +// It is OFF unless asked for. A gateway deployment or an eval-containers run must never +// self-terminate, and "the proxy vanished overnight" is a much worse failure there than a +// process left running on a laptop. +// +// Two things make this less trivial than a timeout, and both are load-bearing: +// +// 1. **The keep-alive inverts "idle".** Pinging is what the proxy does WHILE no client +// traffic arrives, so a watchdog that watches requests alone kills the feature in +// precisely its working window. Hence the pending probe below, which both blocks exit and +// resets the clock. +// 2. **Exit wipes the in-memory store.** A threshold shorter than the store's entry lifetime +// drops live frozen decisions and re-bills their prefix at cache-creation prices. That is +// refused at startup, not documented — see store.ValidateIdleExit. + +// activityClock is the last moment this process did something a user would call "in use". +// Nanoseconds in an atomic so the request path pays one store and no lock. +type activityClock struct{ ns atomic.Int64 } + +func (a *activityClock) touch(now time.Time) { a.ns.Store(now.UnixNano()) } +func (a *activityClock) last() time.Time { return time.Unix(0, a.ns.Load()) } + +// probeRoutes are the paths that do NOT count as use. +// +// They are what a machine asks, not what a person or an agent does: a Kubernetes liveness +// probe, a Prometheus scrape, a `curl /healthz` in a monitoring loop, and the session hook's +// own start-up check. Counting them was a bug that disabled the whole feature rather than +// weakening it — measured: a proxy with a 1h threshold logged +// `idle-exit armed after=1h0m0s`, then reported `idle for 1h3m0s` after 2h03m of wall clock, +// because a /healthz poller had been stamping the clock for the first hour. Any probe on a +// schedule shorter than the threshold means the exit NEVER fires, and logs nothing to say so. +// +// Everything else still counts, including the dashboard's own polling: a person with the +// dashboard open is using this process, and exiting under them is a worse failure than a +// process left running. That is a deliberate asymmetry — a probe is not a viewer. +var probeRoutes = map[string]bool{ + "/healthz": true, + "/metrics": true, +} + +// stampActivity records a request as activity, unless its route is a machine probe. +// +// The stamp happens BEFORE the handler runs, so a long streaming response cannot age out while +// it is still being served — its own duration is not idleness. (The stream also cannot be cut +// off mid-flight regardless: srv.Shutdown waits for in-flight requests, which is why this +// reuses that path rather than calling os.Exit.) +func stampActivity(next http.Handler, act *activityClock, now func() time.Time) http.Handler { + return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if !probeRoutes[r.URL.Path] { + act.touch(now()) + } + next.ServeHTTP(w, r) + }) +} + +// idleExitOptions is everything watchIdle needs, with the clock and the ticker injected so +// the policy is testable without waiting out a real threshold. +type idleExitOptions struct { + // threshold is how long the proxy must be unused before it exits. + threshold time.Duration + // act is stamped by stampActivity on every request. + act *activityClock + // pending reports work that must keep the process alive even with no requests: keep-alive + // sessions with a ping still ahead of them. nil means "nothing pending, ever". + pending func() int + now func() time.Time + // tick drives the check. Production uses a ticker at a fraction of the threshold; the + // resolution only bounds how late the exit is, never how early. + tick <-chan time.Time + // stop abandons the watch (the process is shutting down for another reason). + stop <-chan struct{} +} + +// watchIdle blocks until the proxy has been idle for the whole threshold, and returns a +// human-readable reason for the log. ok is false when the watch was abandoned via stop. +// +// Pending keep-alive work does not merely veto the exit, it RESETS the clock. Vetoing alone +// would exit the instant the last ping retired, taking the store with it at the moment a +// session is most likely to come back — the quiet gap after `end_turn` is where the pings +// were aimed in the first place. Treating a pending ping as activity gives the session a full +// threshold of grace after its last one. +func watchIdle(o idleExitOptions) (string, bool) { + if o.now == nil { + o.now = time.Now + } + // A BACKSTOP, not the real seed. The caller stamps the clock at launch (main), which is + // the only place that knows when "launch" was; seeding here would date the clock from + // whenever this goroutine happened to get scheduled, which is both later and unknowable. + // It stays because the failure mode of an unstamped clock is the worst one available — a + // zero clock reads as "idle since 1970" and exits on the first tick. + if o.act.last().UnixNano() == 0 { + o.act.touch(o.now()) + } + for { + select { + case <-o.stop: + return "", false + case <-o.tick: + now := o.now() + if o.pending != nil { + if n := o.pending(); n > 0 { + o.act.touch(now) + continue + } + } + if idle := now.Sub(o.act.last()); idle >= o.threshold { + return "idle for " + idle.Round(time.Second).String() + + " (--idle-exit " + o.threshold.String() + ")", true + } + } + } +} + +// idleCheckInterval is how often the watchdog looks. A twentieth of the threshold keeps the +// exit within 5% of what was asked for, clamped so a 24h default does not mean an hour of +// slack and a 1h floor does not mean a check every three minutes. +func idleCheckInterval(threshold time.Duration) time.Duration { + d := threshold / 20 + if d < 30*time.Second { + d = 30 * time.Second + } + if d > 5*time.Minute { + d = 5 * time.Minute + } + return d +} + +// checkIdleExit is every reason a requested idle-exit threshold must not start. +// +// A function rather than two inline `if`s in main so both refusals are testable: they are +// startup-fatal, which is the one class of check where "it looked right" is the only evidence +// anyone ever gathers. +func checkIdleExit(d time.Duration, upstreamsPath string, o store.Options) error { + // The floor. Exiting clears the in-memory store, and losing a live frozen decision re-bills + // its whole prefix as cache creation — the 11.5x regression FrozenLost exists to catch. + if err := store.ValidateIdleExit(d, o); err != nil { + return err + } + if d > 0 && upstreamsPath != "" { + // A self-terminating GATEWAY is a different kind of wrong: --upstreams means this + // process serves other people's agents, where "the proxy vanished overnight" is far + // worse than a process left running on a laptop. + // + // The safety used to be accidental — it held only because a hosted deployment also runs + // a liveness probe, and every probe stamped the activity clock. That is no longer true + // (probeRoutes above deliberately excludes them), so what was accidentally safe is now + // explicitly refused rather than quietly reintroduced. + return fmt.Errorf("--idle-exit cannot be combined with --upstreams: a gateway serving " + + "other people's agents must not self-terminate. Drop --idle-exit, or run this " + + "instance without --upstreams") + } + return nil +} diff --git a/cmd/context-guru-proxy/idleexit_test.go b/cmd/context-guru-proxy/idleexit_test.go new file mode 100644 index 00000000..9ecd36f0 --- /dev/null +++ b/cmd/context-guru-proxy/idleexit_test.go @@ -0,0 +1,329 @@ +package main + +import ( + "net/http" + "net/http/httptest" + "strings" + "sync/atomic" + "testing" + "time" + + "github.com/rossoctl/context-guru/store" +) + +// The shipped idle-exit default is 24h, so these tests drive a hand-advanced clock and a +// hand-fed ticker instead of waiting. watchIdle reads the time from o.now() and treats a tick +// purely as "look now", so the value carried on the channel is irrelevant and a tick that +// arrives late still evaluates against the current fake clock. + +type fakeClock struct{ ns atomic.Int64 } + +func newFakeClock(t time.Time) *fakeClock { + c := &fakeClock{} + c.ns.Store(t.UnixNano()) + return c +} +func (c *fakeClock) now() time.Time { return time.Unix(0, c.ns.Load()) } +func (c *fakeClock) advance(d time.Duration) { c.ns.Add(int64(d)) } + +// watcher drives one watchIdle and reports its verdict. +// +// Two things here are deliberate, and both were bugs first: +// +// - **The tick channel is UNBUFFERED.** With a buffer, the first send succeeds against the +// buffer whether or not the watcher goroutine has been scheduled at all — so a test could +// advance its clock believing the watcher had already started, and then measure idleness +// from the wrong instant. Unbuffered makes a send a rendezvous: it completes only once the +// watcher has actually received it. +// - **Every interaction selects on the result channel too.** The moment the watcher exits it +// stops draining ticks, and an unconditional send then blocks until the test deadline — +// a hang, which tells you nothing, rather than a failure. +type watcher struct { + t *testing.T + tick chan time.Time + res chan string + clk *fakeClock +} + +func start(t *testing.T, clk *fakeClock, o idleExitOptions) *watcher { + return startWith(t, clk, o, false) +} + +// startWith exposes the one knob start hides: seedAtLaunch=true leaves the activity +// clock unstamped, so watchIdle's own backstop is what gets tested. +func startWith(t *testing.T, clk *fakeClock, o idleExitOptions, seedAtLaunch bool) *watcher { + t.Helper() + w := &watcher{t: t, tick: make(chan time.Time), res: make(chan string, 1), clk: clk} + o.tick = w.tick + o.now = clk.now + // Stamp the clock the way main does at launch, unless the test is specifically exercising + // the unstamped case. + if !seedAtLaunch { + o.act.touch(clk.now()) + } + go func() { + reason, ok := watchIdle(o) + if !ok { + reason = "" // abandoned via stop + } + w.res <- reason + }() + // Synchronise before returning, with a real tick rather than a sleep: on an unbuffered + // channel a completed send proves the watcher is running and has reached its select, so a + // clock the test advances afterwards cannot be mistaken for the launch time. + // + // It doubles as an assertion: at zero elapsed time nothing may exit. + if verdict, done := w.poke(); done { + t.Fatalf("watchIdle exited (%q) on its first look, with no time elapsed", verdict) + } + return w +} + +// poke delivers one tick, or reports the verdict if the watcher has already finished. +func (w *watcher) poke() (verdict string, done bool) { + w.t.Helper() + select { + case r := <-w.res: + return r, true + case w.tick <- w.clk.now(): + return "", false + case <-time.After(3 * time.Second): + w.t.Fatal("watchIdle is neither consuming ticks nor returning") + return "", true + } +} + +// mustNotExit checks the watcher evaluated the current clock and stayed alive. +// +// It pokes TWICE on purpose: the tick channel holds one, so a second successful send proves +// the first was consumed and the loop came back for more, rather than merely sitting in the +// buffer unexamined. Without that, "no exit" could just mean "never looked". +func (w *watcher) mustNotExit(what string) { + w.t.Helper() + for i := 0; i < 2; i++ { + if verdict, done := w.poke(); done { + w.t.Fatalf("%s: watchIdle exited (%q) when it must not", what, verdict) + } + } +} + +// mustExit gives the watcher a bounded number of looks to decide it is idle. +func (w *watcher) mustExit(what string) string { + w.t.Helper() + for i := 0; i < 4; i++ { + if verdict, done := w.poke(); done { + if verdict == "" { + w.t.Fatalf("%s: the watch was abandoned instead of exiting", what) + } + return verdict + } + } + w.t.Fatalf("%s: idle past the threshold, but watchIdle never exited", what) + return "" +} + +// TestIdleExitFiresWhenNothingIsHappening is the base case the feature exists for: a proxy a +// session started, and then nobody used, goes away by itself instead of being left on the +// evaluator's machine. +func TestIdleExitFiresWhenNothingIsHappening(t *testing.T) { + clk := newFakeClock(time.Unix(1_700_000_000, 0)) + w := start(t, clk, idleExitOptions{threshold: time.Hour, act: &activityClock{}, + pending: func() int { return 0 }, stop: make(chan struct{})}) + + clk.advance(30 * time.Minute) + w.mustNotExit("half a threshold") + + clk.advance(31 * time.Minute) + t.Logf("exit reason: %s", w.mustExit("past the threshold")) +} + +// TestIdleExitWaitsForAPendingKeepAlivePing is the case a naive watchdog gets wrong. +// +// The keep-alive INVERTS the meaning of idle: pinging is what the proxy does precisely while +// no client traffic is arriving — the quiet gap after `end_turn`, where 83.7% of the +// recoverable dollars sit. A watchdog counting requests alone would kill the process in +// exactly the window the feature was built for. +// +// Two properties, the second subtler than the first: +// +// 1. a pending ping VETOES the exit, however long the client silence; +// 2. it also RESETS the clock, so retiring the last ping does not exit moments later — it +// buys a full fresh threshold. Veto-only would drop the in-memory store at the instant +// the session is most likely to come back, which is the cache-write regression the floor +// and this whole feature are meant to avoid. +func TestIdleExitWaitsForAPendingKeepAlivePing(t *testing.T) { + clk := newFakeClock(time.Unix(1_700_000_000, 0)) + var pending atomic.Int64 + pending.Store(1) + w := start(t, clk, idleExitOptions{threshold: time.Hour, act: &activityClock{}, + pending: func() int { return int(pending.Load()) }, stop: make(chan struct{})}) + + // (1) Veto: two full thresholds of silence with a ping still scheduled. + clk.advance(2 * time.Hour) + w.mustNotExit("a keep-alive ping is still scheduled") + + // (2) The ping retires. If the veto reset the clock, a threshold measured from the START + // is not enough — only 30m have passed since the last pending observation. + pending.Store(0) + clk.advance(30 * time.Minute) + w.mustNotExit("30m after the last ping retired") + + clk.advance(31 * time.Minute) + w.mustExit("genuinely idle for a whole threshold") +} + +// TestRequestsDeferIdleExit covers the stamping half: a real request is use, and use defers the +// exit. +func TestRequestsDeferIdleExit(t *testing.T) { + clk := newFakeClock(time.Unix(1_700_000_000, 0)) + act := &activityClock{} + stampedBeforeHandler := false + h := stampActivity(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + // The stamp must land BEFORE the handler runs, so a long streaming response cannot + // age out while it is still being served. + stampedBeforeHandler = act.last().Equal(clk.now()) + w.WriteHeader(http.StatusOK) + }), act, clk.now) + + w := start(t, clk, idleExitOptions{threshold: time.Hour, act: act, + pending: func() int { return 0 }, stop: make(chan struct{})}) + + clk.advance(50 * time.Minute) + h.ServeHTTP(httptest.NewRecorder(), httptest.NewRequest("POST", "/anthropic/v1/messages", nil)) + if !stampedBeforeHandler { + t.Fatal("stampActivity did not record the request before invoking the handler") + } + // 80m since launch, but only 30m since the request. + clk.advance(30 * time.Minute) + w.mustNotExit("30m after serving a request") + + clk.advance(31 * time.Minute) + w.mustExit("an hour after the last request") +} + +// TestIdleExitStopAbandonsTheWatch: when the process is already shutting down for another +// reason (SIGTERM), the watchdog must let go rather than hold a goroutine and push a second +// reason into the shutdown path. +func TestIdleExitStopAbandonsTheWatch(t *testing.T) { + clk := newFakeClock(time.Unix(1_700_000_000, 0)) + stop := make(chan struct{}) + w := start(t, clk, idleExitOptions{threshold: time.Hour, act: &activityClock{}, + pending: func() int { return 0 }, stop: stop}) + close(stop) + select { + case r := <-w.res: + if r != "" { + t.Fatalf("stop should abandon the watch, got exit reason %q", r) + } + case <-time.After(3 * time.Second): + t.Fatal("watchIdle ignored stop") + } +} + +// TestIdleExitStartsItsClockAtLaunch: a proxy that never serves a single request still has to +// exit. Nothing stamps the activity clock in that case, so watchIdle has to seed it itself — +// a zero clock would otherwise read as "idle since 1970" and exit on the first tick, which is +// the opposite failure and just as wrong. +func TestIdleExitStartsItsClockAtLaunch(t *testing.T) { + clk := newFakeClock(time.Unix(1_700_000_000, 0)) + w := startWith(t, clk, idleExitOptions{threshold: time.Hour, act: &activityClock{}, + pending: func() int { return 0 }, stop: make(chan struct{})}, true) + w.mustNotExit("first tick on a proxy that has served nothing") + clk.advance(61 * time.Minute) + w.mustExit("an hour after launch with no traffic at all") +} + +// TestIdleCheckIntervalStaysUseful pins the resolution at both ends: a 24h default must not +// mean an hour of slack past the threshold, and the 1h floor must not mean a check every few +// minutes for nothing. +func TestIdleCheckIntervalStaysUseful(t *testing.T) { + for _, c := range []struct{ threshold, want time.Duration }{ + {24 * time.Hour, 5 * time.Minute}, // clamped high + {time.Hour, 3 * time.Minute}, // threshold/20 + {10 * time.Minute, 30 * time.Second}, // clamped low + } { + if got := idleCheckInterval(c.threshold); got != c.want { + t.Errorf("idleCheckInterval(%s) = %s, want %s", c.threshold, got, c.want) + } + } +} + +// TestProbesDoNotDeferIdleExit is the other half, and it is the one that was a live bug. +// +// A liveness probe or a Prometheus scrape is a machine asking whether the process is up — not +// somebody using it. Counting those did not weaken --idle-exit, it DISABLED it: any probe on a +// schedule shorter than the threshold means the exit never fires, and the only log line is the +// `idle-exit armed` one at startup, so nothing says it silently stopped working. Measured on a +// 1h-threshold proxy: 2h03m of wall clock, then `idle for 1h3m0s`, the clock having been held +// up for an hour by a /healthz poller alone. +func TestProbesDoNotDeferIdleExit(t *testing.T) { + clk := newFakeClock(time.Unix(1_700_000_000, 0)) + act := &activityClock{} + h := stampActivity(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(http.StatusOK) + }), act, clk.now) + + w := start(t, clk, idleExitOptions{threshold: time.Hour, act: act, + pending: func() int { return 0 }, stop: make(chan struct{})}) + + // A probe every 10 minutes for two hours — the shape of a real monitoring loop. + for i := 0; i < 12; i++ { + clk.advance(10 * time.Minute) + for _, path := range []string{"/healthz", "/metrics"} { + h.ServeHTTP(httptest.NewRecorder(), httptest.NewRequest("GET", path, nil)) + } + } + if got := w.mustExit("two hours of nothing but liveness probes"); got == "" { + t.Fatal("no exit reason") + } + + // And the asymmetry is deliberate, so pin it: a dashboard poll IS use. Exiting under + // somebody who is watching is a worse failure than a process left running. + clk2 := newFakeClock(time.Unix(1_700_000_000, 0)) + act2 := &activityClock{} + h2 := stampActivity(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {}), act2, clk2.now) + w2 := start(t, clk2, idleExitOptions{threshold: time.Hour, act: act2, + pending: func() int { return 0 }, stop: make(chan struct{})}) + clk2.advance(50 * time.Minute) + h2.ServeHTTP(httptest.NewRecorder(), httptest.NewRequest("GET", "/api/events", nil)) + clk2.advance(30 * time.Minute) + w2.mustNotExit("a dashboard tab is open and polling") +} + +// TestCheckIdleExitRefusesAGatewaySelfTerminating covers the second startup refusal. +// +// `--upstreams` means this process serves other people's agents. A proxy that vanishes overnight +// there is a far worse failure than one left running on a laptop — and the protection used to be +// accidental: it held only because a hosted deployment runs a liveness probe, and every probe +// stamped the activity clock. probeRoutes deliberately stopped counting probes, which removes +// that accident, so the refusal has to be explicit or the combination silently becomes live. +func TestCheckIdleExitRefusesAGatewaySelfTerminating(t *testing.T) { + ok := store.Options{} // default TTL => floor 5h33m20s + good := 24 * time.Hour // clears the floor + for _, c := range []struct { + name string + d time.Duration + upstreams string + wantErr string + }{ + {"laptop install: no upstreams", good, "", ""}, + {"gateway with idle-exit", good, "/etc/context-guru/upstreams.yaml", "--upstreams"}, + // Off is always fine, including on a gateway: that is the shipped default and the + // refusal must not fire on a configuration everybody runs. + {"gateway without idle-exit", 0, "/etc/context-guru/upstreams.yaml", ""}, + // The floor still applies, and it is reported first — a threshold that is BOTH too short + // and on a gateway should name the floor, since that is the value the operator typed. + {"below the floor", 30 * time.Minute, "", "floor"}, + {"below the floor on a gateway", 30 * time.Minute, "/etc/x.yaml", "floor"}, + } { + err := checkIdleExit(c.d, c.upstreams, ok) + switch { + case c.wantErr == "" && err != nil: + t.Errorf("%s: refused a valid configuration: %v", c.name, err) + case c.wantErr != "" && err == nil: + t.Errorf("%s: accepted a configuration that must not start", c.name) + case c.wantErr != "" && err != nil && !strings.Contains(err.Error(), c.wantErr): + t.Errorf("%s: message does not mention %q: %v", c.name, c.wantErr, err) + } + } +} diff --git a/cmd/context-guru-proxy/main.go b/cmd/context-guru-proxy/main.go index 584c5e4a..25dc0655 100644 --- a/cmd/context-guru-proxy/main.go +++ b/cmd/context-guru-proxy/main.go @@ -63,7 +63,13 @@ func listenAndAnnounce(addr string, attrs ...any) (net.Listener, error) { func main() { var ( - addr = envOr("LISTEN_ADDR", ":4000") + // --listen, not just LISTEN_ADDR. Two reasons beyond taste: an operator reading `ps` + // could not tell which port an instance held (the address reached it only through the + // environment), and a supervisor that needs to stop ONE instance among several had + // nothing in the command line to match on. Pattern-matching a process for shutdown is + // still the wrong tool — but when it happens, the port must at least be visible. + addrFlag = flag.String("listen", envOr("LISTEN_ADDR", ":4000"), "address to listen on") + showVer = flag.Bool("version", false, "print version and exit") cfgPath = flag.String("config", envOr("CONFIG", ""), "path to context-guru YAML config") preset = flag.String("preset", envOr("PRESET", "house"), "preset to use when --config is absent (house = the service default, deterministic; housellm = the same plus the compaction-model pass; codesmart/codesafe = the SWE-bench study's configs, kept so its published numbers stay reproducible)") openai = flag.String("openai-upstream", envOr("OPENAI_UPSTREAM", "https://api.openai.com"), "OpenAI upstream base URL") @@ -78,6 +84,14 @@ func main() { bob = flag.String("bob-upstream", envOr("BOB_UPSTREAM", ""), "Bob (BobShell) backend base URL; enables the Bob gateway routes when set (e.g. https://api.us-east.bob.ibm.com)") storeFlag = flag.String("store", envOr("STORE", ""), "override state store: true|false (default: config store.enabled, else on)") modeFlag = flag.String("mode", envOr("MODE", ""), "operating mode: sync (default) | observe (overrides the config's mode:)") + // OFF by default, and it must stay that way: a gateway or eval-containers deployment + // that self-terminates is a much worse failure than a laptop process left running. + // Set only by the plugin installer, which pairs it with a SessionStart hook that + // starts the proxy again on demand — self-kill without that resurrection is a + // footgun, so they ship together. The floor is enforced below, not documented. + idleExit = flag.Duration("idle-exit", envDuration("IDLE_EXIT", 0), + "exit after this long with no requests and no keep-alive ping pending (0 = never; "+ + "must be at least 2x store.ttl_seconds, see store.IdleExitFloor)") // Dashboard. Off by default so an existing deployment's behavior and route // table are unchanged until asked for; on, it adds /dashboard/ + /api/*. @@ -190,6 +204,17 @@ func main() { ) flag.Parse() + // --version before anything else: an installer asks a binary what it is, and it must be + // able to ask without starting a server or needing a config. buildinfo.Version was already + // compiled in and reachable only via /stats, which requires a running proxy — so + // `context-guru-proxy --version` was answered by the flag package's usage text, and an + // installer parsing it recorded "Usage of context-guru-proxy:" as the installed version. + if *showVer { + fmt.Printf("context-guru-proxy %s (commit %s)\n", buildinfo.Version, buildinfo.Commit) + return + } + addr := *addrFlag + // Logging first, before anything can want to log. Level, format and sink come from // the environment (CG_LOG_LEVEL / CG_LOG_FORMAT / CG_LOG_FILE / CG_LOG_PLAIN) rather // than flags, because the two places that set them are a systemd drop-in and a shell, @@ -571,6 +596,17 @@ func main() { slog.Warn("context-guru: OBSERVE MODE — requests are forwarded UNMODIFIED; " + "/stats reports what compaction WOULD have saved under potential_*/projected_* keys") } + // Idle-exit validation goes BEFORE the "listening" line, because a fatal here used to be + // logged after it: the operator saw `context-guru-proxy listening` and then an exit, which + // reads as a crash rather than as a rejected configuration. + // + // Refused rather than warned about: a threshold below the store's entry lifetime does not + // degrade gracefully, it re-bills live prefixes as cache creation (the 11.5x regression + // FrozenLost exists to catch). A misconfigured value must not start. + if err := checkIdleExit(*idleExit, *upstreamsPath, cfg.Store); err != nil { + log.Fatalf("context-guru: %v", err) + } + // The sink last, so it is the line just above the traffic: "where are the logs and // what level am I getting" is the first question when something looks quiet. ln, err := listenAndAnnounce(addr, "pipeline", cfg.Pipeline, "mode", mode, "logs", sink) @@ -578,9 +614,22 @@ func main() { log.Fatalf("listen: %v", err) } + // Activity stamping is wired ONLY when the watchdog is on, so an ordinary deployment's + // handler chain is byte-identical to before. + var handler http.Handler = h.Mux() + act := &activityClock{} + if *idleExit > 0 { + // Launch counts as activity, so the threshold is measured from a moment that means + // something rather than from whenever the watchdog goroutine is first scheduled. + act.touch(time.Now()) + handler = stampActivity(handler, act, time.Now) + slog.Info("context-guru: idle-exit armed", "after", *idleExit, + "check_every", idleCheckInterval(*idleExit)) + } + srv := &http.Server{ Addr: addr, - Handler: h.Mux(), + Handler: handler, // ReadHeaderTimeout is the one that matters for a service on a network: without // it, a client that opens a connection and never finishes its headers holds a // goroutine and a file descriptor indefinitely. @@ -600,12 +649,36 @@ func main() { // Graceful shutdown, so the dashboard's writer goroutine flushes its batch and any // in-flight archive upload is not abandoned halfway. Without this, a restart loses // the last few hundred milliseconds of captured requests every time. + // + // Both reasons to stop — a signal, and the idle watchdog — converge on ONE teardown, so + // the self-terminating path cannot drift from the one that is known to work. idle := make(chan struct{}) + why := make(chan string, 2) + stopWatch := make(chan struct{}) go func() { sig := make(chan os.Signal, 1) signal.Notify(sig, syscall.SIGINT, syscall.SIGTERM) s := <-sig - slog.Info("context-guru: shutting down", "signal", s.String()) + why <- "signal " + s.String() + }() + if *idleExit > 0 { + t := time.NewTicker(idleCheckInterval(*idleExit)) + go func() { + defer t.Stop() + // h.PendingPings is the half of "idle" that requests cannot express: the + // keep-alive works precisely when no client traffic is arriving. + if reason, ok := watchIdle(idleExitOptions{ + threshold: *idleExit, act: act, pending: h.PendingPings, + now: time.Now, tick: t.C, stop: stopWatch, + }); ok { + why <- reason + } + }() + } + go func() { + reason := <-why + close(stopWatch) + slog.Info("context-guru: shutting down", "reason", reason) ctx, cancel := context.WithTimeout(context.Background(), 25*time.Second) defer cancel() if err := srv.Shutdown(ctx); err != nil { diff --git a/config/config.go b/config/config.go index 4bc6c0b0..f554fb38 100644 --- a/config/config.go +++ b/config/config.go @@ -358,7 +358,27 @@ func (c *Config) applyPreset() error { // sweep found 0 convertible candidates in 11.67M tokens. It was costing 1.53 ms and a // TextTokens call per tool message to convert nothing. var presets = map[string][]string{ - "off": {}, // passthrough: no components (baseline / A-B control) + "off": {}, // passthrough: no components (baseline / A-B control) + // cache: the volatile-tail split and NOTHING else. This is the preset a stranger + // evaluating context-guru on their own Claude Code sessions is pointed at, and the + // reason it exists is that it can be verified by reading this one line: no content is + // dropped, no `<>` marker is written, no expand tool is injected into the + // request, and no model is called. The loudest objection to putting a proxy on the + // wire — "you are editing my agent's context" — does not apply to it. + // + // It is also the best-evidenced single component in the repo: -34.1% cost and 0% -> + // 96.7% prefix-cache hit in an isolated A/B (docs/results/context-guru.md), which is + // why the funnel leads with the cache rather than the offloaders. + // + // Deliberately NOT `safe` (format -> textclean -> searchfold -> cachesplit): those are + // lossless in meaning but they still rewrite the JSON, so "we do not touch your + // context" stops being literally true and a reviewer has to take four components on + // trust instead of reading one. TestCachePresetIsCachesplitAlone holds it to that, and + // the lossless-folds rule exempts it for the same reason. + // + // Anthropic-family only, and the docs say so: cachesplit is a no-op on implicit + // prefix-cache backends (vLLM, llm-d) — see apply/prefixsplit.go. + "cache": {"cachesplit"}, "safe": {"format", "textclean", "searchfold", "cachesplit"}, "balanced": {"format", "textclean", "searchfold", "dedup", "failed_run", "cmdfilter", "linecap", "cachesplit"}, "aggressive": {"format", "textclean", "searchfold", "dedup", "failed_run", "cmdfilter", "smartcrush", "extract", "extract_llm", "linecap", "cachesplit"}, diff --git a/config/config_more_test.go b/config/config_more_test.go index 5af1c4ed..26ffc5fa 100644 --- a/config/config_more_test.go +++ b/config/config_more_test.go @@ -125,7 +125,13 @@ func TestLosslessFoldsAreInEveryWorkingPreset(t *testing.T) { // restructures the transcript alone, and agentdiet reproduces a published baseline // whose whole claim is what ONE reflection achieves — stacking folds beside it would // reduce the same outputs first and there would be nothing left to attribute. - exempt := map[string]bool{"off": true, "summarize": true, "agentdiet": true} + // `cache` is exempt for a reason the other three do not share: its whole product claim + // is that it is ONE component, verifiable by reading one line of the presets map. Adding + // format/textclean/searchfold to it would each be lossless in meaning and would still + // cost the claim — a stranger deciding whether to route their agent through us can check + // "nothing but a cache breakpoint moves" in a second, and cannot check four rewriters as + // fast. See TestCachePresetIsCachesplitAlone, which holds the other side of that trade. + exempt := map[string]bool{"off": true, "summarize": true, "agentdiet": true, "cache": true} for name, pipeline := range presets { if exempt[name] { continue @@ -217,3 +223,36 @@ func TestLinecapRunsLastAmongTheOffloaders(t *testing.T) { } } } + +// TestCachePresetIsCachesplitAlone guards the one preset whose CONTENT is its promise. +// +// `cache` is what the local-distribution funnel points a stranger at, and the pitch is +// exact: no content dropped, no `<>` marker written, no expand tool injected, no +// model called. That is not a property of cachesplit that survives company — every other +// component in the repo either rewrites JSON, offloads content, or calls a model, so ANY +// addition here converts a checkable claim into a trust-me claim, and the docs that make the +// claim (docs/how-to/choose-a-preset.md, the plugin's install skill) do not get to notice. +// +// It is also why `cache` is exempt from TestLosslessFoldsAreInEveryWorkingPreset. That +// exemption is only defensible while this test exists: without it, "cache is exempt from the +// folds rule" would read as permission to put anything at all in it. +func TestCachePresetIsCachesplitAlone(t *testing.T) { + p, ok := presets["cache"] + if !ok { + t.Fatal("preset `cache` is gone; the local-distribution funnel and the install skill both name it") + } + if len(p) != 1 || p[0] != "cachesplit" { + t.Fatalf("preset `cache` = %v, want exactly [cachesplit]: it is the only preset whose "+ + "losslessness is verifiable by reading one line, and every other component either "+ + "rewrites JSON, offloads content, or calls a model", p) + } + // The pipeline the proxy actually builds, not just the map literal: applyPreset and the + // rich-preset path both sit between this map and the wire. + built, ok := PresetPipeline("cache") + if !ok { + t.Fatal(`PresetPipeline("cache") did not resolve, so ?preset=cache would 400`) + } + if len(built) != 1 || built[0] != "cachesplit" { + t.Fatalf(`PresetPipeline("cache") = %v, want [cachesplit]`, built) + } +} diff --git a/docs/get-started/quickstart-proxy.md b/docs/get-started/quickstart-proxy.md index e82ebb44..c554a9ad 100644 --- a/docs/get-started/quickstart-proxy.md +++ b/docs/get-started/quickstart-proxy.md @@ -3,19 +3,30 @@ Run context-guru in front of your provider and point an agent at it. One port serves both the OpenAI and Anthropic dialects. -You need **Go 1.26**. You do **not** need a C toolchain: `make build` builds with cgo disabled, and -the result is a statically linked binary with no runtime dependencies. Everything else is a normal -module dependency — build straight from the repo root. +**You need no toolchain at all to run it.** The shipped binary is statically linked pure Go — no C +compiler, no Go install, no runtime dependencies. Grab it from +[Releases](https://github.com/rossoctl/context-guru/releases): + +```sh +# Pick your platform: linux/darwin × amd64/arm64 +tar xzf context-guru_*_darwin_arm64.tar.gz +install -m 755 context-guru-proxy ~/.local/bin/ +``` + +To build from source instead you need **Go 1.26** — and still no C toolchain: `make build` builds +with cgo disabled and produces the same statically linked binary. CI asserts that natively for +linux/amd64 (the `purego` job), and the release workflow asserts it again before publishing. A C compiler is needed for exactly two things: `make test` (the race detector requires cgo) and the optional [`skeleton`](../components/skeleton.md) component's `cg_skeleton` build tag. ## Steps -1. Build: +1. Build (source path only — skip if you downloaded a release): ```sh make build # → bin/context-guru-proxy + make build-static # the pure-Go build releases ship (CGO_ENABLED=0) ``` 2. Run it. It listens on `:4000`; set `LISTEN_ADDR` to change that. diff --git a/docs/how-to/choose-a-preset.md b/docs/how-to/choose-a-preset.md index 313f6f15..0451229f 100644 --- a/docs/how-to/choose-a-preset.md +++ b/docs/how-to/choose-a-preset.md @@ -11,6 +11,7 @@ context-guru-proxy --preset codesmart # or PRESET=codesmart, or preset: in | Your workload | Preset | |---|---| +| **Trying context-guru for the first time** | **`cache`** | | **Most agents — the recommended pipeline** | **`codesmart`** (pass `--preset codesmart`; the binary defaults to `house`) | | Same, but no LLM on the hot path | `codesafe` | | A guaranteed-safe, lossless win only | `safe` | @@ -28,6 +29,7 @@ context-guru-proxy --preset codesmart # or PRESET=codesmart, or preset: in |---|---| | `codesmart` | `format, textclean, searchfold, dedup, failed_run, cmdfilter, extract_llm, extract, linecap, cachesplit` | | `codesafe` | `format, textclean, searchfold, dedup, failed_run, cmdfilter, extract, collapse, linecap, cachesplit` | +| `cache` | `cachesplit` | | `safe` | `format, textclean, searchfold, cachesplit` | | `balanced` | `format, textclean, searchfold, dedup, failed_run, cmdfilter, linecap, cachesplit` | | `aggressive` | `format, textclean, searchfold, dedup, failed_run, cmdfilter, smartcrush, extract, extract_llm, linecap, cachesplit` | @@ -56,6 +58,39 @@ table so it lists every preset that exists; pick from the table above this one. ## Notes on the ones people pick +### `cache` — start here + +`cachesplit` and nothing else. Pick it when what you want is to find out whether this thing +helps you, with the smallest possible claim to check: + +- **Nothing is dropped, summarised, or replaced.** No `<>` markers, no + `context_guru_expand` tool added to your requests, no model calls. It splits one oversized + system block into two adjacent text blocks whose concatenation is byte-identical, so the + model sees exactly the prompt your agent sent — and moves the cache breakpoint onto the + half that does not churn. +- **What it is worth is regime-dependent, and the funnel's regime is the weak one.** The + headline **−34.1% cost / 0% → 96.7% hit** comes from a benchmark harness running tasks + back-to-back inside the provider's 5-minute cache TTL + ([cacheinject](../components/cacheinject.md#what-the-split-is-worth)), which is + precisely the regime where the split pays — and is *one task measured three times, not a + fleet average*. On this project's own interactive traffic the figure is **$0.0298 across + 1,127 sessions / 11,361 requests** + ([dashboard](../dashboard.md#what-it-is-actually-worth-here-and-why-that-is-small)): Claude + Code captures the environment snapshot once per session, and 1,105 of 1,127 session starts + read zero tokens from cache because the previous prefix had already expired. It is also + **exactly zero** outside a git repository, on a system prompt under the 1,024-token + `minSplitTokens` floor, and on any implicit prefix-cache backend (vLLM, llm-d). Neither + figure is wrong; they differ by three orders of magnitude because the mechanism needs a + second session inside five minutes. + +- **Anthropic-family only.** `cachesplit` is a no-op against implicit prefix-cache backends + (vLLM, llm-d), which match to the divergence on their own — so on those it costs nothing + and buys nothing. + +Move to `codesmart` once you want the offloaders too. `safe` is the next step up and is still +lossless in meaning, but it does rewrite JSON, so `cache` is the one whose promise you can +confirm by reading a single line of `config/config.go`. + **`codesmart`** is the shipped default and the cheapest arm in the [benchmarks](../RESULTS.md) at the highest reward. It is the one preset that ships tuned per-component settings rather than a bare name-list, which is why most turns make no model diff --git a/docs/how-to/use-with-claude-code.md b/docs/how-to/use-with-claude-code.md index ff3eaefd..68862888 100644 --- a/docs/how-to/use-with-claude-code.md +++ b/docs/how-to/use-with-claude-code.md @@ -3,6 +3,22 @@ Route [Claude Code](https://docs.claude.com/en/docs/claude-code) through context-guru with one environment variable — no changes to Claude Code itself. +## You do not need an API key + +Setting `ANTHROPIC_BASE_URL` **without** a credential variable leaves your claude.ai login in +place: a Pro or Max subscription keeps working, with your usage limits and billing unchanged. You +can run context-guru in front of your own sessions with **no API key at all** — which is the +cheapest way to evaluate it. + +Two honest caveats: + +- On subscription billing the saving lands in **usage limits**, not dollars, so `/stats` cost + figures are list-price estimates and will not match a bill you do not receive. +- Setting `ANTHROPIC_API_KEY` or `ANTHROPIC_AUTH_TOKEN` in your Claude Code environment is what + moves you onto metered API billing. Only do it deliberately — see + [Keep the API key out of Claude Code](#keep-the-api-key-out-of-claude-code), which is about + the *proxy* holding the key, not Claude Code. + ## Steps 1. Start the proxy: @@ -38,6 +54,9 @@ Add to `.claude/settings.json` so you don't export anything by hand: } ``` +Use `.claude/settings.local.json` instead if you do not want to commit it: a base URL pointing at +`localhost` breaks Claude Code for everyone who clones the repo whenever the proxy is not running. + ## Keep the API key out of Claude Code Give the proxy the real key and hand Claude Code a placeholder; the proxy injects the diff --git a/docs/reference/config.md b/docs/reference/config.md index 51717b72..64a35a45 100644 --- a/docs/reference/config.md +++ b/docs/reference/config.md @@ -78,7 +78,9 @@ for every component's config block. |---|---|---| | `--preset` / `PRESET` | `house` | Pipeline preset when no `--config`. `codesmart` is the SWE-bench arm and must be asked for by name. | | `--config` / `CONFIG` | — | YAML config file (overrides preset). | -| `LISTEN_ADDR` | `:4000` | Listen address. | +| `--listen` / `LISTEN_ADDR` | `:4000` | Listen address. The flag exists so the port is visible in `ps` and to a supervisor; before it, the address reached the process only through the environment. | +| `--version` | — | Print version and commit, then exit. | +| `--idle-exit` / `IDLE_EXIT` | `0` (never) | Exit after this long with **no requests and no keep-alive ping pending**, so a proxy started on demand does not outlive its use. Refused at startup below `max(2 × store.ttl_seconds, 1h)` — 5h33m20s at the default TTL — because exiting clears the in-memory store, and losing a frozen decision re-bills its whole prefix as cache creation. Also refused together with `--upstreams`: a gateway serving other people's agents must not self-terminate. Liveness probes (`/healthz`, `/metrics`) deliberately do **not** count as activity; anything else does, including the dashboard's own polling. | | `--openai-upstream` / `OPENAI_UPSTREAM` | `https://api.openai.com` | OpenAI upstream base. | | `--anthropic-upstream` / `ANTHROPIC_UPSTREAM` | `https://api.anthropic.com` | Anthropic upstream base. | | `--bob-upstream` / `BOB_UPSTREAM` | — | Bob (BobShell) backend base. Setting it mounts the [Bob gateway routes](routes.md#bob-bobshell-gateway-routes); unset, an unknown path 404s as before. | diff --git a/docs/reference/presets.md b/docs/reference/presets.md index c6a00f84..06d82a13 100644 --- a/docs/reference/presets.md +++ b/docs/reference/presets.md @@ -12,6 +12,7 @@ taken exactly from the `presets` map in `config/config.go`. | `codesmart` | `format` → `textclean` → `searchfold` → `dedup` → `failed_run` → `cmdfilter` → `extract_llm` → `extract` → `linecap` → `cachesplit` | The SWE-bench-winning cache-aware config: structural offloaders + a cheap-model relevance-trimmer (`extract_llm`, routed to `CHEAP_MODEL`, gated so most turns make no model call) + deterministic `extract`. `extract_llm` no-ops (→ deterministic) when no cheap model is configured. **Changed 2026-08:** the lossless trio replaced `toon`, which acted 0 times on 5,752 production requests, and `linecap` was added. Re-measure before quoting the published SWE-bench numbers against it. | | `codesafe` | `format` → `textclean` → `searchfold` → `dedup` → `failed_run` → `cmdfilter` → `extract` → `collapse` → `linecap` → `cachesplit` | `codesmart` minus the LLM pass — **deterministic-only, zero model calls by policy**. The safe control / the choice when you don't want an LLM on the hot path. | | `off` | *(empty)* | Passthrough — no components. The baseline / A-B control. | +| `cache` | `cachesplit` | **The first-run preset** — the one to point a new evaluator at. The volatile-tail split and nothing else: no content dropped, no `<>` markers, no `context_guru_expand` tool added to requests, no model calls. Chosen so a stranger deciding whether to route their agent through a local proxy can verify the claim by reading one line of `config/config.go` rather than trusting four components. The savings claim is regime-dependent and the funnel's regime is the weak one: **−34.1% cost / 0% → 96.7% hit** is a benchmark harness running tasks back-to-back inside the provider's 5-minute TTL (and is one task measured three times), while this project's own interactive traffic yields **$0.0298 across 1,127 sessions** — 1,105 of 1,127 session starts read zero from cache. Zero outside a git repo, under the 1,024-token `minSplitTokens` floor, or on an implicit prefix-cache backend (vLLM, llm-d). See [dashboard](../dashboard.md#what-it-is-actually-worth-here-and-why-that-is-small) and [cacheinject](../components/cacheinject.md). | | `safe` | `format` → `textclean` → `searchfold` → `cachesplit` | Lossless only: repack JSON compactly and split the volatile system tail so the shared prefix stays cacheable. Zero risk of dropping content. | | `balanced` | `format` → `textclean` → `searchfold` → `dedup` → `failed_run` → `cmdfilter` → `linecap` → `cachesplit` | Lossless repack + conservative offloads (dedupe, drop superseded/failed runs, filter command noise) + the cache split. **Not recommended for agentic traffic** — it omits `mask`, the biggest lever there. | | `aggressive` | `format` → `textclean` → `searchfold` → `dedup` → `failed_run` → `cmdfilter` → `smartcrush` → `extract` → `extract_llm` → `linecap` → `cachesplit` | `balanced` plus `smartcrush` (crush long homogeneous arrays), deterministic `extract` (noise collapse), and `extract_llm` (cheap-model relevance trim) for deeper savings. | diff --git a/docs/setup.md b/docs/setup.md index aa63ef66..3c21e35a 100644 --- a/docs/setup.md +++ b/docs/setup.md @@ -10,12 +10,14 @@ SWE-bench task driven by Claude Code. pure Go and statically linked. bifrost's tokenizer does **not** use cgo: o200k_base is embedded (`internal/tokens/tokens.go`). CI asserts the pure-Go build on every PR — natively, for linux/amd64 — in the `purego` job (`.github/workflows/ci.yaml`), which builds with - `CGO_ENABLED=0`, checks the artifact is statically linked, starts it and probes `/healthz`. So the - claim cannot rot back into a false one for the platform CI runs on. + `CGO_ENABLED=0`, checks the artifact is statically linked, starts it and probes `/healthz`. The + release workflow asserts it again before publishing, deliberately: a release must not depend on a + PR check having run. Cross-compilation to the other three release targets (linux/arm64, darwin/amd64, darwin/arm64) is - **not** covered by that job: it was verified by hand on go 1.26.4 and is asserted at release time - by the tag workflow, not per PR. + covered by the release build, not by that per-PR job. +- If you do not need `skeleton`, skip the build entirely and use a + [release binary](https://github.com/rossoctl/context-guru/releases). - **Docker** (for the gateway image / eval-containers), and the **eval-containers** repo. ## Build diff --git a/proxy/conformance_test.go b/proxy/conformance_test.go new file mode 100644 index 00000000..c2369bba --- /dev/null +++ b/proxy/conformance_test.go @@ -0,0 +1,305 @@ +package proxy_test + +import ( + "bufio" + "encoding/json" + "fmt" + "io" + "net/http" + "net/http/httptest" + "strings" + "testing" + "time" + + "github.com/tidwall/gjson" +) + +// Gateway conformance under the funnel's default preset. +// +// The local-distribution funnel puts context-guru on the wire in front of a stranger's Claude +// Code, so "it works" is not enough — it has to not break the client, and the ways it could are +// specific and documented in the gateway protocol reference. Each test below is one of them. +// +// Two of these are worse than an outage, because they make the demo read as NEGATIVE rather +// than broken: +// +// - a buffered SSE response looks like the proxy made the model slow; +// - a rejected `cache_control` marker makes Claude Code disable prompt caching for the rest +// of the conversation, which switches off the exact thing being sold, silently. +// +// The preset under test is `cache` (cachesplit alone) throughout, because that is what an +// evaluator actually runs. The shared Claude-Code-shaped fixtures are in ccbody_test.go, and the +// two items that were DEFECTS rather than confirmations — the expand-tool gate and the missing +// count_tokens route — are tested beside their fixes in expandgate_test.go and +// counttokens_test.go. +// TestCachePresetForwardsAnUpstreamErrorByteForByte covers conformance item 4. +// +// Claude Code's capability-rejection recovery matches on the upstream's error WORDING. A gateway +// that wraps, re-encodes or summarises an error body breaks that recovery path — the client can +// no longer tell "your cache_control was refused" from any other 400, so instead of retrying +// without the capability it surfaces a failure. The status, the body and the content type all +// have to arrive exactly as the upstream wrote them. +func TestCachePresetForwardsAnUpstreamErrorByteForByte(t *testing.T) { + // A real Anthropic error shape, whitespace and key order included: this is what the + // client's matching runs against, so the test compares bytes rather than parsed JSON. + errBody := `{"type":"error","error":{"type":"invalid_request_error","message":"A maximum of 4 blocks with cache_control may be provided, but found 5."}}` + upstream := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Content-Type", "application/json") + w.Header().Set("request-id", "req_upstream_123") + w.WriteHeader(http.StatusBadRequest) + fmt.Fprint(w, errBody) + })) + defer upstream.Close() + + h, _ := buildHandler(t, cachePipeline, upstream.URL) + srv := httptest.NewServer(h.Mux()) + defer srv.Close() + + resp, err := http.Post(srv.URL+"/anthropic/v1/messages", "application/json", + strings.NewReader(string(claudeCodeBody(t, false)))) + if err != nil { + t.Fatal(err) + } + defer resp.Body.Close() + got, _ := io.ReadAll(resp.Body) + + if resp.StatusCode != http.StatusBadRequest { + t.Errorf("status = %d, want 400: a rewritten status breaks the client's retry logic", + resp.StatusCode) + } + if string(got) != errBody { + t.Errorf("the error body was modified.\n got: %s\nwant: %s\n"+ + "Claude Code matches on the upstream's own wording to decide whether to retry "+ + "without a capability; wrapping it disables that recovery.", got, errBody) + } + if resp.Header.Get("request-id") != "req_upstream_123" { + t.Errorf("request-id header lost (%q): it is what support uses to find the call", + resp.Header.Get("request-id")) + } +} + +// TestCachePresetDoesNotBufferSSE covers conformance item 1. +// +// Claude Code aborts a stream that has been silent for 300s, and a gateway that buffers a whole +// response before relaying it stalls the client. context-guru does buffer SOME responses — the +// ones where the model opens by calling the expand tool — but under the `cache` preset nothing +// injects that tool, so the buffering path must be unreachable. This asserts that rather than +// assuming it: the client's first event has to arrive while the upstream is still writing later +// ones. +func TestCachePresetDoesNotBufferSSE(t *testing.T) { + release := make(chan struct{}) + upstream := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Content-Type", "text/event-stream") + w.WriteHeader(http.StatusOK) + fl, _ := w.(http.Flusher) + fmt.Fprint(w, "event: message_start\ndata: {\"type\":\"message_start\",\"message\":{\"usage\":{\"input_tokens\":10}}}\n\n") + if fl != nil { + fl.Flush() + } + // Hold the rest of the stream until the test has SEEN the first event. If the proxy + // buffered, the read below would block here and the test fails on the deadline rather + // than on a wrong byte — which is exactly the client-visible symptom. + <-release + fmt.Fprint(w, "event: message_stop\ndata: {\"type\":\"message_stop\"}\n\n") + if fl != nil { + fl.Flush() + } + })) + defer upstream.Close() + + h, _ := buildHandler(t, cachePipeline, upstream.URL) + srv := httptest.NewServer(h.Mux()) + defer srv.Close() + + req, _ := http.NewRequest("POST", srv.URL+"/anthropic/v1/messages", + strings.NewReader(string(claudeCodeBody(t, true)))) + req.Header.Set("Content-Type", "application/json") + resp, err := (&http.Client{Timeout: 15 * time.Second}).Do(req) + if err != nil { + close(release) + t.Fatal(err) + } + defer resp.Body.Close() + + type read struct { + line string + err error + } + ch := make(chan read, 1) + go func() { + line, err := bufio.NewReader(resp.Body).ReadString('\n') + ch <- read{line, err} + }() + select { + case r := <-ch: + close(release) + if r.err != nil { + t.Fatalf("reading the first event: %v", r.err) + } + if !strings.Contains(r.line, "message_start") { + t.Fatalf("first line was %q, want the upstream's first event", r.line) + } + case <-time.After(5 * time.Second): + close(release) + t.Fatal("no event reached the client while the upstream was still streaming: the " + + "response is being buffered. Claude Code aborts a stream silent for 300s, and a " + + "stalled first byte reads as context-guru making the model slow.") + } +} + +// TestCachePresetNeverAddsACacheControlBreakpoint covers conformance item 2, which is the +// strongest argument for shipping `cache` rather than a placement preset. +// +// The provider caps `cache_control` markers at 4. Exceed it and the request is REJECTED — and +// Claude Code's reaction to a rejected capability is to retry without it and leave prompt +// caching OFF for the rest of the conversation. So a breakpoint-budget mistake is not an error +// the user sees; it silently switches off the thing this whole funnel is selling, and the demo +// reads as "context-guru made my session more expensive". +// +// cachesplit MOVES a breakpoint onto the stable half of a block it splits; it must never add +// one. The body below arrives at the cap, so any addition at all is a 400. +func TestCachePresetNeverAddsACacheControlBreakpoint(t *testing.T) { + var forwarded []byte + upstream := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + forwarded, _ = io.ReadAll(r.Body) + w.Header().Set("Content-Type", "application/json") + fmt.Fprint(w, `{"type":"message","usage":{"input_tokens":1}}`) + })) + defer upstream.Close() + + h, _ := buildHandler(t, cachePipeline, upstream.URL) + srv := httptest.NewServer(h.Mux()) + defer srv.Close() + + // Four inbound breakpoints — the provider's cap — spread the way a real client spreads + // them: two system blocks, one tool, one message. Assembled as text, for the key-order + // reason documented on claudeCodeBody. + bp := `,"cache_control":{"type":"ephemeral"}` + body := []byte(`{"model":"claude-sonnet-5","max_tokens":64,"system":[` + + `{"type":"text","text":` + jsonStr(attributionText) + bp + `},` + + `{"type":"text","text":` + jsonStr(volatileSystemText()) + bp + `}` + + `],"tools":[{"name":"read_file","input_schema":{"type":"object"}` + bp + `}` + + `],"messages":[{"role":"user","content":[{"type":"text","text":"hello"` + bp + `}]}]}`) + if !json.Valid(body) { + t.Fatalf("test fixture is not valid JSON: %s", body) + } + + inbound := countBreakpoints(body) + if inbound != 4 { + t.Fatalf("test setup is wrong: the request carries %d breakpoints, not the cap of 4", inbound) + } + resp, err := http.Post(srv.URL+"/anthropic/v1/messages", "application/json", strings.NewReader(string(body))) + if err != nil { + t.Fatal(err) + } + resp.Body.Close() + if len(forwarded) == 0 { + t.Fatal("upstream received nothing") + } + // The precondition that stops this being a vacuous pass: the component under test has to + // have ACTED. If cachesplit did not split, "breakpoints unchanged" is trivially true and + // asserts nothing about the rewrite. + if n := len(gjson.GetBytes(forwarded, "system").Array()); n != 3 { + t.Fatalf("cachesplit did not split the volatile tail (system has %d blocks, want 3): "+ + "the breakpoint assertion below would be vacuous", n) + } + if out := countBreakpoints(forwarded); out != inbound { + t.Errorf("breakpoints on the wire = %d, inbound = %d (cap 4). Exceeding the cap is a "+ + "400, and Claude Code answers a rejected cache_control by disabling prompt caching "+ + "for the rest of the conversation — silently switching off what this preset exists "+ + "to demonstrate.\nforwarded: %s", out, inbound, forwarded) + } +} + +// countBreakpoints counts cache_control/cachePoint markers anywhere in the body. Both spellings, +// because Bedrock/Vertex write `cachePoint` where Anthropic writes `cache_control`, and the +// provider's cap counts whatever arrives. +func countBreakpoints(body []byte) int { + n := 0 + var walk func(gjson.Result) + walk = func(v gjson.Result) { + v.ForEach(func(k, val gjson.Result) bool { + if k.String() == "cache_control" || k.String() == "cachePoint" { + n++ + } + if val.IsObject() || val.IsArray() { + walk(val) + } + return true + }) + } + walk(gjson.ParseBytes(body)) + return n +} + +// TestCachePresetLeavesTheAttributionBlockUntouched covers conformance item 3. +// +// Claude Code prepends an attribution block as the FIRST system block, and the API strips it +// only if that array arrives unchanged. cachesplit reshapes the system array, so the question is +// whether the first block survives byte-identically. +// +// Three separate properties keep it safe, and the second is the one a plausible change would +// break, so both are exercised below: +// +// 1. the attribution block carries no volatile marker, so it is not a split candidate; +// 2. blocks the split does not act on are re-emitted from their ORIGINAL raw bytes rather than +// re-encoded — re-marshalling would reorder keys and change the bytes even with identical +// content, which is enough to defeat a positional strip; +// 3. the split's minSplitTokens floor (1024) excludes a small block even when it does contain a +// marker — the second case below, where a user's own prompt happens to mention one. +// +// Proving this rather than reasoning about it is what makes the alternative — shipping +// CLAUDE_CODE_ATTRIBUTION_HEADER=0 in the installer — unnecessary, and keeps it unnecessary. +func TestCachePresetLeavesTheAttributionBlockUntouched(t *testing.T) { + for _, c := range []struct{ name, first string }{ + {"the ordinary attribution block", attributionText}, + // Small, but it mentions something the split looks for. Only the token floor keeps this + // out of the rewrite; without it the FIRST eligible block is the one that gets split, + // and that is this one. + {"a small first block that happens to name a volatile marker", + attributionText + "\nCurrent branch: whatever the user was talking about\n"}, + } { + t.Run(c.name, func(t *testing.T) { + var forwarded []byte + upstream := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + forwarded, _ = io.ReadAll(r.Body) + w.Header().Set("Content-Type", "application/json") + fmt.Fprint(w, `{"type":"message","usage":{"input_tokens":1}}`) + })) + defer upstream.Close() + + h, _ := buildHandler(t, cachePipeline, upstream.URL) + srv := httptest.NewServer(h.Mux()) + defer srv.Close() + + body := claudeCodeBodyWithFirst(t, false, c.first) + want := gjson.GetBytes(body, "system.0").Raw + + resp, err := http.Post(srv.URL+"/anthropic/v1/messages", "application/json", + strings.NewReader(string(body))) + if err != nil { + t.Fatal(err) + } + resp.Body.Close() + if len(forwarded) == 0 { + t.Fatal("upstream received nothing") + } + // Precondition: cachesplit must actually have rewritten the array, or "the first + // block is unchanged" is true because nothing happened. It must also have split the + // SECOND block, not the first — 3 blocks with the attribution intact is the only + // shape that means that. + blocks := gjson.GetBytes(forwarded, "system").Array() + if len(blocks) != 3 { + t.Fatalf("cachesplit did not act (system has %d blocks, want 3); the assertion "+ + "below would be vacuous", len(blocks)) + } + if got := blocks[0].Raw; got != want { + t.Errorf("the attribution block changed, so the API will no longer strip it "+ + "positionally.\n got: %s\nwant: %s", got, want) + } + if !strings.HasPrefix(blocks[0].Get("text").String(), attributionText) { + t.Errorf("the attribution block is no longer the first system block: %s", blocks[0].Raw) + } + }) + } +} diff --git a/proxy/keepalive.go b/proxy/keepalive.go index 50ff32a9..bf6d39ca 100644 --- a/proxy/keepalive.go +++ b/proxy/keepalive.go @@ -1158,6 +1158,44 @@ type KeepAliveStats struct { SpentUSD float64 `json:"spend_usd"` } +// PendingPings reports how many tracked sessions still have a ping scheduled ahead of them. +// +// This exists for the idle-exit watchdog, and it exists because the keep-alive INVERTS the +// ordinary meaning of "idle": pinging is what the proxy does precisely while no client +// traffic is arriving. A watchdog counting only requests would therefore kill the process in +// exactly the window the feature was built for — the quiet gap after `end_turn`, where 83.7% +// of the recoverable dollars sit. So "no requests recently" is not sufficient to exit; "and +// nothing is waiting to be pinged" is the other half. +// +// The conditions are `due`'s minus the timing term: an entry that is stopped, or has spent +// its MaxPings, or whose policy is off will never be pinged again and must not hold the +// process open. Everything else is gated at record time (see pingable), so a live entry is by +// construction one we intend to ping. +func (h *Handler) PendingPings() int { + if h == nil { + return 0 + } + return h.keeper.pendingPings() +} + +// pendingPings counts entries with a ping still ahead of them. Nil-safe: a keeper whose +// sweep never launched (the CONTEXT_GURU_KEEPALIVE kill switch) has nothing pending, which +// correctly lets an idle proxy exit. +func (k *keeper) pendingPings() int { + if k == nil { + return 0 + } + k.mu.Lock() + defer k.mu.Unlock() + n := 0 + for _, e := range k.live { + if !e.stopped && e.pol.on() && e.pings < e.pol.MaxPings { + n++ + } + } + return n +} + // Stats snapshots the keeper's counters. func (k *keeper) Stats() KeepAliveStats { if k == nil { diff --git a/store/idleexit_test.go b/store/idleexit_test.go new file mode 100644 index 00000000..f200a9e1 --- /dev/null +++ b/store/idleexit_test.go @@ -0,0 +1,82 @@ +package store + +import ( + "strings" + "testing" + "time" +) + +// TestIdleExitFloorRefusesADestructiveThreshold is about money, not tidiness. +// +// Process exit wipes this store, and what it wipes includes frozen decisions. A frozen +// decision that dies mid-session is the 11.5x cache-WRITE regression FrozenLost exists to +// detect: the next turn re-creates the whole prefix at write prices instead of reading it. So a +// short idle-exit threshold does not degrade gracefully — it turns a convenience feature into a +// cost regression that presents as the proxy misbehaving, on the machine of the first-time +// evaluator this whole funnel is aimed at. +// +// Hence a startup error rather than a doc comment. The 30-minute case below is the one somebody +// will actually reach for ("exit quickly, it is only a laptop"), and it must not start. +func TestIdleExitFloorRefusesADestructiveThreshold(t *testing.T) { + def := Options{} // ttl_seconds unset => DefaultTTL (10000s), floor 2x = 5h33m20s + if got, want := IdleExitFloor(def), 2*DefaultTTL; got != want { + t.Fatalf("IdleExitFloor(default) = %s, want %s", got, want) + } + + for _, c := range []struct { + name string + d time.Duration + o Options + wantErr bool + }{ + {"off is always valid", 0, def, false}, + {"negative is off too", -time.Hour, def, false}, + {"30m on the default TTL is destructive", 30 * time.Minute, def, true}, + {"1h is still below the default floor", time.Hour, def, true}, + {"just under the floor", 2*DefaultTTL - time.Second, def, true}, + {"exactly the floor is allowed", 2 * DefaultTTL, def, false}, + {"the installer's 24h default", 24 * time.Hour, def, false}, + // A tiny configured TTL must not collapse the floor to seconds: 2x30s is 1m, which is + // shorter than the keep-alive's own ping window, so the absolute 1h term takes over. + {"tiny ttl falls back to the 1h term", 30 * time.Minute, Options{TTLSeconds: 30}, true}, + {"tiny ttl accepts 1h", time.Hour, Options{TTLSeconds: 30}, false}, + // A LONG configured TTL must raise the floor above 1h, or an operator who deliberately + // widened the store's lifetime gets a threshold that expires it. + {"long ttl raises the floor above 24h", 24 * time.Hour, Options{TTLSeconds: 100000}, true}, + } { + err := ValidateIdleExit(c.d, c.o) + if c.wantErr && err == nil { + t.Errorf("%s: ValidateIdleExit(%s, ttl=%s) accepted a threshold below the %s floor", + c.name, c.d, c.o.EffectiveTTL(), IdleExitFloor(c.o)) + continue + } + if !c.wantErr && err != nil { + t.Errorf("%s: ValidateIdleExit(%s, ttl=%s) rejected a valid threshold: %v", + c.name, c.d, c.o.EffectiveTTL(), err) + continue + } + // The message has to tell the operator what to change. A bare "invalid value" here + // leaves them guessing at a number they have no reason to know. + if err != nil { + for _, want := range []string{"idle-exit", "floor"} { + if !strings.Contains(err.Error(), want) { + t.Errorf("%s: error message omits %q: %v", c.name, want, err) + } + } + } + } +} + +// TestEffectiveTTLIsWhatNewMemoryUses closes the gap the floor depends on: IdleExitFloor sizes +// a process's whole lifetime from EffectiveTTL, so a store that actually ran with a DIFFERENT +// lifetime would be protected by a floor computed for a lifetime it never had. NewMemory calls +// EffectiveTTL rather than repeating the defaulting rule, and this holds it there. +func TestEffectiveTTLIsWhatNewMemoryUses(t *testing.T) { + for _, o := range []Options{{}, {TTLSeconds: 0}, {TTLSeconds: -5}, {TTLSeconds: 42}, {TTLSeconds: 100000}} { + if got := NewMemory(o).ttl; got != o.EffectiveTTL() { + t.Errorf("NewMemory(%+v).ttl = %s but EffectiveTTL() = %s; the idle-exit floor is "+ + "computed from the second and would protect a lifetime the store is not using", + o, got, o.EffectiveTTL()) + } + } +} diff --git a/store/store.go b/store/store.go index a57e5e5d..eb0c9078 100644 --- a/store/store.go +++ b/store/store.go @@ -13,6 +13,7 @@ package store import ( "container/list" + "fmt" "strings" "sync" "time" @@ -162,13 +163,64 @@ func (Nop) Persists() bool { return false } // (test suites, training runs) with the sliding refresh doing the rest. const DefaultTTL = 10000 * time.Second +// EffectiveTTL is the entry lifetime this Options actually yields, defaulting included. +// +// Exported and used by NewMemory itself rather than duplicated, because a second copy of +// "zero means DefaultTTL" is exactly the drift that would matter: IdleExitFloor sizes a +// process's whole lifetime off this number, and a floor computed from a different default +// than the store runs with is a floor that protects nothing. +func (o Options) EffectiveTTL() time.Duration { + if o.TTLSeconds <= 0 { + return DefaultTTL + } + return time.Duration(o.TTLSeconds) * time.Second +} + +// IdleExitFloor is the shortest idle-exit threshold that is not destructive. +// +// Process exit WIPES this store: rewind stashes, frozen decisions, `cg:len:`. A frozen +// decision that dies mid-session is the 11.5x cache-WRITE regression that FrozenLost exists +// to detect — the session's next turn re-creates the whole prefix at write prices instead of +// reading it. So an idle-exit threshold shorter than the store's own entry lifetime turns a +// convenience feature into a cost regression that looks like the proxy misbehaving. +// +// 2x the TTL, with a 1h absolute floor. Twice, not once, because the TTL is a SLIDING +// window: an entry touched just before the idle clock started still has a full TTL ahead of +// it, so 1x can expire live state. The 1h term covers a config that sets a tiny ttl_seconds +// (a test rig, or an operator trimming memory) where 2x would collapse to seconds and the +// threshold would be shorter than the keep-alive's own ping window. +// +// With the default TTL of 10000s the floor is ~5h34m, so the installer's 24h default clears +// it comfortably; a 30-minute threshold is refused at startup rather than documented. +func IdleExitFloor(o Options) time.Duration { + if f := 2 * o.EffectiveTTL(); f > time.Hour { + return f + } + return time.Hour +} + +// ValidateIdleExit checks an idle-exit threshold against IdleExitFloor. Zero or negative +// means the watchdog is off, which is always valid — a gateway or eval-containers +// deployment must never self-terminate, so off is the default and the only way to a +// self-killing proxy is to ask for one. +func ValidateIdleExit(d time.Duration, o Options) error { + if d <= 0 { + return nil + } + if floor := IdleExitFloor(o); d < floor { + return fmt.Errorf("idle-exit %s is below the floor of %s (2x the store's %s entry "+ + "lifetime): exiting wipes the in-memory store, so a shorter threshold drops live "+ + "frozen decisions and re-bills their prefix as cache creation instead of a cache "+ + "read. Raise --idle-exit, or raise store.ttl_seconds if the short lifetime is "+ + "deliberate", d, floor, o.EffectiveTTL()) + } + return nil +} + // NewMemory builds an in-memory store. Zero/negative option fields fall back to // defaults (DefaultTTL, 1000 entries, 100 sessions of sticky sets). func NewMemory(o Options) *Memory { - ttl := time.Duration(o.TTLSeconds) * time.Second - if o.TTLSeconds <= 0 { - ttl = DefaultTTL - } + ttl := o.EffectiveTTL() max := o.MaxEntries if max <= 0 { max = 1000