3 Commits

Author SHA1 Message Date
hurui200320 1fda56b778 fix(cli): implement plan diff --correction to show real correction attempt diff
CI / lint (pull_request) Successful in 29s
CI / typecheck (pull_request) Successful in 58s
CI / quality (pull_request) Successful in 29s
CI / security (pull_request) Successful in 1m11s
CI / build (pull_request) Successful in 24s
CI / push-validation (pull_request) Successful in 21s
CI / helm (pull_request) Successful in 41s
CI / integration_tests (pull_request) Successful in 4m22s
CI / e2e_tests (pull_request) Successful in 4m39s
CI / unit_tests (pull_request) Successful in 9m51s
CI / coverage (pull_request) Successful in 13m30s
CI / docker (pull_request) Successful in 1m23s
CI / status-check (pull_request) Successful in 2s
Implement spec-compliant correction diff output for `agents plan diff
--correction <CORRECTION_ATTEMPT_ID>`. Fixes the following issues from
the cycle-1 PR review:

- C1/M1: Replace direct `unit_of_work.correction_attempts` access with
  the proper `unit_of_work.transaction()` context manager, eliminating
  the AttributeError crash and the resource (session) leak.
- C2: Add `unit_of_work: UnitOfWork | None = None` constructor parameter
  to `PlanApplyService` and wire it in `_get_apply_service()` via
  `container.unit_of_work()`, removing the illegal `get_container()`
  call inside the method body (ADR-003 DI violation).
- C3: Replace metadata serialization stub with a three-section structured
  diff (Correction Diff summary, Comparison table, Patch Preview) as
  specified in §agents plan diff of the specification.
- C4/M2: Add `features/plan_correction_diff.feature` with 6 BDD
  scenarios covering all 4 output formats plus plan-not-found and
  correction-not-found error paths.
- C5: Update the three existing BDD scenarios that tested old stub
  behavior to mock `_get_apply_service()` and assert the new output.
- C6: Rename branch to `bugfix/m4-plan-diff-correction-stub` per
  CONTRIBUTING.md convention.
- C7: Amend commit message with body and ISSUES CLOSED footer.
- C8: Narrow `except Exception` to `except CorrectionAttemptNotFoundError`
  to avoid masking programming errors.
- M3: Add `robot/plan_correction_diff.robot` and
  `robot/helper_plan_correction_diff.py` integration test covering rich,
  plain, and JSON formats and the not-found error path.
- M4: Type `_build_correction_diff_dict` parameter as
  `CorrectionAttemptRecord` instead of `Any`.
- M5: Change `fmt: str` to `fmt: Literal["rich", "plain", "json", "yaml"]`
  on both `diff()` and `correction_diff()`, with a `cast()` call in the
  CLI layer where Typer supplies a plain `str`.
- M6: Add `ValueError` guards for empty `plan_id` and
  `correction_attempt_id` at the top of `correction_diff()`.
- M7: Add blank line between `diff()` and `correction_diff()` method
  definitions.
- M8: Update PR description to reflect actual implementation.
- m1: Remove unused `plan` variable in `correction_diff()`.
- m2: Reduce three blank lines to two between top-level definitions in
  `plan_apply_service.py`.
- n1: Remove trailing whitespace from blank line in `plan.py`.

Quality gates: lint (ruff), typecheck (pyright strict), unit_tests
(Behave 632 features / 0 failures) all pass.

ISSUES CLOSED: #9085
2026-04-17 08:34:33 +00:00
freemo 48cff5cfe0 refactor(cli): rename plan lifecycle-list and lifecycle-apply to match specification
CI / build (push) Successful in 18s
CI / lint (push) Failing after 31s
CI / helm (push) Successful in 33s
CI / typecheck (push) Successful in 50s
CI / security (push) Failing after 51s
CI / coverage (push) Has been skipped
CI / benchmark-regression (push) Has been skipped
CI / unit_tests (push) Failing after 1m50s
CI / docker (push) Has been skipped
CI / quality (push) Successful in 3m43s
CI / integration_tests (push) Has been cancelled
CI / e2e_tests (push) Has been cancelled
CI / benchmark-publish (push) Has been cancelled
CI / status-check (push) Has been cancelled
Renames `plan lifecycle-list` to `plan list` and `plan lifecycle-apply` to `plan apply` to align with the specification's canonical command names. Removes legacy V2 plan commands that occupied those names.

- Renamed CLI command registrations from lifecycle-list/lifecycle-apply to list/apply
- Removed legacy V2 apply and list commands (~200 lines)
- Updated apply shortcut in main.py to delegate to v3 lifecycle
- Added defensive null check for plan existence in apply command
- Updated 63+ test, doc, and benchmark files for consistency

Closes #881

Co-authored-by: Jeffrey Phillips Freeman <the@jeffreyfreeman.me>
Co-committed-by: Jeffrey Phillips Freeman <the@jeffreyfreeman.me>
2026-04-02 19:09:04 +00:00
freemo 3a2b134f3c test(coverage): add Behave scenarios for remaining under-tested modules
CI / lint (pull_request) Successful in 14s
CI / benchmark-publish (pull_request) Has been skipped
CI / quality (pull_request) Successful in 27s
CI / typecheck (pull_request) Successful in 37s
CI / security (pull_request) Successful in 39s
CI / build (pull_request) Successful in 29s
CI / integration_tests (pull_request) Successful in 5m51s
CI / benchmark-regression (pull_request) Successful in 22m39s
CI / unit_tests (pull_request) Successful in 29m35s
CI / docker (pull_request) Successful in 13s
CI / coverage (pull_request) Successful in 2h3m1s
CI / lint (push) Successful in 21s
CI / typecheck (push) Successful in 57s
CI / security (push) Successful in 51s
CI / quality (push) Successful in 27s
CI / integration_tests (push) Successful in 4m59s
CI / build (push) Successful in 23s
CI / benchmark-publish (push) Successful in 14m39s
CI / benchmark-regression (push) Has been skipped
CI / unit_tests (push) Successful in 29m48s
CI / docker (push) Successful in 15s
CI / coverage (push) Successful in 1h45m42s
Added Behave BDD feature files and step definitions targeting coverage
gaps in six modules:

- container.py: exercise get_database_url env-var fallback, AI provider
  None path, cached container singleton, override_providers edge cases
  (lines 66-69, 125-130; branches at 51, 57, 82, 87, 256, 284-288)
- correction_service.py: exercise exception-handling paths in
  execute_revert and execute_append via monkeypatched analyze_impact
  and ULID failures (lines 254-262, 320-328)
- plan_lifecycle_service.py: exercise _persisted UoW commit paths,
  InvalidPhaseTransitionError custom message branch, non-reusable
  action archive, and error_details merge logic (branches at 100,
  216, 237, 327, 461, 570, 576, 607)
- plan.py (CLI): exercise spec-dict optional field branches,
  _print_lifecycle_plan conditional rendering, use_action argument
  parsing, auto-resolve paths, legacy wrappers, and validation
  error branches across 66 scenarios
- skill.py (CLI): exercise singleton cache, timestamp-absent show,
  no-tools MCP, add/remove/list/show format and error branches
  across 26 scenarios
- models.py (DB): exercise to_domain/from_domain None-field branches
  in SkillModel, SessionModel, ToolModel, LifecycleActionModel,
  LifecyclePlanModel, NamespacedProjectModel, and SessionMessageModel
  across 41 scenarios

All 302 features, 6503 scenarios, 28271 steps pass (nox -e unit_tests).

ISSUES CLOSED: #446
2026-02-25 19:38:43 -05:00