Merge pull request #672 from OsherElhadad/fix/context-guru-review-followups

Fix: Address context-guru plugin review feedback (#660)

Author: huang195

authbridge/CLAUDE.md

@@ -14,7 +14,10 @@ binaries with shared auth logic in `authlib/`:
   a2a-parser, mcp-parser, inference-parser, opa, sparc, ibac, token-broker).
   **Every** plugin is excludable via `-tags exclude_plugin_<name>` — one
   `plugins_<name>.go` file per plugin, gated by `//go:build !exclude_plugin_<name>`;
-  `main.go` imports no plugin package directly.
+  `main.go` imports no plugin package directly. **Exception:** `context-guru` is
+  **opt-IN** (`//go:build include_plugin_contextguru`, not compiled by default),
+  because its embedded engine pulls a large transitive dependency set; build with
+  `-tags include_plugin_contextguru` to link it in.
 - `cmd/authbridge-envoy/` — envoy-sidecar mode. ext_proc gRPC server hooked
   into Envoy. Full plugin set. The `exclude_plugin_ibac` tag applies here too.
 - `authbridge-lite` (**image, not a separate binary**) — `cmd/authbridge-proxy`

authbridge/authlib/plugins/contextguru/plugin.go

@@ -199,7 +199,16 @@ func (p *ContextGuru) Configure(raw json.RawMessage) error {
 		if m.MaxTokens > 0 {
 			p.modelMax = m.MaxTokens
 		}
-		if m.BaseURL != "" && m.Model != "" {
+		switch {
+		case m.BaseURL == "" && m.Model == "":
+			// Empty model block → no static model; LLM components degrade to
+			// deterministic. (Common when the config is present but the operator
+			// hasn't wired an endpoint yet.)
+		case m.BaseURL == "" || m.Model == "":
+			// Exactly one set is almost always a typo/empty ${VAR} expansion that
+			// would silently disable the cheap model. Fail loud instead.
+			return fmt.Errorf("context-guru model config: base_url and model must both be set (got base_url=%q model=%q)", m.BaseURL, m.Model)
+		default:
 			p.static = cgModel{
 				c: llmclient.New(llmclient.Options{
 					Endpoint:           m.BaseURL,
@@ -227,19 +236,27 @@ func (p *ContextGuru) OnRequest(ctx context.Context, pctx *pipeline.Context) pip
 	if p.pipe == nil || len(pctx.Body) == 0 || !p.gated(pctx.Path) {
 		return cont
 	}
+	// The engine Store is keyed by session for per-caller isolation. With no
+	// session identity every such request would share one empty key and bleed
+	// compaction state across unrelated callers, so skip compaction entirely.
+	sid := sessionID(pctx)
+	if sid == "" {
+		slog.Debug("context-guru skipped request with no session identity", "path", pctx.Path)
+		return cont
+	}
 	provider := providerFor(pctx.Path)
 	models := cgcomponents.ModelSpec{
 		Static:   p.static,
 		Incoming: p.incomingModel(pctx, provider),
 	}
 	before := len(pctx.Body)
-	out, changed := apply.BodyWithModel(ctx, p.pipe, p.store, provider, pctx.Body, sessionID(pctx), false, models)
+	out, changed := apply.BodyWithModel(ctx, p.pipe, p.store, provider, pctx.Body, sid, false, models)
 	if changed && len(out) > 0 {
 		pctx.SetBody(out)
 		// Per-request byte-level view (the engine's token-level view is logged by
 		// logEmitter.Run). The framework also emits a body-mutation session event.
 		slog.Info("context-guru rewrote request body",
-			"provider", provider, "path", pctx.Path, "session", sessionID(pctx),
+			"provider", provider, "path", pctx.Path, "session", sid,
 			"bytesBefore", before, "bytesAfter", len(out), "pctSaved", pct(before, len(out)))
 	}
 	return cont
@@ -316,7 +333,9 @@ func modelFromBody(body []byte) string {
 }
 
 // sessionID keys per-session engine state to the conversation: the A2A contextId
-// when present, else the caller subject for multi-tenant isolation.
+// when present, else the caller subject for multi-tenant isolation. Returns ""
+// when neither is available; OnRequest treats that as "skip compaction" rather
+// than sharing one store key across unrelated callers.
 func sessionID(pctx *pipeline.Context) string {
 	if pctx.Session != nil && pctx.Session.ID != "" {
 		return pctx.Session.ID

authbridge/authlib/plugins/contextguru/plugin_test.go

@@ -117,6 +117,48 @@ func TestOnRequest_CompactsLargeToolOutput(t *testing.T) {
 	}
 }
 
+// TestOnRequest_SentinelHeader_ShortCircuits covers the one safety-critical
+// branch: the plugin's own extract:code LLM call carries the sentinel, and if it
+// ever transits this forward proxy OnRequest must bail before touching the body —
+// otherwise the compaction call recurses into itself.
+func TestOnRequest_SentinelHeader_ShortCircuits(t *testing.T) {
+	p := configured(t, collapseEngine)
+	body := chatBody(bigToolOutput())
+	pctx := inferencePctx("/v1/chat/completions", body)
+	pctx.Headers.Set(sentinelHeader, "1")
+	if act := invokeReq(p, pctx); act.Type != pipeline.Continue {
+		t.Fatalf("expected Continue, got %v", act.Type)
+	}
+	if pctx.BodyMutated() {
+		t.Fatal("sentinel header must short-circuit before any compaction")
+	}
+}
+
+// TestOnRequest_NoSession_Skips verifies that a request with neither session nor
+// identity is passed through untouched, so sessionless callers can't share one
+// empty engine-store key.
+func TestOnRequest_NoSession_Skips(t *testing.T) {
+	p := configured(t, collapseEngine)
+	body := chatBody(bigToolOutput())
+	pctx := inferencePctx("/v1/chat/completions", body)
+	pctx.Session = nil // and Identity is nil → sessionID == ""
+	if act := invokeReq(p, pctx); act.Type != pipeline.Continue {
+		t.Fatalf("expected Continue, got %v", act.Type)
+	}
+	if pctx.BodyMutated() {
+		t.Fatal("no session identity must skip compaction (store-key isolation)")
+	}
+}
+
+func TestConfigure_RejectsPartialModel(t *testing.T) {
+	if err := New().Configure(json.RawMessage(`{"model":{"base_url":"http://x"}}`)); err == nil {
+		t.Fatal("expected error when model.model is unset but base_url is set")
+	}
+	if err := New().Configure(json.RawMessage(`{"model":{"model":"gpt-4o-mini"}}`)); err == nil {
+		t.Fatal("expected error when model.base_url is unset but model is set")
+	}
+}
+
 func TestOnRequest_EmptyBody_PassThrough(t *testing.T) {
 	p := configured(t, collapseEngine)
 	pctx := inferencePctx("/v1/chat/completions", nil)

authbridge/cmd/authbridge-envoy/Dockerfile

@@ -27,7 +27,7 @@ ENV GOWORK=off
 RUN cd cmd/authbridge-envoy && CGO_ENABLED=0 GOOS=linux go build -tags "${GO_BUILD_TAGS}" -ldflags="-s -w" -o /authbridge-envoy .
 
 # Stage 2: Get Envoy binary. Already stripped upstream; nothing to do.
-FROM docker.io/envoyproxy/envoy:v1.37.1 AS envoy-source
+FROM docker.io/envoyproxy/envoy:v1.37.1@sha256:29496a88fba9c4c9cdef4afe8fec70f536c5ba111b1c2bddbc5436b091ceca33 AS envoy-source
 
 # Stage 3: Runtime — UBI9-micro (glibc-compatible, ~24 MB, has bash)
 FROM registry.access.redhat.com/ubi9/ubi-micro@sha256:2173487b3b72b1a7b11edc908e9bbf1726f9df46a4f78fd6d19a2bab0a701f38

authbridge/cmd/authbridge-envoy/plugins_contextguru.go

@@ -1,5 +1,10 @@
-//go:build !exclude_plugin_contextguru
+//go:build include_plugin_contextguru
 
+// context-guru is opt-IN (positive build tag), unlike the other plugins which are
+// compiled in by default and dropped via exclude_plugin_*. Its embedded engine
+// pulls a large transitive set (bifrost/core, tiktoken-go, tree-sitter grammars,
+// starlark), so it is kept out of the default authbridge-proxy/-envoy binaries and
+// linked only when built with -tags include_plugin_contextguru.
 package main
 
 import _ "github.com/kagenti/kagenti-extensions/authbridge/authlib/plugins/contextguru"

authbridge/cmd/authbridge-proxy/plugins_contextguru.go

@@ -1,5 +1,10 @@
-//go:build !exclude_plugin_contextguru
+//go:build include_plugin_contextguru
 
+// context-guru is opt-IN (positive build tag), unlike the other plugins which are
+// compiled in by default and dropped via exclude_plugin_*. Its embedded engine
+// pulls a large transitive set (bifrost/core, tiktoken-go, tree-sitter grammars,
+// starlark), so it is kept out of the default authbridge-proxy/-envoy binaries and
+// linked only when built with -tags include_plugin_contextguru.
 package main
 
 import _ "github.com/kagenti/kagenti-extensions/authbridge/authlib/plugins/contextguru"

authbridge/demos/context-guru/README.md

@@ -91,19 +91,34 @@ And the observed answers:
 ## Run it
 
 ```bash
-export CG_MODEL_KEY=<api key for the extract-code model>   # e.g. an OpenAI-compatible key
+export CG_MODEL_KEY=<api key for the extract-code model>       # e.g. an OpenAI-compatible key
+export CG_MODEL_BASE=https://api.openai.com                    # any OpenAI-wire endpoint
 ./run.sh all        # setup + drive off, observe, enforce and print the comparison
 # or step by step:
 ./run.sh setup
 ./run.sh drive enforce      # (or observe / off)
 ```
 
-`run.sh setup` builds the `authbridge-proxy` image **with the context-guru plugin**,
-loads it + the enhanced `finance-mcp` into the `kagenti` kind cluster, creates a
-12,288-token-window Ollama model (`llama3.2-ctx12k`) so the raw request truncates,
-and deploys the agent + sidecar. `run.sh drive <mode>` flips `on_error`, restarts,
-drives the audit, and prints the agent's answer + the byte/token gain from the
-sidecar session API (`:9094`).
+`run.sh setup` builds the `authbridge-proxy` image **with the context-guru plugin**
+(`-tags include_plugin_contextguru` — see *Build integration* below), loads it + the
+enhanced `finance-mcp` into the `kagenti` kind cluster, creates a 12,288-token-window
+Ollama model (`llama3.2-ctx12k`) so the raw request truncates, and deploys the agent
++ sidecar. `run.sh drive <mode>` flips `on_error`, restarts, drives the audit, and
+prints the agent's answer + the byte/token gain from the sidecar session API (`:9094`).
+
+## Build integration
+
+context-guru is **opt-in**: unlike the other plugins (compiled in by default,
+dropped via `-tags exclude_plugin_*`), it is linked only when the binary is built
+with `-tags include_plugin_contextguru`. Its embedded engine pulls a large
+transitive dependency set (bifrost/core, tiktoken-go, tree-sitter grammars,
+starlark), so the default `authbridge-proxy`/`authbridge-envoy` binaries stay lean
+and a deployment that doesn't want compaction never pays for it.
+
+```bash
+cd authbridge && podman build -f cmd/authbridge-proxy/Dockerfile \
+  --build-arg GO_BUILD_TAGS=include_plugin_contextguru -t authbridge-cg:latest .
+```
 
 ## Configuration reference
 

authbridge/demos/context-guru/k8s/agent.yaml

@@ -39,17 +39,25 @@ spec:
             - { name: FINANCE_MCP_URL, value: "http://finance-mcp.team1.svc.cluster.local:8888" }
             - { name: AGENT_PUBLIC_URL, value: "http://cg-finance-agent.team1.svc.cluster.local:8080/" }
           volumeMounts: [{ mountPath: /shared, name: shared-data }]
+          resources:
+            requests: { cpu: "100m", memory: "128Mi" }
+            limits:   { cpu: "1", memory: "512Mi" }
         - name: authbridge-proxy
           image: authbridge-cg:latest       # authbridge-proxy built with the context-guru plugin (see run.sh)
           imagePullPolicy: IfNotPresent
           args: ["--config", "/etc/authbridge/config.yaml"]
           env:
-            - { name: CG_MODEL_BASE, value: "https://ete-litellm.ai-models.vpc-int.res.ibm.com" }
-            - { name: CG_MODEL_NAME, value: "claude-haiku-4-5" }   # capable, cheap extract-code model
+            # Neutral placeholders — run.sh overrides these via `kubectl set env`
+            # from CG_MODEL_BASE / CG_MODEL_NAME so no real endpoint lands in the repo.
+            - { name: CG_MODEL_BASE, value: "https://api.openai.com" }
+            - { name: CG_MODEL_NAME, value: "gpt-4o-mini" }        # capable, cheap extract-code model
             - name: CG_MODEL_KEY
               valueFrom: { secretKeyRef: { name: cg-model-key, key: key } }
           ports: [{ containerPort: 8080 }, { containerPort: 8081 }, { containerPort: 9094 }]
           volumeMounts: [{ mountPath: /etc/authbridge, name: ab-config }]
+          resources:
+            requests: { cpu: "50m", memory: "64Mi" }
+            limits:   { cpu: "500m", memory: "256Mi" }
       volumes:
         - { name: shared-data, emptyDir: {} }
         - { name: ab-config, configMap: { name: cg-authbridge-config } }

authbridge/demos/context-guru/run.sh

@@ -21,20 +21,20 @@ HERE="$(cd "$(dirname "$0")" && pwd)"
 AB="$(cd "$HERE/../.." && pwd)"                 # authbridge/
 FS="$AB/demos/finance-sparc"
 OLLAMA_HOST_URL="${OLLAMA_HOST_URL:-http://127.0.0.1:11434}"    # host Ollama, as seen from THIS machine
-: "${CG_MODEL_BASE:=https://ete-litellm.ai-models.vpc-int.res.ibm.com}"
-: "${CG_MODEL_NAME:=claude-haiku-4-5}"
+: "${CG_MODEL_NAME:=gpt-4o-mini}"                               # any capable, cheap OpenAI-wire model
 CTX_MODEL="llama3.2-ctx12k"
 
 setup() {
   : "${CG_MODEL_KEY:?set CG_MODEL_KEY to the extract-code model API key}"
+  : "${CG_MODEL_BASE:?set CG_MODEL_BASE to an OpenAI-compatible endpoint for the extract-code model, e.g. https://api.openai.com}"
   local arch; arch=$(docker exec kagenti-control-plane uname -m | sed 's/aarch64/arm64/;s/x86_64/amd64/')
 
-  echo "==> build authbridge-proxy WITH the context-guru plugin ($arch, pure-Go) + image"
+  echo "==> build authbridge-proxy WITH the context-guru plugin ($arch, pure-Go, -tags include_plugin_contextguru) + image"
   ( cd "$AB" && GOTOOLCHAIN=auto GOOS=linux GOARCH="$arch" CGO_ENABLED=0 \
-      go build -ldflags="-s -w" -o /tmp/authbridge-proxy-cg ./cmd/authbridge-proxy )
+      go build -tags include_plugin_contextguru -ldflags="-s -w" -o /tmp/authbridge-proxy-cg ./cmd/authbridge-proxy )
   local d=/tmp/cg-img; mkdir -p "$d"; cp /tmp/authbridge-proxy-cg "$d/authbridge-proxy"
   cat > "$d/Dockerfile" <<'DOCKER'
-FROM alpine:3.20
+FROM alpine:3.20@sha256:beefdbd8a1da6d2915566fde36db9db0b524eb737fc57cd1367effd16dc0d06d
 RUN apk add --no-cache ca-certificates
 COPY authbridge-proxy /usr/local/bin/authbridge-proxy
 ENTRYPOINT ["/usr/local/bin/authbridge-proxy"]
@@ -53,12 +53,18 @@ DOCKER
     -d "{\"model\":\"$CTX_MODEL\",\"from\":\"llama3.2:3b\",\"parameters\":{\"num_ctx\":12288}}" >/dev/null
 
   echo "==> secret + configmap + deploy agent + context-guru sidecar"
-  kubectl -n "$NS" create secret generic cg-model-key --from-literal=key="$CG_MODEL_KEY" \
+  # Feed the key via a builtin (process substitution), not --from-literal, so it
+  # never appears in kubectl's argv / the process table on a shared host.
+  kubectl -n "$NS" create secret generic cg-model-key --from-file=key=<(printf %s "$CG_MODEL_KEY") \
     --dry-run=client -o yaml | kubectl apply -f -
   kubectl -n "$NS" create configmap cg-authbridge-config --from-file=config.yaml="$HERE/k8s/authbridge-config.yaml" \
     --dry-run=client -o yaml | kubectl apply -f -
   kubectl apply -f "$HERE/k8s/agent.yaml"
   kubectl -n "$NS" set env deploy/cg-finance-agent OLLAMA_MODEL="$CTX_MODEL" >/dev/null
+  # Point the extract-code model at the operator-provided endpoint (agent.yaml
+  # ships only a neutral placeholder; the real endpoint never lands in the repo).
+  kubectl -n "$NS" set env deploy/cg-finance-agent -c authbridge-proxy \
+    CG_MODEL_BASE="$CG_MODEL_BASE" CG_MODEL_NAME="$CG_MODEL_NAME" >/dev/null
   kubectl -n "$NS" rollout status deploy/cg-finance-agent --timeout=120s
 }
 
@@ -77,8 +83,20 @@ JSON
   kubectl -n "$NS" delete pod cgdrive --ignore-not-found >/dev/null; sleep 2
   kubectl -n "$NS" run cgdrive --restart=Never --image=curlimages/curl:8.10.1 \
     --overrides='{"spec":{"containers":[{"name":"c","image":"curlimages/curl:8.10.1","command":["sh","-c","curl -m 300 -s -X POST http://cg-finance-agent.team1.svc.cluster.local:8080/ -H '"'"'Content-Type: application/json'"'"' --data @/data/drive.json"],"volumeMounts":[{"name":"d","mountPath":"/data"}]}],"volumes":[{"name":"d","configMap":{"name":"cg-drive"}}]}}' >/dev/null
-  while [ "$(kubectl -n "$NS" get pod cgdrive -o jsonpath='{.status.phase}')" = "Running" ] || \
-        [ "$(kubectl -n "$NS" get pod cgdrive -o jsonpath='{.status.phase}')" = "Pending" ]; do sleep 5; done
+  # Wait for the driver pod to finish. Break explicitly on terminal failure so a
+  # crashed/unschedulable pod gives a readable error instead of piping non-JSON
+  # logs into json.loads below.
+  while :; do
+    phase=$(kubectl -n "$NS" get pod cgdrive -o jsonpath='{.status.phase}' 2>/dev/null || echo Unknown)
+    case "$phase" in
+      Succeeded) break ;;
+      Running|Pending) sleep 5 ;;
+      *) echo "!! cgdrive pod ended in phase=$phase — the audit did not complete:"
+         kubectl -n "$NS" describe pod cgdrive | tail -n 25
+         kubectl -n "$NS" logs cgdrive 2>/dev/null || true
+         exit 1 ;;
+    esac
+  done
   echo "--- agent answer ($mode) ---"
   kubectl -n "$NS" logs cgdrive | python3 -c 'import sys,json;b=sys.stdin.read().split("EXIT=")[0].strip();d=json.loads(b);r=d.get("result",{});m=(r.get("status")or{}).get("message")or{};p=m.get("parts") or (r.get("artifacts") or [{}])[0].get("parts",[]);print(" ".join(x.get("text","") for x in p)[:600])'
   echo "--- context-guru gain ($mode) ---"