cleveragents/cleveragents-core

Fork 3

feat(server): Kubernetes/Helm deployment configuration #928

New issue

Closed

opened 2026-03-14 00:11:52 +00:00 by freemo · 6 comments

freemo commented

2026-03-14 00:11:52 +00:00

Owner

Background

The specification (lines 43436-43441) requires Kubernetes deployment support:

Helm chart in k8s/ directory
Kubernetes Ingress with TLS termination
Configurable replicas and resource limits
Optional Redis for multi-instance session affinity and rate limiting (spec line 43444)

Currently only a basic CLI-packaging Dockerfile exists at /app/Dockerfile. No k8s/ directory, no Helm chart, no server-specific Docker configuration.

Acceptance Criteria

k8s/ directory with Helm chart
Kubernetes Deployment, Service, Ingress manifests
TLS termination configuration
Configurable replicas and resource limits in values.yaml
Server-specific Dockerfile (not just CLI packaging)
Optional Redis subchart/configuration for session affinity
README with deployment instructions

Dependencies

Depends on #862 (ASGI endpoint must exist)
Depends on #878 (PostgreSQL backend for stateless server pods)

Metadata

Suggested commit message: feat(server): add Kubernetes Helm chart for server deployment
Suggested branch name: feat/k8s-helm-deployment

Definition of Done

Code merged to main, helm install produces a working server deployment.

## Background The specification (lines 43436-43441) requires Kubernetes deployment support: - Helm chart in `k8s/` directory - Kubernetes Ingress with TLS termination - Configurable replicas and resource limits - Optional Redis for multi-instance session affinity and rate limiting (spec line 43444) Currently only a basic CLI-packaging `Dockerfile` exists at `/app/Dockerfile`. No `k8s/` directory, no Helm chart, no server-specific Docker configuration. ## Acceptance Criteria - [x] `k8s/` directory with Helm chart - [x] Kubernetes Deployment, Service, Ingress manifests - [x] TLS termination configuration - [x] Configurable replicas and resource limits in `values.yaml` - [x] Server-specific Dockerfile (not just CLI packaging) - [x] Optional Redis subchart/configuration for session affinity - [x] README with deployment instructions ## Dependencies - Depends on #862 (ASGI endpoint must exist) - Depends on #878 (PostgreSQL backend for stateless server pods) ## Metadata - **Suggested commit message:** `feat(server): add Kubernetes Helm chart for server deployment` - **Suggested branch name:** `feat/k8s-helm-deployment` ## Definition of Done Code merged to `main`, `helm install` produces a working server deployment.

freemo added the

labels

2026-03-14 00:12:00 +00:00

freemo added this to the v3.7.0 milestone

2026-03-14 00:12:08 +00:00

brent.edwards was assigned by freemo

2026-03-14 04:27:37 +00:00

brent.edwards added

and removed

labels

2026-03-20 21:59:58 +00:00

brent.edwards commented

2026-03-20 23:07:25 +00:00

Member

Implementation Notes

Design Decisions

Helm Chart v2 API: Used apiVersion: v2 (Helm 3 format) following modern practices. Chart type is application as this deploys a runnable server, not a library.
Redis as Optional Subchart: Redis is configured as a Bitnami subchart dependency with condition: redis.enabled, defaulting to disabled. When enabled, it provides session affinity and rate limiting for multi-replica deployments per spec line 43538.
Ingress with TLS Termination: The ingress template supports TLS termination at the controller level (per spec line 43459: "HTTPS required for all production deployments, TLS termination at Kubernetes ingress"). The server itself communicates plain HTTP within the cluster — TLS is terminated by the ingress controller.
Server Dockerfile vs CLI Dockerfile: Created Dockerfile.server separate from the existing Dockerfile (which packages the CLI). The server Dockerfile:
- Uses multi-stage build (builder + runtime stages) per spec line 43691
- Runs as non-root user appuser (uid 1000) per spec
- Exposes port 8000 for the ASGI server
- Uses uvicorn as the ASGI entrypoint targeting cleveragents.a2a.asgi:app
- Includes a HEALTHCHECK against /health
Security Hardening: The Deployment template enforces:
- runAsNonRoot: true at pod level
- runAsUser: 1000 (matching Dockerfile user)
- readOnlyRootFilesystem: true at container level
- capabilities.drop: ["ALL"] at container level
- allowPrivilegeEscalation: false
ConfigMap for Server Settings: Environment variables for server configuration (host, port, workers, log level) are managed via a ConfigMap, with secrets (database URL, Redis password) pulled from Kubernetes Secrets via existingSecret references.

Files Created

k8s/Chart.yaml — Helm chart metadata with Redis subchart dependency
k8s/values.yaml — All configurable values (replicas, resources, ingress, TLS, Redis, database, server settings)
k8s/templates/_helpers.tpl — Template helper functions (name, fullname, labels, selectors, serviceAccountName, redisHost)
k8s/templates/deployment.yaml — Kubernetes Deployment with environment variable injection
k8s/templates/service.yaml — ClusterIP Service
k8s/templates/ingress.yaml — Ingress with TLS support (conditional on ingress.enabled)
k8s/templates/configmap.yaml — Server configuration ConfigMap
k8s/templates/NOTES.txt — Post-install usage instructions
k8s/README.md — Deployment guide with quick start, scaling, TLS, Redis, and database configuration
Dockerfile.server — Multi-stage server container build
features/k8s_helm_chart.feature — 20 BDD scenarios validating chart structure
features/steps/k8s_helm_chart_steps.py — Step definitions for chart validation
robot/k8s_helm_chart.robot — 11 Robot Framework integration tests
benchmarks/k8s_helm_chart_bench.py — ASV benchmarks for YAML parsing

Quality Gate Results

lint: ✅ passed
format: ✅ passed (1603 files unchanged)
typecheck: ✅ passed (0 errors)
security_scan: ✅ passed
dead_code: ✅ passed
unit_tests: ✅ passed (452 features, 12096 scenarios, 0 failures)
integration_tests: ✅ passed (1618 tests, 0 failures)
docs: ✅ passed
build: ✅ passed
benchmark: ✅ passed
coverage_report: ✅ 99% (threshold: 97%)

## Implementation Notes ### Design Decisions 1. **Helm Chart v2 API**: Used `apiVersion: v2` (Helm 3 format) following modern practices. Chart type is `application` as this deploys a runnable server, not a library. 2. **Redis as Optional Subchart**: Redis is configured as a Bitnami subchart dependency with `condition: redis.enabled`, defaulting to disabled. When enabled, it provides session affinity and rate limiting for multi-replica deployments per spec line 43538. 3. **Ingress with TLS Termination**: The ingress template supports TLS termination at the controller level (per spec line 43459: "HTTPS required for all production deployments, TLS termination at Kubernetes ingress"). The server itself communicates plain HTTP within the cluster — TLS is terminated by the ingress controller. 4. **Server Dockerfile vs CLI Dockerfile**: Created `Dockerfile.server` separate from the existing `Dockerfile` (which packages the CLI). The server Dockerfile: - Uses multi-stage build (builder + runtime stages) per spec line 43691 - Runs as non-root user `appuser` (uid 1000) per spec - Exposes port 8000 for the ASGI server - Uses `uvicorn` as the ASGI entrypoint targeting `cleveragents.a2a.asgi:app` - Includes a HEALTHCHECK against `/health` 5. **Security Hardening**: The Deployment template enforces: - `runAsNonRoot: true` at pod level - `runAsUser: 1000` (matching Dockerfile user) - `readOnlyRootFilesystem: true` at container level - `capabilities.drop: ["ALL"]` at container level - `allowPrivilegeEscalation: false` 6. **ConfigMap for Server Settings**: Environment variables for server configuration (host, port, workers, log level) are managed via a ConfigMap, with secrets (database URL, Redis password) pulled from Kubernetes Secrets via `existingSecret` references. ### Files Created - `k8s/Chart.yaml` — Helm chart metadata with Redis subchart dependency - `k8s/values.yaml` — All configurable values (replicas, resources, ingress, TLS, Redis, database, server settings) - `k8s/templates/_helpers.tpl` — Template helper functions (name, fullname, labels, selectors, serviceAccountName, redisHost) - `k8s/templates/deployment.yaml` — Kubernetes Deployment with environment variable injection - `k8s/templates/service.yaml` — ClusterIP Service - `k8s/templates/ingress.yaml` — Ingress with TLS support (conditional on `ingress.enabled`) - `k8s/templates/configmap.yaml` — Server configuration ConfigMap - `k8s/templates/NOTES.txt` — Post-install usage instructions - `k8s/README.md` — Deployment guide with quick start, scaling, TLS, Redis, and database configuration - `Dockerfile.server` — Multi-stage server container build - `features/k8s_helm_chart.feature` — 20 BDD scenarios validating chart structure - `features/steps/k8s_helm_chart_steps.py` — Step definitions for chart validation - `robot/k8s_helm_chart.robot` — 11 Robot Framework integration tests - `benchmarks/k8s_helm_chart_bench.py` — ASV benchmarks for YAML parsing ### Quality Gate Results - **lint**: ✅ passed - **format**: ✅ passed (1603 files unchanged) - **typecheck**: ✅ passed (0 errors) - **security_scan**: ✅ passed - **dead_code**: ✅ passed - **unit_tests**: ✅ passed (452 features, 12096 scenarios, 0 failures) - **integration_tests**: ✅ passed (1618 tests, 0 failures) - **docs**: ✅ passed - **build**: ✅ passed - **benchmark**: ✅ passed - **coverage_report**: ✅ 99% (threshold: 97%)

brent.edwards referenced this issue from a commit

2026-03-20 23:08:23 +00:00

feat(server): add Kubernetes Helm chart for server deployment

~~brent.edwards referenced this issue 2026-03-20 23:08:48 +00:00~~

feat(server): add Kubernetes Helm chart for server deployment #1085

brent.edwards added a new dependency

2026-03-20 23:10:40 +00:00

#1085 feat(server): add Kubernetes Helm chart for server deployment

brent.edwards added

and removed

labels

2026-03-20 23:10:53 +00:00

brent.edwards referenced this issue from a commit

2026-03-21 01:32:34 +00:00

feat(server): add Kubernetes Helm chart for server deployment

brent.edwards referenced this issue from a pull request that will close it,

2026-03-21 01:33:02 +00:00

feat(server): add Kubernetes Helm chart for server deployment #1085

brent.edwards referenced this issue from a commit

2026-03-21 02:59:44 +00:00

feat(server): add Kubernetes Helm chart for server deployment

brent.edwards referenced this issue from a commit

2026-03-21 04:13:33 +00:00

feat(server): add Kubernetes Helm chart for server deployment

brent.edwards referenced this issue

2026-03-21 04:13:43 +00:00

feat(server): align Dockerfile entrypoint with spec (python -m cleveragents serve) #1088

brent.edwards referenced this issue

2026-03-21 04:13:48 +00:00

feat(ci): add Helm lint and template rendering to CI pipeline #1089

brent.edwards referenced this issue from a commit

2026-03-21 05:30:12 +00:00

feat(server): add Kubernetes Helm chart for server deployment

brent.edwards commented

2026-03-21 05:40:27 +00:00

Member

Self-QA Implementation Notes (Cycles 1–5)

Cycle 1

Review findings: 7C / 12M / 11m / 5n

C1: Missing ServiceAccount template — deployment would fail
C2: readOnlyRootFilesystem: true without writable volumes — container crash
C3: Database URL exposed as plaintext
C4: Redis password exposed as plaintext
C5: Unrelated repo CLI deletion bundled (false positive — verified not in diff)
C6: Unrelated Robot timeout changes bundled (false positive)
C7: PR description empty

Fixes applied: 22 items fixed including ServiceAccount template, emptyDir volumes for /tmp and /app/data, secrets.yaml for credential management, ConfigMap envFrom wiring, NOTES.txt scoping fix, Redis host helper, Deployment command/args override, uv pinning (0.6.14), HEALTHCHECK removal, regex-based Dockerfile validation, Robot helper extraction, PR description added. 13 items deferred.

Cycle 2

Review findings: 1C / 7M / 11m / 6n

C1: Estimation actor removal bundled in commit (atomic commit violation)
M2–M8: containerPort mismatch, Redis password fallback, minimal .dockerignore, uv below spec minimum, Helm CI missing, Pyright error in Robot helper, Robot tests duplicate Behave

Fixes applied: 21 items fixed including estimation actor revert (rebased onto master which has it), containerPort → server.port, Redis Bitnami fallback, expanded .dockerignore, uv bumped to 0.8.0, typed _COMMANDS dispatch dict, Robot tests refocused on cross-file consistency, new BDD scenarios for design decisions, seccompProfile added, CONTRIBUTING.md updated. Follow-up issues #1088 (Dockerfile entrypoint) and #1089 (Helm CI) created.

Cycle 3

Review findings: 0C / 7M / 8m / 7n

M1: Ingress duplicate annotation keys, M2: pod-level automountServiceAccountToken, M3: Redis version too wide, M4: untracked spec entrypoint, M5: Robot tests misrepresented as cross-file, M6: zero conditional branch coverage, M7: shallow substring matching

Fixes applied: All 7 majors + 8 minors fixed. Ingress ssl-redirect dedup guard, automountServiceAccountToken on pod spec, Redis pinned to ~18.19.0, Robot dead code removed, 11 new conditional branch BDD scenarios, regex template matching step added, service.targetPort dead config removed, ConfigMap workers var removed.

Cycle 4

Review findings: 0C / 2M / 11m / 7n

M1: NOTES.txt TLS warning in wrong block, M2: index nil panic on annotations

Fixes applied: Both majors + 7 minors fixed. TLS warning moved to ingress-enabled block, hasKey with default dict for nil safety, automountServiceAccountToken SA test, readOnlyRootFilesystem/allowPrivilegeEscalation/uv removal BDD scenarios, volume mount assertions, TLS regex assertion, Robot test #9 refactored to helper.

Cycle 5

Review findings: 2C / 8M / 12m / 9n

C1: BuiltinAdapter feature deletion bundled (atomic commit violation — new unrelated changes appearing from rebase drift)
C2: CHANGELOG entry for #882 removed
M1–M8: Missing all restore, Redis Bitnami fallback test gap, ConfigMap env var name testing, redis.enabled guard test, weak command/entrypoint Robot validation, negative BDD deferral untracked, Dockerfile entrypoint, Helm CI

Status: Not yet fixed — at cycle 5 checkpoint.

Remaining Issues (After 5 Cycles)

The PR has steadily improved (7C/12M → 1C/7M → 0C/7M → 0C/2M → 2C/8M). The cycle 5 regression is primarily due to rebase-related contamination where unrelated commits on master are being pulled into the diff. Core Helm chart quality is high. Outstanding work: isolate unrelated changes from commit, add ~4 missing test scenarios, create tracking issue for negative tests.

Quality Gates (Latest)

## Self-QA Implementation Notes (Cycles 1–5) ### Cycle 1 **Review findings:** 7C / 12M / 11m / 5n - C1: Missing ServiceAccount template — deployment would fail - C2: `readOnlyRootFilesystem: true` without writable volumes — container crash - C3: Database URL exposed as plaintext - C4: Redis password exposed as plaintext - C5: Unrelated repo CLI deletion bundled (false positive — verified not in diff) - C6: Unrelated Robot timeout changes bundled (false positive) - C7: PR description empty **Fixes applied:** 22 items fixed including ServiceAccount template, emptyDir volumes for /tmp and /app/data, secrets.yaml for credential management, ConfigMap envFrom wiring, NOTES.txt scoping fix, Redis host helper, Deployment command/args override, uv pinning (0.6.14), HEALTHCHECK removal, regex-based Dockerfile validation, Robot helper extraction, PR description added. 13 items deferred. ### Cycle 2 **Review findings:** 1C / 7M / 11m / 6n - C1: Estimation actor removal bundled in commit (atomic commit violation) - M2–M8: containerPort mismatch, Redis password fallback, minimal .dockerignore, uv below spec minimum, Helm CI missing, Pyright error in Robot helper, Robot tests duplicate Behave **Fixes applied:** 21 items fixed including estimation actor revert (rebased onto master which has it), containerPort → server.port, Redis Bitnami fallback, expanded .dockerignore, uv bumped to 0.8.0, typed _COMMANDS dispatch dict, Robot tests refocused on cross-file consistency, new BDD scenarios for design decisions, seccompProfile added, CONTRIBUTING.md updated. Follow-up issues #1088 (Dockerfile entrypoint) and #1089 (Helm CI) created. ### Cycle 3 **Review findings:** 0C / 7M / 8m / 7n - M1: Ingress duplicate annotation keys, M2: pod-level automountServiceAccountToken, M3: Redis version too wide, M4: untracked spec entrypoint, M5: Robot tests misrepresented as cross-file, M6: zero conditional branch coverage, M7: shallow substring matching **Fixes applied:** All 7 majors + 8 minors fixed. Ingress ssl-redirect dedup guard, automountServiceAccountToken on pod spec, Redis pinned to ~18.19.0, Robot dead code removed, 11 new conditional branch BDD scenarios, regex template matching step added, service.targetPort dead config removed, ConfigMap workers var removed. ### Cycle 4 **Review findings:** 0C / 2M / 11m / 7n - M1: NOTES.txt TLS warning in wrong block, M2: `index` nil panic on annotations **Fixes applied:** Both majors + 7 minors fixed. TLS warning moved to ingress-enabled block, hasKey with default dict for nil safety, automountServiceAccountToken SA test, readOnlyRootFilesystem/allowPrivilegeEscalation/uv removal BDD scenarios, volume mount assertions, TLS regex assertion, Robot test #9 refactored to helper. ### Cycle 5 **Review findings:** 2C / 8M / 12m / 9n - C1: BuiltinAdapter feature deletion bundled (atomic commit violation — new unrelated changes appearing from rebase drift) - C2: CHANGELOG entry for #882 removed - M1–M8: Missing __all__ restore, Redis Bitnami fallback test gap, ConfigMap env var name testing, redis.enabled guard test, weak command/entrypoint Robot validation, negative BDD deferral untracked, Dockerfile entrypoint, Helm CI **Status:** Not yet fixed — at cycle 5 checkpoint. ### Remaining Issues (After 5 Cycles) The PR has steadily improved (7C/12M → 1C/7M → 0C/7M → 0C/2M → 2C/8M). The cycle 5 regression is primarily due to rebase-related contamination where unrelated commits on master are being pulled into the diff. Core Helm chart quality is high. Outstanding work: isolate unrelated changes from commit, add ~4 missing test scenarios, create tracking issue for negative tests. ### Quality Gates (Latest) lint ✅ | typecheck ✅ | unit_tests ✅ (12,248 scenarios) | integration_tests ✅ | e2e_tests ✅ (37) | coverage 98%

brent.edwards referenced this issue from a commit

2026-03-21 14:59:31 +00:00

feat(server): add Kubernetes Helm chart for server deployment

brent.edwards referenced this issue from a commit

2026-03-21 17:06:07 +00:00

feat(server): add Kubernetes Helm chart for server deployment

brent.edwards referenced this issue from a commit

2026-03-21 19:28:02 +00:00

feat(server): add Kubernetes Helm chart for server deployment

brent.edwards referenced this issue from a commit

2026-03-21 20:37:21 +00:00

feat(server): add Kubernetes Helm chart for server deployment

brent.edwards referenced this issue from a commit

2026-03-21 21:52:17 +00:00

feat(server): add Kubernetes Helm chart for server deployment

brent.edwards commented

2026-03-21 21:57:05 +00:00

Member

Self-QA Implementation Notes (Cycles 6–10)

Cycle 6

Review findings: 1C / 8M / 5m

C: Deployment/Docker ASGI target validity concerns.
M: entrypoint/spec mismatch, DB read-only fallback risk, missing Redis fallback branch test coverage, CI Helm enforcement gaps, missing redis-disabled negative coverage, branch hygiene concerns.

Fixes applied:

Added concrete src/cleveragents/a2a/asgi.py app target and /health endpoint.
Introduced cleveragents server serve command and wired Deployment/Docker command paths through CLI.
Added DB fail-fast in Helm template when DB config is missing.
Added Redis fallback branch tests and rendered checks.
Added Helm CI job and status-check wiring improvements.
Improved README dependency/security guidance and rendered test strictness.

Cycle 7

Review findings: 0C / 6M / 4m / 1n

M: ASGI protocol handling for non-http scopes, rendered assertions not hard-required in CI, Redis-disabled skip false-positive, missing behavioral DB fail-fast test, ingress warning-only fail-open posture, incomplete status-check dependency enforcement.

Fixes applied:

Implemented scope-correct ASGI handling for http / lifespan / websocket.
Made Helm-render assertions strict in CI (CLEVERAGENTS_REQUIRE_HELM_RENDER_ASSERTIONS=true).
Reworked Robot strict OK/SKIP markers to prevent skip-text false positives.
Added real behavioral helm template negative tests for DB required config.
Changed ingress behavior to fail when TLS missing unless explicit dev opt-out.
Updated status-check to enforce additional job results.

Cycle 8

Review findings: 0C / 8M / 1m

M: Helm dependency prep missing in docs/CI, lint inputs conflicting with DB fail-fast default, status-check not fully enforcing integration/e2e paths, rendered coverage gaps for DB existingSecret and Redis enabled branches, scope clarity gaps.

Fixes applied:

Added helm dependency build guidance in README and CI.
Updated Helm lint invocation with valid DB input values.
Added integration/e2e gating in status-check logic.
Added rendered tests for DB existingSecret and Redis credential branches.
Added positive rendered TLS ingress success tests.
Clarified Redis scope as infra scaffold/wiring in docs.

Cycle 9

Review findings: 1C / 5M

C/M: unrelated high-risk wrapping.py changes in PR scope, missing direct behavior tests for server serve, ASGI branch coverage gaps, readiness/liveness semantics, A2A scope ambiguity.

Fixes applied:

Removed src/cleveragents/tool/wrapping.py from PR diff (scope cleanup).
Added direct behavioral tests for server serve → uvicorn.run argument mapping.
Expanded ASGI tests for /, 404, and unsupported scope branch.
Split probe endpoints to /live and /ready with /health compatibility alias.
Documented explicit scope boundary for full production A2A behavior.

Cycle 10

Review findings: 0C / 6M / 3m

Remaining majors:
1. Missing Redis auth-disabled branch render test.
2. Missing rendered validation for ingress ssl-redirect override/default behavior.
3. Docker image references still tag-pinned (not digest-pinned).
4. /ready endpoint still dependency-blind.
5. ServiceAccount fallback to default when create=false and name unset.
6. Commit message formatting compliance concern.

Fixes applied this cycle:

No additional fixes yet (checkpoint reached).

Remaining Issues

After cycles 6–10, core deployment functionality, CI Helm enforcement, and branch atomicity improved significantly. Remaining issues are now concentrated in hardening and stricter test/process controls (render branch coverage, digest pinning, readiness depth, SA fallback strictness, commit formatting).

Latest Quality Gates (from cycle 9/10 branch state)

lint ✅
typecheck ✅
unit_tests ✅
integration_tests ✅
e2e_tests ✅
coverage_report ✅ (98%+)

## Self-QA Implementation Notes (Cycles 6–10) ### Cycle 6 **Review findings:** 1C / 8M / 5m - C: Deployment/Docker ASGI target validity concerns. - M: entrypoint/spec mismatch, DB read-only fallback risk, missing Redis fallback branch test coverage, CI Helm enforcement gaps, missing redis-disabled negative coverage, branch hygiene concerns. **Fixes applied:** - Added concrete `src/cleveragents/a2a/asgi.py` app target and `/health` endpoint. - Introduced `cleveragents server serve` command and wired Deployment/Docker command paths through CLI. - Added DB fail-fast in Helm template when DB config is missing. - Added Redis fallback branch tests and rendered checks. - Added Helm CI job and status-check wiring improvements. - Improved README dependency/security guidance and rendered test strictness. ### Cycle 7 **Review findings:** 0C / 6M / 4m / 1n - M: ASGI protocol handling for non-http scopes, rendered assertions not hard-required in CI, Redis-disabled skip false-positive, missing behavioral DB fail-fast test, ingress warning-only fail-open posture, incomplete status-check dependency enforcement. **Fixes applied:** - Implemented scope-correct ASGI handling for `http` / `lifespan` / `websocket`. - Made Helm-render assertions strict in CI (`CLEVERAGENTS_REQUIRE_HELM_RENDER_ASSERTIONS=true`). - Reworked Robot strict OK/SKIP markers to prevent skip-text false positives. - Added real behavioral `helm template` negative tests for DB required config. - Changed ingress behavior to fail when TLS missing unless explicit dev opt-out. - Updated status-check to enforce additional job results. ### Cycle 8 **Review findings:** 0C / 8M / 1m - M: Helm dependency prep missing in docs/CI, lint inputs conflicting with DB fail-fast default, status-check not fully enforcing integration/e2e paths, rendered coverage gaps for DB existingSecret and Redis enabled branches, scope clarity gaps. **Fixes applied:** - Added `helm dependency build` guidance in README and CI. - Updated Helm lint invocation with valid DB input values. - Added integration/e2e gating in status-check logic. - Added rendered tests for DB existingSecret and Redis credential branches. - Added positive rendered TLS ingress success tests. - Clarified Redis scope as infra scaffold/wiring in docs. ### Cycle 9 **Review findings:** 1C / 5M - C/M: unrelated high-risk `wrapping.py` changes in PR scope, missing direct behavior tests for `server serve`, ASGI branch coverage gaps, readiness/liveness semantics, A2A scope ambiguity. **Fixes applied:** - Removed `src/cleveragents/tool/wrapping.py` from PR diff (scope cleanup). - Added direct behavioral tests for `server serve` → `uvicorn.run` argument mapping. - Expanded ASGI tests for `/`, `404`, and unsupported scope branch. - Split probe endpoints to `/live` and `/ready` with `/health` compatibility alias. - Documented explicit scope boundary for full production A2A behavior. ### Cycle 10 **Review findings:** 0C / 6M / 3m - Remaining majors: 1. Missing Redis auth-disabled branch render test. 2. Missing rendered validation for ingress ssl-redirect override/default behavior. 3. Docker image references still tag-pinned (not digest-pinned). 4. `/ready` endpoint still dependency-blind. 5. ServiceAccount fallback to `default` when `create=false` and name unset. 6. Commit message formatting compliance concern. **Fixes applied this cycle:** - No additional fixes yet (checkpoint reached). ### Remaining Issues After cycles 6–10, core deployment functionality, CI Helm enforcement, and branch atomicity improved significantly. Remaining issues are now concentrated in hardening and stricter test/process controls (render branch coverage, digest pinning, readiness depth, SA fallback strictness, commit formatting). ### Latest Quality Gates (from cycle 9/10 branch state) - lint ✅ - typecheck ✅ - unit_tests ✅ - integration_tests ✅ - e2e_tests ✅ - coverage_report ✅ (98%+)

freemo commented

2026-03-22 16:40:02 +00:00

Author

Owner

Planning Agent — Discussion Review

@brent.edwards — This is substantial work. 10 self-QA cycles with detailed findings and fixes demonstrates strong engineering discipline. Let me evaluate the key points.

Design decisions — approved:

Helm v2 API, application chart type: correct.
Redis as optional subchart with condition: redis.enabled: properly scoped.
Separate Dockerfile.server from CLI Dockerfile: good separation of concerns.
Security hardening (nonroot, readonly rootfs, drop ALL caps): aligns with spec.
ServiceAccount template addition (C1 fix): critical catch — deployment would have failed without it.
emptyDir volumes for writable paths with readOnlyRootFilesystem: correct pattern.

Concerns:

Rebase contamination pattern: Cycles 5 and beyond show unrelated changes (BuiltinAdapter deletion, CHANGELOG entry removal, estimation actor changes) appearing in the PR diff due to rebase drift. Per CONTRIBUTING.md §Atomic Commits: "Never bundle cosmetic changes with functional changes" and "isolate large-scale code moves." The PR must be cleaned up to contain only Helm/K8s deployment changes before merge. This is a blocking concern.
Cycle count: 10 QA cycles is high. While thoroughness is valued, this suggests the scope may be too large for a single PR. Consider whether the ASGI server implementation (asgi.py, server serve command, health endpoints) should be split into a separate PR from the Helm chart itself. Each should be an atomic, independently reviewable unit per CONTRIBUTING.md.
Follow-up issues created: #1088 (Dockerfile entrypoint) and #1089 (Helm CI) — good. These are correctly deferred to separate milestones rather than bloating this PR further.

Action items:

Clean the PR branch to remove all unrelated changes from the diff (rebase contamination).
Evaluate whether to split ASGI server implementation from Helm chart into two PRs.
Address the 6 remaining majors from Cycle 10 (Redis auth branch test, ingress SSL redirect, digest pinning, readiness depth, SA fallback, commit formatting).

This PR should not be merged until the rebase contamination is resolved. The core Helm work is high quality.

## Planning Agent — Discussion Review @brent.edwards — This is substantial work. 10 self-QA cycles with detailed findings and fixes demonstrates strong engineering discipline. Let me evaluate the key points. **Design decisions — approved:** - Helm v2 API, application chart type: correct. - Redis as optional subchart with `condition: redis.enabled`: properly scoped. - Separate `Dockerfile.server` from CLI Dockerfile: good separation of concerns. - Security hardening (nonroot, readonly rootfs, drop ALL caps): aligns with spec. - ServiceAccount template addition (C1 fix): critical catch — deployment would have failed without it. - `emptyDir` volumes for writable paths with `readOnlyRootFilesystem`: correct pattern. **Concerns:** 1. **Rebase contamination pattern**: Cycles 5 and beyond show unrelated changes (BuiltinAdapter deletion, CHANGELOG entry removal, estimation actor changes) appearing in the PR diff due to rebase drift. Per CONTRIBUTING.md §Atomic Commits: "Never bundle cosmetic changes with functional changes" and "isolate large-scale code moves." The PR must be cleaned up to contain **only** Helm/K8s deployment changes before merge. This is a blocking concern. 2. **Cycle count**: 10 QA cycles is high. While thoroughness is valued, this suggests the scope may be too large for a single PR. Consider whether the ASGI server implementation (`asgi.py`, `server serve` command, health endpoints) should be split into a separate PR from the Helm chart itself. Each should be an atomic, independently reviewable unit per CONTRIBUTING.md. 3. **Follow-up issues created**: #1088 (Dockerfile entrypoint) and #1089 (Helm CI) — good. These are correctly deferred to separate milestones rather than bloating this PR further. **Action items:** 1. Clean the PR branch to remove all unrelated changes from the diff (rebase contamination). 2. Evaluate whether to split ASGI server implementation from Helm chart into two PRs. 3. Address the 6 remaining majors from Cycle 10 (Redis auth branch test, ingress SSL redirect, digest pinning, readiness depth, SA fallback, commit formatting). This PR should not be merged until the rebase contamination is resolved. The core Helm work is high quality.

freemo referenced this issue

2026-03-22 16:43:13 +00:00

feat(ci): add Helm lint and template rendering to CI pipeline #1089

freemo referenced this issue

2026-03-22 16:43:18 +00:00

feat(server): align Dockerfile entrypoint with spec (python -m cleveragents serve) #1088

brent.edwards referenced this issue from a commit

2026-03-24 00:27:16 +00:00

feat(server): add Kubernetes Helm chart for server deployment

brent.edwards commented

2026-03-24 00:28:20 +00:00

Member

Implementation Notes — Review Cycle 10 Fixes

Addressed all 4 critical/major items from hurui200320's REQUEST_CHANGES review and all minor/nit items from both reviews.

Critical Fixes

Merge commit removed — Rebased feat/k8s-helm-deployment onto origin/master, eliminating the merge commit 6b43dade. Branch now has clean linear history per CONTRIBUTING.md rebase-only policy.
Commit message body reformatted — Replaced literal \n sequences with actual newlines. The body now has proper line wrapping with the ISSUES CLOSED: #928 footer on its own line after a blank separator.
Function-local import moved to module top — from uvicorn import run as uvicorn_run moved from inside server_serve() to the top of src/cleveragents/cli/commands/server.py. Test mock target updated from uvicorn.run to cleveragents.cli.commands.server.uvicorn_run in features/steps/server_cli_coverage_boost_steps.py.
Helm binary integrity verification — All 3 Helm install blocks in .forgejo/workflows/ci.yml now download the .sha256sum file and verify it with sha256sum -c before extraction. This closes the supply-chain risk for CI environments with access to API keys.

ASGI Improvements

405 responses — Added _KNOWN_PATHS frozenset in src/cleveragents/a2a/asgi.py. Known paths with wrong HTTP method now return 405 with Allow: GET header per RFC 9110 §15.5.6.
WebSocket protocol compliance — await receive() now consumes the websocket.connect event before sending close. Close code changed from 1000 to 1008 (Policy Violation).
Lifespan logging — Unrecognised lifespan messages are now logged via _logger.warning() instead of silently consumed.
Security headers — _send_response now sets content-length, x-content-type-options: nosniff, and cache-control: no-store.
Scope dispatch — Changed if/if/if to if/elif/elif for mutually exclusive scope types.

CI and Docker

Dockerfile.server build in CI — Added "Build Docker image (Server)" step to the docker job.
Base image standardised — python:3.13-slim (floating) consistent with CLI Dockerfile.
Layer caching improved — Split build tool install and wheel build into separate RUN instructions.
Credential patterns — .dockerignore now excludes *.pem, *.key, *.p12, *.pfx, credentials*.json.
Semgrep change reverted — Unrelated pass_filenames/entry change removed per atomic commit hygiene.

CLI

--log-level validation — Constrained via click.Choice to ["critical", "error", "warning", "info", "debug", "trace"].

Test Fixes

Removed unused ReceiveCallable alias and Callable/Awaitable imports.
_skip_if_helm_missing now returns bool; _render_chart uses return value instead of duplicate shutil.which check.
Lifespan test receive mock raises AssertionError with descriptive message instead of opaque IndexError.
WebSocket close frame assertion updated to expect code 1008.
Added BDD scenario "POST to known path returns method not allowed".

Quality Gates

nox -e lint ✅
nox -e typecheck ✅ (0 errors)
nox -e unit_tests ✅ (12,318 scenarios passed, 4 skipped)
nox -e coverage_report ✅ (98%)
Integration/E2E: 3 pre-existing failures each in unrelated areas (plan correction, resource types, automation profiles)

## Implementation Notes — Review Cycle 10 Fixes Addressed all 4 critical/major items from hurui200320's REQUEST_CHANGES review and all minor/nit items from both reviews. ### Critical Fixes 1. **Merge commit removed** — Rebased `feat/k8s-helm-deployment` onto `origin/master`, eliminating the merge commit `6b43dade`. Branch now has clean linear history per CONTRIBUTING.md rebase-only policy. 2. **Commit message body reformatted** — Replaced literal `\n` sequences with actual newlines. The body now has proper line wrapping with the `ISSUES CLOSED: #928` footer on its own line after a blank separator. 3. **Function-local import moved to module top** — `from uvicorn import run as uvicorn_run` moved from inside `server_serve()` to the top of `src/cleveragents/cli/commands/server.py`. Test mock target updated from `uvicorn.run` to `cleveragents.cli.commands.server.uvicorn_run` in `features/steps/server_cli_coverage_boost_steps.py`. 4. **Helm binary integrity verification** — All 3 Helm install blocks in `.forgejo/workflows/ci.yml` now download the `.sha256sum` file and verify it with `sha256sum -c` before extraction. This closes the supply-chain risk for CI environments with access to API keys. ### ASGI Improvements - **405 responses** — Added `_KNOWN_PATHS` frozenset in `src/cleveragents/a2a/asgi.py`. Known paths with wrong HTTP method now return 405 with `Allow: GET` header per RFC 9110 §15.5.6. - **WebSocket protocol compliance** — `await receive()` now consumes the `websocket.connect` event before sending close. Close code changed from 1000 to 1008 (Policy Violation). - **Lifespan logging** — Unrecognised lifespan messages are now logged via `_logger.warning()` instead of silently consumed. - **Security headers** — `_send_response` now sets `content-length`, `x-content-type-options: nosniff`, and `cache-control: no-store`. - **Scope dispatch** — Changed `if/if/if` to `if/elif/elif` for mutually exclusive scope types. ### CI and Docker - **Dockerfile.server build in CI** — Added "Build Docker image (Server)" step to the `docker` job. - **Base image standardised** — `python:3.13-slim` (floating) consistent with CLI Dockerfile. - **Layer caching improved** — Split build tool install and wheel build into separate RUN instructions. - **Credential patterns** — `.dockerignore` now excludes `*.pem`, `*.key`, `*.p12`, `*.pfx`, `credentials*.json`. - **Semgrep change reverted** — Unrelated `pass_filenames`/`entry` change removed per atomic commit hygiene. ### CLI - **`--log-level` validation** — Constrained via `click.Choice` to `["critical", "error", "warning", "info", "debug", "trace"]`. ### Test Fixes - Removed unused `ReceiveCallable` alias and `Callable`/`Awaitable` imports. - `_skip_if_helm_missing` now returns `bool`; `_render_chart` uses return value instead of duplicate `shutil.which` check. - Lifespan test receive mock raises `AssertionError` with descriptive message instead of opaque `IndexError`. - WebSocket close frame assertion updated to expect code 1008. - Added BDD scenario "POST to known path returns method not allowed". ### Quality Gates - `nox -e lint` ✅ - `nox -e typecheck` ✅ (0 errors) - `nox -e unit_tests` ✅ (12,318 scenarios passed, 4 skipped) - `nox -e coverage_report` ✅ (98%) - Integration/E2E: 3 pre-existing failures each in unrelated areas (plan correction, resource types, automation profiles)

brent.edwards referenced this issue

2026-03-24 00:28:52 +00:00

feat(server): add Kubernetes Helm chart for server deployment #1085

brent.edwards referenced this issue

2026-03-24 00:29:14 +00:00

feat(server): add Kubernetes Helm chart for server deployment #1085

brent.edwards referenced this issue from a commit

2026-03-25 21:51:02 +00:00

feat(server): add Kubernetes Helm chart for server deployment

brent.edwards commented

2026-03-25 21:52:33 +00:00

Member

Implementation Notes — Review Cycle 11 (PR #1085)

Addressed hurui200320's REQUEST_CHANGES review (#2687). All 4 blocking items resolved:

Changes Made

CI SHA256 checksum fix — .forgejo/workflows/ci.yml: All 3 Helm install blocks now save the tarball as its original filename (helm-v3.16.4-linux-amd64.tar.gz) instead of helm.tgz, fixing the checksum verification mismatch that was causing all 3 CI Helm jobs to fail.
Lifespan warning test — features/asgi_app.feature + features/steps/asgi_app_steps.py: New scenario exercises the _logger.warning() branch in cleveragents.a2a.asgi.app lifespan handler by injecting lifespan.bogus between startup and shutdown. Verifies both cycle completion and warning log emission.
Security headers test — features/asgi_app.feature + features/steps/asgi_app_steps.py: New scenario verifies content-length, x-content-type-options: nosniff, and cache-control: no-store headers are present in cleveragents.a2a.asgi._send_response output.
405 Allow header test — features/asgi_app.feature + features/steps/asgi_app_steps.py: Existing 405 scenario enhanced with Allow: GET header assertion. Additional 405 scenario added for /live path to improve _KNOWN_PATHS frozenset validation coverage.

Quality Gates

lint ✅ | typecheck ✅ | unit_tests ✅ (12,321 scenarios) | coverage ✅ (97.7%)
integration_tests: 3 pre-existing failures (unrelated)
Commit: e11593c7 on feat/k8s-helm-deployment (amend + force push)

## Implementation Notes — Review Cycle 11 (PR #1085) Addressed hurui200320's REQUEST_CHANGES review (#2687). All 4 blocking items resolved: ### Changes Made 1. **CI SHA256 checksum fix** — `.forgejo/workflows/ci.yml`: All 3 Helm install blocks now save the tarball as its original filename (`helm-v3.16.4-linux-amd64.tar.gz`) instead of `helm.tgz`, fixing the checksum verification mismatch that was causing all 3 CI Helm jobs to fail. 2. **Lifespan warning test** — `features/asgi_app.feature` + `features/steps/asgi_app_steps.py`: New scenario exercises the `_logger.warning()` branch in `cleveragents.a2a.asgi.app` lifespan handler by injecting `lifespan.bogus` between startup and shutdown. Verifies both cycle completion and warning log emission. 3. **Security headers test** — `features/asgi_app.feature` + `features/steps/asgi_app_steps.py`: New scenario verifies `content-length`, `x-content-type-options: nosniff`, and `cache-control: no-store` headers are present in `cleveragents.a2a.asgi._send_response` output. 4. **405 Allow header test** — `features/asgi_app.feature` + `features/steps/asgi_app_steps.py`: Existing 405 scenario enhanced with `Allow: GET` header assertion. Additional 405 scenario added for `/live` path to improve `_KNOWN_PATHS` frozenset validation coverage. ### Quality Gates - lint ✅ | typecheck ✅ | unit_tests ✅ (12,321 scenarios) | coverage ✅ (97.7%) - integration_tests: 3 pre-existing failures (unrelated) - Commit: `e11593c7` on `feat/k8s-helm-deployment` (amend + force push)

brent.edwards referenced this issue from a commit

2026-03-27 21:23:18 +00:00

fix(server): address review feedback and security scan stability

brent.edwards referenced this issue from a commit

2026-03-27 22:42:32 +00:00

feat(server): add Kubernetes Helm chart for server deployment

brent.edwards referenced this issue from a commit

2026-03-27 22:42:32 +00:00

fix(server): address review feedback and security scan stability

brent.edwards referenced this issue from a commit

2026-03-27 23:14:06 +00:00

feat(server): add Kubernetes Helm chart for server deployment