Add ULID format validation to all v3 plan commands to prevent users from
accidentally mixing legacy ('agents tell') and v3 ('agents plan use')
workflows. The two systems use separate storage backends and cannot be
mixed; this change detects the mismatch early and provides actionable
error messages.
Changes:
- Add _validate_plan_ulid() with proper Crockford Base32 regex
(^[0-9A-HJKMNP-TV-Z]{26}$, re.IGNORECASE) that correctly rejects
invalid characters (I, L, O, U), hyphens, and wrong-length strings
- Add _PLAN_ULID_RE compiled regex and _ULID_VALIDATION_ERROR_MSG constant
with actionable guidance explaining the legacy/v3 incompatibility
- Apply ULID validation to all v3 commands: execute_plan,
_lifecycle_apply_with_id, lifecycle_apply_plan, plan_status,
plan_errors, cancel_plan (only on user-provided IDs, not auto-discovered)
- Update _LEGACY_DEPRECATION_MSG and tell/build command warnings to
explicitly state that the two workflows are INCOMPATIBLE and cannot be mixed
- Add comprehensive BDD tests in features/plan_ulid_validation.feature
with step definitions using Typer CLI runner (not subprocess)
- Update CONTRIBUTING.md with 'Workflow Choice: Legacy vs. v3 Plan
Lifecycle' section documenting the incompatibility and migration path
ISSUES CLOSED: #1560
## Summary
This PR adds Kubernetes Helm deployment support for the CleverAgents server.
Closes#928
### What this PR includes
- Helm chart under `k8s/` with Deployment, Service, optional Ingress, ConfigMap,
ServiceAccount, Secrets, NOTES, and optional Redis subchart configuration.
- Multi-stage `Dockerfile.server` for server runtime deployment.
- Deployment-focused docs in `k8s/README.md`.
- Behave + Robot + benchmark coverage for chart/deployment wiring.
### Review fixes applied (cycle 11 — hurui200320 review #2687)
**Critical fix:**
1. **CI SHA256 checksum verification fixed** — All 3 Helm install blocks (`unit_tests`, `integration_tests`, `helm` jobs) now save the tarball using its original filename (`helm-v3.16.4-linux-amd64.tar.gz`) instead of `helm.tgz`, so the `.sha256sum` file can correctly locate and verify it. This was causing all 3 CI Helm jobs to fail with "No such file or directory".
**Major fixes (test coverage gaps):**
2. **405 Allow header now tested** — The existing "POST to known path returns method not allowed" scenario now asserts that the `Allow: GET` header is present. Added a second 405 scenario testing `POST /live` for broader `_KNOWN_PATHS` coverage (review item #11).
3. **Security-hardening headers now tested** — New scenario "HTTP responses include security-hardening headers" verifies `content-length`, `x-content-type-options: nosniff`, and `cache-control: no-store` headers are present on HTTP responses.
4. **Lifespan warning logging now tested** — New scenario "Unrecognised lifespan message type logs warning and continues" queues `lifespan.startup` → `lifespan.bogus` → `lifespan.shutdown` and verifies: (a) the app completes the lifespan cycle cleanly, (b) a warning is logged mentioning the unrecognised type.
### Review fixes applied (cycle 10)
**Critical/Major fixes (from hurui200320's prior REQUEST_CHANGES):**
1. **Rebased branch onto master** — Removed merge commit per CONTRIBUTING.md rebase-only policy. Clean linear history restored.
2. **Fixed commit message body** — Replaced literal `\n` sequences with actual newlines. `ISSUES CLOSED: #928` footer is now on its own line after a blank separator.
3. **Moved uvicorn import to module top level** — `from uvicorn import run as uvicorn_run` is now at the top of `src/cleveragents/cli/commands/server.py` per Import Guidelines. Updated test mock target from `uvicorn.run` to `cleveragents.cli.commands.server.uvicorn_run`.
4. **Added SHA256 checksum verification** — All three Helm CLI install blocks in `.forgejo/workflows/ci.yml` now download and verify `helm.sha256sum` before extracting the binary.
**Minor fixes:**
5. **Added `Dockerfile.server` build to CI** — New "Build Docker image (Server)" step in the `docker` job validates the server Dockerfile.
6. **ASGI 405 Method Not Allowed** — Known paths (`/`, `/live`, `/ready`, `/health`) now return 405 with `Allow: GET` header for non-GET methods, per RFC 9110 §15.5.6. Added `_KNOWN_PATHS` frozenset.
7. **WebSocket close protocol fix** — App now calls `await receive()` to consume the `websocket.connect` event before closing. Changed close code from 1000 (Normal Closure) to 1008 (Policy Violation).
8. **Lifespan handler logging** — Unrecognised lifespan message types are now logged as warnings instead of silently consumed.
9. **Security-hardening headers** — `_send_response` now includes `content-length`, `x-content-type-options: nosniff`, and `cache-control: no-store` on all HTTP responses.
10. **`.dockerignore` credential patterns** — Added `*.pem`, `*.key`, `*.p12`, `*.pfx`, `credentials*.json`.
11. **`--log-level` validation** — Constrained to `click.Choice(["critical", "error", "warning", "info", "debug", "trace"])` for clean CLI validation errors.
12. **Reverted unrelated semgrep pre-commit change** — `pass_filenames` and `entry` restored to original values per atomic commit hygiene.
13. **Removed unused `ReceiveCallable` type alias** and `Callable`/`Awaitable` imports from `asgi_app_steps.py`.
14. **Fixed redundant `shutil.which("helm")` check** — `_skip_if_helm_missing` now returns `bool` to eliminate the duplicate check in `_render_chart`.
15. **Improved test deque error handling** — Lifespan test receive mock now raises descriptive `AssertionError` instead of opaque `IndexError`.
16. **Scope type dispatch** — Changed `if/if/if` to `if/elif/elif` for mutually exclusive ASGI scope types.
17. **Dockerfile.server base image** — Standardised to `python:3.13-slim` (floating minor) consistent with CLI Dockerfile.
18. **Dockerfile layer caching** — Split `uv pip install build` and `python -m build` into separate `RUN` instructions.
19. **Removed extraneous double blank line** in Dockerfile.server.
### Deferred items (acknowledged, not in scope)
- PodDisruptionBudget, HorizontalPodAutoscaler, NetworkPolicy — Follow-up for production hardening.
- `appVersion: "1.0.0"` placeholder — Needs tracking issue for release versioning alignment.
- Readiness probe with downstream dependency checks — Documented limitation.
- Cross-system test for probe paths matching ASGI routes — Test enhancement.
- Improved benchmarks (helm template timing vs PyYAML parsing) — Benchmark quality improvement.
- CI DRY violation (Helm install 3×) — Code quality improvement, consider composite action.
- File length limits exceeded (`k8s_helm_chart_steps.py` 551 lines, `helper_k8s_helm_chart.py` 678 lines) — Non-blocking, can be split in follow-up.
- `runAsGroup: 1000` in pod security context — Defense-in-depth improvement.
- HEAD method support on known paths — RFC compliance, does not affect K8s probes.
- `click.Choice` log-level validation via CLI runner test — Test gap.
### Scope note: status-check CI gate
The `status-check` job now includes `integration_tests`, `e2e_tests`, and `helm` in its `needs` list. The `helm` job is new in this PR. The `integration_tests` and `e2e_tests` additions fix previously-missing gate checks — included here since this PR modifies both of those jobs to install Helm.
### Quality gates
- `nox -e lint` ✅
- `nox -e typecheck` ✅
- `nox -e unit_tests` ✅ (12,321 scenarios passed, 4 skipped)
- `nox -e integration_tests` — 3 pre-existing failures in unrelated areas (plan correction, resource types)
- `nox -e e2e_tests` — pre-existing failures (LLM API keys not available in local env)
- `nox -e coverage_report` ✅ (**97.7%**)
Reviewed-on: cleveragents/cleveragents-core#1085
Reviewed-by: Jeffrey Phillips Freeman <jeffrey.freeman@cleverthis.com>
Co-authored-by: Brent E. Edwards <brent.edwards@cleverthis.com>
Co-committed-by: Brent E. Edwards <brent.edwards@cleverthis.com>
Rename the TDD tag system from tdd_bug/tdd_bug_<N> to tdd_issue/tdd_issue_<N>
across the entire codebase. The tdd_expected_fail tag is unchanged.
The TDD expected-failure workflow is not limited to bug fixes — it applies
equally to any issue type (features, tasks, refactors). The _bug suffix was
misleading and narrowed the perceived scope. The new _issue suffix accurately
reflects that the TDD tagging system applies to any Forgejo issue.
Changes span 92 files:
- features/environment.py: validate_tdd_tags(), should_invert_result(), and
apply_tdd_inversion() updated — regex, variables, error messages
- robot/tdd_expected_fail_listener.py: _validate_tdd_tags(), _should_invert_result(),
start_test(), end_test() updated consistently
- 33 Behave .feature files: all @tdd_bug/@tdd_bug_<N> tags renamed
- 29 Robot .robot files: all tdd_bug/tdd_bug_<N> tags renamed
- 3 Robot fixture files renamed (tdd_bug_alone, tdd_missing_tdd_bug,
tdd_expected_fail_missing_bug_n) with content and references updated
- Tag validation tests and helpers updated (function names, command dispatch
keys, output strings, fixture references)
- CONTRIBUTING.md: section renamed from 'TDD Bug Test Tags' to
'TDD Issue Test Tags', all tag references and examples updated
- noxfile.py: comment references updated
- Step definition files, mock helpers, and benchmark files: docstring
references updated
ISSUES CLOSED: #965
Created ADR-042 (Resource Type Inheritance) defining single-inheritance
`inherits` field on resource type definitions with field resolution,
collection merging, handler inheritance, polymorphic tool binding,
auto-discovery, and DAG query matching. Max depth 5, single inheritance.
Created ADR-043 (Devcontainer Integration) defining devcontainer-instance
as a subtype of container-instance with lazy activation lifecycle,
devcontainer.json parsing, three container-project association patterns
(auto-detect, explicit mount, clone-into), and execution environment
routing with a 6-level precedence chain.
Specification updates across 20+ sections:
- Glossary: Resource Type Inheritance, Devcontainer, Execution Environment
- Resource type YAML schema: `inherits` field with structure reference
- Handler, sandbox strategy, and coherence tables: devcontainer-instance
- Auto-discovery: devcontainer detection subsection with WBS diagram
- Tool capability metadata: structured environment subfields
- Technology stack: devcontainer CLI row
- Project model: execution environment subsection with YAML example
- CLI agents resource add: --mount, --clone-into flags + 4 new examples
- CLI agents plan use: --execution-environment, --execution-env-priority
- CLI agents project context set: same execution environment flags
- Command synopsis block: updated for all new flags
- Execution environment routing section with precedence table + algorithm
- Resource type YAML Example 6: Devcontainer Instance (inherited type)
- End-to-end Example 16: Devcontainer-Driven Development
- End-to-end Example 17: Explicit Container with Directory Mount
- End-to-end Example 18: Container with Remote Repo Clone
Also fixed ADR index: added missing ADR-036 through ADR-040 entries,
updated next ADR number to 044.
CONTRIBUTING.md fixes backported from sister project:
- Fixed State label capitalization (State/In Progress -> State/In progress,
State/In Review -> State/In review) to match label definitions.
- Subtasks section: changed from optional to required, with exception
clause for trivially simple issues.
- Parent links: updated to use Forgejo dependency system instead of
textual references in issue descriptions.
- Fixed nox session flag typos: nox -e -> nox -s (7 occurrences).
- Replaced "Epics and Legendaries" section with comprehensive "Ticket
Type Hierarchy" defining the three-tier hierarchy (Issue -> Epic ->
Legendary) with formal criteria tables, cross-cutting rules for
hierarchy enforcement, completion semantics, promotion/demotion, and
milestone relationship rules.
- Fixed broken internal links referencing old "Epics and Legendaries"
anchor to use "Ticket Type Hierarchy" (3 occurrences).
- Traceability example: replaced project-specific code reference with
generic example.
- Removed stray horizontal rule before Project-Specific Guidelines.
ISSUES CLOSED: #491
Refined the branch/tag (Ref) field requirement: development issues (features,
fixes, chores) must set it to the same branch as the one in the Metadata
section once the issue reaches State/In Progress, and must always reference a
branch not a tag. Non-development issues (bug reports, questions, support)
may leave it blank or optionally point to the branch or tag being referenced.
Refs: #408
Added requirement that every issue must have its Forgejo branch/tag (Ref) field
set to the working branch once work begins. This field is separate from the
branch name in the issue body Metadata section but must match it. Setting the
field allows Forgejo to associate issues with branches in its UI.
Refs: #408
Added explicit requirement that every PR must include a detailed description
with a summary, closing keywords for linked issues, and a Forgejo dependency
link. Updated the merge checklist with PR description and dependency link
verification items. Reworked PR Process item 1 to be comprehensive about
both the description body and the issue linkage.
Refs: #408
Added milestone requirements: issues beyond State/Unverified must be assigned
to a milestone; PRs must be assigned to the same milestone as their linked
issues. Added PR label rules: every PR must carry exactly one Type/ label
matching the nature of the change. Updated the triaging checklist to include
milestone assignment and updated the merge checklist with milestone and label
verification items.
Refs: #408
freemo
brent.edwards
hurui200320
