Production-readiness pass: observability, store-backed approvals, retention, shutdown, config validation - #17
Open
shadowhunter-92 wants to merge 5 commits into
Open
Production-readiness pass: observability, store-backed approvals, retention, shutdown, config validation#17shadowhunter-92 wants to merge 5 commits into
shadowhunter-92 wants to merge 5 commits into
Conversation
…ention, shutdown, config validation Closes the gaps flagged in docs/ROADMAP.md as "demand-gated" / "known limitations": observability (#1), store-backed ApprovalQueue (#2), JWKS auto-fetch for OIDC (#3), audit retention + legal hold, graceful shutdown + split health probes, retry/backoff on transient store errors, config validation at startup. Tests: 157 passing, 7 skipped (was 136; +21 new in tests/test_production_readiness.py). Skips are the 6 Postgres integration tests (need AGENTBRIDGE_TEST_PG) and 1 conformance test that needs redis. See CHANGELOG.md and docs/ENTERPRISE.md for the full feature list and production deployment checklist.
CI-blocker: - requirements.txt: add prometheus-client (CI installs from it; /metrics tests were red). Correctness: - cli.py: graceful-shutdown timeout to uvicorn in SECONDS (was *1000 -> ~2.8h). - audit.py: truncated chain stays verifiable (verify_chain require_genesis + auto-detect); drop false 'truncate pseudo-entry' docstring claim. - resilience.py: only retry sqlite3.OperationalError, not DatabaseError (parent of IntegrityError/ProgrammingError) -> permanent errors fail fast. - auth_oidc.py: JWKS resolves the cryptography key object directly (no brittle JWK->PEM __import__ dance); + an end-to-end JWKS round-trip test. Performance / safety: - gateway.py: AuditLog.count() (O(1)) instead of copying the whole audit list per call. - control_plane.py: HTTP metrics label by route TEMPLATE, not raw path (cardinality fix). Hygiene: - observability: real __version__ in build-info metric (was hardcoded 1.0.0). - normalized 8 files back to mode 644; strengthened retention + permanent-error tests; Windows-safe temp-db cleanup in the approvals test.
Owner
Author
|
Reviewed file-by-file and fixed all findings (commit d4a2a4b — CI now green on 3.11 + 3.12):
Decision: parking, not merging. Solid, green, mergeable work — but it's demand-gated production infra (observability / audit-retention / JWKS) and there are no production users yet. Keeping this branch ready; will merge the moment a design-partner needs metrics or audit-retention. |
…ce, deployment) The pass added the features + updated CHANGELOG/ROADMAP/ENTERPRISE but left the README and endpoint docs stale. Now documented: - README: new 'Production & operations' section (observability/metrics, /health vs /ready, audit retention + signed checkpoints, OIDC JWKS, fail-fast config, graceful shutdown, --workers); quick-start lists /ready + /metrics. - API_REFERENCE: /ready, /version, /metrics; /control/audit/checkpoint + /retention (audit:export). - DEPLOYMENT: new env vars (JWKS, ENV, LOG_JSON, SLOW_LOG, SHUTDOWN_GRACE, OTEL) + k8s probe note.
… flaky CI hang) The threaded concurrency test could deadlock under CI scheduling jitter: a slow/absent worker left the others waiting on a no-timeout threading.Barrier forever, and _run's per-thread joins summed past pytest-timeout -> a blunt 90s kill (intermittent red on 3.12). - Barrier now has a generous 30s timeout (self-heals; on CI workers arrive in <1s, so it never fires in the normal case but bounds a true stall well under pytest-timeout). - _run bounds the whole run with one shared deadline instead of cumulative per-thread joins.
shadowhunter-92
added a commit
that referenced
this pull request
Jun 30, 2026
…ang) Same flaky deadlock that just went red on PR #17's 3.12 run exists here on main: a slow/absent worker left the others on a no-timeout threading.Barrier forever, and _run's per-thread joins summed past pytest-timeout -> blunt 90s kill. Barrier now has a generous 30s self-heal timeout (workers arrive in <1s on CI, so it never fires normally but bounds a true stall); _run uses one shared deadline instead of cumulative per-thread joins.
…rs) — sync from main
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What this PR does
Closes the gaps flagged in
docs/ROADMAP.mdas demand-gated / known limitations. The maintainer's own roadmap listed these as the work needed before the project is production-ready; this PR ships all of it.Test suite: 157 passing, 7 skipped (was 136; +21 new tests). Skips need external resources (Postgres / redis).
The 8 things this PR ships
1. Observability (was roadmap #1)
/metricsPrometheus endpoint: call counter (agentbridge_calls_total{src,dst,capability,decision}), latency histograms (agentbridge_call_duration_seconds,agentbridge_translate_duration_seconds), audit-entry gauge, per-agent budget gauges, pending-approvals gauge, HTTP request counter + duration histogram, rate-limit-hit counter, auth-failure counter.OTEL_EXPORTER_OTLP_ENDPOINTto ship spans. Gateway opens a span around everyroute_call.AGENTBRIDGE_LOG_JSON=1, auto-on in production / k8s): one JSON object per line withts,level,logger,msg,request_id, plus anyextra=fields.X-Request-ID(echoed in the response), surfaced in logs via aContextVar.2. Store-backed ApprovalQueue (was roadmap #2 — last piece of in-process state)
store.consume_approval()atomically transitionsapproved -> consumed, so two workers can't double-consume a one-shot grant.approvalstable in SQLite + Postgres with indexes onstatusandagent_id.3. JWKS auto-fetch for OIDC (was roadmap #3)
<issuer>/.well-known/openid-configurationto discoverjwks_uri, then fetches + caches JWKS keys (TTL 15min, refresh onkidmiss).AGENTBRIDGE_OIDC_JWKS_URLalso supported.4. Audit retention + legal hold
POST /control/audit/checkpoint— sign the current audit head with Ed25519 so a third party can later prove the log wasn't truncated before this point.POST /control/audit/retention—{"action":"truncate","seq":N}removes entries withseq < N(safe after a checkpoint).{"action":"legal_hold","on":true}freezes truncation (subsequent truncate attempts return 409).store.truncate_audit_before(seq)on all 3 backends.5. Graceful shutdown + split health probes
lifespaninstalls SIGTERM/SIGINT handlers; flips readiness to False; drains in-flight requests up toAGENTBRIDGE_SHUTDOWN_GRACE(default 10s); then closes./health— liveness (always 200, even during drain, so k8s doesn't restart the pod mid-shutdown)./ready— readiness (503 during drain OR if the governance store is unreachable)./version— build info (version, Python, store type).6. Retry/backoff on transient store errors
append_audit_chainedandmutate_budgetnow retry on SQLitedatabase is locked/ psycopgOperationalError(up to 4 attempts, exponential + jitter, capped at 0.5s). Permanent errors bubble immediately.7. Config validation at startup
src/config.pyvalidates env vars before any state is created.AGENTBRIDGE_ADMIN_KEYandAGENTBRIDGE_DB; rate-limit and shutdown-grace values are range-checked; OIDC issuer must be a URL; psycopg must be importable when a postgres URL is configured.ConfigError(fail-fast at boot, not first request).8. Production-safety polish
/docs,/redoc) suppressed whenAGENTBRIDGE_ENV=productionunlessAGENTBRIDGE_DOCS=1.serveimprovements:--workers N,--log-level, disables uvicorn's noisy access log (we have our own structured middleware).prometheus-clientadded as a runtime dependency;[otel]optional extra added for OpenTelemetry.File changes (17 files, +2072/-214)
New files (5):
src/observability/__init__.py— Prometheus metrics + OpenTelemetry tracingsrc/observability/logging.py— structured JSON logging with correlation IDssrc/config.py— startup config validation (fail-fast)src/governance/resilience.py— retry/backoff on transient store errorstests/test_production_readiness.py— 21 new testsModified files (12):
src/api/control_plane.py— middleware rewrite,/metrics+/ready+/version, lifespan, validate_config, audit retention endpoints, AUTH_FAILURES / approvals gaugessrc/api/auth_oidc.py— JWKS auto-discovery + cache + key rotation onkidmisssrc/governance/store.py— approval table + methods,consume_approval,truncate_audit_before, retry on atomic opssrc/governance/approvals.py— store-backed rewritesrc/governance/audit.py—set_legal_hold,truncate_beforesrc/governance/gateway.py— span + metrics instrumentationsrc/protocols/registry.py— translation-duration metricsrc/cli.py—--workers,--log-level, graceful shutdownpyproject.toml—prometheus-clientruntime dep,[otel]extraCHANGELOG.md,docs/ROADMAP.md,docs/ENTERPRISE.md— updated to reflect what's now doneTest results
Skips:
AGENTBRIDGE_TEST_PG=postgres://...)New test coverage in
tests/test_production_readiness.py:/health+/ready+/version+/metricsendpointsWhat's NOT in this PR (still genuinely demand-gated)
Reviewer notes
src/observability/→src/governance/resilience.py→src/governance/store.py(approvals + truncate) →src/governance/approvals.py+audit.py→src/config.py→src/api/control_plane.py(the biggest single file) →src/api/auth_oidc.py→ tests.ApprovalQueue(store=store)change incontrol_plane.pyis the only line that changes existing runtime behavior — approvals now persist across restarts.Disclosure
This PR was produced with AI assistance (Claude) under human direction. The author reviewed every change, ran the test suite, and verified the end-to-end smoke test (see CHANGELOG for the smoke-test checklist).