diff --git a/.github/workflows/ci.yaml b/.github/workflows/ci.yaml index b79f658c..95e5a1e1 100644 --- a/.github/workflows/ci.yaml +++ b/.github/workflows/ci.yaml @@ -33,6 +33,57 @@ jobs: - name: Build run: make build + # The binary needs no C toolchain — asserted, not asserted-in-a-comment. + # + # Our own docs told every evaluator to install one (`docs/setup.md` even named bifrost's + # tokenizer as a cgo dependency, which it never was). The only cgo dependency is tree-sitter, + # behind the `cg_skeleton` build tag. Nothing checked that, and the `build-test` job above runs + # exclusively with CGO_ENABLED=1 because `go test -race` requires it — so the configuration a + # user would actually build was never exercised. + # + # CGO_ENABLED=0 is what does the work: with cgo off the toolchain never consults CC at all, so a + # dependency that needs it fails to BUILD here rather than in a stranger's terminal. CC is pointed + # at a nonexistent path anyway, as a tripwire for the day somebody sets CGO_ENABLED=1 in this job + # and expects it to still be testing the same thing. + purego: + runs-on: ubuntu-latest + env: + CGO_ENABLED: "0" + CC: /nonexistent-c-compiler + steps: + - uses: actions/checkout@v4 + - uses: actions/setup-go@v5 + with: + go-version: '1.25' + check-latest: true + - name: Build with no C compiler available + run: | + go build -o /tmp/cg-purego ./cmd/context-guru-proxy + file /tmp/cg-purego | tee /dev/stderr | grep -q "statically linked" + - name: It has to start, not just link + run: | + LISTEN_ADDR=127.0.0.1:4471 CONTEXT_GURU_KEEPALIVE=off /tmp/cg-purego & + 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 + # And the guard that matters most to a CGO-free build: a preset naming a component which is + # not registered without cgo starts the proxy and then fails at pipeline build. That is the + # `preset: coding` / `unknown component "skeleton"` breakage, and TestEveryPresetBuilds + # covers it — but it had never run in this configuration. + # -p 1: run one package binary at a time. + # + # A CI runner has 2 cores, and `go test` otherwise starts up to GOMAXPROCS package binaries + # in parallel. Adding this job therefore added a SECOND heavily-parallel run of the proxy + # package per PR, and under that contention a timing-sensitive control-plane test + # (TestCtlGetCampaignAggregatesPredictedAndRealPerTenant — rossoctl/context-guru#163) failed + # twice on unrelated PRs, then passed on re-run and passes 3/3 locally on main and on the + # branch. That flake is not this job's business to hunt, but it IS this job's business not to + # provoke it: serialising costs about a minute and removes the contention this job added. + - name: Test the packages whose behaviour depends on which components compile in + run: go test -p 1 ./config/... ./components/... ./apply/... ./proxy/... ./store/... + trivy: runs-on: ubuntu-latest steps: diff --git a/CLAUDE.md b/CLAUDE.md index c56ca73a..3654563f 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -23,7 +23,9 @@ reference — port its *logic*, re-implement its transport in Go. ## Conventions -- Go 1.26, module `github.com/rossoctl/context-guru`. Build needs `CGO_ENABLED=1` (tree-sitter). +- Go 1.26, module `github.com/rossoctl/context-guru`. The build is pure Go — `make build` sets + `CGO_ENABLED=0`. cgo is needed only for `go test -race` and for `-tags cg_skeleton` (tree-sitter), + which is why the Makefile still exports `CGO_ENABLED=1` for the test targets. - Match the surrounding code's style; keep packages small and single-purpose. - **Commits: DCO sign-off is mandatory** — `git commit -s`. Author as the repo owner. AI attribution uses `Assisted-By:` — never `Co-Authored-By`, never a "Generated with" diff --git a/Makefile b/Makefile index 9139f2b7..bca99df9 100644 --- a/Makefile +++ b/Makefile @@ -6,8 +6,16 @@ LDFLAGS := -s -w \ -X $(PKG)/internal/buildinfo.Version=$(VERSION) \ -X $(PKG)/internal/buildinfo.Commit=$(COMMIT) -# CGO is required to compile the tree-sitter binding; a C toolchain (gcc/clang) -# must be present for make test/build/lint. +# CGO is on for DEVELOPMENT, and the reason is `go test -race`, which does not work +# without it. It is NOT a requirement of the shipped binary: tree-sitter is the only cgo +# dependency in the tree and it is behind the `cg_skeleton` build tag, so a default build +# is pure Go, and `make build` now builds it that way — so the documented "no C toolchain" claim +# is true of the command the docs actually tell you to run, which it was not while this variable +# applied to every target. CI proves it with no compiler on PATH at all (the `purego` job). +# +# Reading this variable as a SHIPPING requirement is what put "install a C toolchain" at the top of +# our own quickstart, and named bifrost's tokenizer as a cgo dependency in docs/setup.md, which it +# never was. It is needed for `go test -race` and for `-tags cg_skeleton`; nothing else. export CGO_ENABLED=1 .DEFAULT_GOAL := help @@ -18,9 +26,15 @@ help: ## Display this help awk 'BEGIN {FS = ":.*?## "}; {printf " \033[36m%-12s\033[0m %s\n", $$1, $$2}' .PHONY: build -build: ## Build the context-guru-proxy binary into ./bin +build: ## Build the context-guru-proxy binary into ./bin (pure Go — no C toolchain needed) @mkdir -p bin - go build -ldflags "$(LDFLAGS)" -o bin/$(BINARY) ./cmd/context-guru-proxy + CGO_ENABLED=0 go build -ldflags "$(LDFLAGS)" -o bin/$(BINARY) ./cmd/context-guru-proxy + +.PHONY: build-static +build-static: ## Same as build, plus -trimpath — the exact build releases ship + @mkdir -p bin + CGO_ENABLED=0 go build -trimpath -ldflags "$(LDFLAGS)" -o bin/$(BINARY) ./cmd/context-guru-proxy + @file bin/$(BINARY) 2>/dev/null || true .PHONY: test test: ## Run all tests with the race detector diff --git a/README.md b/README.md index 2382368a..3fd159ec 100644 --- a/README.md +++ b/README.md @@ -85,7 +85,9 @@ flowchart TD ## Install -Requires **Go 1.26** and a **C toolchain** (`CGO_ENABLED=1`). Build from the repo root: +Requires **Go 1.26**; no C toolchain — `make build` builds with cgo off and produces a statically +linked binary. (A C compiler is needed only for `make test`'s race detector and the optional +`cg_skeleton` tag.) Build from the repo root: ```sh CGO_ENABLED=1 go build -tags cg_skeleton -o bin/context-guru-proxy ./cmd/context-guru-proxy @@ -108,7 +110,7 @@ docker build -t context-guru:local . ```sh # 1 — run the proxy (ships with the SWE-bench-winning cache-aware config by default) -./bin/context-guru-proxy # --preset codesmart; listens on :4000 (LISTEN_ADDR to change) +./bin/context-guru-proxy # --preset house (the default); listens on :4000 # 2 — point any agent at it (one port serves both dialects) export ANTHROPIC_BASE_URL=http://localhost:4000/anthropic @@ -129,9 +131,10 @@ curl -s localhost:4000/anthropic/v1/messages \ -d '{"model":"...","max_tokens":64,"messages":[ ... ]}' ``` -Presets: **`codesmart`** (the default — the SWE-bench-winning cache-aware config -`[format, toon, dedup, failed_run, cmdfilter, extract_llm, extract, cachesplit]`), **`codesafe`** (the same -minus the LLM pass — deterministic-only `[format, dedup, failed_run, cmdfilter, extract, collapse, cachesplit]`, +Presets: **`house`** is the binary's default. **`codesmart`** is the SWE-bench-winning +cache-aware config, `[format, textclean, searchfold, dedup, failed_run, cmdfilter, extract_llm, extract, linecap, cachesplit]`, and is what the +published benchmark numbers describe — pass `--preset codesmart` to run it. **`codesafe`** (the same +minus the LLM pass — deterministic-only `[format, textclean, searchfold, dedup, failed_run, cmdfilter, extract, collapse, linecap, cachesplit]`, zero model calls by policy), plus `general`, `agent`, `aggressive`, `coding`, `mcp`, `balanced`, `safe`, `summarize`, `off`. `codesmart`'s LLM relevance-trimmer (`extract_llm`) engages only when a cheap model is configured @@ -140,7 +143,7 @@ See [docs/components.md](docs/components.md) and [docs/reference/presets.md](doc | Flag / env | Default | Purpose | |---|---|---| -| `--preset` / `PRESET` | `codesmart` | pipeline preset when no `--config` | +| `--preset` / `PRESET` | `house` | pipeline preset when no `--config` | | `--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/components/all/hasoffload_test.go b/components/all/hasoffload_test.go new file mode 100644 index 00000000..085ba96e --- /dev/null +++ b/components/all/hasoffload_test.go @@ -0,0 +1,58 @@ +package all + +import ( + "testing" + + "github.com/rossoctl/context-guru/components" +) + +// Pipeline.HasOffload must agree with the Offload interface for every component the binary can +// build — that agreement is the whole reason the gate asks the interface instead of keeping a list +// of "lossy" component names, and it is what stops the list-shaped version of this from rotting. +// +// This lives in `all` rather than in `components` because the registry is only populated here: the +// blank imports in all.go are what run each component's init(). The same test inside `components` +// skipped for lack of registrations, which looked like coverage and was not. +// +// It walks the registry rather than a fixture, so a new lossy component needs no change here, and a +// component that quietly stops implementing Offload fails immediately. +func TestHasOffloadAgreesWithTheOffloadInterface(t *testing.T) { + names := components.Names() + if len(names) == 0 { + t.Fatal("no components registered: this package's blank imports are what register them, " + + "so an empty registry means the test is checking nothing") + } + var checked, offloaders int + for _, name := range names { + c, err := components.New(name, nil) + if err != nil || c == nil { + // A component that will not build from a nil config block; the proxy-level tests + // exercise those with real configuration. + continue + } + _, isOffload := c.(components.Offload) + if isOffload { + offloaders++ + } + got := components.NewPipeline([]components.Component{c}, nil).HasOffload() + if got != isOffload { + t.Errorf("component %q: implements components.Offload = %v, but a pipeline holding "+ + "only it reports HasOffload() = %v", name, isOffload, got) + } + checked++ + } + if checked == 0 { + t.Fatal("no component could be built from a nil config, so nothing was actually checked") + } + // Both sides must be non-empty or the agreement is trivial: all-false would agree with a + // HasOffload that always returns false, which is the mirror-image defect. + if offloaders == 0 { + t.Errorf("checked %d components and none implement Offload — either the registry is not "+ + "what it should be, or this test would pass against a broken HasOffload", checked) + } + if offloaders == checked { + t.Errorf("all %d checked components implement Offload, so this test would also pass "+ + "against a HasOffload that always returns true", checked) + } + t.Logf("checked %d of %d registered components; %d implement Offload", checked, len(names), offloaders) +} diff --git a/components/hasoffload_test.go b/components/hasoffload_test.go new file mode 100644 index 00000000..7d050490 --- /dev/null +++ b/components/hasoffload_test.go @@ -0,0 +1,66 @@ +package components + +import ( + "testing" + + schemas "github.com/maximhq/bifrost/core/schemas" +) + +// HasOffload's whole selling point is that it cannot rot the way a list of component names would, +// so it is worth a test in its own package rather than only proxy-level integration coverage. +// +// It is a type assertion over the pipeline's components, and the property that matters is that it +// answers by INTERFACE: a component that drops bytes must satisfy components.Offload, and adding +// one must not require anybody to remember to update a second list somewhere. These fakes stand in +// for the three shapes a real component takes. + +type offloadStub struct{ name string } + +func (o offloadStub) Name() string { return o.name } +func (o offloadStub) Enabled(*Ctx) bool { return true } +func (o offloadStub) Offload(*schemas.BifrostChatRequest, *Report, *Ctx) ([]string, error) { + return []string{"key"}, nil +} + +type reformatStub struct{ name string } + +func (r reformatStub) Name() string { return r.name } +func (r reformatStub) Enabled(*Ctx) bool { return true } +func (r reformatStub) Reformat(*schemas.BifrostChatRequest, *Report, *Ctx) error { return nil } + +func TestHasOffload(t *testing.T) { + for _, c := range []struct { + name string + comps []Component + want bool + }{ + {"nil pipeline", nil, false}, + {"empty pipeline — the A/B control arm", []Component{}, false}, + {"reformatters only", []Component{reformatStub{"format"}, reformatStub{"cachesplit"}}, false}, + {"one offloader", []Component{offloadStub{"linecap"}}, true}, + {"an offloader among reformatters", []Component{ + reformatStub{"format"}, offloadStub{"mask"}, reformatStub{"cachesplit"}}, true}, + // Position must not matter: the host asks "can this pipeline ever mint a marker", which is + // a property of the set, not of the order. + {"offloader last", []Component{reformatStub{"format"}, offloadStub{"extract"}}, true}, + } { + t.Run(c.name, func(t *testing.T) { + var p *Pipeline + if c.comps != nil || c.name != "nil pipeline" { + p = NewPipeline(c.comps, nil) + } + if got := p.HasOffload(); got != c.want { + t.Errorf("HasOffload() = %v, want %v", got, c.want) + } + }) + } +} + +// TestHasOffloadIsNilSafe: the host calls this on a per-request pipeline that a fail-open path may +// leave nil, and a panic there would take down a request rather than degrade it. +func TestHasOffloadIsNilSafe(t *testing.T) { + var p *Pipeline + if p.HasOffload() { + t.Error("a nil pipeline reported an offloader") + } +} diff --git a/components/pipeline.go b/components/pipeline.go index 9be77d07..4477e98e 100644 --- a/components/pipeline.go +++ b/components/pipeline.go @@ -286,6 +286,30 @@ func (p *Pipeline) Find(name string) Component { return nil } +// HasOffload reports whether any component in this pipeline is an Offload — i.e. whether a +// request through it can produce a `<>` marker at all. +// +// Asked by the host to decide whether advertising `context_guru_expand` is meaningful. A +// pipeline with no Offload mints no markers, so every expand call a model makes against it MUST +// fail: there is nothing in the Store to resolve. Advertising it there costs a wasted round trip +// and a step of the user's turn to learn that. +// +// A type assertion, deliberately, rather than a list of component names: a name list is a second +// copy of "which components are lossy" that drifts the moment somebody adds one, and the +// interface is the definition. `components.Offload` cannot be implemented by accident — it +// requires returning cache keys that prove the original was stashed. +func (p *Pipeline) HasOffload() bool { + if p == nil { + return false + } + for _, c := range p.comps { + if _, ok := c.(Offload); ok { + return true + } + } + return false +} + // Has reports whether a component with this name is configured in the pipeline. // Hosts use it to gate body-level work that belongs to a component's concern but // cannot be done inside it — e.g. cacheinject's cache-prefix repair, which must diff --git a/config/config.go b/config/config.go index 977d0a0c..4bc6c0b0 100644 --- a/config/config.go +++ b/config/config.go @@ -402,8 +402,11 @@ var presets = map[string][]string{ // stacking our offloaders beside it would reduce the same tool outputs first and // there would be nothing left to attribute. "agentdiet": {"format", "agentdiet", "cachesplit"}, - // codesmart / codesafe are the SWE-bench study's winning configs, shipped as the - // recommended defaults (codesmart is the proxy default). Their tuned per-component + // codesmart / codesafe are the SWE-bench study's winning configs and the RECOMMENDED + // pipelines — but neither is the proxy default, which is `house` (see the --preset flag in + // cmd/context-guru-proxy/main.go). Asserting it here is how five documents came to repeat + // it: the claim was copied from three lines away from the flag that disproves it. Their + // tuned per-component // settings live in presetConfigs; the name-lists here keep PresetPipeline (used by // /compact?preset=) resolving them. "codesmart": {"format", "textclean", "searchfold", "dedup", "failed_run", "cmdfilter", "extract_llm", "extract", "linecap", "cachesplit"}, @@ -442,7 +445,7 @@ var presets = map[string][]string{ // reader relies on when deciding whether the published numbers describe the shipped // default. They describe an ancestor of it. Treat any preset change as a reason to // re-measure, not as a documentation edit. -// - codesmart (the winning cache-aware config, and the proxy default): the LLM +// - codesmart (the winning cache-aware config; NOT the proxy default, which is `house`): the LLM // relevance-trimmer extract_llm routed to the CHEAP model (model.source: config, // nil-when-unset ⇒ it silently no-ops to deterministic — see docs), gated at 3000 // tok so most turns make no model call, ≤4 calls/req; the free deterministic extract diff --git a/docs/components.md b/docs/components.md index f5643e5f..410a656d 100644 --- a/docs/components.md +++ b/docs/components.md @@ -31,7 +31,7 @@ messages (`role:"tool"`; for Anthropic, `tool_result` blocks normalized to that | `summarize` | Offload (LLM) | the middle of the transcript → one summary | via expand | long trajectories | `summary_level` (regular), `keep_last` (3), `min_tokens` (500), `resummarize_tokens` (6000), `model.source`, `trigger` | | [`agentdiet`](components/agentdiet.md) | Offload (LLM) | useless/redundant/**expired** content in the step that just aged past the delay | via expand | a step above `min_step_tokens`, `delay_steps` turns back | `delay_steps` (2), `context_steps` (1), `min_step_tokens` (500), `min_saved_tokens` (400), `max_keep_ratio` (0.8), `model.source` | -Presets (`config/config.go`), verbatim: **`codesmart`** (the proxy default) +Presets (`config/config.go`), verbatim: **`house`** (the proxy default), **`codesmart`** (the SWE-bench arm) `[format, textclean, searchfold, dedup, failed_run, cmdfilter, extract_llm, extract, linecap, cachesplit]` · **`codesafe`** `[format, textclean, searchfold, dedup, failed_run, cmdfilter, extract, collapse, linecap, cachesplit]` @@ -79,10 +79,12 @@ Absolutes (`min_request_tokens`, etc.) still win; when the window is unknown, fr absolutes apply (backward compatible). This lets one config generalize across models/benchmarks. **Reversibility in practice.** The `context_guru_expand` tool is advertised on outgoing requests -(`INJECT_EXPAND=auto|always|never`, default `auto` = whenever the request already declares -tools and the store persists), so Offload markers are genuinely recoverable — not just described in marker text. -Both conditions are properties of the **session**, not of the turn, so the `tools` array a session -sends is byte-identical on every request in it. That matters more than it looks: `tools` sits ahead of +(`INJECT_EXPAND=auto|always|never`, default `auto` = whenever the request already declares tools, +the store persists, **and the pipeline contains at least one Offload**), so Offload markers are +genuinely recoverable — not just described in marker text. That third condition is what keeps the +tool off pipelines that mint no markers, where every call to it would have to fail. +All three conditions are properties of the **session**, not of the turn, so the `tools` array a +session sends is byte-identical on every request in it. That matters more than it looks: `tools` sits ahead of `system` and `messages` in the provider's prompt-cache hash, so the first request carrying a **new** tools array re-creates the **entire** prefix at the write rate. `auto` used to also require a marker on the request, which made the array grow on the first offloading turn and shrink again on the next turn diff --git a/docs/get-started/connect-ibm-service.md b/docs/get-started/connect-ibm-service.md index e9906054..a37c4fa0 100644 --- a/docs/get-started/connect-ibm-service.md +++ b/docs/get-started/connect-ibm-service.md @@ -19,7 +19,7 @@ your agent ──▶ https://contextguru.vpc.cloud9.ibm.com ──▶ the model | Your provider key | **stays yours** — your agent keeps sending it, and the proxy forwards it upstream unchanged | | Cost control | none needed: every account's traffic is billed to that account's own provider credential | | Transcript capture | your account consents **on registration** — [what that means, and the off switch](#three-things-worth-knowing-before-you-rely-on-it) | -| Default pipeline | `[format, toon, dedup, failed_run, cmdfilter, extract, cachesplit]`, `mode: sync` | +| Default pipeline | `[format, dedup, toon, cmdfilter, searchfold, textclean, extract, cachesplit, toolfilter]` (the `house` preset), `mode: sync` | The default pipeline is **fully deterministic** — no cheap-model calls anywhere in it. That is why it is the default on a shared box: it adds no upstream spend, contends for no shared diff --git a/docs/get-started/quickstart-proxy.md b/docs/get-started/quickstart-proxy.md index b4bb7255..e82ebb44 100644 --- a/docs/get-started/quickstart-proxy.md +++ b/docs/get-started/quickstart-proxy.md @@ -3,9 +3,13 @@ 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** and a **C toolchain** (`CGO_ENABLED=1`). Everything else is a normal +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. +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: @@ -17,7 +21,7 @@ module dependency — build straight from the repo root. 2. Run it. It listens on `:4000`; set `LISTEN_ADDR` to change that. ```sh - ./bin/context-guru-proxy # default preset: codesmart + ./bin/context-guru-proxy # default preset: house ``` 3. Point your agent at it: diff --git a/docs/hosted.md b/docs/hosted.md index fdb38c91..c7bd8ad8 100644 --- a/docs/hosted.md +++ b/docs/hosted.md @@ -86,7 +86,7 @@ Everything below was derived from the scripts in `deploy/service/`, which are th | For | You need | |---|---| -| Building the binary | Go 1.26 and a C toolchain (`CGO_ENABLED=1`) — or an already-installed `/usr/local/bin/context-guru-proxy`, which `install` keeps if there is no fresh build | +| Building the binary | Go 1.26 — no C toolchain: the binary is pure Go and statically linked (`make build-static`, which CI proves needs no C compiler). Or an already-installed `/usr/local/bin/context-guru-proxy`, which `install` keeps if there is no fresh build. A C toolchain is needed only for the optional `cg_skeleton` tag | | Installing | root, and systemd. The scripts are written for **RHEL 9** (`dnf`, and nginx 1.20's config dialect) | | The TLS front end | `nginx`, plus a certificate and key at `/etc/context-guru/tls/{fullchain,privkey}.pem` | | Each upstream | a host to send it to. A key only if you want the server to hold one — the default forwards each caller’s own | diff --git a/docs/how-to/choose-a-preset.md b/docs/how-to/choose-a-preset.md index 7f3b501f..313f6f15 100644 --- a/docs/how-to/choose-a-preset.md +++ b/docs/how-to/choose-a-preset.md @@ -11,7 +11,7 @@ context-guru-proxy --preset codesmart # or PRESET=codesmart, or preset: in | Your workload | Preset | |---|---| -| **Most agents — the default** | **`codesmart`** | +| **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` | | General non-agentic traffic | `balanced` | diff --git a/docs/how-to/use-with-claude-code.md b/docs/how-to/use-with-claude-code.md index 2a7cf217..ff3eaefd 100644 --- a/docs/how-to/use-with-claude-code.md +++ b/docs/how-to/use-with-claude-code.md @@ -81,7 +81,7 @@ ANTHROPIC_BASE_URL=https://127.0.0.1:1/nope ANTHROPIC_AUTH_TOKEN=bogus \ claude -p 'say PONG' --max-turns 1 # must FAIL ``` -**Which preset?** `codesmart` is the default and the cheapest arm in the +**Which preset?** `codesmart` is the recommended pipeline and the cheapest arm in the [benchmarks](../RESULTS.md) at the highest reward. Use `coding` if you want `skeleton` to strip function bodies out of large source reads — it needs a `cg_skeleton` build. See [Choose a preset](choose-a-preset.md). diff --git a/docs/reference/config.md b/docs/reference/config.md index 721f0716..51717b72 100644 --- a/docs/reference/config.md +++ b/docs/reference/config.md @@ -76,7 +76,7 @@ for every component's config block. | Flag / env | Default | Purpose | |---|---|---| -| `--preset` / `PRESET` | `codesmart` | Pipeline preset when no `--config`. | +| `--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. | | `--openai-upstream` / `OPENAI_UPSTREAM` | `https://api.openai.com` | OpenAI upstream base. | @@ -85,7 +85,7 @@ for every component's config block. | `OPENAI_API_KEY` / `ANTHROPIC_API_KEY` | — | Real key injected on forward (gateway mode); empty = pass client auth through. | | `CHEAP_MODEL` (+ `CHEAP_MODEL_BASE` / `_KEY` / `_AUTH` / `_PROVIDER`) | — | Dedicated cheap model for the LLM components (`extract_llm`, `summarize`) — the `model.source: config` client. Without it they no-op. | | `FORCE_MODEL` | — | Overwrite the request `model` (eval-containers uses `EVAL_MODEL`). | -| `INJECT_EXPAND` | `auto` | Whether the `context_guru_expand` tool is advertised: `auto` (whenever the request already declares tools and the store persists — both session-stable, so the `tools` array never changes shape mid-session and the prompt-cache prefix survives) \| `always` (also when the request declares no tools, creating the array) \| `never`. | +| `INJECT_EXPAND` | `auto` | Whether the `context_guru_expand` tool is advertised: `auto` (whenever the request declares tools, the store persists, **and the pipeline contains at least one Offload** — all three session-stable, so the `tools` array never changes shape mid-session and the prompt-cache prefix survives) \| `always` (unconditional, including when the request declares no tools) \| `never`. The pipeline condition exists because a pipeline that mints no markers would be advertising a tool whose every call must fail. | | `CACHE_MODE` | `auto` | Cache-aware compaction: `auto` (on when the agent sets its own breakpoints) \| `on` \| `off`. | | `MODEL_INFO_URL` / `MODEL_INFO` | LiteLLM map | Source for context-window sizes (used by the fractional triggers). `MODEL_INFO=off` disables the lookup; fractions are then ignored and absolutes apply. | | `MODEL_PRICES` | — | Path to an **operator price list**, consulted before the public map. See below. A file that fails to load is fatal. | diff --git a/docs/reference/routes.md b/docs/reference/routes.md index 318c4789..89daa994 100644 --- a/docs/reference/routes.md +++ b/docs/reference/routes.md @@ -8,6 +8,7 @@ The proxy serves both provider dialects on one port (default `:4000`). |---|---| | `POST /openai/v1/chat/completions` | OpenAI chat dialect — runs the pipeline, forwards to the OpenAI upstream. | | `POST /anthropic/v1/messages` | Anthropic Messages dialect — runs the pipeline, forwards to the Anthropic upstream. | +| `POST /anthropic/v1/messages/count_tokens` | Token counting, forwarded **verbatim** — the pipeline does not run. Absent this route a client falls back to counting context with *inference* requests, which a proxy sold on reducing spend must not cause. See the note below on what it costs. | | `POST /compact` | Stateless compaction: run the pipeline and return the rewritten body, no upstream call. `?provider=anthropic` switches dialect; `?preset=` / `x-context-guru-pipeline` override the pipeline; `?cache=on\|off\|auto` overrides cache-awareness. | | `GET /healthz` | Liveness check. | | `GET /stats` | Savings rollups and health counters — see below. | @@ -15,6 +16,30 @@ The proxy serves both provider dialects on one port (default `:4000`). | `GET /expand?id=` | Recover an offloaded original by its `<>` id. Scoped to the caller's session. | | `GET /favicon.ico` | `204`. Present so a browser's unprompted request does not fall through to the Bob catch-all below and answer `401`. | + +### `count_tokens` answers about the ORIGINAL body, and that is deliberate + +The route forwards the client's body unchanged, so the count it returns describes what the client +sent — not what context-guru will forward. That is the safe direction and it is the literal API +answer, but it has a cost worth stating plainly. + +Returning the *compacted* count would be smaller and would look better. It would also be wrong in +the dangerous direction: the client would believe it has more room than it does, and because every +component fails open (a reverted component forwards the full body), the very next request could +send the uncompacted body and take a `400`. Over-reporting is recoverable; under-reporting is a +failed turn. + +The cost: Claude Code uses this number to decide when to run **its own** compaction, so a routed +session self-compacts earlier than it needs to — paying for a summarization call and discarding +transcript the proxy was already handling. On a measured body: `115,933` tokens reported, +`32,802` actually forwarded. The count also excludes any tool declaration the proxy adds. + +Two upstream caveats: on an implicit prefix-cache backend the numbers are unaffected because +`cachesplit` is a no-op there, and at least one LiteLLM-fronted gateway answers this endpoint with +an implausible count (`13` for a body whose system prompt alone is ~7,929 tokens) — the same answer +it gives when called directly, so the undercount is upstream's, but it means the route's +cheap-budgeting justification does not hold on that upstream. + ### Bob (BobShell) gateway routes Mounted only when `--bob-upstream` / `BOB_UPSTREAM` is set, or in hosted mode. Without diff --git a/docs/setup.md b/docs/setup.md index fc5a2460..aa63ef66 100644 --- a/docs/setup.md +++ b/docs/setup.md @@ -5,9 +5,17 @@ SWE-bench task driven by Claude Code. ## Prerequisites -- **Go 1.26** and a **C toolchain** — `CGO_ENABLED=1` (bifrost's tokenizer and, with the - `cg_skeleton` tag, tree-sitter, use cgo). bifrost is an ordinary module dependency; nothing - to check out beside this repo. +- **Go 1.26**. A **C toolchain** is needed only for the `cg_skeleton` tag used below + (tree-sitter), and for `make test`'s race detector — *not* for the binary itself, which is + 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. + + 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. - **Docker** (for the gateway image / eval-containers), and the **eval-containers** repo. ## Build diff --git a/proxy/agentcompaction_test.go b/proxy/agentcompaction_test.go index b4eed3a0..7c586322 100644 --- a/proxy/agentcompaction_test.go +++ b/proxy/agentcompaction_test.go @@ -229,7 +229,12 @@ func TestExpandToolAdvertisedOnEveryTurnButNeverOnACompaction(t *testing.T) { })) defer upstream.Close() - h, st := buildHandler(t, "pipeline: []\n", upstream.URL) + // A pipeline that CAN offload: advertising is gated on that, because a pipeline + // which mints no markers would advertise a tool whose every call must fail. The + // premise of this test is that an offload happened (hence the store seed below), + // so the fixture has to be a pipeline where that is possible. `linecap` does not + // act on these short bodies, so the forwarded bytes are unchanged. + h, st := buildHandler(t, "pipeline: [linecap]\n", upstream.URL) st.Put("HASH", []byte("THE ORIGINAL CONTENT")) srv := httptest.NewServer(h.Mux()) defer srv.Close() diff --git a/proxy/ccbody_test.go b/proxy/ccbody_test.go new file mode 100644 index 00000000..dac15980 --- /dev/null +++ b/proxy/ccbody_test.go @@ -0,0 +1,60 @@ +// Claude-Code-shaped request fixtures, shared by the proxy's wire-level tests. +// +// They live in their own file because several test groups need them and each was inventing its own +// body, which is how a fixture ends up not resembling what the client actually sends. +package proxy_test + +import ( + "encoding/json" + "strings" + "testing" +) + +// cachePipeline is a cachesplit-only pipeline: the shape a cache-focused deployment runs, and the +// one whose promise is that nothing else touches the request. +const cachePipeline = "pipeline: [cachesplit]\n" + +// attributionText is the block Claude Code prepends as the first system block. +const attributionText = "You are Claude Code, Anthropic's official CLI for Claude." + +// jsonStr quotes a Go string as a JSON string. +func jsonStr(v string) string { + b, _ := json.Marshal(v) + return string(b) +} + +// volatileSystemText is a system block over cachesplit's 1024-token floor (~4 chars/token) that +// ends in the environment snapshot the split exists to move out of the hashed prefix. +func volatileSystemText() string { + return strings.Repeat("You are a coding agent. Follow the instructions carefully.\n", 120) + + "\nCurrent branch: main\nRecent commits:\n0898367954 SWE-bench\n" +} + +// claudeCodeBody is a Claude-Code-shaped Anthropic request: a small attribution block as the +// FIRST system block, then a large one ending in the volatile environment snapshot, with the +// cache breakpoint at its end. +// +// The JSON is assembled as TEXT rather than marshalled from a map, and that is not fussiness. +// Go's json.Marshal sorts map keys, so a map-built body arrives pre-sorted — and a proxy bug +// that re-encodes blocks instead of passing their original bytes through would then produce +// byte-identical output and no test would notice. Claude Code sends `{"type":...,"text":...}`, +// unsorted, and the API's positional strip of the attribution block depends on those bytes +// arriving unchanged. So the fixture has to carry the real order. +func claudeCodeBody(t *testing.T, stream bool) []byte { + t.Helper() + return claudeCodeBodyWithFirst(t, stream, attributionText) +} + +func claudeCodeBodyWithFirst(t *testing.T, stream bool, first string) []byte { + t.Helper() + body := `{"model":"claude-sonnet-5","max_tokens":64,"stream":` + + map[bool]string{true: "true", false: "false"}[stream] + + `,"system":[` + + `{"type":"text","text":` + jsonStr(first) + `},` + + `{"type":"text","text":` + jsonStr(volatileSystemText()) + `,"cache_control":{"type":"ephemeral"}}` + + `],"messages":[{"role":"user","content":"hello"}]}` + if !json.Valid([]byte(body)) { + t.Fatalf("test fixture is not valid JSON: %s", body) + } + return []byte(body) +} diff --git a/proxy/counttokens.go b/proxy/counttokens.go new file mode 100644 index 00000000..ea2844b8 --- /dev/null +++ b/proxy/counttokens.go @@ -0,0 +1,99 @@ +package proxy + +import ( + "bytes" + "io" + "log/slog" + "net/http" +) + +// countTokensPath is the upstream route this forwards to, and the suffix Claude Code appends +// to whatever ANTHROPIC_BASE_URL it was given. +const countTokensPath = "/v1/messages/count_tokens" + +// countTokens forwards POST /v1/messages/count_tokens to the Anthropic upstream, verbatim. +// +// **Why it has to exist.** Claude Code asks the API how many tokens its context is worth, to +// decide when to compact. Route the client through a gateway that does not serve this endpoint +// and the fallback is not a warning — the client works it out by issuing INFERENCE requests +// instead. On a funnel whose entire pitch is "this makes your sessions cheaper", the absence of +// a cheap endpoint silently adds billed calls. Cheap to add; expensive to leave out. +// +// **Why the body is forwarded UNCHANGED, with no pipeline.** The client is asking about the +// context IT holds, and it uses the answer to manage its own transcript. Handing back a count +// for a compacted body would answer a question nobody asked and would make the client's own +// budgeting wrong in the direction that hurts — it would believe it has more room than it does, +// and only find out at a 400 on the real request. So: no components, no cache split, no +// rewriting. The count is the truth about what the client sent. +// +// The response is relayed through h.stream, which copies status, headers and body byte for byte +// — the same path the chat routes use, and the reason an upstream error here reaches the client +// with the upstream's own wording intact. +func (h *Handler) countTokens(static upstream) http.HandlerFunc { + return func(w http.ResponseWriter, r *http.Request) { + up := static + if h.opts.Tenants != nil { + // Hosted mode: authenticate, meter and resolve the tenant's own upstream, exactly + // as the catch-all does. Without this the route would be an unmetered open + // forwarder that also leaked OUR token to the upstream in place of the tenant's. + tn, err := h.authenticate(r) + if err != nil { + h.refuse(w, r, err) + return + } + release, ok := h.meter(w, r, tn) + defer release() + if !ok { + return + } + if up, err = h.upstreamFor(r, tn, pickAnthropic, static); err != nil { + h.refuseRoute(w, r, tn, err) + return + } + } + if up.base == "" { + recordRefusal(refuseNoUpstream, "") + http.Error(w, "no upstream configured", http.StatusBadGateway) + return + } + + r.Body = http.MaxBytesReader(w, r.Body, maxRequestBytes) + body, err := io.ReadAll(r.Body) + if err != nil { + http.Error(w, "count_tokens: unreadable request body", http.StatusBadRequest) + return + } + req, err := http.NewRequestWithContext(r.Context(), http.MethodPost, + up.base+countTokensPath, bytes.NewReader(body)) + if err != nil { + http.Error(w, "proxy: "+err.Error(), http.StatusBadGateway) + return + } + copyHeaders(req.Header, r.Header) + setUpstreamAuth(req.Header, up) + + resp, err := h.client.Do(req) + if err != nil { + recordRefusal(refuseUpstream, "") + // The fixed string, not err.Error(): a *url.Error stringifies as + // `Post "": ...`, which would publish the operator's + // upstream address — and any userinfo in it — to every caller. Detail goes to + // the log, where the operator can see it and the caller cannot. + slog.Warn("context-guru: count_tokens upstream call failed", "err", err) + http.Error(w, "upstream request failed", http.StatusBadGateway) + return + } + h.stream(w, resp) + } +} + +// anthropicCountTokensUpstream is the single-tenant upstream for the route above: the same +// base and the same credential handling as POST /anthropic/v1/messages, so a proxy configured +// with an API key injects it here too, and one configured without forwards the caller's own. +func (h *Handler) anthropicCountTokensUpstream() upstream { + return upstream{ + base: h.opts.AnthropicUpstream, + path: countTokensPath, + setKey: headerKey("x-api-key", h.opts.AnthropicKey), + } +} diff --git a/proxy/counttokens_hosted_test.go b/proxy/counttokens_hosted_test.go new file mode 100644 index 00000000..8baffb7f --- /dev/null +++ b/proxy/counttokens_hosted_test.go @@ -0,0 +1,66 @@ +package proxy + +import ( + "net/http" + "strings" + "testing" +) + +// The hosted branch of the count_tokens route. +// +// This is here because the branch was shipped untested. `Mux()` has one caller, shared between +// the single-tenant binary and the hosted service, so the multi-tenant deployment serves this +// route — and the `h.opts.Tenants != nil` block is the only thing standing between it and an +// unmetered open forwarder that would send OUR credential upstream in place of the caller's. +// The conformance tests build a handler with `Tenants == nil`, so they exercise none of it. + +// TestCountTokensHostedRequiresAuth: no token, no forwarding. The failure this prevents is not +// subtle — an unauthenticated POST that reaches the upstream is an open relay on somebody else's +// credential, and it would also be unmetered. +func TestCountTokensHostedRequiresAuth(t *testing.T) { + f := newHostedFixture(t, "up1", "anthropic") + const path = "/anthropic/v1/messages/count_tokens" + + if w := f.post(path, "", ""); w.Code != http.StatusUnauthorized { + t.Errorf("%s without a token = %d, want 401", path, w.Code) + } + if w := f.post(path, "cg_live_"+strings.Repeat("A", 26), ""); w.Code != http.StatusUnauthorized { + t.Errorf("%s with an unissued token = %d, want 401", path, w.Code) + } + // And nothing reached the upstream on either attempt. + f.mu.Lock() + n := len(f.seen) + f.mu.Unlock() + if n != 0 { + t.Fatalf("%d unauthenticated request(s) reached the upstream", n) + } +} + +// TestCountTokensHostedForwardsWithTheTenantsCredential: an authenticated call resolves the +// TENANT's upstream and injects the configured key, exactly as the chat route does — and the +// caller's own context-guru token must not travel with it. +func TestCountTokensHostedForwardsWithTheTenantsCredential(t *testing.T) { + f := newHostedFixture(t, "up1", "anthropic") + _, tok := f.register(t, "user@ibm.com") + + w := f.post("/anthropic/v1/messages/count_tokens", tok, "") + if w.Code != http.StatusOK { + t.Fatalf("authenticated count_tokens = %d, want 200: %s", w.Code, w.Body.String()) + } + up := f.lastUpstream(t) + if got := up.URL.Path; got != countTokensPath { + t.Errorf("upstream path = %q, want %q", got, countTokensPath) + } + // The server key is injected in the slot the Anthropic dialect uses... + if got := up.Header.Get("x-api-key"); got != "real-upstream-secret" { + t.Errorf("upstream x-api-key = %q, want the injected server key", got) + } + // ...and OUR token does not leak upstream. This is the same rule the catch-all documents: + // the caller's header holds a context-guru credential, which must not leave the box. + if got := up.Header.Get(TokenHeader); got != "" { + t.Errorf("the context-guru token leaked upstream in %s: %q", TokenHeader, got) + } + if got := up.Header.Get("Authorization"); strings.Contains(got, tok) { + t.Errorf("the context-guru token leaked upstream in Authorization: %q", got) + } +} diff --git a/proxy/counttokens_test.go b/proxy/counttokens_test.go new file mode 100644 index 00000000..14e73518 --- /dev/null +++ b/proxy/counttokens_test.go @@ -0,0 +1,87 @@ +// The count_tokens route: served at all, forwarded verbatim, errors passed through. +package proxy_test + +import ( + "fmt" + "github.com/tidwall/gjson" + "io" + "net/http" + "net/http/httptest" + "strings" + "testing" +) + +// TestCountTokensIsServed covers conformance item 5. +// +// Without this route the client gets a 404 and falls back to counting context by issuing +// INFERENCE requests — billed calls added by a proxy sold on removing them. The body must arrive +// unmodified (the client is asking about the context IT holds, and uses the answer to budget its +// own transcript), and the answer must come back verbatim. +func TestCountTokensIsServed(t *testing.T) { + var gotPath string + var gotBody []byte + upstream := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + gotPath = r.URL.Path + gotBody, _ = io.ReadAll(r.Body) + w.Header().Set("Content-Type", "application/json") + fmt.Fprint(w, `{"input_tokens":4321}`) + })) + defer upstream.Close() + + h, _ := buildHandler(t, cachePipeline, upstream.URL) + srv := httptest.NewServer(h.Mux()) + defer srv.Close() + + body := claudeCodeBody(t, false) + resp, err := http.Post(srv.URL+"/anthropic/v1/messages/count_tokens", "application/json", + strings.NewReader(string(body))) + if err != nil { + t.Fatal(err) + } + defer resp.Body.Close() + out, _ := io.ReadAll(resp.Body) + + if resp.StatusCode != http.StatusOK { + t.Fatalf("status = %d, want 200 (a 404 here sends the client back to counting with "+ + "inference requests): %s", resp.StatusCode, out) + } + if gotPath != "/v1/messages/count_tokens" { + t.Errorf("upstream path = %q, want /v1/messages/count_tokens", gotPath) + } + if string(gotBody) != string(body) { + t.Errorf("the body was modified before counting. The client is asking about the context "+ + "IT holds and budgets its own transcript from the answer, so a count of a compacted "+ + "body would make it believe it has room it does not.\n got: %s\nwant: %s", gotBody, body) + } + if gjson.GetBytes(out, "input_tokens").Int() != 4321 { + t.Errorf("response not relayed verbatim: %s", out) + } +} + +// TestCountTokensForwardsUpstreamErrors: same wording-preservation rule as the chat route. A +// client that cannot read the real error here has no way to tell a malformed request from an +// auth failure. +func TestCountTokensForwardsUpstreamErrors(t *testing.T) { + errBody := `{"type":"error","error":{"type":"authentication_error","message":"invalid x-api-key"}}` + upstream := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Content-Type", "application/json") + w.WriteHeader(http.StatusUnauthorized) + 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/count_tokens", "application/json", + strings.NewReader(`{"model":"claude-sonnet-5","messages":[]}`)) + if err != nil { + t.Fatal(err) + } + defer resp.Body.Close() + got, _ := io.ReadAll(resp.Body) + if resp.StatusCode != http.StatusUnauthorized || string(got) != errBody { + t.Errorf("status/body not forwarded verbatim: %d %s", resp.StatusCode, got) + } +} diff --git a/proxy/expandgate_test.go b/proxy/expandgate_test.go new file mode 100644 index 00000000..483a9b5e --- /dev/null +++ b/proxy/expandgate_test.go @@ -0,0 +1,129 @@ +// The expand-tool advertising gate. +package proxy_test + +import ( + "encoding/json" + "fmt" + "github.com/tidwall/gjson" + "io" + "net/http" + "net/http/httptest" + "strings" + "testing" +) + +// TestExpandToolIsAdvertisedOnlyWhereMarkersCanExist pins the gate this change adds. +// +// `expand.Inject` under `auto` gated on two things — the request declares tools, and the store +// persists. Nothing asked whether the pipeline could produce a `<>` marker at all, so an +// offloader-free pipeline advertised `context_guru_expand` to the provider. Measured before the +// fix, on a cachesplit-only pipeline: +// +// tools SENT by client : [Read Bash] +// tools FORWARDED upstream: [Read Bash context_guru_expand] +// +// That is not cosmetic. A pipeline with no Offload mints no markers, so EVERY expand call against +// it must fail — on a real session with marker-shaped text in a file (this repo's own docs contain +// literal `<>`), the model called it unprompted and got "[expand: original for id ... is +// no longer available]", costing a round trip and a step of the user's turn. +// +// It has to be asserted HERE, on the bytes that leave the process. Any test of the presets map or +// of the built pipeline would pass throughout: the injection happens in proxy.go, after apply has +// already returned. +// +// The `mcp` case below is the one that keeps this honest. It looks offloader-free and is not, so a +// gate written against a list of preset names would get it wrong — as the first draft of this test +// did. +func TestExpandToolIsAdvertisedOnlyWhereMarkersCanExist(t *testing.T) { + for _, c := range []struct { + name string + pipeline string + wantAdd bool + }{ + // A cache-only pipeline: no offloader, so no marker can ever exist. + {"cachesplit only", "pipeline: [cachesplit]\n", false}, + // `safe` is a shipped preset with no Offload — lossless folds and the cache split only — + // and it was affected too. + {"safe", "pipeline: [format, textclean, searchfold, cachesplit]\n", false}, + // `mcp` looks similar and is NOT affected, which is the distinction the gate has to get + // right: `smartcrush` implements components.Offload (components/offload/smartcrush.go), + // so that pipeline really can mint a marker and really does need the tool. I had this + // case the wrong way round at first and the test caught it — which is the argument for + // asking the interface rather than keeping a hand-written list of "lossy" component names. + {"mcp (smartcrush IS an Offload)", "pipeline: [format, textclean, smartcrush, cachesplit]\n", true}, + // `off` is the A/B control arm. A control that carries an extra tool declaration is not + // a control — and this was broken too. + {"off (the A/B control)", "pipeline: []\n", false}, + // The feature must still work where markers can exist, or the fix has traded one silent + // defect for another: an offloader whose output nothing can expand. + {"a pipeline with an offloader", "pipeline: [linecap, cachesplit]\n", true}, + } { + 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, c.pipeline, upstream.URL) + srv := httptest.NewServer(h.Mux()) + defer srv.Close() + + // Tools declared, which is what `auto` keys on, plus a long tool output so the + // offloader case has something to act on. + body := []byte(`{"model":"claude-sonnet-5","max_tokens":64,` + + `"tools":[{"name":"Read","input_schema":{"type":"object"}},` + + `{"name":"Bash","input_schema":{"type":"object"}}],` + + `"messages":[{"role":"user","content":[{"type":"tool_result","tool_use_id":"a",` + + `"content":` + jsonStr(strings.Repeat("output line that is long enough to offload\n", 400)) + + `}]}]}`) + if !json.Valid(body) { + t.Fatalf("fixture is not valid JSON") + } + + 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") + } + + var sent, got []string + for _, v := range gjson.GetBytes(body, "tools").Array() { + sent = append(sent, v.Get("name").String()) + } + for _, v := range gjson.GetBytes(forwarded, "tools").Array() { + got = append(got, v.Get("name").String()) + } + has := false + for _, n := range got { + if n == "context_guru_expand" { + has = true + } + } + if has != c.wantAdd { + if c.wantAdd { + t.Fatalf("pipeline %q mints markers but no longer advertises the expand tool, "+ + "so a model cannot recover what it offloaded: sent %v, forwarded %v", + c.pipeline, sent, got) + } + t.Fatalf("pipeline %q has no offloader, so it mints no markers and every expand "+ + "call against it must fail — but it advertises the tool anyway: sent %v, "+ + "forwarded %v", c.pipeline, sent, got) + } + // The client's own tools must keep their exact identity and order either way: the + // tools array has to be byte-stable across a session or every turn is a full + // prefix-cache miss. + for i, n := range sent { + if got[i] != n { + t.Errorf("client tool %d changed: %q -> %q (full: %v)", i, n, got[i], got) + } + } + }) + } +} diff --git a/proxy/expandsplice_test.go b/proxy/expandsplice_test.go index 86bd7554..36349555 100644 --- a/proxy/expandsplice_test.go +++ b/proxy/expandsplice_test.go @@ -76,7 +76,7 @@ func TestExpandCalledAfterALeadingBlockIsNeverGivenToTheClient(t *testing.T) { })) defer upstream.Close() - h, st := buildHandler(t, "pipeline: []\n", upstream.URL) + h, st := buildHandler(t, offloadCapablePipeline, upstream.URL) st.Put("HASH", []byte("THE ORIGINAL CONTENT")) srv := httptest.NewServer(h.Mux()) defer srv.Close() @@ -155,7 +155,7 @@ func TestTheStreamedPrefixReachesTheClientBeforeTheExpandCall(t *testing.T) { })) defer upstream.Close() - h, st := buildHandler(t, "pipeline: []\n", upstream.URL) + h, st := buildHandler(t, offloadCapablePipeline, upstream.URL) st.Put("HASH", []byte("THE ORIGINAL CONTENT")) srv := httptest.NewServer(h.Mux()) defer srv.Close() @@ -217,7 +217,7 @@ func TestTheClientsNoSuchToolErrorNeverReachesTheModel(t *testing.T) { })) defer upstream.Close() - h, st := buildHandler(t, "pipeline: []\n", upstream.URL) + h, st := buildHandler(t, offloadCapablePipeline, upstream.URL) st.Put("HASH", []byte("THE ORIGINAL CONTENT")) srv := httptest.NewServer(h.Mux()) defer srv.Close() @@ -299,7 +299,7 @@ func TestAFailedContinuationRoundStillEndsTheClientsTurn(t *testing.T) { })) defer upstream.Close() - h, st := buildHandler(t, "pipeline: []\n", upstream.URL) + h, st := buildHandler(t, offloadCapablePipeline, upstream.URL) st.Put("HASH", []byte("THE ORIGINAL CONTENT")) srv := httptest.NewServer(h.Mux()) defer srv.Close() @@ -357,7 +357,7 @@ func TestAJSONContinuationRoundStillEndsTheClientsTurn(t *testing.T) { })) defer upstream.Close() - h, st := buildHandler(t, "pipeline: []\n", upstream.URL) + h, st := buildHandler(t, offloadCapablePipeline, upstream.URL) st.Put("HASH", []byte("THE ORIGINAL CONTENT")) srv := httptest.NewServer(h.Mux()) defer srv.Close() @@ -404,7 +404,7 @@ func TestARoundPastTheRetainBoundCountsTheLeakAndDoesNotContinue(t *testing.T) { })) defer upstream.Close() - h, st := buildHandler(t, "pipeline: []\n", upstream.URL) + h, st := buildHandler(t, offloadCapablePipeline, upstream.URL) st.Put("HASH", []byte("THE ORIGINAL CONTENT")) srv := httptest.NewServer(h.Mux()) defer srv.Close() @@ -467,7 +467,7 @@ func TestEveryContinuationShapeStillClosesTheClientsMessage(t *testing.T) { })) defer upstream.Close() - h, st := buildHandler(t, "pipeline: []\n", upstream.URL) + h, st := buildHandler(t, offloadCapablePipeline, upstream.URL) st.Put("HASH", []byte("THE ORIGINAL CONTENT")) srv := httptest.NewServer(h.Mux()) defer srv.Close() @@ -523,7 +523,7 @@ func TestASplicedRoundThatCallsExpandAgainStaysWellFormed(t *testing.T) { })) defer upstream.Close() - h, st := buildHandler(t, "pipeline: []\n", upstream.URL) + h, st := buildHandler(t, offloadCapablePipeline, upstream.URL) st.Put("HASH", []byte("ORIGINAL ONE")) st.Put("HASH2", []byte("ORIGINAL TWO")) srv := httptest.NewServer(h.Mux()) @@ -608,7 +608,7 @@ func TestATruncatedRoundStillGivesTheClientAnEnd(t *testing.T) { })) defer upstream.Close() - h, st := buildHandler(t, "pipeline: []\n", upstream.URL) + h, st := buildHandler(t, offloadCapablePipeline, upstream.URL) st.Put("HASH", []byte("THE ORIGINAL CONTENT")) srv := httptest.NewServer(h.Mux()) defer srv.Close() diff --git a/proxy/expandunresolved_test.go b/proxy/expandunresolved_test.go index d84ecd95..9c63276e 100644 --- a/proxy/expandunresolved_test.go +++ b/proxy/expandunresolved_test.go @@ -45,7 +45,7 @@ func TestAnExpandCallThatResolvesNothingStillFinishesTheTurn(t *testing.T) { })) defer upstream.Close() - h, _ := buildHandler(t, "pipeline: []\n", upstream.URL) + h, _ := buildHandler(t, offloadCapablePipeline, upstream.URL) srv := httptest.NewServer(h.Mux()) defer srv.Close() diff --git a/proxy/proxy.go b/proxy/proxy.go index bac2858c..5d65a967 100644 --- a/proxy/proxy.go +++ b/proxy/proxy.go @@ -82,9 +82,20 @@ type Options struct { } // InjectExpand controls advertising the context_guru_expand tool on outgoing // requests so Offload markers are actually recoverable (expand.InjectAuto | - // InjectAlways | InjectNever). Empty defaults to auto. auto injects only when the - // request already declares tools, carries an expandable marker, and the store - // persists — safe for any agent. + // InjectAlways | InjectNever). Empty defaults to auto. + // + // `auto` injects when the request already declares tools, the store persists, and THE + // PIPELINE CONTAINS AT LEAST ONE OFFLOAD. It does NOT depend on the request carrying a + // marker — that would make the tools array vary turn to turn, and every variation is a + // whole-prefix cache miss (see the note on expand.InjectAuto). + // + // This comment used to claim a marker condition that expand.Inject explicitly disclaims, + // and the pipeline condition was missing entirely. The consequence was live: a + // cachesplit-only pipeline forwarded `context_guru_expand` to the provider, a model that + // saw marker-shaped text in a file called it, and the call could only ever fail because + // nothing in that pipeline mints a marker to resolve. + // + // `always` still injects unconditionally — an operator asking for it by name gets it. InjectExpand string // CacheMode controls cache-aware compaction ("auto"|"on"|"off"; empty=auto). // auto/on keep offloaders from mutating already-cached content on prompt-caching @@ -346,6 +357,10 @@ func (h *Handler) Mux() *http.ServeMux { path: "/v1/messages", setKey: headerKey("x-api-key", h.opts.AnthropicKey), }, pickAnthropic)) + // Token counting, forwarded verbatim with no pipeline. Absent this route, a client that + // asks how big its context is gets a 404 and falls back to working it out with inference + // requests — billed calls, added by a proxy sold on reducing them. See counttokens.go. + m.HandleFunc("POST /anthropic"+countTokensPath, h.countTokens(h.anthropicCountTokensUpstream())) m.HandleFunc("POST /compact", h.compact) m.HandleFunc("GET /healthz", func(w http.ResponseWriter, _ *http.Request) { w.Write([]byte("ok")) }) // A browser asks for /favicon.ico unprompted, and in hosted mode Bob's "/" catch-all @@ -1145,7 +1160,24 @@ func (h *Handler) chat(provider bschemas.ModelProvider, static upstream, pick fu if im == "" { im = expand.InjectAuto } - body, _ = expand.Inject(string(provider), im, body, tn.Store.Persists()) + // Under `auto`, advertise only when THIS request's pipeline can actually mint a + // marker. Without it, an offloader-free pipeline — `off`, `safe`, or any + // cachesplit-only configuration — declared a tool whose every use is guaranteed + // to fail: measured `[Read Bash]` in, `[Read Bash context_guru_expand]` out, and + // on a transcript containing marker-shaped text the model duly called it and got + // "[expand: original for id ... is no longer available]". + // + // Which presets those are is NOT a list worth writing down here — `mcp` looks + // offloader-free and is not, because `smartcrush` implements components.Offload. + // That is exactly why the gate asks the interface (Pipeline.HasOffload) instead + // of naming presets: a list in a comment is a second source of truth, and this + // one was wrong about `mcp` in its first draft. + // + // `off` mattered most: it is the A/B control arm, and a control that carries an + // extra tool declaration is not a control. + if im != expand.InjectAuto || tn.Pipe.HasOffload() { + body, _ = expand.Inject(string(provider), im, body, tn.Store.Persists()) + } } }() // Load the request's one INFO line with everything the pipeline decided. serve diff --git a/proxy/proxy_test.go b/proxy/proxy_test.go index 53714666..956e5ccf 100644 --- a/proxy/proxy_test.go +++ b/proxy/proxy_test.go @@ -35,6 +35,20 @@ func buildHandler(t *testing.T, yaml string, upstream string) (*proxy.Handler, s return proxy.New(pipe, st, agg, proxy.Options{OpenAIUpstream: upstream, AnthropicUpstream: upstream}), st } +// offloadCapablePipeline is the fixture for any test whose premise is that content was +// OFFLOADED — i.e. every test of the context_guru_expand loop. +// +// Advertising that tool is gated on the pipeline containing an Offload, because a pipeline that +// mints no markers would be declaring a tool whose every call must fail (the defect that shipped +// in `off` — the A/B control arm — and in `safe`). A `pipeline: []` fixture therefore no longer +// advertises it, +// and a test that hand-seeds the Store to simulate an offload has to use a pipeline where the +// offload it is simulating could actually have happened. +// +// `linecap` is an Offload that does not act on the short bodies in these tests, so it leaves the +// asserted bytes alone. +const offloadCapablePipeline = "pipeline: [linecap]\n" + func openAIBody(msgs ...map[string]any) []byte { b, _ := json.Marshal(map[string]any{"model": "gpt-x", "temperature": 0.2, "messages": msgs}) return b @@ -298,7 +312,7 @@ func TestExpandToolLoop(t *testing.T) { })) defer upstream.Close() - h, st := buildHandler(t, "pipeline: []\n", upstream.URL) + h, st := buildHandler(t, offloadCapablePipeline, upstream.URL) st.Put("HASH", []byte("THE ORIGINAL CONTENT")) // as if a prior turn offloaded it srv := httptest.NewServer(h.Mux()) defer srv.Close() @@ -356,7 +370,7 @@ func TestExpandSSELoop(t *testing.T) { })) defer upstream.Close() - h, st := buildHandler(t, "pipeline: []\n", upstream.URL) + h, st := buildHandler(t, offloadCapablePipeline, upstream.URL) st.Put("HASH", []byte("THE ORIGINAL CONTENT")) srv := httptest.NewServer(h.Mux()) defer srv.Close() @@ -428,7 +442,7 @@ func TestExpandPartialResolutionWellFormed(t *testing.T) { })) defer upstream.Close() - h, st := buildHandler(t, "pipeline: []\n", upstream.URL) + h, st := buildHandler(t, offloadCapablePipeline, upstream.URL) st.Put("GOOD", []byte("RESOLVED ORIGINAL")) // only one of the two resolves srv := httptest.NewServer(h.Mux()) defer srv.Close() @@ -770,7 +784,7 @@ func TestExpandSSEMultiRoundCapped(t *testing.T) { })) defer upstream.Close() - h, st := buildHandler(t, "pipeline: []\n", upstream.URL) + h, st := buildHandler(t, offloadCapablePipeline, upstream.URL) st.Put("HASH", []byte("THE ORIGINAL CONTENT")) srv := httptest.NewServer(h.Mux()) defer srv.Close() diff --git a/proxy/tenancy_test.go b/proxy/tenancy_test.go index b07a1e6b..59a45061 100644 --- a/proxy/tenancy_test.go +++ b/proxy/tenancy_test.go @@ -179,6 +179,7 @@ func TestHostedRejectsUnauthenticated(t *testing.T) { for _, path := range []string{ "/openai/v1/chat/completions", "/anthropic/v1/messages", + "/anthropic/v1/messages/count_tokens", "/inference/v1/chat/completions", "/compact", } {