fix(cli): plan correct active-plan resolution in isolated environments #1184

Merged
brent.edwards merged 3 commits from fix/plan-correct-resolve into master 2026-03-31 01:49:16 +00:00
Member

Summary

  • tighten plan correct active-plan fallback so it only runs for isolated CLEVERAGENTS_HOME mismatch cases and never when explicit DB env overrides are configured
  • narrow fallback exception handling in _resolve_active_plan_id() to expected DB/path/service failures; unexpected errors now surface instead of being silently swallowed
  • add BDD regression coverage for both safeguards in features/consolidated_plan_misc.feature + features/steps/plan_cli_legacy_r2_steps.py

Validation

  • nox -e lint: PASS
  • nox -e typecheck: PASS
  • nox -e unit_tests: PASS
  • nox -e integration_tests: FAIL in current branch baseline (29 failing Robot integration tests in this environment)
  • nox -e e2e_tests: FAIL in current branch baseline (45 failing E2E tests in this environment)
  • nox -e coverage_report: PASS (97%)

Closes #1025

## Summary - tighten `plan correct` active-plan fallback so it only runs for isolated `CLEVERAGENTS_HOME` mismatch cases and never when explicit DB env overrides are configured - narrow fallback exception handling in `_resolve_active_plan_id()` to expected DB/path/service failures; unexpected errors now surface instead of being silently swallowed - add BDD regression coverage for both safeguards in `features/consolidated_plan_misc.feature` + `features/steps/plan_cli_legacy_r2_steps.py` ## Validation - `nox -e lint`: PASS - `nox -e typecheck`: PASS - `nox -e unit_tests`: PASS - `nox -e integration_tests`: FAIL in current branch baseline (29 failing Robot integration tests in this environment) - `nox -e e2e_tests`: FAIL in current branch baseline (45 failing E2E tests in this environment) - `nox -e coverage_report`: PASS (97%) Closes #1025
brent.edwards added this to the v3.3.0 milestone 2026-03-29 00:26:12 +00:00
brent.edwards force-pushed fix/plan-correct-resolve from 6e2556a51a
All checks were successful
CI / benchmark-publish (pull_request) Has been skipped
CI / lint (pull_request) Successful in 3m22s
CI / quality (pull_request) Successful in 3m46s
CI / typecheck (pull_request) Successful in 4m1s
CI / build (pull_request) Successful in 14s
CI / unit_tests (pull_request) Successful in 4m9s
CI / security (pull_request) Successful in 4m13s
CI / helm (pull_request) Successful in 21s
CI / docker (pull_request) Successful in 1m26s
CI / integration_tests (pull_request) Successful in 7m8s
CI / e2e_tests (pull_request) Successful in 9m34s
CI / coverage (pull_request) Successful in 11m35s
CI / status-check (pull_request) Successful in 1s
CI / benchmark-regression (pull_request) Successful in 53m6s
to a48410547b
All checks were successful
CI / lint (pull_request) Successful in 25s
CI / typecheck (pull_request) Successful in 50s
CI / security (pull_request) Successful in 55s
CI / build (pull_request) Successful in 18s
CI / helm (pull_request) Successful in 34s
CI / quality (pull_request) Successful in 3m45s
CI / unit_tests (pull_request) Successful in 4m16s
CI / integration_tests (pull_request) Successful in 4m22s
CI / benchmark-publish (pull_request) Has been skipped
CI / docker (pull_request) Successful in 1m20s
CI / e2e_tests (pull_request) Successful in 9m23s
CI / coverage (pull_request) Successful in 12m32s
CI / status-check (pull_request) Successful in 2s
CI / benchmark-regression (pull_request) Successful in 58m4s
2026-03-30 00:30:14 +00:00
Compare
freemo approved these changes 2026-03-30 04:21:12 +00:00
Dismissed
freemo left a comment

Review: APPROVED with Comments

Solid fix with good multi-level test coverage (BDD + Robot).

Notes

  1. Broad exception suppression: The fallback catches (CleverAgentsError, OSError, SQLAlchemyError, ValueError) and silently returns None. Per CONTRIBUTING.md §Exception Propagation, consider adding a warning-level log instead of silent suppression so issues are traceable.
  2. Direct UnitOfWork usage: _resolve_from_cleveragents_home creates a secondary DB connection outside DI wiring. Acceptable for this fallback path but worth documenting.
  3. Good: Guard conditions (skip when explicit DB env overrides set, skip when paths match) are well-reasoned.
## Review: APPROVED with Comments Solid fix with good multi-level test coverage (BDD + Robot). ### Notes 1. **Broad exception suppression**: The fallback catches `(CleverAgentsError, OSError, SQLAlchemyError, ValueError)` and silently returns `None`. Per CONTRIBUTING.md §Exception Propagation, consider adding a `warning`-level log instead of silent suppression so issues are traceable. 2. **Direct `UnitOfWork` usage**: `_resolve_from_cleveragents_home` creates a secondary DB connection outside DI wiring. Acceptable for this fallback path but worth documenting. 3. **Good**: Guard conditions (skip when explicit DB env overrides set, skip when paths match) are well-reasoned.
Merge branch 'master' into fix/plan-correct-resolve
Some checks failed
CI / benchmark-publish (pull_request) Has been skipped
CI / build (pull_request) Successful in 21s
CI / helm (pull_request) Successful in 23s
CI / lint (pull_request) Successful in 3m20s
CI / quality (pull_request) Successful in 3m43s
CI / integration_tests (pull_request) Failing after 3m49s
CI / typecheck (pull_request) Successful in 3m57s
CI / unit_tests (pull_request) Successful in 4m13s
CI / security (pull_request) Successful in 4m18s
CI / docker (pull_request) Successful in 1m21s
CI / coverage (pull_request) Successful in 12m4s
CI / e2e_tests (pull_request) Successful in 20m33s
CI / status-check (pull_request) Failing after 1s
CI / benchmark-regression (pull_request) Has been cancelled
b00264ed3f
brent.edwards dismissed freemo's review 2026-03-31 00:33:40 +00:00
Reason:

New commits pushed, approval review dismissed automatically according to repository settings

brent.edwards scheduled this pull request to auto merge when all checks succeed 2026-03-31 00:33:59 +00:00
fix(cli): use container DI override for correction service mock in integration test
All checks were successful
CI / benchmark-publish (pull_request) Has been skipped
CI / build (pull_request) Successful in 21s
CI / helm (pull_request) Successful in 23s
CI / typecheck (pull_request) Successful in 47s
CI / lint (pull_request) Successful in 3m20s
CI / quality (pull_request) Successful in 3m55s
CI / security (pull_request) Successful in 4m8s
CI / unit_tests (pull_request) Successful in 6m19s
CI / docker (pull_request) Successful in 12s
CI / integration_tests (pull_request) Successful in 9m11s
CI / coverage (pull_request) Successful in 13m11s
CI / e2e_tests (pull_request) Successful in 20m10s
CI / status-check (pull_request) Successful in 2s
CI / benchmark-regression (pull_request) Successful in 55m31s
9dae67737e
The plan_correct_isolated_resolve robot helper patched CorrectionService
at the module level, but correct_decision now resolves the service via
container.correction_service(). The module-level patch had no effect on
the container's already-imported reference, causing the mock to never
intercept the call and the test to fail with exit code 1.

Replace unittest.mock.patch with container.correction_service.override()
which is the idiomatic dependency-injector mechanism and correctly
intercepts the singleton provider.

ISSUES CLOSED: #1025
brent.edwards deleted branch fix/plan-correct-resolve 2026-03-31 01:49:17 +00:00
Sign in to join this conversation.
No reviewers
No milestone
No project
No assignees
2 participants
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Reference
cleveragents/cleveragents-core!1184
No description provided.