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

[OsherElhadad]

Summary

Follow-up to the merged context-guru plugin (#660), addressing the non-blocking
review feedback from @mrsabath and @huang195. No behavior change to the compaction
path itself — these are correctness, isolation, footprint, and hardening fixes plus
the two review-requested tests.

Relates to epic #659 and issue #641. Builds on #660 (merged).

What changed, mapped to review comments

Plugin (authlib/plugins/contextguru)

Review comment	Fix
@mrsabath — sessionID falls back to "", so sessionless callers share one engine-store key and bleed compaction state	OnRequest now skips compaction when there is no session identity (returns Continue, logs at DEBUG) instead of using the "" key. sessionID doc updated.
@mrsabath — the sentinel-header recursion guard is untested	Added TestOnRequest_SentinelHeader_ShortCircuits (asserts Continue + no body mutation when X-Context-Guru-LLM is set) and TestOnRequest_NoSession_Skips.
@mrsabath — Configure silently degrades if only one of model.base_url / model.model is set	Configure now fails loud when exactly one is set (a typo'd/empty ${VAR} expansion no longer silently disables the cheap model). Empty-both still degrades intentionally. Covered by TestConfigure_RejectsPartialModel.

Build footprint

Review comment	Fix
@huang195 — registered opt-out, so the heavy embedded engine compiles into every core binary	Flipped to opt-IN: //go:build include_plugin_contextguru. Default authbridge-proxy/-envoy binaries no longer link context-guru (≈16 MB smaller, 29 MB → 45 MB only with the tag; 0 vs 312 context-guru symbols). Documented in authbridge/CLAUDE.md as the deliberate exception to the exclude_plugin_* convention. run.sh and the demo Dockerfile build with -tags include_plugin_contextguru.

Demo (demos/context-guru)

Review comment	Fix
@huang195 — default CG_MODEL_BASE hardcodes an internal IBM VPC endpoint in a public repo	Removed from run.sh and agent.yaml. CG_MODEL_BASE is now required (fail-loud like CG_MODEL_KEY) and wired into the deployment via kubectl set env; agent.yaml ships a neutral https://api.openai.com placeholder. No real endpoint lands in the repo.
@mrsabath — drive() poll loop pipes a crashed pod's logs into json.loads	Loop now breaks on terminal phases (Failed/Unknown) with a readable error + describe/logs and a non-zero exit.
@mrsabath — demo containers declare no resources	Added modest requests/limits to both containers so the manifest doubles as a template.

Supply-chain / hardening (review-body nits, @mrsabath)

• Digest-pinned the envoy:v1.37.1 builder stage in cmd/authbridge-envoy/Dockerfile (parity with the other three stages).
• Digest-pinned alpine:3.20 in run.sh's generated demo Dockerfile.
• run.sh now creates the model-key Secret via --from-file=key=<(printf %s …) (a shell builtin) instead of --from-literal, so the key never transits kubectl's argv / the process table on a shared host.

Two go.mod nits that can't be fixed on our side (@huang195)

Both are transitively forced by bifrost/core v1.7.0 (pulled by context-guru); I verified and reverted:

• go 1.26 + toolchain go1.26.4 split — go mod tidy immediately rewrites the go directive back to go 1.26.4 because a dependency requires that exact patch. The idiomatic split isn't achievable while depending on bifrost.
• protobuf → stable v1.36.11 — go get google.golang.org/protobuf@v1.36.11 downgrades bifrost to v1.5.16 and drops context-guru; go mod tidy then restores context-guru → bifrost v1.7.0 → protobuf v1.36.12-pre. The pre-release is pinned upstream.

Both resolve once bifrost/context-guru pins a stable protobuf and relaxes its go directive; tracked as the "pin to a tagged release" follow-up already noted in #660.

Testing

• go test ./authlib/plugins/contextguru/... — pass (incl. the 3 new tests).
• Default build (no tag) and -tags include_plugin_contextguru build — both clean for authbridge-proxy and authbridge-envoy, in workspace and GOWORK=off (Docker/CI) modes. go vet + gofmt clean.
• End-to-end on a live kind cluster with the opt-in image, all three modes (results below).

Result (re-validated end-to-end with the opt-in build)

Same agent, same llama3.2 pinned to a 12,288-token window, same task:

mode	context-guru	request the model sees	agent answer
off	disabled	raw ~18K tok → truncated to the 12K window	❌ hallucinates a refund for all three transactions
observe	shadow (measures only)	raw ~18K tok; logs it would compact 59.8 KB → 30.1 KB (50%)	❌ same wrong answer (behavior unchanged)
enforce	applied	compacted 51.9 KB → 30.0 KB (42%) → fits	✅ "duplicate settlement for TX4827 … a refund has been issued for TX4827. For TX5310 and TX2981, no anomalies were found."

context-guru remains the only variable that flips the answer from wrong to right — now proven with the opt-in (include_plugin_contextguru) image.

Assisted-By: Claude (Anthropic AI) noreply@anthropic.com

[coderabbitai]

📝 Walkthrough

Walkthrough

ContextGuru now validates static model configuration, requires session identity before compaction, and tests sentinel and no-session behavior. Its plugin binaries use explicit opt-in build tags. The demo adds configurable model settings, resource limits, pinned images, and clearer pod failure handling.

Changes

ContextGuru integration

Layer / File(s)	Summary
ContextGuru validation and session isolation authbridge/authlib/plugins/contextguru/plugin.go, authbridge/authlib/plugins/contextguru/plugin_test.go	Partial model configuration is rejected, compaction skips requests without session identity, sentinel requests remain unchanged, and these behaviors are covered by tests.
Opt-in plugin builds authbridge/cmd/authbridge-envoy/plugins_contextguru.go, authbridge/cmd/authbridge-proxy/plugins_contextguru.go, authbridge/CLAUDE.md, authbridge/cmd/authbridge-envoy/Dockerfile	ContextGuru is compiled only with include_plugin_contextguru; build guidance documents this behavior, and the Envoy source image is digest-pinned.
Demo configuration and execution authbridge/demos/context-guru/run.sh, authbridge/demos/context-guru/k8s/agent.yaml, authbridge/demos/context-guru/README.md	The demo uses operator-supplied model settings, applies proxy-container environment overrides and resource limits, pins its sidecar image, and reports failed pod phases with diagnostics.

Estimated code review effort: 3 (Moderate) | ~25 minutes

Suggested labels: ready-for-ai-review

Suggested reviewers: huang195

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title correctly points to the main change: addressing context-guru plugin review feedback.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[coderabbitai]

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@authbridge/CLAUDE.md`:
- Around line 17-20: Update the Envoy entry in CLAUDE.md to state that
ContextGuru is opt-in there as well, requiring the include_plugin_contextguru
build tag, and replace or qualify the “Full plugin set” wording so it does not
imply ContextGuru is included by default.

In `@authbridge/demos/context-guru/run.sh`:
- Around line 33-35: Update the build flow in the run script to create a unique
temporary directory with mktemp -d, use that directory directly as the Go build
output location, and remove the predictable /tmp/cg-img assignment and
subsequent cp step. Preserve the existing binary name and build options.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 2ca6be6c-ce2f-413f-ac14-143f9373c55f

📥 Commits

Reviewing files that changed from the base of the PR and between 468386d and 6e0ab58.

📒 Files selected for processing (9)

• authbridge/CLAUDE.md
• authbridge/authlib/plugins/contextguru/plugin.go
• authbridge/authlib/plugins/contextguru/plugin_test.go
• authbridge/cmd/authbridge-envoy/Dockerfile
• authbridge/cmd/authbridge-envoy/plugins_contextguru.go
• authbridge/cmd/authbridge-proxy/plugins_contextguru.go
• authbridge/demos/context-guru/README.md
• authbridge/demos/context-guru/k8s/agent.yaml
• authbridge/demos/context-guru/run.sh

[huang195]

Review summary

Follow-up that cleanly addresses the #660 review feedback. I reviewed all 9 changed files in full; all 18 CI checks pass and the security-motivated changes verify end to end.

Verified

• Configure now fails loud when exactly one of model.base_url / model.model is set, and degrades only when both are empty — switch logic is correct.
• OnRequest skips compaction when there is no session identity, fixing the shared empty-store-key state bleed @mrsabath flagged; sid is reused consistently (log line included).
• Three new tests (sentinel short-circuit, no-session skip, partial-model reject) are assertive with no hidden skips.
• Build-tag flip to opt-in (include_plugin_contextguru) is consistent across both cmd/*/plugins_contextguru.go files and documented in authbridge/CLAUDE.md + the demo README.
• Hardening lands: digest-pinned envoy:v1.37.1 and alpine:3.20, and the model-key Secret is created via --from-file=key=<(printf %s …) — valid, since run.sh is #!/usr/bin/env bash with set -euo pipefail, so the key stays out of argv.
• Internal IBM VPC endpoint fully removed from run.sh and agent.yaml; authbridge-config.yaml reads ${CG_MODEL_BASE}, so no real host remains anywhere in the repo.
• drive() breaks on terminal pod phases before piping into json.loads, fixing the crashed-pod-into-parser path.

One optional nit inline. Well-scoped, well-documented PR — thanks!

Assisted-By: Claude Code

[huang195]

nit (optional): the Run it block exports CG_MODEL_KEY and CG_MODEL_BASE but not CG_MODEL_NAME, which run.sh defaults to gpt-4o-mini. If someone points CG_MODEL_BASE at an OpenAI-wire proxy that doesn't serve gpt-4o-mini (e.g. a litellm gateway with custom model names), the extract-code calls fail with a confusing error. Consider a one-line note here that CG_MODEL_NAME is overridable.